From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 6/6] drm/i915: drop locking from asle pipestat enable Date: Tue, 30 Apr 2013 10:47:58 +0200 Message-ID: <20130430084758.GD6169@phenom.ffwll.local> References: <0a77d3e8564ea40866fe5b301d68e49120e7213d.1367223972.git.jani.nikula@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com [209.85.212.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 6C267E5C2F for ; Tue, 30 Apr 2013 01:44:53 -0700 (PDT) Received: by mail-wi0-f177.google.com with SMTP id hi8so257250wib.16 for ; Tue, 30 Apr 2013 01:44:52 -0700 (PDT) Content-Disposition: inline In-Reply-To: <0a77d3e8564ea40866fe5b301d68e49120e7213d.1367223972.git.jani.nikula@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 To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Apr 29, 2013 at 01:02:55PM +0300, Jani Nikula wrote: > Enable asle pipestat earlier in i915/i965 irq postinstall to not need > irq_lock in i915_enable_asle_pipestat(). > > Signed-off-by: Jani Nikula Honestly I'm not too fond of too clever init sequence ordering - we already get the inherent constraints of the setup sequence wrong way too often. Trying to avoid a spinlock at setup time with clever ordering feels like the wrong tradeoff here. So I'd prefer a WARN(!spin_is_locked) instead in enable_pipestat (plus lock grabbing in the vlv/i965 postinstall code). All other patches from this series merged, thanks. -Daniel > --- > drivers/gpu/drm/i915/i915_irq.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 03a31be..0243db1 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -354,18 +354,13 @@ i915_disable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask) > static void i915_enable_asle_pipestat(struct drm_device *dev) > { > drm_i915_private_t *dev_priv = dev->dev_private; > - unsigned long irqflags; > > if (!dev_priv->opregion.asle || !IS_MOBILE(dev)) > return; > > - spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > - > i915_enable_pipestat(dev_priv, 1, PIPE_LEGACY_BLC_EVENT_ENABLE); > if (INTEL_INFO(dev)->gen >= 4) > i915_enable_pipestat(dev_priv, 0, PIPE_LEGACY_BLC_EVENT_ENABLE); > - > - spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > } > > /** > @@ -2953,6 +2948,8 @@ static int i915_irq_postinstall(struct drm_device *dev) > I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT | > I915_USER_INTERRUPT; > > + i915_enable_asle_pipestat(dev); > + > if (I915_HAS_HOTPLUG(dev)) { > I915_WRITE(PORT_HOTPLUG_EN, 0); > POSTING_READ(PORT_HOTPLUG_EN); > @@ -2967,8 +2964,6 @@ static int i915_irq_postinstall(struct drm_device *dev) > I915_WRITE(IER, enable_mask); > POSTING_READ(IER); > > - i915_enable_asle_pipestat(dev); > - > return 0; > } > > @@ -3178,6 +3173,7 @@ static int i965_irq_postinstall(struct drm_device *dev) > enable_mask |= I915_BSD_USER_INTERRUPT; > > i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE); > + i915_enable_asle_pipestat(dev); > > /* > * Enable some error detection, note the instruction error mask > @@ -3201,8 +3197,6 @@ static int i965_irq_postinstall(struct drm_device *dev) > I915_WRITE(PORT_HOTPLUG_EN, 0); > POSTING_READ(PORT_HOTPLUG_EN); > > - i915_enable_asle_pipestat(dev); > - > return 0; > } > > -- > 1.7.9.5 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch