Skip to contentSkip to conversation

ILK+ interrupt improvements, v3

PATCH In review Paulo Zanoni 4 participants 28 messages Apr 1, 2014 – Apr 3, 2014

Subsystems (2)
DRM drivers Intel i915
lore
Share & tools
lore

Paulo Zanoni

author
#0 lore raw

cover ILK+ interrupt improvements, v3

From: Paulo Zanoni <[email protected]>

Hi

This is a follow up to "ILK+ interrupt improvements, v3", which was reveiwed by Ben.

The biggest difference is that patch 20 was squashed into the previous patch, so different patches received different chunks of patch 20. This caused the need to rebase pretty much every patch of the series. There were also some small changes, like typos on commit messages, changes on comments and the removal of some assertions.

We had some discussions regarding renaming, where Ben proposed renames but Daniel disagreed with the proposals. Ben expressed he was not comfortable with giving Reviewed-by tags on some patches without the renames and I understand his point of view, so I just did not add the tags for those patches. The patches that still do not have Reviewed-by tags are: 6, 8, 11, 14, 15, 17, 18, 19.

Thanks,
Paulo

Paulo Zanoni (19):
drm/i915: add GEN5_IRQ_INIT macro
drm/i915: also use GEN5_IRQ_INIT with south display interrupts drm/i915: use GEN8_IRQ_INIT on GEN5
drm/i915: add GEN5_IRQ_FINI
drm/i915: don't forget to uninstall the PM IRQs drm/i915: properly clear IIR at irq_uninstall on Gen5+ drm/i915: add GEN5_IRQ_INIT
drm/i915: check if IIR is still zero at postinstall on Gen5+ drm/i915: fix SERR_INT init/reset code
drm/i915: fix GEN7_ERR_INT init/reset code
drm/i915: fix open coded gen5_gt_irq_preinstall drm/i915: extract ibx_irq_uninstall
drm/i915: call ibx_irq_uninstall from gen8_irq_uninstall drm/i915: enable SDEIER later
drm/i915: remove ibx_irq_uninstall
drm/i915: add missing intel_hpd_irq_uninstall drm/i915: add ironlake_irq_reset
drm/i915: add gen8_irq_reset
drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

 drivers/gpu/drm/i915/i915_irq.c | 281 ++++++++++++++++++----------------------
 1 file changed, 126 insertions(+), 155 deletions(-)
Signature · 1 line
1.8.5.3
author
#1 lore raw

PATCH 1/19 drm/i915: add GEN5_IRQ_INIT macro

From: Paulo Zanoni <[email protected]>

The goal is to reuse the GEN8 macros, but a few changes are needed, so let's make things easier to review.

I could also use these macros on older code, but since I plan to change how the interrupts are initialized, we'll risk breaking the older code in the next commits, so I'll leave this out for now.

v2: - Rebase.

Reviewed-by Ben Widawsky <[email protected]>
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +10 -14
index 2163f18..f487068 100644
@@ -80,6 +80,12 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
 	[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
 };
 
+#define GEN5_IRQ_INIT(type) do { \
+	I915_WRITE(type##IMR, 0xffffffff); \
+	I915_WRITE(type##IER, 0); \
+	POSTING_READ(type##IER); \
+} while (0)
+
 /* For display hotplug interrupt */
 static void
 ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)

@@ -2837,17 +2843,9 @@ static void gen5_gt_irq_preinstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	/* and GT */
-	I915_WRITE(GTIMR, 0xffffffff);
-	I915_WRITE(GTIER, 0x0);
-	POSTING_READ(GTIER);
-
-	if (INTEL_INFO(dev)->gen >= 6) {
-		/* and PM */
-		I915_WRITE(GEN6_PMIMR, 0xffffffff);
-		I915_WRITE(GEN6_PMIER, 0x0);
-		POSTING_READ(GEN6_PMIER);
-	}
+	GEN5_IRQ_INIT(GT);
+	if (INTEL_INFO(dev)->gen >= 6)
+		GEN5_IRQ_INIT(GEN6_PM);
 }
 
 /* drm_dma.h hooks

@@ -2858,9 +2856,7 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 
 	I915_WRITE(HWSTAM, 0xeffe);
 
-	I915_WRITE(DEIMR, 0xffffffff);
-	I915_WRITE(DEIER, 0x0);
-	POSTING_READ(DEIER);
+	GEN5_IRQ_INIT(DE);
 
 	gen5_gt_irq_preinstall(dev);
Signature · 1 line
1.8.5.3
author
#2 lore raw

PATCH 2/19 drm/i915: also use GEN5_IRQ_INIT with south display interrupts

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

This interrupt gets initialized with a different IER value, so it was not using the macro. The problem is that we plan to modify the macro to make it do additional things, and we want the SDE interrupts updated too. So let's make sure we call the macro, then, after it, we do the necessary SDE-specific changes.

Reviewed-by Ben Widawsky <[email protected]>
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +1 -2
index f487068..af1d43c 100644
@@ -2827,8 +2827,7 @@ static void ibx_irq_preinstall(struct drm_device *dev)
 	if (HAS_PCH_NOP(dev))
 		return;
 
-	/* south display irq */
-	I915_WRITE(SDEIMR, 0xffffffff);
+	GEN5_IRQ_INIT(SDE);
 	/*
 	 * SDEIER is also touched by the interrupt handler to work around missed
 	 * PCH interrupts. Hence we can't update it after the interrupt handler
Signature · 1 line
1.8.5.3
author
#3 lore raw

PATCH 3/19 drm/i915: use GEN8_IRQ_INIT on GEN5

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

And rename it to GEN5_IRQ_INIT.

We have discussed doing equivalent changes on July 2013, and I even sent a patch series for this: "[PATCH 00/15] Unify interrupt register init/reset". Now that the BDW code was merged, I have one more argument in favor of these changes.

Here's what really changes with the Gen 5 IRQ init code:
- We now clear the IIR registers at preinstall (they are also cleared at postinstall, but we will change that later).
- We have an additional POSTING_READ at the IMR register.

v2: - Fix typo in commit message.
- Add POSTING_READ calls to the macros (Ben, Daniel, Jani).

Reviewed-by Ben Widawsky <[email protected]> (v1)
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 46 +++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +19 -27
index af1d43c..bc7e230 100644
@@ -80,10 +80,25 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
 	[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
 };
 
+/* IIR can theoretically queue up two events. Be paranoid. */
+#define GEN8_IRQ_INIT_NDX(type, which) do { \
+	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
+	POSTING_READ(GEN8_##type##_IMR(which)); \
+	I915_WRITE(GEN8_##type##_IER(which), 0); \
+	I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \
+	POSTING_READ(GEN8_##type##_IIR(which)); \
+	I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \
+	POSTING_READ(GEN8_##type##_IIR(which)); \
+} while (0)
+
 #define GEN5_IRQ_INIT(type) do { \
 	I915_WRITE(type##IMR, 0xffffffff); \
+	POSTING_READ(type##IMR); \
 	I915_WRITE(type##IER, 0); \
-	POSTING_READ(type##IER); \
+	I915_WRITE(type##IIR, 0xffffffff); \
+	POSTING_READ(type##IIR); \
+	I915_WRITE(type##IIR, 0xffffffff); \
+	POSTING_READ(type##IIR); \
 } while (0)
 
 /* For display hotplug interrupt */

@@ -2899,25 +2914,6 @@ static void gen8_irq_preinstall(struct drm_device *dev)
 	I915_WRITE(GEN8_MASTER_IRQ, 0);
 	POSTING_READ(GEN8_MASTER_IRQ);
 
-	/* IIR can theoretically queue up two events. Be paranoid */
-#define GEN8_IRQ_INIT_NDX(type, which) do { \
-		I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
-		POSTING_READ(GEN8_##type##_IMR(which)); \
-		I915_WRITE(GEN8_##type##_IER(which), 0); \
-		I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \
-		POSTING_READ(GEN8_##type##_IIR(which)); \
-		I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \
-	} while (0)
-
-#define GEN8_IRQ_INIT(type) do { \
-		I915_WRITE(GEN8_##type##_IMR, 0xffffffff); \
-		POSTING_READ(GEN8_##type##_IMR); \
-		I915_WRITE(GEN8_##type##_IER, 0); \
-		I915_WRITE(GEN8_##type##_IIR, 0xffffffff); \
-		POSTING_READ(GEN8_##type##_IIR); \
-		I915_WRITE(GEN8_##type##_IIR, 0xffffffff); \
-	} while (0)
-
 	GEN8_IRQ_INIT_NDX(GT, 0);
 	GEN8_IRQ_INIT_NDX(GT, 1);
 	GEN8_IRQ_INIT_NDX(GT, 2);

@@ -2927,13 +2923,9 @@ static void gen8_irq_preinstall(struct drm_device *dev)
 		GEN8_IRQ_INIT_NDX(DE_PIPE, pipe);
 	}
 
-	GEN8_IRQ_INIT(DE_PORT);
-	GEN8_IRQ_INIT(DE_MISC);
-	GEN8_IRQ_INIT(PCU);
-#undef GEN8_IRQ_INIT
-#undef GEN8_IRQ_INIT_NDX
-
-	POSTING_READ(GEN8_PCU_IIR);
+	GEN5_IRQ_INIT(GEN8_DE_PORT_);
+	GEN5_IRQ_INIT(GEN8_DE_MISC_);
+	GEN5_IRQ_INIT(GEN8_PCU_);
 
 	ibx_irq_preinstall(dev);
 }
Signature · 1 line
1.8.5.3
author
#4 lore raw

PATCH 4/19 drm/i915: add GEN5_IRQ_FINI

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

Same as the _INIT macro: the goal is to reuse the GEN8 macros, but there are still some slight differences.

v2: - Rebase.

Reviewed-by Ben Widawsky <[email protected]>
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +9 -9
index bc7e230..26bfe1b 100644
@@ -101,6 +101,12 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
 	POSTING_READ(type##IIR); \
 } while (0)
 
+#define GEN5_IRQ_FINI(type) do { \
+	I915_WRITE(type##IMR, 0xffffffff); \
+	I915_WRITE(type##IER, 0); \
+	I915_WRITE(type##IIR, I915_READ(type##IIR)); \
+} while (0)
+
 /* For display hotplug interrupt */
 static void
 ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)

@@ -3353,22 +3359,16 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	I915_WRITE(HWSTAM, 0xffffffff);
 
-	I915_WRITE(DEIMR, 0xffffffff);
-	I915_WRITE(DEIER, 0x0);
-	I915_WRITE(DEIIR, I915_READ(DEIIR));
+	GEN5_IRQ_FINI(DE);
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
 
-	I915_WRITE(GTIMR, 0xffffffff);
-	I915_WRITE(GTIER, 0x0);
-	I915_WRITE(GTIIR, I915_READ(GTIIR));
+	GEN5_IRQ_FINI(GT);
 
 	if (HAS_PCH_NOP(dev))
 		return;
 
-	I915_WRITE(SDEIMR, 0xffffffff);
-	I915_WRITE(SDEIER, 0x0);
-	I915_WRITE(SDEIIR, I915_READ(SDEIIR));
+	GEN5_IRQ_FINI(SDE);
 	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
 		I915_WRITE(SERR_INT, I915_READ(SERR_INT));
 }
Signature · 1 line
1.8.5.3
author
#5 lore raw

PATCH 5/19 drm/i915: don't forget to uninstall the PM IRQs

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It's the only thing missing, apparently.

v2: - Fix typo (Ben).

Reviewed-by Ben Widawsky <[email protected]>
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/gpu/drm/i915/i915_irq.c +2 -0
index 26bfe1b..72bb443 100644
@@ -3364,6 +3364,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
 
 	GEN5_IRQ_FINI(GT);
+	if (INTEL_INFO(dev)->gen >= 6)
+		GEN5_IRQ_FINI(GEN6_PM);
 
 	if (HAS_PCH_NOP(dev))
 		return;
Signature · 1 line
1.8.5.3
author
#6 lore raw

PATCH 6/19 drm/i915: properly clear IIR at irq_uninstall on Gen5+

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The IRQ_INIT and IRQ_FINI macros are basically the same thing, with the exception that IRQ_FINI doesn't properly clear IIR twice and doesn't have as many POSTING_READs as IRQ_INIT. So rename the INIT macro to IRQ_RESET and use it everywhere.

v2: - Fix error in the commit message (Chris).
- Adjust to the new POSTING_READ scheme (Ben).

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 77 +++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 50 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +27 -50
index 72bb443..a2a2d51 100644
@@ -81,7 +81,7 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
 };
 
 /* IIR can theoretically queue up two events. Be paranoid. */
-#define GEN8_IRQ_INIT_NDX(type, which) do { \
+#define GEN8_IRQ_RESET_NDX(type, which) do { \
 	I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
 	POSTING_READ(GEN8_##type##_IMR(which)); \
 	I915_WRITE(GEN8_##type##_IER(which), 0); \

@@ -91,7 +91,7 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
 	POSTING_READ(GEN8_##type##_IIR(which)); \
 } while (0)
 
-#define GEN5_IRQ_INIT(type) do { \
+#define GEN5_IRQ_RESET(type) do { \
 	I915_WRITE(type##IMR, 0xffffffff); \
 	POSTING_READ(type##IMR); \
 	I915_WRITE(type##IER, 0); \

@@ -101,12 +101,6 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
 	POSTING_READ(type##IIR); \
 } while (0)
 
-#define GEN5_IRQ_FINI(type) do { \
-	I915_WRITE(type##IMR, 0xffffffff); \
-	I915_WRITE(type##IER, 0); \
-	I915_WRITE(type##IIR, I915_READ(type##IIR)); \
-} while (0)
-
 /* For display hotplug interrupt */
 static void
 ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)

@@ -2848,7 +2842,7 @@ static void ibx_irq_preinstall(struct drm_device *dev)
 	if (HAS_PCH_NOP(dev))
 		return;
 
-	GEN5_IRQ_INIT(SDE);
+	GEN5_IRQ_RESET(SDE);
 	/*
 	 * SDEIER is also touched by the interrupt handler to work around missed
 	 * PCH interrupts. Hence we can't update it after the interrupt handler

@@ -2863,9 +2857,9 @@ static void gen5_gt_irq_preinstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	GEN5_IRQ_INIT(GT);
+	GEN5_IRQ_RESET(GT);
 	if (INTEL_INFO(dev)->gen >= 6)
-		GEN5_IRQ_INIT(GEN6_PM);
+		GEN5_IRQ_RESET(GEN6_PM);
 }
 
 /* drm_dma.h hooks

@@ -2876,7 +2870,7 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 
 	I915_WRITE(HWSTAM, 0xeffe);
 
-	GEN5_IRQ_INIT(DE);
+	GEN5_IRQ_RESET(DE);
 
 	gen5_gt_irq_preinstall(dev);
 

@@ -2920,18 +2914,18 @@ static void gen8_irq_preinstall(struct drm_device *dev)
 	I915_WRITE(GEN8_MASTER_IRQ, 0);
 	POSTING_READ(GEN8_MASTER_IRQ);
 
-	GEN8_IRQ_INIT_NDX(GT, 0);
-	GEN8_IRQ_INIT_NDX(GT, 1);
-	GEN8_IRQ_INIT_NDX(GT, 2);
-	GEN8_IRQ_INIT_NDX(GT, 3);
+	GEN8_IRQ_RESET_NDX(GT, 0);
+	GEN8_IRQ_RESET_NDX(GT, 1);
+	GEN8_IRQ_RESET_NDX(GT, 2);
+	GEN8_IRQ_RESET_NDX(GT, 3);
 
 	for_each_pipe(pipe) {
-		GEN8_IRQ_INIT_NDX(DE_PIPE, pipe);
+		GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
 	}
 
-	GEN5_IRQ_INIT(GEN8_DE_PORT_);
-	GEN5_IRQ_INIT(GEN8_DE_MISC_);
-	GEN5_IRQ_INIT(GEN8_PCU_);
+	GEN5_IRQ_RESET(GEN8_DE_PORT_);
+	GEN5_IRQ_RESET(GEN8_DE_MISC_);
+	GEN5_IRQ_RESET(GEN8_PCU_);
 
 	ibx_irq_preinstall(dev);
 }

@@ -3287,34 +3281,17 @@ static void gen8_irq_uninstall(struct drm_device *dev)
 
 	I915_WRITE(GEN8_MASTER_IRQ, 0);
 
-#define GEN8_IRQ_FINI_NDX(type, which) do { \
-		I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
-		I915_WRITE(GEN8_##type##_IER(which), 0); \
-		I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \
-	} while (0)
-
-#define GEN8_IRQ_FINI(type) do { \
-		I915_WRITE(GEN8_##type##_IMR, 0xffffffff); \
-		I915_WRITE(GEN8_##type##_IER, 0); \
-		I915_WRITE(GEN8_##type##_IIR, 0xffffffff); \
-	} while (0)
+	GEN8_IRQ_RESET_NDX(GT, 0);
+	GEN8_IRQ_RESET_NDX(GT, 1);
+	GEN8_IRQ_RESET_NDX(GT, 2);
+	GEN8_IRQ_RESET_NDX(GT, 3);
 
-	GEN8_IRQ_FINI_NDX(GT, 0);
-	GEN8_IRQ_FINI_NDX(GT, 1);
-	GEN8_IRQ_FINI_NDX(GT, 2);
-	GEN8_IRQ_FINI_NDX(GT, 3);
-
-	for_each_pipe(pipe) {
-		GEN8_IRQ_FINI_NDX(DE_PIPE, pipe);
-	}
-
-	GEN8_IRQ_FINI(DE_PORT);
-	GEN8_IRQ_FINI(DE_MISC);
-	GEN8_IRQ_FINI(PCU);
-#undef GEN8_IRQ_FINI
-#undef GEN8_IRQ_FINI_NDX
+	for_each_pipe(pipe)
+		GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
 
-	POSTING_READ(GEN8_PCU_IIR);
+	GEN5_IRQ_RESET(GEN8_DE_PORT_);
+	GEN5_IRQ_RESET(GEN8_DE_MISC_);
+	GEN5_IRQ_RESET(GEN8_PCU_);
 }
 
 static void valleyview_irq_uninstall(struct drm_device *dev)

@@ -3359,18 +3336,18 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	I915_WRITE(HWSTAM, 0xffffffff);
 
-	GEN5_IRQ_FINI(DE);
+	GEN5_IRQ_RESET(DE);
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
 
-	GEN5_IRQ_FINI(GT);
+	GEN5_IRQ_RESET(GT);
 	if (INTEL_INFO(dev)->gen >= 6)
-		GEN5_IRQ_FINI(GEN6_PM);
+		GEN5_IRQ_RESET(GEN6_PM);
 
 	if (HAS_PCH_NOP(dev))
 		return;
 
-	GEN5_IRQ_FINI(SDE);
+	GEN5_IRQ_RESET(SDE);
 	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
 		I915_WRITE(SERR_INT, I915_READ(SERR_INT));
 }
Signature · 1 line
1.8.5.3
author
#7 lore raw

PATCH 7/19 drm/i915: add GEN5_IRQ_INIT

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

And the equivalent GEN8_IRQ_INIT_NDX macro. These macros are for the postinstall functions. The next patch will improve this macro.

v2: - Adjust to the new POSTING_READ scheme (Ben).

Reviewed-by Ben Widawsky <[email protected]> (v1)
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +19 -18
index a2a2d51..ef4adf6 100644
@@ -101,6 +101,18 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
 	POSTING_READ(type##IIR); \
 } while (0)
 
+#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \
+	I915_WRITE(GEN8_##type##_IMR(which), (imr_val)); \
+	I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \
+	POSTING_READ(GEN8_##type##_IER(which)); \
+} while (0)
+
+#define GEN5_IRQ_INIT(type, imr_val, ier_val) do { \
+	I915_WRITE(type##IMR, (imr_val)); \
+	I915_WRITE(type##IER, (ier_val)); \
+	POSTING_READ(type##IER); \
+} while (0)
+
 /* For display hotplug interrupt */
 static void
 ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)

@@ -3008,9 +3020,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 	}
 
 	I915_WRITE(GTIIR, I915_READ(GTIIR));
-	I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
-	I915_WRITE(GTIER, gt_irqs);
-	POSTING_READ(GTIER);
+	GEN5_IRQ_INIT(GT, dev_priv->gt_irq_mask, gt_irqs);
 
 	if (INTEL_INFO(dev)->gen >= 6) {
 		pm_irqs |= dev_priv->pm_rps_events;

@@ -3020,9 +3030,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 
 		dev_priv->pm_irq_mask = 0xffffffff;
 		I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
-		I915_WRITE(GEN6_PMIMR, dev_priv->pm_irq_mask);
-		I915_WRITE(GEN6_PMIER, pm_irqs);
-		POSTING_READ(GEN6_PMIER);
+		GEN5_IRQ_INIT(GEN6_PM, dev_priv->pm_irq_mask, pm_irqs);
 	}
 }
 

@@ -3055,9 +3063,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	/* should always can generate irq */
 	I915_WRITE(DEIIR, I915_READ(DEIIR));
-	I915_WRITE(DEIMR, dev_priv->irq_mask);
-	I915_WRITE(DEIER, display_mask | extra_mask);
-	POSTING_READ(DEIER);
+	GEN5_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);
 
 	gen5_gt_irq_postinstall(dev);
 

@@ -3222,10 +3228,8 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 		if (tmp)
 			DRM_ERROR("Interrupt (%d) should have been masked in pre-install 0x%08x\n",
 				  i, tmp);
-		I915_WRITE(GEN8_GT_IMR(i), ~gt_interrupts[i]);
-		I915_WRITE(GEN8_GT_IER(i), gt_interrupts[i]);
+		GEN8_IRQ_INIT_NDX(GT, i, ~gt_interrupts[i], gt_interrupts[i]);
 	}
-	POSTING_READ(GEN8_GT_IER(0));
 }
 
 static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)

@@ -3246,14 +3250,11 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 		if (tmp)
 			DRM_ERROR("Interrupt (%d) should have been masked in pre-install 0x%08x\n",
 				  pipe, tmp);
-		I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
-		I915_WRITE(GEN8_DE_PIPE_IER(pipe), de_pipe_enables);
+		GEN8_IRQ_INIT_NDX(DE_PIPE, pipe, dev_priv->de_irq_mask[pipe],
+				  de_pipe_enables);
 	}
-	POSTING_READ(GEN8_DE_PIPE_ISR(0));
 
-	I915_WRITE(GEN8_DE_PORT_IMR, ~GEN8_AUX_CHANNEL_A);
-	I915_WRITE(GEN8_DE_PORT_IER, GEN8_AUX_CHANNEL_A);
-	POSTING_READ(GEN8_DE_PORT_IER);
+	GEN5_IRQ_INIT(GEN8_DE_PORT_, ~GEN8_AUX_CHANNEL_A, GEN8_AUX_CHANNEL_A);
 }
 
 static int gen8_irq_postinstall(struct drm_device *dev)
Signature · 1 line
1.8.5.3
author
#8 lore raw

PATCH 8/19 drm/i915: check if IIR is still zero at postinstall on Gen5+

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

It should already be masked and disabled and zeroed at the preinstall and uninstall stages. Also, the current code just writes to IIR once, and this is not a guarantee that it will be cleared, so it's wrong anyway.

The whole reason for the paranoia is that we're going to start calling the IRQ preinstall/postinstall/uninstall from the runtime PM callbacks, so we need to make sure everything is behaving as expected.

v2: - Change the original DRM_ERROR to WARN and clear IIR in case it's
not zero (Ben).
- Improve commit message (Daniel).

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +20 -17
index ef4adf6..65e901e 100644
@@ -101,13 +101,30 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
 	POSTING_READ(type##IIR); \
 } while (0)
 
+/*
+ * We should clear IMR at preinstall/uninstall, and just check at postinstall.
+ */
+#define GEN5_ASSERT_IIR_IS_ZERO(reg) do { \
+	u32 val = I915_READ(reg); \
+	if (val) { \
+		WARN(1, "Interrupt register 0x%x is not zero: 0x%08x\n", \
+		     (reg), val); \
+		I915_WRITE((reg), 0xffffffff); \
+		POSTING_READ(reg); \
+		I915_WRITE((reg), 0xffffffff); \
+		POSTING_READ(reg); \
+	} \
+} while (0)
+
 #define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \
+	GEN5_ASSERT_IIR_IS_ZERO(GEN8_##type##_IIR(which)); \
 	I915_WRITE(GEN8_##type##_IMR(which), (imr_val)); \
 	I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \
 	POSTING_READ(GEN8_##type##_IER(which)); \
 } while (0)
 
 #define GEN5_IRQ_INIT(type, imr_val, ier_val) do { \
+	GEN5_ASSERT_IIR_IS_ZERO(type##IIR); \
 	I915_WRITE(type##IMR, (imr_val)); \
 	I915_WRITE(type##IER, (ier_val)); \
 	POSTING_READ(type##IER); \

@@ -2993,7 +3010,7 @@ static void ibx_irq_postinstall(struct drm_device *dev)
 		I915_WRITE(SERR_INT, I915_READ(SERR_INT));
 	}
 
-	I915_WRITE(SDEIIR, I915_READ(SDEIIR));
+	GEN5_ASSERT_IIR_IS_ZERO(SDEIIR);
 	I915_WRITE(SDEIMR, ~mask);
 }
 

@@ -3019,7 +3036,6 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 		gt_irqs |= GT_BLT_USER_INTERRUPT | GT_BSD_USER_INTERRUPT;
 	}
 
-	I915_WRITE(GTIIR, I915_READ(GTIIR));
 	GEN5_IRQ_INIT(GT, dev_priv->gt_irq_mask, gt_irqs);
 
 	if (INTEL_INFO(dev)->gen >= 6) {

@@ -3029,7 +3045,6 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 			pm_irqs |= PM_VEBOX_USER_INTERRUPT;
 
 		dev_priv->pm_irq_mask = 0xffffffff;
-		I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
 		GEN5_IRQ_INIT(GEN6_PM, dev_priv->pm_irq_mask, pm_irqs);
 	}
 }

@@ -3061,8 +3076,6 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	dev_priv->irq_mask = ~display_mask;
 
-	/* should always can generate irq */
-	I915_WRITE(DEIIR, I915_READ(DEIIR));
 	GEN5_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);
 
 	gen5_gt_irq_postinstall(dev);

@@ -3223,13 +3236,8 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 		GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT
 		};
 
-	for (i = 0; i < ARRAY_SIZE(gt_interrupts); i++) {
-		u32 tmp = I915_READ(GEN8_GT_IIR(i));
-		if (tmp)
-			DRM_ERROR("Interrupt (%d) should have been masked in pre-install 0x%08x\n",
-				  i, tmp);
+	for (i = 0; i < ARRAY_SIZE(gt_interrupts); i++)
 		GEN8_IRQ_INIT_NDX(GT, i, ~gt_interrupts[i], gt_interrupts[i]);
-	}
 }
 
 static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)

@@ -3245,14 +3253,9 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 	dev_priv->de_irq_mask[PIPE_B] = ~de_pipe_masked;
 	dev_priv->de_irq_mask[PIPE_C] = ~de_pipe_masked;
 
-	for_each_pipe(pipe) {
-		u32 tmp = I915_READ(GEN8_DE_PIPE_IIR(pipe));
-		if (tmp)
-			DRM_ERROR("Interrupt (%d) should have been masked in pre-install 0x%08x\n",
-				  pipe, tmp);
+	for_each_pipe(pipe)
 		GEN8_IRQ_INIT_NDX(DE_PIPE, pipe, dev_priv->de_irq_mask[pipe],
 				  de_pipe_enables);
-	}
 
 	GEN5_IRQ_INIT(GEN8_DE_PORT_, ~GEN8_AUX_CHANNEL_A, GEN8_AUX_CHANNEL_A);
 }
Signature · 1 line
1.8.5.3
author
#9 lore raw

PATCH 9/19 drm/i915: fix SERR_INT init/reset code

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The SERR_INT register is very similar to the other IIR registers, so let's zero it at preinstall/uninstall and WARN for a non-zero value at postinstall, just like we do with the other IIR registers. For this one, there's no need to double-clear since it can't store more than one interrupt.

v2: - Remove the is_zero assertion (Ben).

Reviewed-by Ben Widawsky <[email protected]>
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +7 -6
index 65e901e..bfb7e14 100644
@@ -2872,6 +2872,10 @@ static void ibx_irq_preinstall(struct drm_device *dev)
 		return;
 
 	GEN5_IRQ_RESET(SDE);
+
+	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
+		I915_WRITE(SERR_INT, 0xffffffff);
+
 	/*
 	 * SDEIER is also touched by the interrupt handler to work around missed
 	 * PCH interrupts. Hence we can't update it after the interrupt handler

@@ -3002,14 +3006,11 @@ static void ibx_irq_postinstall(struct drm_device *dev)
 	if (HAS_PCH_NOP(dev))
 		return;
 
-	if (HAS_PCH_IBX(dev)) {
+	if (HAS_PCH_IBX(dev))
 		mask = SDE_GMBUS | SDE_AUX_MASK | SDE_POISON;
-	} else {
+	else
 		mask = SDE_GMBUS_CPT | SDE_AUX_MASK_CPT;
 
-		I915_WRITE(SERR_INT, I915_READ(SERR_INT));
-	}
-
 	GEN5_ASSERT_IIR_IS_ZERO(SDEIIR);
 	I915_WRITE(SDEIMR, ~mask);
 }

@@ -3353,7 +3354,7 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	GEN5_IRQ_RESET(SDE);
 	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
-		I915_WRITE(SERR_INT, I915_READ(SERR_INT));
+		I915_WRITE(SERR_INT, 0xffffffff);
 }
 
 static void i8xx_irq_preinstall(struct drm_device * dev)
Signature · 1 line
1.8.5.3
author
#10 lore raw

PATCH 10/19 drm/i915: fix GEN7_ERR_INT init/reset code

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

Same as SERR_INT and the other IIR registers: reset on preinstall/uninstall and WARN for non-zero values at postinstall. This one also doesn't need double-clear.

v2: - Remove the is_zero assertion (Ben).

Reviewed-by Ben Widawsky <[email protected]>
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +4 -3
index bfb7e14..ace3703 100644
@@ -2905,6 +2905,9 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 
 	GEN5_IRQ_RESET(DE);
 
+	if (IS_GEN7(dev))
+		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
+
 	gen5_gt_irq_preinstall(dev);
 
 	ibx_irq_preinstall(dev);

@@ -3063,8 +3066,6 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 				DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB);
 		extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
 			      DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB);
-
-		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
 	} else {
 		display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
 				DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |

@@ -3343,7 +3344,7 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	GEN5_IRQ_RESET(DE);
 	if (IS_GEN7(dev))
-		I915_WRITE(GEN7_ERR_INT, I915_READ(GEN7_ERR_INT));
+		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
 
 	GEN5_IRQ_RESET(GT);
 	if (INTEL_INFO(dev)->gen >= 6)
Signature · 1 line
1.8.5.3
author
#11 lore raw

PATCH 11/19 drm/i915: fix open coded gen5_gt_irq_preinstall

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The duplicate was at an _uninstall function, so rename it to gen5_gt_irq_reset.

v2: - Rebase.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +4 -6
index ace3703..5e1a6d3 100644
@@ -2886,7 +2886,7 @@ static void ibx_irq_preinstall(struct drm_device *dev)
 	POSTING_READ(SDEIER);
 }
 
-static void gen5_gt_irq_preinstall(struct drm_device *dev)
+static void gen5_gt_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 

@@ -2908,7 +2908,7 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
 
-	gen5_gt_irq_preinstall(dev);
+	gen5_gt_irq_reset(dev);
 
 	ibx_irq_preinstall(dev);
 }

@@ -2928,7 +2928,7 @@ static void valleyview_irq_preinstall(struct drm_device *dev)
 	I915_WRITE(GTIIR, I915_READ(GTIIR));
 	I915_WRITE(GTIIR, I915_READ(GTIIR));
 
-	gen5_gt_irq_preinstall(dev);
+	gen5_gt_irq_reset(dev);
 
 	I915_WRITE(DPINVGTT, 0xff);
 

@@ -3346,9 +3346,7 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
 
-	GEN5_IRQ_RESET(GT);
-	if (INTEL_INFO(dev)->gen >= 6)
-		GEN5_IRQ_RESET(GEN6_PM);
+	gen5_gt_irq_reset(dev);
 
 	if (HAS_PCH_NOP(dev))
 		return;
Signature · 1 line
1.8.5.3
author
#12 lore raw

PATCH 12/19 drm/i915: extract ibx_irq_uninstall

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

Just like ibx_irq_preinstall. We'll call this from somewhere else in the next patch.

Reviewed-by Ben Widawsky <[email protected]>
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +14 -7
index 5e1a6d3..ad2c3de 100644
@@ -2904,7 +2904,6 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 	I915_WRITE(HWSTAM, 0xeffe);
 
 	GEN5_IRQ_RESET(DE);
-
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
 

@@ -3277,6 +3276,19 @@ static int gen8_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
+static void ibx_irq_uninstall(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (HAS_PCH_NOP(dev))
+		return;
+
+	GEN5_IRQ_RESET(SDE);
+
+	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
+		I915_WRITE(SERR_INT, 0xffffffff);
+}
+
 static void gen8_irq_uninstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;

@@ -3348,12 +3360,7 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	gen5_gt_irq_reset(dev);
 
-	if (HAS_PCH_NOP(dev))
-		return;
-
-	GEN5_IRQ_RESET(SDE);
-	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
-		I915_WRITE(SERR_INT, 0xffffffff);
+	ibx_irq_uninstall(dev);
 }
 
 static void i8xx_irq_preinstall(struct drm_device * dev)
Signature · 1 line
1.8.5.3
author
#13 lore raw

PATCH 13/19 drm/i915: call ibx_irq_uninstall from gen8_irq_uninstall

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

After all, we call ibx_irq_preinstall from gen8_irq_preinstall.

v2: - Rebase.

Reviewed-by Ben Widawsky <[email protected]>
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/gpu/drm/i915/i915_irq.c +2 -0
index ad2c3de..1f601ef 100644
@@ -3310,6 +3310,8 @@ static void gen8_irq_uninstall(struct drm_device *dev)
 	GEN5_IRQ_RESET(GEN8_DE_PORT_);
 	GEN5_IRQ_RESET(GEN8_DE_MISC_);
 	GEN5_IRQ_RESET(GEN8_PCU_);
+
+	ibx_irq_uninstall(dev);
 }
 
 static void valleyview_irq_uninstall(struct drm_device *dev)
Signature · 1 line
1.8.5.3
author
#14 lore raw

PATCH 14/19 drm/i915: enable SDEIER later

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

On the preinstall stage we should just disable all the interrupts, but we currently enable all the south display interrupts due to the way we touch SDEIER at the IRQ handlers (note: they are still masked and our IRQ handler is disabled). Instead of doing that, let's make the preinstall stage just disable all the south interrupts, and do the proper interrupt dance/ordering at the postinstall stage, including an assert to check if everything is behaving as expected.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +21 -6
index 1f601ef..0b03c55 100644
@@ -2875,13 +2875,24 @@ static void ibx_irq_preinstall(struct drm_device *dev)
 
 	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
 		I915_WRITE(SERR_INT, 0xffffffff);
+}
 
-	/*
-	 * SDEIER is also touched by the interrupt handler to work around missed
-	 * PCH interrupts. Hence we can't update it after the interrupt handler
-	 * is enabled - instead we unconditionally enable all PCH interrupt
-	 * sources here, but then only unmask them as needed with SDEIMR.
-	 */
+/*
+ * SDEIER is also touched by the interrupt handler to work around missed PCH
+ * interrupts. Hence we can't update it after the interrupt handler is enabled -
+ * instead we unconditionally enable all PCH interrupt sources here, but then
+ * only unmask them as needed with SDEIMR.
+ *
+ * This function needs to be called before interrupts are enabled.
+ */
+static void ibx_irq_pre_postinstall(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (HAS_PCH_NOP(dev))
+		return;
+
+	WARN_ON(I915_READ(SDEIER) != 0);
 	I915_WRITE(SDEIER, 0xffffffff);
 	POSTING_READ(SDEIER);
 }

@@ -3077,6 +3088,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	dev_priv->irq_mask = ~display_mask;
 
+	ibx_irq_pre_postinstall(dev);
+
 	GEN5_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);
 
 	gen5_gt_irq_postinstall(dev);

@@ -3265,6 +3278,8 @@ static int gen8_irq_postinstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	ibx_irq_pre_postinstall(dev);
+
 	gen8_gt_irq_postinstall(dev_priv);
 	gen8_de_irq_postinstall(dev_priv);
Signature · 1 line
1.8.5.3
author
#15 lore raw

PATCH 15/19 drm/i915: remove ibx_irq_uninstall

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

After the latest changes, ibx_irq_preinstall and ibx_irq_uninstall are the same, so remove one of the copies and rename the other to ibx_irq_reset (since we're using the "reset" name for things which are called both at preinstall and uninstall).

v2: - Rebase.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +5 -18
index 0b03c55..b69b7b2 100644
@@ -2864,7 +2864,7 @@ void i915_queue_hangcheck(struct drm_device *dev)
 		  round_jiffies_up(jiffies + DRM_I915_HANGCHECK_JIFFIES));
 }
 
-static void ibx_irq_preinstall(struct drm_device *dev)
+static void ibx_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 

@@ -2920,7 +2920,7 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 
 	gen5_gt_irq_reset(dev);
 
-	ibx_irq_preinstall(dev);
+	ibx_irq_reset(dev);
 }
 
 static void valleyview_irq_preinstall(struct drm_device *dev)

@@ -2973,7 +2973,7 @@ static void gen8_irq_preinstall(struct drm_device *dev)
 	GEN5_IRQ_RESET(GEN8_DE_MISC_);
 	GEN5_IRQ_RESET(GEN8_PCU_);
 
-	ibx_irq_preinstall(dev);
+	ibx_irq_reset(dev);
 }
 
 static void ibx_hpd_irq_setup(struct drm_device *dev)

@@ -3291,19 +3291,6 @@ static int gen8_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
-static void ibx_irq_uninstall(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (HAS_PCH_NOP(dev))
-		return;
-
-	GEN5_IRQ_RESET(SDE);
-
-	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
-		I915_WRITE(SERR_INT, 0xffffffff);
-}
-
 static void gen8_irq_uninstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;

@@ -3326,7 +3313,7 @@ static void gen8_irq_uninstall(struct drm_device *dev)
 	GEN5_IRQ_RESET(GEN8_DE_MISC_);
 	GEN5_IRQ_RESET(GEN8_PCU_);
 
-	ibx_irq_uninstall(dev);
+	ibx_irq_reset(dev);
 }
 
 static void valleyview_irq_uninstall(struct drm_device *dev)

@@ -3377,7 +3364,7 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	gen5_gt_irq_reset(dev);
 
-	ibx_irq_uninstall(dev);
+	ibx_irq_reset(dev);
 }
 
 static void i8xx_irq_preinstall(struct drm_device * dev)
Signature · 1 line
1.8.5.3
author
#16 lore raw

PATCH 16/19 drm/i915: add missing intel_hpd_irq_uninstall

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

Missing from gen8_irq_uninstall.

Reviewed-by Ben Widawsky <[email protected]>
Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/gpu/drm/i915/i915_irq.c +2 -0
index b69b7b2..ee89a25 100644
@@ -3299,6 +3299,8 @@ static void gen8_irq_uninstall(struct drm_device *dev)
 	if (!dev_priv)
 		return;
 
+	intel_hpd_irq_uninstall(dev_priv);
+
 	I915_WRITE(GEN8_MASTER_IRQ, 0);
 
 	GEN8_IRQ_RESET_NDX(GT, 0);
Signature · 1 line
1.8.5.3
author
#17 lore raw

PATCH 17/19 drm/i915: add ironlake_irq_reset

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

To merge the common code of ironlake_irq_preinstall and ironlake_irq_uninstall.

We should also probably do something about that HSWSTAM write on a later commit.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +11 -10
index ee89a25..373620c 100644
@@ -2908,12 +2908,10 @@ static void gen5_gt_irq_reset(struct drm_device *dev)
 
 /* drm_dma.h hooks
 */
-static void ironlake_irq_preinstall(struct drm_device *dev)
+static void ironlake_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	I915_WRITE(HWSTAM, 0xeffe);
-
 	GEN5_IRQ_RESET(DE);
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, 0xffffffff);

@@ -2923,6 +2921,15 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 	ibx_irq_reset(dev);
 }
 
+static void ironlake_irq_preinstall(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+
+	I915_WRITE(HWSTAM, 0xeffe);
+
+	ironlake_irq_reset(dev);
+}
+
 static void valleyview_irq_preinstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;

@@ -3360,13 +3367,7 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	I915_WRITE(HWSTAM, 0xffffffff);
 
-	GEN5_IRQ_RESET(DE);
-	if (IS_GEN7(dev))
-		I915_WRITE(GEN7_ERR_INT, 0xffffffff);
-
-	gen5_gt_irq_reset(dev);
-
-	ibx_irq_reset(dev);
+	ironlake_irq_reset(dev);
 }
 
 static void i8xx_irq_preinstall(struct drm_device * dev)
Signature · 1 line
1.8.5.3
author
#18 lore raw

PATCH 18/19 drm/i915: add gen8_irq_reset

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

So we can merge all the common code from postinstall and uninstall.

v2: - Rebase.
- While at it, remove useless { and }.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +8 -19
index 373620c..fd1ef09 100644
@@ -2959,7 +2959,7 @@ static void valleyview_irq_preinstall(struct drm_device *dev)
 	POSTING_READ(VLV_IER);
 }
 
-static void gen8_irq_preinstall(struct drm_device *dev)
+static void gen8_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe;

@@ -2972,9 +2972,8 @@ static void gen8_irq_preinstall(struct drm_device *dev)
 	GEN8_IRQ_RESET_NDX(GT, 2);
 	GEN8_IRQ_RESET_NDX(GT, 3);
 
-	for_each_pipe(pipe) {
+	for_each_pipe(pipe)
 		GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
-	}
 
 	GEN5_IRQ_RESET(GEN8_DE_PORT_);
 	GEN5_IRQ_RESET(GEN8_DE_MISC_);

@@ -2983,6 +2982,11 @@ static void gen8_irq_preinstall(struct drm_device *dev)
 	ibx_irq_reset(dev);
 }
 
+static void gen8_irq_preinstall(struct drm_device *dev)
+{
+	gen8_irq_reset(dev);
+}
+
 static void ibx_hpd_irq_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;

@@ -3301,28 +3305,13 @@ static int gen8_irq_postinstall(struct drm_device *dev)
 static void gen8_irq_uninstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int pipe;
 
 	if (!dev_priv)
 		return;
 
 	intel_hpd_irq_uninstall(dev_priv);
 
-	I915_WRITE(GEN8_MASTER_IRQ, 0);
-
-	GEN8_IRQ_RESET_NDX(GT, 0);
-	GEN8_IRQ_RESET_NDX(GT, 1);
-	GEN8_IRQ_RESET_NDX(GT, 2);
-	GEN8_IRQ_RESET_NDX(GT, 3);
-
-	for_each_pipe(pipe)
-		GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
-
-	GEN5_IRQ_RESET(GEN8_DE_PORT_);
-	GEN5_IRQ_RESET(GEN8_DE_MISC_);
-	GEN5_IRQ_RESET(GEN8_PCU_);
-
-	ibx_irq_reset(dev);
+	gen8_irq_reset(dev);
 }
 
 static void valleyview_irq_uninstall(struct drm_device *dev)
Signature · 1 line
1.8.5.3
author
#19 lore raw

PATCH 19/19 drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

In reply to Paulo Zanoni

From: Paulo Zanoni <[email protected]>

We should only enable interrupts at postinstall.

And now on ILK/SNB/IVB/HSW the irq_preinstall and irq_postinstall functions leave the hardware in the same state.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +4 -6
index fd1ef09..75f1997 100644
@@ -2912,6 +2912,8 @@ static void ironlake_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	I915_WRITE(HWSTAM, 0xffffffff);
+
 	GEN5_IRQ_RESET(DE);
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, 0xffffffff);

@@ -2923,10 +2925,6 @@ static void ironlake_irq_reset(struct drm_device *dev)
 
 static void ironlake_irq_preinstall(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-
-	I915_WRITE(HWSTAM, 0xeffe);
-
 	ironlake_irq_reset(dev);
 }
 

@@ -3099,6 +3097,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	dev_priv->irq_mask = ~display_mask;
 
+	I915_WRITE(HWSTAM, 0xeffe);
+
 	ibx_irq_pre_postinstall(dev);
 
 	GEN5_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);

@@ -3354,8 +3354,6 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	intel_hpd_irq_uninstall(dev_priv);
 
-	I915_WRITE(HWSTAM, 0xffffffff);
-
 	ironlake_irq_reset(dev);
 }
Signature · 1 line
1.8.5.3
#20 lore raw

Re: [PATCH 19/19] drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

Quoted 26 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

We should only enable interrupts at postinstall.

And now on ILK/SNB/IVB/HSW the irq_preinstall and irq_postinstall functions leave the hardware in the same state.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/i915_irq.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +2 -0
index fd1ef09..75f1997 100644
@@ -2912,6 +2912,8 @@ static void ironlake_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	I915_WRITE(HWSTAM, 0xffffffff);
+
 	GEN5_IRQ_RESET(DE);
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, 0xffffffff);

drivers/gpu/drm/i915/i915_irq.c static void ironlake_irq_reset(struct drm_device *dev) · line 2929

Quoted 9 lines
drivers/gpu/drm/i915/i915_irq.c +0 -4
@@ -2923,10 +2925,6 @@ static void ironlake_irq_reset(struct drm_device *dev)
 
 static void ironlake_irq_preinstall(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-
-	I915_WRITE(HWSTAM, 0xeffe);
-
 	ironlake_irq_reset(dev);
 }

Follow-up patch to remove this wrapper functions and just put foo_irq_reset into the preinstall hooks?

In any case entire series merged (with one tiny rebase conflict since drm_i915_private_t is now gone).
-Daniel

Quoted 26 lines
drivers/gpu/drm/i915/i915_irq.c +2 -2
@@ -3099,6 +3097,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	dev_priv->irq_mask = ~display_mask;
 
