diff mbox series

[2/6] drm/i915/display/adl_p: Implement Wa_22012278275

Message ID 20210616203158.118111-2-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/6] drm/i915/display/psr: Handle SU Y granularity | expand

Commit Message

Souza, Jose June 16, 2021, 8:31 p.m. UTC 
The PSR2_CTL io buffer wake and fast wake values do not match
expected in pre production hardware, so here adding a table that
matches with HW to program it with values that HW expect.

Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 29 +++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h          |  6 +++--
 2 files changed, 32 insertions(+), 3 deletions(-)

Comments

Gwan-gyeong Mun June 23, 2021, 3:30 p.m. UTC | #1 
On 6/16/21 11:31 PM, José Roberto de Souza wrote:
> The PSR2_CTL io buffer wake and fast wake values do not match
> expected in pre production hardware, so here adding a table that
> matches with HW to program it with values that HW expect.
> 
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_psr.c | 29 +++++++++++++++++++++++-
>   drivers/gpu/drm/i915/i915_reg.h          |  6 +++--
>   2 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index fde30f937504e..c8d56387d9233 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -542,7 +542,34 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>   	val |= EDP_PSR2_FRAME_BEFORE_SU(intel_dp->psr.sink_sync_latency + 1);
>   	val |= intel_psr2_get_tp_time(intel_dp);
>   
> -	if (DISPLAY_VER(dev_priv) >= 12) {
> +	/* Wa_22012278275:adlp */
> +	if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_D1)) {
> +		static const u8 map[] = {
> +			2, /* 5 lines */
> +			1, /* 6 lines */
> +			0, /* 7 lines */
> +			3, /* 8 lines */
> +			6, /* 9 lines */
> +			5, /* 10 lines */
> +			4, /* 11 lines */
> +			7, /* 12 lines */
> +		};
> +		/*
> +		 * Still using the default IO_BUFFER_WAKE and FAST_WAKE, see
> +		 * comments bellow for more information
typo on comments; bellow
except for the typo, looks good to me.

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> +		 */
> +		u32 tmp, lines = 7;
> +
> +		val |= TGL_EDP_PSR2_BLOCK_COUNT_NUM_2;
> +
> +		tmp = map[lines - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> +		tmp = tmp << TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT;
> +		val |= tmp;
> +
> +		tmp = map[lines - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> +		tmp = tmp << TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT;
> +		val |= tmp;
> +	} else if (DISPLAY_VER(dev_priv) >= 12) {
>   		/*
>   		 * TODO: 7 lines of IO_BUFFER_WAKE and FAST_WAKE are default
>   		 * values from BSpec. In order to setting an optimal power
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 955027799d1de..4a98e49c58812 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4596,13 +4596,15 @@ enum {
>   #define   EDP_PSR2_IO_BUFFER_WAKE(lines)	((EDP_PSR2_IO_BUFFER_WAKE_MAX_LINES - (lines)) << 13)
>   #define   EDP_PSR2_IO_BUFFER_WAKE_MASK		(3 << 13)
>   #define   TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES	5
> -#define   TGL_EDP_PSR2_IO_BUFFER_WAKE(lines)	(((lines) - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES) << 13)
> +#define   TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT	13
> +#define   TGL_EDP_PSR2_IO_BUFFER_WAKE(lines)	(((lines) - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES) << TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT)
>   #define   TGL_EDP_PSR2_IO_BUFFER_WAKE_MASK	(7 << 13)
>   #define   EDP_PSR2_FAST_WAKE_MAX_LINES		8
>   #define   EDP_PSR2_FAST_WAKE(lines)		((EDP_PSR2_FAST_WAKE_MAX_LINES - (lines)) << 11)
>   #define   EDP_PSR2_FAST_WAKE_MASK		(3 << 11)
>   #define   TGL_EDP_PSR2_FAST_WAKE_MIN_LINES	5
> -#define   TGL_EDP_PSR2_FAST_WAKE(lines)		(((lines) - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES) << 10)
> +#define   TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT	10
> +#define   TGL_EDP_PSR2_FAST_WAKE(lines)		(((lines) - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES) << TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT)
>   #define   TGL_EDP_PSR2_FAST_WAKE_MASK		(7 << 10)
>   #define   EDP_PSR2_TP2_TIME_500us		(0 << 8)
>   #define   EDP_PSR2_TP2_TIME_100us		(1 << 8)
>
Jani Nikula July 8, 2021, 8:56 a.m. UTC | #2 
This is well after this has been merged, but only spotted this now.

In the future, please add something sensible to the subject lines
instead of just the platform and workaround number. I'm looking at a
shortlog with:

      drm/i915/display/adl_p: Implement Wa_22012278275
      drm/i915/display/adl_p: Implement Wa_16011168373
      drm/i915/display/adl_p: Implement Wa_16011303918

and that's just completely unhelpful. :(

BR,
Jani.


On Wed, 16 Jun 2021, José Roberto de Souza <jose.souza@intel.com> wrote:
> The PSR2_CTL io buffer wake and fast wake values do not match
> expected in pre production hardware, so here adding a table that
> matches with HW to program it with values that HW expect.
>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 29 +++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h          |  6 +++--
>  2 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index fde30f937504e..c8d56387d9233 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -542,7 +542,34 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	val |= EDP_PSR2_FRAME_BEFORE_SU(intel_dp->psr.sink_sync_latency + 1);
>  	val |= intel_psr2_get_tp_time(intel_dp);
>  
> -	if (DISPLAY_VER(dev_priv) >= 12) {
> +	/* Wa_22012278275:adlp */
> +	if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_D1)) {
> +		static const u8 map[] = {
> +			2, /* 5 lines */
> +			1, /* 6 lines */
> +			0, /* 7 lines */
> +			3, /* 8 lines */
> +			6, /* 9 lines */
> +			5, /* 10 lines */
> +			4, /* 11 lines */
> +			7, /* 12 lines */
> +		};
> +		/*
> +		 * Still using the default IO_BUFFER_WAKE and FAST_WAKE, see
> +		 * comments bellow for more information
> +		 */
> +		u32 tmp, lines = 7;
> +
> +		val |= TGL_EDP_PSR2_BLOCK_COUNT_NUM_2;
> +
> +		tmp = map[lines - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> +		tmp = tmp << TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT;
> +		val |= tmp;
> +
> +		tmp = map[lines - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> +		tmp = tmp << TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT;
> +		val |= tmp;
> +	} else if (DISPLAY_VER(dev_priv) >= 12) {
>  		/*
>  		 * TODO: 7 lines of IO_BUFFER_WAKE and FAST_WAKE are default
>  		 * values from BSpec. In order to setting an optimal power
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 955027799d1de..4a98e49c58812 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4596,13 +4596,15 @@ enum {
>  #define   EDP_PSR2_IO_BUFFER_WAKE(lines)	((EDP_PSR2_IO_BUFFER_WAKE_MAX_LINES - (lines)) << 13)
>  #define   EDP_PSR2_IO_BUFFER_WAKE_MASK		(3 << 13)
>  #define   TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES	5
> -#define   TGL_EDP_PSR2_IO_BUFFER_WAKE(lines)	(((lines) - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES) << 13)
> +#define   TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT	13
> +#define   TGL_EDP_PSR2_IO_BUFFER_WAKE(lines)	(((lines) - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES) << TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT)
>  #define   TGL_EDP_PSR2_IO_BUFFER_WAKE_MASK	(7 << 13)
>  #define   EDP_PSR2_FAST_WAKE_MAX_LINES		8
>  #define   EDP_PSR2_FAST_WAKE(lines)		((EDP_PSR2_FAST_WAKE_MAX_LINES - (lines)) << 11)
>  #define   EDP_PSR2_FAST_WAKE_MASK		(3 << 11)
>  #define   TGL_EDP_PSR2_FAST_WAKE_MIN_LINES	5
> -#define   TGL_EDP_PSR2_FAST_WAKE(lines)		(((lines) - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES) << 10)
> +#define   TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT	10
> +#define   TGL_EDP_PSR2_FAST_WAKE(lines)		(((lines) - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES) << TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT)
>  #define   TGL_EDP_PSR2_FAST_WAKE_MASK		(7 << 10)
>  #define   EDP_PSR2_TP2_TIME_500us		(0 << 8)
>  #define   EDP_PSR2_TP2_TIME_100us		(1 << 8)
Jani Nikula July 8, 2021, 8:56 a.m. UTC | #3 
This is well after this has been merged, but only spotted this now.

In the future, please add something sensible to the subject lines
instead of just the platform and workaround number. I'm looking at a
shortlog with:

      drm/i915/display/adl_p: Implement Wa_22012278275
      drm/i915/display/adl_p: Implement Wa_16011168373
      drm/i915/display/adl_p: Implement Wa_16011303918

and that's just completely unhelpful. :(

BR,
Jani.


On Wed, 16 Jun 2021, José Roberto de Souza <jose.souza@intel.com> wrote:
> The PSR2_CTL io buffer wake and fast wake values do not match
> expected in pre production hardware, so here adding a table that
> matches with HW to program it with values that HW expect.
>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 29 +++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h          |  6 +++--
>  2 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index fde30f937504e..c8d56387d9233 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -542,7 +542,34 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	val |= EDP_PSR2_FRAME_BEFORE_SU(intel_dp->psr.sink_sync_latency + 1);
>  	val |= intel_psr2_get_tp_time(intel_dp);
>  
> -	if (DISPLAY_VER(dev_priv) >= 12) {
> +	/* Wa_22012278275:adlp */
> +	if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_D1)) {
> +		static const u8 map[] = {
> +			2, /* 5 lines */
> +			1, /* 6 lines */
> +			0, /* 7 lines */
> +			3, /* 8 lines */
> +			6, /* 9 lines */
> +			5, /* 10 lines */
> +			4, /* 11 lines */
> +			7, /* 12 lines */
> +		};
> +		/*
> +		 * Still using the default IO_BUFFER_WAKE and FAST_WAKE, see
> +		 * comments bellow for more information
> +		 */
> +		u32 tmp, lines = 7;
> +
> +		val |= TGL_EDP_PSR2_BLOCK_COUNT_NUM_2;
> +
> +		tmp = map[lines - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> +		tmp = tmp << TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT;
> +		val |= tmp;
> +
> +		tmp = map[lines - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> +		tmp = tmp << TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT;
> +		val |= tmp;
> +	} else if (DISPLAY_VER(dev_priv) >= 12) {
>  		/*
>  		 * TODO: 7 lines of IO_BUFFER_WAKE and FAST_WAKE are default
>  		 * values from BSpec. In order to setting an optimal power
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 955027799d1de..4a98e49c58812 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4596,13 +4596,15 @@ enum {
>  #define   EDP_PSR2_IO_BUFFER_WAKE(lines)	((EDP_PSR2_IO_BUFFER_WAKE_MAX_LINES - (lines)) << 13)
>  #define   EDP_PSR2_IO_BUFFER_WAKE_MASK		(3 << 13)
>  #define   TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES	5
> -#define   TGL_EDP_PSR2_IO_BUFFER_WAKE(lines)	(((lines) - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES) << 13)
> +#define   TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT	13
> +#define   TGL_EDP_PSR2_IO_BUFFER_WAKE(lines)	(((lines) - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES) << TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT)
>  #define   TGL_EDP_PSR2_IO_BUFFER_WAKE_MASK	(7 << 13)
>  #define   EDP_PSR2_FAST_WAKE_MAX_LINES		8
>  #define   EDP_PSR2_FAST_WAKE(lines)		((EDP_PSR2_FAST_WAKE_MAX_LINES - (lines)) << 11)
>  #define   EDP_PSR2_FAST_WAKE_MASK		(3 << 11)
>  #define   TGL_EDP_PSR2_FAST_WAKE_MIN_LINES	5
> -#define   TGL_EDP_PSR2_FAST_WAKE(lines)		(((lines) - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES) << 10)
> +#define   TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT	10
> +#define   TGL_EDP_PSR2_FAST_WAKE(lines)		(((lines) - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES) << TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT)
>  #define   TGL_EDP_PSR2_FAST_WAKE_MASK		(7 << 10)
>  #define   EDP_PSR2_TP2_TIME_500us		(0 << 8)
>  #define   EDP_PSR2_TP2_TIME_100us		(1 << 8)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index fde30f937504e..c8d56387d9233 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -542,7 +542,34 @@  static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	val |= EDP_PSR2_FRAME_BEFORE_SU(intel_dp->psr.sink_sync_latency + 1);
 	val |= intel_psr2_get_tp_time(intel_dp);
 
-	if (DISPLAY_VER(dev_priv) >= 12) {
+	/* Wa_22012278275:adlp */
+	if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_D1)) {
+		static const u8 map[] = {
+			2, /* 5 lines */
+			1, /* 6 lines */
+			0, /* 7 lines */
+			3, /* 8 lines */
+			6, /* 9 lines */
+			5, /* 10 lines */
+			4, /* 11 lines */
+			7, /* 12 lines */
+		};
+		/*
+		 * Still using the default IO_BUFFER_WAKE and FAST_WAKE, see
+		 * comments bellow for more information
+		 */
+		u32 tmp, lines = 7;
+
+		val |= TGL_EDP_PSR2_BLOCK_COUNT_NUM_2;
+
+		tmp = map[lines - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
+		tmp = tmp << TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT;
+		val |= tmp;
+
+		tmp = map[lines - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
+		tmp = tmp << TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT;
+		val |= tmp;
+	} else if (DISPLAY_VER(dev_priv) >= 12) {
 		/*
 		 * TODO: 7 lines of IO_BUFFER_WAKE and FAST_WAKE are default
 		 * values from BSpec. In order to setting an optimal power
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 955027799d1de..4a98e49c58812 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4596,13 +4596,15 @@  enum {
 #define   EDP_PSR2_IO_BUFFER_WAKE(lines)	((EDP_PSR2_IO_BUFFER_WAKE_MAX_LINES - (lines)) << 13)
 #define   EDP_PSR2_IO_BUFFER_WAKE_MASK		(3 << 13)
 #define   TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES	5
-#define   TGL_EDP_PSR2_IO_BUFFER_WAKE(lines)	(((lines) - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES) << 13)
+#define   TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT	13
+#define   TGL_EDP_PSR2_IO_BUFFER_WAKE(lines)	(((lines) - TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES) << TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT)
 #define   TGL_EDP_PSR2_IO_BUFFER_WAKE_MASK	(7 << 13)
 #define   EDP_PSR2_FAST_WAKE_MAX_LINES		8
 #define   EDP_PSR2_FAST_WAKE(lines)		((EDP_PSR2_FAST_WAKE_MAX_LINES - (lines)) << 11)
 #define   EDP_PSR2_FAST_WAKE_MASK		(3 << 11)
 #define   TGL_EDP_PSR2_FAST_WAKE_MIN_LINES	5
-#define   TGL_EDP_PSR2_FAST_WAKE(lines)		(((lines) - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES) << 10)
+#define   TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT	10
+#define   TGL_EDP_PSR2_FAST_WAKE(lines)		(((lines) - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES) << TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT)
 #define   TGL_EDP_PSR2_FAST_WAKE_MASK		(7 << 10)
 #define   EDP_PSR2_TP2_TIME_500us		(0 << 8)
 #define   EDP_PSR2_TP2_TIME_100us		(1 << 8)