[Intel-gfx] [PATCH 07/14] drm/i915: s/pm._irqs_disabled/pm.irqs_enabled/

Paulo Zanoni przanoni at gmail.com
Thu Oct 2 22:36:11 CEST 2014


2014-09-30 5:56 GMT-03:00 Daniel Vetter <daniel.vetter at ffwll.ch>:
> Double negations just parse harder. Also this allows us to ditch some
> init code since clearing to 0 dtrt. Also ditch the assignment in
> intel_pm_setup, that's not redundant since we do the assignement now
> while setting up interrupts.
>
> While at it do engage in a bit of OCD and wrap up the few lines of
> setup/teardown code into little helper functions: intel_irq_fini for
> cleanup and intel_irq_init_hw for hw setup.

So the werid thing is that we now have:
- intel_irq_init
- intel_irq_init_hw
- intel_irq_fini

But the intel_irq_fini doesn't finish what intel_irq_init started, it
finishes what intel_irq_init_hw started. Since the functions you
introduced are basically wrappers to drm_irq_{un,}install, my bikeshed
would be to call the new functions simply intel_irq_install and
intel_irq_uninstall.

With or without that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c      |  9 +--------
>  drivers/gpu/drm/i915/i915_drv.h      |  4 +++-
>  drivers/gpu/drm/i915/i915_irq.c      | 26 +++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_display.c |  4 +---
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c      |  1 -
>  6 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 7075bd2adee8..969f0cff9fef 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1338,14 +1338,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>
>         intel_power_domains_init_hw(dev_priv);
>
> -       /*
> -        * We enable some interrupt sources in our postinstall hooks, so mark
> -        * interrupts as enabled _before_ actually enabling them to avoid
> -        * special cases in our ordering checks.
> -        */
> -       dev_priv->pm._irqs_disabled = false;
> -
> -       ret = drm_irq_install(dev, dev->pdev->irq);
> +       ret = intel_irq_init_hw(dev_priv);
>         if (ret)
>                 goto cleanup_gem_stolen;
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1d8a4b7bf61a..1eec0e81a5ca 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1405,7 +1405,7 @@ struct ilk_wm_values {
>   */
>  struct i915_runtime_pm {
>         bool suspended;
> -       bool _irqs_disabled;
> +       bool irqs_enabled;
>  };
>
>  enum intel_pipe_crc_source {
> @@ -2285,6 +2285,8 @@ void gen6_set_pm_mask(struct drm_i915_private *dev_priv, u32 pm_iir,
>                                                         int new_delay);
>  extern void intel_irq_init(struct drm_device *dev);
>  extern void intel_hpd_init(struct drm_device *dev);
> +int intel_irq_init_hw(struct drm_i915_private *dev_priv);
> +void intel_irq_fini(struct drm_i915_private *dev_priv);
>
>  extern void intel_uncore_sanitize(struct drm_device *dev);
>  extern void intel_uncore_early_sanitize(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1ddfc81caaaa..d9d09a9b4fc7 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4656,9 +4656,6 @@ void intel_irq_init(struct drm_device *dev)
>
>         pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
>
> -       /* Haven't installed the IRQ handler yet */
> -       dev_priv->pm._irqs_disabled = true;
> -
>         if (IS_GEN2(dev)) {
>                 dev->max_vblank_count = 0;
>                 dev->driver->get_vblank_counter = i8xx_get_vblank_counter;
> @@ -4767,13 +4764,32 @@ void intel_hpd_init(struct drm_device *dev)
>         spin_unlock_irq(&dev_priv->irq_lock);
>  }
>
> +int intel_irq_init_hw(struct drm_i915_private *dev_priv)
> +{
> +       /*
> +        * We enable some interrupt sources in our postinstall hooks, so mark
> +        * interrupts as enabled _before_ actually enabling them to avoid
> +        * special cases in our ordering checks.
> +        */
> +       dev_priv->pm.irqs_enabled = true;
> +
> +       return drm_irq_install(dev_priv->dev, dev_priv->dev->pdev->irq);
> +}
> +
> +void intel_irq_fini(struct drm_i915_private *dev_priv)
> +{
> +       drm_irq_uninstall(dev_priv->dev);
> +       intel_hpd_cancel_work(dev_priv);
> +       dev_priv->pm.irqs_enabled = false;
> +}
> +
>  /* Disable interrupts so we can allow runtime PM. */
>  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_enabled = false;
>  }
>
>  /* Restore interrupts so we can recover from runtime PM. */
> @@ -4781,7 +4797,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_enabled = true;
>         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 1dd470fcddf2..41607ac3e6eb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13263,9 +13263,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
>          * Too much stuff here (turning of rps, connectors, ...) would
>          * experience fancy races otherwise.
>          */
> -       drm_irq_uninstall(dev);
> -       intel_hpd_cancel_work(dev_priv);
> -       dev_priv->pm._irqs_disabled = true;
> +       intel_irq_fini(dev_priv);
>
>         /*
>          * Due to the hpd irq storm handling the hotplug work can re-arm the
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bf321993a88e..cf07e2225911 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -764,7 +764,7 @@ 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;
> +       return dev_priv->pm.irqs_enabled;
>  }
>
>  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 06c1ea0a7bfd..bf79f86ed06b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6507,5 +6507,4 @@ void intel_pm_setup(struct drm_device *dev)
>                           intel_gen6_powersave_work);
>
>         dev_priv->pm.suspended = false;
> -       dev_priv->pm._irqs_disabled = false;
>  }
> --
> 2.1.1
>



-- 
Paulo Zanoni



More information about the Intel-gfx mailing list