On Wed, Nov 14, 2018 at 11:07:27PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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 accoung of only having to worry about a single pipe.
Minor typo on 'account' here.
Otherwise, patch looks good.
Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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 69fc7010190c..ff8d3e577bbf 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -169,24 +169,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 =
> @@ -202,14 +253,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 2981cea3704a..114b2f3c6274 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12661,7 +12661,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 =
> @@ -12684,8 +12683,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 4a9af09c483a..2af994942fa7 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2299,10 +2299,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.18.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
