From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: Flush outstanding unpin tasks before pageflipping Date: Fri, 28 Sep 2012 13:07:59 +0100 Message-ID: <453bf0$5t4oh8@azsmga001.ch.intel.com> References: <1348831796-22631-1-git-send-email-chris@chris-wilson.co.uk> <20120928120501.GO19732@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0469627633==" Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id F1CAF9EEF1 for ; Fri, 28 Sep 2012 05:08:04 -0700 (PDT) In-Reply-To: <20120928120501.GO19732@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0469627633== Content-Type: text/plain On Fri, 28 Sep 2012 15:05:01 +0300, Ville Syrjälä 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 > > --- > > 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 --===============0469627633== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0469627633==--