[Intel-gfx] [PATCH] drm/i915: Make legacy cursor updates more unsynced

[Maarten Lankhorst]

Op 28-03-17 om 21:35 schreef ville.syrjala at linux.intel.com:
> From: Ville Syrjälä <ville.syrjala at 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.lankhorst at linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Uwe Kleine-König <uwe at kleine-koenig.org>
> Cc: Rafael Ristovski <rafael.ristovski at gmail.com>
> Fixes: a5509abda48e ("drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW")
> Signed-off-by: Ville Syrjälä <ville.syrjala at 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. :)