From 5f2e511205bbc40b6f58f7d2125c10bd39007748 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 14 Nov 2018 23:07:27 +0200 Subject: [PATCH] drm/i915: Commit skl+ planes in an order that avoids ddb overlaps MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit skl+ can go belly up if there are overlapping ddb allocations between planes. If we could absolutely guarantee that we can perform the atomic update within a single frame we shouldn't have to worry about this. But we can't rely on that so let's steal the ddb overlap check trick from skl_update_crtcs() and apply it to the plane updates. Since each step of the sequence is free from ddb overlaps we don't have to worry about a vblank sneaking up on us in the middle of the sequence. The partial state that gets latched by the hardware will be safe. And unlike skl_update_crtcs() we don't have to intoduce any extra vblank waits on account of only having to worry about a single pipe. v2: Fix typo in commit msg (Matt) Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181114210729.16185-12-ville.syrjala@linux.intel.com Reviewed-by: Matt Roper Reviewed-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_atomic_plane.c | 96 ++++++++++++++++++++--- drivers/gpu/drm/i915/intel_display.c | 7 +- drivers/gpu/drm/i915/intel_drv.h | 8 +- 3 files changed, 93 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index ad3d135bdc15..0a73e6e65c20 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -171,24 +171,75 @@ static int intel_plane_atomic_check(struct drm_plane *plane, to_intel_plane_state(new_plane_state)); } -void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, - struct intel_crtc *crtc, - struct intel_crtc_state *old_crtc_state, - struct intel_crtc_state *new_crtc_state) +static struct intel_plane * +skl_next_plane_to_commit(struct intel_atomic_state *state, + struct intel_crtc *crtc, + struct skl_ddb_entry entries_y[I915_MAX_PLANES], + struct skl_ddb_entry entries_uv[I915_MAX_PLANES], + unsigned int *update_mask) { - u32 update_mask = new_crtc_state->update_planes; - struct intel_plane_state *new_plane_state; + struct intel_crtc_state *crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + struct intel_plane_state *plane_state; struct intel_plane *plane; int i; - for_each_new_intel_plane_in_state(old_state, plane, new_plane_state, i) { + if (*update_mask == 0) + return NULL; + + for_each_new_intel_plane_in_state(state, plane, plane_state, i) { + enum plane_id plane_id = plane->id; + if (crtc->pipe != plane->pipe || - !(update_mask & BIT(plane->id))) + !(*update_mask & BIT(plane_id))) + continue; + + if (skl_ddb_allocation_overlaps(&crtc_state->wm.skl.plane_ddb_y[plane_id], + entries_y, + I915_MAX_PLANES, plane_id) || + skl_ddb_allocation_overlaps(&crtc_state->wm.skl.plane_ddb_uv[plane_id], + entries_uv, + I915_MAX_PLANES, plane_id)) continue; + *update_mask &= ~BIT(plane_id); + entries_y[plane_id] = crtc_state->wm.skl.plane_ddb_y[plane_id]; + entries_uv[plane_id] = crtc_state->wm.skl.plane_ddb_uv[plane_id]; + + return plane; + } + + /* should never happen */ + WARN_ON(1); + + return NULL; +} + +void skl_update_planes_on_crtc(struct intel_atomic_state *state, + struct intel_crtc *crtc) +{ + struct intel_crtc_state *old_crtc_state = + intel_atomic_get_old_crtc_state(state, crtc); + struct intel_crtc_state *new_crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + struct skl_ddb_entry entries_y[I915_MAX_PLANES]; + struct skl_ddb_entry entries_uv[I915_MAX_PLANES]; + u32 update_mask = new_crtc_state->update_planes; + struct intel_plane *plane; + + memcpy(entries_y, old_crtc_state->wm.skl.plane_ddb_y, + sizeof(old_crtc_state->wm.skl.plane_ddb_y)); + memcpy(entries_uv, old_crtc_state->wm.skl.plane_ddb_uv, + sizeof(old_crtc_state->wm.skl.plane_ddb_uv)); + + while ((plane = skl_next_plane_to_commit(state, crtc, + entries_y, entries_uv, + &update_mask))) { + struct intel_plane_state *new_plane_state = + intel_atomic_get_new_plane_state(state, plane); + if (new_plane_state->base.visible) { trace_intel_update_plane(&plane->base, crtc); - plane->update_plane(plane, new_crtc_state, new_plane_state); } else if (new_plane_state->slave) { struct intel_plane *master = @@ -204,14 +255,37 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, * plane_state. */ new_plane_state = - intel_atomic_get_new_plane_state(old_state, master); + intel_atomic_get_new_plane_state(state, master); trace_intel_update_plane(&plane->base, crtc); - plane->update_slave(plane, new_crtc_state, new_plane_state); } else { trace_intel_disable_plane(&plane->base, crtc); + plane->disable_plane(plane, new_crtc_state); + } + } +} +void i9xx_update_planes_on_crtc(struct intel_atomic_state *state, + struct intel_crtc *crtc) +{ + struct intel_crtc_state *new_crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + u32 update_mask = new_crtc_state->update_planes; + struct intel_plane_state *new_plane_state; + struct intel_plane *plane; + int i; + + for_each_new_intel_plane_in_state(state, plane, new_plane_state, i) { + if (crtc->pipe != plane->pipe || + !(update_mask & BIT(plane->id))) + continue; + + if (new_plane_state->base.visible) { + trace_intel_update_plane(&plane->base, crtc); + plane->update_plane(plane, new_crtc_state, new_plane_state); + } else { + trace_intel_disable_plane(&plane->base, crtc); plane->disable_plane(plane, new_crtc_state); } } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6c5d71b44740..d07fa4456150 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12692,7 +12692,6 @@ static void intel_update_crtc(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_crtc_state *old_intel_cstate = to_intel_crtc_state(old_crtc_state); struct intel_crtc_state *pipe_config = to_intel_crtc_state(new_crtc_state); bool modeset = needs_modeset(new_crtc_state); struct intel_plane_state *new_plane_state = @@ -12715,8 +12714,10 @@ static void intel_update_crtc(struct drm_crtc *crtc, intel_begin_crtc_commit(crtc, old_crtc_state); - intel_update_planes_on_crtc(to_intel_atomic_state(state), intel_crtc, - old_intel_cstate, pipe_config); + if (INTEL_GEN(dev_priv) >= 9) + skl_update_planes_on_crtc(to_intel_atomic_state(state), intel_crtc); + else + i9xx_update_planes_on_crtc(to_intel_atomic_state(state), intel_crtc); intel_finish_crtc_commit(crtc, old_crtc_state); } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1a3a396862d1..40edb21087a7 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2300,10 +2300,10 @@ struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); void intel_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; -void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, - struct intel_crtc *crtc, - struct intel_crtc_state *old_crtc_state, - struct intel_crtc_state *new_crtc_state); +void skl_update_planes_on_crtc(struct intel_atomic_state *state, + struct intel_crtc *crtc); +void i9xx_update_planes_on_crtc(struct intel_atomic_state *state, + struct intel_crtc *crtc); int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_state, struct intel_crtc_state *crtc_state, const struct intel_plane_state *old_plane_state, -- 2.30.2