On Tue, Mar 04, 2014 at 08:58:31AM -0800, Jesse Barnes wrote: > 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? It should never get this far with a pending flip. If it does, then there must be a bug somewhere. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx