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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Aug 20, 2019 at 01:22:00PM +0000, Kahola, Mika wrote:
> 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@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?

There is nothing to implement. The current policy already picks the
minimum cdclk that will work.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx