From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 6.3/9] drm/i915: merge HSW and SNB PM irq handlers Date: Tue, 20 Aug 2013 16:27:52 +0200 Message-ID: <20130820142752.GV776@phenom.ffwll.local> References: <20130814192109.GE490@bwidawsk.net> <1376578292-9915-1-git-send-email-przanoni@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f180.google.com (mail-ea0-f180.google.com [209.85.215.180]) by gabe.freedesktop.org (Postfix) with ESMTP id 59AF6E712A for ; Tue, 20 Aug 2013 07:27:41 -0700 (PDT) Received: by mail-ea0-f180.google.com with SMTP id h10so261024eaj.11 for ; Tue, 20 Aug 2013 07:27:40 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1376578292-9915-1-git-send-email-przanoni@gmail.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: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Thu, Aug 15, 2013 at 11:51:32AM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni > > Because hsw_pm_irq_handler does exactly what gen6_rps_irq_handler does > and also processes the 2 additional VEBOX bits. So merge those > functions and wrap the VEBOX bits on a HAS_VEBOX check. This > check isn't really necessary since the bits are reserved on > SNB/IVB/VLV, but it's a good documentation on who uses them. > > v2: - Change IS_HASWELL check to HAS_VEBOX > > Signed-off-by: Paulo Zanoni > Reviewed-by: Ben Widawsky Ok, I've merged everything up to this patch. Please double check that I didn't botch the job and merge an older version of a revised patch ;-) Thanks for patches & review. -Daniel > --- > drivers/gpu/drm/i915/i915_irq.c | 50 ++++++++++------------------------------- > 1 file changed, 12 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index e0c6f7d..caf83da 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -942,28 +942,6 @@ static void snb_gt_irq_handler(struct drm_device *dev, > ivybridge_parity_error_irq_handler(dev); > } > > -/* Legacy way of handling PM interrupts */ > -static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, > - u32 pm_iir) > -{ > - /* > - * IIR bits should never already be set because IMR should > - * prevent an interrupt from being shown in IIR. The warning > - * displays a case where we've unsafely cleared > - * dev_priv->rps.pm_iir. Although missing an interrupt of the same > - * type is not a problem, it displays a problem in the logic. > - * > - * The mask bit in IMR is cleared by dev_priv->rps.work. > - */ > - > - spin_lock(&dev_priv->irq_lock); > - dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RPS_EVENTS; > - snb_disable_pm_irq(dev_priv, pm_iir & GEN6_PM_RPS_EVENTS); > - spin_unlock(&dev_priv->irq_lock); > - > - queue_work(dev_priv->wq, &dev_priv->rps.work); > -} > - > #define HPD_STORM_DETECT_PERIOD 1000 > #define HPD_STORM_THRESHOLD 5 > > @@ -1030,13 +1008,10 @@ static void dp_aux_irq_handler(struct drm_device *dev) > wake_up_all(&dev_priv->gmbus_wait_queue); > } > > -/* Unlike gen6_rps_irq_handler() from which this function is originally derived, > - * we must be able to deal with other PM interrupts. This is complicated because > - * of the way in which we use the masks to defer the RPS work (which for > - * posterity is necessary because of forcewake). > - */ > -static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv, > - u32 pm_iir) > +/* The RPS events need forcewake, so we add them to a work queue and mask their > + * IMR bits until the work is done. Other interrupts can be processed without > + * the work queue. */ > +static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir) > { > if (pm_iir & GEN6_PM_RPS_EVENTS) { > spin_lock(&dev_priv->irq_lock); > @@ -1047,12 +1022,14 @@ static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv, > queue_work(dev_priv->wq, &dev_priv->rps.work); > } > > - if (pm_iir & PM_VEBOX_USER_INTERRUPT) > - notify_ring(dev_priv->dev, &dev_priv->ring[VECS]); > + if (HAS_VEBOX(dev_priv->dev)) { > + if (pm_iir & PM_VEBOX_USER_INTERRUPT) > + notify_ring(dev_priv->dev, &dev_priv->ring[VECS]); > > - if (pm_iir & PM_VEBOX_CS_ERROR_INTERRUPT) { > - DRM_ERROR("VEBOX CS error interrupt 0x%08x\n", pm_iir); > - i915_handle_error(dev_priv->dev, false); > + if (pm_iir & PM_VEBOX_CS_ERROR_INTERRUPT) { > + DRM_ERROR("VEBOX CS error interrupt 0x%08x\n", pm_iir); > + i915_handle_error(dev_priv->dev, false); > + } > } > } > > @@ -1427,10 +1404,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg) > if (INTEL_INFO(dev)->gen >= 6) { > u32 pm_iir = I915_READ(GEN6_PMIIR); > if (pm_iir) { > - if (IS_HASWELL(dev)) > - hsw_pm_irq_handler(dev_priv, pm_iir); > - else > - gen6_rps_irq_handler(dev_priv, pm_iir); > + gen6_rps_irq_handler(dev_priv, pm_iir); > I915_WRITE(GEN6_PMIIR, pm_iir); > ret = IRQ_HANDLED; > } > -- > 1.8.1.2 > > _______________________________________________ > 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