[Intel-gfx] [PATCH] drm/i915: Flush outstanding unpin tasks before pageflipping

[Chris Wilson]

On Fri, 28 Sep 2012 15:05:01 +0300, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> On Fri, Sep 28, 2012 at 12:29:56PM +0100, Chris Wilson wrote:
> > If we accumulate unpin tasks because we are pageflipping faster than the
> > system can schedule its workers, we can effectively create a
> > pin-leak. The solution taken here is to limit the number of unpin tasks
> > we have per-crtc and to flush those outstanding tasks if we accumulate
> > too many. This should prevent any jitter in the normal case, and also
> > prevent the hang if we should run too fast.
> > 
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=46991
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |   20 +++++++++++++++-----
> >  drivers/gpu/drm/i915/intel_drv.h     |    4 +++-
> >  2 files changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 04407fd..14f1b51 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6310,14 +6310,19 @@ static void intel_unpin_work_fn(struct work_struct *__work)
> >  {
> >  	struct intel_unpin_work *work =
> >  		container_of(__work, struct intel_unpin_work, work);
> > +	struct drm_device *dev = work->crtc->dev;
> >  
> > -	mutex_lock(&work->dev->struct_mutex);
> > +	mutex_lock(&dev->struct_mutex);
> >  	intel_unpin_fb_obj(work->old_fb_obj);
> >  	drm_gem_object_unreference(&work->pending_flip_obj->base);
> >  	drm_gem_object_unreference(&work->old_fb_obj->base);
> >  
> > -	intel_update_fbc(work->dev);
> > -	mutex_unlock(&work->dev->struct_mutex);
> > +	intel_update_fbc(dev);
> > +	mutex_unlock(&dev->struct_mutex);
> > +
> > +	BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0);
> > +	atomic_dec(&to_intel_crtc(work->crtc)->unpin_work_count);
> 
> AFAICS you always have struct_mutex locked in the relevant functions,
> so no need for an atomic variable.

It's not in every case, since we need to do the flush without holding
the lock, we have the choice of making this variable atomic, or taking
and dropping the lock. Obviously I choose the former.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre