On Tue, Mar 04, 2014 at 08:58:31AM -0800, Jesse Barnes wrote:
> On Tue,  4 Mar 2014 13:15:08 +0000
> Chris Wilson <[email protected]> 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 <[email protected]>
> > ---
> >  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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to