intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [RFC 0/2] Add new CDCLK step for RPL-U
@ 2022-11-30  7:46 Chaitanya Kumar Borah
  2022-11-30  7:46 ` [Intel-gfx] [RFC 1/2] drm/i915: Add RPL-U CDCLK table Chaitanya Kumar Borah
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Chaitanya Kumar Borah @ 2022-11-30  7:46 UTC (permalink / raw)
  To: intel-gfx; +Cc: ville.syrjala

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 patchset adds a new table to include this new CDCLK
step. Details can be found in BSpec entry 55409.

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.

Even though ES parts will be deprecated in future we are adding this
distinction since they are currently in use. However, here the question
arises if we keep this change in upstream or not as this could just be dead
code down the line. Feedbacks are appreciated on this.

Chaitanya Kumar Borah (2):
  drm/i915: Add RPL-U CDCLK table
  drm/i915: Add additional check for 480Mhz step CDCLK

 drivers/gpu/drm/i915/display/intel_cdclk.c | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Intel-gfx] [RFC 1/2] drm/i915: Add RPL-U CDCLK table
  2022-11-30  7:46 [Intel-gfx] [RFC 0/2] Add new CDCLK step for RPL-U Chaitanya Kumar Borah
@ 2022-11-30  7:46 ` Chaitanya Kumar Borah
  2022-11-30  8:28   ` Jani Nikula
  2022-11-30  7:46 ` [Intel-gfx] [RFC 2/2] drm/i915: Add additional check for 480Mhz step CDCLK Chaitanya Kumar Borah
  2022-11-30 10:11 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for Add new CDCLK step for RPL-U Patchwork
  2 siblings, 1 reply; 8+ messages in thread
From: Chaitanya Kumar Borah @ 2022-11-30  7:46 UTC (permalink / raw)
  To: intel-gfx; +Cc: ville.syrjala

A new step of 480MHz has been added on SKUs that have a RPL-U
device id. Add a new table which include this new CDCLK step.

BSpec: 55409

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

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 0c107a38f9d0..9bfeb1abba47 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -180,6 +180,18 @@ static void i85x_get_cdclk(struct drm_i915_private *dev_priv,
 	}
 }
 
