From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 2/2] drm/i915: i8xx also doesn't like multiple oustanding pageflips Date: Wed, 4 Aug 2010 12:49:09 -0700 Message-ID: <20100804124909.0ca79a59@virtuousgeek.org> References: <1280949730-5288-1-git-send-email-daniel.vetter@ffwll.ch> <1280949730-5288-3-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cpoproxy3-pub.bluehost.com (cpoproxy3-pub.bluehost.com [67.222.54.6]) by gabe.freedesktop.org (Postfix) with SMTP id 0ADAC9E779 for ; Wed, 4 Aug 2010 12:49:11 -0700 (PDT) In-Reply-To: <1280949730-5288-3-git-send-email-daniel.vetter@ffwll.ch> 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 To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, stabel@kernel.org List-Id: intel-gfx@lists.freedesktop.org On Wed, 4 Aug 2010 21:22:10 +0200 Daniel Vetter wrote: > My i855GM suffers from a 80k/s interrupt storm without this. > So add 2nd gen to the list of things that don't like more than > one outstanding pageflip request. > > Furthermore I've changed the busy loop into a ringbuffer wait. > Busy-loops that don't check whether the chip died are simply evil. > And performance should actually improve, because there's usually > a decent amount of rendering queued on the gpu, hopefully rendering > that MI_WAIT into a noop by the time it's executed. > > The current code holds dev->struct_mutex while executing this loop, > hence stalling all other gem activity anyway. > > Signed-off-by: Daniel Vetter > Cc: stabel@kernel.org > --- > drivers/gpu/drm/i915/intel_display.c | 13 ++++++++----- > 1 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 8135ee0..7b6035e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4916,14 +4916,17 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > work->pending_flip_obj = obj; > > if (intel_crtc->plane) > - flip_mask = I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT; > + flip_mask = MI_WAIT_FOR_PLANE_B_FLIP; > else > - flip_mask = I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT; > + flip_mask = MI_WAIT_FOR_PLANE_A_FLIP; > > /* Wait for any previous flip to finish */ > - if (IS_GEN3(dev)) > - while (I915_READ(ISR) & flip_mask) > - ; > + if (IS_GEN3(dev) || IS_GEN2(dev)) { > + BEGIN_LP_RING(2); > + OUT_RING(MI_WAIT_FOR_EVENT | flip_mask); > + OUT_RING(0); > + ADVANCE_LP_RING(); > + } > > BEGIN_LP_RING(4); > if (IS_I965G(dev)) { This looks nicer than what was there before. Hope it still works on 9xx... Reviewed-by: Jesse Barnes -- Jesse Barnes, Intel Open Source Technology Center