Re: [Intel-gfx] [PATCH 4/4] drm/i915: Enable display workaround 827 for all planes.
Patch looks good to me.
Reviewed-by: Vidya Srinivas
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Maarten Lankhorst
> Sent: Monday, April 9, 2018 6:17 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 4/4] drm/i915: Enable display workaround 827
> for all planes.
>
> The workaround was applied only to the primary plane, but is required on all
> planes. Iterate over all planes in the crtc atomic check to see if the
> workaround is enabled, and only perform the actual toggling in the pre/post
> plane update functions.
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/intel_display.c | 40 +-
> --
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 2 files changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 487a6e235222..829b593a3cee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5162,7 +5162,6 @@ static void intel_post_plane_update(struct
> intel_crtc_state *old_crtc_state)
> if (old_primary_state) {
> struct drm_plane_state *new_primary_state =
> drm_atomic_get_new_plane_state(old_state,
> primary);
> - struct drm_framebuffer *fb = new_primary_state->fb;
>
> intel_fbc_post_update(crtc);
>
> @@ -5170,15 +5169,11 @@ static void intel_post_plane_update(struct
> intel_crtc_state *old_crtc_state)
> (needs_modeset(&pipe_config->base) ||
>!old_primary_state->visible))
> intel_post_enable_primary(&crtc->base,
> pipe_config);
> -
> - /* Display WA 827 */
> - if ((INTEL_GEN(dev_priv) == 9 &&
> !IS_GEMINILAKE(dev_priv)) ||
> - IS_CANNONLAKE(dev_priv)) {
> - if (fb && fb->format->format ==
> DRM_FORMAT_NV12)
> - skl_wa_clkgate(dev_priv, crtc->pipe, false);
> - }
> -
> }
> +
> + /* Display WA 827 */
> + if (old_crtc_state->nv12_wa && !pipe_config->nv12_wa)
> + skl_wa_clkgate(dev_priv, crtc->pipe, false);
> }
>
> static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
> @@ -5202,14 +5197,6 @@ static void intel_pre_plane_update(struct
> intel_crtc_state *old_crtc_state,
> struct intel_plane_state *new_primary_state =
> intel_atomic_get_new_plane_state(old_intel_state,
>
> to_intel_plane(primary));
> - struct drm_framebuffer *fb = new_primary_state->base.fb;
> -
> - /* Display WA 827 */
> - if ((INTEL_GEN(dev_priv) == 9 &&
> !IS_GEMINILAKE(dev_priv)) ||
> - IS_CANNONLAKE(dev_priv)) {
> - if (fb && fb->format->format ==
> DRM_FORMAT_NV12)
> - skl_wa_clkgate(dev_priv, crtc->pipe, true);
> - }
>
> intel_fbc_pre_update(crtc, pipe_config, new_primary_state);
> /*
> @@ -5221,6 +5208,10 @@ static void intel_pre_plane_update(struct
> intel_crtc_state *old_crtc_state,
> intel_set_cpu_fifo_underrun_reporting(dev_priv,
> crtc->pipe, false);
> }
>
> + /* Display WA 827 */
> + if (!old_crtc_state->nv12_wa && pipe_config->nv12_wa)
> + skl_wa_clkgate(dev_priv, crtc->pipe, true);
> +
> /*
>* Vblank time updates from the shadow to live plane control
> register
>* are blocked if the memory self-refresh mode is active at that @@
> -10485,6 +10476,21 @@ static int intel_crtc_atomic_check(struct drm_crtc
> *crtc,
> return ret;
> }
>
> + pipe_config->nv12_wa = false;
> +
> + /* Apply display WA 827 if required. */
> + if (crtc_state->active &&
> + ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
> + IS_CANNONLAKE(dev_priv))) {
> + struct drm_plane *plane;
> + const struct drm_plane_state *plane_state;
> +
> + drm_atomic_crtc_state_for_each_plane_state(plane,
> plane_state, crtc_state)
> + if (plane_state->fb &&
> + plane_state->fb->format->format ==
> DRM_FORMAT_NV12)
> + pipe_config->nv12_wa = true;
> + }
> +
> if (crtc_state->color_mgmt_changed) {
>
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Enable display workaround 827 for all planes.
On Mon, Apr 09, 2018 at 02:46:56PM +0200, Maarten Lankhorst wrote:
> The workaround was applied only to the primary plane, but is required
> on all planes. Iterate over all planes in the crtc atomic check to see
> if the workaround is enabled, and only perform the actual toggling in
> the pre/post plane update functions.
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/intel_display.c | 40
> +---
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 2 files changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 487a6e235222..829b593a3cee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5162,7 +5162,6 @@ static void intel_post_plane_update(struct
> intel_crtc_state *old_crtc_state)
> if (old_primary_state) {
> struct drm_plane_state *new_primary_state =
> drm_atomic_get_new_plane_state(old_state, primary);
> - struct drm_framebuffer *fb = new_primary_state->fb;
>
> intel_fbc_post_update(crtc);
>
> @@ -5170,15 +5169,11 @@ static void intel_post_plane_update(struct
> intel_crtc_state *old_crtc_state)
> (needs_modeset(&pipe_config->base) ||
>!old_primary_state->visible))
> intel_post_enable_primary(&crtc->base, pipe_config);
> -
> - /* Display WA 827 */
> - if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
> - IS_CANNONLAKE(dev_priv)) {
> - if (fb && fb->format->format == DRM_FORMAT_NV12)
> - skl_wa_clkgate(dev_priv, crtc->pipe, false);
> - }
> -
> }
> +
> + /* Display WA 827 */
> + if (old_crtc_state->nv12_wa && !pipe_config->nv12_wa)
> + skl_wa_clkgate(dev_priv, crtc->pipe, false);
> }
>
> static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
> @@ -5202,14 +5197,6 @@ static void intel_pre_plane_update(struct
> intel_crtc_state *old_crtc_state,
> struct intel_plane_state *new_primary_state =
> intel_atomic_get_new_plane_state(old_intel_state,
>
> to_intel_plane(primary));
> - struct drm_framebuffer *fb = new_primary_state->base.fb;
> -
> - /* Display WA 827 */
> - if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
> - IS_CANNONLAKE(dev_priv)) {
> - if (fb && fb->format->format == DRM_FORMAT_NV12)
> - skl_wa_clkgate(dev_priv, crtc->pipe, true);
> - }
>
> intel_fbc_pre_update(crtc, pipe_config, new_primary_state);
> /*
> @@ -5221,6 +5208,10 @@ static void intel_pre_plane_update(struct
> intel_crtc_state *old_crtc_state,
> intel_set_cpu_fifo_underrun_reporting(dev_priv,
> crtc->pipe, false);
> }
>
> + /* Display WA 827 */
> + if (!old_crtc_state->nv12_wa && pipe_config->nv12_wa)
> + skl_wa_clkgate(dev_priv, crtc->pipe, true);
> +
> /*
>* Vblank time updates from the shadow to live plane control register
>* are blocked if the memory self-refresh mode is active at that
> @@ -10485,6 +10476,21 @@ static int intel_crtc_atomic_check(struct drm_crtc
> *crtc,
> return ret;
> }
>
> + pipe_config->nv12_wa = false;
> +
> + /* Apply display WA 827 if required. */
> + if (crtc_state->active &&
> + ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
> + IS_CANNONLAKE(dev_priv))) {
GLK doesn't need this? That seems odd to say the least.
> + struct drm_plane *plane;
> + const struct drm_plane_state *plane_state;
> +
> + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state,
> crtc_state)
I'd prefer a bitmask instead of that sneaky peeking of the plane states.
It should also avoid polluting this sort of central/generic piece of
code with platform specific w/as.
> + if (plane_state->fb &&
> + plane_state->fb->format->format == DRM_FORMAT_NV12)
> + pipe_config->nv12_wa = true;
> + }
> +
> if (crtc_state->color_mgmt_changed) {
> ret = intel_color_check(crtc, crtc_state);
> if (ret)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 9969309132d0..9c2b7f78d5dd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -723,6 +723,7 @@ struct intel_crtc_state {
> bool update_wm_pre, update_wm_post; /* watermarks are updated */
> bool fb_changed; /* fb on any of the planes is changed */
> bool fifo_changed; /* FIFO spli
[Intel-gfx] [PATCH 4/4] drm/i915: Enable display workaround 827 for all planes.
The workaround was applied only to the primary plane, but is required
on all planes. Iterate over all planes in the crtc atomic check to see
if the workaround is enabled, and only perform the actual toggling in
the pre/post plane update functions.
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/intel_display.c | 40 +---
drivers/gpu/drm/i915/intel_drv.h | 1 +
2 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 487a6e235222..829b593a3cee 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5162,7 +5162,6 @@ static void intel_post_plane_update(struct
intel_crtc_state *old_crtc_state)
if (old_primary_state) {
struct drm_plane_state *new_primary_state =
drm_atomic_get_new_plane_state(old_state, primary);
- struct drm_framebuffer *fb = new_primary_state->fb;
intel_fbc_post_update(crtc);
@@ -5170,15 +5169,11 @@ static void intel_post_plane_update(struct
intel_crtc_state *old_crtc_state)
(needs_modeset(&pipe_config->base) ||
!old_primary_state->visible))
intel_post_enable_primary(&crtc->base, pipe_config);
-
- /* Display WA 827 */
- if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
- IS_CANNONLAKE(dev_priv)) {
- if (fb && fb->format->format == DRM_FORMAT_NV12)
- skl_wa_clkgate(dev_priv, crtc->pipe, false);
- }
-
}
+
+ /* Display WA 827 */
+ if (old_crtc_state->nv12_wa && !pipe_config->nv12_wa)
+ skl_wa_clkgate(dev_priv, crtc->pipe, false);
}
static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
@@ -5202,14 +5197,6 @@ static void intel_pre_plane_update(struct
intel_crtc_state *old_crtc_state,
struct intel_plane_state *new_primary_state =
intel_atomic_get_new_plane_state(old_intel_state,
to_intel_plane(primary));
- struct drm_framebuffer *fb = new_primary_state->base.fb;
-
- /* Display WA 827 */
- if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
- IS_CANNONLAKE(dev_priv)) {
- if (fb && fb->format->format == DRM_FORMAT_NV12)
- skl_wa_clkgate(dev_priv, crtc->pipe, true);
- }
intel_fbc_pre_update(crtc, pipe_config, new_primary_state);
/*
@@ -5221,6 +5208,10 @@ static void intel_pre_plane_update(struct
intel_crtc_state *old_crtc_state,
intel_set_cpu_fifo_underrun_reporting(dev_priv,
crtc->pipe, false);
}
+ /* Display WA 827 */
+ if (!old_crtc_state->nv12_wa && pipe_config->nv12_wa)
+ skl_wa_clkgate(dev_priv, crtc->pipe, true);
+
/*
* Vblank time updates from the shadow to live plane control register
* are blocked if the memory self-refresh mode is active at that
@@ -10485,6 +10476,21 @@ static int intel_crtc_atomic_check(struct drm_crtc
*crtc,
return ret;
}
+ pipe_config->nv12_wa = false;
+
+ /* Apply display WA 827 if required. */
+ if (crtc_state->active &&
+ ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
+IS_CANNONLAKE(dev_priv))) {
+ struct drm_plane *plane;
+ const struct drm_plane_state *plane_state;
+
+ drm_atomic_crtc_state_for_each_plane_state(plane, plane_state,
crtc_state)
+ if (plane_state->fb &&
+ plane_state->fb->format->format == DRM_FORMAT_NV12)
+ pipe_config->nv12_wa = true;
+ }
+
if (crtc_state->color_mgmt_changed) {
ret = intel_color_check(crtc, crtc_state);
if (ret)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9969309132d0..9c2b7f78d5dd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -723,6 +723,7 @@ struct intel_crtc_state {
bool update_wm_pre, update_wm_post; /* watermarks are updated */
bool fb_changed; /* fb on any of the planes is changed */
bool fifo_changed; /* FIFO split is changed */
+ bool nv12_wa; /* Whether display WA 827 needs to be enabled. */
/* Pipe source size (ie. panel fitter input size)
* All planes will be positioned inside this space,
--
2.16.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
