Op 28-03-17 om 21:35 schreef ville.syrj...@linux.intel.com:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> We're clearing the legacy_cursor_update flag before calling
> drm_atomic_helper_setup_commit() which means the helper will
> wait for the flip to complete before cleaning up the framebuffers.
> That's not what we want for the legacy cursor, so let's clear
> the flag after setting up the commit.
>
> Also toss in a FIXME about solving these problems in a nicer
> way using the fabled vblank workers.
>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
> Cc: Uwe Kleine-König <u...@kleine-koenig.org>
> Cc: Rafael Ristovski <rafael.ristov...@gmail.com>
> Fixes: a5509abda48e ("drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW")
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 654b8a0c28ee..05ff193f2dd8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13024,6 +13024,10 @@ static int intel_atomic_commit(struct drm_device 
> *dev,
>       struct drm_i915_private *dev_priv = to_i915(dev);
>       int ret = 0;
>  
> +     ret = drm_atomic_helper_setup_commit(state, nonblock);
> +     if (ret)
> +             return ret;
> +
>       /*
>        * The intel_legacy_cursor_update() fast path takes care
>        * of avoiding the vblank waits for simple cursor
> @@ -13031,14 +13035,18 @@ static int intel_atomic_commit(struct drm_device 
> *dev,
>        * we want to perform the vblank waits so that watermark
>        * updates happen during the correct frames. Gen9+ have
>        * double buffered watermarks and so shouldn't need this.
> +      *
> +      * Do this after drm_atomic_helper_setup_commit() because
> +      * we still want to skip the fb cleanup waits from the
> +      * atomic helper. Although that does risk yanking the
> +      * mapping from under the display engine.
> +      *
> +      * FIXME doing watermarks and fb cleanup from a vblank worker
> +      * (assuming we had any) would solve these problems.
>        */
>       if (INTEL_GEN(dev_priv) < 9)
>               state->legacy_cursor_update = false;
>  
> -     ret = drm_atomic_helper_setup_commit(state, nonblock);
> -     if (ret)
> -             return ret;
> -
>       drm_atomic_state_get(state);
>       i915_sw_fence_init(&intel_state->commit_ready,
>                          intel_atomic_commit_ready);

From irc:
<vsyrjala> hmm. actually should probably move that thing to happen even later. 
we should skip the flip waits from intel_atomic_prepare_commit() as well i recon


Makes sense. Not sure how much we care about legacy page flips, since they're 
on their way out.

If this is fixed and trybot is happy with v2 then I will be too. :)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to