On Thu, Oct 02, 2014 at 05:36:11PM -0300, Paulo Zanoni wrote:
> 2014-09-30 5:56 GMT-03:00 Daniel Vetter <daniel.vetter@xxxxxxxx>:
> > 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.
I like this idea, so changed the names while merging.
> With or without that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
Thanks for the review.
-Daniel
>
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > ---
> > 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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
