Re: [Intel-gfx] [PATCH 9/9] drm/i915: s/enable/active/ for DRRS

["Souza, Jose" <jose.souza@intel.com>]

On Tue, 2022-03-15 at 22:23 +0200, Ville Syrjälä wrote:
> On Tue, Mar 15, 2022 at 06:54:21PM +0000, Souza, Jose wrote:
> > On Tue, 2022-03-15 at 15:27 +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Rename the DRRS functiosn to say "(de)activate" rather than
> > > "enable/disable". This let's us differentiate between the
> > > logically enabled vs. actually currently active cases.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c  |  4 ++--
> > >  .../drm/i915/display/intel_display_debugfs.c  | 15 ++++++++-----
> > >  drivers/gpu/drm/i915/display/intel_drrs.c     | 22 ++++++++++---------
> > >  drivers/gpu/drm/i915/display/intel_drrs.h     |  6 ++---
> > >  4 files changed, 27 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 86fc8ddd0b8f..90d54281535d 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -1246,7 +1246,7 @@ static void intel_post_plane_update(struct intel_atomic_state *state,
> > >  	    !needs_cursorclk_wa(new_crtc_state))
> > >  		icl_wa_cursorclkgating(dev_priv, pipe, false);
> > >  
> > > -	intel_drrs_enable(new_crtc_state);
> > > +	intel_drrs_activate(new_crtc_state);
> > >  }
> > >  
> > >  static void intel_crtc_enable_flip_done(struct intel_atomic_state *state,
> > > @@ -1324,7 +1324,7 @@ static void intel_pre_plane_update(struct intel_atomic_state *state,
> > >  		intel_atomic_get_new_crtc_state(state, crtc);
> > >  	enum pipe pipe = crtc->pipe;
> > >  
> > > -	intel_drrs_disable(old_crtc_state);
> > > +	intel_drrs_deactivate(old_crtc_state);
> > >  
> > >  	intel_psr_pre_plane_update(state, crtc);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > index e0a126e7ebb8..18b98788b23e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > @@ -1159,6 +1159,9 @@ static int i915_drrs_status(struct seq_file *m, void *unused)
> > >  	seq_puts(m, "\n");
> > >  
> > >  	for_each_intel_crtc(&dev_priv->drm, crtc) {
> > > +		const struct intel_crtc_state *crtc_state =
> > > +			to_intel_crtc_state(crtc->base.state);
> > > +
> > >  		seq_printf(m, "[CRTC:%d:%s]:\n",
> > >  			   crtc->base.base.id, crtc->base.name);
> > >  
> > > @@ -1166,7 +1169,10 @@ static int i915_drrs_status(struct seq_file *m, void *unused)
> > >  
> > >  		/* DRRS Supported */
> > >  		seq_printf(m, "\tDRRS Enabled: %s\n",
> > > -			   str_yes_no(intel_drrs_is_enabled(crtc)));
> > > +			   str_yes_no(crtc_state->has_drrs));
> > > +
> > > +		seq_printf(m, "\tDRRS Active: %s\n",
> > > +			   str_yes_no(intel_drrs_is_active(crtc)));
> > 
> > So there is no way to know if the worker thread have activated the downclock mode?
> > I prefer to have the "activated" as meaning that the downlock is in use.
> 
> The "DRRS refresh rate" print shows if we're actually running at
> high or low refresh rate right now.
> 


Okay, reading it again makes sense to have 'enabled' when has_drrs is set and 'active' when feature is armed.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>