* [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value
@ 2018-11-19 16:33 Imre Deak
2018-11-19 16:44 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (9 more replies)
0 siblings, 10 replies; 15+ messages in thread
From: Imre Deak @ 2018-11-19 16:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
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];
+}
+
+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);
}
/*
--
2.13.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Make CHICKEN_TRANS reg not depend on enum value
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
@ 2018-11-19 16:44 ` Patchwork
2018-11-19 17:01 ` ✗ Fi.CI.BAT: failure " Patchwork
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-11-19 16:44 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Make CHICKEN_TRANS reg not depend on enum value
URL : https://patchwork.freedesktop.org/series/52700/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
5e3143add138 drm/i915: Make CHICKEN_TRANS reg not depend on enum value
-:11: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#11:
transcoder. Add an explicit mapping instead, by using helpers to look up the
total: 0 errors, 1 warnings, 0 checks, 118 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Make CHICKEN_TRANS reg not depend on enum value
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
2018-11-19 16:44 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-11-19 17:01 ` Patchwork
2018-11-19 17:06 ` [PATCH] " Ville Syrjälä
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-11-19 17:01 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Make CHICKEN_TRANS reg not depend on enum value
URL : https://patchwork.freedesktop.org/series/52700/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_5161 -> Patchwork_10849 =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_10849 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10849, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/52700/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10849:
=== IGT changes ===
==== Possible regressions ====
igt@i915_selftest@live_objects:
fi-kbl-7560u: NOTRUN -> INCOMPLETE
== Known issues ==
Here are the changes found in Patchwork_10849 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-icl-u: PASS -> INCOMPLETE (fdo#107713)
igt@kms_flip@basic-flip-vs-modeset:
fi-skl-6700hq: PASS -> DMESG-WARN (fdo#105998)
igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
fi-byt-clapper: PASS -> FAIL (fdo#103191, fdo#107362) +1
igt@kms_pipe_crc_basic@read-crc-pipe-a:
fi-byt-clapper: PASS -> FAIL (fdo#107362)
==== 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-xy:
fi-glk-dsi: INCOMPLETE (fdo#103359, k.org#198133) -> PASS
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
fi-byt-clapper: FAIL (fdo#103191, fdo#107362) -> PASS
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
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
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 (52 -> 47) ==
Additional (1): fi-kbl-7560u
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u3
== Build changes ==
* Linux: CI_DRM_5161 -> Patchwork_10849
CI_DRM_5161: 641f11d83579a00ff2ee757b1b587a7b1e8637f9 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4720: c27aaca295d3ca2a38521e571c012449371e4bb5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10849: 5e3143add138160ce3d009483a8afc5213eb8dd6 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
5e3143add138 drm/i915: Make CHICKEN_TRANS reg not depend on enum value
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10849/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
2018-11-19 16:44 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-11-19 17:01 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2018-11-19 17:06 ` Ville Syrjälä
2018-11-19 18:00 ` [PATCH v2] " Imre Deak
` (6 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2018-11-19 17:06 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx, Lucas De Marchi
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
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] drm/i915: Make CHICKEN_TRANS reg not depend on enum value
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
` (2 preceding siblings ...)
2018-11-19 17:06 ` [PATCH] " Ville Syrjälä
@ 2018-11-19 18:00 ` Imre Deak
2018-11-19 18:57 ` ✓ Fi.CI.BAT: success for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev2) Patchwork
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Imre Deak @ 2018-11-19 18:00 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
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()).
v2 (Ville):
- Make gen9_chicken_trans_reg() internal to intel_psr.c.
- s/trans/cpu_transcoder/
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 | 7 ++++---
drivers/gpu/drm/i915/intel_ddi.c | 37 +++++++++++++++++++++++++------------
drivers/gpu/drm/i915/intel_psr.c | 25 +++++++++++++++++++++++--
3 files changed, 52 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..ad11540ac436 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3380,6 +3380,26 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder,
intel_audio_codec_enable(encoder, crtc_state, conn_state);
}
+static i915_reg_t
+gen9_chicken_trans_reg_by_port(struct drm_i915_private *dev_priv,
+ enum port port)
+{
+ static const i915_reg_t regs[] = {
+ [PORT_A] = CHICKEN_TRANS_EDP,
+ [PORT_B] = CHICKEN_TRANS_A,
+ [PORT_C] = CHICKEN_TRANS_B,
+ [PORT_D] = CHICKEN_TRANS_C,
+ [PORT_E] = CHICKEN_TRANS_A,
+ };
+
+ WARN_ON(INTEL_GEN(dev_priv) < 9);
+
+ if (WARN_ON(port < PORT_A || port > PORT_E))
+ port = PORT_A;
+
+ return regs[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 +3423,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 +3435,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 +3447,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_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 5fdc2f196045..1743df91c9a5 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -592,6 +592,25 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
dev_priv->psr.active = true;
}
+static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
+ enum transcoder cpu_transcoder)
+{
+ 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(cpu_transcoder >= ARRAY_SIZE(regs) ||
+ !regs[cpu_transcoder].reg))
+ cpu_transcoder = TRANSCODER_A;
+
+ return regs[cpu_transcoder];
+}
+
static void intel_psr_enable_source(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state)
{
@@ -606,7 +625,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 +635,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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev2)
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
` (3 preceding siblings ...)
2018-11-19 18:00 ` [PATCH v2] " Imre Deak
@ 2018-11-19 18:57 ` Patchwork
2018-11-20 1:00 ` ✓ Fi.CI.IGT: " Patchwork
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-11-19 18:57 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev2)
URL : https://patchwork.freedesktop.org/series/52700/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5162 -> Patchwork_10851 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/52700/revisions/2/mbox/
== Known issues ==
Here are the changes found in Patchwork_10851 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ctx_create@basic-files:
fi-bsw-n3050: PASS -> FAIL (fdo#108656)
igt@i915_selftest@live_contexts:
fi-icl-u: NOTRUN -> DMESG-FAIL (fdo#108569)
igt@i915_selftest@live_gtt:
fi-kbl-7560u: PASS -> INCOMPLETE (fdo#108790)
igt@i915_selftest@live_sanitycheck:
fi-gdg-551: PASS -> INCOMPLETE (fdo#108789)
igt@kms_frontbuffer_tracking@basic:
fi-byt-clapper: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-byt-clapper: PASS -> FAIL (fdo#107362, fdo#103191)
fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718)
==== Possible fixes ====
igt@i915_selftest@live_contexts:
fi-bsw-kefka: DMESG-FAIL (fdo#108656) -> PASS
igt@kms_flip@basic-flip-vs-modeset:
fi-skl-6700hq: DMESG-WARN (fdo#105998) -> PASS
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569
fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656
fdo#108789 https://bugs.freedesktop.org/show_bug.cgi?id=108789
fdo#108790 https://bugs.freedesktop.org/show_bug.cgi?id=108790
== 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_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
== Linux commits ==
699e7fbd7dff drm/i915: Make CHICKEN_TRANS reg not depend on enum value
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10851/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev2)
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
` (4 preceding siblings ...)
2018-11-19 18:57 ` ✓ Fi.CI.BAT: success for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev2) Patchwork
@ 2018-11-20 1:00 ` Patchwork
2018-11-21 11:50 ` Imre Deak
2018-11-23 12:03 ` [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Jani Nikula
` (3 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Patchwork @ 2018-11-20 1:00 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev2)
URL : https://patchwork.freedesktop.org/series/52700/
State : success
== 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: ✓ Fi.CI.IGT: success for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev2)
2018-11-20 1:00 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-11-21 11:50 ` Imre Deak
0 siblings, 0 replies; 15+ messages in thread
From: Imre Deak @ 2018-11-21 11:50 UTC (permalink / raw)
To: intel-gfx, Ville Syrjälä; +Cc: Lucas De Marchi
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
` (5 preceding siblings ...)
2018-11-20 1:00 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-11-23 12:03 ` Jani Nikula
2018-11-23 16:03 ` Imre Deak
2018-11-23 16:56 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev3) Patchwork
` (2 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2018-11-23 12:03 UTC (permalink / raw)
To: Imre Deak, intel-gfx; +Cc: Lucas De Marchi
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);
> }
>
> /*
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value
2018-11-23 12:03 ` [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Jani Nikula
@ 2018-11-23 16:03 ` Imre Deak
2018-11-26 7:33 ` Jani Nikula
0 siblings, 1 reply; 15+ messages in thread
From: Imre Deak @ 2018-11-23 16:03 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Lucas De Marchi
On Fri, Nov 23, 2018 at 02:03:18PM +0200, Jani Nikula wrote:
> 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.
Didn't occur to me that _PICK could be used with named initializers.. We
would still miss the checks which I added here for incorrect indices,
but that's a separate issue from what I wanted to solve in this patch.
So the following would work too:
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 47baf2fe8f71..dfa547832dae 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7399,10 +7399,31 @@ 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 _MMIO(0x420c0)
-#define CHICKEN_TRANS_B _MMIO(0x420c4)
-#define CHICKEN_TRANS_C _MMIO(0x420c8)
-#define CHICKEN_TRANS_EDP _MMIO(0x420cc)
+#define _CHICKEN_TRANS_A 0x420c0
+#define _CHICKEN_TRANS_B 0x420c4
+#define _CHICKEN_TRANS_C 0x420c8
+#define _CHICKEN_TRANS_EDP 0x420cc
+#define CHICKEN_TRANS(trans) _MMIO(_PICK(trans, \
+ [TRANSCODER_A] = \
+ _CHICKEN_TRANS_A, \
+ [TRANSCODER_B] = \
+ _CHICKEN_TRANS_B, \
+ [TRANSCODER_C] = \
+ _CHICKEN_TRANS_C, \
+ [TRANSCODER_EDP] = \
+ _CHICKEN_TRANS_EDP))
+
+#define CHICKEN_TRANS_BYPORT(port) _MMIO(_PICK(port, \
+ [PORT_A] = \
+ _CHICKEN_TRANS_EDP, \
+ [PORT_B] = \
+ _CHICKEN_TRANS_A, \
+ [PORT_C] = \
+ _CHICKEN_TRANS_B, \
+ [PORT_D] = \
+ _CHICKEN_TRANS_C, \
+ [PORT_E] = \
+ _CHICKEN_TRANS_A))
#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 ad11540ac436..c261320330b6 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3380,26 +3380,6 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder,
intel_audio_codec_enable(encoder, crtc_state, conn_state);
}
-static i915_reg_t
-gen9_chicken_trans_reg_by_port(struct drm_i915_private *dev_priv,
- enum port port)
-{
- static const i915_reg_t regs[] = {
- [PORT_A] = CHICKEN_TRANS_EDP,
- [PORT_B] = CHICKEN_TRANS_A,
- [PORT_C] = CHICKEN_TRANS_B,
- [PORT_D] = CHICKEN_TRANS_C,
- [PORT_E] = CHICKEN_TRANS_A,
- };
-
- WARN_ON(INTEL_GEN(dev_priv) < 9);
-
- if (WARN_ON(port < PORT_A || port > PORT_E))
- port = PORT_A;
-
- return regs[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)
@@ -3423,10 +3403,7 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
* the bits affect a specific DDI port rather than
* a specific transcoder.
*/
- i915_reg_t reg = gen9_chicken_trans_reg_by_port(dev_priv, port);
- u32 val;
-
- val = I915_READ(reg);
+ u32 val = I915_READ(CHICKEN_TRANS_BYPORT(port));
if (port == PORT_E)
val |= DDIE_TRAINING_OVERRIDE_ENABLE |
@@ -3435,8 +3412,8 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
val |= DDI_TRAINING_OVERRIDE_ENABLE |
DDI_TRAINING_OVERRIDE_VALUE;
- I915_WRITE(reg, val);
- POSTING_READ(reg);
+ I915_WRITE(CHICKEN_TRANS_BYPORT(port), val);
+ POSTING_READ(CHICKEN_TRANS_BYPORT(port));
udelay(1);
@@ -3447,7 +3424,7 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
val &= ~(DDI_TRAINING_OVERRIDE_ENABLE |
DDI_TRAINING_OVERRIDE_VALUE);
- I915_WRITE(reg, val);
+ I915_WRITE(CHICKEN_TRANS_BYPORT(port), val);
}
/* In HDMI/DVI mode, the port width, and swing/emphasis values
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 54fa17a5596a..76667c5d7800 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -577,25 +577,6 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
dev_priv->psr.active = true;
}
-static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
- enum transcoder cpu_transcoder)
-{
- 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(cpu_transcoder >= ARRAY_SIZE(regs) ||
- !regs[cpu_transcoder].reg))
- cpu_transcoder = TRANSCODER_A;
-
- return regs[cpu_transcoder];
-}
-
static void intel_psr_enable_source(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state)
{
@@ -610,9 +591,10 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
hsw_psr_setup_aux(intel_dp);
if (dev_priv->psr.psr2_enabled) {
- i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
- cpu_transcoder);
- u32 chicken = I915_READ(reg);
+ u32 chicken;
+
+ WARN_ON(INTEL_GEN(dev_priv) < 9);
+ chicken = I915_READ(CHICKEN_TRANS(cpu_transcoder));
if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
@@ -620,7 +602,8 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
else
chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
- I915_WRITE(reg, chicken);
+
+ I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
}
/*
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev3)
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
` (6 preceding siblings ...)
2018-11-23 12:03 ` [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Jani Nikula
@ 2018-11-23 16:56 ` Patchwork
2018-11-23 17:13 ` ✓ Fi.CI.BAT: success " Patchwork
2018-11-23 21:37 ` ✓ Fi.CI.IGT: " Patchwork
9 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-11-23 16:56 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev3)
URL : https://patchwork.freedesktop.org/series/52700/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
3ac91fa0d1e6 drm/i915: Make CHICKEN_TRANS reg not depend on enum value
-:11: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#11:
> > Depending on the transcoder enum values to translate from transcoder to the
-:242: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)
total: 1 errors, 1 warnings, 0 checks, 137 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev3)
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
` (7 preceding siblings ...)
2018-11-23 16:56 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev3) Patchwork
@ 2018-11-23 17:13 ` Patchwork
2018-11-23 21:37 ` ✓ Fi.CI.IGT: " Patchwork
9 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-11-23 17:13 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev3)
URL : https://patchwork.freedesktop.org/series/52700/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5196 -> Patchwork_10899 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/52700/revisions/3/mbox/
== Known issues ==
Here are the changes found in Patchwork_10899 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ctx_create@basic-files:
{fi-icl-u3}: PASS -> DMESG-WARN (fdo#107724)
fi-bsw-kefka: PASS -> FAIL (fdo#108656)
igt@gem_exec_suspend@basic-s3:
fi-icl-u2: NOTRUN -> DMESG-WARN (fdo#107724)
igt@kms_chamelium@common-hpd-after-suspend:
fi-icl-u2: NOTRUN -> DMESG-FAIL (fdo#107732, fdo#108070, fdo#103375)
igt@kms_frontbuffer_tracking@basic:
fi-byt-clapper: PASS -> FAIL (fdo#103167)
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
fi-byt-clapper: PASS -> FAIL (fdo#107362, fdo#103191)
{igt@runner@aborted}:
fi-icl-u2: NOTRUN -> FAIL (fdo#108070)
==== Possible fixes ====
igt@i915_selftest@live_hangcheck:
fi-bwr-2160: DMESG-FAIL (fdo#108735) -> PASS
igt@kms_frontbuffer_tracking@basic:
{fi-icl-u3}: FAIL (fdo#103167) -> PASS
{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#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#107732 https://bugs.freedesktop.org/show_bug.cgi?id=107732
fdo#108070 https://bugs.freedesktop.org/show_bug.cgi?id=108070
fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656
fdo#108735 https://bugs.freedesktop.org/show_bug.cgi?id=108735
== Participating hosts (47 -> 42) ==
Additional (1): fi-icl-u2
Missing (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_5196 -> Patchwork_10899
CI_DRM_5196: 6faf34996697818131ce4448a696e586b31a574e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4726: f48bebb15d3d2c1e6382e1f11b0aeac06fae6082 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10899: 3ac91fa0d1e618bbd0e1231aea4785b6a446e5ac @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
3ac91fa0d1e6 drm/i915: Make CHICKEN_TRANS reg not depend on enum value
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10899/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev3)
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
` (8 preceding siblings ...)
2018-11-23 17:13 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-11-23 21:37 ` Patchwork
9 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-11-23 21:37 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev3)
URL : https://patchwork.freedesktop.org/series/52700/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5196_full -> Patchwork_10899_full =
== Summary - SUCCESS ==
No regressions found.
== Known issues ==
Here are the changes found in Patchwork_10899_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_busy@extended-modeset-hang-newfb-render-b:
shard-skl: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
{shard-iclb}: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_chv_cursor_fail@pipe-c-64x64-left-edge:
shard-skl: PASS -> FAIL (fdo#104671)
igt@kms_cursor_crc@cursor-128x42-offscreen:
shard-apl: PASS -> DMESG-WARN (fdo#103558, fdo#105602) +6
igt@kms_cursor_crc@cursor-256x256-random:
shard-apl: PASS -> FAIL (fdo#103232) +1
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-skl: PASS -> FAIL (fdo#105363)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
{shard-iclb}: PASS -> FAIL (fdo#103167) +4
shard-glk: PASS -> FAIL (fdo#103167) +2
igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move:
{shard-iclb}: NOTRUN -> FAIL (fdo#103167)
igt@kms_frontbuffer_tracking@psr-rgb565-draw-pwrite:
shard-skl: PASS -> FAIL (fdo#103167) +3
igt@kms_plane@pixel-format-pipe-c-planes:
{shard-iclb}: NOTRUN -> FAIL (fdo#103166)
igt@kms_plane@plane-position-covered-pipe-c-planes:
shard-apl: PASS -> FAIL (fdo#103166)
igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
shard-skl: NOTRUN -> FAIL (fdo#108145)
igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
shard-glk: PASS -> FAIL (fdo#108145)
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-apl: NOTRUN -> FAIL (fdo#103166)
igt@kms_psr@no_drrs:
{shard-iclb}: PASS -> FAIL (fdo#108341)
igt@perf_pmu@rc6-runtime-pm-long:
{shard-iclb}: PASS -> FAIL (fdo#105010)
igt@pm_rpm@modeset-non-lpsp:
shard-skl: SKIP -> INCOMPLETE (fdo#107807)
igt@pm_rpm@system-suspend-devices:
shard-skl: PASS -> INCOMPLETE (fdo#107807) +2
==== Possible fixes ====
igt@gem_fence_thrash@bo-copy:
shard-apl: INCOMPLETE (fdo#103927) -> PASS
igt@kms_color@pipe-b-ctm-max:
shard-apl: FAIL (fdo#108147) -> PASS
igt@kms_color@pipe-b-legacy-gamma:
shard-apl: FAIL (fdo#104782) -> PASS
igt@kms_color@pipe-c-ctm-max:
shard-skl: FAIL (fdo#108147) -> PASS
igt@kms_cursor_crc@cursor-128x128-random:
shard-apl: FAIL (fdo#103232) -> PASS
igt@kms_cursor_crc@cursor-256x256-suspend:
shard-glk: FAIL (fdo#103232) -> PASS
igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
shard-glk: FAIL (fdo#106509, fdo#105454) -> PASS
igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
shard-glk: DMESG-WARN (fdo#105763, fdo#106538) -> PASS +1
igt@kms_draw_crc@draw-method-xrgb2101010-blt-xtiled:
shard-skl: FAIL (fdo#103184) -> PASS
igt@kms_flip@2x-flip-vs-expired-vblank:
shard-glk: FAIL (fdo#105363) -> PASS
igt@kms_flip@dpms-vs-vblank-race:
shard-kbl: DMESG-WARN (fdo#103313, fdo#105345) -> PASS
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-skl: FAIL (fdo#100368) -> PASS
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-blt:
shard-glk: DMESG-FAIL (fdo#105763, fdo#106538) -> PASS +1
igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt:
{shard-iclb}: FAIL (fdo#103167) -> PASS +2
igt@kms_plane@plane-position-covered-pipe-c-planes:
shard-glk: FAIL (fdo#103166) -> PASS
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
igt@kms_vblank@pipe-a-ts-continuation-suspend:
shard-hsw: FAIL (fdo#104894) -> PASS
==== Warnings ====
igt@i915_suspend@shrink:
shard-snb: DMESG-WARN (fdo#108784) -> INCOMPLETE (fdo#106886, fdo#105411)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
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#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
fdo#104894 https://bugs.freedesktop.org/show_bug.cgi?id=104894
fdo#105010 https://bugs.freedesktop.org/show_bug.cgi?id=105010
fdo#105345 https://bugs.freedesktop.org/show_bug.cgi?id=105345
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
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#108341 https://bugs.freedesktop.org/show_bug.cgi?id=108341
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_5196 -> Patchwork_10899
CI_DRM_5196: 6faf34996697818131ce4448a696e586b31a574e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4726: f48bebb15d3d2c1e6382e1f11b0aeac06fae6082 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10899: 3ac91fa0d1e618bbd0e1231aea4785b6a446e5ac @ 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_10899/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value
2018-11-23 16:03 ` Imre Deak
@ 2018-11-26 7:33 ` Jani Nikula
2018-11-26 10:14 ` Imre Deak
0 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2018-11-26 7:33 UTC (permalink / raw)
To: imre.deak; +Cc: intel-gfx, Lucas De Marchi
On Fri, 23 Nov 2018, Imre Deak <imre.deak@intel.com> wrote:
> On Fri, Nov 23, 2018 at 02:03:18PM +0200, Jani Nikula wrote:
>> 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.
>
> Didn't occur to me that _PICK could be used with named initializers.. We
> would still miss the checks which I added here for incorrect indices,
> but that's a separate issue from what I wanted to solve in this patch.
> So the following would work too:
Ha! It didn't occur to me either you could use designated initializers
with _PICK. It's kind of neat... in a scary way. :)
We do miss the checks for incorrect indices throughout the code base. So
I'm not that worried about it in this particular case. As you say, a
separate issue.
I'd go with this change.
BR,
Jani.
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>b/drivers/gpu/drm/i915/i915_reg.h index 47baf2fe8f71..dfa547832dae
>100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++
>b/drivers/gpu/drm/i915/i915_reg.h @@ -7399,10 +7399,31 @@ 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 _MMIO(0x420c0)
> -#define CHICKEN_TRANS_B _MMIO(0x420c4)
> -#define CHICKEN_TRANS_C _MMIO(0x420c8)
> -#define CHICKEN_TRANS_EDP _MMIO(0x420cc)
> +#define _CHICKEN_TRANS_A 0x420c0
> +#define _CHICKEN_TRANS_B 0x420c4
> +#define _CHICKEN_TRANS_C 0x420c8
> +#define _CHICKEN_TRANS_EDP 0x420cc
> +#define CHICKEN_TRANS(trans) _MMIO(_PICK(trans, \
> + [TRANSCODER_A] = \
> + _CHICKEN_TRANS_A, \
> + [TRANSCODER_B] = \
> + _CHICKEN_TRANS_B, \
> + [TRANSCODER_C] = \
> + _CHICKEN_TRANS_C, \
> + [TRANSCODER_EDP] = \
> + _CHICKEN_TRANS_EDP))
> +
> +#define CHICKEN_TRANS_BYPORT(port) _MMIO(_PICK(port, \
> + [PORT_A] = \
> + _CHICKEN_TRANS_EDP, \
> + [PORT_B] = \
> + _CHICKEN_TRANS_A, \
> + [PORT_C] = \
> + _CHICKEN_TRANS_B, \
> + [PORT_D] = \
> + _CHICKEN_TRANS_C, \
> + [PORT_E] = \
> + _CHICKEN_TRANS_A))
> #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 ad11540ac436..c261320330b6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3380,26 +3380,6 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder,
> intel_audio_codec_enable(encoder, crtc_state, conn_state);
> }
>
> -static i915_reg_t
> -gen9_chicken_trans_reg_by_port(struct drm_i915_private *dev_priv,
> - enum port port)
> -{
> - static const i915_reg_t regs[] = {
> - [PORT_A] = CHICKEN_TRANS_EDP,
> - [PORT_B] = CHICKEN_TRANS_A,
> - [PORT_C] = CHICKEN_TRANS_B,
> - [PORT_D] = CHICKEN_TRANS_C,
> - [PORT_E] = CHICKEN_TRANS_A,
> - };
> -
> - WARN_ON(INTEL_GEN(dev_priv) < 9);
> -
> - if (WARN_ON(port < PORT_A || port > PORT_E))
> - port = PORT_A;
> -
> - return regs[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)
> @@ -3423,10 +3403,7 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
> * the bits affect a specific DDI port rather than
> * a specific transcoder.
> */
> - i915_reg_t reg = gen9_chicken_trans_reg_by_port(dev_priv, port);
> - u32 val;
> -
> - val = I915_READ(reg);
> + u32 val = I915_READ(CHICKEN_TRANS_BYPORT(port));
>
> if (port == PORT_E)
> val |= DDIE_TRAINING_OVERRIDE_ENABLE |
> @@ -3435,8 +3412,8 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
> val |= DDI_TRAINING_OVERRIDE_ENABLE |
> DDI_TRAINING_OVERRIDE_VALUE;
>
> - I915_WRITE(reg, val);
> - POSTING_READ(reg);
> + I915_WRITE(CHICKEN_TRANS_BYPORT(port), val);
> + POSTING_READ(CHICKEN_TRANS_BYPORT(port));
>
> udelay(1);
>
> @@ -3447,7 +3424,7 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
> val &= ~(DDI_TRAINING_OVERRIDE_ENABLE |
> DDI_TRAINING_OVERRIDE_VALUE);
>
> - I915_WRITE(reg, val);
> + I915_WRITE(CHICKEN_TRANS_BYPORT(port), val);
> }
>
> /* In HDMI/DVI mode, the port width, and swing/emphasis values
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 54fa17a5596a..76667c5d7800 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -577,25 +577,6 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
> dev_priv->psr.active = true;
> }
>
> -static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
> - enum transcoder cpu_transcoder)
> -{
> - 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(cpu_transcoder >= ARRAY_SIZE(regs) ||
> - !regs[cpu_transcoder].reg))
> - cpu_transcoder = TRANSCODER_A;
> -
> - return regs[cpu_transcoder];
> -}
> -
> static void intel_psr_enable_source(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state)
> {
> @@ -610,9 +591,10 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
> hsw_psr_setup_aux(intel_dp);
>
> if (dev_priv->psr.psr2_enabled) {
> - i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> - cpu_transcoder);
> - u32 chicken = I915_READ(reg);
> + u32 chicken;
> +
> + WARN_ON(INTEL_GEN(dev_priv) < 9);
> + chicken = I915_READ(CHICKEN_TRANS(cpu_transcoder));
>
> if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
> chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
> @@ -620,7 +602,8 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>
> else
> chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
> - I915_WRITE(reg, chicken);
> +
> + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
> }
>
> /*
>
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value
2018-11-26 7:33 ` Jani Nikula
@ 2018-11-26 10:14 ` Imre Deak
0 siblings, 0 replies; 15+ messages in thread
From: Imre Deak @ 2018-11-26 10:14 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Lucas De Marchi
On Mon, Nov 26, 2018 at 09:33:13AM +0200, Jani Nikula wrote:
> On Fri, 23 Nov 2018, Imre Deak <imre.deak@intel.com> wrote:
> > On Fri, Nov 23, 2018 at 02:03:18PM +0200, Jani Nikula wrote:
> >> 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.
> >
> > Didn't occur to me that _PICK could be used with named initializers.. We
> > would still miss the checks which I added here for incorrect indices,
> > but that's a separate issue from what I wanted to solve in this patch.
> > So the following would work too:
>
> Ha! It didn't occur to me either you could use designated initializers
> with _PICK. It's kind of neat... in a scary way. :)
Do you mean it starts to look like some python code?:)
>
> We do miss the checks for incorrect indices throughout the code base. So
> I'm not that worried about it in this particular case. As you say, a
> separate issue.
>
> I'd go with this change.
Ok, will follow up after checking the generated code.
>
> BR,
> Jani.
>
>
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >b/drivers/gpu/drm/i915/i915_reg.h index 47baf2fe8f71..dfa547832dae
> >100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++
> >b/drivers/gpu/drm/i915/i915_reg.h @@ -7399,10 +7399,31 @@ 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 _MMIO(0x420c0)
> > -#define CHICKEN_TRANS_B _MMIO(0x420c4)
> > -#define CHICKEN_TRANS_C _MMIO(0x420c8)
> > -#define CHICKEN_TRANS_EDP _MMIO(0x420cc)
> > +#define _CHICKEN_TRANS_A 0x420c0
> > +#define _CHICKEN_TRANS_B 0x420c4
> > +#define _CHICKEN_TRANS_C 0x420c8
> > +#define _CHICKEN_TRANS_EDP 0x420cc
> > +#define CHICKEN_TRANS(trans) _MMIO(_PICK(trans, \
> > + [TRANSCODER_A] = \
> > + _CHICKEN_TRANS_A, \
> > + [TRANSCODER_B] = \
> > + _CHICKEN_TRANS_B, \
> > + [TRANSCODER_C] = \
> > + _CHICKEN_TRANS_C, \
> > + [TRANSCODER_EDP] = \
> > + _CHICKEN_TRANS_EDP))
> > +
> > +#define CHICKEN_TRANS_BYPORT(port) _MMIO(_PICK(port, \
> > + [PORT_A] = \
> > + _CHICKEN_TRANS_EDP, \
> > + [PORT_B] = \
> > + _CHICKEN_TRANS_A, \
> > + [PORT_C] = \
> > + _CHICKEN_TRANS_B, \
> > + [PORT_D] = \
> > + _CHICKEN_TRANS_C, \
> > + [PORT_E] = \
> > + _CHICKEN_TRANS_A))
> > #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 ad11540ac436..c261320330b6 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3380,26 +3380,6 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder,
> > intel_audio_codec_enable(encoder, crtc_state, conn_state);
> > }
> >
> > -static i915_reg_t
> > -gen9_chicken_trans_reg_by_port(struct drm_i915_private *dev_priv,
> > - enum port port)
> > -{
> > - static const i915_reg_t regs[] = {
> > - [PORT_A] = CHICKEN_TRANS_EDP,
> > - [PORT_B] = CHICKEN_TRANS_A,
> > - [PORT_C] = CHICKEN_TRANS_B,
> > - [PORT_D] = CHICKEN_TRANS_C,
> > - [PORT_E] = CHICKEN_TRANS_A,
> > - };
> > -
> > - WARN_ON(INTEL_GEN(dev_priv) < 9);
> > -
> > - if (WARN_ON(port < PORT_A || port > PORT_E))
> > - port = PORT_A;
> > -
> > - return regs[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)
> > @@ -3423,10 +3403,7 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
> > * the bits affect a specific DDI port rather than
> > * a specific transcoder.
> > */
> > - i915_reg_t reg = gen9_chicken_trans_reg_by_port(dev_priv, port);
> > - u32 val;
> > -
> > - val = I915_READ(reg);
> > + u32 val = I915_READ(CHICKEN_TRANS_BYPORT(port));
> >
> > if (port == PORT_E)
> > val |= DDIE_TRAINING_OVERRIDE_ENABLE |
> > @@ -3435,8 +3412,8 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
> > val |= DDI_TRAINING_OVERRIDE_ENABLE |
> > DDI_TRAINING_OVERRIDE_VALUE;
> >
> > - I915_WRITE(reg, val);
> > - POSTING_READ(reg);
> > + I915_WRITE(CHICKEN_TRANS_BYPORT(port), val);
> > + POSTING_READ(CHICKEN_TRANS_BYPORT(port));
> >
> > udelay(1);
> >
> > @@ -3447,7 +3424,7 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
> > val &= ~(DDI_TRAINING_OVERRIDE_ENABLE |
> > DDI_TRAINING_OVERRIDE_VALUE);
> >
> > - I915_WRITE(reg, val);
> > + I915_WRITE(CHICKEN_TRANS_BYPORT(port), val);
> > }
> >
> > /* In HDMI/DVI mode, the port width, and swing/emphasis values
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 54fa17a5596a..76667c5d7800 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -577,25 +577,6 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
> > dev_priv->psr.active = true;
> > }
> >
> > -static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
> > - enum transcoder cpu_transcoder)
> > -{
> > - 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(cpu_transcoder >= ARRAY_SIZE(regs) ||
> > - !regs[cpu_transcoder].reg))
> > - cpu_transcoder = TRANSCODER_A;
> > -
> > - return regs[cpu_transcoder];
> > -}
> > -
> > static void intel_psr_enable_source(struct intel_dp *intel_dp,
> > const struct intel_crtc_state *crtc_state)
> > {
> > @@ -610,9 +591,10 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
> > hsw_psr_setup_aux(intel_dp);
> >
> > if (dev_priv->psr.psr2_enabled) {
> > - i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> > - cpu_transcoder);
> > - u32 chicken = I915_READ(reg);
> > + u32 chicken;
> > +
> > + WARN_ON(INTEL_GEN(dev_priv) < 9);
> > + chicken = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> >
> > if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))
> > chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
> > @@ -620,7 +602,8 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
> >
> > else
> > chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
> > - I915_WRITE(reg, chicken);
> > +
> > + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
> > }
> >
> > /*
> >
>
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-11-26 10:14 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-19 16:33 [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Imre Deak
2018-11-19 16:44 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-11-19 17:01 ` ✗ Fi.CI.BAT: failure " Patchwork
2018-11-19 17:06 ` [PATCH] " Ville Syrjälä
2018-11-19 18:00 ` [PATCH v2] " Imre Deak
2018-11-19 18:57 ` ✓ Fi.CI.BAT: success for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev2) Patchwork
2018-11-20 1:00 ` ✓ Fi.CI.IGT: " Patchwork
2018-11-21 11:50 ` Imre Deak
2018-11-23 12:03 ` [PATCH] drm/i915: Make CHICKEN_TRANS reg not depend on enum value Jani Nikula
2018-11-23 16:03 ` Imre Deak
2018-11-26 7:33 ` Jani Nikula
2018-11-26 10:14 ` Imre Deak
2018-11-23 16:56 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Make CHICKEN_TRANS reg not depend on enum value (rev3) Patchwork
2018-11-23 17:13 ` ✓ Fi.CI.BAT: success " Patchwork
2018-11-23 21:37 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.