[Intel-gfx] [RFC 0/4] Add new CDCLK step for RPL-U

[Intel-gfx] [RFC 0/4] Add new CDCLK step for RPL-U

[Chaitanya Kumar Borah]

A new step of 480MHz has been added on SKUs that have a RPL-U
device id. This particular step is to support 120Hz panels
more efficiently.

This patchset adds a new table to include this new CDCLK
step. Details can be found in BSpec entry 55409.

RPL-U device ids are currently added within the RPL-P sub
platform. It seems to be an overkill to add a separate sub
platform just to support this change. Therefore, quirks
are a good way to achieve this.

In addition to identifying RPL-U device id, we need to make a
distinction between ES and QS parts as this change comes only to
QS parts. CPUID Brand string is the only way to make this distinction
currently. ES parts have "Genuine Intel" in their brand string
while QS parts have a more specific brand string, for ex.
"13th Gen Intel(R) Core(TM) i5-1345U". Therefore, 480Mhz step is only
supported in SKUs which does not contain the string "Genuine Intel" in
the Brand string.

The patch "drm/i915: Apply CDCLK quirk only on QS parts" adds this
change. We have separated this patch because we request feedback
from the community if this change needs to be upstreamed or not
as ES parts will be deprecated in future. Feedbacks are welcome.

Chaitanya Kumar Borah (4):
  drm/i915/quirks: Add quirk for 480MHz CDCLK step
  drm/i915/display: Add 480 MHz CDCLK steps for RPL-U
  drm/i915: Initialize intel quirks before CDCLK initialization
  drm/i915: Apply CDCLK quirk only on QS parts

 drivers/gpu/drm/i915/display/intel_cdclk.c   | 25 ++++++++++++
 drivers/gpu/drm/i915/display/intel_display.c |  2 -
 drivers/gpu/drm/i915/display/intel_quirks.c  | 40 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_quirks.h  |  1 +
 drivers/gpu/drm/i915/i915_driver.c           |  2 +
 5 files changed, 68 insertions(+), 2 deletions(-)

-- 
2.25.1

[Intel-gfx] [RFC 1/4] drm/i915/quirks: Add quirk for 480MHz CDCLK step

[Chaitanya Kumar Borah]

A new CDCLK step of 480MHz has been added on SKUs that has a
RPL-U device id. This is done to support 120Hz displays with
more efficiency.

RPL-U device ids are currently added within the RPL-P sub
platform. It seems to be an overkill to add a separate sub
platform just to support this change. Therefore, quirks
are a good way to achieve the same.

BSpec: 55409

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_quirks.c | 14 ++++++++++++++
 drivers/gpu/drm/i915/display/intel_quirks.h |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
index 6e48d3bcdfec..0a30499835b3 100644
--- a/drivers/gpu/drm/i915/display/intel_quirks.c
+++ b/drivers/gpu/drm/i915/display/intel_quirks.c
@@ -65,6 +65,16 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
 	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");
 }
 
