On Mon, May 05, 2014 at 06:17:30PM +0530, deepa...@linux.intel.com wrote:
> From: Ben Widawsky <benjamin.widaw...@intel.com>
>
> 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.
>
> v2: Move the commit about not touching the ringbuffer interrupt to the
> snb_* function where it belongs (Rodrigo)
>
> v3: Rebased on Paulo's runtime PM changes
>
> v4: Not well validated, but rebase on
> commit 730488b2eddded4497f63f70867b1256cd9e117c
> Author: Paulo Zanoni <paulo.r.zan...@intel.com>
> Date: Fri Mar 7 20:12:32 2014 -0300
>
> drm/i915: kill dev_priv->pm.regsave
>
> v5: Rebased on latest code base. (Deepak)
>
> v6: Remove conflict markers, Unnecessary empty line and use right
> IIR interrupt (Ville)
>
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> Signed-off-by: Deepak S <deepa...@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 75
> ++++++++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_pm.c | 38 ++++++++++++++++++--
> 4 files changed, 112 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2d76183..6af51ad 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -248,6 +248,49 @@ 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->pm.irqs_disabled) {
> + WARN(1, "IRQs disabled\n");
> + return;
> + }
Could be just 'if (WARN...' just like in the snb function.
> +
> + 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);
I just realized that this doesn't actually clear the old bits. I think
we should just kill the RMW here and mandate that all users of GT_IMR(2)
need to use this function.
Also you're forgetting to initialize pm_irq_mask. I guess it should be
initialized in gen8_gt_irq_postinstall() to match the gen6 code.
> + 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;
> @@ -1098,8 +1141,12 @@ static void gen6_pm_rps_work(struct work_struct *work)
> spin_lock_irq(&dev_priv->irq_lock);
> 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, dev_priv->pm_rps_events);
> + if (IS_BROADWELL(dev_priv->dev))
> + bdw_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
> + else {
> + /* Make sure not to corrupt PMIMR state used by ringbuffer */
> + snb_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
> + }
> spin_unlock_irq(&dev_priv->irq_lock);
>
> /* Make sure we didn't queue anything we're not going to process. */
> @@ -1296,6 +1343,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 & dev_priv->pm_rps_events) == 0)
> + return;
> +
> + spin_lock(&dev_priv->irq_lock);
> + dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events;
> + bdw_disable_pm_irq(dev_priv, pm_iir & dev_priv->pm_rps_events);
> + spin_unlock(&dev_priv->irq_lock);
> +
> + queue_work(dev_priv->wq, &dev_priv->rps.work);
> +}
> +
> static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev,
> struct drm_i915_private *dev_priv,
> u32 master_ctl)
> @@ -1334,6 +1394,17 @@ 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 & dev_priv->pm_rps_events) {
> + ret = IRQ_HANDLED;
> + gen8_rps_irq_handler(dev_priv, tmp);
> + I915_WRITE(GEN8_GT_IIR(2),
> + tmp & dev_priv->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 0eff337..ca4f8b9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4194,6 +4194,7 @@ enum punit_power_well {
> #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 d8b540b..d560a9b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -653,6 +653,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 intel_runtime_pm_disable_interrupts(struct drm_device *dev);
> void intel_runtime_pm_restore_interrupts(struct drm_device *dev);
> int intel_get_crtc_scanline(struct intel_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 834c49c..0e69c97 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3246,6 +3246,26 @@ 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)) &
> + ~dev_priv->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 (GEN8_GT_IMR(2)) to mask PM interrupts. The only risk is in
> + * leaving stale bits in GEN8_GT_IIR(2) and GEN8_GT_IMR(2) which
> + * gen8_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), dev_priv->pm_rps_events);
> +}
> +
> static void gen6_disable_rps_interrupts(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3272,7 +3292,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)
> @@ -3344,6 +3367,17 @@ int intel_enable_rc6(const struct drm_device *dev)
> return i915.enable_rc6;
> }
>
> +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, dev_priv->pm_rps_events);
> + I915_WRITE(GEN8_GT_IIR(2), dev_priv->pm_rps_events);
> + spin_unlock_irq(&dev_priv->irq_lock);
> +}
> +
> static void gen6_enable_rps_interrupts(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3446,7 +3480,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.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
