On pe, 2016-02-19 at 20:47 +0200, [email protected] wrote: > From: Ville Syrjälä <[email protected]> > > Starting from BDW the DE_PIPE interrupts for pipe B and C belong to the > relevant display power well. So we should make sure we've finished > processing them before turning off the power well. > > The pipe interrupts shouldn't really happen at this point anymore since > we've already shut down the planes/pipes/whatnot, but being a bit > paranoid shouldn't hurt. > > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/i915_irq.c | 16 ++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 19 +++++++++++++++++++ > 3 files changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index d56c261ad867..a9048e1b96e5 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3366,6 +3366,22 @@ void gen8_irq_power_well_post_enable(struct > drm_i915_private *dev_priv, > spin_unlock_irq(&dev_priv->irq_lock); > } > > +void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv, > + unsigned int pipe_mask) > +{ > + spin_lock_irq(&dev_priv->irq_lock); > + if (pipe_mask & 1 << PIPE_A) > + GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_A); > + if (pipe_mask & 1 << PIPE_B) > + GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_B); > + if (pipe_mask & 1 << PIPE_C) > + GEN8_IRQ_RESET_NDX(DE_PIPE, PIPE_C); > + spin_unlock_irq(&dev_priv->irq_lock); > + > + /* make sure we're done processing display irqs */ > + synchronize_irq(dev_priv->dev->irq); > +} > + > static void cherryview_irq_preinstall(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 285b0570be9c..2618e3026e8b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -993,6 +993,8 @@ static inline bool intel_irqs_enabled(struct > drm_i915_private *dev_priv) > int intel_get_crtc_scanline(struct intel_crtc *crtc); > void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, > unsigned int pipe_mask); > +void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv, > + unsigned int pipe_mask); > > /* intel_crt.c */ > void intel_crt_init(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 59e9222223c9..1afa00855ed8 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -284,6 +284,13 @@ static void hsw_power_well_post_enable(struct > drm_i915_private *dev_priv) > 1 << PIPE_C | 1 << PIPE_B); > } > > +static void hsw_power_well_pre_disable(struct drm_i915_private *dev_priv) > +{ > + if (IS_BROADWELL(dev_priv)) > + gen8_irq_power_well_pre_disable(dev_priv, > + 1 << PIPE_C | 1 << PIPE_B); > +} > + > static void skl_power_well_post_enable(struct drm_i915_private *dev_priv, > struct i915_power_well *power_well) > { > @@ -309,6 +316,14 @@ static void skl_power_well_post_enable(struct > drm_i915_private *dev_priv, > } > } > > +static void skl_power_well_pre_disable(struct drm_i915_private *dev_priv, > + struct i915_power_well *power_well) > +{ > + if (power_well->data == SKL_DISP_PW_2) > + gen8_irq_power_well_pre_disable(dev_priv, > + 1 << PIPE_C | 1 << PIPE_B); > +} > + > static void hsw_set_power_well(struct drm_i915_private *dev_priv, > struct i915_power_well *power_well, bool enable) > { > @@ -334,6 +349,7 @@ static void hsw_set_power_well(struct drm_i915_private > *dev_priv, > > } else { > if (enable_requested) { > + hsw_power_well_pre_disable(dev_priv); > I915_WRITE(HSW_PWR_WELL_DRIVER, 0); > POSTING_READ(HSW_PWR_WELL_DRIVER); > DRM_DEBUG_KMS("Requesting to disable the power well\n"); > @@ -663,6 +679,9 @@ static void skl_set_power_well(struct drm_i915_private > *dev_priv, > state_mask = SKL_POWER_WELL_STATE(power_well->data); > is_enabled = tmp & state_mask; > > + if (!enable && enable_requested) > + skl_power_well_pre_disable(dev_priv, power_well); > +
Why not putting this in the existing if branch? Either way: Reviewed-by: Imre Deak <[email protected]> > if (enable) { > if (!enable_requested) { > WARN((tmp & state_mask) && _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
