[Intel-gfx] [PATCH] drm/i915: hold ref on flip object until it completes

[Jesse Barnes]

On Wed, 10 Feb 2010 15:07:26 -0800
Jesse Barnes <jbarnes at virtuousgeek.org> wrote:

> On Wed, 10 Feb 2010 23:07:06 +0000
> Owain Ainsworth <zerooa at googlemail.com> wrote:
> 
> > On Wed, Feb 10, 2010 at 02:55:56PM -0800, Jesse Barnes wrote:
> > > This will prevent things from falling over if the user frees the
> > > flip buffer before we complete the flip, since we'll hold an
> > > internal reference.
> > > 
> > > Reported-by: Kristian H??gsberg <krh at bitplanet.net>
> > > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c index 2476fb4..372a2f5
> > > 100644 --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -4039,6 +4039,9 @@ void intel_finish_page_flip(struct
> > > drm_device *dev, int pipe)
> > > atomic_dec_and_test(&obj_priv->pending_flip))
> > > DRM_WAKEUP(&dev_priv->pending_flip_queue);
> > > schedule_work(&work->work); +
> > > +	/* Drop the flip ref we took when the ioctl was called */
> > > +	drm_gem_object_unreference(work->pending_flip_obj);
> > >  }
> > 
> > two problems:
> > 1) not holdin struct mutex ((this is in interrupt context).
> > 2) you're doing that from interrupt context. stuff like the unref,
> > the possible free and unbind should not happen here.
> 
> Arg you're right.  I need to move this to the work handler.

So ignore the last one.  This one is less buggy.

-- 
Jesse Barnes, Intel Open Source Technology Center

This will prevent things from falling over if the user frees the flip
buffer before we complete the flip, since we'll hold an internal
reference.

Reported-by: Kristian Høgsberg <krh at bitplanet.net>
Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2476fb4..1f92970 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3983,6 +3983,7 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 
 	mutex_lock(&work->dev->struct_mutex);
 	i915_gem_object_unpin(work->old_fb_obj);
+	drm_gem_object_unreference(work->pending_flip_obj);
 	drm_gem_object_unreference(work->old_fb_obj);
 	mutex_unlock(&work->dev->struct_mutex);
 	kfree(work);
@@ -4109,8 +4110,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		return ret;
 	}
 
-	/* Reference the old fb object for the scheduled work. */
+	/* Reference the objects for the scheduled work. */
 	drm_gem_object_reference(work->old_fb_obj);
+	drm_gem_object_reference(obj);
 
 	crtc->fb = fb;
 	i915_gem_object_flush_write_domain(obj);