On Tue, Jan 29, 2013 at 06:13:38PM +0200, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrj?l? <ville.syrjala at 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 at 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 at 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
