From mboxrd@z Thu Jan 1 00:00:00 1970 From: "S, Deepak" Subject: Re: [PATCH 8/9] drm/i915/bdw: Implement a basic PM interrupt handler Date: Fri, 07 Feb 2014 12:13:23 +0530 Message-ID: <52F4808B.6040803@intel.com> References: <1390969547-1018-1-git-send-email-benjamin.widawsky@intel.com> <1390969547-1018-10-git-send-email-benjamin.widawsky@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id 649D6FB6EE for ; Thu, 6 Feb 2014 22:43:27 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: benjamin.widawsky@intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Jan 29, 2014 at 9:55 AM, Ben Widawsky > > wrote: > > Almost all of it is reusable from the existing code. The primary > difference is we need to do even less in the interrupt handler, since > interrupts are not shared in the same way. > > The patch is mostly a copy-paste of the existing snb+ code, with updates > to the relevant parts requiring changes to the interrupt handling. As > such it /should/ be relatively trivial. It's highly likely that I missed > some places where I need a gen8 version of the PM interrupts, but it has > become invisible to me by now. > > This patch could probably be split into adding the new functions, > followed by actually handling the interrupts. Since the code is > currently disabled (and broken) I think the patch stands better by > itself. > > Signed-off-by: Ben Widawsky > > --- > drivers/gpu/drm/i915/i915_irq.c | 80 > ++++++++++++++++++++++++++++++++++++++-- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_drv.h | 2 + > drivers/gpu/drm/i915/intel_pm.c | 39 +++++++++++++++++++- > 4 files changed, 117 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > b/drivers/gpu/drm/i915/i915_irq.c > index 72ade87..ab0c7ac 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -214,6 +214,53 @@ static bool ivb_can_enable_err_int(struct > drm_device *dev) > return true; > } > > +/** > + * bdw_update_pm_irq - update GT interrupt 2 > + * @dev_priv: driver private > + * @interrupt_mask: mask of interrupt bits to update > + * @enabled_irq_mask: mask of interrupt bits to enable > + * > + * Copied from the snb function, updated with relevant register > offsets > + */ > +static void bdw_update_pm_irq(struct drm_i915_private *dev_priv, > + uint32_t interrupt_mask, > + uint32_t enabled_irq_mask) > +{ > + uint32_t new_val; > + > + assert_spin_locked(&dev_priv->irq_lock); > + > + if (dev_priv->pc8.irqs_disabled) { > + WARN(1, "IRQs disabled\n"); > + dev_priv->pc8.regsave.gen6_pmimr &= ~interrupt_mask; > + dev_priv->pc8.regsave.gen6_pmimr |= (~enabled_irq_mask & > + interrupt_mask); > + return; > + } > + > + new_val = dev_priv->pm_irq_mask; > + new_val &= ~interrupt_mask; > + new_val |= (~enabled_irq_mask & interrupt_mask); > + > + if (new_val != dev_priv->pm_irq_mask) { > + dev_priv->pm_irq_mask = new_val; > + I915_WRITE(GEN8_GT_IMR(2), I915_READ(GEN8_GT_IMR(2)) | > + dev_priv->pm_irq_mask); > + POSTING_READ(GEN8_GT_IMR(2)); > + } > +} > + > +void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t > mask) > +{ > + bdw_update_pm_irq(dev_priv, mask, mask); > +} > + > +void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t > mask) > +{ > + bdw_update_pm_irq(dev_priv, mask, 0); > +} > + > + > static bool cpt_can_enable_serr_int(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -997,11 +1044,15 @@ static void gen6_pm_rps_work(struct > work_struct *work) > pm_iir = dev_priv->rps.pm_iir; > dev_priv->rps.pm_iir = 0; > /* Make sure not to corrupt PMIMR state used by ringbuffer > code */ > - snb_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS); > + if (IS_BROADWELL(dev_priv->dev)) > + bdw_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS); > + else { > + /* Make sure we didn't queue anything we're not > going to process. */ > + snb_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS); > + WARN_ON(pm_iir & ~GEN6_PM_RPS_EVENTS); > + } > spin_unlock_irq(&dev_priv->irq_lock); > > - /* Make sure we didn't queue anything we're not going to > process. */ > - WARN_ON(pm_iir & ~GEN6_PM_RPS_EVENTS); > > if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0) > return; > @@ -1192,6 +1243,19 @@ static void snb_gt_irq_handler(struct > drm_device *dev, > ivybridge_parity_error_irq_handler(dev, gt_iir); > } > > +static void gen8_rps_irq_handler(struct drm_i915_private *dev_priv, > u32 pm_iir) > +{ > + if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0) > + return; > + > + spin_lock(&dev_priv->irq_lock); > + dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RPS_EVENTS; > + bdw_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); > +} We can use the "gen6_rps_irq_handler" right? > static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev, > struct drm_i915_private > *dev_priv, > u32 master_ctl) > @@ -1227,6 +1291,16 @@ static irqreturn_t gen8_gt_irq_handler(struct > drm_device *dev, > DRM_ERROR("The master control interrupt > lied (GT1)!\n"); > } > > + if (master_ctl & GEN8_GT_PM_IRQ) { > + tmp = I915_READ(GEN8_GT_IIR(2)); > + if (tmp & GEN6_PM_RPS_EVENTS) { > + ret = IRQ_HANDLED; > + gen8_rps_irq_handler(dev_priv, tmp); > + I915_WRITE(GEN8_GT_IIR(1), tmp & > GEN6_PM_RPS_EVENTS); > + } else > + DRM_ERROR("The master control interrupt lied > (PM)!\n"); > + } > + > if (master_ctl & GEN8_GT_VECS_IRQ) { > tmp = I915_READ(GEN8_GT_IIR(3)); > if (tmp) { > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index d77720e..f9e582e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4024,6 +4024,7 @@ > #define GEN8_DE_PIPE_A_IRQ (1<<16) > #define GEN8_DE_PIPE_IRQ(pipe) (1<<(16+pipe)) > #define GEN8_GT_VECS_IRQ (1<<6) > +#define GEN8_GT_PM_IRQ (1<<4) > #define GEN8_GT_VCS2_IRQ (1<<3) > #define GEN8_GT_VCS1_IRQ (1<<2) > #define GEN8_GT_BCS_IRQ (1<<1) > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 44067bc..7b49779 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -608,6 +608,8 @@ void ilk_enable_gt_irq(struct drm_i915_private > *dev_priv, uint32_t mask); > void ilk_disable_gt_irq(struct drm_i915_private *dev_priv, > uint32_t mask); > void snb_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t > mask); > void snb_disable_pm_irq(struct drm_i915_private *dev_priv, > uint32_t mask); > +void bdw_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t > mask); > +void bdw_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t > mask); > void hsw_pc8_disable_interrupts(struct drm_device *dev); > void hsw_pc8_restore_interrupts(struct drm_device *dev); > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index deaaaf2..a5c9142 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3091,6 +3091,25 @@ void valleyview_set_rps(struct drm_device > *dev, u8 val) > trace_intel_gpu_freq_change(vlv_gpu_freq(dev_priv, val)); > } > > +static void gen8_disable_rps_interrupts(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + I915_WRITE(GEN6_PMINTRMSK, 0xffffffff); > + I915_WRITE(GEN8_GT_IER(2), I915_READ(GEN8_GT_IER(2)) & > + ~GEN6_PM_RPS_EVENTS); > + /* Complete PM interrupt masking here doesn't race with the > rps work > + * item again unmasking PM interrupts because that is using > a different > + * register (PMIMR) to mask PM interrupts. The only risk is > in leaving > + * stale bits in PMIIR and PMIMR which gen6_enable_rps will > clean up. */ > + > + spin_lock_irq(&dev_priv->irq_lock); > + dev_priv->rps.pm_iir = 0; > + spin_unlock_irq(&dev_priv->irq_lock); > + > + I915_WRITE(GEN8_GT_IIR(2), GEN6_PM_RPS_EVENTS); > +} > + > static void gen6_disable_rps_interrupts(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -3116,7 +3135,10 @@ static void gen6_disable_rps(struct > drm_device *dev) > I915_WRITE(GEN6_RC_CONTROL, 0); > I915_WRITE(GEN6_RPNSWREQ, 1 << 31); > > - gen6_disable_rps_interrupts(dev); > + if (IS_BROADWELL(dev)) > + gen8_disable_rps_interrupts(dev); > + else > + gen6_disable_rps_interrupts(dev); > } > > static void valleyview_disable_rps(struct drm_device *dev) > @@ -3161,6 +3183,19 @@ int intel_enable_rc6(const struct drm_device > *dev) > return INTEL_RC6_ENABLE; > } > > +static void gen8_enable_rps_interrupts(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + spin_lock_irq(&dev_priv->irq_lock); > + WARN_ON(dev_priv->rps.pm_iir); > + bdw_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS); > + I915_WRITE(GEN8_GT_IIR(2), GEN6_PM_RPS_EVENTS); > + spin_unlock_irq(&dev_priv->irq_lock); > + > + I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS); > +} As per the spec, I think we need to unmask bit 31 also, if not i don't this we will get PM inetrrupts. > static void gen6_enable_rps_interrupts(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -3263,7 +3298,7 @@ static void gen8_enable_rps(struct drm_device > *dev) > > gen6_set_rps(dev, (I915_READ(GEN6_GT_PERF_STATUS) & 0xff00) > >> 8); > > - gen6_enable_rps_interrupts(dev); > + gen8_enable_rps_interrupts(dev); > > gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); > } > -- > 1.8.5.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >