[Intel-gfx] [PATCH 6/6] drm/i915/display: Dump the new cdclk config values

[Srivatsa, Anusha]

> -----Original Message-----
> From: Jani Nikula <jani.nikula at linux.intel.com>
> Sent: Tuesday, September 20, 2022 12:27 AM
> To: Srivatsa, Anusha <anusha.srivatsa at intel.com>; intel-
> gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 6/6] drm/i915/display: Dump the new cdclk
> config values
> 
> On Fri, 16 Sep 2022, Anusha Srivatsa <anusha.srivatsa at intel.com> wrote:
> > Add a helper function to get stringify values of the desired cdclk
> > action and dump it with rest of the cdclk config values
> >
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index bc627daade3e..12f5e4d23245 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -1688,6 +1688,19 @@ static u32 cdclk_squash_waveform(struct
> > drm_i915_private *dev_priv,
> >
> >  	return 0xffff;
> >  }
> > +static const char *cdclk_sequence_to_string(enum cdclk_sequence
> > +cdclk_sequence)
> 
> For any enum foobar, this would be customarily called foobar_name(), i.e.
> cdclk_sequence_name(). (Though the enum should probably be renamed as
> mentioned earlier.)

Will do. The initial change and then change here.

> > +{
> > +	switch (cdclk_sequence) {
> > +	case CDCLK_SQUASH_ONLY:
> > +		return "Squash only";
> > +	case CDCLK_CRAWL_ONLY:
> > +		return "Crawl only";
> > +	case CDCLK_LEGACY:
> > +		return "Legacy method";
> > +	default:
> > +		return "Not a valid cdclk sequence";
> > +	}
> > +}
> >
> >  static void dg2_prog_squash_ctl(struct drm_i915_private *i915, u16
> > waveform)  { @@ -2083,10 +2096,11 @@ void
> > intel_cdclk_dump_config(struct drm_i915_private *i915,
> >  			     const struct intel_cdclk_config *cdclk_config,
> >  			     const char *context)
> >  {
> > -	drm_dbg_kms(&i915->drm, "%s %d kHz, VCO %d kHz, ref %d kHz,
> bypass %d kHz, voltage level %d\n",
> > +	drm_dbg_kms(&i915->drm, "%s %d kHz, VCO %d kHz, ref %d kHz,
> bypass
> > +%d kHz, voltage level %d, %s action\n",
> 
> "%s action" leads to:
> 
> Squash only action
> Crawl only action
> Legacy method action
> Not a valid cdclk sequence action
> 
> A bit odd perhaps. *shrug*

Yeah now I see it.

Will thin over what can be a better replacement.

Anusha

> BR,
> Jani.
> 
> >  		    context, cdclk_config->cdclk, cdclk_config->vco,
> >  		    cdclk_config->ref, cdclk_config->bypass,
> > -		    cdclk_config->voltage_level);
> > +		    cdclk_config->voltage_level,
> > +		    cdclk_sequence_to_string(cdclk_config->steps->action));
> >  }
> >
> >  /**
> 
> --
> Jani Nikula, Intel Open Source Graphics Center