+static bool is_rplu(struct drm_i915_private *dev_priv)
+{
+	switch (INTEL_DEVID(dev_priv)) {
+	case 0xA7A1:
+	case 0xA721:
+	case 0xA7A9:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static void i915gm_get_cdclk(struct drm_i915_private *dev_priv,
 			     struct intel_cdclk_config *cdclk_config)
 {
@@ -1329,6 +1341,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 +3386,12 @@ 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
+		 * 480 MHz supported on SKUs that have a RPL-U Device ID
+		 */
+		else if (is_rplu(dev_priv))
+			dev_priv->cdclk.table = rplu_cdclk_table;
 		else
 			dev_priv->display.cdclk.table = adlp_cdclk_table;
 	} else if (IS_ROCKETLAKE(dev_priv)) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Intel-gfx] [RFC 2/2] drm/i915: Add additional check for 480Mhz step CDCLK
  2022-11-30  7:46 [Intel-gfx] [RFC 0/2] Add new CDCLK step for RPL-U Chaitanya Kumar Borah
  2022-11-30  7:46 ` [Intel-gfx] [RFC 1/2] drm/i915: Add RPL-U CDCLK table Chaitanya Kumar Borah
@ 2022-11-30  7:46 ` Chaitanya Kumar Borah
  2022-11-30  8:37   ` Jani Nikula
  2022-11-30 10:11 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for Add new CDCLK step for RPL-U Patchwork
  2 siblings, 1 reply; 8+ messages in thread
From: Chaitanya Kumar Borah @ 2022-11-30  7:46 UTC (permalink / raw)
  To: intel-gfx; +Cc: ville.syrjala

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;
+
+	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;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Intel-gfx] [RFC 1/2] drm/i915: Add RPL-U CDCLK table
  2022-11-30  7:46 ` [Intel-gfx] [RFC 1/2] drm/i915: Add RPL-U CDCLK table Chaitanya Kumar Borah
@ 2022-11-30  8:28   ` Jani Nikula
  2023-01-02  6:28     ` Borah, Chaitanya Kumar
  0 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2022-11-30  8:28 UTC (permalink / raw)
  To: Chaitanya Kumar Borah, intel-gfx; +Cc: ville.syrjala

On Wed, 30 Nov 2022, 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. Add a new table which include this new CDCLK step.
>
> BSpec: 55409
>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 0c107a38f9d0..9bfeb1abba47 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -180,6 +180,18 @@ static void i85x_get_cdclk(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> +static bool is_rplu(struct drm_i915_private *dev_priv)
> +{
> +	switch (INTEL_DEVID(dev_priv)) {
> +	case 0xA7A1:
> +	case 0xA721:
> +	case 0xA7A9:

No. Under no circumstances are you to sprinkle PCI ID checks directly in
code like this.

The only place where PCI IDs go are the macros in
include/drm/i915_pciids.h, and each PCI ID should be present there only
once.

Those macros should only be used in i915_pci.c and intel_device_info.c.

If you need a distinction between RPL-P and RPL-U like this (and I
didn't check this in the spec, just commenting on the implementation)
you need to split RPL-P and RPL-U definitions and add them as
subplatforms in intel_device_info.c.

As a general tip, when you consider using a function or a macro, look up
where it's used and how. INTEL_DEVID() isn't used like this anywhere.


BR,
Jani.


> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  static void i915gm_get_cdclk(struct drm_i915_private *dev_priv,
>  			     struct intel_cdclk_config *cdclk_config)
>  {
> @@ -1329,6 +1341,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 +3386,12 @@ 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
> +		 * 480 MHz supported on SKUs that have a RPL-U Device ID
> +		 */
> +		else if (is_rplu(dev_priv))
> +			dev_priv->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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Intel-gfx] [RFC 2/2] drm/i915: Add additional check for 480Mhz step CDCLK
  2022-11-30  7:46 ` [Intel-gfx] [RFC 2/2] drm/i915: Add additional check for 480Mhz step CDCLK Chaitanya Kumar Borah
@ 2022-11-30  8:37   ` Jani Nikula
  2023-01-02  6:32     ` Borah, Chaitanya Kumar
  0 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2022-11-30  8:37 UTC (permalink / raw)
  To: Chaitanya Kumar Borah, intel-gfx; +Cc: ville.syrjala

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;

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for Add new CDCLK step for RPL-U
  2022-11-30  7:46 [Intel-gfx] [RFC 0/2] Add new CDCLK step for RPL-U Chaitanya Kumar Borah
  2022-11-30  7:46 ` [Intel-gfx] [RFC 1/2] drm/i915: Add RPL-U CDCLK table Chaitanya Kumar Borah
  2022-11-30  7:46 ` [Intel-gfx] [RFC 2/2] drm/i915: Add additional check for 480Mhz step CDCLK Chaitanya Kumar Borah
@ 2022-11-30 10:11 ` Patchwork
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2022-11-30 10:11 UTC (permalink / raw)
  To: Chaitanya Kumar Borah; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

Error: make failed
  CALL    scripts/checksyscalls.sh
  DESCEND objtool
  CC [M]  drivers/gpu/drm/i915/display/intel_cdclk.o
drivers/gpu/drm/i915/display/intel_cdclk.c: In function ‘intel_init_cdclk_hooks’:
drivers/gpu/drm/i915/display/intel_cdclk.c:3408:12: error: ‘struct drm_i915_private’ has no member named ‘cdclk’
    dev_priv->cdclk.table = rplu_cdclk_table;
            ^~
scripts/Makefile.build:250: recipe for target 'drivers/gpu/drm/i915/display/intel_cdclk.o' failed
make[5]: *** [drivers/gpu/drm/i915/display/intel_cdclk.o] Error 1
scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm/i915' failed
make[4]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm' failed
make[3]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:500: recipe for target 'drivers/gpu' failed
make[2]: *** [drivers/gpu] Error 2
scripts/Makefile.build:500: recipe for target 'drivers' failed
make[1]: *** [drivers] Error 2
Makefile:1992: recipe for target '.' failed
make: *** [.] Error 2



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Intel-gfx] [RFC 1/2] drm/i915: Add RPL-U CDCLK table
  2022-11-30  8:28   ` Jani Nikula
@ 2023-01-02  6:28     ` Borah, Chaitanya Kumar
  0 siblings, 0 replies; 8+ messages in thread
From: Borah, Chaitanya Kumar @ 2023-01-02  6:28 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx; +Cc: Syrjala, Ville

Hello Jani,

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Wednesday, November 30, 2022 1:58 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 1/2] drm/i915: Add RPL-U CDCLK table
> 
> On Wed, 30 Nov 2022, 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. Add a new table which include this new CDCLK step.
> >
> > BSpec: 55409
> >
> > Signed-off-by: Chaitanya Kumar Borah
> <chaitanya.kumar.borah@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 39
> > ++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 0c107a38f9d0..9bfeb1abba47 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -180,6 +180,18 @@ static void i85x_get_cdclk(struct drm_i915_private
> *dev_priv,
> >  	}
> >  }
> >
> > +static bool is_rplu(struct drm_i915_private *dev_priv) {
> > +	switch (INTEL_DEVID(dev_priv)) {
> > +	case 0xA7A1:
> > +	case 0xA721:
> > +	case 0xA7A9:
> 
> No. Under no circumstances are you to sprinkle PCI ID checks directly in code
> like this.
> 
> The only place where PCI IDs go are the macros in include/drm/i915_pciids.h,
> and each PCI ID should be present there only once.
> 
> Those macros should only be used in i915_pci.c and intel_device_info.c.
> 
> If you need a distinction between RPL-P and RPL-U like this (and I didn't check
> this in the spec, just commenting on the implementation) you need to split
> RPL-P and RPL-U definitions and add them as subplatforms in
> intel_device_info.c.
> 
> As a general tip, when you consider using a function or a macro, look up
> where it's used and how. INTEL_DEVID() isn't used like this anywhere.

Thank you for your feedback. I send out a new patch set which implements this change using quirks to identify
the PCI ids. Adding a separate sub platform just for this change seems to be a bit of an overkill. Let us know your thoughts.

Regards,

Chaitanya

> 
> 
> BR,
> Jani.
> 
> 
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> > +
> >  static void i915gm_get_cdclk(struct drm_i915_private *dev_priv,
> >  			     struct intel_cdclk_config *cdclk_config)  { @@ -
> 1329,6
> > +1341,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 +3386,12 @@ 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
> > +		 * 480 MHz supported on SKUs that have a RPL-U Device ID
> > +		 */
> > +		else if (is_rplu(dev_priv))
> > +			dev_priv->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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Intel-gfx] [RFC 2/2] drm/i915: Add additional check for 480Mhz step CDCLK
  2022-11-30  8:37   ` Jani Nikula
@ 2023-01-02  6:32     ` Borah, Chaitanya Kumar
  0 siblings, 0 replies; 8+ messages in thread
From: Borah, Chaitanya Kumar @ 2023-01-02  6:32 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx; +Cc: Syrjala, Ville

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-01-02  6:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-30  7:46 [Intel-gfx] [RFC 0/2] Add new CDCLK step for RPL-U Chaitanya Kumar Borah
2022-11-30  7:46 ` [Intel-gfx] [RFC 1/2] drm/i915: Add RPL-U CDCLK table Chaitanya Kumar Borah
2022-11-30  8:28   ` Jani Nikula
2023-01-02  6:28     ` Borah, Chaitanya Kumar
2022-11-30  7:46 ` [Intel-gfx] [RFC 2/2] drm/i915: Add additional check for 480Mhz step CDCLK Chaitanya Kumar Borah
2022-11-30  8:37   ` Jani Nikula
2023-01-02  6:32     ` Borah, Chaitanya Kumar
2022-11-30 10:11 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for Add new CDCLK step for RPL-U Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).