[Intel-gfx] [PATCH] drm/i915/tgl: Lower cdclk for sub 4k resolutions

[Kahola, Mika]

On Tue, 2019-08-20 at 16:03 +0300, Ville Syrjälä wrote:
> On Tue, Aug 20, 2019 at 02:06:31PM +0300, Mika Kahola wrote:
> > 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 at 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;
> 
> The maximum is just that, the maximum. It doesn't affect the actual
> cdclk chosen (outside of rejecting everything exceeding the max).
> And the maximum won't ever change, so trying to calculate it based
> on some ephemeral crtc states doesn't make sense.
> 
> Given that our policy is to always go for the minimum acceptable
> cdclk
> frequency I don't think there is any work to be done to get proper
> power savings for <4k. What is the actual problem you're seeing?
The actual problem is that this is a requested feature for TGL. I
admit, with these suggested optimizations the gains will be marginal.

My interpretation of this feature was that we should not exceed
172.8MHz with the sub 4k modes, hence I'm suggesting in this patch to limit the max cdclock to this number. 

So, how do we get forward? Should I propose that we drop this feature
or should we implement this differently?

> 
> > +	} else if (IS_ELKHARTLAKE(dev_priv)) {
> >  		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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
>