+/*
+ * A new step of 480MHz has been added on SKUs that have a RPL-U device id.
+ * This particular step is to better support 120Hz panels.
+ */
+static void quirk_480mhz_cdclk_step_hook(struct drm_i915_private *i915)
+{
+	intel_set_quirk(i915, QUIRK_480MHZ_CDCLK_STEP);
+	drm_info(&i915->drm, "Applying 480MHz CDCLK step quirk\n");
+}
+
 struct intel_quirk {
 	int device;
 	int subsystem_vendor;
@@ -199,6 +209,10 @@ static struct intel_quirk intel_quirks[] = {
 	/* ECS Liva Q2 */
 	{ 0x3185, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
 	{ 0x3184, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
+	/* RPL-U */
+	{ 0xA7A1, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
+	{ 0xA721, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
+	{ 0xA7A9, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
 };
 
 void intel_init_quirks(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/display/intel_quirks.h b/drivers/gpu/drm/i915/display/intel_quirks.h
index 10a4d163149f..71e05684f5f4 100644
--- a/drivers/gpu/drm/i915/display/intel_quirks.h
+++ b/drivers/gpu/drm/i915/display/intel_quirks.h
@@ -17,6 +17,7 @@ enum intel_quirk_id {
 	QUIRK_INVERT_BRIGHTNESS,
 	QUIRK_LVDS_SSC_DISABLE,
 	QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK,
+	QUIRK_480MHZ_CDCLK_STEP,
 };
 
 void intel_init_quirks(struct drm_i915_private *i915);
-- 
2.25.1

Re: [Intel-gfx] [RFC 1/4] drm/i915/quirks: Add quirk for 480MHz CDCLK step

[Jani Nikula]

On Mon, 02 Jan 2023, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> wrote:
> A new CDCLK step of 480MHz has been added on SKUs that has a
> RPL-U device id. This is done to support 120Hz displays with
> more efficiency.
>
> RPL-U device ids are currently added within the RPL-P sub
> platform. It seems to be an overkill to add a separate sub
> platform just to support this change. Therefore, quirks
> are a good way to achieve the same.

The thing is, this part is *not* a quirk. It's basic enabling for RPL-U.

If you start conflating quirks and basic enabling to avoid overkill,
you're eventually going to end up in all kinds of trouble with
maintenance.


BR,
Jani.

>
> BSpec: 55409
>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_quirks.c | 14 ++++++++++++++
>  drivers/gpu/drm/i915/display/intel_quirks.h |  1 +
>  2 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
> index 6e48d3bcdfec..0a30499835b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.c
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
> @@ -65,6 +65,16 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
>  	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");
>  }
>  
> +/*
> + * A new step of 480MHz has been added on SKUs that have a RPL-U device id.
> + * This particular step is to better support 120Hz panels.
> + */
> +static void quirk_480mhz_cdclk_step_hook(struct drm_i915_private *i915)
> +{
> +	intel_set_quirk(i915, QUIRK_480MHZ_CDCLK_STEP);
> +	drm_info(&i915->drm, "Applying 480MHz CDCLK step quirk\n");
> +}
> +
>  struct intel_quirk {
>  	int device;
>  	int subsystem_vendor;
> @@ -199,6 +209,10 @@ static struct intel_quirk intel_quirks[] = {
>  	/* ECS Liva Q2 */
>  	{ 0x3185, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
>  	{ 0x3184, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
> +	/* RPL-U */
> +	{ 0xA7A1, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
> +	{ 0xA721, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
> +	{ 0xA7A9, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
>  };
>  
>  void intel_init_quirks(struct drm_i915_private *i915)
> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.h b/drivers/gpu/drm/i915/display/intel_quirks.h
> index 10a4d163149f..71e05684f5f4 100644
> --- a/drivers/gpu/drm/i915/display/intel_quirks.h
> +++ b/drivers/gpu/drm/i915/display/intel_quirks.h
> @@ -17,6 +17,7 @@ enum intel_quirk_id {
>  	QUIRK_INVERT_BRIGHTNESS,
>  	QUIRK_LVDS_SSC_DISABLE,
>  	QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK,
> +	QUIRK_480MHZ_CDCLK_STEP,
>  };
>  
>  void intel_init_quirks(struct drm_i915_private *i915);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [RFC 1/4] drm/i915/quirks: Add quirk for 480MHz CDCLK step

[Jani Nikula]

On Mon, 02 Jan 2023, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 02 Jan 2023, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> wrote:
>> A new CDCLK step of 480MHz has been added on SKUs that has a
>> RPL-U device id. This is done to support 120Hz displays with
>> more efficiency.
>>
>> RPL-U device ids are currently added within the RPL-P sub
>> platform. It seems to be an overkill to add a separate sub
>> platform just to support this change. Therefore, quirks
>> are a good way to achieve the same.
>
> The thing is, this part is *not* a quirk. It's basic enabling for RPL-U.
>
> If you start conflating quirks and basic enabling to avoid overkill,
> you're eventually going to end up in all kinds of trouble with
> maintenance.

Please start off with adding RPL-U as a subplatform, and let's worry
about the CDCLK after that.

BR,
Jani.

>
>
> BR,
> Jani.
>
>>
>> BSpec: 55409
>>
>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_quirks.c | 14 ++++++++++++++
>>  drivers/gpu/drm/i915/display/intel_quirks.h |  1 +
>>  2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
>> index 6e48d3bcdfec..0a30499835b3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_quirks.c
>> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
>> @@ -65,6 +65,16 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
>>  	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");
>>  }
>>  
>> +/*
>> + * A new step of 480MHz has been added on SKUs that have a RPL-U device id.
>> + * This particular step is to better support 120Hz panels.
>> + */
>> +static void quirk_480mhz_cdclk_step_hook(struct drm_i915_private *i915)
>> +{
>> +	intel_set_quirk(i915, QUIRK_480MHZ_CDCLK_STEP);
>> +	drm_info(&i915->drm, "Applying 480MHz CDCLK step quirk\n");
>> +}
>> +
>>  struct intel_quirk {
>>  	int device;
>>  	int subsystem_vendor;
>> @@ -199,6 +209,10 @@ static struct intel_quirk intel_quirks[] = {
>>  	/* ECS Liva Q2 */
>>  	{ 0x3185, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
>>  	{ 0x3184, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
>> +	/* RPL-U */
>> +	{ 0xA7A1, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
>> +	{ 0xA721, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
>> +	{ 0xA7A9, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook },
>>  };
>>  
>>  void intel_init_quirks(struct drm_i915_private *i915)
>> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.h b/drivers/gpu/drm/i915/display/intel_quirks.h
>> index 10a4d163149f..71e05684f5f4 100644
>> --- a/drivers/gpu/drm/i915/display/intel_quirks.h
>> +++ b/drivers/gpu/drm/i915/display/intel_quirks.h
>> @@ -17,6 +17,7 @@ enum intel_quirk_id {
>>  	QUIRK_INVERT_BRIGHTNESS,
>>  	QUIRK_LVDS_SSC_DISABLE,
>>  	QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK,
>> +	QUIRK_480MHZ_CDCLK_STEP,
>>  };
>>  
>>  void intel_init_quirks(struct drm_i915_private *i915);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [RFC 1/4] drm/i915/quirks: Add quirk for 480MHz CDCLK step

[Borah, Chaitanya Kumar]

Hello Jani,

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Monday, January 2, 2023 3:34 PM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Deak, Imre <imre.deak@intel.com>; Kahola, Mika
> <mika.kahola@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Syrjala, Ville <ville.syrjala@intel.com>; Srivatsa, Anusha
> <anusha.srivatsa@intel.com>; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com>
> Subject: Re: [RFC 1/4] drm/i915/quirks: Add quirk for 480MHz CDCLK step
> 
> On Mon, 02 Jan 2023, Chaitanya Kumar Borah
> <chaitanya.kumar.borah@intel.com> wrote:
> > A new CDCLK step of 480MHz has been added on SKUs that has a RPL-U
> > device id. This is done to support 120Hz displays with more
> > efficiency.
> >
> > RPL-U device ids are currently added within the RPL-P sub platform. It
> > seems to be an overkill to add a separate sub platform just to support
> > this change. Therefore, quirks are a good way to achieve the same.
> 
> The thing is, this part is *not* a quirk. It's basic enabling for RPL-U.
> 
> If you start conflating quirks and basic enabling to avoid overkill, you're
> eventually going to end up in all kinds of trouble with maintenance.
> 

I have floated the latest version of the patch series with addition of a sub-platform for RPL-U. This version of the patch series does not make the ES/QS differentiation.

However, I have some doubts if RPL-U can be considered as a separate sub-platform as we interpret it now. For example, it does not show up as a different row in the "Stepping info" in Bspec.

Also I had to duplicate the revision ids for it from RPL-P, for which there is no precedence yet. Nevertheless, it could be a better way to do it than quirks.

+	} else if (IS_ADLP_RPLU(i915)) {
+		revids = adlp_rplp_revids;
+		size = ARRAY_SIZE(adlp_rplp_revids);

Regards

Chaitanya

> 
> BR,
> Jani.
> 
> >
> > BSpec: 55409
> >
> > Signed-off-by: Chaitanya Kumar Borah
> <chaitanya.kumar.borah@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_quirks.c | 14 ++++++++++++++
> > drivers/gpu/drm/i915/display/intel_quirks.h |  1 +
> >  2 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c
> > b/drivers/gpu/drm/i915/display/intel_quirks.c
> > index 6e48d3bcdfec..0a30499835b3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_quirks.c
> > +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
> > @@ -65,6 +65,16 @@ static void
> quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
> >  	drm_info(&i915->drm, "Applying no pps backlight power quirk\n");  }
> >
> > +/*
> > + * A new step of 480MHz has been added on SKUs that have a RPL-U
> device id.
> > + * This particular step is to better support 120Hz panels.
> > + */
> > +static void quirk_480mhz_cdclk_step_hook(struct drm_i915_private
> > +*i915) {
> > +	intel_set_quirk(i915, QUIRK_480MHZ_CDCLK_STEP);
> > +	drm_info(&i915->drm, "Applying 480MHz CDCLK step quirk\n"); }
> > +
> >  struct intel_quirk {
> >  	int device;
> >  	int subsystem_vendor;
> > @@ -199,6 +209,10 @@ static struct intel_quirk intel_quirks[] = {
> >  	/* ECS Liva Q2 */
> >  	{ 0x3185, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
> >  	{ 0x3184, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
> > +	/* RPL-U */
> > +	{ 0xA7A1, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook
> },
> > +	{ 0xA721, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook
> },
> > +	{ 0xA7A9, PCI_ANY_ID, PCI_ANY_ID, quirk_480mhz_cdclk_step_hook
> },
> >  };
> >
> >  void intel_init_quirks(struct drm_i915_private *i915) diff --git
> > a/drivers/gpu/drm/i915/display/intel_quirks.h
> > b/drivers/gpu/drm/i915/display/intel_quirks.h
> > index 10a4d163149f..71e05684f5f4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_quirks.h
> > +++ b/drivers/gpu/drm/i915/display/intel_quirks.h
> > @@ -17,6 +17,7 @@ enum intel_quirk_id {
> >  	QUIRK_INVERT_BRIGHTNESS,
> >  	QUIRK_LVDS_SSC_DISABLE,
> >  	QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK,
> > +	QUIRK_480MHZ_CDCLK_STEP,
> >  };
> >
> >  void intel_init_quirks(struct drm_i915_private *i915);
> 
> --
> Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [RFC 2/4] drm/i915/display: Add 480 MHz CDCLK steps for RPL-U

[Chaitanya Kumar Borah]

A new step of 480MHz has been added on SKUs that have a RPL-U
device id to support 120Hz displays more efficiently. Use a
new quirk to identify the machine for which this change needs
to be applied.

BSpec: 55409

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 0c107a38f9d0..f5df0a806765 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -38,6 +38,7 @@
 #include "intel_pcode.h"
 #include "intel_psr.h"
 #include "vlv_sideband.h"
+#include "intel_quirks.h"
 
 /**
  * DOC: CDCLK / RAWCLK
@@ -1329,6 +1330,27 @@ static const struct intel_cdclk_vals adlp_cdclk_table[] = {
 	{}
 };
 
+static const struct intel_cdclk_vals rplu_cdclk_table[] = {
+	{ .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 },
+	{ .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 },
+	{ .refclk = 19200, .cdclk = 480000, .divider = 2, .ratio = 50 },
+	{ .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 },
+	{ .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 },
+
+	{ .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 },
+	{ .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 },
+	{ .refclk = 24000, .cdclk = 480000, .divider = 2, .ratio = 40 },
+	{ .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 },
+	{ .refclk = 24400, .cdclk = 648000, .divider = 2, .ratio = 54 },
+
+	{ .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 },
+	{ .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 },
+	{ .refclk = 38400, .cdclk = 480000, .divider = 2, .ratio = 25 },
+	{ .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 },
+	{ .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 },
+	{}
+};
+
 static const struct intel_cdclk_vals dg2_cdclk_table[] = {
 	{ .refclk = 38400, .cdclk = 163200, .divider = 2, .ratio = 34, .waveform = 0x8888 },
 	{ .refclk = 38400, .cdclk = 204000, .divider = 2, .ratio = 34, .waveform = 0x9248 },
@@ -3353,6 +3375,9 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 		/* Wa_22011320316:adl-p[a0] */
 		if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0))
 			dev_priv->display.cdclk.table = adlp_a_step_cdclk_table;
+		/* BSpec: 55409 */
+		else if (intel_has_quirk(dev_priv, QUIRK_480MHZ_CDCLK_STEP))
+			dev_priv->display.cdclk.table = rplu_cdclk_table;
 		else
 			dev_priv->display.cdclk.table = adlp_cdclk_table;
 	} else if (IS_ROCKETLAKE(dev_priv)) {
-- 
2.25.1

Re: [Intel-gfx] [RFC 2/4] drm/i915/display: Add 480 MHz CDCLK steps for RPL-U

[Jani Nikula]

On Mon, 02 Jan 2023, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> wrote:
> A new step of 480MHz has been added on SKUs that have a RPL-U
> device id to support 120Hz displays more efficiently. Use a
> new quirk to identify the machine for which this change needs
> to be applied.

Again, it's not a quirk, and should not be added as one.

BR,
Jani.

>
> BSpec: 55409
>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 0c107a38f9d0..f5df0a806765 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -38,6 +38,7 @@
>  #include "intel_pcode.h"
>  #include "intel_psr.h"
>  #include "vlv_sideband.h"
> +#include "intel_quirks.h"
>  
>  /**
>   * DOC: CDCLK / RAWCLK
> @@ -1329,6 +1330,27 @@ static const struct intel_cdclk_vals adlp_cdclk_table[] = {
>  	{}
>  };
>  
> +static const struct intel_cdclk_vals rplu_cdclk_table[] = {
> +	{ .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 },
> +	{ .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 },
> +	{ .refclk = 19200, .cdclk = 480000, .divider = 2, .ratio = 50 },
> +	{ .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 },
> +	{ .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 },
> +
> +	{ .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 },
> +	{ .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 },
> +	{ .refclk = 24000, .cdclk = 480000, .divider = 2, .ratio = 40 },
> +	{ .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 },
> +	{ .refclk = 24400, .cdclk = 648000, .divider = 2, .ratio = 54 },
> +
> +	{ .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 },
> +	{ .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 },
> +	{ .refclk = 38400, .cdclk = 480000, .divider = 2, .ratio = 25 },
> +	{ .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 },
> +	{ .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 },
> +	{}
> +};
> +
>  static const struct intel_cdclk_vals dg2_cdclk_table[] = {
>  	{ .refclk = 38400, .cdclk = 163200, .divider = 2, .ratio = 34, .waveform = 0x8888 },
>  	{ .refclk = 38400, .cdclk = 204000, .divider = 2, .ratio = 34, .waveform = 0x9248 },
> @@ -3353,6 +3375,9 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  		/* Wa_22011320316:adl-p[a0] */
>  		if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0))
>  			dev_priv->display.cdclk.table = adlp_a_step_cdclk_table;
> +		/* BSpec: 55409 */

We don't bother with bspec references in code. Add them in commit
messages.

> +		else if (intel_has_quirk(dev_priv, QUIRK_480MHZ_CDCLK_STEP))
> +			dev_priv->display.cdclk.table = rplu_cdclk_table;
>  		else
>  			dev_priv->display.cdclk.table = adlp_cdclk_table;
>  	} else if (IS_ROCKETLAKE(dev_priv)) {

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [RFC 3/4] drm/i915: Initialize intel quirks before CDCLK initialization

[Chaitanya Kumar Borah]

With addition of new quirk QUIRK_480MHZ_CDCLK_STEP, it is imperative
that quirks should be initialized before CDCLK initialization. Refactor
the code accordingly.

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 2 --
 drivers/gpu/drm/i915/i915_driver.c           | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e75b9b2a0e01..5c71fd83c25b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8666,8 +8666,6 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
 	INIT_WORK(&i915->display.atomic_helper.free_work,
 		  intel_atomic_helper_free_state_worker);
 
-	intel_init_quirks(i915);
-
 	intel_fbc_init(i915);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index c1e427ba57ae..4d1cb46f9863 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -104,6 +104,7 @@
 #include "intel_pm.h"
 #include "intel_region_ttm.h"
 #include "vlv_suspend.h"
+#include "display/intel_quirks.h"
 
 static const struct drm_driver i915_drm_driver;
 
@@ -388,6 +389,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	if (ret < 0)
 		goto err_gem;
 	intel_irq_init(dev_priv);
+	intel_init_quirks(dev_priv);
 	intel_init_display_hooks(dev_priv);
 	intel_init_clock_gating_hooks(dev_priv);
 
-- 
2.25.1

Re: [Intel-gfx] [RFC 3/4] drm/i915: Initialize intel quirks before CDCLK initialization

[Jani Nikula]

On Mon, 02 Jan 2023, Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> wrote:
> With addition of new quirk QUIRK_480MHZ_CDCLK_STEP, it is imperative
> that quirks should be initialized before CDCLK initialization. Refactor
> the code accordingly.

Any refactors here should improve the clarity between display and the
rest; this is going the opposite direction with calling display code
from common driver code.

BR,
Jani.


>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 2 --
>  drivers/gpu/drm/i915/i915_driver.c           | 2 ++
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e75b9b2a0e01..5c71fd83c25b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8666,8 +8666,6 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
>  	INIT_WORK(&i915->display.atomic_helper.free_work,
>  		  intel_atomic_helper_free_state_worker);
>  
> -	intel_init_quirks(i915);
> -
>  	intel_fbc_init(i915);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index c1e427ba57ae..4d1cb46f9863 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -104,6 +104,7 @@
>  #include "intel_pm.h"
>  #include "intel_region_ttm.h"
>  #include "vlv_suspend.h"
> +#include "display/intel_quirks.h"
>  
>  static const struct drm_driver i915_drm_driver;
>  
> @@ -388,6 +389,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>  	if (ret < 0)
>  		goto err_gem;
>  	intel_irq_init(dev_priv);
> +	intel_init_quirks(dev_priv);
>  	intel_init_display_hooks(dev_priv);
>  	intel_init_clock_gating_hooks(dev_priv);

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [RFC 4/4] drm/i915: Apply CDCLK quirk only on QS parts

[Chaitanya Kumar Borah]

RPL-U boards with ES silicon does not support the 480Mhz step of
CDCLK. To differentiate between QS and ES part CPU brand string
is the only feasible way as of now. ES parts have "Genuine Intel"
in their brand string while QS parts have a more specific brand
string, for ex. "13th Gen Intel(R) Core(TM) i5-1345U"

BSpec: 55409

Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---
 drivers/gpu/drm/i915/display/intel_quirks.c | 32 +++++++++++++++++++--
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
index 0a30499835b3..a6d7a2430626 100644
--- a/drivers/gpu/drm/i915/display/intel_quirks.c
+++ b/drivers/gpu/drm/i915/display/intel_quirks.c
@@ -14,6 +14,25 @@ static void intel_set_quirk(struct drm_i915_private *i915, enum intel_quirk_id q
 	i915->display.quirks.mask |= BIT(quirk);
 }
 
+/*
+ * To differentiate between QS and ES part CPU brand string is the only feasible way
+ * as of now. ES parts have "Genuine Intel" in their brand string while QS parts have a more
+ * specific brand string, for ex. "13th Gen Intel(R) Core(TM) i5-1345U"
+ */
+static bool is_QS_part(void)
+{
+	struct cpuinfo_x86 *c;
+	unsigned int cpu = get_cpu();
+
+	c = &cpu_data(cpu);
+	put_cpu();
+
+	if (c->x86_model_id[0] && !strstr(c->x86_model_id, "Genuine Intel"))
+		return true;
+
+	return false;
+}
+
 /*
  * Some machines (Lenovo U160) do not work with SSC on LVDS for some reason
  */
@@ -67,12 +86,19 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915)
 
 /*
  * A new step of 480MHz has been added on SKUs that have a RPL-U device id.
- * This particular step is to better support 120Hz panels.
+ * This particular step is to better support 120Hz panels. In addition to
+ * identifying RPL-U device id, we need to make a distinction between ES and
+ * QS parts as this change comes only to QS parts. For this CPUID Brand
+ * string is used. 480Mhz step is only supported in SKUs which does not
+ * contain the string "Genuine Intel" in the Brand string.
  */
+
 static void quirk_480mhz_cdclk_step_hook(struct drm_i915_private *i915)
 {
-	intel_set_quirk(i915, QUIRK_480MHZ_CDCLK_STEP);
-	drm_info(&i915->drm, "Applying 480MHz CDCLK step quirk\n");
+	if (is_QS_part()) {
+		intel_set_quirk(i915, QUIRK_480MHZ_CDCLK_STEP);
+		drm_info(&i915->drm, "Applying 480MHz CDCLK step quirk\n");
+	}
 }
 
 struct intel_quirk {
-- 
2.25.1

[Intel-gfx] ✗ Fi.CI.BAT: failure for Add new CDCLK step for RPL-U (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 3961 bytes --]

== Series Details ==

Series: Add new CDCLK step for RPL-U (rev2)
URL   : https://patchwork.freedesktop.org/series/111472/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12534 -> Patchwork_111472v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_111472v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_111472v2, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111472v2/index.html

Participating hosts (44 -> 40)
------------------------------

  Missing    (4): fi-kbl-soraka fi-rkl-11600 fi-bsw-kefka fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_111472v2:

### IGT changes ###

#### Possible regressions ####

  * igt@debugfs_test@read_all_entries:
    - fi-icl-u2:          [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12534/fi-icl-u2/igt@debugfs_test@read_all_entries.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111472v2/fi-icl-u2/igt@debugfs_test@read_all_entries.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - {bat-dg1-7}:        [SKIP][3] ([i915#4078]) -> [SKIP][4] +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12534/bat-dg1-7/igt@kms_flip@basic-flip-vs-wf_vblank.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111472v2/bat-dg1-7/igt@kms_flip@basic-flip-vs-wf_vblank.html

  
Known issues
------------

  Here are the changes found in Patchwork_111472v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@migrate:
    - bat-adlp-4:         [PASS][5] -> [DMESG-FAIL][6] ([i915#7699])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12534/bat-adlp-4/igt@i915_selftest@live@migrate.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111472v2/bat-adlp-4/igt@i915_selftest@live@migrate.html

  * igt@runner@aborted:
    - fi-icl-u2:          NOTRUN -> [FAIL][7] ([i915#4312])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111472v2/fi-icl-u2/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
  [i915#7654]: https://gitlab.freedesktop.org/drm/intel/issues/7654
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699


Build changes
-------------

  * Linux: CI_DRM_12534 -> Patchwork_111472v2

  CI-20190529: 20190529
  CI_DRM_12534: 2eb7b99b8190efc92b708a51e41c5f7f86843e42 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7105: 305e8d105abf033cb850d1fb118e5cbfb6c9cd40 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_111472v2: 2eb7b99b8190efc92b708a51e41c5f7f86843e42 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

29768498ea74 drm/i915: Apply CDCLK quirk only on QS parts
a7821e51f61e drm/i915: Initialize intel quirks before CDCLK initialization
9d8bb24ebdf6 drm/i915/display: Add 480 MHz CDCLK steps for RPL-U
87d7b9c95594 drm/i915/quirks: Add quirk for 480MHz CDCLK step

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111472v2/index.html

[-- Attachment #2: Type: text/html, Size: 4426 bytes --]