On Wed, Feb 13, 2013 at 04:49:35PM +0100, Daniel Vetter wrote: > 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 ... I didn't want to slow down intel_pipe_set_base() too much. If we wait for pending flips before pinning the new fb, we can never achieve any parallelism there. But if no-one cares about that, we can reorder. -- Ville Syrj?l? Intel OTC
