[Intel-gfx] [PATCH 2/4] drm/i915: add helper for checking whether IRQs are enabled

Paulo Zanoni przanoni at gmail.com
Mon Jul 14 17:19:54 CEST 2014


2014-07-14 12:06 GMT-03:00 Paulo Zanoni <przanoni at gmail.com>:
> 2014-06-20 13:29 GMT-03:00 Jesse Barnes <jbarnes at virtuousgeek.org>:
>> Now that we use the runtime IRQ enable/disable functions in our suspend
>> path, we can simply check the pm._irqs_disabled flag everywhere.  So
>> rename it to catch the users, and add an inline for it to make the
>> checks clear everywhere.
>>
>> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

Looks like this patch brings some WARNs at init time, so I retract
that R-B tag :)

>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 +-
>>  drivers/gpu/drm/i915/i915_drv.h      |  2 +-
>>  drivers/gpu/drm/i915/i915_gem.c      |  2 +-
>>  drivers/gpu/drm/i915/i915_irq.c      | 16 ++++++++--------
>>  drivers/gpu/drm/i915/intel_display.c |  2 +-
>>  drivers/gpu/drm/i915/intel_drv.h     |  9 ++++++++-
>>  drivers/gpu/drm/i915/intel_pm.c      |  6 +++---
>>  7 files changed, 23 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 76c2572..f3c0482 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2076,7 +2076,7 @@ static int i915_pc8_status(struct seq_file *m, void *unused)
>>
>>         seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->mm.busy));
>>         seq_printf(m, "IRQs disabled: %s\n",
>> -                  yesno(dev_priv->pm.irqs_disabled));
>> +                  yesno(!intel_irqs_enabled(dev_priv)));
>>
>>         return 0;
>>  }
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 0640071..d12e9b7 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1298,7 +1298,7 @@ struct ilk_wm_values {
>>   */
>>  struct i915_runtime_pm {
>>         bool suspended;
>> -       bool irqs_disabled;
>> +       bool _irqs_disabled;
>>  };
>>
>>  enum intel_pipe_crc_source {
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index d857f58..2f1815e 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1161,7 +1161,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
>>         unsigned long timeout_expire;
>>         int ret;
>>
>> -       WARN(dev_priv->pm.irqs_disabled, "IRQs disabled\n");
>> +       WARN(!intel_irqs_enabled(dev_priv), "IRQs disabled");
>>
>>         if (i915_seqno_passed(ring->get_seqno(ring, true), seqno))
>>                 return 0;
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index fe3b309..bc953cc 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -136,7 +136,7 @@ ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
>>  {
>>         assert_spin_locked(&dev_priv->irq_lock);
>>
>> -       if (WARN_ON(dev_priv->pm.irqs_disabled))
>> +       if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>>                 return;
>>
>>         if ((dev_priv->irq_mask & mask) != 0) {
>> @@ -151,7 +151,7 @@ ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
>>  {
>>         assert_spin_locked(&dev_priv->irq_lock);
>>
>> -       if (dev_priv->pm.irqs_disabled)
>> +       if (!intel_irqs_enabled(dev_priv))
>>                 return;
>>
>>         if ((dev_priv->irq_mask & mask) != mask) {
>> @@ -173,7 +173,7 @@ static void ilk_update_gt_irq(struct drm_i915_private *dev_priv,
>>  {
>>         assert_spin_locked(&dev_priv->irq_lock);
>>
>> -       if (WARN_ON(dev_priv->pm.irqs_disabled))
>> +       if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>>                 return;
>>
>>         dev_priv->gt_irq_mask &= ~interrupt_mask;
>> @@ -206,7 +206,7 @@ static void snb_update_pm_irq(struct drm_i915_private *dev_priv,
>>
>>         assert_spin_locked(&dev_priv->irq_lock);
>>
>> -       if (WARN_ON(dev_priv->pm.irqs_disabled))
>> +       if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>>                 return;
>>
>>         new_val = dev_priv->pm_irq_mask;
>> @@ -264,7 +264,7 @@ static void bdw_update_pm_irq(struct drm_i915_private *dev_priv,
>>
>>         assert_spin_locked(&dev_priv->irq_lock);
>>
>> -       if (WARN_ON(dev_priv->pm.irqs_disabled))
>> +       if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>>                 return;
>>
>>         new_val = dev_priv->pm_irq_mask;
>> @@ -420,7 +420,7 @@ static void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
>>
>>         assert_spin_locked(&dev_priv->irq_lock);
>>
>> -       if (WARN_ON(dev_priv->pm.irqs_disabled))
>> +       if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>>                 return;
>>
>>         I915_WRITE(SDEIMR, sdeimr);
>> @@ -4478,7 +4478,7 @@ void intel_runtime_pm_disable_interrupts(struct drm_device *dev)
>>         struct drm_i915_private *dev_priv = dev->dev_private;
>>
>>         dev->driver->irq_uninstall(dev);
>> -       dev_priv->pm.irqs_disabled = true;
>> +       dev_priv->pm._irqs_disabled = true;
>>  }
>>
>>  /* Restore interrupts so we can recover from runtime PM. */
>> @@ -4486,7 +4486,7 @@ void intel_runtime_pm_restore_interrupts(struct drm_device *dev)
>>  {
>>         struct drm_i915_private *dev_priv = dev->dev_private;
>>
>> -       dev_priv->pm.irqs_disabled = false;
>> +       dev_priv->pm._irqs_disabled = false;
>>         dev->driver->irq_preinstall(dev);
>>         dev->driver->irq_postinstall(dev);
>>  }
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 5e8e711..d993b69 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7323,7 +7323,7 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
>>          * gen-specific and since we only disable LCPLL after we fully disable
>>          * the interrupts, the check below should be enough.
>>          */
>> -       WARN(!dev_priv->pm.irqs_disabled, "IRQs enabled\n");
>> +       WARN(intel_irqs_enabled(dev_priv), "IRQs enabled\n");
>>  }
>>
>>  static void hsw_write_dcomp(struct drm_i915_private *dev_priv, uint32_t val)
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index ab5962b..4912738 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -687,7 +687,14 @@ 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);
>>  void i9xx_check_fifo_underruns(struct drm_device *dev);
>> -
>> +static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
>> +{
>> +       /*
>> +        * We only use drm_irq_uninstall() at unload and VT switch, so
>> +        * this is the only thing we need to check.
>> +        */
>> +       return !dev_priv->pm._irqs_disabled;
>> +}
>>
>>  /* intel_crt.c */
>>  void intel_crt_init(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 0bf9b0c..88734cb 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4879,7 +4879,7 @@ void intel_suspend_gt_powersave(struct drm_device *dev)
>>         struct drm_i915_private *dev_priv = dev->dev_private;
>>
>>         /* Interrupts should be disabled already to avoid re-arming. */
>> -       WARN_ON(dev->irq_enabled && !dev_priv->pm.irqs_disabled);
>> +       WARN_ON(intel_irqs_enabled(dev_priv));
>>
>>         flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>>
>> @@ -4891,7 +4891,7 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>>         struct drm_i915_private *dev_priv = dev->dev_private;
>>
>>         /* Interrupts should be disabled already to avoid re-arming. */
>> -       WARN_ON(dev->irq_enabled && !dev_priv->pm.irqs_disabled);
>> +       WARN_ON(intel_irqs_enabled(dev_priv));
>>
>>         if (IS_IRONLAKE_M(dev)) {
>>                 ironlake_disable_drps(dev);
>> @@ -6802,5 +6802,5 @@ void intel_pm_setup(struct drm_device *dev)
>>                           intel_gen6_powersave_work);
>>
>>         dev_priv->pm.suspended = false;
>> -       dev_priv->pm.irqs_disabled = false;
>> +       dev_priv->pm._irqs_disabled = false;
>>  }
>> --
>> 1.8.3.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni



-- 
Paulo Zanoni



More information about the Intel-gfx mailing list