[PATCH 1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values

[PATCH 1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values

[Imre Deak]

Depending on the transcoder enum values to translate from transcoder
to pipe/transcoder register addresses can easily break if we add a new
transcoder. So remove the dependency by using named initializers.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c | 52 ++++++++++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 983ae7fd8217..1b81d7cb209e 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -33,16 +33,30 @@
 #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
 
 #define GEN_DEFAULT_PIPEOFFSETS \
-	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
-			  PIPE_C_OFFSET, PIPE_EDP_OFFSET }, \
-	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
-			   TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET }
+	.pipe_offsets = { \
+		[TRANSCODER_A] = PIPE_A_OFFSET,	\
+		[TRANSCODER_B] = PIPE_B_OFFSET, \
+		[TRANSCODER_C] = PIPE_C_OFFSET, \
+		[TRANSCODER_EDP] = PIPE_EDP_OFFSET, \
+	}, \
+	.trans_offsets = { \
+		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
+		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
+		[TRANSCODER_C] = TRANSCODER_C_OFFSET, \
+		[TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET, \
+	}
 
 #define GEN_CHV_PIPEOFFSETS \
-	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
-			  CHV_PIPE_C_OFFSET }, \
-	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
-			   CHV_TRANSCODER_C_OFFSET }
+	.pipe_offsets = { \
+		[TRANSCODER_A] = PIPE_A_OFFSET, \
+		[TRANSCODER_B] = PIPE_B_OFFSET, \
+		[TRANSCODER_C] = CHV_PIPE_C_OFFSET, \
+	}, \
+	.trans_offsets = { \
+		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
+		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
+		[TRANSCODER_C] = CHV_TRANSCODER_C_OFFSET, \
+	}
 
 #define CURSOR_OFFSETS \
 	.cursor_offsets = { CURSOR_A_OFFSET, CURSOR_B_OFFSET, CHV_CURSOR_C_OFFSET }
@@ -592,12 +606,22 @@ static const struct intel_device_info intel_cannonlake_info = {
 
 #define GEN11_FEATURES \
 	GEN10_FEATURES, \
-	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
-			  PIPE_C_OFFSET, PIPE_EDP_OFFSET, \
-			  PIPE_DSI0_OFFSET, PIPE_DSI1_OFFSET }, \
-	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
-			   TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET, \
-			   TRANSCODER_DSI0_OFFSET, TRANSCODER_DSI1_OFFSET}, \
+	.pipe_offsets = { \
+		[TRANSCODER_A] = PIPE_A_OFFSET, \
+		[TRANSCODER_B] = PIPE_B_OFFSET, \
+		[TRANSCODER_C] = PIPE_C_OFFSET, \
+		[TRANSCODER_EDP] = PIPE_EDP_OFFSET, \
+		[TRANSCODER_DSI_0] = PIPE_DSI0_OFFSET, \
+		[TRANSCODER_DSI_1] = PIPE_DSI1_OFFSET, \
+	}, \
+	.trans_offsets = { \
+		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
+		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
+		[TRANSCODER_C] = TRANSCODER_C_OFFSET, \
+		[TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET, \
+		[TRANSCODER_DSI_0] = TRANSCODER_DSI0_OFFSET, \
+		[TRANSCODER_DSI_1] = TRANSCODER_DSI1_OFFSET, \
+	}, \
 	GEN(11), \
 	.ddb_size = 2048, \
 	.has_logical_ring_elsq = 1
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Make EDP PSR flags not depend on enum values

[Imre Deak]

Depending on the transcoder enum values to translate from transcoder
to EDP PSR flags can easily break if we add a new transcoder. So remove
the dependency by using an explicit mapping.

While at it also add a WARN for unexpected trancoders.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 10 ++++--
 drivers/gpu/drm/i915/intel_psr.c | 70 +++++++++++++++++++++++++++++-----------
 2 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index edb58af1e903..25b069175c2a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4150,9 +4150,13 @@ enum {
 /* Bspec claims those aren't shifted but stay at 0x64800 */
 #define EDP_PSR_IMR				_MMIO(0x64834)
 #define EDP_PSR_IIR				_MMIO(0x64838)
-#define   EDP_PSR_ERROR(trans)			(1 << (((trans) * 8 + 10) & 31))
-#define   EDP_PSR_POST_EXIT(trans)		(1 << (((trans) * 8 + 9) & 31))
-#define   EDP_PSR_PRE_ENTRY(trans)		(1 << (((trans) * 8 + 8) & 31))
+#define   _EDP_PSR_ERROR(idx)			(1 << ((idx) * 8 + 2))
+#define   _EDP_PSR_POST_EXIT(idx)		(1 << ((idx) * 8 + 1))
+#define   _EDP_PSR_PRE_ENTRY(idx)		(1 << ((idx) * 8))
+#define   _EDP_PSR_TRANSCODER_A_IDX		1
+#define   _EDP_PSR_TRANSCODER_B_IDX		2
+#define   _EDP_PSR_TRANSCODER_C_IDX		3
+#define   _EDP_PSR_TRANSCODER_EDP_IDX		0
 
 #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
 #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 48df16a02fac..5fdc2f196045 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -83,25 +83,59 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
 	}
 }
 
+static int edp_psr_transcoder_idx(enum transcoder trans)
+{
+	int trans_to_idx[] = {
+		[TRANSCODER_A] = _EDP_PSR_TRANSCODER_A_IDX,
+		[TRANSCODER_B] = _EDP_PSR_TRANSCODER_B_IDX,
+		[TRANSCODER_C] = _EDP_PSR_TRANSCODER_C_IDX,
+		[TRANSCODER_EDP] = _EDP_PSR_TRANSCODER_EDP_IDX,
+	};
+
+	switch (trans) {
+	case TRANSCODER_A:
+	case TRANSCODER_B:
+	case TRANSCODER_C:
+	case TRANSCODER_EDP:
+		return trans_to_idx[trans];
+	default:
+		MISSING_CASE(trans);
+		return trans_to_idx[TRANSCODER_EDP];
+	}
+}
+
+static u32 edp_psr_error(enum transcoder trans)
+{
+	return _EDP_PSR_ERROR(edp_psr_transcoder_idx(trans));
+}
+
+static u32 edp_psr_post_exit(enum transcoder trans)
+{
+	return _EDP_PSR_POST_EXIT(edp_psr_transcoder_idx(trans));
+}
+
+static u32 edp_psr_pre_entry(enum transcoder trans)
+{
+	return _EDP_PSR_PRE_ENTRY(edp_psr_transcoder_idx(trans));
+}
+
 void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32 debug)
 {
 	u32 debug_mask, mask;
+	enum transcoder cpu_transcoder;
+	u32 transcoders = BIT(TRANSCODER_EDP);
 
-	mask = EDP_PSR_ERROR(TRANSCODER_EDP);
-	debug_mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
-		     EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
-
-	if (INTEL_GEN(dev_priv) >= 8) {
-		mask |= EDP_PSR_ERROR(TRANSCODER_A) |
-			EDP_PSR_ERROR(TRANSCODER_B) |
-			EDP_PSR_ERROR(TRANSCODER_C);
-
-		debug_mask |= EDP_PSR_POST_EXIT(TRANSCODER_A) |
-			      EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
-			      EDP_PSR_POST_EXIT(TRANSCODER_B) |
-			      EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
-			      EDP_PSR_POST_EXIT(TRANSCODER_C) |
-			      EDP_PSR_PRE_ENTRY(TRANSCODER_C);
+	if (INTEL_GEN(dev_priv) >= 8)
+		transcoders |= BIT(TRANSCODER_A) |
+			       BIT(TRANSCODER_B) |
+			       BIT(TRANSCODER_C);
+
+	debug_mask = 0;
+	mask = 0;
+	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
+		mask |= edp_psr_error(cpu_transcoder);
+		debug_mask |= edp_psr_post_exit(cpu_transcoder) |
+			      edp_psr_pre_entry(cpu_transcoder);
 	}
 
 	if (debug & I915_PSR_DEBUG_IRQ)
@@ -160,17 +194,17 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
 
 	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
 		/* FIXME: Exit PSR and link train manually when this happens. */
-		if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
+		if (psr_iir & edp_psr_error(cpu_transcoder))
 			DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
 				      transcoder_name(cpu_transcoder));
 
-		if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
+		if (psr_iir & edp_psr_pre_entry(cpu_transcoder)) {
 			dev_priv->psr.last_entry_attempt = time_ns;
 			DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
 				      transcoder_name(cpu_transcoder));
 		}
 
-		if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
+		if (psr_iir & edp_psr_post_exit(cpu_transcoder)) {
 			dev_priv->psr.last_exit = time_ns;
 			DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
 				      transcoder_name(cpu_transcoder));
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/3] drm/i915: Make EDP PSR flags not depend on enum values

[Imre Deak]

Depending on the transcoder enum values to translate from transcoder
to EDP PSR flags can easily break if we add a new transcoder. So remove
the dependency by using an explicit mapping.

While at it also add a WARN for unexpected trancoders.

v2:
- Simplify things by defining flag shift values instead of indices.
- s/trans/cpu_transcoder/ (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 10 +++++---
 drivers/gpu/drm/i915/intel_psr.c | 55 +++++++++++++++++++++++++++-------------
 2 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index edb58af1e903..f80b16e3ff33 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4150,9 +4150,13 @@ enum {
 /* Bspec claims those aren't shifted but stay at 0x64800 */
 #define EDP_PSR_IMR				_MMIO(0x64834)
 #define EDP_PSR_IIR				_MMIO(0x64838)
-#define   EDP_PSR_ERROR(trans)			(1 << (((trans) * 8 + 10) & 31))
-#define   EDP_PSR_POST_EXIT(trans)		(1 << (((trans) * 8 + 9) & 31))
-#define   EDP_PSR_PRE_ENTRY(trans)		(1 << (((trans) * 8 + 8) & 31))
+#define   EDP_PSR_ERROR(shift)			(4 << (shift))
+#define   EDP_PSR_POST_EXIT(shift)		(2 << (shift))
+#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
+#define   EDP_PSR_TRANSCODER_C_SHIFT		24
+#define   EDP_PSR_TRANSCODER_B_SHIFT		16
+#define   EDP_PSR_TRANSCODER_A_SHIFT		8
+#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
 
 #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
 #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 48df16a02fac..26292961d693 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -83,25 +83,42 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
 	}
 }
 
+static int edp_psr_shift(enum transcoder cpu_transcoder)
+{
+	switch (cpu_transcoder) {
+	case TRANSCODER_A:
+		return EDP_PSR_TRANSCODER_A_SHIFT;
+	case TRANSCODER_B:
+		return EDP_PSR_TRANSCODER_B_SHIFT;
+	case TRANSCODER_C:
+		return EDP_PSR_TRANSCODER_C_SHIFT;
+	default:
+		MISSING_CASE(cpu_transcoder);
+		/* fallthrough */
+	case TRANSCODER_EDP:
+		return EDP_PSR_TRANSCODER_EDP_SHIFT;
+	}
+}
+
 void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32 debug)
 {
 	u32 debug_mask, mask;
+	enum transcoder cpu_transcoder;
+	u32 transcoders = BIT(TRANSCODER_EDP);
 
-	mask = EDP_PSR_ERROR(TRANSCODER_EDP);
-	debug_mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
-		     EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
-
-	if (INTEL_GEN(dev_priv) >= 8) {
-		mask |= EDP_PSR_ERROR(TRANSCODER_A) |
-			EDP_PSR_ERROR(TRANSCODER_B) |
-			EDP_PSR_ERROR(TRANSCODER_C);
-
-		debug_mask |= EDP_PSR_POST_EXIT(TRANSCODER_A) |
-			      EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
-			      EDP_PSR_POST_EXIT(TRANSCODER_B) |
-			      EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
-			      EDP_PSR_POST_EXIT(TRANSCODER_C) |
-			      EDP_PSR_PRE_ENTRY(TRANSCODER_C);
+	if (INTEL_GEN(dev_priv) >= 8)
+		transcoders |= BIT(TRANSCODER_A) |
+			       BIT(TRANSCODER_B) |
+			       BIT(TRANSCODER_C);
+
+	debug_mask = 0;
+	mask = 0;
+	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
+		int shift = edp_psr_shift(cpu_transcoder);
+
+		mask |= EDP_PSR_ERROR(shift);
+		debug_mask |= EDP_PSR_POST_EXIT(shift) |
+			      EDP_PSR_PRE_ENTRY(shift);
 	}
 
 	if (debug & I915_PSR_DEBUG_IRQ)
@@ -159,18 +176,20 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
 			       BIT(TRANSCODER_C);
 
 	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
+		int shift = edp_psr_shift(cpu_transcoder);
+
 		/* FIXME: Exit PSR and link train manually when this happens. */
-		if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
+		if (psr_iir & EDP_PSR_ERROR(shift))
 			DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
 				      transcoder_name(cpu_transcoder));
 
-		if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
+		if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
 			dev_priv->psr.last_entry_attempt = time_ns;
 			DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
 				      transcoder_name(cpu_transcoder));
 		}
 
-		if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
+		if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
 			dev_priv->psr.last_exit = time_ns;
 			DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
 				      transcoder_name(cpu_transcoder));
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/3] drm/i915: Make EDP PSR flags not depend on enum values

[Ville Syrjälä]

On Mon, Nov 19, 2018 at 10:46:37PM +0200, Imre Deak wrote:
> Depending on the transcoder enum values to translate from transcoder
> to EDP PSR flags can easily break if we add a new transcoder. So remove
> the dependency by using an explicit mapping.
> 
> While at it also add a WARN for unexpected trancoders.
> 
> v2:
> - Simplify things by defining flag shift values instead of indices.
> - s/trans/cpu_transcoder/ (Ville)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 10 +++++---
>  drivers/gpu/drm/i915/intel_psr.c | 55 +++++++++++++++++++++++++++-------------
>  2 files changed, 44 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index edb58af1e903..f80b16e3ff33 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4150,9 +4150,13 @@ enum {
>  /* Bspec claims those aren't shifted but stay at 0x64800 */
>  #define EDP_PSR_IMR				_MMIO(0x64834)
>  #define EDP_PSR_IIR				_MMIO(0x64838)
> -#define   EDP_PSR_ERROR(trans)			(1 << (((trans) * 8 + 10) & 31))
> -#define   EDP_PSR_POST_EXIT(trans)		(1 << (((trans) * 8 + 9) & 31))
> -#define   EDP_PSR_PRE_ENTRY(trans)		(1 << (((trans) * 8 + 8) & 31))
> +#define   EDP_PSR_ERROR(shift)			(4 << (shift))
> +#define   EDP_PSR_POST_EXIT(shift)		(2 << (shift))
> +#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))

The 1,2,4 suggest that these are values for the same bitfield, which
they are not. So I would prefer 1<<((shift)+n) etc. instead.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +#define   EDP_PSR_TRANSCODER_C_SHIFT		24
> +#define   EDP_PSR_TRANSCODER_B_SHIFT		16
> +#define   EDP_PSR_TRANSCODER_A_SHIFT		8
> +#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
>  
>  #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
>  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 48df16a02fac..26292961d693 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -83,25 +83,42 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> +static int edp_psr_shift(enum transcoder cpu_transcoder)
> +{
> +	switch (cpu_transcoder) {
> +	case TRANSCODER_A:
> +		return EDP_PSR_TRANSCODER_A_SHIFT;
> +	case TRANSCODER_B:
> +		return EDP_PSR_TRANSCODER_B_SHIFT;
> +	case TRANSCODER_C:
> +		return EDP_PSR_TRANSCODER_C_SHIFT;
> +	default:
> +		MISSING_CASE(cpu_transcoder);
> +		/* fallthrough */
> +	case TRANSCODER_EDP:
> +		return EDP_PSR_TRANSCODER_EDP_SHIFT;
> +	}
> +}
> +
>  void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32 debug)
>  {
>  	u32 debug_mask, mask;
> +	enum transcoder cpu_transcoder;
> +	u32 transcoders = BIT(TRANSCODER_EDP);
>  
> -	mask = EDP_PSR_ERROR(TRANSCODER_EDP);
> -	debug_mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
> -		     EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
> -
> -	if (INTEL_GEN(dev_priv) >= 8) {
> -		mask |= EDP_PSR_ERROR(TRANSCODER_A) |
> -			EDP_PSR_ERROR(TRANSCODER_B) |
> -			EDP_PSR_ERROR(TRANSCODER_C);
> -
> -		debug_mask |= EDP_PSR_POST_EXIT(TRANSCODER_A) |
> -			      EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
> -			      EDP_PSR_POST_EXIT(TRANSCODER_B) |
> -			      EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
> -			      EDP_PSR_POST_EXIT(TRANSCODER_C) |
> -			      EDP_PSR_PRE_ENTRY(TRANSCODER_C);
> +	if (INTEL_GEN(dev_priv) >= 8)
> +		transcoders |= BIT(TRANSCODER_A) |
> +			       BIT(TRANSCODER_B) |
> +			       BIT(TRANSCODER_C);
> +
> +	debug_mask = 0;
> +	mask = 0;
> +	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
> +		int shift = edp_psr_shift(cpu_transcoder);
> +
> +		mask |= EDP_PSR_ERROR(shift);
> +		debug_mask |= EDP_PSR_POST_EXIT(shift) |
> +			      EDP_PSR_PRE_ENTRY(shift);
>  	}
>  
>  	if (debug & I915_PSR_DEBUG_IRQ)
> @@ -159,18 +176,20 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
>  			       BIT(TRANSCODER_C);
>  
>  	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
> +		int shift = edp_psr_shift(cpu_transcoder);
> +
>  		/* FIXME: Exit PSR and link train manually when this happens. */
> -		if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> +		if (psr_iir & EDP_PSR_ERROR(shift))
>  			DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
>  				      transcoder_name(cpu_transcoder));
>  
> -		if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> +		if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
>  			dev_priv->psr.last_entry_attempt = time_ns;
>  			DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
>  				      transcoder_name(cpu_transcoder));
>  		}
>  
> -		if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
> +		if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
>  			dev_priv->psr.last_exit = time_ns;
>  			DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
>  				      transcoder_name(cpu_transcoder));
> -- 
> 2.13.2

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 2/3] drm/i915: Make EDP PSR flags not depend on enum values

[Imre Deak]

Depending on the transcoder enum values to translate from transcoder
to EDP PSR flags can easily break if we add a new transcoder. So remove
the dependency by using an explicit mapping.

While at it also add a WARN for unexpected trancoders.

v2:
- Simplify things by defining flag shift values instead of indices.
- s/trans/cpu_transcoder/ (Ville)
v3:
- Define flags to look like seperate bits instead of the values of
  the same bitfield. (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 10 +++++---
 drivers/gpu/drm/i915/intel_psr.c | 55 +++++++++++++++++++++++++++-------------
 2 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index edb58af1e903..e6b371e986ee 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4150,9 +4150,13 @@ enum {
 /* Bspec claims those aren't shifted but stay at 0x64800 */
 #define EDP_PSR_IMR				_MMIO(0x64834)
 #define EDP_PSR_IIR				_MMIO(0x64838)
-#define   EDP_PSR_ERROR(trans)			(1 << (((trans) * 8 + 10) & 31))
-#define   EDP_PSR_POST_EXIT(trans)		(1 << (((trans) * 8 + 9) & 31))
-#define   EDP_PSR_PRE_ENTRY(trans)		(1 << (((trans) * 8 + 8) & 31))
+#define   EDP_PSR_ERROR(shift)			(1 << ((shift) + 2))
+#define   EDP_PSR_POST_EXIT(shift)		(1 << ((shift) + 1))
+#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
+#define   EDP_PSR_TRANSCODER_C_SHIFT		24
+#define   EDP_PSR_TRANSCODER_B_SHIFT		16
+#define   EDP_PSR_TRANSCODER_A_SHIFT		8
+#define   EDP_PSR_TRANSCODER_EDP_SHIFT		0
 
 #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
 #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 48df16a02fac..26292961d693 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -83,25 +83,42 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
 	}
 }
 
+static int edp_psr_shift(enum transcoder cpu_transcoder)
+{
+	switch (cpu_transcoder) {
+	case TRANSCODER_A:
+		return EDP_PSR_TRANSCODER_A_SHIFT;
+	case TRANSCODER_B:
+		return EDP_PSR_TRANSCODER_B_SHIFT;
+	case TRANSCODER_C:
+		return EDP_PSR_TRANSCODER_C_SHIFT;
+	default:
+		MISSING_CASE(cpu_transcoder);
+		/* fallthrough */
+	case TRANSCODER_EDP:
+		return EDP_PSR_TRANSCODER_EDP_SHIFT;
+	}
+}
+
 void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32 debug)
 {
 	u32 debug_mask, mask;
+	enum transcoder cpu_transcoder;
+	u32 transcoders = BIT(TRANSCODER_EDP);
 
-	mask = EDP_PSR_ERROR(TRANSCODER_EDP);
-	debug_mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
-		     EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
-
-	if (INTEL_GEN(dev_priv) >= 8) {
-		mask |= EDP_PSR_ERROR(TRANSCODER_A) |
-			EDP_PSR_ERROR(TRANSCODER_B) |
-			EDP_PSR_ERROR(TRANSCODER_C);
-
-		debug_mask |= EDP_PSR_POST_EXIT(TRANSCODER_A) |
-			      EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
-			      EDP_PSR_POST_EXIT(TRANSCODER_B) |
-			      EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
-			      EDP_PSR_POST_EXIT(TRANSCODER_C) |
-			      EDP_PSR_PRE_ENTRY(TRANSCODER_C);
+	if (INTEL_GEN(dev_priv) >= 8)
+		transcoders |= BIT(TRANSCODER_A) |
+			       BIT(TRANSCODER_B) |
+			       BIT(TRANSCODER_C);
+
+	debug_mask = 0;
+	mask = 0;
+	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
+		int shift = edp_psr_shift(cpu_transcoder);
+
+		mask |= EDP_PSR_ERROR(shift);
+		debug_mask |= EDP_PSR_POST_EXIT(shift) |
+			      EDP_PSR_PRE_ENTRY(shift);
 	}
 
 	if (debug & I915_PSR_DEBUG_IRQ)
@@ -159,18 +176,20 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
 			       BIT(TRANSCODER_C);
 
 	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
+		int shift = edp_psr_shift(cpu_transcoder);
+
 		/* FIXME: Exit PSR and link train manually when this happens. */
-		if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
+		if (psr_iir & EDP_PSR_ERROR(shift))
 			DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
 				      transcoder_name(cpu_transcoder));
 
-		if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
+		if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
 			dev_priv->psr.last_entry_attempt = time_ns;
 			DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
 				      transcoder_name(cpu_transcoder));
 		}
 
-		if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
+		if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
 			dev_priv->psr.last_exit = time_ns;
 			DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
 				      transcoder_name(cpu_transcoder));
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/3] drm/i915: Add code comment on assumption of pipe==transcoder

[Imre Deak]

Add a comment to the pipe and transcoder enum definitions about our
assumption in the code about enum values for pipes and transcoders
with a 1:1 transcoder->pipe mapping.

v2:
- Clarify more what are the assumptions about the enum values. (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index 43eb4ebbcc35..846f63d056fb 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -43,6 +43,11 @@ enum i915_gpio {
 	GPIOM,
 };
 
+/*
+ * Keep the pipe enum values fixed: the code assumes that PIPE_A=0, the
+ * rest have consecutive values and match the enum values of transcoders
+ * with a 1:1 transcoder->pipe mapping.
+ */
 enum pipe {
 	INVALID_PIPE = -1,
 
@@ -57,9 +62,20 @@ enum pipe {
 #define pipe_name(p) ((p) + 'A')
 
 enum transcoder {
+	/*
+	 * The following transcoders have a 1:1 transcoder->pipe mapping, keep
+	 * their values fixed: the code assumes that TRANSCODER_A=0, the rest
+	 * have consecutive values and match the enum values of the pipes they
+	 * map to.
+	 */
 	TRANSCODER_A = 0,
 	TRANSCODER_B,
 	TRANSCODER_C,
+
+	/*
+	 * The following transcoders can map to any pipe, their enum value
+	 * doesn't need to stay fixed.
+	 */
 	TRANSCODER_EDP,
 	TRANSCODER_DSI_0,
 	TRANSCODER_DSI_1,
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Add code comment on assumption of pipe==transcoder

[Imre Deak]

Add a comment to the pipe and transcoder enum definitions about our
assumption in the code that pipe==transcoder for PIPE_A-C /
TRANSCODER_A-C. This means we have to keep the values for these
pipe/transcoder enums fixed.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
index 43eb4ebbcc35..cbb5d79d6a4c 100644
--- a/drivers/gpu/drm/i915/intel_display.h
+++ b/drivers/gpu/drm/i915/intel_display.h
@@ -43,6 +43,10 @@ enum i915_gpio {
 	GPIOM,
 };
 
+/*
+ * Keep the PIPE_A-C values fixed, we assume that pipe==transcoder for
+ * these pipes.
+ */
 enum pipe {
 	INVALID_PIPE = -1,
 
@@ -56,6 +60,10 @@ enum pipe {
 
 #define pipe_name(p) ((p) + 'A')
 
+/*
+ * Keep the TRANSCODER_A-C values fixed, we assume that pipe==transcoder for
+ * these transcoders.
+ */
 enum transcoder {
 	TRANSCODER_A = 0,
 	TRANSCODER_B,
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Add code comment on assumption of pipe==transcoder

[Ville Syrjälä]

On Mon, Nov 19, 2018 at 04:41:09PM +0200, Imre Deak wrote:
> Add a comment to the pipe and transcoder enum definitions about our
> assumption in the code that pipe==transcoder for PIPE_A-C /
> TRANSCODER_A-C. This means we have to keep the values for these
> pipe/transcoder enums fixed.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> index 43eb4ebbcc35..cbb5d79d6a4c 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -43,6 +43,10 @@ enum i915_gpio {
>  	GPIOM,
>  };
>  
> +/*
> + * Keep the PIPE_A-C values fixed, we assume that pipe==transcoder for
> + * these pipes.
> + */

I suspect the A-C part is going to bitrot. Also I'm sure we
make more assupmtions about these values (PIPE_A == 0,
PIPE_N+1 == n+1, etc.). We should probably try to spell it
all out here.

>  enum pipe {
>  	INVALID_PIPE = -1,
>  
> @@ -56,6 +60,10 @@ enum pipe {
>  
>  #define pipe_name(p) ((p) + 'A')
>  
> +/*
> + * Keep the TRANSCODER_A-C values fixed, we assume that pipe==transcoder for
> + * these transcoders.
> + */

Same issue with the A-C part perhaps.

>  enum transcoder {
>  	TRANSCODER_A = 0,
>  	TRANSCODER_B,
> -- 
> 2.13.2

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Add code comment on assumption of pipe==transcoder

[Imre Deak]

On Mon, Nov 19, 2018 at 05:51:31PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 19, 2018 at 04:41:09PM +0200, Imre Deak wrote:
> > Add a comment to the pipe and transcoder enum definitions about our
> > assumption in the code that pipe==transcoder for PIPE_A-C /
> > TRANSCODER_A-C. This means we have to keep the values for these
> > pipe/transcoder enums fixed.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > index 43eb4ebbcc35..cbb5d79d6a4c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@ -43,6 +43,10 @@ enum i915_gpio {
> >  	GPIOM,
> >  };
> >  
> > +/*
> > + * Keep the PIPE_A-C values fixed, we assume that pipe==transcoder for
> > + * these pipes.
> > + */
> 
> I suspect the A-C part is going to bitrot. Also I'm sure we
> make more assupmtions about these values (PIPE_A == 0,
> PIPE_N+1 == n+1, etc.). We should probably try to spell it
> all out here.

Ok, can add instead:

/*
 * Keep the pipe enum values fixed: the code assumes that PIPE_A=0, the
 * rest have consecutive values and match the enum values of transcoders
 * with a 1:1 transcoder->pipe mapping.
 */

> 
> >  enum pipe {
> >  	INVALID_PIPE = -1,
> >  
> > @@ -56,6 +60,10 @@ enum pipe {
> >  
> >  #define pipe_name(p) ((p) + 'A')
> >  
> > +/*
> > + * Keep the TRANSCODER_A-C values fixed, we assume that pipe==transcoder for
> > + * these transcoders.
> > + */
> 
> Same issue with the A-C part perhaps.

and here:

> 
> >  enum transcoder {

/*
 * The following transcoders have a 1:1 transcoder->pipe mapping, keep
 * their values fixed: the code assumes that TRANSCODER_A=0, the rest
 * have consecutive values and match the enum values of the pipes they map
 * to.
 */


> >  	TRANSCODER_A = 0,
> >  	TRANSCODER_B,
> >     TRANSCODER_C,

/*
 * The following transcoders can map to any pipe, their enum value
 * doesn't need to stay fixed.
 */

> >     TRANSCODER_EDP,
> >     TRANSCODER_DSI_0,
> >     TRANSCODER_DSI_1,


> > -- 
> > 2.13.2
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Add code comment on assumption of pipe==transcoder

[Ville Syrjälä]

On Mon, Nov 19, 2018 at 08:43:27PM +0200, Imre Deak wrote:
> On Mon, Nov 19, 2018 at 05:51:31PM +0200, Ville Syrjälä wrote:
> > On Mon, Nov 19, 2018 at 04:41:09PM +0200, Imre Deak wrote:
> > > Add a comment to the pipe and transcoder enum definitions about our
> > > assumption in the code that pipe==transcoder for PIPE_A-C /
> > > TRANSCODER_A-C. This means we have to keep the values for these
> > > pipe/transcoder enums fixed.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.h | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > index 43eb4ebbcc35..cbb5d79d6a4c 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > @@ -43,6 +43,10 @@ enum i915_gpio {
> > >  	GPIOM,
> > >  };
> > >  
> > > +/*
> > > + * Keep the PIPE_A-C values fixed, we assume that pipe==transcoder for
> > > + * these pipes.
> > > + */
> > 
> > I suspect the A-C part is going to bitrot. Also I'm sure we
> > make more assupmtions about these values (PIPE_A == 0,
> > PIPE_N+1 == n+1, etc.). We should probably try to spell it
> > all out here.
> 
> Ok, can add instead:
> 
> /*
>  * Keep the pipe enum values fixed: the code assumes that PIPE_A=0, the
>  * rest have consecutive values and match the enum values of transcoders
>  * with a 1:1 transcoder->pipe mapping.
>  */
> 
> > 
> > >  enum pipe {
> > >  	INVALID_PIPE = -1,
> > >  
> > > @@ -56,6 +60,10 @@ enum pipe {
> > >  
> > >  #define pipe_name(p) ((p) + 'A')
> > >  
> > > +/*
> > > + * Keep the TRANSCODER_A-C values fixed, we assume that pipe==transcoder for
> > > + * these transcoders.
> > > + */
> > 
> > Same issue with the A-C part perhaps.
> 
> and here:
> 
> > 
> > >  enum transcoder {
> 
> /*
>  * The following transcoders have a 1:1 transcoder->pipe mapping, keep
>  * their values fixed: the code assumes that TRANSCODER_A=0, the rest
>  * have consecutive values and match the enum values of the pipes they map
>  * to.
>  */
> 
> 
> > >  	TRANSCODER_A = 0,
> > >  	TRANSCODER_B,
> > >     TRANSCODER_C,
> 
> /*
>  * The following transcoders can map to any pipe, their enum value
>  * doesn't need to stay fixed.
>  */

lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> > >     TRANSCODER_EDP,
> > >     TRANSCODER_DSI_0,
> > >     TRANSCODER_DSI_1,
> 
> 
> > > -- 
> > > 2.13.2
> > 
> > -- 
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Add code comment on assumption of pipe==transcoder

[Lucas De Marchi]

On Mon, Nov 19, 2018 at 08:43:27PM +0200, Imre Deak wrote:
> On Mon, Nov 19, 2018 at 05:51:31PM +0200, Ville Syrjälä wrote:
> > On Mon, Nov 19, 2018 at 04:41:09PM +0200, Imre Deak wrote:
> > > Add a comment to the pipe and transcoder enum definitions about our
> > > assumption in the code that pipe==transcoder for PIPE_A-C /
> > > TRANSCODER_A-C. This means we have to keep the values for these
> > > pipe/transcoder enums fixed.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.h | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > index 43eb4ebbcc35..cbb5d79d6a4c 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > @@ -43,6 +43,10 @@ enum i915_gpio {
> > >  	GPIOM,
> > >  };
> > >  
> > > +/*
> > > + * Keep the PIPE_A-C values fixed, we assume that pipe==transcoder for
> > > + * these pipes.
> > > + */
> > 
> > I suspect the A-C part is going to bitrot. Also I'm sure we
> > make more assupmtions about these values (PIPE_A == 0,
> > PIPE_N+1 == n+1, etc.). We should probably try to spell it
> > all out here.
> 
> Ok, can add instead:
> 
> /*
>  * Keep the pipe enum values fixed: the code assumes that PIPE_A=0, the
>  * rest have consecutive values and match the enum values of transcoders
>  * with a 1:1 transcoder->pipe mapping.

1:1 transcoder <-> pipe mapping, so it doesn't look like it's a pointer :)

>  */
> 
> > 
> > >  enum pipe {
> > >  	INVALID_PIPE = -1,
> > >  
> > > @@ -56,6 +60,10 @@ enum pipe {
> > >  
> > >  #define pipe_name(p) ((p) + 'A')
> > >  
> > > +/*
> > > + * Keep the TRANSCODER_A-C values fixed, we assume that pipe==transcoder for
> > > + * these transcoders.
> > > + */
> > 
> > Same issue with the A-C part perhaps.
> 
> and here:
> 
> > 
> > >  enum transcoder {
> 
> /*
>  * The following transcoders have a 1:1 transcoder->pipe mapping, keep
>  * their values fixed: the code assumes that TRANSCODER_A=0, the rest
>  * have consecutive values and match the enum values of the pipes they map
>  * to.
>  */
> 
> 
> > >  	TRANSCODER_A = 0,
> > >  	TRANSCODER_B,
> > >     TRANSCODER_C,

could we additionally do like below?

TRANSCODER_A = PIPE_A,
TRANSCODER_B = PIPE_B,
TRANSCODER_C = PIPE_C,


Or at least a BUILD_BUG_ON(TRANSCODER_A != PIPE_A || TRANSCODER_B != PIPE_B || TRANSCODER_C != PIPE_C)

Lucas De Marchi

> 
> /*
>  * The following transcoders can map to any pipe, their enum value
>  * doesn't need to stay fixed.
>  */
> 
> > >     TRANSCODER_EDP,
> > >     TRANSCODER_DSI_0,
> > >     TRANSCODER_DSI_1,
> 
> 
> > > -- 
> > > 2.13.2
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Add code comment on assumption of pipe==transcoder

[Imre Deak]

On Mon, Nov 19, 2018 at 02:06:31PM -0800, Lucas De Marchi wrote:
> On Mon, Nov 19, 2018 at 08:43:27PM +0200, Imre Deak wrote:
> > On Mon, Nov 19, 2018 at 05:51:31PM +0200, Ville Syrjälä wrote:
> > > On Mon, Nov 19, 2018 at 04:41:09PM +0200, Imre Deak wrote:
> > > > Add a comment to the pipe and transcoder enum definitions about our
> > > > assumption in the code that pipe==transcoder for PIPE_A-C /
> > > > TRANSCODER_A-C. This means we have to keep the values for these
> > > > pipe/transcoder enums fixed.
> > > > 
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.h | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h
> > > > index 43eb4ebbcc35..cbb5d79d6a4c 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.h
> > > > +++ b/drivers/gpu/drm/i915/intel_display.h
> > > > @@ -43,6 +43,10 @@ enum i915_gpio {
> > > >  	GPIOM,
> > > >  };
> > > >  
> > > > +/*
> > > > + * Keep the PIPE_A-C values fixed, we assume that pipe==transcoder for
> > > > + * these pipes.
> > > > + */
> > > 
> > > I suspect the A-C part is going to bitrot. Also I'm sure we
> > > make more assupmtions about these values (PIPE_A == 0,
> > > PIPE_N+1 == n+1, etc.). We should probably try to spell it
> > > all out here.
> > 
> > Ok, can add instead:
> > 
> > /*
> >  * Keep the pipe enum values fixed: the code assumes that PIPE_A=0, the
> >  * rest have consecutive values and match the enum values of transcoders
> >  * with a 1:1 transcoder->pipe mapping.
> 
> 1:1 transcoder <-> pipe mapping, so it doesn't look like it's a pointer :)

You can only map from transcoder to pipe not the other way around (a pipe
can always be connected to any transcoder). But will add spaces around
'->'.

> 
> >  */
> > 
> > > 
> > > >  enum pipe {
> > > >  	INVALID_PIPE = -1,
> > > >  
> > > > @@ -56,6 +60,10 @@ enum pipe {
> > > >  
> > > >  #define pipe_name(p) ((p) + 'A')
> > > >  
> > > > +/*
> > > > + * Keep the TRANSCODER_A-C values fixed, we assume that pipe==transcoder for
> > > > + * these transcoders.
> > > > + */
> > > 
> > > Same issue with the A-C part perhaps.
> > 
> > and here:
> > 
> > > 
> > > >  enum transcoder {
> > 
> > /*
> >  * The following transcoders have a 1:1 transcoder->pipe mapping, keep
> >  * their values fixed: the code assumes that TRANSCODER_A=0, the rest
> >  * have consecutive values and match the enum values of the pipes they map
> >  * to.
> >  */
> > 
> > 
> > > >  	TRANSCODER_A = 0,
> > > >  	TRANSCODER_B,
> > > >     TRANSCODER_C,
> 
> could we additionally do like below?
> 
> TRANSCODER_A = PIPE_A,
> TRANSCODER_B = PIPE_B,
> TRANSCODER_C = PIPE_C,

Good idea, will change it.

> Or at least a BUILD_BUG_ON(TRANSCODER_A != PIPE_A || TRANSCODER_B != PIPE_B || TRANSCODER_C != PIPE_C)

This could get stale.

> 
> Lucas De Marchi
> 
> > 
> > /*
> >  * The following transcoders can map to any pipe, their enum value
> >  * doesn't need to stay fixed.
> >  */
> > 
> > > >     TRANSCODER_EDP,
> > > >     TRANSCODER_DSI_0,
> > > >     TRANSCODER_DSI_1,
> > 
> > 
> > > > -- 
> > > > 2.13.2
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values
URL   : https://patchwork.freedesktop.org/series/52693/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5160 -> Patchwork_10847 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52693/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    {igt@runner@aborted}:
      fi-icl-u:           NOTRUN -> FAIL

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_create@basic-files:
      fi-icl-u2:          PASS -> DMESG-WARN (fdo#107724)

    igt@gem_exec_suspend@basic-s3:
      fi-cfl-8109u:       PASS -> FAIL (fdo#103375)

    
    ==== Possible fixes ====

    igt@gem_ctx_create@basic-files:
      fi-bsw-kefka:       FAIL (fdo#108656) -> PASS

    igt@gem_exec_suspend@basic-s3:
      fi-icl-u2:          DMESG-WARN (fdo#107724) -> PASS

    igt@gem_mmap_gtt@basic-small-copy:
      fi-glk-dsi:         INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-icl-u:           FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    
    ==== Warnings ====

    igt@i915_selftest@live_contexts:
      fi-icl-u:           DMESG-FAIL (fdo#108569) -> INCOMPLETE (fdo#108315)

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

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315
  fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569
  fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (51 -> 47) ==

  Additional (1): fi-pnv-d510 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_5160 -> Patchwork_10847

  CI_DRM_5160: 80350f0233d79937675bdc76d7864f51843ccc63 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4720: c27aaca295d3ca2a38521e571c012449371e4bb5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10847: 799f685980adcac64afb2b90df4a0f38330553ff @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

799f685980ad drm/i915: Add code comment on assumption of pipe==transcoder
2dee3a985fdf drm/i915: Make EDP PSR flags not depend on enum values
15efd4a42d55 drm/i915: Make pipe/transcoder offsets not depend on enum values

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10847/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values

[Ville Syrjälä]

On Mon, Nov 19, 2018 at 04:41:07PM +0200, Imre Deak wrote:
> Depending on the transcoder enum values to translate from transcoder
> to pipe/transcoder register addresses can easily break if we add a new
> transcoder. So remove the dependency by using named initializers.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pci.c | 52 ++++++++++++++++++++++++++++++-----------
>  1 file changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 983ae7fd8217..1b81d7cb209e 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -33,16 +33,30 @@
>  #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
>  
>  #define GEN_DEFAULT_PIPEOFFSETS \
> -	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> -			  PIPE_C_OFFSET, PIPE_EDP_OFFSET }, \
> -	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> -			   TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET }
> +	.pipe_offsets = { \
> +		[TRANSCODER_A] = PIPE_A_OFFSET,	\
> +		[TRANSCODER_B] = PIPE_B_OFFSET, \
> +		[TRANSCODER_C] = PIPE_C_OFFSET, \
> +		[TRANSCODER_EDP] = PIPE_EDP_OFFSET, \

Hmm. We do define this as int pipe_offsets[I915_MAX_TRANSCODERS], and
PIPECONF/TRANSCONF is per-transcoder so I suppose using TRANSCODER_foo
here make sense.

Couple of things that came to mind while thinking about this:
- pipe_offsets[] & co. could probably be u32 since we don't
  need negative values
- _PIPE_EDP could be removed, which would maybe shrink a few
  arrays based on I915_MAX_PIPES

Patch is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	}, \
> +	.trans_offsets = { \
> +		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> +		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> +		[TRANSCODER_C] = TRANSCODER_C_OFFSET, \
> +		[TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET, \
> +	}
>  
>  #define GEN_CHV_PIPEOFFSETS \
> -	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> -			  CHV_PIPE_C_OFFSET }, \
> -	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> -			   CHV_TRANSCODER_C_OFFSET }
> +	.pipe_offsets = { \
> +		[TRANSCODER_A] = PIPE_A_OFFSET, \
> +		[TRANSCODER_B] = PIPE_B_OFFSET, \
> +		[TRANSCODER_C] = CHV_PIPE_C_OFFSET, \
> +	}, \
> +	.trans_offsets = { \
> +		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> +		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> +		[TRANSCODER_C] = CHV_TRANSCODER_C_OFFSET, \
> +	}
>  
>  #define CURSOR_OFFSETS \
>  	.cursor_offsets = { CURSOR_A_OFFSET, CURSOR_B_OFFSET, CHV_CURSOR_C_OFFSET }
> @@ -592,12 +606,22 @@ static const struct intel_device_info intel_cannonlake_info = {
>  
>  #define GEN11_FEATURES \
>  	GEN10_FEATURES, \
> -	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> -			  PIPE_C_OFFSET, PIPE_EDP_OFFSET, \
> -			  PIPE_DSI0_OFFSET, PIPE_DSI1_OFFSET }, \
> -	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> -			   TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET, \
> -			   TRANSCODER_DSI0_OFFSET, TRANSCODER_DSI1_OFFSET}, \
> +	.pipe_offsets = { \
> +		[TRANSCODER_A] = PIPE_A_OFFSET, \
> +		[TRANSCODER_B] = PIPE_B_OFFSET, \
> +		[TRANSCODER_C] = PIPE_C_OFFSET, \
> +		[TRANSCODER_EDP] = PIPE_EDP_OFFSET, \
> +		[TRANSCODER_DSI_0] = PIPE_DSI0_OFFSET, \
> +		[TRANSCODER_DSI_1] = PIPE_DSI1_OFFSET, \
> +	}, \
> +	.trans_offsets = { \
> +		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> +		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> +		[TRANSCODER_C] = TRANSCODER_C_OFFSET, \
> +		[TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET, \
> +		[TRANSCODER_DSI_0] = TRANSCODER_DSI0_OFFSET, \
> +		[TRANSCODER_DSI_1] = TRANSCODER_DSI1_OFFSET, \
> +	}, \
>  	GEN(11), \
>  	.ddb_size = 2048, \
>  	.has_logical_ring_elsq = 1
> -- 
> 2.13.2

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values

[Imre Deak]

On Mon, Nov 19, 2018 at 05:29:26PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 19, 2018 at 04:41:07PM +0200, Imre Deak wrote:
> > Depending on the transcoder enum values to translate from transcoder
> > to pipe/transcoder register addresses can easily break if we add a new
> > transcoder. So remove the dependency by using named initializers.
> > 
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_pci.c | 52 ++++++++++++++++++++++++++++++-----------
> >  1 file changed, 38 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > index 983ae7fd8217..1b81d7cb209e 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -33,16 +33,30 @@
> >  #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
> >  
> >  #define GEN_DEFAULT_PIPEOFFSETS \
> > -	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> > -			  PIPE_C_OFFSET, PIPE_EDP_OFFSET }, \
> > -	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> > -			   TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET }
> > +	.pipe_offsets = { \
> > +		[TRANSCODER_A] = PIPE_A_OFFSET,	\
> > +		[TRANSCODER_B] = PIPE_B_OFFSET, \
> > +		[TRANSCODER_C] = PIPE_C_OFFSET, \
> > +		[TRANSCODER_EDP] = PIPE_EDP_OFFSET, \
> 
> Hmm. We do define this as int pipe_offsets[I915_MAX_TRANSCODERS], and
> PIPECONF/TRANSCONF is per-transcoder so I suppose using TRANSCODER_foo
> here make sense.
> 
> Couple of things that came to mind while thinking about this:
> - pipe_offsets[] & co. could probably be u32 since we don't
>   need negative values

Ok, can follow up with that.

> - _PIPE_EDP could be removed, which would maybe shrink a few
>   arrays based on I915_MAX_PIPES

Agreed. Looks like all the users are in gvt:

- PIPECONF(_PIPE_EDP) should use PIPECONF(TRANSCODER_EDP) instead. The
  current code would also break if we added a new transcoder.
- AFAICS
	PIPEDSL(_PIPE_EDP)
	PIPESTAT(_PIPE_EDP)
	PIPE_FRMCOUNT_G4X(_PIPE_EDP)
	PIPE_FLIPCOUNT_G4X(_PIPE_EDP)
  are bogus, since these registers don't exist.

Adding gvt folks for these.

> 
> Patch is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > +	}, \
> > +	.trans_offsets = { \
> > +		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> > +		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> > +		[TRANSCODER_C] = TRANSCODER_C_OFFSET, \
> > +		[TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET, \
> > +	}
> >  
> >  #define GEN_CHV_PIPEOFFSETS \
> > -	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> > -			  CHV_PIPE_C_OFFSET }, \
> > -	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> > -			   CHV_TRANSCODER_C_OFFSET }
> > +	.pipe_offsets = { \
> > +		[TRANSCODER_A] = PIPE_A_OFFSET, \
> > +		[TRANSCODER_B] = PIPE_B_OFFSET, \
> > +		[TRANSCODER_C] = CHV_PIPE_C_OFFSET, \
> > +	}, \
> > +	.trans_offsets = { \
> > +		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> > +		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> > +		[TRANSCODER_C] = CHV_TRANSCODER_C_OFFSET, \
> > +	}
> >  
> >  #define CURSOR_OFFSETS \
> >  	.cursor_offsets = { CURSOR_A_OFFSET, CURSOR_B_OFFSET, CHV_CURSOR_C_OFFSET }
> > @@ -592,12 +606,22 @@ static const struct intel_device_info intel_cannonlake_info = {
> >  
> >  #define GEN11_FEATURES \
> >  	GEN10_FEATURES, \
> > -	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> > -			  PIPE_C_OFFSET, PIPE_EDP_OFFSET, \
> > -			  PIPE_DSI0_OFFSET, PIPE_DSI1_OFFSET }, \
> > -	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> > -			   TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET, \
> > -			   TRANSCODER_DSI0_OFFSET, TRANSCODER_DSI1_OFFSET}, \
> > +	.pipe_offsets = { \
> > +		[TRANSCODER_A] = PIPE_A_OFFSET, \
> > +		[TRANSCODER_B] = PIPE_B_OFFSET, \
> > +		[TRANSCODER_C] = PIPE_C_OFFSET, \
> > +		[TRANSCODER_EDP] = PIPE_EDP_OFFSET, \
> > +		[TRANSCODER_DSI_0] = PIPE_DSI0_OFFSET, \
> > +		[TRANSCODER_DSI_1] = PIPE_DSI1_OFFSET, \
> > +	}, \
> > +	.trans_offsets = { \
> > +		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> > +		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> > +		[TRANSCODER_C] = TRANSCODER_C_OFFSET, \
> > +		[TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET, \
> > +		[TRANSCODER_DSI_0] = TRANSCODER_DSI0_OFFSET, \
> > +		[TRANSCODER_DSI_1] = TRANSCODER_DSI1_OFFSET, \
> > +	}, \
> >  	GEN(11), \
> >  	.ddb_size = 2048, \
> >  	.has_logical_ring_elsq = 1
> > -- 
> > 2.13.2
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 5178 bytes --]

On 2018.11.19 20:54:30 +0200, Imre Deak wrote:
> On Mon, Nov 19, 2018 at 05:29:26PM +0200, Ville Syrjälä wrote:
> > On Mon, Nov 19, 2018 at 04:41:07PM +0200, Imre Deak wrote:
> > > Depending on the transcoder enum values to translate from transcoder
> > > to pipe/transcoder register addresses can easily break if we add a new
> > > transcoder. So remove the dependency by using named initializers.
> > > 
> > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_pci.c | 52 ++++++++++++++++++++++++++++++-----------
> > >  1 file changed, 38 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > > index 983ae7fd8217..1b81d7cb209e 100644
> > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > @@ -33,16 +33,30 @@
> > >  #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
> > >  
> > >  #define GEN_DEFAULT_PIPEOFFSETS \
> > > -	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> > > -			  PIPE_C_OFFSET, PIPE_EDP_OFFSET }, \
> > > -	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> > > -			   TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET }
> > > +	.pipe_offsets = { \
> > > +		[TRANSCODER_A] = PIPE_A_OFFSET,	\
> > > +		[TRANSCODER_B] = PIPE_B_OFFSET, \
> > > +		[TRANSCODER_C] = PIPE_C_OFFSET, \
> > > +		[TRANSCODER_EDP] = PIPE_EDP_OFFSET, \
> > 
> > Hmm. We do define this as int pipe_offsets[I915_MAX_TRANSCODERS], and
> > PIPECONF/TRANSCONF is per-transcoder so I suppose using TRANSCODER_foo
> > here make sense.
> > 
> > Couple of things that came to mind while thinking about this:
> > - pipe_offsets[] & co. could probably be u32 since we don't
> >   need negative values
> 
> Ok, can follow up with that.
> 
> > - _PIPE_EDP could be removed, which would maybe shrink a few
> >   arrays based on I915_MAX_PIPES
> 
> Agreed. Looks like all the users are in gvt:
> 
> - PIPECONF(_PIPE_EDP) should use PIPECONF(TRANSCODER_EDP) instead. The
>   current code would also break if we added a new transcoder.

yeah, agreed.

> - AFAICS
> 	PIPEDSL(_PIPE_EDP)
> 	PIPESTAT(_PIPE_EDP)
> 	PIPE_FRMCOUNT_G4X(_PIPE_EDP)
> 	PIPE_FLIPCOUNT_G4X(_PIPE_EDP)
>   are bogus, since these registers don't exist.
>

Thanks for pointing this out, looks they were added in very beginning,
will remove those after double check.

thanks

> Adding gvt folks for these.
> 
> > 
> > Patch is
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > > +	}, \
> > > +	.trans_offsets = { \
> > > +		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> > > +		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> > > +		[TRANSCODER_C] = TRANSCODER_C_OFFSET, \
> > > +		[TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET, \
> > > +	}
> > >  
> > >  #define GEN_CHV_PIPEOFFSETS \
> > > -	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> > > -			  CHV_PIPE_C_OFFSET }, \
> > > -	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> > > -			   CHV_TRANSCODER_C_OFFSET }
> > > +	.pipe_offsets = { \
> > > +		[TRANSCODER_A] = PIPE_A_OFFSET, \
> > > +		[TRANSCODER_B] = PIPE_B_OFFSET, \
> > > +		[TRANSCODER_C] = CHV_PIPE_C_OFFSET, \
> > > +	}, \
> > > +	.trans_offsets = { \
> > > +		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> > > +		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> > > +		[TRANSCODER_C] = CHV_TRANSCODER_C_OFFSET, \
> > > +	}
> > >  
> > >  #define CURSOR_OFFSETS \
> > >  	.cursor_offsets = { CURSOR_A_OFFSET, CURSOR_B_OFFSET, CHV_CURSOR_C_OFFSET }
> > > @@ -592,12 +606,22 @@ static const struct intel_device_info intel_cannonlake_info = {
> > >  
> > >  #define GEN11_FEATURES \
> > >  	GEN10_FEATURES, \
> > > -	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> > > -			  PIPE_C_OFFSET, PIPE_EDP_OFFSET, \
> > > -			  PIPE_DSI0_OFFSET, PIPE_DSI1_OFFSET }, \
> > > -	.trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \
> > > -			   TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET, \
> > > -			   TRANSCODER_DSI0_OFFSET, TRANSCODER_DSI1_OFFSET}, \
> > > +	.pipe_offsets = { \
> > > +		[TRANSCODER_A] = PIPE_A_OFFSET, \
> > > +		[TRANSCODER_B] = PIPE_B_OFFSET, \
> > > +		[TRANSCODER_C] = PIPE_C_OFFSET, \
> > > +		[TRANSCODER_EDP] = PIPE_EDP_OFFSET, \
> > > +		[TRANSCODER_DSI_0] = PIPE_DSI0_OFFSET, \
> > > +		[TRANSCODER_DSI_1] = PIPE_DSI1_OFFSET, \
> > > +	}, \
> > > +	.trans_offsets = { \
> > > +		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
> > > +		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
> > > +		[TRANSCODER_C] = TRANSCODER_C_OFFSET, \
> > > +		[TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET, \
> > > +		[TRANSCODER_DSI_0] = TRANSCODER_DSI0_OFFSET, \
> > > +		[TRANSCODER_DSI_1] = TRANSCODER_DSI1_OFFSET, \
> > > +	}, \
> > >  	GEN(11), \
> > >  	.ddb_size = 2048, \
> > >  	.has_logical_ring_elsq = 1
> > > -- 
> > > 2.13.2
> > 
> > -- 
> > Ville Syrjälä
> > Intel

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values
URL   : https://patchwork.freedesktop.org/series/52693/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5160_full -> Patchwork_10847_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@pm_rpm@pc8-residency:
      {shard-iclb}:       SKIP -> INCOMPLETE

    
    ==== Warnings ====

    igt@tools_test@tools_test:
      {shard-iclb}:       PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-bsd1:
      shard-kbl:          NOTRUN -> FAIL (fdo#103158)

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
      {shard-iclb}:       PASS -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-hang-newfb-render-b:
      shard-glk:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
      shard-snb:          NOTRUN -> DMESG-WARN (fdo#107956) +1
      shard-apl:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_color@pipe-a-ctm-max:
      shard-apl:          PASS -> FAIL (fdo#108147)

    igt@kms_cursor_crc@cursor-64x21-random:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-64x21-sliding:
      shard-apl:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-size-change:
      shard-glk:          PASS -> FAIL (fdo#103232)

    igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
      shard-skl:          PASS -> FAIL (fdo#103184)

    igt@kms_fbcon_fbt@psr:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#107882)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-apl:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move:
      {shard-iclb}:       PASS -> FAIL (fdo#103167)

    igt@kms_hdmi_inject@inject-audio:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#102370)

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      shard-skl:          NOTRUN -> FAIL (fdo#107362)

    igt@kms_plane@plane-position-covered-pipe-c-planes:
      shard-glk:          PASS -> FAIL (fdo#103166) +1

    igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
      shard-glk:          PASS -> FAIL (fdo#108145) +1

    igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
      shard-skl:          PASS -> FAIL (fdo#107815)

    igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
      {shard-iclb}:       PASS -> FAIL (fdo#103166) +1

    igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
      {shard-iclb}:       NOTRUN -> DMESG-WARN (fdo#107724)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    igt@pm_rpm@debugfs-read:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807) +1

    igt@prime_vgem@busy-bsd:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    
    ==== Possible fixes ====

    igt@gem_eio@in-flight-10ms:
      shard-glk:          FAIL (fdo#107799) -> PASS

    igt@gem_exec_whisper@normal:
      shard-skl:          TIMEOUT (fdo#108592) -> PASS

    igt@gem_render_copy_redux@normal:
      shard-kbl:          INCOMPLETE (fdo#106650, fdo#103665) -> PASS

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
      shard-skl:          FAIL (fdo#108470, fdo#108228) -> PASS

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-apl:          FAIL (fdo#106641) -> PASS

    igt@kms_color@pipe-c-degamma:
      shard-apl:          FAIL (fdo#104782) -> PASS

    igt@kms_color@pipe-c-gamma:
      shard-skl:          FAIL (fdo#104782, fdo#108228) -> PASS

    igt@kms_cursor_crc@cursor-128x128-random:
      shard-apl:          FAIL (fdo#103232) -> PASS +5

    igt@kms_cursor_crc@cursor-256x256-dpms:
      shard-skl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_crc@cursor-64x21-onscreen:
      shard-glk:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-apl:          FAIL (fdo#103232, fdo#103191) -> PASS +1

    igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-xtiled:
      shard-skl:          FAIL (fdo#103184) -> PASS

    igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-xtiled:
      {shard-iclb}:       WARN (fdo#108336) -> PASS +3

    igt@kms_flip@flip-vs-expired-vblank:
      shard-skl:          FAIL (fdo#105363) -> PASS
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_flip_tiling@flip-changes-tiling:
      {shard-iclb}:       DMESG-WARN (fdo#107724, fdo#108336) -> PASS +12

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-cpu:
      shard-skl:          FAIL (fdo#103167) -> PASS +6

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt:
      {shard-iclb}:       DMESG-FAIL (fdo#107724) -> PASS +5

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-glk:          FAIL (fdo#103166) -> PASS

    igt@kms_plane@plane-position-covered-pipe-c-planes:
      shard-apl:          FAIL (fdo#103166) -> PASS +1

    igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
      shard-skl:          FAIL (fdo#107815, fdo#108145) -> PASS

    igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
      {shard-iclb}:       FAIL (fdo#103166) -> PASS

    igt@kms_psr@sprite_mmap_gtt:
      {shard-iclb}:       DMESG-WARN (fdo#107724) -> PASS +22

    igt@pm_rpm@universal-planes:
      shard-skl:          INCOMPLETE (fdo#107807) -> PASS

    
    ==== Warnings ====

    igt@i915_suspend@shrink:
      shard-skl:          DMESG-WARN (fdo#108784) -> INCOMPLETE (fdo#106886)

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-cpu:
      {shard-iclb}:       DMESG-FAIL (fdo#107724) -> FAIL (fdo#103167)

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

  fdo#102370 https://bugs.freedesktop.org/show_bug.cgi?id=102370
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106650 https://bugs.freedesktop.org/show_bug.cgi?id=106650
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#107799 https://bugs.freedesktop.org/show_bug.cgi?id=107799
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107882 https://bugs.freedesktop.org/show_bug.cgi?id=107882
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108147 https://bugs.freedesktop.org/show_bug.cgi?id=108147
  fdo#108228 https://bugs.freedesktop.org/show_bug.cgi?id=108228
  fdo#108336 https://bugs.freedesktop.org/show_bug.cgi?id=108336
  fdo#108470 https://bugs.freedesktop.org/show_bug.cgi?id=108470
  fdo#108592 https://bugs.freedesktop.org/show_bug.cgi?id=108592
  fdo#108784 https://bugs.freedesktop.org/show_bug.cgi?id=108784
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (7 -> 7) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5160 -> Patchwork_10847

  CI_DRM_5160: 80350f0233d79937675bdc76d7864f51843ccc63 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4720: c27aaca295d3ca2a38521e571c012449371e4bb5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10847: 799f685980adcac64afb2b90df4a0f38330553ff @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10847/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values (rev2)
URL   : https://patchwork.freedesktop.org/series/52693/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5165 -> Patchwork_10853 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52693/revisions/2/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-icl-u:           NOTRUN -> INCOMPLETE (fdo#107713)

    igt@i915_selftest@live_coherency:
      fi-gdg-551:         PASS -> DMESG-FAIL (fdo#107164)

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)

    
    ==== Possible fixes ====

    igt@gem_ctx_create@basic-files:
      fi-icl-u2:          DMESG-WARN (fdo#107724) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362, fdo#103191) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724


== Participating hosts (53 -> 47) ==

  Additional (1): fi-icl-u 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u3 


== Build changes ==

    * Linux: CI_DRM_5165 -> Patchwork_10853

  CI_DRM_5165: 1715a40008b3d4d47ed0171925063b07ab0012f7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4720: c27aaca295d3ca2a38521e571c012449371e4bb5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10853: 6df0f6ae34ea980b362f0595b578dc9e9e0e0b16 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6df0f6ae34ea drm/i915: Add code comment on assumption of pipe==transcoder
3ad600726483 drm/i915: Make EDP PSR flags not depend on enum values
189055c6a3c6 drm/i915: Make pipe/transcoder offsets not depend on enum values

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10853/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values (rev4)

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Make pipe/transcoder offsets not depend on enum values (rev4)
URL   : https://patchwork.freedesktop.org/series/52693/
State : failure

== Summary ==

Applying: drm/i915: Make pipe/transcoder offsets not depend on enum values
Applying: drm/i915: Add code comment on assumption of pipe==transcoder
Applying: drm/i915: Add code comment on assumption of pipe==transcoder
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/intel_display.h
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_display.h
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_display.h
error: Failed to merge in the changes.
Patch failed at 0003 drm/i915: Add code comment on assumption of pipe==transcoder
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx