From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/3] drm/i915: Implement sema idle msg disable for all rings Date: Mon, 31 Mar 2014 21:01:15 +0300 Message-ID: <20140331180114.GY21652@intel.com> References: <1396279038-28914-1-git-send-email-ville.syrjala@linux.intel.com> <1396279038-28914-3-git-send-email-ville.syrjala@linux.intel.com> <20140331172320.GV22327@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 9F5E96E3AF for ; Mon, 31 Mar 2014 11:02:20 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140331172320.GV22327@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Mar 31, 2014 at 07:23:20PM +0200, Daniel Vetter wrote: > On Mon, Mar 31, 2014 at 06:17:17PM +0300, ville.syrjala@linux.intel.com w= rote: > > From: Ville Syrj=E4l=E4 > > = > > Previously in > > commit 295e8bb73a4785b65db6655fbf6ad57c4177b551 > > Author: Ville Syrj=E4l=E4 > > Date: Thu Feb 27 21:59:01 2014 +0200 > > = > > drm/i915: Disable semaphore wait event idle message on BDW > > = > > I failed to notice that all rings have their own copy of the bit that > > disables the semaphore wait even idle message. So that patch only succe= eded > > in disabling it for the render ring. Instead we should set the bit for = all > > rings. > > = > > Signed-off-by: Ville Syrj=E4l=E4 > > --- > > drivers/gpu/drm/i915/i915_gem.c | 8 ++++++++ > > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > > drivers/gpu/drm/i915/intel_pm.c | 3 --- > > 3 files changed, 10 insertions(+), 3 deletions(-) > > = > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i91= 5_gem.c > > index 33bbaa0..84a7171 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -4372,6 +4372,14 @@ static int i915_gem_init_rings(struct drm_device= *dev) > > goto cleanup_blt_ring; > > } > > = > > + if (IS_GEN8(dev)) { > > + struct intel_ring_buffer *ring; > > + int i; > > + > > + for_each_ring(ring, dev_priv, i) > > + I915_WRITE(RING_RC_PSMI_CONTROL(ring), > > + _MASKED_BIT_ENABLE(GEN8_RC_SEMA_IDLE_MSG_DISABLE)); > > + } > = > Why move this to here? Is this one of those bits which get reset on ring > init? If that's the case I think we really need to have a w/a checker to > make sure that after driver load, suspend/resume and gpu reset we always > have the same set of workarounds ... Cause I needed ring->mmio_base to be there and I couldn't be bothered to find a better place. Now that I looked a bit, I suppose init_ring_common() might be the right place for it. I have no idea when it gets reset. But hold on, now that I look at the spec again it seems the bit isn't there for the other rings after all. I must have been doubly blind when I wrote the patch. So let's just drop it. -- = Ville Syrj=E4l=E4 Intel OTC