Re: [Intel-gfx] [PATCH v13 17/17] drm/i915: Display WA 827
> -Original Message- > From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] > Sent: Wednesday, March 14, 2018 2:43 PM > To: Srinivas, Vidya <vidya.srini...@intel.com>; intel- > g...@lists.freedesktop.org > Cc: Syrjala, Ville <ville.syrj...@intel.com>; Lankhorst, Maarten > <maarten.lankho...@intel.com> > Subject: Re: [Intel-gfx] [PATCH v13 17/17] drm/i915: Display WA 827 > > Op 09-03-18 om 09:49 schreef Vidya Srinivas: > > Display WA 827 applies to GEN9 (excluede GLK) and CNL. > > Switching the plane format from NV12 to RGB and leaving system idle > > results in display underrun and corruption. > > WA: Set the bit 15 & bit 19 to 1b in the CLKGATE_DIS_PSL register for > > the pipe in which NV12 plane is enabled. > > > > Signed-off-by: Chandra Konduru <chandra.kond...@intel.com> > > Signed-off-by: Nabendu Maiti <nabendu.bikash.ma...@intel.com> > > Signed-off-by: Vidya Srinivas <vidya.srini...@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > drivers/gpu/drm/i915/intel_display.c | 31 > > +++ > > 2 files changed, 34 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index 8e61b68..dd8ee98 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -3939,6 +3939,9 @@ enum { > > #define _CLKGATE_DIS_PSL_A 0x46520 > > #define _CLKGATE_DIS_PSL_B 0x46524 > > #define _CLKGATE_DIS_PSL_C 0x46528 > > +#define DUPS1_GATING_DIS (1 << 15) > > +#define DUPS2_GATING_DIS (1 << 19) > > +#define DUPS3_GATING_DIS (1 << 23) > > #define DPF_GATING_DIS (1 << 10) > > #define DPF_RAM_GATING_DIS (1 << 9) > > #define DPFR_GATING_DIS (1 << 8) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 941f00d..f5fdf8a 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -504,6 +504,21 @@ static const struct intel_limit intel_limits_bxt = { > > .p2 = { .p2_slow = 1, .p2_fast = 20 }, }; > > > > +static void > > +skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool > > +enable) { > > + if (IS_SKYLAKE(dev_priv)) > > + return; > > + > > + if (enable) > > + I915_WRITE(CLKGATE_DIS_PSL(pipe), > > + DUPS1_GATING_DIS | DUPS2_GATING_DIS); > > + else > > + I915_WRITE(CLKGATE_DIS_PSL(pipe), > > + I915_READ(CLKGATE_DIS_PSL(pipe)) & > > + ~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)); > > +} > > + > > static bool > > needs_modeset(const struct drm_crtc_state *state) { @@ -3151,6 > > +3166,9 @@ int skl_check_plane_surface(const struct intel_crtc_state > *crtc_state, > > const struct drm_framebuffer *fb = plane_state->base.fb; > > unsigned int rotation = plane_state->base.rotation; > > int ret; > > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc); > > + struct drm_i915_private *dev_priv = > > + to_i915(plane_state->base.plane->dev); > > > > if (rotation & DRM_MODE_REFLECT_X && > > fb->modifier == DRM_FORMAT_MOD_LINEAR) { @@ -3175,6 > +3193,13 @@ > > int skl_check_plane_surface(const struct intel_crtc_state *crtc_state, > > ret = skl_check_nv12_aux_surface(plane_state); > > if (ret) > > return ret; > > + > > + /* Display WA 827 */ > > + if (INTEL_GEN(dev_priv) == 9 || IS_CANNONLAKE(dev_priv)) > { > > + if (!IS_GEMINILAKE(dev_priv)) > > + skl_wa_clkgate(dev_priv, > > + intel_crtc->pipe, true); > > + } > > } else if (fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS || > >fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS) { > > ret = skl_check_ccs_aux_surface(plane_state); > This is absolutely the wrong place to put this. atomic check can be called > without any power well enabled, and the only way The right place is > intel_pre_plane_update/intel_post_plane_update. > > We already have a active_planes property, so we would need something > similar for nv12 planes. > > In pre_plane_update, we could enable the w/a if new_crtc_state- > >nv12_planes. > In post_plane_update, we can disable it if !new_crtc_state->nv12_planes. Thank you, I will make this change and float the patch. Regards Vidya ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v13 17/17] drm/i915: Display WA 827
Op 09-03-18 om 09:49 schreef Vidya Srinivas: > Display WA 827 applies to GEN9 (excluede GLK) and CNL. > Switching the plane format from NV12 to RGB and leaving system idle > results in display underrun and corruption. > WA: Set the bit 15 & bit 19 to 1b in the CLKGATE_DIS_PSL > register for the pipe in which NV12 plane is enabled. > > Signed-off-by: Chandra Konduru> Signed-off-by: Nabendu Maiti > Signed-off-by: Vidya Srinivas > --- > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > drivers/gpu/drm/i915/intel_display.c | 31 +++ > 2 files changed, 34 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 8e61b68..dd8ee98 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3939,6 +3939,9 @@ enum { > #define _CLKGATE_DIS_PSL_A 0x46520 > #define _CLKGATE_DIS_PSL_B 0x46524 > #define _CLKGATE_DIS_PSL_C 0x46528 > +#define DUPS1_GATING_DIS (1 << 15) > +#define DUPS2_GATING_DIS (1 << 19) > +#define DUPS3_GATING_DIS (1 << 23) > #define DPF_GATING_DIS (1 << 10) > #define DPF_RAM_GATING_DIS (1 << 9) > #define DPFR_GATING_DIS(1 << 8) > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 941f00d..f5fdf8a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -504,6 +504,21 @@ static const struct intel_limit intel_limits_bxt = { > .p2 = { .p2_slow = 1, .p2_fast = 20 }, > }; > > +static void > +skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool enable) > +{ > + if (IS_SKYLAKE(dev_priv)) > + return; > + > + if (enable) > + I915_WRITE(CLKGATE_DIS_PSL(pipe), > +DUPS1_GATING_DIS | DUPS2_GATING_DIS); > + else > + I915_WRITE(CLKGATE_DIS_PSL(pipe), > +I915_READ(CLKGATE_DIS_PSL(pipe)) & > +~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)); > +} > + > static bool > needs_modeset(const struct drm_crtc_state *state) > { > @@ -3151,6 +3166,9 @@ int skl_check_plane_surface(const struct > intel_crtc_state *crtc_state, > const struct drm_framebuffer *fb = plane_state->base.fb; > unsigned int rotation = plane_state->base.rotation; > int ret; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc); > + struct drm_i915_private *dev_priv = > + to_i915(plane_state->base.plane->dev); > > if (rotation & DRM_MODE_REFLECT_X && > fb->modifier == DRM_FORMAT_MOD_LINEAR) { > @@ -3175,6 +3193,13 @@ int skl_check_plane_surface(const struct > intel_crtc_state *crtc_state, > ret = skl_check_nv12_aux_surface(plane_state); > if (ret) > return ret; > + > + /* Display WA 827 */ > + if (INTEL_GEN(dev_priv) == 9 || IS_CANNONLAKE(dev_priv)) { > + if (!IS_GEMINILAKE(dev_priv)) > + skl_wa_clkgate(dev_priv, > +intel_crtc->pipe, true); > + } > } else if (fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS || > fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS) { > ret = skl_check_ccs_aux_surface(plane_state); This is absolutely the wrong place to put this. atomic check can be called without any power well enabled, and the only way The right place is intel_pre_plane_update/intel_post_plane_update. We already have a active_planes property, so we would need something similar for nv12 planes. In pre_plane_update, we could enable the w/a if new_crtc_state->nv12_planes. In post_plane_update, we can disable it if !new_crtc_state->nv12_planes. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v13 17/17] drm/i915: Display WA 827
On 09.03.2018 10:49, Vidya Srinivas wrote: Display WA 827 applies to GEN9 (excluede GLK) and CNL. Switching the plane format from NV12 to RGB and leaving system idle results in display underrun and corruption. WA: Set the bit 15 & bit 19 to 1b in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane is enabled. Signed-off-by: Chandra KonduruSigned-off-by: Nabendu Maiti Signed-off-by: Vidya Srinivas --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 31 +++ 2 files changed, 34 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 8e61b68..dd8ee98 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3939,6 +3939,9 @@ enum { #define _CLKGATE_DIS_PSL_A0x46520 #define _CLKGATE_DIS_PSL_B0x46524 #define _CLKGATE_DIS_PSL_C0x46528 +#define DUPS1_GATING_DIS (1 << 15) +#define DUPS2_GATING_DIS (1 << 19) +#define DUPS3_GATING_DIS (1 << 23) #define DPF_GATING_DIS (1 << 10) #define DPF_RAM_GATING_DIS (1 << 9) #define DPFR_GATING_DIS (1 << 8) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 941f00d..f5fdf8a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -504,6 +504,21 @@ static const struct intel_limit intel_limits_bxt = { .p2 = { .p2_slow = 1, .p2_fast = 20 }, }; +static void +skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool enable) +{ + if (IS_SKYLAKE(dev_priv)) + return; + + if (enable) + I915_WRITE(CLKGATE_DIS_PSL(pipe), + DUPS1_GATING_DIS | DUPS2_GATING_DIS); + else + I915_WRITE(CLKGATE_DIS_PSL(pipe), + I915_READ(CLKGATE_DIS_PSL(pipe)) & + ~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)); +} + static bool needs_modeset(const struct drm_crtc_state *state) { @@ -3151,6 +3166,9 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state, const struct drm_framebuffer *fb = plane_state->base.fb; unsigned int rotation = plane_state->base.rotation; int ret; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc); + struct drm_i915_private *dev_priv = + to_i915(plane_state->base.plane->dev); if (rotation & DRM_MODE_REFLECT_X && fb->modifier == DRM_FORMAT_MOD_LINEAR) { @@ -3175,6 +3193,13 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state, ret = skl_check_nv12_aux_surface(plane_state); if (ret) return ret; + + /* Display WA 827 */ + if (INTEL_GEN(dev_priv) == 9 || IS_CANNONLAKE(dev_priv)) { + if (!IS_GEMINILAKE(dev_priv)) Maybe these nested if's could be combined? + skl_wa_clkgate(dev_priv, + intel_crtc->pipe, true); + } } else if (fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS || fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS) { ret = skl_check_ccs_aux_surface(plane_state); @@ -5746,6 +5771,12 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state, intel_ddi_disable_pipe_clock(intel_crtc->config); intel_encoders_post_disable(crtc, old_crtc_state, old_state); + + if (INTEL_GEN(dev_priv) == 9 || IS_CANNONLAKE(dev_priv)) { + if (!IS_GEMINILAKE(dev_priv)) same as above. + skl_wa_clkgate(dev_priv, + intel_crtc->pipe, false); + } } static void i9xx_pfit_enable(struct intel_crtc *crtc) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v13 17/17] drm/i915: Display WA 827
Display WA 827 applies to GEN9 (excluede GLK) and CNL. Switching the plane format from NV12 to RGB and leaving system idle results in display underrun and corruption. WA: Set the bit 15 & bit 19 to 1b in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane is enabled. Signed-off-by: Chandra KonduruSigned-off-by: Nabendu Maiti Signed-off-by: Vidya Srinivas --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 31 +++ 2 files changed, 34 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 8e61b68..dd8ee98 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3939,6 +3939,9 @@ enum { #define _CLKGATE_DIS_PSL_A 0x46520 #define _CLKGATE_DIS_PSL_B 0x46524 #define _CLKGATE_DIS_PSL_C 0x46528 +#define DUPS1_GATING_DIS (1 << 15) +#define DUPS2_GATING_DIS (1 << 19) +#define DUPS3_GATING_DIS (1 << 23) #define DPF_GATING_DIS (1 << 10) #define DPF_RAM_GATING_DIS (1 << 9) #define DPFR_GATING_DIS (1 << 8) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 941f00d..f5fdf8a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -504,6 +504,21 @@ static const struct intel_limit intel_limits_bxt = { .p2 = { .p2_slow = 1, .p2_fast = 20 }, }; +static void +skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool enable) +{ + if (IS_SKYLAKE(dev_priv)) + return; + + if (enable) + I915_WRITE(CLKGATE_DIS_PSL(pipe), + DUPS1_GATING_DIS | DUPS2_GATING_DIS); + else + I915_WRITE(CLKGATE_DIS_PSL(pipe), + I915_READ(CLKGATE_DIS_PSL(pipe)) & + ~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)); +} + static bool needs_modeset(const struct drm_crtc_state *state) { @@ -3151,6 +3166,9 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state, const struct drm_framebuffer *fb = plane_state->base.fb; unsigned int rotation = plane_state->base.rotation; int ret; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc); + struct drm_i915_private *dev_priv = + to_i915(plane_state->base.plane->dev); if (rotation & DRM_MODE_REFLECT_X && fb->modifier == DRM_FORMAT_MOD_LINEAR) { @@ -3175,6 +3193,13 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state, ret = skl_check_nv12_aux_surface(plane_state); if (ret) return ret; + + /* Display WA 827 */ + if (INTEL_GEN(dev_priv) == 9 || IS_CANNONLAKE(dev_priv)) { + if (!IS_GEMINILAKE(dev_priv)) + skl_wa_clkgate(dev_priv, + intel_crtc->pipe, true); + } } else if (fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS || fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS) { ret = skl_check_ccs_aux_surface(plane_state); @@ -5746,6 +5771,12 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state, intel_ddi_disable_pipe_clock(intel_crtc->config); intel_encoders_post_disable(crtc, old_crtc_state, old_state); + + if (INTEL_GEN(dev_priv) == 9 || IS_CANNONLAKE(dev_priv)) { + if (!IS_GEMINILAKE(dev_priv)) + skl_wa_clkgate(dev_priv, + intel_crtc->pipe, false); + } } static void i9xx_pfit_enable(struct intel_crtc *crtc) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx