diff mbox series

[7/7] drm/i915: Use Engine1 instance for gen11 pm interrupts

Message ID 20190409161310.20382-7-mika.kuoppala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/7] drm/i915: Use dedicated rc6 enabling sequence for gen11 | expand

Commit Message

Mika Kuoppala April 9, 2019, 4:13 p.m. UTC 
With gen11 the interrupt registers are shared between 2 engines,
with Engine1 instance being upper word and Engine0 instance being
lower. Annoyingly gen11 selected the pm interrupts to be in the
Engine1 instance.

Rectify the situation by shifting the access accordingly,
based on gen.

Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=108059
Testcase: igt/i915_pm_rps@min-max-config-loaded
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 50 +++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 18 deletions(-)

Comments

Chris Wilson April 9, 2019, 4:26 p.m. UTC | #1 
Quoting Mika Kuoppala (2019-04-09 17:13:10)
> With gen11 the interrupt registers are shared between 2 engines,
> with Engine1 instance being upper word and Engine0 instance being
> lower. Annoyingly gen11 selected the pm interrupts to be in the
> Engine1 instance.

Sounds weird, but I can't fault the solution. The choice would either to
have been shift pm_rps_events andadd gen11_pm_imr/_ier, so this patch
looks to be the smaller delta.
-Chris
Mika Kuoppala April 10, 2019, 8:20 a.m. UTC | #2 
Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-04-09 17:13:10)
>> With gen11 the interrupt registers are shared between 2 engines,
>> with Engine1 instance being upper word and Engine0 instance being
>> lower. Annoyingly gen11 selected the pm interrupts to be in the
>> Engine1 instance.
>
> Sounds weird, but I can't fault the solution. The choice would either to
> have been shift pm_rps_events andadd gen11_pm_imr/_ier, so this patch
> looks to be the smaller delta.

Small wart added is one extra posting read on disabling.

I didn't like the under the hood shifting but the much
smaller delta was too tempting.

Perhaps a comment to the register definitions
would be warranted at minimum.

-Mika
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 619e6ab273e7..be501e069b60 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -369,24 +369,39 @@  static i915_reg_t gen6_pm_iir(struct drm_i915_private *dev_priv)
 	return INTEL_GEN(dev_priv) >= 8 ? GEN8_GT_IIR(2) : GEN6_PMIIR;
 }
 
-static i915_reg_t gen6_pm_imr(struct drm_i915_private *dev_priv)
+static void write_pm_imr(struct drm_i915_private *dev_priv)
 {
-	if (INTEL_GEN(dev_priv) >= 11)
-		return GEN11_GPM_WGBOXPERF_INTR_MASK;
-	else if (INTEL_GEN(dev_priv) >= 8)
-		return GEN8_GT_IMR(2);
-	else
-		return GEN6_PMIMR;
+	i915_reg_t reg;
+	u32 mask = dev_priv->pm_imr;
+
+	if (INTEL_GEN(dev_priv) >= 11) {
+		reg = GEN11_GPM_WGBOXPERF_INTR_MASK;
+		mask = mask << 16;
+	} else if (INTEL_GEN(dev_priv) >= 8) {
+		reg = GEN8_GT_IMR(2);
+	} else {
+		reg = GEN6_PMIMR;
+	}
+
+	I915_WRITE(reg, mask);
+	POSTING_READ(reg);
 }
 
-static i915_reg_t gen6_pm_ier(struct drm_i915_private *dev_priv)
+static void write_pm_ier(struct drm_i915_private *dev_priv)
 {
-	if (INTEL_GEN(dev_priv) >= 11)
-		return GEN11_GPM_WGBOXPERF_INTR_ENABLE;
-	else if (INTEL_GEN(dev_priv) >= 8)
-		return GEN8_GT_IER(2);
-	else
-		return GEN6_PMIER;
+	i915_reg_t reg;
+	u32 mask = dev_priv->pm_ier;
+
+	if (INTEL_GEN(dev_priv) >= 11) {
+		reg = GEN11_GPM_WGBOXPERF_INTR_ENABLE;
+		mask = mask << 16;
+	} else if (INTEL_GEN(dev_priv) >= 8) {
+		reg = GEN8_GT_IER(2);
+	} else {
+		reg = GEN6_PMIER;
+	}
+
+	I915_WRITE(reg, mask);
 }
 
 /**
@@ -411,8 +426,7 @@  static void snb_update_pm_irq(struct drm_i915_private *dev_priv,
 
 	if (new_val != dev_priv->pm_imr) {
 		dev_priv->pm_imr = new_val;
-		I915_WRITE(gen6_pm_imr(dev_priv), dev_priv->pm_imr);
-		POSTING_READ(gen6_pm_imr(dev_priv));
+		write_pm_imr(dev_priv);
 	}
 }
 
@@ -453,7 +467,7 @@  static void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, u32 enable_mas
 	lockdep_assert_held(&dev_priv->irq_lock);
 
 	dev_priv->pm_ier |= enable_mask;
-	I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier);
+	write_pm_ier(dev_priv);
 	gen6_unmask_pm_irq(dev_priv, enable_mask);
 	/* unmask_pm_irq provides an implicit barrier (POSTING_READ) */
 }
@@ -464,7 +478,7 @@  static void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 disable_m
 
 	dev_priv->pm_ier &= ~disable_mask;
 	__gen6_mask_pm_irq(dev_priv, disable_mask);
-	I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier);
+	write_pm_ier(dev_priv);
 	/* though a barrier is missing here, but don't really need a one */
 }