From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 45/53] drm/i915/bdw: Do not call intel_runtime_pm_get() in an interrupt Date: Wed, 18 Jun 2014 22:54:13 +0200 Message-ID: <20140618205413.GH5821@phenom.ffwll.local> References: <1402673891-14618-1-git-send-email-oscar.mateo@intel.com> <1402673891-14618-46-git-send-email-oscar.mateo@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f176.google.com (mail-wi0-f176.google.com [209.85.212.176]) by gabe.freedesktop.org (Postfix) with ESMTP id A9A196E419 for ; Wed, 18 Jun 2014 13:54:21 -0700 (PDT) Received: by mail-wi0-f176.google.com with SMTP id n3so8332984wiv.3 for ; Wed, 18 Jun 2014 13:54:20 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1402673891-14618-46-git-send-email-oscar.mateo@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: oscar.mateo@intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Jun 13, 2014 at 04:38:03PM +0100, oscar.mateo@intel.com wrote: > From: Oscar Mateo > > Or with a spinlock grabbed, because it might sleep, which is not > a nice thing to do. Instead, do the runtime_pm get/put together > with the create/destroy request, and handle the forcewake get/put > directly. > > Signed-off-by: Oscar Mateo Looks like a fixup that should be squashed into relevant earlier patches. -Daniel > --- > drivers/gpu/drm/i915/intel_lrc.c | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index d33e622..ea4b358 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -159,6 +159,7 @@ static void execlists_elsp_write(struct intel_engine_cs *ring, > struct drm_i915_private *dev_priv = ring->dev->dev_private; > uint64_t temp = 0; > uint32_t desc[4]; > + unsigned long flags; > > /* XXX: You must always write both descriptors in the order below. */ > if (ctx_obj1) > @@ -172,9 +173,17 @@ static void execlists_elsp_write(struct intel_engine_cs *ring, > desc[3] = (u32)(temp >> 32); > desc[2] = (u32)temp; > > - /* Set Force Wakeup bit to prevent GT from entering C6 while > - * ELSP writes are in progress */ > - gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL); > + /* Set Force Wakeup bit to prevent GT from entering C6 while ELSP writes > + * are in progress. > + * > + * The other problem is that we can't just call gen6_gt_force_wake_get() > + * because that function calls intel_runtime_pm_get(), which might sleep. > + * Instead, we do the runtime_pm_get/put when creating/destroying requests. > + */ > + spin_lock_irqsave(&dev_priv->uncore.lock, flags); > + if (dev_priv->uncore.forcewake_count++ == 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]); > I915_WRITE(RING_ELSP(ring), desc[0]); > @@ -185,8 +194,11 @@ static void execlists_elsp_write(struct intel_engine_cs *ring, > /* ELSP is a write only register, so this serves as a posting read */ > POSTING_READ(RING_EXECLIST_STATUS(ring)); > > - /* Release Force Wakeup */ > - gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); > + /* Release Force Wakeup (see the big comment above). */ > + spin_lock_irqsave(&dev_priv->uncore.lock, flags); > + if (--dev_priv->uncore.forcewake_count == 0) > + dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL); > + spin_unlock_irqrestore(&dev_priv->uncore.lock, flags); > } > > static int execlists_ctx_write_tail(struct drm_i915_gem_object *ctx_obj, u32 tail) > @@ -353,6 +365,9 @@ static void execlists_free_request_task(struct work_struct *work) > struct intel_ctx_submit_request *req = > container_of(work, struct intel_ctx_submit_request, work); > struct drm_device *dev = req->ring->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + intel_runtime_pm_put(dev_priv); > > mutex_lock(&dev->struct_mutex); > i915_gem_context_unreference(req->ctx); > @@ -378,6 +393,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring, > req->ring = ring; > req->tail = tail; > INIT_WORK(&req->work, execlists_free_request_task); > + intel_runtime_pm_get(dev_priv); > > spin_lock_irqsave(&ring->execlist_lock, flags); > > -- > 1.9.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch