drm/i915: Make CHICKEN_TRANS reg not depend on enum value

Submitted by Imre Deak on Nov. 19, 2018, 4:33 p.m.

Details

Message ID 20181119163326.23743-1-imre.deak@intel.com
State New
Headers show
Series "drm/i915: Make CHICKEN_TRANS reg not depend on enum value" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Imre Deak Nov. 19, 2018, 4:33 p.m.
Depending on the transcoder enum values to translate from transcoder to the
corresponding CHICKEN_TRANS register can easily break if we add a new
transcoder. Add an explicit mapping instead, by using helpers to look up the
register instance either by transcoder or port (since unconveniently the
registers have both port and transcoder specific bits).

While at it also check for the correctness of GEN, port, transcoder. I wasn't
sure if psr2_enabled can only be set for GEN9+, but that seems to be the case
indeed (see setting of sink_psr2_support in intel_psr_init_dpcd()).

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  |  7 +++---
 drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_psr.c |  6 +++--
 4 files changed, 51 insertions(+), 17 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 25b069175c2a..1beed39de303 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7399,9 +7399,10 @@  enum {
 #define  BDW_DPRS_MASK_VBLANK_SRD	(1 << 0)
 #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, _CHICKEN_PIPESL_1_B)
 
-#define CHICKEN_TRANS_A         0x420c0
-#define CHICKEN_TRANS_B         0x420c4
-#define CHICKEN_TRANS(trans) _MMIO_TRANS(trans, CHICKEN_TRANS_A, CHICKEN_TRANS_B)
+#define CHICKEN_TRANS_A		_MMIO(0x420c0)
+#define CHICKEN_TRANS_B		_MMIO(0x420c4)
+#define CHICKEN_TRANS_C		_MMIO(0x420c8)
+#define CHICKEN_TRANS_EDP	_MMIO(0x420cc)
 #define  VSC_DATA_SEL_SOFTWARE_CONTROL	(1 << 25) /* GLK and CNL+ */
 #define  DDI_TRAINING_OVERRIDE_ENABLE	(1 << 19)
 #define  DDI_TRAINING_OVERRIDE_VALUE	(1 << 18)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 040483c96029..126e6aac335d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3380,6 +3380,42 @@  static void intel_enable_ddi_dp(struct intel_encoder *encoder,
 		intel_audio_codec_enable(encoder, crtc_state, conn_state);
 }
 
+i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
+				  enum transcoder trans)
+{
+	static const i915_reg_t regs[] = {
+		[TRANSCODER_A] = CHICKEN_TRANS_A,
+		[TRANSCODER_B] = CHICKEN_TRANS_B,
+		[TRANSCODER_C] = CHICKEN_TRANS_C,
+		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
+	};
+
+	WARN_ON(INTEL_GEN(dev_priv) < 9);
+
+	if (WARN_ON(trans >= ARRAY_SIZE(regs) || !regs[trans].reg))
+		trans = TRANSCODER_A;
+
+	return regs[trans];
+}
+
+static i915_reg_t
+gen9_chicken_trans_reg_by_port(struct drm_i915_private *dev_priv,
+			       enum port port)
+{
+	static const enum transcoder port_to_transcoder[] = {
+		[PORT_A] = TRANSCODER_EDP,
+		[PORT_B] = TRANSCODER_A,
+		[PORT_C] = TRANSCODER_B,
+		[PORT_D] = TRANSCODER_C,
+		[PORT_E] = TRANSCODER_A,
+	};
+
+	if (WARN_ON(port < PORT_A || port > PORT_E))
+		port = PORT_A;
+
+	return gen9_chicken_trans_reg(dev_priv, port_to_transcoder[port]);
+}
+
 static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
 				  const struct intel_crtc_state *crtc_state,
 				  const struct drm_connector_state *conn_state)
@@ -3403,17 +3439,10 @@  static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
 		 * the bits affect a specific DDI port rather than
 		 * a specific transcoder.
 		 */
-		static const enum transcoder port_to_transcoder[] = {
-			[PORT_A] = TRANSCODER_EDP,
-			[PORT_B] = TRANSCODER_A,
-			[PORT_C] = TRANSCODER_B,
-			[PORT_D] = TRANSCODER_C,
-			[PORT_E] = TRANSCODER_A,
-		};
-		enum transcoder transcoder = port_to_transcoder[port];
+		i915_reg_t reg = gen9_chicken_trans_reg_by_port(dev_priv, port);
 		u32 val;
 
-		val = I915_READ(CHICKEN_TRANS(transcoder));
+		val = I915_READ(reg);
 
 		if (port == PORT_E)
 			val |= DDIE_TRAINING_OVERRIDE_ENABLE |
@@ -3422,8 +3451,8 @@  static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
 			val |= DDI_TRAINING_OVERRIDE_ENABLE |
 				DDI_TRAINING_OVERRIDE_VALUE;
 
-		I915_WRITE(CHICKEN_TRANS(transcoder), val);
-		POSTING_READ(CHICKEN_TRANS(transcoder));
+		I915_WRITE(reg, val);
+		POSTING_READ(reg);
 
 		udelay(1);
 
@@ -3434,7 +3463,7 @@  static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
 			val &= ~(DDI_TRAINING_OVERRIDE_ENABLE |
 				 DDI_TRAINING_OVERRIDE_VALUE);
 
-		I915_WRITE(CHICKEN_TRANS(transcoder), val);
+		I915_WRITE(reg, val);
 	}
 
 	/* In HDMI/DVI mode, the port width, and swing/emphasis values
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f575ba2a59da..0d064b71002b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1526,6 +1526,8 @@  void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
 
 unsigned int intel_fb_align_height(const struct drm_framebuffer *fb,
 				   int color_plane, unsigned int height);
+i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
+				  enum transcoder trans);
 
 /* intel_audio.c */
 void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 5fdc2f196045..b4d811c62473 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -606,7 +606,9 @@  static void intel_psr_enable_source(struct intel_dp *intel_dp,
 		hsw_psr_setup_aux(intel_dp);
 
 	if (dev_priv->psr.psr2_enabled) {
-		u32 chicken = I915_READ(CHICKEN_TRANS(cpu_transcoder));
+		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
+							cpu_transcoder);
+		u32 chicken = I915_READ(reg);
 
 		if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
 			chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
@@ -614,7 +616,7 @@  static void intel_psr_enable_source(struct intel_dp *intel_dp,
 
 		else
 			chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
-		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
+		I915_WRITE(reg, chicken);
 	}
 
 	/*

Comments

On Mon, Nov 19, 2018 at 06:33:26PM +0200, Imre Deak wrote:
> Depending on the transcoder enum values to translate from transcoder to the
> corresponding CHICKEN_TRANS register can easily break if we add a new
> transcoder. Add an explicit mapping instead, by using helpers to look up the
> register instance either by transcoder or port (since unconveniently the
> registers have both port and transcoder specific bits).
> 
> While at it also check for the correctness of GEN, port, transcoder. I wasn't
> sure if psr2_enabled can only be set for GEN9+, but that seems to be the case
> indeed (see setting of sink_psr2_support in intel_psr_init_dpcd()).
> 
> 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  |  7 +++---
>  drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  drivers/gpu/drm/i915/intel_psr.c |  6 +++--
>  4 files changed, 51 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 25b069175c2a..1beed39de303 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7399,9 +7399,10 @@ enum {
>  #define  BDW_DPRS_MASK_VBLANK_SRD	(1 << 0)
>  #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, _CHICKEN_PIPESL_1_B)
>  
> -#define CHICKEN_TRANS_A         0x420c0
> -#define CHICKEN_TRANS_B         0x420c4
> -#define CHICKEN_TRANS(trans) _MMIO_TRANS(trans, CHICKEN_TRANS_A, CHICKEN_TRANS_B)
> +#define CHICKEN_TRANS_A		_MMIO(0x420c0)
> +#define CHICKEN_TRANS_B		_MMIO(0x420c4)
> +#define CHICKEN_TRANS_C		_MMIO(0x420c8)
> +#define CHICKEN_TRANS_EDP	_MMIO(0x420cc)
>  #define  VSC_DATA_SEL_SOFTWARE_CONTROL	(1 << 25) /* GLK and CNL+ */
>  #define  DDI_TRAINING_OVERRIDE_ENABLE	(1 << 19)
>  #define  DDI_TRAINING_OVERRIDE_VALUE	(1 << 18)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 040483c96029..126e6aac335d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3380,6 +3380,42 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder,
>  		intel_audio_codec_enable(encoder, crtc_state, conn_state);
>  }
>  
> +i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
> +				  enum transcoder trans)

s/trans/cpu_transcoder/ perhaps

> +{
> +	static const i915_reg_t regs[] = {
> +		[TRANSCODER_A] = CHICKEN_TRANS_A,
> +		[TRANSCODER_B] = CHICKEN_TRANS_B,
> +		[TRANSCODER_C] = CHICKEN_TRANS_C,
> +		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
> +	};
> +
> +	WARN_ON(INTEL_GEN(dev_priv) < 9);
> +
> +	if (WARN_ON(trans >= ARRAY_SIZE(regs) || !regs[trans].reg))
> +		trans = TRANSCODER_A;
> +
> +	return regs[trans];
> +}
> +
> +static i915_reg_t
> +gen9_chicken_trans_reg_by_port(struct drm_i915_private *dev_priv,
> +			       enum port port)
> +{
> +	static const enum transcoder port_to_transcoder[] = {
> +		[PORT_A] = TRANSCODER_EDP,
> +		[PORT_B] = TRANSCODER_A,
> +		[PORT_C] = TRANSCODER_B,
> +		[PORT_D] = TRANSCODER_C,
> +		[PORT_E] = TRANSCODER_A,
> +	};
> +
> +	if (WARN_ON(port < PORT_A || port > PORT_E))
> +		port = PORT_A;
> +
> +	return gen9_chicken_trans_reg(dev_priv, port_to_transcoder[port]);

Or we could just return the correct reg directly based on the port,
and  then gen9_chicken_trans_reg() can be moved into intel_psr.c
and made static.

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

> +}
> +
>  static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
>  				  const struct intel_crtc_state *crtc_state,
>  				  const struct drm_connector_state *conn_state)
> @@ -3403,17 +3439,10 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
>  		 * the bits affect a specific DDI port rather than
>  		 * a specific transcoder.
>  		 */
> -		static const enum transcoder port_to_transcoder[] = {
> -			[PORT_A] = TRANSCODER_EDP,
> -			[PORT_B] = TRANSCODER_A,
> -			[PORT_C] = TRANSCODER_B,
> -			[PORT_D] = TRANSCODER_C,
> -			[PORT_E] = TRANSCODER_A,
> -		};
> -		enum transcoder transcoder = port_to_transcoder[port];
> +		i915_reg_t reg = gen9_chicken_trans_reg_by_port(dev_priv, port);
>  		u32 val;
>  
> -		val = I915_READ(CHICKEN_TRANS(transcoder));
> +		val = I915_READ(reg);
>  
>  		if (port == PORT_E)
>  			val |= DDIE_TRAINING_OVERRIDE_ENABLE |
> @@ -3422,8 +3451,8 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
>  			val |= DDI_TRAINING_OVERRIDE_ENABLE |
>  				DDI_TRAINING_OVERRIDE_VALUE;
>  
> -		I915_WRITE(CHICKEN_TRANS(transcoder), val);
> -		POSTING_READ(CHICKEN_TRANS(transcoder));
> +		I915_WRITE(reg, val);
> +		POSTING_READ(reg);
>  
>  		udelay(1);
>  
> @@ -3434,7 +3463,7 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
>  			val &= ~(DDI_TRAINING_OVERRIDE_ENABLE |
>  				 DDI_TRAINING_OVERRIDE_VALUE);
>  
> -		I915_WRITE(CHICKEN_TRANS(transcoder), val);
> +		I915_WRITE(reg, val);
>  	}
>  
>  	/* In HDMI/DVI mode, the port width, and swing/emphasis values
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f575ba2a59da..0d064b71002b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1526,6 +1526,8 @@ void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
>  
>  unsigned int intel_fb_align_height(const struct drm_framebuffer *fb,
>  				   int color_plane, unsigned int height);
> +i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
> +				  enum transcoder trans);
>  
>  /* intel_audio.c */
>  void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 5fdc2f196045..b4d811c62473 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -606,7 +606,9 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  		hsw_psr_setup_aux(intel_dp);
>  
>  	if (dev_priv->psr.psr2_enabled) {
> -		u32 chicken = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> +							cpu_transcoder);
> +		u32 chicken = I915_READ(reg);
>  
>  		if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
>  			chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
> @@ -614,7 +616,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  
>  		else
>  			chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
> -		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
> +		I915_WRITE(reg, chicken);
>  	}
>  
>  	/*
> -- 
> 2.13.2
On Tue, Nov 20, 2018 at 01:00:17AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev2)
> URL   : https://patchwork.freedesktop.org/series/52700/
> State : success

Pushed to -dinq, thanks for the review.

> 
> == Summary ==
> 
> = CI Bug Log - changes from CI_DRM_5162_full -> Patchwork_10851_full =
> 
> == Summary - WARNING ==
> 
>   Minor unknown changes coming with Patchwork_10851_full need to be verified
>   manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_10851_full, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   
> 
> == Possible new issues ==
> 
>   Here are the unknown changes that may have been introduced in Patchwork_10851_full:
> 
>   === IGT changes ===
> 
>     ==== Warnings ====
> 
>     igt@pm_rc6_residency@rc6-accuracy:
>       shard-kbl:          PASS -> SKIP
> 
>     
> == Known issues ==
> 
>   Here are the changes found in Patchwork_10851_full that come from known issues:
> 
>   === IGT changes ===
> 
>     ==== Issues hit ====
> 
>     igt@gem_eio@in-flight-1us:
>       shard-glk:          PASS -> FAIL (fdo#105957)
> 
>     igt@gem_exec_schedule@pi-ringfull-bsd:
>       shard-skl:          NOTRUN -> FAIL (fdo#103158)
> 
>     igt@gem_ppgtt@blt-vs-render-ctxn:
>       shard-skl:          NOTRUN -> TIMEOUT (fdo#108039)
> 
>     igt@i915_suspend@shrink:
>       shard-snb:          NOTRUN -> DMESG-WARN (fdo#102365, fdo#108784)
>       {shard-iclb}:       NOTRUN -> DMESG-WARN (fdo#108784)
> 
>     igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
>       shard-skl:          PASS -> FAIL (fdo#108228, fdo#108470)
> 
>     igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
>       shard-skl:          PASS -> FAIL (fdo#108470, fdo#107815)
> 
>     igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
>       {shard-iclb}:       NOTRUN -> DMESG-WARN (fdo#107956) +1
> 
>     igt@kms_ccs@pipe-a-crc-primary-rotation-180:
>       {shard-iclb}:       NOTRUN -> FAIL (fdo#107725) +2
> 
>     igt@kms_chv_cursor_fail@pipe-b-256x256-top-edge:
>       shard-skl:          PASS -> FAIL (fdo#104671)
> 
>     igt@kms_chv_cursor_fail@pipe-c-128x128-bottom-edge:
>       shard-skl:          NOTRUN -> FAIL (fdo#104671)
> 
>     igt@kms_color@pipe-a-degamma:
>       shard-apl:          PASS -> FAIL (fdo#108145, fdo#104782)
> 
>     igt@kms_color@pipe-c-gamma:
>       shard-skl:          PASS -> FAIL (fdo#108228, fdo#104782)
> 
>     igt@kms_cursor_crc@cursor-128x128-onscreen:
>       shard-apl:          PASS -> FAIL (fdo#103232) +3
> 
>     igt@kms_cursor_crc@cursor-256x256-dpms:
>       shard-skl:          PASS -> FAIL (fdo#103232) +1
> 
>     igt@kms_cursor_crc@cursor-64x21-sliding:
>       shard-glk:          PASS -> FAIL (fdo#103232)
> 
>     igt@kms_cursor_crc@cursor-64x64-suspend:
>       {shard-iclb}:       NOTRUN -> FAIL (fdo#103232) +8
>       shard-apl:          PASS -> FAIL (fdo#103232, fdo#103191)
> 
>     igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-xtiled:
>       shard-skl:          PASS -> FAIL (fdo#103184)
> 
>     igt@kms_draw_crc@draw-method-xrgb8888-pwrite-xtiled:
>       shard-skl:          PASS -> FAIL (fdo#107791)
> 
>     igt@kms_fbcon_fbt@psr:
>       {shard-iclb}:       NOTRUN -> FAIL (fdo#107882)
> 
>     igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
>       shard-glk:          PASS -> FAIL (fdo#105363)
> 
>     igt@kms_flip@modeset-vs-vblank-race:
>       shard-apl:          PASS -> FAIL (fdo#103060)
> 
>     igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
>       shard-apl:          PASS -> FAIL (fdo#103167) +2
> 
>     igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
>       shard-glk:          PASS -> FAIL (fdo#103167) +1
> 
>     igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-cpu:
>       shard-skl:          PASS -> FAIL (fdo#105682) +1
> 
>     igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-gtt:
>       shard-skl:          PASS -> FAIL (fdo#103167) +4
> 
>     igt@kms_hdmi_inject@inject-audio:
>       {shard-iclb}:       NOTRUN -> FAIL (fdo#102370)
> 
>     igt@kms_panel_fitting@legacy:
>       shard-skl:          NOTRUN -> FAIL (fdo#105456)
> 
>     igt@kms_plane@plane-panning-bottom-right-pipe-c-planes:
>       shard-skl:          PASS -> FAIL (fdo#103166)
> 
>     igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
>       shard-glk:          PASS -> FAIL (fdo#108145) +1
> 
>     igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
>       shard-skl:          PASS -> FAIL (fdo#108145, fdo#107815)
> 
>     igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
>       shard-kbl:          NOTRUN -> FAIL (fdo#108145, fdo#108590)
> 
>     igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
>       shard-skl:          NOTRUN -> FAIL (fdo#108145, fdo#107815)
> 
>     igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
>       shard-skl:          PASS -> FAIL (fdo#107815)
> 
>     igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
>       shard-kbl:          NOTRUN -> FAIL (fdo#108145)
> 
>     igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
>       shard-skl:          NOTRUN -> FAIL (fdo#108145)
> 
>     igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
>       shard-apl:          PASS -> FAIL (fdo#103166) +3
> 
>     igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
>       shard-glk:          PASS -> FAIL (fdo#103166)
> 
>     igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
>       {shard-iclb}:       NOTRUN -> FAIL (fdo#103166) +2
> 
>     igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
>       {shard-iclb}:       NOTRUN -> DMESG-WARN (fdo#107724) +1
> 
>     igt@kms_psr@no_drrs:
>       {shard-iclb}:       NOTRUN -> FAIL (fdo#108341)
> 
>     igt@kms_rotation_crc@primary-rotation-180:
>       shard-skl:          NOTRUN -> FAIL (fdo#107815, fdo#103925)
> 
>     igt@kms_sysfs_edid_timing:
>       shard-kbl:          NOTRUN -> FAIL (fdo#100047)
> 
>     igt@pm_backlight@fade_with_suspend:
>       shard-skl:          NOTRUN -> FAIL (fdo#107847)
> 
>     igt@pm_rpm@legacy-planes-dpms:
>       shard-kbl:          PASS -> DMESG-WARN (fdo#105602, fdo#103558) +37
> 
>     igt@pm_rpm@modeset-stress-extra-wait:
>       shard-skl:          PASS -> INCOMPLETE (fdo#107807) +1
> 
>     
>     ==== Possible fixes ====
> 
>     igt@gem_ctx_isolation@rcs0-s3:
>       shard-kbl:          DMESG-WARN (fdo#108566) -> PASS
> 
>     igt@gem_ppgtt@blt-vs-render-ctx0:
>       shard-kbl:          INCOMPLETE (fdo#106887, fdo#103665, fdo#106023) -> PASS
> 
>     igt@gem_workarounds@suspend-resume-fd:
>       shard-skl:          INCOMPLETE (fdo#104108, fdo#107773) -> PASS
> 
>     igt@kms_available_modes_crc@available_mode_test_crc:
>       shard-apl:          FAIL (fdo#106641) -> PASS
> 
>     igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
>       shard-glk:          DMESG-WARN (fdo#107956) -> PASS
> 
>     igt@kms_chv_cursor_fail@pipe-a-128x128-left-edge:
>       shard-skl:          FAIL (fdo#104671) -> PASS
> 
>     igt@kms_chv_cursor_fail@pipe-c-128x128-bottom-edge:
>       shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS +2
> 
>     igt@kms_cursor_crc@cursor-256x256-onscreen:
>       shard-glk:          FAIL (fdo#103232) -> PASS +1
> 
>     igt@kms_cursor_crc@cursor-64x64-suspend:
>       shard-skl:          INCOMPLETE (fdo#104108) -> PASS
> 
>     igt@kms_flip@flip-vs-absolute-wf_vblank:
>       shard-apl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +26
> 
>     igt@kms_flip@flip-vs-expired-vblank-interruptible:
>       shard-glk:          FAIL (fdo#102887, fdo#105363) -> PASS
> 
>     igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
>       shard-apl:          FAIL (fdo#103167) -> PASS
> 
>     igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-pwrite:
>       shard-glk:          FAIL (fdo#103167) -> PASS
> 
>     igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt:
>       shard-skl:          FAIL (fdo#103167) -> PASS
> 
>     igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
>       shard-glk:          FAIL (fdo#103166) -> PASS +1
>       shard-apl:          FAIL (fdo#103166) -> PASS
> 
>     igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
>       {shard-iclb}:       FAIL (fdo#103166) -> PASS
> 
>     igt@pm_rpm@basic-rte:
>       shard-skl:          INCOMPLETE (fdo#107807) -> PASS
> 
>     
>     ==== Warnings ====
> 
>     igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-move:
>       shard-glk:          DMESG-FAIL (fdo#105763, fdo#106538, fdo#103167) -> INCOMPLETE (fdo#103359, k.org#198133)
> 
>     
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
>   fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
>   fdo#102370 https://bugs.freedesktop.org/show_bug.cgi?id=102370
>   fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
>   fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
>   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#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
>   fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
>   fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
>   fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
>   fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
>   fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
>   fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
>   fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
>   fdo#105456 https://bugs.freedesktop.org/show_bug.cgi?id=105456
>   fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
>   fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
>   fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
>   fdo#105957 https://bugs.freedesktop.org/show_bug.cgi?id=105957
>   fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
>   fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
>   fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
>   fdo#106887 https://bugs.freedesktop.org/show_bug.cgi?id=106887
>   fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
>   fdo#107725 https://bugs.freedesktop.org/show_bug.cgi?id=107725
>   fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
>   fdo#107791 https://bugs.freedesktop.org/show_bug.cgi?id=107791
>   fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
>   fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
>   fdo#107847 https://bugs.freedesktop.org/show_bug.cgi?id=107847
>   fdo#107882 https://bugs.freedesktop.org/show_bug.cgi?id=107882
>   fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
>   fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
>   fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
>   fdo#108228 https://bugs.freedesktop.org/show_bug.cgi?id=108228
>   fdo#108341 https://bugs.freedesktop.org/show_bug.cgi?id=108341
>   fdo#108470 https://bugs.freedesktop.org/show_bug.cgi?id=108470
>   fdo#108566 https://bugs.freedesktop.org/show_bug.cgi?id=108566
>   fdo#108590 https://bugs.freedesktop.org/show_bug.cgi?id=108590
>   fdo#108784 https://bugs.freedesktop.org/show_bug.cgi?id=108784
>   k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
> 
> 
> == Participating hosts (7 -> 7) ==
> 
>   No changes in participating hosts
> 
> 
> == Build changes ==
> 
>     * Linux: CI_DRM_5162 -> Patchwork_10851
> 
>   CI_DRM_5162: 30290ec858904adcb173a94adad2bf2052f95f50 @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_4720: c27aaca295d3ca2a38521e571c012449371e4bb5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_10851: 699e7fbd7dffa22fdefaecbee633c066107ac072 @ 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_10851/shards.html
On Mon, 19 Nov 2018, Imre Deak <imre.deak@intel.com> wrote:
> Depending on the transcoder enum values to translate from transcoder to the
> corresponding CHICKEN_TRANS register can easily break if we add a new
> transcoder. Add an explicit mapping instead, by using helpers to look up the
> register instance either by transcoder or port (since unconveniently the
> registers have both port and transcoder specific bits).
>
> While at it also check for the correctness of GEN, port, transcoder. I wasn't
> sure if psr2_enabled can only be set for GEN9+, but that seems to be the case
> indeed (see setting of sink_psr2_support in intel_psr_init_dpcd()).
>
> 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  |  7 +++---
>  drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  drivers/gpu/drm/i915/intel_psr.c |  6 +++--
>  4 files changed, 51 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 25b069175c2a..1beed39de303 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7399,9 +7399,10 @@ enum {
>  #define  BDW_DPRS_MASK_VBLANK_SRD	(1 << 0)
>  #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, _CHICKEN_PIPESL_1_B)
>  
> -#define CHICKEN_TRANS_A         0x420c0
> -#define CHICKEN_TRANS_B         0x420c4
> -#define CHICKEN_TRANS(trans) _MMIO_TRANS(trans, CHICKEN_TRANS_A, CHICKEN_TRANS_B)
> +#define CHICKEN_TRANS_A		_MMIO(0x420c0)
> +#define CHICKEN_TRANS_B		_MMIO(0x420c4)
> +#define CHICKEN_TRANS_C		_MMIO(0x420c8)
> +#define CHICKEN_TRANS_EDP	_MMIO(0x420cc)
>  #define  VSC_DATA_SEL_SOFTWARE_CONTROL	(1 << 25) /* GLK and CNL+ */
>  #define  DDI_TRAINING_OVERRIDE_ENABLE	(1 << 19)
>  #define  DDI_TRAINING_OVERRIDE_VALUE	(1 << 18)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 040483c96029..126e6aac335d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3380,6 +3380,42 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder,
>  		intel_audio_codec_enable(encoder, crtc_state, conn_state);
>  }
>  
> +i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
> +				  enum transcoder trans)
> +{
> +	static const i915_reg_t regs[] = {
> +		[TRANSCODER_A] = CHICKEN_TRANS_A,
> +		[TRANSCODER_B] = CHICKEN_TRANS_B,
> +		[TRANSCODER_C] = CHICKEN_TRANS_C,
> +		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
> +	};
> +
> +	WARN_ON(INTEL_GEN(dev_priv) < 9);
> +
> +	if (WARN_ON(trans >= ARRAY_SIZE(regs) || !regs[trans].reg))
> +		trans = TRANSCODER_A;
> +
> +	return regs[trans];
> +}

I'm late to the party, but it kind of makes me sad that we're now
introducing yet another way to choose registers based on transcoder (or
some other index). And this one is hand-rolled, with local functions
instead of putting it to i915_reg.h.

BR,
Jani.


> +
> +static i915_reg_t
> +gen9_chicken_trans_reg_by_port(struct drm_i915_private *dev_priv,
> +			       enum port port)
> +{
> +	static const enum transcoder port_to_transcoder[] = {
> +		[PORT_A] = TRANSCODER_EDP,
> +		[PORT_B] = TRANSCODER_A,
> +		[PORT_C] = TRANSCODER_B,
> +		[PORT_D] = TRANSCODER_C,
> +		[PORT_E] = TRANSCODER_A,
> +	};
> +
> +	if (WARN_ON(port < PORT_A || port > PORT_E))
> +		port = PORT_A;
> +
> +	return gen9_chicken_trans_reg(dev_priv, port_to_transcoder[port]);
> +}
> +
>  static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
>  				  const struct intel_crtc_state *crtc_state,
>  				  const struct drm_connector_state *conn_state)
> @@ -3403,17 +3439,10 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
>  		 * the bits affect a specific DDI port rather than
>  		 * a specific transcoder.
>  		 */
> -		static const enum transcoder port_to_transcoder[] = {
> -			[PORT_A] = TRANSCODER_EDP,
> -			[PORT_B] = TRANSCODER_A,
> -			[PORT_C] = TRANSCODER_B,
> -			[PORT_D] = TRANSCODER_C,
> -			[PORT_E] = TRANSCODER_A,
> -		};
> -		enum transcoder transcoder = port_to_transcoder[port];
> +		i915_reg_t reg = gen9_chicken_trans_reg_by_port(dev_priv, port);
>  		u32 val;
>  
> -		val = I915_READ(CHICKEN_TRANS(transcoder));
> +		val = I915_READ(reg);
>  
>  		if (port == PORT_E)
>  			val |= DDIE_TRAINING_OVERRIDE_ENABLE |
> @@ -3422,8 +3451,8 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
>  			val |= DDI_TRAINING_OVERRIDE_ENABLE |
>  				DDI_TRAINING_OVERRIDE_VALUE;
>  
> -		I915_WRITE(CHICKEN_TRANS(transcoder), val);
> -		POSTING_READ(CHICKEN_TRANS(transcoder));
> +		I915_WRITE(reg, val);
> +		POSTING_READ(reg);
>  
>  		udelay(1);
>  
> @@ -3434,7 +3463,7 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
>  			val &= ~(DDI_TRAINING_OVERRIDE_ENABLE |
>  				 DDI_TRAINING_OVERRIDE_VALUE);
>  
> -		I915_WRITE(CHICKEN_TRANS(transcoder), val);
> +		I915_WRITE(reg, val);
>  	}
>  
>  	/* In HDMI/DVI mode, the port width, and swing/emphasis values
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f575ba2a59da..0d064b71002b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1526,6 +1526,8 @@ void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
>  
>  unsigned int intel_fb_align_height(const struct drm_framebuffer *fb,
>  				   int color_plane, unsigned int height);
> +i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
> +				  enum transcoder trans);
>  
>  /* intel_audio.c */
>  void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 5fdc2f196045..b4d811c62473 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -606,7 +606,9 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  		hsw_psr_setup_aux(intel_dp);
>  
>  	if (dev_priv->psr.psr2_enabled) {
> -		u32 chicken = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> +		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> +							cpu_transcoder);
> +		u32 chicken = I915_READ(reg);
>  
>  		if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
>  			chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
> @@ -614,7 +616,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  
>  		else
>  			chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
> -		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
> +		I915_WRITE(reg, chicken);
>  	}
>  
>  	/*