Deepak, this patch can use review. If you have time, can you please take
a look.

There were some rebase conflicts from the last version, so please make
sure to recheck carefully (since I think you did look before).

Thanks.

On Wed, Mar 19, 2014 at 06:31:17PM -0700, 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.
> 
> 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
> 
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_irq.c  | 84 
> +++++++++++++++++++++++++++++++++++++---
>  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, 119 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 4b4aeb3..2f9ec6e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -175,6 +175,7 @@ static void snb_update_pm_irq(struct drm_i915_private 
> *dev_priv,
>               return;
>       }
>  
> +     /* Make sure not to corrupt PMIMR state used by ringbuffer code */
>       new_val = dev_priv->pm_irq_mask;
>       new_val &= ~interrupt_mask;
>       new_val |= (~enabled_irq_mask & interrupt_mask);
> @@ -214,6 +215,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->pm.irqs_disabled) {
> +             WARN(1, "IRQs disabled\n");
> +             dev_priv->pm.regsave.gen6_pmimr &= ~interrupt_mask;
> +             dev_priv->pm.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;
> @@ -1131,13 +1179,16 @@ 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, GEN6_PM_RPS_EVENTS);
> +     if (IS_BROADWELL(dev_priv->dev))
> +             bdw_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
> +     else {
> +             snb_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
> +             /* Make sure we didn't queue anything we're not going to
> +              * process. */
> +             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;
>  
> @@ -1330,6 +1381,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);
> +}
> +
>  static irqreturn_t gen8_gt_irq_handler(struct drm_device *dev,
>                                      struct drm_i915_private *dev_priv,
>                                      u32 master_ctl)
> @@ -1365,6 +1429,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 6174fda..67952c7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4086,6 +4086,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 60ffad3..fde5df7 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -623,6 +623,8 @@ 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 hsw_runtime_pm_disable_interrupts(struct drm_device *dev);
>  void hsw_runtime_pm_restore_interrupts(struct drm_device *dev);
> +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);
>  
>  
>  /* intel_crt.c */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9486396..38e4d60 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3155,6 +3155,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;
> @@ -3180,7 +3199,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)
> @@ -3225,6 +3247,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);
> +}
> +
>  static void gen6_enable_rps_interrupts(struct drm_device *dev)
>  {
>       struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3339,7 +3374,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.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to