From 9939fba226649c62630a74d36ee45c5d5402b460 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Thu, 20 Nov 2014 23:01:47 +0200 Subject: [PATCH] drm/i915: mask RPS IRQs properly when disabling RPS Atm, igt/gem_reset_stats can trigger the recently added WARN on left-over PM_IIR bits in gen6_enable_rps_interrupts(). There are two reasons for this: 1. we call intel_enable_gt_powersave() without a preceeding intel_disable_gt_powersave() 2. gen6_disable_rps_interrupts() doesn't mask interrupts in PM_IMR 1. means RPS interrupts will remain enabled and can be serviced during the HW initialization after a GPU reset. 2. means even if we called gen6_disable_rps_interrupts() any new RPS interrupt during RPS initialization would still propagate to PM_IIR too early (though wouldn't be serviced). This patch solves the 2. issue by also masking interrupts in PM_IMR, the following patch fixes 1. getting rid of the WARN. This also makes intel_enable_gt_powersave() and intel_disable_gt_powersave() more symmetric. Since gen6_disable_rps_interrupts() is called during driver loading with i915 interrupts disabled add a new version of gen6_disable_pm_irq() that doesn't WARN for this. Also while at it, get the irq_lock around the whole PM_IMR/IER/IIR programming sequence and make sure that any queued PM_IIR bit is also cleared. The WARN was caught by PRTS after I sent my previous RPS sanitizing patchset and I could easily reproduce it on HSW. To actually fix it we also need the next patch. Reported-by: He, Shuang Signed-off-by: Imre Deak Reviewed-by: Paulo Zanoni Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_irq.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4eea1818b1ab..981834b0f9b6 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -231,9 +231,6 @@ static void snb_update_pm_irq(struct drm_i915_private *dev_priv, assert_spin_locked(&dev_priv->irq_lock); - if (WARN_ON(!intel_irqs_enabled(dev_priv))) - return; - new_val = dev_priv->pm_irq_mask; new_val &= ~interrupt_mask; new_val |= (~enabled_irq_mask & interrupt_mask); @@ -247,14 +244,26 @@ static void snb_update_pm_irq(struct drm_i915_private *dev_priv, void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask) { + if (WARN_ON(!intel_irqs_enabled(dev_priv))) + return; + snb_update_pm_irq(dev_priv, mask, mask); } -void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask) +static void __gen6_disable_pm_irq(struct drm_i915_private *dev_priv, + uint32_t mask) { snb_update_pm_irq(dev_priv, mask, 0); } +void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask) +{ + if (WARN_ON(!intel_irqs_enabled(dev_priv))) + return; + + __gen6_disable_pm_irq(dev_priv, mask); +} + void gen6_reset_rps_interrupts(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -289,16 +298,20 @@ void gen6_disable_rps_interrupts(struct drm_device *dev) cancel_work_sync(&dev_priv->rps.work); + spin_lock_irq(&dev_priv->irq_lock); + I915_WRITE(GEN6_PMINTRMSK, INTEL_INFO(dev_priv)->gen >= 8 ? ~GEN8_PMINTR_REDIRECT_TO_NON_DISP : ~0); + + __gen6_disable_pm_irq(dev_priv, dev_priv->pm_rps_events); I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) & ~dev_priv->pm_rps_events); + I915_WRITE(gen6_pm_iir(dev_priv), dev_priv->pm_rps_events); + I915_WRITE(gen6_pm_iir(dev_priv), dev_priv->pm_rps_events); - spin_lock_irq(&dev_priv->irq_lock); dev_priv->rps.pm_iir = 0; - spin_unlock_irq(&dev_priv->irq_lock); - I915_WRITE(gen6_pm_iir(dev_priv), dev_priv->pm_rps_events); + spin_unlock_irq(&dev_priv->irq_lock); } /** -- 2.20.1