[Intel-gfx] [RFC 2/2] drm/i915: Add additional check for 480Mhz step CDCLK

[Borah, Chaitanya Kumar]

Hello Jani,

> -----Original Message-----
> From: Jani Nikula <jani.nikula at linux.intel.com>
> Sent: Wednesday, November 30, 2022 2:08 PM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: Syrjala, Ville <ville.syrjala at intel.com>
> Subject: Re: [Intel-gfx] [RFC 2/2] drm/i915: Add additional check for 480Mhz
> step CDCLK
> 
> On Wed, 30 Nov 2022, Chaitanya Kumar Borah
> <chaitanya.kumar.borah at intel.com> wrote:
> > There are still RPL-U boards which does not support the 480Mhz step of
> > CDCLK. We can differentiate these board by checking the CPUID Brand
> > String. 480Mhz step is only supported in SKUs which does not contain
> > the string "Genuine Intel" in the Brand string.
> >
> > BSpec: 55409
> >
> > Signed-off-by: Chaitanya Kumar Borah
> <chaitanya.kumar.borah at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 9bfeb1abba47..1890e5135cfc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -192,6 +192,19 @@ static bool is_rplu(struct drm_i915_private
> *dev_priv)
> >  	}
> >  }
> >
> > +static bool is_480mhz_step_valid(void) {
> > +	struct cpuinfo_x86 *c;
> > +	unsigned int cpu = smp_processor_id();
> > +
> > +	c = &cpu_data(cpu);
> > +
> > +	if (c->x86_model_id[0] && !strstr(c->x86_model_id, "Genuine Intel"))
> > +		return true;
> 
> Ugh, this is super ugly.
> 
> The usual way to quirk this stuff is in display/intel_quirks.c. There are two
> kinds of quirks, device and dmi. (And I realize that's one place where we do
> have PCI IDs written, but it's for slightly different
> purpose.)
> 
> If this really can't be done using quirks, and cpuinfo is the only way (I doubt
> it), then we need to add the cpuinfo quirk to intel_quirks.c and not sprinkle
> these around.

This change has now been moved to quirk. Since we are having this discussion, first I would like to get your feedback
on upstreaming this change. Do you see value in making this distinction between ES and QS parts or should we not care
at all since ES parts will be deprecated in near future?

In case we decide to keep this, let me know if the new version of the change is more acceptable?

Regards

Chaitanya

> 
> BR,
> Jani.
> 
> 
> > +
> > +	return false;
> > +}
> > +
> >  static void i915gm_get_cdclk(struct drm_i915_private *dev_priv,
> >  			     struct intel_cdclk_config *cdclk_config)  { @@ -
> 3389,8
> > +3402,9 @@ void intel_init_cdclk_hooks(struct drm_i915_private
> *dev_priv)
> >  		/*
> >  		 * BSpec: 55409
> >  		 * 480 MHz supported on SKUs that have a RPL-U Device ID
> > +		 * and  CPUID Brand String that does not contain "Genuine
> Intel".
> >  		 */
> > -		else if (is_rplu(dev_priv))
> > +		else if (is_rplu(dev_priv) && is_480mhz_step_valid())
> >  			dev_priv->cdclk.table = rplu_cdclk_table;
> >  		else
> >  			dev_priv->display.cdclk.table = adlp_cdclk_table;
> 
> --
> Jani Nikula, Intel Open Source Graphics Center