On Tue, 4 Mar 2014 13:15:08 +0000 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > This should be impossible due to the wait for outstanding flips that the > caller is meant to perform prior to updating the scanout base. Paranoia > tells me to check anyway. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=75502 > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++++---------------- > 1 file changed, 24 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 25c486d5fb6a..6dc93bd6594f 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2349,6 +2349,25 @@ intel_finish_fb(struct drm_framebuffer *old_fb) > return ret; > } > > +static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + unsigned long flags; > + bool pending; > + > + if (i915_reset_in_progress(&dev_priv->gpu_error) || > + intel_crtc->reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter)) > + return false; > + > + spin_lock_irqsave(&dev->event_lock, flags); > + pending = to_intel_crtc(crtc)->unpin_work != NULL; > + spin_unlock_irqrestore(&dev->event_lock, flags); > + > + return pending; > +} > + > static int > intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > struct drm_framebuffer *fb) > @@ -2359,6 +2378,11 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > struct drm_framebuffer *old_fb; > int ret; > > + if (intel_crtc_has_pending_flip(crtc)) { > + DRM_ERROR("pipe is still busy with an old pageflip\n"); > + return -EBUSY; > + } > + > /* no fb bound */ > if (!fb) { > DRM_ERROR("No FB bound\n"); > @@ -2984,25 +3008,6 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc) > udelay(100); > } > > -static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) > -{ > - struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - unsigned long flags; > - bool pending; > - > - if (i915_reset_in_progress(&dev_priv->gpu_error) || > - intel_crtc->reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter)) > - return false; > - > - spin_lock_irqsave(&dev->event_lock, flags); > - pending = to_intel_crtc(crtc)->unpin_work != NULL; > - spin_unlock_irqrestore(&dev->event_lock, flags); > - > - return pending; > -} > - > bool intel_has_pending_fb_unpin(struct drm_device *dev) > { > struct intel_crtc *crtc; Looks fine, my only comment is do we want this to be a DRM_ERROR? It would be easy for userspace to trigger this by queueing a flip on a busy ring, then doing a mode set that ends up doing just a pipe base update, right? Otherwise, Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
