Re: [PATCH 6/6] drm/i915: Really wait for pending flips in intel_pipe_set_base()

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jan 29, 2013 at 06:13:38PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since obj->pending_flips was never set, intel_pipe_set_base() never
> actually waited for pending page flips to complete.
> 
> We really do want to wait for the pending flips, because otherwise the
> mmio surface base address update could overtake the flip, and you
> could end up with an old frame on the screen once the flip really
> completes.
> 
> Just call intel_crtc_wait_pending_flips_locked() instead of
> intel_finish_fb() from intel_pipe_set_base() to achieve the
> desired result.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a2e04f7..7e047c1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2330,8 +2330,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>  		return ret;
>  	}
>  
> -	if (crtc->fb)
> -		intel_finish_fb(crtc->fb);
> +	intel_crtc_wait_for_pending_flips_locked(crtc);

Ah, I see now why you grab dev->struct_mutex and need to kick waiters from
the pending flip queue. I think grabbing the mutex twice isn't a major
offense, since both the crtc disable code and set_base are slowpaths used
rarely. So what about simply calling wait_for_pending_flips before
grabbing the mutex in intel_pipe_set_base? We could then also inline
finish_fb into it's only callsite ...
-Daniel

>  
>  	ret = dev_priv->display.update_plane(crtc, fb, x, y);
>  	if (ret) {
> -- 
> 1.7.12.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch