Re: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions

["Kahola, Mika" <mika.kahola@intel.com>]

On Tue, 2019-08-20 at 19:05 +0530, Shankar, Uma wrote:
> > -----Original Message-----
> > From: Kahola, Mika
> > Sent: Tuesday, August 20, 2019 4:37 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Shankar, Uma <uma.shankar@intel.com>; Kahola, Mika
> > <mika.kahola@intel.com>
> > Subject: [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions
> > 
> > In order to achieve improved power savings we can tune down CD
> > clock frequency for
> > sub 4k resolutions. The maximum CD clock frequency for sub 4k
> > resolutions is set to
> > 172.8 MHz.
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_cdclk.c | 26
> > +++++++++++++++++++++-
> > drivers/gpu/drm/i915/display/intel_cdclk.h |  3 +++
> > 2 files changed, 28 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index d0bc42e5039c..1d6c7bc79470 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -2610,6 +2610,24 @@ static int intel_compute_max_dotclk(struct
> > drm_i915_private *dev_priv)
> > 		return max_cdclk_freq*90/100;
> > }
> > 
> > +bool mode_is_4k(struct drm_i915_private *dev_priv) {
> > +	struct intel_crtc *crtc;
> > +	struct intel_crtc_state *pipe_config;
> > +
> > +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> > +		pipe_config = to_intel_crtc_state(crtc->base.state);
> > +
> > +		if (pipe_config->base.active) {
> > +			if (pipe_config->pipe_src_w >= WIDTH_4K &&
> > +			    pipe_config->pipe_src_h >= HEIGHT_4K)
> > +				return true;
> > +		}
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > /**
> >  * intel_update_max_cdclk - Determine the maximum support CDCLK
> > frequency
> >  * @dev_priv: i915 device
> > @@ -2620,7 +2638,13 @@ static int intel_compute_max_dotclk(struct
> > drm_i915_private *dev_priv)
> >  */
> > void intel_update_max_cdclk(struct drm_i915_private *dev_priv)  {
> > -	if (IS_ELKHARTLAKE(dev_priv)) {
> > +	/*
> > +	 * Use lower CDCLK frequency on Tigerlake when selected
> > +	 * mode is less than 4k.
> > +	 */
> > +	if (INTEL_GEN(dev_priv) >= 12 && !mode_is_4k(dev_priv)) {
> > +		dev_priv->max_cdclk_freq = 172800;
> > +	} else if (IS_ELKHARTLAKE(dev_priv)) {
> 
> Setting the max cd clock supported itself to a lower value is not a
> good idea.
> This should return what is the max frequency of cd clock the hardware
> supports.
> Driver is not going to program this to max based on this data.
> 
> Actual cd clock which will be programmed in hardware should be based
> on the
> maximum pixel clock we are driving. So if we are not driving 4K and
> say at 1920x1080, pixel
> clock will be less and we should be selecting a lower value of cd
> clock in that case which
> gets programmed in CDCLK_CTL. But max cd clock still remains what
> maximum the platform can
> support. You can check intel_compute_min_cdclk which calculates the
> minimum cd clock required.
> 
> So when we just have 1920x1080@60Hz pixel clock of 148500, 
> min cd clock required should be 148500 and nearest higher value of CD
> Clock will be 172.8Mhz.
> Similarly for 4096x2160@60, pixel clock 556188, min cd clock
> calculated will be 556.188 MHz, thus
> getting nearest supported cd clock value of 556 or 648Mhz.
> 
> This should be taken care by this logic in driver. 
> 
> Also with the current patch, modes like 2560x1600@60, pixel clock
> 268.5MHz will not work. This will need
> cd clock of 324Mhz (or 307, 312 whatever are supported on the
> platform. 
Right. The dev_priv->max_cdclk_freq was HW max cdclock. We don't want
to mess with that. Since we already pick up the minimum cdclock, the
feature is already built-in cd clock selection process. Thanks for
clarification!

I'll propose that we drop this task since its obsolete.

> 
> > 		if (dev_priv->cdclk.hw.ref == 24000)
> > 			dev_priv->max_cdclk_freq = 552000;
> > 		else
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > index 4d6f7f5f8930..cefb5146ddca 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > @@ -15,6 +15,9 @@ struct intel_atomic_state;  struct
> > intel_cdclk_state;  struct
> > intel_crtc_state;
> > 
> > +#define WIDTH_4K  3860
> > +#define HEIGHT_4K 2160
> > +
> > int intel_crtc_compute_min_cdclk(const struct intel_crtc_state
> > *crtc_state);  void
> > intel_cdclk_init(struct drm_i915_private *i915);  void
> > intel_cdclk_uninit(struct
> > drm_i915_private *i915);
> > --
> > 2.17.1
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx