| Message ID | 20180409142123.4404-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 09, 2018 at 04:21:23PM +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. > > Changes since v1: > - Track active NV12 planes in a nv12_planes bitmask. (Ville) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_atomic_plane.c | 7 +++++- > drivers/gpu/drm/i915/intel_display.c | 40 ++++++++++++++++++------------- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 3 files changed, 30 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index 7481ce85746b..6d068786eb41 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -183,11 +183,16 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ > } > > /* FIXME pre-g4x don't work like this */ > - if (intel_state->base.visible) > + if (state->visible) > crtc_state->active_planes |= BIT(intel_plane->id); > else > crtc_state->active_planes &= ~BIT(intel_plane->id); > > + if (state->visible && state->fb->format->format == DRM_FORMAT_NV12) > + crtc_state->nv12_planes |= BIT(intel_plane->id); > + else > + crtc_state->nv12_planes &= ~BIT(intel_plane->id); > + > return intel_plane_atomic_calc_changes(old_crtc_state, > &crtc_state->base, > old_plane_state, > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 487a6e235222..3039d00546c2 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5138,6 +5138,19 @@ static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_s > return !old_crtc_state->ips_enabled; > } > > +static bool needs_nv12_wa(struct drm_i915_private *dev_priv, > + const struct intel_crtc_state *crtc_state) > +{ > + if (!crtc_state->nv12_planes) > + return false; > + > + if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || > + IS_CANNONLAKE(dev_priv)) > + return true; > + > + return false; > +} > + > static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) > { > struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc); > @@ -5162,7 +5175,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 +5182,12 @@ 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 (needs_nv12_wa(dev_priv, old_crtc_state) && > + !needs_nv12_wa(dev_priv, pipe_config)) > + skl_wa_clkgate(dev_priv, crtc->pipe, false); > } > > static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > @@ -5202,14 +5211,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 +5222,11 @@ 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 (!needs_nv12_wa(dev_priv, old_crtc_state) && > + needs_nv12_wa(dev_priv, pipe_config)) > + skl_wa_clkgate(dev_priv, crtc->pipe, true); That function name is pretty non-descriptive. What w/a? Oh, and the implementation looks somewhat bogus as well. It's using rmw for disable, but no rmw for enable. Someone may want to fix that up a bit as well. This patch lgtm Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > + > /* > * Vblank time updates from the shadow to live plane control register > * are blocked if the memory self-refresh mode is active at that > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 9969309132d0..e6473d684607 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -882,6 +882,7 @@ struct intel_crtc_state { > > /* bitmask of visible planes (enum plane_id) */ > u8 active_planes; > + u8 nv12_planes; > > /* HDMI scrambling status */ > bool hdmi_scrambling; > -- > 2.16.3
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 7481ce85746b..6d068786eb41 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -183,11 +183,16 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ } /* FIXME pre-g4x don't work like this */ - if (intel_state->base.visible) + if (state->visible) crtc_state->active_planes |= BIT(intel_plane->id); else crtc_state->active_planes &= ~BIT(intel_plane->id); + if (state->visible && state->fb->format->format == DRM_FORMAT_NV12) + crtc_state->nv12_planes |= BIT(intel_plane->id); + else + crtc_state->nv12_planes &= ~BIT(intel_plane->id); + return intel_plane_atomic_calc_changes(old_crtc_state, &crtc_state->base, old_plane_state, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 487a6e235222..3039d00546c2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5138,6 +5138,19 @@ static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_s return !old_crtc_state->ips_enabled; } +static bool needs_nv12_wa(struct drm_i915_private *dev_priv, + const struct intel_crtc_state *crtc_state) +{ + if (!crtc_state->nv12_planes) + return false; + + if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || + IS_CANNONLAKE(dev_priv)) + return true; + + return false; +} + static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) { struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc); @@ -5162,7 +5175,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 +5182,12 @@ 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 (needs_nv12_wa(dev_priv, old_crtc_state) && + !needs_nv12_wa(dev_priv, pipe_config)) + skl_wa_clkgate(dev_priv, crtc->pipe, false); } static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, @@ -5202,14 +5211,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 +5222,11 @@ 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 (!needs_nv12_wa(dev_priv, old_crtc_state) && + needs_nv12_wa(dev_priv, pipe_config)) + 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 diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 9969309132d0..e6473d684607 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -882,6 +882,7 @@ struct intel_crtc_state { /* bitmask of visible planes (enum plane_id) */ u8 active_planes; + u8 nv12_planes; /* HDMI scrambling status */ bool hdmi_scrambling;
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. Changes since v1: - Track active NV12 planes in a nv12_planes bitmask. (Ville) Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_atomic_plane.c | 7 +++++- drivers/gpu/drm/i915/intel_display.c | 40 ++++++++++++++++++------------- drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 30 insertions(+), 18 deletions(-)