+	I915_WRITE(HWSTAM, 0xeffe);
+
 	ibx_irq_pre_postinstall(dev);
 
 	GEN5_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);

@@ -3354,8 +3354,6 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	intel_hpd_irq_uninstall(dev_priv);
 
-	I915_WRITE(HWSTAM, 0xffffffff);
-
 	ironlake_irq_reset(dev);
 }
Signature · 6 lines
1.8.5.3

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
#21 lore raw

Re: [PATCH 19/19] drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

In reply to Paulo Zanoni
Quoted 8 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

We should only enable interrupts at postinstall.

And now on ILK/SNB/IVB/HSW the irq_preinstall and irq_postinstall functions leave the hardware in the same state.

Signed-off-by Paulo Zanoni <[email protected]>

Orthogonal note to this patch, I'm wondering why we enable any interrupt reporting in the HW status page, I don't see anywhere we read that information back? Any idea?

Signature · 1 line
Damien
Quoted 53 lines
---
 drivers/gpu/drm/i915/i915_irq.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/i915_irq.c +4 -6
index fd1ef09..75f1997 100644
@@ -2912,6 +2912,8 @@ static void ironlake_irq_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	I915_WRITE(HWSTAM, 0xffffffff);
+
 	GEN5_IRQ_RESET(DE);
 	if (IS_GEN7(dev))
 		I915_WRITE(GEN7_ERR_INT, 0xffffffff);

@@ -2923,10 +2925,6 @@ static void ironlake_irq_reset(struct drm_device *dev)
 
 static void ironlake_irq_preinstall(struct drm_device *dev)
 {
-	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-
-	I915_WRITE(HWSTAM, 0xeffe);
-
 	ironlake_irq_reset(dev);
 }
 

@@ -3099,6 +3097,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	dev_priv->irq_mask = ~display_mask;
 
+	I915_WRITE(HWSTAM, 0xeffe);
+
 	ibx_irq_pre_postinstall(dev);
 
 	GEN5_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);

@@ -3354,8 +3354,6 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 
 	intel_hpd_irq_uninstall(dev_priv);
 
-	I915_WRITE(HWSTAM, 0xffffffff);
-
 	ironlake_irq_reset(dev);
 }
Signature · 6 lines
1.8.5.3

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#22 lore raw

Re: [PATCH 19/19] drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

Quoted 5 lines · Damien Lespiau
Quoted 4 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

We should only enable interrupts at postinstall.

Quoted 8 lines
Quoted 4 lines

And now on ILK/SNB/IVB/HSW the irq_preinstall and irq_postinstall functions leave the hardware in the same state.

Signed-off-by Paulo Zanoni <[email protected]>

Orthogonal note to this patch, I'm wondering why we enable any interrupt reporting in the HW status page, I don't see anywhere we read that information back? Any idea?

Back in the SNB days, interrupts broke without this piece of magic. -Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
#23 lore raw

Re: [PATCH 19/19] drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

Quoted 9 lines · Chris Wilson
Quoted 8 lines · Damien Lespiau
Quoted 7 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

We should only enable interrupts at postinstall.

And now on ILK/SNB/IVB/HSW the irq_preinstall and irq_postinstall functions leave the hardware in the same state.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Signed-off-by Paulo Zanoni <[email protected]>

Orthogonal note to this patch, I'm wondering why we enable any interrupt reporting in the HW status page, I don't see anywhere we read that information back? Any idea?

Back in the SNB days, interrupts broke without this piece of magic.

To be more precise, iirc, it was a step towards getting coherent breadcrumb writes into the HWS. As it turns out, there were more steps required. Considering that it can now probably be dropped again? Any takers?
-Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
author
#24 lore raw

Re: [PATCH 19/19] drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

2014-04-02 8:27 GMT-03:00 Chris Wilson <[email protected]>:

Quoted 16 lines · Chris Wilson
Quoted 15 lines · Damien Lespiau
Quoted 13 lines · Paulo Zanoni
Quoted 8 lines

From: Paulo Zanoni <[email protected]>

We should only enable interrupts at postinstall.

And now on ILK/SNB/IVB/HSW the irq_preinstall and irq_postinstall functions leave the hardware in the same state.

Signed-off-by Paulo Zanoni <[email protected]>

Orthogonal note to this patch, I'm wondering why we enable any interrupt reporting in the HW status page, I don't see anywhere we read that information back? Any idea?

Quoted 6 lines
Quoted 1 line

Back in the SNB days, interrupts broke without this piece of magic.

To be more precise, iirc, it was a step towards getting coherent breadcrumb writes into the HWS. As it turns out, there were more steps required. Considering that it can now probably be dropped again? Any takers?

I noticed the HWSTAM code looks weird, but I don't really know much about why it's there, so I decided to not add any regressions. I wrote this patch a long time ago, and gave up on upstreaming it, but in case anybody wants, I can send it to the list:
http://cgit.freedesktop.org/~pzanoni/linux/commit/?h=c8-wip&id=7ebe245a2c55379ddc7b36f1fb440215c23f1570
. Look at the commit message, it may be an interesting starting point if anybody wants to dig on the issue...

Quoted 4 lines

-Chris

--
Chris Wilson, Intel Open Source Technology Centre

Signature · 1 line
Paulo Zanoni
#25 lore raw

Re: [PATCH 19/19] drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

Quoted 24 lines · Paulo Zanoni

2014-04-02 8:27 GMT-03:00 Chris Wilson <[email protected]>:

Quoted 16 lines · Chris Wilson
Quoted 15 lines · Damien Lespiau
Quoted 13 lines · Paulo Zanoni
Quoted 8 lines

From: Paulo Zanoni <[email protected]>

We should only enable interrupts at postinstall.

And now on ILK/SNB/IVB/HSW the irq_preinstall and irq_postinstall functions leave the hardware in the same state.

Signed-off-by Paulo Zanoni <[email protected]>

Orthogonal note to this patch, I'm wondering why we enable any interrupt reporting in the HW status page, I don't see anywhere we read that information back? Any idea?

Quoted 6 lines
Quoted 1 line

Back in the SNB days, interrupts broke without this piece of magic.

To be more precise, iirc, it was a step towards getting coherent breadcrumb writes into the HWS. As it turns out, there were more steps required. Considering that it can now probably be dropped again? Any takers?

Quoted 7 lines

I noticed the HWSTAM code looks weird, but I don't really know much about why it's there, so I decided to not add any regressions. I wrote this patch a long time ago, and gave up on upstreaming it, but in case anybody wants, I can send it to the list:
http://cgit.freedesktop.org/~pzanoni/linux/commit/?h=c8-wip&id=7ebe245a2c55379ddc7b36f1fb440215c23f1570
. Look at the commit message, it may be an interesting starting point if anybody wants to dig on the issue...

Yup, that is very interesting. That may be the minimal piece of HWSTAM magic we need, and be an interim step to removing HWSTAM entirely. -Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
#26 lore raw

Re: [PATCH 19/19] drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

Quoted 33 lines · Chris Wilson
Quoted 24 lines · Paulo Zanoni

2014-04-02 8:27 GMT-03:00 Chris Wilson <[email protected]>:

Quoted 16 lines · Chris Wilson
Quoted 15 lines · Damien Lespiau
Quoted 13 lines · Paulo Zanoni
Quoted 8 lines

From: Paulo Zanoni <[email protected]>

We should only enable interrupts at postinstall.

And now on ILK/SNB/IVB/HSW the irq_preinstall and irq_postinstall functions leave the hardware in the same state.

Signed-off-by Paulo Zanoni <[email protected]>

Orthogonal note to this patch, I'm wondering why we enable any interrupt reporting in the HW status page, I don't see anywhere we read that information back? Any idea?

Quoted 6 lines
Quoted 1 line

Back in the SNB days, interrupts broke without this piece of magic.

To be more precise, iirc, it was a step towards getting coherent breadcrumb writes into the HWS. As it turns out, there were more steps required. Considering that it can now probably be dropped again? Any takers?

Quoted 7 lines

I noticed the HWSTAM code looks weird, but I don't really know much about why it's there, so I decided to not add any regressions. I wrote this patch a long time ago, and gave up on upstreaming it, but in case anybody wants, I can send it to the list:
http://cgit.freedesktop.org/~pzanoni/linux/commit/?h=c8-wip&id=7ebe245a2c55379ddc7b36f1fb440215c23f1570
. Look at the commit message, it may be an interesting starting point if anybody wants to dig on the issue...

Quoted 2 lines

Yup, that is very interesting. That may be the minimal piece of HWSTAM magic we need, and be an interim step to removing HWSTAM entirely.

Would you mind resending:
- a patch with Chris' comment above, that's alone is useful for the next guy looking at the code,
- a patch with the change (Maybe reusing GT_RENDER_USER_INTERRUPT as HWSTAM uses the same definitions as the other interrupts registers)?

I'm sure we can get some reviews on the first, hopefully some testing on the second.

Signature · 1 line
Damien
#27 lore raw

Re: [PATCH 19/19] drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

Quoted 37 lines · Damien Lespiau
Quoted 33 lines · Chris Wilson
Quoted 24 lines · Paulo Zanoni

2014-04-02 8:27 GMT-03:00 Chris Wilson <[email protected]>:

Quoted 16 lines · Chris Wilson
Quoted 15 lines · Damien Lespiau
Quoted 13 lines · Paulo Zanoni
Quoted 8 lines

From: Paulo Zanoni <[email protected]>

We should only enable interrupts at postinstall.

And now on ILK/SNB/IVB/HSW the irq_preinstall and irq_postinstall functions leave the hardware in the same state.

Signed-off-by Paulo Zanoni <[email protected]>

Orthogonal note to this patch, I'm wondering why we enable any interrupt reporting in the HW status page, I don't see anywhere we read that information back? Any idea?

Quoted 6 lines
Quoted 1 line

Back in the SNB days, interrupts broke without this piece of magic.

To be more precise, iirc, it was a step towards getting coherent breadcrumb writes into the HWS. As it turns out, there were more steps required. Considering that it can now probably be dropped again? Any takers?

Quoted 7 lines

I noticed the HWSTAM code looks weird, but I don't really know much about why it's there, so I decided to not add any regressions. I wrote this patch a long time ago, and gave up on upstreaming it, but in case anybody wants, I can send it to the list:
http://cgit.freedesktop.org/~pzanoni/linux/commit/?h=c8-wip&id=7ebe245a2c55379ddc7b36f1fb440215c23f1570
. Look at the commit message, it may be an interesting starting point if anybody wants to dig on the issue...

Quoted 2 lines

Yup, that is very interesting. That may be the minimal piece of HWSTAM magic we need, and be an interim step to removing HWSTAM entirely.

Quoted 8 lines

Would you mind resending:
- a patch with Chris' comment above, that's alone is useful for the next guy looking at the code,
- a patch with the change (Maybe reusing GT_RENDER_USER_INTERRUPT as HWSTAM uses the same definitions as the other interrupts registers)?

I'm sure we can get some reviews on the first, hopefully some testing on the second.

Seconded. Documenting what we're doing is always good ;-) -Daniel

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch