diff mbox series

[36/42] drm/i915/lnl: Add support for CDCLK initialization sequence

Message ID 20230823170740.1180212-37-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series Enable Lunar Lake display | expand

Commit Message

Lucas De Marchi Aug. 23, 2023, 5:07 p.m. UTC 
From: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@intel.com>

Add CDCLK initialization sequence changes and CDCLK set frequency
sequence for LNL platform.

CDCLK frequency change sequence is different for LNL compared to MTL
when a change in mdclk/cdclk ratio is observed. Below are changes to be
made:

1. In MBUS_CTL register translation Throttle Min value.
2. In DBUF_CTL_S* register Min Tracker State Service value.

BSpec: 68846, 68864
Signed-off-by: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 58 ++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h            |  2 +
 2 files changed, 57 insertions(+), 3 deletions(-)

Comments

Matt Roper Aug. 24, 2023, 11:54 p.m. UTC | #1 
On Wed, Aug 23, 2023 at 10:07:34AM -0700, Lucas De Marchi wrote:
> From: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@intel.com>
> 
> Add CDCLK initialization sequence changes and CDCLK set frequency
> sequence for LNL platform.
> 
> CDCLK frequency change sequence is different for LNL compared to MTL
> when a change in mdclk/cdclk ratio is observed. Below are changes to be
> made:
> 
> 1. In MBUS_CTL register translation Throttle Min value.
> 2. In DBUF_CTL_S* register Min Tracker State Service value.

The previous patch just did these same changes, but made the changes to
the existing functions.  It looks like we wound up with two patches
doing the same thing?

> 
> BSpec: 68846, 68864
> Signed-off-by: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 58 ++++++++++++++++++++--
>  drivers/gpu/drm/i915/i915_reg.h            |  2 +
>  2 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index aa1000db3cb9..4d8b960389ec 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 "skl_watermark.h"
> +#include "skl_watermark_regs.h"
>  #include "vlv_sideband.h"
>  
>  /**
> @@ -1727,7 +1728,12 @@ static void adlp_cdclk_pll_crawl(struct drm_i915_private *dev_priv, int vco)
>  
>  static u32 bxt_cdclk_cd2x_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
> -	if (DISPLAY_VER(dev_priv) >= 12) {
> +	if (DISPLAY_VER(dev_priv) >= 20) {
> +		if (pipe == INVALID_PIPE)
> +			return LNL_CDCLK_CD2X_PIPE_NONE;
> +		else
> +			return LNL_CDCLK_CD2X_PIPE(pipe);

I don't think this change is correct; see note farther down on the
register definitions.

> +	} else if (DISPLAY_VER(dev_priv) >= 12) {
>  		if (pipe == INVALID_PIPE)
>  			return TGL_CDCLK_CD2X_PIPE_NONE;
>  		else
> @@ -1837,6 +1843,47 @@ static int get_mdclk_cdclk_ratio(struct drm_i915_private *i915,
>  		return 1;
>  }
>  
> +static void lnl_prog_mbus_dbuf_ctrl(struct drm_i915_private *i915,
> +				    const struct intel_cdclk_config *cdclk_config)
> +{
> +	int min_throttle_val;
> +	int min_tracker_state;
> +	enum dbuf_slice slice;
> +	int mdclk_cdclk_div_ratio;
> +	int mbus_join = intel_de_read(i915, MBUS_CTL) & MBUS_JOIN;
> +
> +	mdclk_cdclk_div_ratio = get_mdclk_cdclk_ratio(i915, cdclk_config);
> +
> +	min_throttle_val = MBUS_TRANS_THROTTLE_MIN_SELECT(mdclk_cdclk_div_ratio);
> +
> +	intel_de_rmw(i915, MBUS_CTL, MBUS_TRANS_THROTTLE_MIN_MASK, min_throttle_val);
> +
> +	if (mbus_join)
> +		mdclk_cdclk_div_ratio = (mdclk_cdclk_div_ratio << 1) + 1;
> +
> +	min_tracker_state = DBUF_MIN_TRACKER_STATE_SERVICE(mdclk_cdclk_div_ratio);
> +
> +	for_each_dbuf_slice(i915, slice)
> +		intel_de_rmw(i915, DBUF_CTL_S(slice),
> +			     DBUF_MIN_TRACKER_STATE_SERVICE_MASK,
> +			     min_tracker_state);
> +}
> +
> +static void lnl_cdclk_squash_program(struct drm_i915_private *i915,
> +				     const struct intel_cdclk_config *cdclk_config,
> +				     u16 waveform)
> +{
> +	if (cdclk_config->cdclk < i915->display.cdclk.hw.cdclk)
> +		/* Program mbus_ctrl and dbuf_ctrl registers as Pre hook */
> +		lnl_prog_mbus_dbuf_ctrl(i915, cdclk_config);
> +
> +	dg2_cdclk_squash_program(i915, waveform);
> +
> +	if (cdclk_config->cdclk > i915->display.cdclk.hw.cdclk)
> +		/* Program mbus_ctrl and dbuf_ctrl registers as Post hook */
> +		lnl_prog_mbus_dbuf_ctrl(i915, cdclk_config);
> +}
> +
>  static int cdclk_squash_divider(u16 waveform)
>  {
>  	return hweight16(waveform ?: 0xffff);
> @@ -1938,8 +1985,13 @@ static void _bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  	else
>  		clock = cdclk;
>  
> -	if (HAS_CDCLK_SQUASH(dev_priv))
> -		dg2_cdclk_squash_program(dev_priv, waveform);
> +	if (HAS_CDCLK_SQUASH(dev_priv)) {
> +		if (DISPLAY_VER(dev_priv) >= 20)
> +			lnl_cdclk_squash_program(dev_priv, cdclk_config,
> +						 waveform);
> +		else
> +			dg2_cdclk_squash_program(dev_priv, waveform);
> +	}
>  
>  	val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) |
>  		bxt_cdclk_cd2x_pipe(dev_priv, pipe);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d5850761a75a..c9639f0f4f49 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5944,6 +5944,8 @@ enum skl_power_gate {
>  #define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe) << 20)
>  #define  CDCLK_DIVMUX_CD_OVERRIDE	(1 << 19)
>  #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
> +#define  LNL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)

This doesn't match what I see on bspec 69090:

Bits 21:19
  000 => Pipe A
  010 => Pipe B
  100 => Pipe C
  110 => Pipe D

So the pipe ID (0-3) should actually be shifted by 20 since bit 19 is
always 0 (except for the "none" case).  I think 


Matt

> +#define  LNL_CDCLK_CD2X_PIPE_NONE	(7 << 19)
>  #define  ICL_CDCLK_CD2X_PIPE(pipe)	(_PICK(pipe, 0, 2, 6) << 19)
>  #define  ICL_CDCLK_CD2X_PIPE_NONE	(7 << 19)
>  #define  TGL_CDCLK_CD2X_PIPE(pipe)	BXT_CDCLK_CD2X_PIPE(pipe)
> -- 
> 2.40.1
>
Lucas De Marchi Aug. 29, 2023, 10:21 p.m. UTC | #2 
On Thu, Aug 24, 2023 at 04:54:57PM -0700, Matt Roper wrote:
>On Wed, Aug 23, 2023 at 10:07:34AM -0700, Lucas De Marchi wrote:
>> From: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@intel.com>
>>
>> Add CDCLK initialization sequence changes and CDCLK set frequency
>> sequence for LNL platform.
>>
>> CDCLK frequency change sequence is different for LNL compared to MTL
>> when a change in mdclk/cdclk ratio is observed. Below are changes to be
>> made:
>>
>> 1. In MBUS_CTL register translation Throttle Min value.
>> 2. In DBUF_CTL_S* register Min Tracker State Service value.
>
>The previous patch just did these same changes, but made the changes to
>the existing functions.  It looks like we wound up with two patches
>doing the same thing?

The change here is because now during the squash phase there is
extra programming steps touching DBUF_CTL_S and MBUS_CTL. It doesn't
seem good we have this in 2 different places though.  Ravi / Stan, any
comment here?

Lucas De Marchi

>
>>
>> BSpec: 68846, 68864
>> Signed-off-by: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_cdclk.c | 58 ++++++++++++++++++++--
>>  drivers/gpu/drm/i915/i915_reg.h            |  2 +
>>  2 files changed, 57 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> index aa1000db3cb9..4d8b960389ec 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 "skl_watermark.h"
>> +#include "skl_watermark_regs.h"
>>  #include "vlv_sideband.h"
>>
>>  /**
>> @@ -1727,7 +1728,12 @@ static void adlp_cdclk_pll_crawl(struct drm_i915_private *dev_priv, int vco)
>>
>>  static u32 bxt_cdclk_cd2x_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
>>  {
>> -	if (DISPLAY_VER(dev_priv) >= 12) {
>> +	if (DISPLAY_VER(dev_priv) >= 20) {
>> +		if (pipe == INVALID_PIPE)
>> +			return LNL_CDCLK_CD2X_PIPE_NONE;
>> +		else
>> +			return LNL_CDCLK_CD2X_PIPE(pipe);
>
>I don't think this change is correct; see note farther down on the
>register definitions.
>
>> +	} else if (DISPLAY_VER(dev_priv) >= 12) {
>>  		if (pipe == INVALID_PIPE)
>>  			return TGL_CDCLK_CD2X_PIPE_NONE;
>>  		else
>> @@ -1837,6 +1843,47 @@ static int get_mdclk_cdclk_ratio(struct drm_i915_private *i915,
>>  		return 1;
>>  }
>>
>> +static void lnl_prog_mbus_dbuf_ctrl(struct drm_i915_private *i915,
>> +				    const struct intel_cdclk_config *cdclk_config)
>> +{
>> +	int min_throttle_val;
>> +	int min_tracker_state;
>> +	enum dbuf_slice slice;
>> +	int mdclk_cdclk_div_ratio;
>> +	int mbus_join = intel_de_read(i915, MBUS_CTL) & MBUS_JOIN;
>> +
>> +	mdclk_cdclk_div_ratio = get_mdclk_cdclk_ratio(i915, cdclk_config);
>> +
>> +	min_throttle_val = MBUS_TRANS_THROTTLE_MIN_SELECT(mdclk_cdclk_div_ratio);
>> +
>> +	intel_de_rmw(i915, MBUS_CTL, MBUS_TRANS_THROTTLE_MIN_MASK, min_throttle_val);
>> +
>> +	if (mbus_join)
>> +		mdclk_cdclk_div_ratio = (mdclk_cdclk_div_ratio << 1) + 1;
>> +
>> +	min_tracker_state = DBUF_MIN_TRACKER_STATE_SERVICE(mdclk_cdclk_div_ratio);
>> +
>> +	for_each_dbuf_slice(i915, slice)
>> +		intel_de_rmw(i915, DBUF_CTL_S(slice),
>> +			     DBUF_MIN_TRACKER_STATE_SERVICE_MASK,
>> +			     min_tracker_state);
>> +}
>> +
>> +static void lnl_cdclk_squash_program(struct drm_i915_private *i915,
>> +				     const struct intel_cdclk_config *cdclk_config,
>> +				     u16 waveform)
>> +{
>> +	if (cdclk_config->cdclk < i915->display.cdclk.hw.cdclk)
>> +		/* Program mbus_ctrl and dbuf_ctrl registers as Pre hook */
>> +		lnl_prog_mbus_dbuf_ctrl(i915, cdclk_config);
>> +
>> +	dg2_cdclk_squash_program(i915, waveform);
>> +
>> +	if (cdclk_config->cdclk > i915->display.cdclk.hw.cdclk)
>> +		/* Program mbus_ctrl and dbuf_ctrl registers as Post hook */
>> +		lnl_prog_mbus_dbuf_ctrl(i915, cdclk_config);
>> +}
>> +
>>  static int cdclk_squash_divider(u16 waveform)
>>  {
>>  	return hweight16(waveform ?: 0xffff);
>> @@ -1938,8 +1985,13 @@ static void _bxt_set_cdclk(struct drm_i915_private *dev_priv,
>>  	else
>>  		clock = cdclk;
>>
>> -	if (HAS_CDCLK_SQUASH(dev_priv))
>> -		dg2_cdclk_squash_program(dev_priv, waveform);
>> +	if (HAS_CDCLK_SQUASH(dev_priv)) {
>> +		if (DISPLAY_VER(dev_priv) >= 20)
>> +			lnl_cdclk_squash_program(dev_priv, cdclk_config,
>> +						 waveform);
>> +		else
>> +			dg2_cdclk_squash_program(dev_priv, waveform);
>> +	}
>>
>>  	val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) |
>>  		bxt_cdclk_cd2x_pipe(dev_priv, pipe);
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index d5850761a75a..c9639f0f4f49 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -5944,6 +5944,8 @@ enum skl_power_gate {
>>  #define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe) << 20)
>>  #define  CDCLK_DIVMUX_CD_OVERRIDE	(1 << 19)
>>  #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
>> +#define  LNL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)
>
>This doesn't match what I see on bspec 69090:
>
>Bits 21:19
>  000 => Pipe A
>  010 => Pipe B
>  100 => Pipe C
>  110 => Pipe D
>
>So the pipe ID (0-3) should actually be shifted by 20 since bit 19 is
>always 0 (except for the "none" case).  I think
>
>
>Matt
>
>> +#define  LNL_CDCLK_CD2X_PIPE_NONE	(7 << 19)
>>  #define  ICL_CDCLK_CD2X_PIPE(pipe)	(_PICK(pipe, 0, 2, 6) << 19)
>>  #define  ICL_CDCLK_CD2X_PIPE_NONE	(7 << 19)
>>  #define  TGL_CDCLK_CD2X_PIPE(pipe)	BXT_CDCLK_CD2X_PIPE(pipe)
>> --
>> 2.40.1
>>
>
>-- 
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index aa1000db3cb9..4d8b960389ec 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 "skl_watermark.h"
+#include "skl_watermark_regs.h"
 #include "vlv_sideband.h"
 
 /**
@@ -1727,7 +1728,12 @@  static void adlp_cdclk_pll_crawl(struct drm_i915_private *dev_priv, int vco)
 
 static u32 bxt_cdclk_cd2x_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
-	if (DISPLAY_VER(dev_priv) >= 12) {
+	if (DISPLAY_VER(dev_priv) >= 20) {
+		if (pipe == INVALID_PIPE)
+			return LNL_CDCLK_CD2X_PIPE_NONE;
+		else
+			return LNL_CDCLK_CD2X_PIPE(pipe);
+	} else if (DISPLAY_VER(dev_priv) >= 12) {
 		if (pipe == INVALID_PIPE)
 			return TGL_CDCLK_CD2X_PIPE_NONE;
 		else
@@ -1837,6 +1843,47 @@  static int get_mdclk_cdclk_ratio(struct drm_i915_private *i915,
 		return 1;
 }
 
+static void lnl_prog_mbus_dbuf_ctrl(struct drm_i915_private *i915,
+				    const struct intel_cdclk_config *cdclk_config)
+{
+	int min_throttle_val;
+	int min_tracker_state;
+	enum dbuf_slice slice;
+	int mdclk_cdclk_div_ratio;
+	int mbus_join = intel_de_read(i915, MBUS_CTL) & MBUS_JOIN;
+
+	mdclk_cdclk_div_ratio = get_mdclk_cdclk_ratio(i915, cdclk_config);
+
+	min_throttle_val = MBUS_TRANS_THROTTLE_MIN_SELECT(mdclk_cdclk_div_ratio);
+
+	intel_de_rmw(i915, MBUS_CTL, MBUS_TRANS_THROTTLE_MIN_MASK, min_throttle_val);
+
+	if (mbus_join)
+		mdclk_cdclk_div_ratio = (mdclk_cdclk_div_ratio << 1) + 1;
+
+	min_tracker_state = DBUF_MIN_TRACKER_STATE_SERVICE(mdclk_cdclk_div_ratio);
+
+	for_each_dbuf_slice(i915, slice)
+		intel_de_rmw(i915, DBUF_CTL_S(slice),
+			     DBUF_MIN_TRACKER_STATE_SERVICE_MASK,
+			     min_tracker_state);
+}
+
+static void lnl_cdclk_squash_program(struct drm_i915_private *i915,
+				     const struct intel_cdclk_config *cdclk_config,
+				     u16 waveform)
+{
+	if (cdclk_config->cdclk < i915->display.cdclk.hw.cdclk)
+		/* Program mbus_ctrl and dbuf_ctrl registers as Pre hook */
+		lnl_prog_mbus_dbuf_ctrl(i915, cdclk_config);
+
+	dg2_cdclk_squash_program(i915, waveform);
+
+	if (cdclk_config->cdclk > i915->display.cdclk.hw.cdclk)
+		/* Program mbus_ctrl and dbuf_ctrl registers as Post hook */
+		lnl_prog_mbus_dbuf_ctrl(i915, cdclk_config);
+}
+
 static int cdclk_squash_divider(u16 waveform)
 {
 	return hweight16(waveform ?: 0xffff);
@@ -1938,8 +1985,13 @@  static void _bxt_set_cdclk(struct drm_i915_private *dev_priv,
 	else
 		clock = cdclk;
 
-	if (HAS_CDCLK_SQUASH(dev_priv))
-		dg2_cdclk_squash_program(dev_priv, waveform);
+	if (HAS_CDCLK_SQUASH(dev_priv)) {
+		if (DISPLAY_VER(dev_priv) >= 20)
+			lnl_cdclk_squash_program(dev_priv, cdclk_config,
+						 waveform);
+		else
+			dg2_cdclk_squash_program(dev_priv, waveform);
+	}
 
 	val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) |
 		bxt_cdclk_cd2x_pipe(dev_priv, pipe);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d5850761a75a..c9639f0f4f49 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5944,6 +5944,8 @@  enum skl_power_gate {
 #define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe) << 20)
 #define  CDCLK_DIVMUX_CD_OVERRIDE	(1 << 19)
 #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
+#define  LNL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)
+#define  LNL_CDCLK_CD2X_PIPE_NONE	(7 << 19)
 #define  ICL_CDCLK_CD2X_PIPE(pipe)	(_PICK(pipe, 0, 2, 6) << 19)
 #define  ICL_CDCLK_CD2X_PIPE_NONE	(7 << 19)
 #define  TGL_CDCLK_CD2X_PIPE(pipe)	BXT_CDCLK_CD2X_PIPE(pipe)