[Intel-gfx] [PATCH] drm/i915: Flush outstanding unpin tasks before pageflipping
Ville Syrjälä
ville.syrjala at linux.intel.com
Fri Sep 28 14:20:53 CEST 2012
On Fri, Sep 28, 2012 at 01:07:59PM +0100, Chris Wilson wrote:
> 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.
Ah right. I missed the conditional flush.
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list