| Message ID | 20221130074617.1619099-3-chaitanya.kumar.borah@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add new CDCLK step for RPL-U | expand |
On Wed, 30 Nov 2022, Chaitanya Kumar Borah <chaitanya.kumar.borah@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@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. 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;
Hello Jani, > -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Wednesday, November 30, 2022 2:08 PM > To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Syrjala, Ville <ville.syrjala@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@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@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
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; + + 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;
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@intel.com> --- drivers/gpu/drm/i915/display/intel_cdclk.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)