From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: add cherryview specfic forcewake in execlists_elsp_write Date: Mon, 8 Sep 2014 17:14:23 +0300 Message-ID: <20140908141423.GA4193@intel.com> References: <1410270256-26413-1-git-send-email-deepak.s@linux.intel.com> <20140908140243.GZ4193@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 507156E2B8 for ; Mon, 8 Sep 2014 07:14:28 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140908140243.GZ4193@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: deepak.s@linux.intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Sep 08, 2014 at 05:02:43PM +0300, Ville Syrj=E4l=E4 wrote: > On Tue, Sep 09, 2014 at 07:14:16PM +0530, deepak.s@linux.intel.com wrote: > > From: Deepak S > > = > > In chv, we have two power wells Render & Media. We need to use > > corresponsing forcewake count. If we dont follow this we are getting > > error "*ERROR*: Timed out waiting for forcewake old ack to clear" due to > > multiple entry into __vlv_force_wake_get. > > = > > Signed-off-by: Deepak S > > --- > > drivers/gpu/drm/i915/intel_lrc.c | 29 +++++++++++++++++++++++++---- > > 1 file changed, 25 insertions(+), 4 deletions(-) > > = > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/in= tel_lrc.c > > index bd1b28d..bafd38b 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -300,8 +300,18 @@ static void execlists_elsp_write(struct intel_engi= ne_cs *ring, > > * Instead, we do the runtime_pm_get/put when creating/destroying req= uests. > > */ > > spin_lock_irqsave(&dev_priv->uncore.lock, flags); > > - if (dev_priv->uncore.forcewake_count++ =3D=3D 0) > > - dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL); > > + if (IS_CHERRYVIEW(dev_priv->dev)) { > > + if (dev_priv->uncore.fw_rendercount++ =3D=3D 0) > > + dev_priv->uncore.funcs.force_wake_get(dev_priv, > > + FORCEWAKE_RENDER); > > + if (dev_priv->uncore.fw_mediacount++ =3D=3D 0) > > + dev_priv->uncore.funcs.force_wake_get(dev_priv, > > + FORCEWAKE_MEDIA); > = > This will wake both wells. Is that needed or should we just pick one > based on the ring? Also unlike the comment says runtime_pm_get() can't sleep since someone must already be holding a reference, othwewise we surely can't go writing any registers. So in theory we should be able to call gen6_gt_force_wake_get() here, but maybe that would trigger a might_sleep() warning. the current force wake code duplication (esp. outside intel_uncore.c) is rather unfortunate and I'd like to see it killed off. Maybe we just need to pull the rpm get/put outside gen6_gt_force_wake_get()? I never really liked hiding it there anyway. > = > > + } else { > > + if (dev_priv->uncore.forcewake_count++ =3D=3D 0) > > + dev_priv->uncore.funcs.force_wake_get(dev_priv, > > + FORCEWAKE_ALL); > > + } > > spin_unlock_irqrestore(&dev_priv->uncore.lock, flags); > > = > > I915_WRITE(RING_ELSP(ring), desc[1]); > > @@ -315,8 +325,19 @@ static void execlists_elsp_write(struct intel_engi= ne_cs *ring, > > = > > /* Release Force Wakeup (see the big comment above). */ > > spin_lock_irqsave(&dev_priv->uncore.lock, flags); > > - if (--dev_priv->uncore.forcewake_count =3D=3D 0) > > - dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL); > > + if (IS_CHERRYVIEW(dev_priv->dev)) { > > + if (--dev_priv->uncore.fw_rendercount =3D=3D 0) > > + dev_priv->uncore.funcs.force_wake_put(dev_priv, > > + FORCEWAKE_RENDER); > > + if (--dev_priv->uncore.fw_mediacount =3D=3D 0) > > + dev_priv->uncore.funcs.force_wake_put(dev_priv, > > + FORCEWAKE_MEDIA); > > + } else { > > + if (--dev_priv->uncore.forcewake_count =3D=3D 0) > > + dev_priv->uncore.funcs.force_wake_put(dev_priv, > > + FORCEWAKE_ALL); > > + } > > + > > spin_unlock_irqrestore(&dev_priv->uncore.lock, flags); > > } > > = > > -- = > > 1.9.1 > = > -- = > Ville Syrj=E4l=E4 > Intel OTC -- = Ville Syrj=E4l=E4 Intel OTC