* [PATCH 0/3] IRQ initialization debloat and conversion to uncore
@ 2019-04-09 0:37 Paulo Zanoni
2019-04-09 0:37 ` [PATCH 1/3] drm/i915: refactor the IRQ init/reset macros Paulo Zanoni
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Paulo Zanoni @ 2019-04-09 0:37 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
The first patch is a simple refactor to try to debloat our IRQ
initialization and the second is a tiny conversion to the new
intel_uncore model. I'm not sure how much we want patch 3 right now,
but my understanding is that we want to move in that direction anyway,
so why not now.
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Paulo Zanoni (3):
drm/i915: refactor the IRQ init/reset macros
drm/i915: convert the IRQ initialization functions to intel_uncore
drm/i915: fully convert the IRQ initialization macros to intel_uncore
drivers/gpu/drm/i915/i915_irq.c | 275 +++++++++++++++++++-------------
1 file changed, 165 insertions(+), 110 deletions(-)
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] drm/i915: refactor the IRQ init/reset macros
2019-04-09 0:37 [PATCH 0/3] IRQ initialization debloat and conversion to uncore Paulo Zanoni
@ 2019-04-09 0:37 ` Paulo Zanoni
2019-04-09 18:10 ` Ville Syrjälä
2019-04-09 0:37 ` [PATCH 2/3] drm/i915: convert the IRQ initialization functions to intel_uncore Paulo Zanoni
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Paulo Zanoni @ 2019-04-09 0:37 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
The whole point of having macros here is for the token pasting
necessary to automatically have IMR, IIR and IER selected. We don't
really need or want all the inlining that happens as a consequence.
The good thing about the current code is that it works regardless of
the relative offsets between these registers (they change after gen3).
One thing which we can do is to split the logic of what we do with
imr/ier/iir to functions separate from the macros that pick them.
That's what we do in this commit. This allows us to get rid of the
gen8 duplicates and also all the inlining:
add/remove: 2/0 grow/shrink: 0/21 up/down: 383/-6006 (-5623)
Function old new delta
gen3_irq_reset - 233 +233
gen3_irq_init - 150 +150
i8xx_irq_postinstall 459 442 -17
gen11_irq_postinstall 804 744 -60
ironlake_irq_postinstall 450 353 -97
vlv_display_irq_postinstall 348 245 -103
i965_irq_postinstall 378 275 -103
i915_irq_postinstall 333 228 -105
gen8_irq_power_well_post_enable 374 210 -164
ironlake_irq_reset 397 218 -179
vlv_display_irq_reset 616 433 -183
i965_irq_reset 374 180 -194
cherryview_irq_reset 379 185 -194
i915_irq_reset 407 209 -198
ibx_irq_reset 332 133 -199
gen5_gt_irq_postinstall 533 332 -201
gen8_irq_power_well_pre_disable 434 204 -230
gen8_gt_irq_postinstall 469 196 -273
gen8_de_irq_postinstall 1200 805 -395
gen5_gt_irq_reset 471 76 -395
gen8_gt_irq_reset 775 99 -676
gen8_irq_reset 1100 333 -767
gen11_irq_reset 1959 686 -1273
Total: Before=2262051, After=2256428, chg -0.25%
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++-------------
1 file changed, 73 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6454ddc37f8b..a1e7944fb5d0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -136,36 +136,45 @@ static const u32 hpd_icp[HPD_NUM_PINS] = {
[HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP
};
-/* IIR can theoretically queue up two events. Be paranoid. */
-#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); \
- 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 GEN3_IRQ_RESET(type) do { \
- I915_WRITE(type##IMR, 0xffffffff); \
- POSTING_READ(type##IMR); \
- I915_WRITE(type##IER, 0); \
- I915_WRITE(type##IIR, 0xffffffff); \
- POSTING_READ(type##IIR); \
- I915_WRITE(type##IIR, 0xffffffff); \
- POSTING_READ(type##IIR); \
-} while (0)
-
-#define GEN2_IRQ_RESET(type) do { \
- I915_WRITE16(type##IMR, 0xffff); \
- POSTING_READ16(type##IMR); \
- I915_WRITE16(type##IER, 0); \
- I915_WRITE16(type##IIR, 0xffff); \
- POSTING_READ16(type##IIR); \
- I915_WRITE16(type##IIR, 0xffff); \
- POSTING_READ16(type##IIR); \
-} while (0)
+static void gen3_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr,
+ i915_reg_t iir, i915_reg_t ier)
+{
+ I915_WRITE(imr, 0xffffffff);
+ POSTING_READ(imr);
+
+ I915_WRITE(ier, 0);
+
+ /* IIR can theoretically queue up two events. Be paranoid. */
+ I915_WRITE(iir, 0xffffffff);
+ POSTING_READ(iir);
+ I915_WRITE(iir, 0xffffffff);
+ POSTING_READ(iir);
+}
+
+static void gen2_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr,
+ i915_reg_t iir, i915_reg_t ier)
+{
+ I915_WRITE16(imr, 0xffff);
+ POSTING_READ16(imr);
+
+ I915_WRITE16(ier, 0);
+
+ /* IIR can theoretically queue up two events. Be paranoid. */
+ I915_WRITE16(iir, 0xffff);
+ POSTING_READ16(iir);
+ I915_WRITE16(iir, 0xffff);
+ POSTING_READ16(iir);
+}
+
+#define GEN8_IRQ_RESET_NDX(type, which) \
+ gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \
+ GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
+
+#define GEN3_IRQ_RESET(type) \
+ gen3_irq_reset(dev_priv, type##IMR, type##IIR, type##IER)
+
+#define GEN2_IRQ_RESET(type) \
+ gen2_irq_reset(dev_priv, type##IMR, type##IIR, type##IER)
/*
* We should clear IMR at preinstall/uninstall, and just check at postinstall.
@@ -202,26 +211,40 @@ static void gen2_assert_iir_is_zero(struct drm_i915_private *dev_priv,
POSTING_READ16(reg);
}
-#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \
- gen3_assert_iir_is_zero(dev_priv, GEN8_##type##_IIR(which)); \
- I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \
- I915_WRITE(GEN8_##type##_IMR(which), (imr_val)); \
- POSTING_READ(GEN8_##type##_IMR(which)); \
-} while (0)
-
-#define GEN3_IRQ_INIT(type, imr_val, ier_val) do { \
- gen3_assert_iir_is_zero(dev_priv, type##IIR); \
- I915_WRITE(type##IER, (ier_val)); \
- I915_WRITE(type##IMR, (imr_val)); \
- POSTING_READ(type##IMR); \
-} while (0)
-
-#define GEN2_IRQ_INIT(type, imr_val, ier_val) do { \
- gen2_assert_iir_is_zero(dev_priv, type##IIR); \
- I915_WRITE16(type##IER, (ier_val)); \
- I915_WRITE16(type##IMR, (imr_val)); \
- POSTING_READ16(type##IMR); \
-} while (0)
+static void gen3_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr,
+ i915_reg_t iir, i915_reg_t ier, u32 imr_val,
+ u32 ier_val)
+{
+ gen3_assert_iir_is_zero(dev_priv, iir);
+
+ I915_WRITE(ier, ier_val);
+ I915_WRITE(imr, imr_val);
+ POSTING_READ(imr);
+}
+
+static void gen2_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr,
+ i915_reg_t iir, i915_reg_t ier, u32 imr_val,
+ u32 ier_val)
+{
+ gen2_assert_iir_is_zero(dev_priv, iir);
+
+ I915_WRITE16(ier, ier_val);
+ I915_WRITE16(imr, imr_val);
+ POSTING_READ16(imr);
+}
+
+#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
+ gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \
+ GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
+ imr_val, ier_val)
+
+#define GEN3_IRQ_INIT(type, imr_val, ier_val) \
+ gen3_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \
+ ier_val)
+
+#define GEN2_IRQ_INIT(type, imr_val, ier_val) \
+ gen2_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \
+ ier_val)
static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] drm/i915: convert the IRQ initialization functions to intel_uncore
2019-04-09 0:37 [PATCH 0/3] IRQ initialization debloat and conversion to uncore Paulo Zanoni
2019-04-09 0:37 ` [PATCH 1/3] drm/i915: refactor the IRQ init/reset macros Paulo Zanoni
@ 2019-04-09 0:37 ` Paulo Zanoni
2019-04-09 0:37 ` [PATCH 3/3] drm/i915: fully convert the IRQ initialization macros " Paulo Zanoni
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Paulo Zanoni @ 2019-04-09 0:37 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
The IRQ initialization helpers are simple and self-contained. Continue
the transition started in the recent uncore rework to get us rid of
I915_READ/WRITE and the implicit dev_priv variables.
While the implicit dev_priv is removed from the IRQ initialization
helpers, we didn't get rid of them in the macro callers. Doing that
should be very simple now.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 100 ++++++++++++++++----------------
1 file changed, 50 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a1e7944fb5d0..99a6527568cf 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -136,115 +136,115 @@ static const u32 hpd_icp[HPD_NUM_PINS] = {
[HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP
};
-static void gen3_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr,
+static void gen3_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
i915_reg_t iir, i915_reg_t ier)
{
- I915_WRITE(imr, 0xffffffff);
- POSTING_READ(imr);
+ intel_uncore_write(uncore, imr, 0xffffffff);
+ intel_uncore_posting_read(uncore, imr);
- I915_WRITE(ier, 0);
+ intel_uncore_write(uncore, ier, 0);
/* IIR can theoretically queue up two events. Be paranoid. */
- I915_WRITE(iir, 0xffffffff);
- POSTING_READ(iir);
- I915_WRITE(iir, 0xffffffff);
- POSTING_READ(iir);
+ intel_uncore_write(uncore, iir, 0xffffffff);
+ intel_uncore_posting_read(uncore, iir);
+ intel_uncore_write(uncore, iir, 0xffffffff);
+ intel_uncore_posting_read(uncore, iir);
}
-static void gen2_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr,
+static void gen2_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
i915_reg_t iir, i915_reg_t ier)
{
- I915_WRITE16(imr, 0xffff);
- POSTING_READ16(imr);
+ intel_uncore_write16(uncore, imr, 0xffff);
+ intel_uncore_posting_read16(uncore, imr);
- I915_WRITE16(ier, 0);
+ intel_uncore_write16(uncore, ier, 0);
/* IIR can theoretically queue up two events. Be paranoid. */
- I915_WRITE16(iir, 0xffff);
- POSTING_READ16(iir);
- I915_WRITE16(iir, 0xffff);
- POSTING_READ16(iir);
+ intel_uncore_write16(uncore, iir, 0xffff);
+ intel_uncore_posting_read16(uncore, iir);
+ intel_uncore_write16(uncore, iir, 0xffff);
+ intel_uncore_posting_read16(uncore, iir);
}
#define GEN8_IRQ_RESET_NDX(type, which) \
- gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \
+ gen3_irq_reset(&dev_priv->uncore, GEN8_##type##_IMR(which), \
GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
#define GEN3_IRQ_RESET(type) \
- gen3_irq_reset(dev_priv, type##IMR, type##IIR, type##IER)
+ gen3_irq_reset(&dev_priv->uncore, type##IMR, type##IIR, type##IER)
#define GEN2_IRQ_RESET(type) \
- gen2_irq_reset(dev_priv, type##IMR, type##IIR, type##IER)
+ gen2_irq_reset(&dev_priv->uncore, type##IMR, type##IIR, type##IER)
/*
* We should clear IMR at preinstall/uninstall, and just check at postinstall.
*/
-static void gen3_assert_iir_is_zero(struct drm_i915_private *dev_priv,
+static void gen3_assert_iir_is_zero(struct intel_uncore *uncore,
i915_reg_t reg)
{
- u32 val = I915_READ(reg);
+ u32 val = intel_uncore_read(uncore, reg);
if (val == 0)
return;
WARN(1, "Interrupt register 0x%x is not zero: 0x%08x\n",
i915_mmio_reg_offset(reg), val);
- I915_WRITE(reg, 0xffffffff);
- POSTING_READ(reg);
- I915_WRITE(reg, 0xffffffff);
- POSTING_READ(reg);
+ intel_uncore_write(uncore, reg, 0xffffffff);
+ intel_uncore_posting_read(uncore, reg);
+ intel_uncore_write(uncore, reg, 0xffffffff);
+ intel_uncore_posting_read(uncore, reg);
}
-static void gen2_assert_iir_is_zero(struct drm_i915_private *dev_priv,
+static void gen2_assert_iir_is_zero(struct intel_uncore *uncore,
i915_reg_t reg)
{
- u16 val = I915_READ16(reg);
+ u16 val = intel_uncore_read16(uncore, reg);
if (val == 0)
return;
WARN(1, "Interrupt register 0x%x is not zero: 0x%08x\n",
i915_mmio_reg_offset(reg), val);
- I915_WRITE16(reg, 0xffff);
- POSTING_READ16(reg);
- I915_WRITE16(reg, 0xffff);
- POSTING_READ16(reg);
+ intel_uncore_write16(uncore, reg, 0xffff);
+ intel_uncore_posting_read16(uncore, reg);
+ intel_uncore_write16(uncore, reg, 0xffff);
+ intel_uncore_posting_read16(uncore, reg);
}
-static void gen3_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr,
+static void gen3_irq_init(struct intel_uncore *uncore, i915_reg_t imr,
i915_reg_t iir, i915_reg_t ier, u32 imr_val,
u32 ier_val)
{
- gen3_assert_iir_is_zero(dev_priv, iir);
+ gen3_assert_iir_is_zero(uncore, iir);
- I915_WRITE(ier, ier_val);
- I915_WRITE(imr, imr_val);
- POSTING_READ(imr);
+ intel_uncore_write(uncore, ier, ier_val);
+ intel_uncore_write(uncore, imr, imr_val);
+ intel_uncore_posting_read(uncore, imr);
}
-static void gen2_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr,
+static void gen2_irq_init(struct intel_uncore *uncore, i915_reg_t imr,
i915_reg_t iir, i915_reg_t ier, u32 imr_val,
u32 ier_val)
{
- gen2_assert_iir_is_zero(dev_priv, iir);
+ gen2_assert_iir_is_zero(uncore, iir);
- I915_WRITE16(ier, ier_val);
- I915_WRITE16(imr, imr_val);
- POSTING_READ16(imr);
+ intel_uncore_write16(uncore, ier, ier_val);
+ intel_uncore_write16(uncore, imr, imr_val);
+ intel_uncore_posting_read16(uncore, imr);
}
#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
- gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \
+ gen3_irq_init(&dev_priv->uncore, GEN8_##type##_IMR(which), \
GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
imr_val, ier_val)
#define GEN3_IRQ_INIT(type, imr_val, ier_val) \
- gen3_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \
- ier_val)
+ gen3_irq_init(&dev_priv->uncore, type##IMR, type##IIR, type##IER, \
+ imr_val, ier_val)
#define GEN2_IRQ_INIT(type, imr_val, ier_val) \
- gen2_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \
- ier_val)
+ gen2_irq_init(&dev_priv->uncore, type##IMR, type##IIR, type##IER, \
+ imr_val, ier_val)
static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
@@ -3849,7 +3849,7 @@ static void ibx_irq_postinstall(struct drm_device *dev)
else
mask = SDE_GMBUS_CPT;
- gen3_assert_iir_is_zero(dev_priv, SDEIIR);
+ gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR);
I915_WRITE(SDEIMR, ~mask);
if (HAS_PCH_IBX(dev_priv) || HAS_PCH_CPT(dev_priv) ||
@@ -3918,7 +3918,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
}
if (IS_HASWELL(dev_priv)) {
- gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
+ gen3_assert_iir_is_zero(&dev_priv->uncore, EDP_PSR_IIR);
intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
display_mask |= DE_EDP_PSR_INT_HSW;
}
@@ -4064,7 +4064,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
else if (IS_BROADWELL(dev_priv))
de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
- gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
+ gen3_assert_iir_is_zero(&dev_priv->uncore, EDP_PSR_IIR);
intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
for_each_pipe(dev_priv, pipe) {
@@ -4148,7 +4148,7 @@ static void icp_irq_postinstall(struct drm_device *dev)
I915_WRITE(SDEIER, 0xffffffff);
POSTING_READ(SDEIER);
- gen3_assert_iir_is_zero(dev_priv, SDEIIR);
+ gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR);
I915_WRITE(SDEIMR, ~mask);
icp_hpd_detection_setup(dev_priv);
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] drm/i915: fully convert the IRQ initialization macros to intel_uncore
2019-04-09 0:37 [PATCH 0/3] IRQ initialization debloat and conversion to uncore Paulo Zanoni
2019-04-09 0:37 ` [PATCH 1/3] drm/i915: refactor the IRQ init/reset macros Paulo Zanoni
2019-04-09 0:37 ` [PATCH 2/3] drm/i915: convert the IRQ initialization functions to intel_uncore Paulo Zanoni
@ 2019-04-09 0:37 ` Paulo Zanoni
2019-04-09 20:21 ` Ville Syrjälä
2019-04-09 0:44 ` ✗ Fi.CI.CHECKPATCH: warning for IRQ initialization debloat and conversion to uncore Patchwork
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Paulo Zanoni @ 2019-04-09 0:37 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Make them take the uncore argument from the caller instead of passing
the implicit &dev_priv->uncore directly. This will allow us to finally
pass something that's not dev_priv->uncore in the future, and gets rid
of the implicit variables in register macros.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 144 +++++++++++++++++++-------------
1 file changed, 88 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 99a6527568cf..b6361bab1086 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -166,15 +166,15 @@ static void gen2_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
intel_uncore_posting_read16(uncore, iir);
}
-#define GEN8_IRQ_RESET_NDX(type, which) \
- gen3_irq_reset(&dev_priv->uncore, GEN8_##type##_IMR(which), \
+#define GEN8_IRQ_RESET_NDX(uncore, type, which) \
+ gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \
GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
-#define GEN3_IRQ_RESET(type) \
- gen3_irq_reset(&dev_priv->uncore, type##IMR, type##IIR, type##IER)
+#define GEN3_IRQ_RESET(uncore, type) \
+ gen3_irq_reset((uncore), type##IMR, type##IIR, type##IER)
-#define GEN2_IRQ_RESET(type) \
- gen2_irq_reset(&dev_priv->uncore, type##IMR, type##IIR, type##IER)
+#define GEN2_IRQ_RESET(uncore, type) \
+ gen2_irq_reset((uncore), type##IMR, type##IIR, type##IER)
/*
* We should clear IMR at preinstall/uninstall, and just check at postinstall.
@@ -233,17 +233,17 @@ static void gen2_irq_init(struct intel_uncore *uncore, i915_reg_t imr,
intel_uncore_posting_read16(uncore, imr);
}
-#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
- gen3_irq_init(&dev_priv->uncore, GEN8_##type##_IMR(which), \
+#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \
+ gen3_irq_init((uncore), GEN8_##type##_IMR(which), \
GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
imr_val, ier_val)
-#define GEN3_IRQ_INIT(type, imr_val, ier_val) \
- gen3_irq_init(&dev_priv->uncore, type##IMR, type##IIR, type##IER, \
+#define GEN3_IRQ_INIT(uncore, type, imr_val, ier_val) \
+ gen3_irq_init((uncore), type##IMR, type##IIR, type##IER, \
imr_val, ier_val)
-#define GEN2_IRQ_INIT(type, imr_val, ier_val) \
- gen2_irq_init(&dev_priv->uncore, type##IMR, type##IIR, type##IER, \
+#define GEN2_IRQ_INIT(uncore, type, imr_val, ier_val) \
+ gen2_irq_init((uncore), type##IMR, type##IIR, type##IER, \
imr_val, ier_val)
static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
@@ -3331,10 +3331,12 @@ static void i945gm_vblank_work_fini(struct drm_i915_private *dev_priv)
static void ibx_irq_reset(struct drm_i915_private *dev_priv)
{
+ struct intel_uncore *uncore = &dev_priv->uncore;
+
if (HAS_PCH_NOP(dev_priv))
return;
- GEN3_IRQ_RESET(SDE);
+ GEN3_IRQ_RESET(uncore, SDE);
if (HAS_PCH_CPT(dev_priv) || HAS_PCH_LPT(dev_priv))
I915_WRITE(SERR_INT, 0xffffffff);
@@ -3362,13 +3364,17 @@ static void ibx_irq_pre_postinstall(struct drm_device *dev)
static void gen5_gt_irq_reset(struct drm_i915_private *dev_priv)
{
- GEN3_IRQ_RESET(GT);
+ struct intel_uncore *uncore = &dev_priv->uncore;
+
+ GEN3_IRQ_RESET(uncore, GT);
if (INTEL_GEN(dev_priv) >= 6)
- GEN3_IRQ_RESET(GEN6_PM);
+ GEN3_IRQ_RESET(uncore, GEN6_PM);
}
static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
{
+ struct intel_uncore *uncore = &dev_priv->uncore;
+
if (IS_CHERRYVIEW(dev_priv))
I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK_CHV);
else
@@ -3379,12 +3385,14 @@ static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
i9xx_pipestat_irq_reset(dev_priv);
- GEN3_IRQ_RESET(VLV_);
+ GEN3_IRQ_RESET(uncore, VLV_);
dev_priv->irq_mask = ~0u;
}
static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
{
+ struct intel_uncore *uncore = &dev_priv->uncore;
+
u32 pipestat_mask;
u32 enable_mask;
enum pipe pipe;
@@ -3409,7 +3417,7 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
dev_priv->irq_mask = ~enable_mask;
- GEN3_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask);
+ GEN3_IRQ_INIT(uncore, VLV_, dev_priv->irq_mask, enable_mask);
}
/* drm_dma.h hooks
@@ -3417,8 +3425,9 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
static void ironlake_irq_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
- GEN3_IRQ_RESET(DE);
+ GEN3_IRQ_RESET(uncore, DE);
if (IS_GEN(dev_priv, 7))
I915_WRITE(GEN7_ERR_INT, 0xffffffff);
@@ -3449,15 +3458,18 @@ static void valleyview_irq_reset(struct drm_device *dev)
static void gen8_gt_irq_reset(struct drm_i915_private *dev_priv)
{
- GEN8_IRQ_RESET_NDX(GT, 0);
- GEN8_IRQ_RESET_NDX(GT, 1);
- GEN8_IRQ_RESET_NDX(GT, 2);
- GEN8_IRQ_RESET_NDX(GT, 3);
+ struct intel_uncore *uncore = &dev_priv->uncore;
+
+ GEN8_IRQ_RESET_NDX(uncore, GT, 0);
+ GEN8_IRQ_RESET_NDX(uncore, GT, 1);
+ GEN8_IRQ_RESET_NDX(uncore, GT, 2);
+ GEN8_IRQ_RESET_NDX(uncore, GT, 3);
}
static void gen8_irq_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
int pipe;
gen8_master_intr_disable(dev_priv->uncore.regs);
@@ -3470,11 +3482,11 @@ static void gen8_irq_reset(struct drm_device *dev)
for_each_pipe(dev_priv, pipe)
if (intel_display_power_is_enabled(dev_priv,
POWER_DOMAIN_PIPE(pipe)))
- GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
+ GEN8_IRQ_RESET_NDX(uncore, DE_PIPE, pipe);
- GEN3_IRQ_RESET(GEN8_DE_PORT_);
- GEN3_IRQ_RESET(GEN8_DE_MISC_);
- GEN3_IRQ_RESET(GEN8_PCU_);
+ GEN3_IRQ_RESET(uncore, GEN8_DE_PORT_);
+ GEN3_IRQ_RESET(uncore, GEN8_DE_MISC_);
+ GEN3_IRQ_RESET(uncore, GEN8_PCU_);
if (HAS_PCH_SPLIT(dev_priv))
ibx_irq_reset(dev_priv);
@@ -3500,6 +3512,7 @@ static void gen11_gt_irq_reset(struct drm_i915_private *dev_priv)
static void gen11_irq_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_uncore *uncore = &dev_priv->uncore;
int pipe;
gen11_master_intr_disable(dev_priv->uncore.regs);
@@ -3514,21 +3527,23 @@ static void gen11_irq_reset(struct drm_device *dev)
for_each_pipe(dev_priv, pipe)
if (intel_display_power_is_enabled(dev_priv,
POWER_DOMAIN_PIPE(pipe)))
- GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
+ GEN8_IRQ_RESET_NDX(uncore, DE_PIPE, pipe);
- GEN3_IRQ_RESET(GEN8_DE_PORT_);
- GEN3_IRQ_RESET(GEN8_DE_MISC_);
- GEN3_IRQ_RESET(GEN11_DE_HPD_);
- GEN3_IRQ_RESET(GEN11_GU_MISC_);
- GEN3_IRQ_RESET(GEN8_PCU_);
+ GEN3_IRQ_RESET(uncore, GEN8_DE_PORT_);
+ GEN3_IRQ_RESET(uncore, GEN8_DE_MISC_);
+ GEN3_IRQ_RESET(uncore, GEN11_DE_HPD_);
+ GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_);
+ GEN3_IRQ_RESET(uncore, GEN8_PCU_);
if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
- GEN3_IRQ_RESET(SDE);
+ GEN3_IRQ_RESET(uncore, SDE);
}
void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
u8 pipe_mask)
{
+ struct intel_uncore *uncore = &dev_priv->uncore;
+
u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN;
enum pipe pipe;
@@ -3540,7 +3555,7 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
}
for_each_pipe_masked(dev_priv, pipe, pipe_mask)
- GEN8_IRQ_INIT_NDX(DE_PIPE, pipe,
+ GEN8_IRQ_INIT_NDX(uncore, DE_PIPE, pipe,
dev_priv->de_irq_mask[pipe],
~dev_priv->de_irq_mask[pipe] | extra_ier);
@@ -3550,6 +3565,7 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
u8 pipe_mask)
{
+ struct intel_uncore *uncore = &dev_priv->uncore;
enum pipe pipe;
spin_lock_irq(&dev_priv->irq_lock);
@@ -3560,7 +3576,7 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
}
for_each_pipe_masked(dev_priv, pipe, pipe_mask)
- GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
+ GEN8_IRQ_RESET_NDX(uncore, DE_PIPE, pipe);
spin_unlock_irq(&dev_priv->irq_lock);
@@ -3571,13 +3587,14 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
static void cherryview_irq_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
I915_WRITE(GEN8_MASTER_IRQ, 0);
POSTING_READ(GEN8_MASTER_IRQ);
gen8_gt_irq_reset(dev_priv);
- GEN3_IRQ_RESET(GEN8_PCU_);
+ GEN3_IRQ_RESET(uncore, GEN8_PCU_);
spin_lock_irq(&dev_priv->irq_lock);
if (dev_priv->display_irqs_enabled)
@@ -3862,6 +3879,7 @@ static void ibx_irq_postinstall(struct drm_device *dev)
static void gen5_gt_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
u32 pm_irqs, gt_irqs;
pm_irqs = gt_irqs = 0;
@@ -3880,7 +3898,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
gt_irqs |= GT_BLT_USER_INTERRUPT | GT_BSD_USER_INTERRUPT;
}
- GEN3_IRQ_INIT(GT, dev_priv->gt_irq_mask, gt_irqs);
+ GEN3_IRQ_INIT(uncore, GT, dev_priv->gt_irq_mask, gt_irqs);
if (INTEL_GEN(dev_priv) >= 6) {
/*
@@ -3893,13 +3911,14 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
}
dev_priv->pm_imr = 0xffffffff;
- GEN3_IRQ_INIT(GEN6_PM, dev_priv->pm_imr, pm_irqs);
+ GEN3_IRQ_INIT(uncore, GEN6_PM, dev_priv->pm_imr, pm_irqs);
}
}
static int ironlake_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
u32 display_mask, extra_mask;
if (INTEL_GEN(dev_priv) >= 7) {
@@ -3918,7 +3937,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
}
if (IS_HASWELL(dev_priv)) {
- gen3_assert_iir_is_zero(&dev_priv->uncore, EDP_PSR_IIR);
+ gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
display_mask |= DE_EDP_PSR_INT_HSW;
}
@@ -3927,7 +3946,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
ibx_irq_pre_postinstall(dev);
- GEN3_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);
+ GEN3_IRQ_INIT(uncore, DE, dev_priv->irq_mask,
+ display_mask | extra_mask);
gen5_gt_irq_postinstall(dev);
@@ -3997,6 +4017,8 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
{
+ struct intel_uncore *uncore = &dev_priv->uncore;
+
/* These are interrupts we'll toggle with the ring mask register */
u32 gt_interrupts[] = {
(GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
@@ -4017,18 +4039,20 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
dev_priv->pm_ier = 0x0;
dev_priv->pm_imr = ~dev_priv->pm_ier;
- GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
- GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
+ GEN8_IRQ_INIT_NDX(uncore, GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
+ GEN8_IRQ_INIT_NDX(uncore, GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
/*
* RPS interrupts will get enabled/disabled on demand when RPS itself
* is enabled/disabled. Same wil be the case for GuC interrupts.
*/
- GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_imr, dev_priv->pm_ier);
- GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
+ GEN8_IRQ_INIT_NDX(uncore, GT, 2, dev_priv->pm_imr, dev_priv->pm_ier);
+ GEN8_IRQ_INIT_NDX(uncore, GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
}
static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
{
+ struct intel_uncore *uncore = &dev_priv->uncore;
+
u32 de_pipe_masked = GEN8_PIPE_CDCLK_CRC_DONE;
u32 de_pipe_enables;
u32 de_port_masked = GEN8_AUX_CHANNEL_A;
@@ -4064,7 +4088,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
else if (IS_BROADWELL(dev_priv))
de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
- gen3_assert_iir_is_zero(&dev_priv->uncore, EDP_PSR_IIR);
+ gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
for_each_pipe(dev_priv, pipe) {
@@ -4072,20 +4096,21 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
if (intel_display_power_is_enabled(dev_priv,
POWER_DOMAIN_PIPE(pipe)))
- GEN8_IRQ_INIT_NDX(DE_PIPE, pipe,
+ GEN8_IRQ_INIT_NDX(uncore, DE_PIPE, pipe,
dev_priv->de_irq_mask[pipe],
de_pipe_enables);
}
- GEN3_IRQ_INIT(GEN8_DE_PORT_, ~de_port_masked, de_port_enables);
- GEN3_IRQ_INIT(GEN8_DE_MISC_, ~de_misc_masked, de_misc_masked);
+ GEN3_IRQ_INIT(uncore, GEN8_DE_PORT_, ~de_port_masked, de_port_enables);
+ GEN3_IRQ_INIT(uncore, GEN8_DE_MISC_, ~de_misc_masked, de_misc_masked);
if (INTEL_GEN(dev_priv) >= 11) {
u32 de_hpd_masked = 0;
u32 de_hpd_enables = GEN11_DE_TC_HOTPLUG_MASK |
GEN11_DE_TBT_HOTPLUG_MASK;
- GEN3_IRQ_INIT(GEN11_DE_HPD_, ~de_hpd_masked, de_hpd_enables);
+ GEN3_IRQ_INIT(uncore, GEN11_DE_HPD_, ~de_hpd_masked,
+ de_hpd_enables);
gen11_hpd_detection_setup(dev_priv);
} else if (IS_GEN9_LP(dev_priv)) {
bxt_hpd_detection_setup(dev_priv);
@@ -4157,6 +4182,7 @@ static void icp_irq_postinstall(struct drm_device *dev)
static int gen11_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_uncore *uncore = &dev_priv->uncore;
u32 gu_misc_masked = GEN11_GU_MISC_GSE;
if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
@@ -4165,7 +4191,7 @@ static int gen11_irq_postinstall(struct drm_device *dev)
gen11_gt_irq_postinstall(dev_priv);
gen8_de_irq_postinstall(dev_priv);
- GEN3_IRQ_INIT(GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
+ GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
I915_WRITE(GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE);
@@ -4195,15 +4221,17 @@ static int cherryview_irq_postinstall(struct drm_device *dev)
static void i8xx_irq_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
i9xx_pipestat_irq_reset(dev_priv);
- GEN2_IRQ_RESET();
+ GEN2_IRQ_RESET(uncore, );
}
static int i8xx_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
u16 enable_mask;
I915_WRITE16(EMR, ~(I915_ERROR_PAGE_TABLE |
@@ -4221,7 +4249,7 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
I915_MASTER_ERROR_INTERRUPT |
I915_USER_INTERRUPT;
- GEN2_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
+ GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
/* Interrupt setup is already guaranteed to be single-threaded, this is
* just to make the assert_spin_locked check happy. */
@@ -4357,6 +4385,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
static void i915_irq_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
if (I915_HAS_HOTPLUG(dev_priv)) {
i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
@@ -4365,12 +4394,13 @@ static void i915_irq_reset(struct drm_device *dev)
i9xx_pipestat_irq_reset(dev_priv);
- GEN3_IRQ_RESET();
+ GEN3_IRQ_RESET(uncore, );
}
static int i915_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
u32 enable_mask;
I915_WRITE(EMR, ~(I915_ERROR_PAGE_TABLE |
@@ -4397,7 +4427,7 @@ static int i915_irq_postinstall(struct drm_device *dev)
dev_priv->irq_mask &= ~I915_DISPLAY_PORT_INTERRUPT;
}
- GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
+ GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
/* Interrupt setup is already guaranteed to be single-threaded, this is
* just to make the assert_spin_locked check happy. */
@@ -4468,18 +4498,20 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
static void i965_irq_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
i9xx_pipestat_irq_reset(dev_priv);
- GEN3_IRQ_RESET();
+ GEN3_IRQ_RESET(uncore, );
}
static int i965_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_uncore *uncore = &dev_priv->uncore;
u32 enable_mask;
u32 error_mask;
@@ -4517,7 +4549,7 @@ static int i965_irq_postinstall(struct drm_device *dev)
if (IS_G4X(dev_priv))
enable_mask |= I915_BSD_USER_INTERRUPT;
- GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
+ GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
/* Interrupt setup is already guaranteed to be single-threaded, this is
* just to make the assert_spin_locked check happy. */
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for IRQ initialization debloat and conversion to uncore
2019-04-09 0:37 [PATCH 0/3] IRQ initialization debloat and conversion to uncore Paulo Zanoni
` (2 preceding siblings ...)
2019-04-09 0:37 ` [PATCH 3/3] drm/i915: fully convert the IRQ initialization macros " Paulo Zanoni
@ 2019-04-09 0:44 ` Patchwork
2019-04-09 17:34 ` Paulo Zanoni
2019-04-09 1:07 ` ✓ Fi.CI.BAT: success " Patchwork
2019-04-09 7:32 ` ✓ Fi.CI.IGT: " Patchwork
5 siblings, 1 reply; 14+ messages in thread
From: Patchwork @ 2019-04-09 0:44 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
== Series Details ==
Series: IRQ initialization debloat and conversion to uncore
URL : https://patchwork.freedesktop.org/series/59202/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
7f73d1fe31bb drm/i915: refactor the IRQ init/reset macros
-:114: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
#114: FILE: drivers/gpu/drm/i915/i915_irq.c:169:
+#define GEN8_IRQ_RESET_NDX(type, which) \
+ gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \
+ GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
-:172: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
#172: FILE: drivers/gpu/drm/i915/i915_irq.c:236:
+#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
+ gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \
+ GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
+ imr_val, ier_val)
total: 0 errors, 0 warnings, 2 checks, 135 lines checked
82160241d80f drm/i915: convert the IRQ initialization functions to intel_uncore
8c1c76059a41 drm/i915: fully convert the IRQ initialization macros to intel_uncore
-:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
#24: FILE: drivers/gpu/drm/i915/i915_irq.c:169:
+#define GEN8_IRQ_RESET_NDX(uncore, type, which) \
+ gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \
GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
-:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
#46: FILE: drivers/gpu/drm/i915/i915_irq.c:236:
+#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \
+ gen3_irq_init((uncore), GEN8_##type##_IMR(which), \
GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
imr_val, ier_val)
-:401: ERROR:SPACING: space prohibited before that close parenthesis ')'
#401: FILE: drivers/gpu/drm/i915/i915_irq.c:4228:
+ GEN2_IRQ_RESET(uncore, );
-:416: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
#416: FILE: drivers/gpu/drm/i915/i915_irq.c:4252:
+ GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
^
-:433: ERROR:SPACING: space prohibited before that close parenthesis ')'
#433: FILE: drivers/gpu/drm/i915/i915_irq.c:4397:
+ GEN3_IRQ_RESET(uncore, );
-:448: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
#448: FILE: drivers/gpu/drm/i915/i915_irq.c:4430:
+ GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
^
-:464: ERROR:SPACING: space prohibited before that close parenthesis ')'
#464: FILE: drivers/gpu/drm/i915/i915_irq.c:4508:
+ GEN3_IRQ_RESET(uncore, );
-:479: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
#479: FILE: drivers/gpu/drm/i915/i915_irq.c:4552:
+ GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
^
total: 6 errors, 0 warnings, 2 checks, 432 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.BAT: success for IRQ initialization debloat and conversion to uncore
2019-04-09 0:37 [PATCH 0/3] IRQ initialization debloat and conversion to uncore Paulo Zanoni
` (3 preceding siblings ...)
2019-04-09 0:44 ` ✗ Fi.CI.CHECKPATCH: warning for IRQ initialization debloat and conversion to uncore Patchwork
@ 2019-04-09 1:07 ` Patchwork
2019-04-09 7:32 ` ✓ Fi.CI.IGT: " Patchwork
5 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2019-04-09 1:07 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
== Series Details ==
Series: IRQ initialization debloat and conversion to uncore
URL : https://patchwork.freedesktop.org/series/59202/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5891 -> Patchwork_12734
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/59202/revisions/1/mbox/
Known issues
------------
Here are the changes found in Patchwork_12734 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_create@basic-files:
- fi-gdg-551: NOTRUN -> SKIP [fdo#109271] +106
* igt@i915_module_load@reload:
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]
* igt@i915_pm_rpm@module-reload:
- fi-glk-dsi: NOTRUN -> DMESG-WARN [fdo#105538] / [fdo#107732] / [fdo#109513]
* igt@i915_selftest@live_contexts:
- fi-skl-gvtdvm: PASS -> DMESG-FAIL [fdo#110235 ]
* igt@i915_selftest@live_hangcheck:
- fi-skl-iommu: PASS -> INCOMPLETE [fdo#108602] / [fdo#108744]
- fi-bxt-dsi: PASS -> INCOMPLETE [fdo#103927]
* igt@kms_busy@basic-flip-c:
- fi-gdg-551: NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
* igt@kms_flip@basic-flip-vs-wf_vblank:
- fi-bwr-2160: PASS -> FAIL [fdo#100368]
* igt@kms_force_connector_basic@force-edid:
- fi-glk-dsi: NOTRUN -> SKIP [fdo#109271] +8
* igt@kms_frontbuffer_tracking@basic:
- fi-glk-dsi: NOTRUN -> FAIL [fdo#103167]
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- fi-icl-u3: PASS -> INCOMPLETE [fdo#107713]
* igt@runner@aborted:
- fi-glk-dsi: NOTRUN -> FAIL [k.org#202321]
- fi-skl-iommu: NOTRUN -> FAIL [fdo#104108] / [fdo#108602]
#### Possible fixes ####
* igt@gem_cpu_reloc@basic:
- {fi-icl-u2}: INCOMPLETE [fdo#110246] -> PASS
* igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
- fi-glk-dsi: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105538]: https://bugs.freedesktop.org/show_bug.cgi?id=105538
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#107732]: https://bugs.freedesktop.org/show_bug.cgi?id=107732
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#109316]: https://bugs.freedesktop.org/show_bug.cgi?id=109316
[fdo#109513]: https://bugs.freedesktop.org/show_bug.cgi?id=109513
[fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
[fdo#110246]: https://bugs.freedesktop.org/show_bug.cgi?id=110246
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
[k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321
Participating hosts (48 -> 44)
------------------------------
Additional (1): fi-gdg-551
Missing (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper
Build changes
-------------
* Linux: CI_DRM_5891 -> Patchwork_12734
CI_DRM_5891: a46e12e83547c781a779776f33fbeeefe2978905 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4932: 08cf63a8fac11e3594b57580331fb319241a0d69 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12734: 8c1c76059a415d79f400407ac60aa30d550cd805 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
8c1c76059a41 drm/i915: fully convert the IRQ initialization macros to intel_uncore
82160241d80f drm/i915: convert the IRQ initialization functions to intel_uncore
7f73d1fe31bb drm/i915: refactor the IRQ init/reset macros
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12734/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.IGT: success for IRQ initialization debloat and conversion to uncore
2019-04-09 0:37 [PATCH 0/3] IRQ initialization debloat and conversion to uncore Paulo Zanoni
` (4 preceding siblings ...)
2019-04-09 1:07 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-04-09 7:32 ` Patchwork
5 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2019-04-09 7:32 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
== Series Details ==
Series: IRQ initialization debloat and conversion to uncore
URL : https://patchwork.freedesktop.org/series/59202/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5891_full -> Patchwork_12734_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Known issues
------------
Here are the changes found in Patchwork_12734_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_sseu@invalid-args:
- shard-skl: NOTRUN -> SKIP [fdo#109271] +107
* igt@gem_exec_nop@basic-series:
- shard-iclb: PASS -> FAIL [fdo#110342]
* igt@gem_exec_schedule@independent-bsd1:
- shard-iclb: NOTRUN -> SKIP [fdo#109276] +3
* igt@gen3_render_tiledy_blits:
- shard-iclb: NOTRUN -> SKIP [fdo#109289]
* igt@i915_pm_rpm@debugfs-read:
- shard-skl: PASS -> INCOMPLETE [fdo#107807] +1
* igt@i915_pm_rpm@dpms-non-lpsp:
- shard-skl: NOTRUN -> INCOMPLETE [fdo#107807]
* igt@i915_pm_rpm@i2c:
- shard-iclb: PASS -> DMESG-WARN [fdo#109982]
* igt@i915_pm_rpm@pc8-residency:
- shard-iclb: NOTRUN -> SKIP [fdo#109293]
* igt@kms_atomic_transition@3x-modeset-transitions:
- shard-skl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +9
- shard-kbl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-d:
- shard-apl: NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +10
* igt@kms_chamelium@hdmi-edid-read:
- shard-iclb: NOTRUN -> SKIP [fdo#109284] +2
* igt@kms_color@pipe-a-ctm-max:
- shard-skl: NOTRUN -> FAIL [fdo#108147]
* igt@kms_content_protection@legacy:
- shard-iclb: NOTRUN -> SKIP [fdo#109300]
* igt@kms_cursor_crc@cursor-512x512-rapid-movement:
- shard-iclb: NOTRUN -> SKIP [fdo#109279]
* igt@kms_cursor_crc@cursor-64x64-sliding:
- shard-skl: NOTRUN -> FAIL [fdo#103232]
* igt@kms_cursor_crc@cursor-64x64-suspend:
- shard-skl: PASS -> INCOMPLETE [fdo#104108]
* igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
- shard-iclb: NOTRUN -> SKIP [fdo#109274]
* igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
- shard-glk: PASS -> FAIL [fdo#106509] / [fdo#107409]
* igt@kms_draw_crc@draw-method-rgb565-render-ytiled:
- shard-apl: PASS -> INCOMPLETE [fdo#103927]
* igt@kms_flip@flip-vs-suspend:
- shard-apl: PASS -> DMESG-WARN [fdo#108566]
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
- shard-iclb: PASS -> FAIL [fdo#103167] +1
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-gtt:
- shard-iclb: PASS -> FAIL [fdo#109247] +6
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc:
- shard-kbl: NOTRUN -> SKIP [fdo#109271] +12
* igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-move:
- shard-iclb: NOTRUN -> SKIP [fdo#109280] +3
* igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-cur-indfb-move:
- shard-apl: NOTRUN -> SKIP [fdo#109271] +177
* igt@kms_frontbuffer_tracking@psr-suspend:
- shard-skl: PASS -> INCOMPLETE [fdo#104108] / [fdo#106978]
* igt@kms_lease@setcrtc_implicit_plane:
- shard-skl: NOTRUN -> FAIL [fdo#110281]
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-f:
- shard-iclb: NOTRUN -> SKIP [fdo#109278]
* igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
- shard-apl: NOTRUN -> FAIL [fdo#108145] +3
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
- shard-skl: NOTRUN -> FAIL [fdo#108145] +1
* igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
- shard-kbl: NOTRUN -> FAIL [fdo#108145] / [fdo#108590]
- shard-skl: NOTRUN -> FAIL [fdo#107815] / [fdo#108145] +1
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
- shard-skl: PASS -> FAIL [fdo#108145]
* igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
- shard-skl: PASS -> FAIL [fdo#107815]
* igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl: NOTRUN -> FAIL [fdo#107815]
* igt@kms_plane_lowres@pipe-a-tiling-y:
- shard-iclb: PASS -> FAIL [fdo#103166]
* igt@kms_psr@cursor_blt:
- shard-iclb: PASS -> FAIL [fdo#107383] / [fdo#110215] +2
* igt@kms_psr@psr2_cursor_plane_onoff:
- shard-iclb: PASS -> SKIP [fdo#109441] +2
* igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
- shard-kbl: PASS -> DMESG-FAIL [fdo#105763]
* igt@kms_setmode@basic:
- shard-apl: PASS -> FAIL [fdo#99912]
- shard-skl: PASS -> FAIL [fdo#99912]
- shard-kbl: PASS -> FAIL [fdo#99912]
* igt@kms_vblank@pipe-b-ts-continuation-suspend:
- shard-iclb: PASS -> FAIL [fdo#104894]
* igt@prime_nv_test@i915_import_gtt_mmap:
- shard-iclb: NOTRUN -> SKIP [fdo#109291]
* igt@tools_test@tools_test:
- shard-iclb: PASS -> SKIP [fdo#109352]
* igt@v3d_get_param@get-bad-flags:
- shard-iclb: NOTRUN -> SKIP [fdo#109315]
#### Possible fixes ####
* igt@gem_exec_reloc@basic-cpu-wc-active:
- shard-iclb: INCOMPLETE [fdo#107713] -> PASS
* igt@gem_mmap_gtt@forked-big-copy-odd:
- shard-iclb: TIMEOUT [fdo#109673] -> PASS
* igt@gem_mmap_gtt@hang:
- shard-iclb: FAIL [fdo#109677] -> PASS
* igt@gem_softpin@noreloc-s3:
- shard-skl: INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS
* igt@gem_tiled_swapping@non-threaded:
- shard-iclb: FAIL [fdo#108686] -> PASS
* igt@i915_pm_rpm@dpms-mode-unset-lpsp:
- shard-skl: INCOMPLETE [fdo#107807] -> PASS
* igt@kms_cursor_crc@cursor-256x256-suspend:
- shard-skl: FAIL [fdo#103191] / [fdo#103232] -> PASS
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-skl: FAIL [fdo#105363] -> PASS
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
- shard-iclb: FAIL [fdo#103167] -> PASS +3
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu:
- shard-iclb: FAIL [fdo#109247] -> PASS +5
* igt@kms_frontbuffer_tracking@psr-rgb565-draw-render:
- shard-skl: FAIL [fdo#103167] -> PASS +1
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: FAIL [fdo#108145] -> PASS
* igt@kms_psr@cursor_render:
- shard-iclb: DMESG-WARN [fdo#110025] -> PASS
* igt@kms_psr@psr2_cursor_mmap_cpu:
- shard-iclb: SKIP [fdo#109441] -> PASS +1
* igt@kms_psr@sprite_blt:
- shard-iclb: FAIL [fdo#107383] / [fdo#110215] -> PASS +1
* igt@kms_rotation_crc@multiplane-rotation:
- shard-iclb: DMESG-WARN [fdo#106885] -> PASS
- shard-kbl: INCOMPLETE [fdo#103665] -> PASS +1
#### Warnings ####
* igt@kms_dp_dsc@basic-dsc-enable-edp:
- shard-iclb: SKIP [fdo#109349] -> FAIL [fdo#110270]
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
[fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
[fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
[fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
[fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
[fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
[fdo#109293]: https://bugs.freedesktop.org/show_bug.cgi?id=109293
[fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
[fdo#109352]: https://bugs.freedesktop.org/show_bug.cgi?id=109352
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
[fdo#109677]: https://bugs.freedesktop.org/show_bug.cgi?id=109677
[fdo#109982]: https://bugs.freedesktop.org/show_bug.cgi?id=109982
[fdo#110025]: https://bugs.freedesktop.org/show_bug.cgi?id=110025
[fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
[fdo#110270]: https://bugs.freedesktop.org/show_bug.cgi?id=110270
[fdo#110281]: https://bugs.freedesktop.org/show_bug.cgi?id=110281
[fdo#110342]: https://bugs.freedesktop.org/show_bug.cgi?id=110342
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 9)
------------------------------
Missing (1): shard-hsw
Build changes
-------------
* Linux: CI_DRM_5891 -> Patchwork_12734
CI_DRM_5891: a46e12e83547c781a779776f33fbeeefe2978905 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4932: 08cf63a8fac11e3594b57580331fb319241a0d69 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12734: 8c1c76059a415d79f400407ac60aa30d550cd805 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12734/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ✗ Fi.CI.CHECKPATCH: warning for IRQ initialization debloat and conversion to uncore
2019-04-09 0:44 ` ✗ Fi.CI.CHECKPATCH: warning for IRQ initialization debloat and conversion to uncore Patchwork
@ 2019-04-09 17:34 ` Paulo Zanoni
2019-04-09 18:20 ` Ville Syrjälä
0 siblings, 1 reply; 14+ messages in thread
From: Paulo Zanoni @ 2019-04-09 17:34 UTC (permalink / raw)
To: intel-gfx
Em ter, 2019-04-09 às 00:44 +0000, Patchwork escreveu:
> == Series Details ==
>
> Series: IRQ initialization debloat and conversion to uncore
> URL : https://patchwork.freedesktop.org/series/59202/
> State : warning
>
> == Summary ==
>
> $ dim checkpatch origin/drm-tip
> 7f73d1fe31bb drm/i915: refactor the IRQ init/reset macros
> -:114: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> #114: FILE: drivers/gpu/drm/i915/i915_irq.c:169:
> +#define GEN8_IRQ_RESET_NDX(type, which) \
> + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \
> + GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
>
> -:172: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> #172: FILE: drivers/gpu/drm/i915/i915_irq.c:236:
> +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
> + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \
> + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> + imr_val, ier_val)
>
> total: 0 errors, 0 warnings, 2 checks, 135 lines checked
> 82160241d80f drm/i915: convert the IRQ initialization functions to intel_uncore
> 8c1c76059a41 drm/i915: fully convert the IRQ initialization macros to intel_uncore
> -:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> #24: FILE: drivers/gpu/drm/i915/i915_irq.c:169:
> +#define GEN8_IRQ_RESET_NDX(uncore, type, which) \
> + gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \
> GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
>
> -:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> #46: FILE: drivers/gpu/drm/i915/i915_irq.c:236:
> +#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \
> + gen3_irq_init((uncore), GEN8_##type##_IMR(which), \
> GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> imr_val, ier_val)
The whiches are not really a regression, but OK we can deal with them
to make the robots happy.
>
> -:401: ERROR:SPACING: space prohibited before that close parenthesis ')'
> #401: FILE: drivers/gpu/drm/i915/i915_irq.c:4228:
> + GEN2_IRQ_RESET(uncore, );
>
> -:416: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> #416: FILE: drivers/gpu/drm/i915/i915_irq.c:4252:
> + GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
> ^
>
> -:433: ERROR:SPACING: space prohibited before that close parenthesis ')'
> #433: FILE: drivers/gpu/drm/i915/i915_irq.c:4397:
> + GEN3_IRQ_RESET(uncore, );
>
> -:448: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> #448: FILE: drivers/gpu/drm/i915/i915_irq.c:4430:
> + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
> ^
>
> -:464: ERROR:SPACING: space prohibited before that close parenthesis ')'
> #464: FILE: drivers/gpu/drm/i915/i915_irq.c:4508:
> + GEN3_IRQ_RESET(uncore, );
>
> -:479: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> #479: FILE: drivers/gpu/drm/i915/i915_irq.c:4552:
> + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
For these ones I really think the spaces help. I would love to read
some opinions. Perhaps some comment like /* paste token here */ would
help make the code more readable and could help silence checkpatch.
Opinions?
> ^
>
> total: 6 errors, 0 warnings, 2 checks, 432 lines checked
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/i915: refactor the IRQ init/reset macros
2019-04-09 0:37 ` [PATCH 1/3] drm/i915: refactor the IRQ init/reset macros Paulo Zanoni
@ 2019-04-09 18:10 ` Ville Syrjälä
2019-04-10 20:45 ` Paulo Zanoni
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2019-04-09 18:10 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Mon, Apr 08, 2019 at 05:37:27PM -0700, Paulo Zanoni wrote:
> The whole point of having macros here is for the token pasting
> necessary to automatically have IMR, IIR and IER selected. We don't
> really need or want all the inlining that happens as a consequence.
> The good thing about the current code is that it works regardless of
> the relative offsets between these registers (they change after gen3).
Did you mean "after gen4"? The DE/GT split happened at ilk, and I
guess that's when the four registers also got reshuffled for some
reason. Ignoring the funkyness of vlv/chv or course :)
>
> One thing which we can do is to split the logic of what we do with
> imr/ier/iir to functions separate from the macros that pick them.
> That's what we do in this commit. This allows us to get rid of the
> gen8 duplicates and also all the inlining:
>
> add/remove: 2/0 grow/shrink: 0/21 up/down: 383/-6006 (-5623)
> Function old new delta
> gen3_irq_reset - 233 +233
> gen3_irq_init - 150 +150
> i8xx_irq_postinstall 459 442 -17
> gen11_irq_postinstall 804 744 -60
> ironlake_irq_postinstall 450 353 -97
> vlv_display_irq_postinstall 348 245 -103
> i965_irq_postinstall 378 275 -103
> i915_irq_postinstall 333 228 -105
> gen8_irq_power_well_post_enable 374 210 -164
> ironlake_irq_reset 397 218 -179
> vlv_display_irq_reset 616 433 -183
> i965_irq_reset 374 180 -194
> cherryview_irq_reset 379 185 -194
> i915_irq_reset 407 209 -198
> ibx_irq_reset 332 133 -199
> gen5_gt_irq_postinstall 533 332 -201
> gen8_irq_power_well_pre_disable 434 204 -230
> gen8_gt_irq_postinstall 469 196 -273
> gen8_de_irq_postinstall 1200 805 -395
> gen5_gt_irq_reset 471 76 -395
> gen8_gt_irq_reset 775 99 -676
> gen8_irq_reset 1100 333 -767
> gen11_irq_reset 1959 686 -1273
> Total: Before=2262051, After=2256428, chg -0.25%
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++-------------
> 1 file changed, 73 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 6454ddc37f8b..a1e7944fb5d0 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -136,36 +136,45 @@ static const u32 hpd_icp[HPD_NUM_PINS] = {
> [HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP
> };
>
> -/* IIR can theoretically queue up two events. Be paranoid. */
> -#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); \
> - 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 GEN3_IRQ_RESET(type) do { \
> - I915_WRITE(type##IMR, 0xffffffff); \
> - POSTING_READ(type##IMR); \
> - I915_WRITE(type##IER, 0); \
> - I915_WRITE(type##IIR, 0xffffffff); \
> - POSTING_READ(type##IIR); \
> - I915_WRITE(type##IIR, 0xffffffff); \
> - POSTING_READ(type##IIR); \
> -} while (0)
> -
> -#define GEN2_IRQ_RESET(type) do { \
> - I915_WRITE16(type##IMR, 0xffff); \
> - POSTING_READ16(type##IMR); \
> - I915_WRITE16(type##IER, 0); \
> - I915_WRITE16(type##IIR, 0xffff); \
> - POSTING_READ16(type##IIR); \
> - I915_WRITE16(type##IIR, 0xffff); \
> - POSTING_READ16(type##IIR); \
> -} while (0)
> +static void gen3_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr,
> + i915_reg_t iir, i915_reg_t ier)
> +{
> + I915_WRITE(imr, 0xffffffff);
> + POSTING_READ(imr);
> +
> + I915_WRITE(ier, 0);
> +
> + /* IIR can theoretically queue up two events. Be paranoid. */
> + I915_WRITE(iir, 0xffffffff);
> + POSTING_READ(iir);
> + I915_WRITE(iir, 0xffffffff);
> + POSTING_READ(iir);
> +}
> +
> +static void gen2_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr,
> + i915_reg_t iir, i915_reg_t ier)
> +{
> + I915_WRITE16(imr, 0xffff);
> + POSTING_READ16(imr);
> +
> + I915_WRITE16(ier, 0);
> +
> + /* IIR can theoretically queue up two events. Be paranoid. */
> + I915_WRITE16(iir, 0xffff);
> + POSTING_READ16(iir);
> + I915_WRITE16(iir, 0xffff);
> + POSTING_READ16(iir);
> +}
> +
> +#define GEN8_IRQ_RESET_NDX(type, which) \
> + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \
> + GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
> +
> +#define GEN3_IRQ_RESET(type) \
> + gen3_irq_reset(dev_priv, type##IMR, type##IIR, type##IER)
> +
> +#define GEN2_IRQ_RESET(type) \
> + gen2_irq_reset(dev_priv, type##IMR, type##IIR, type##IER)
>
> /*
> * We should clear IMR at preinstall/uninstall, and just check at postinstall.
> @@ -202,26 +211,40 @@ static void gen2_assert_iir_is_zero(struct drm_i915_private *dev_priv,
> POSTING_READ16(reg);
> }
>
> -#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \
> - gen3_assert_iir_is_zero(dev_priv, GEN8_##type##_IIR(which)); \
> - I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \
> - I915_WRITE(GEN8_##type##_IMR(which), (imr_val)); \
> - POSTING_READ(GEN8_##type##_IMR(which)); \
> -} while (0)
> -
> -#define GEN3_IRQ_INIT(type, imr_val, ier_val) do { \
> - gen3_assert_iir_is_zero(dev_priv, type##IIR); \
> - I915_WRITE(type##IER, (ier_val)); \
> - I915_WRITE(type##IMR, (imr_val)); \
> - POSTING_READ(type##IMR); \
> -} while (0)
> -
> -#define GEN2_IRQ_INIT(type, imr_val, ier_val) do { \
> - gen2_assert_iir_is_zero(dev_priv, type##IIR); \
> - I915_WRITE16(type##IER, (ier_val)); \
> - I915_WRITE16(type##IMR, (imr_val)); \
> - POSTING_READ16(type##IMR); \
> -} while (0)
> +static void gen3_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr,
> + i915_reg_t iir, i915_reg_t ier, u32 imr_val,
> + u32 ier_val)
Maybe we should order these more like this
foo(imr, imr_value, ier, ier_value, iir) ?
Could make a bit more obvious which values goes to which register. But
I suppose as long as they're always wrapped in those macros it doesn't
really matter.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> +{
> + gen3_assert_iir_is_zero(dev_priv, iir);
> +
> + I915_WRITE(ier, ier_val);
> + I915_WRITE(imr, imr_val);
> + POSTING_READ(imr);
> +}
> +
> +static void gen2_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr,
> + i915_reg_t iir, i915_reg_t ier, u32 imr_val,
> + u32 ier_val)
> +{
> + gen2_assert_iir_is_zero(dev_priv, iir);
> +
> + I915_WRITE16(ier, ier_val);
> + I915_WRITE16(imr, imr_val);
> + POSTING_READ16(imr);
> +}
> +
> +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
> + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \
> + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> + imr_val, ier_val)
> +
> +#define GEN3_IRQ_INIT(type, imr_val, ier_val) \
> + gen3_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \
> + ier_val)
> +
> +#define GEN2_IRQ_INIT(type, imr_val, ier_val) \
> + gen2_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \
> + ier_val)
>
> static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
> static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ✗ Fi.CI.CHECKPATCH: warning for IRQ initialization debloat and conversion to uncore
2019-04-09 17:34 ` Paulo Zanoni
@ 2019-04-09 18:20 ` Ville Syrjälä
2019-04-10 22:53 ` Paulo Zanoni
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2019-04-09 18:20 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Tue, Apr 09, 2019 at 10:34:22AM -0700, Paulo Zanoni wrote:
> Em ter, 2019-04-09 às 00:44 +0000, Patchwork escreveu:
> > == Series Details ==
> >
> > Series: IRQ initialization debloat and conversion to uncore
> > URL : https://patchwork.freedesktop.org/series/59202/
> > State : warning
> >
> > == Summary ==
> >
> > $ dim checkpatch origin/drm-tip
> > 7f73d1fe31bb drm/i915: refactor the IRQ init/reset macros
> > -:114: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > #114: FILE: drivers/gpu/drm/i915/i915_irq.c:169:
> > +#define GEN8_IRQ_RESET_NDX(type, which) \
> > + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \
> > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
> >
> > -:172: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > #172: FILE: drivers/gpu/drm/i915/i915_irq.c:236:
> > +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
> > + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \
> > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> > + imr_val, ier_val)
> >
> > total: 0 errors, 0 warnings, 2 checks, 135 lines checked
> > 82160241d80f drm/i915: convert the IRQ initialization functions to intel_uncore
> > 8c1c76059a41 drm/i915: fully convert the IRQ initialization macros to intel_uncore
> > -:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > #24: FILE: drivers/gpu/drm/i915/i915_irq.c:169:
> > +#define GEN8_IRQ_RESET_NDX(uncore, type, which) \
> > + gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \
> > GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
> >
> > -:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > #46: FILE: drivers/gpu/drm/i915/i915_irq.c:236:
> > +#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \
> > + gen3_irq_init((uncore), GEN8_##type##_IMR(which), \
> > GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> > imr_val, ier_val)
>
> The whiches are not really a regression, but OK we can deal with them
> to make the robots happy.
>
> >
> > -:401: ERROR:SPACING: space prohibited before that close parenthesis ')'
> > #401: FILE: drivers/gpu/drm/i915/i915_irq.c:4228:
> > + GEN2_IRQ_RESET(uncore, );
> >
> > -:416: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> > #416: FILE: drivers/gpu/drm/i915/i915_irq.c:4252:
> > + GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
> > ^
> >
> > -:433: ERROR:SPACING: space prohibited before that close parenthesis ')'
> > #433: FILE: drivers/gpu/drm/i915/i915_irq.c:4397:
> > + GEN3_IRQ_RESET(uncore, );
> >
> > -:448: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> > #448: FILE: drivers/gpu/drm/i915/i915_irq.c:4430:
> > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
> > ^
> >
> > -:464: ERROR:SPACING: space prohibited before that close parenthesis ')'
> > #464: FILE: drivers/gpu/drm/i915/i915_irq.c:4508:
> > + GEN3_IRQ_RESET(uncore, );
> >
> > -:479: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> > #479: FILE: drivers/gpu/drm/i915/i915_irq.c:4552:
> > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
>
> For these ones I really think the spaces help. I would love to read
> some opinions. Perhaps some comment like /* paste token here */ would
> help make the code more readable and could help silence checkpatch.
> Opinions?
Or maybe rename the registers to eg. I9XX_IIR?
>
> > ^
> >
> > total: 6 errors, 0 warnings, 2 checks, 432 lines checked
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drm/i915: fully convert the IRQ initialization macros to intel_uncore
2019-04-09 0:37 ` [PATCH 3/3] drm/i915: fully convert the IRQ initialization macros " Paulo Zanoni
@ 2019-04-09 20:21 ` Ville Syrjälä
0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2019-04-09 20:21 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Mon, Apr 08, 2019 at 05:37:29PM -0700, Paulo Zanoni wrote:
> Make them take the uncore argument from the caller instead of passing
> the implicit &dev_priv->uncore directly. This will allow us to finally
> pass something that's not dev_priv->uncore in the future, and gets rid
> of the implicit variables in register macros.
I've not been paying much attention to the uncore developments,
so I'm not sure this matches what other people think we should
do. But techinally these look OK to me.
Patches 2 and 3 are
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
But I guess give other people a bit of time to complain before
pushing :)
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 144 +++++++++++++++++++-------------
> 1 file changed, 88 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 99a6527568cf..b6361bab1086 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -166,15 +166,15 @@ static void gen2_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
> intel_uncore_posting_read16(uncore, iir);
> }
>
> -#define GEN8_IRQ_RESET_NDX(type, which) \
> - gen3_irq_reset(&dev_priv->uncore, GEN8_##type##_IMR(which), \
> +#define GEN8_IRQ_RESET_NDX(uncore, type, which) \
> + gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \
> GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
>
> -#define GEN3_IRQ_RESET(type) \
> - gen3_irq_reset(&dev_priv->uncore, type##IMR, type##IIR, type##IER)
> +#define GEN3_IRQ_RESET(uncore, type) \
> + gen3_irq_reset((uncore), type##IMR, type##IIR, type##IER)
>
> -#define GEN2_IRQ_RESET(type) \
> - gen2_irq_reset(&dev_priv->uncore, type##IMR, type##IIR, type##IER)
> +#define GEN2_IRQ_RESET(uncore, type) \
> + gen2_irq_reset((uncore), type##IMR, type##IIR, type##IER)
>
> /*
> * We should clear IMR at preinstall/uninstall, and just check at postinstall.
> @@ -233,17 +233,17 @@ static void gen2_irq_init(struct intel_uncore *uncore, i915_reg_t imr,
> intel_uncore_posting_read16(uncore, imr);
> }
>
> -#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
> - gen3_irq_init(&dev_priv->uncore, GEN8_##type##_IMR(which), \
> +#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \
> + gen3_irq_init((uncore), GEN8_##type##_IMR(which), \
> GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> imr_val, ier_val)
>
> -#define GEN3_IRQ_INIT(type, imr_val, ier_val) \
> - gen3_irq_init(&dev_priv->uncore, type##IMR, type##IIR, type##IER, \
> +#define GEN3_IRQ_INIT(uncore, type, imr_val, ier_val) \
> + gen3_irq_init((uncore), type##IMR, type##IIR, type##IER, \
> imr_val, ier_val)
>
> -#define GEN2_IRQ_INIT(type, imr_val, ier_val) \
> - gen2_irq_init(&dev_priv->uncore, type##IMR, type##IIR, type##IER, \
> +#define GEN2_IRQ_INIT(uncore, type, imr_val, ier_val) \
> + gen2_irq_init((uncore), type##IMR, type##IIR, type##IER, \
> imr_val, ier_val)
>
> static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
> @@ -3331,10 +3331,12 @@ static void i945gm_vblank_work_fini(struct drm_i915_private *dev_priv)
>
> static void ibx_irq_reset(struct drm_i915_private *dev_priv)
> {
> + struct intel_uncore *uncore = &dev_priv->uncore;
> +
> if (HAS_PCH_NOP(dev_priv))
> return;
>
> - GEN3_IRQ_RESET(SDE);
> + GEN3_IRQ_RESET(uncore, SDE);
>
> if (HAS_PCH_CPT(dev_priv) || HAS_PCH_LPT(dev_priv))
> I915_WRITE(SERR_INT, 0xffffffff);
> @@ -3362,13 +3364,17 @@ static void ibx_irq_pre_postinstall(struct drm_device *dev)
>
> static void gen5_gt_irq_reset(struct drm_i915_private *dev_priv)
> {
> - GEN3_IRQ_RESET(GT);
> + struct intel_uncore *uncore = &dev_priv->uncore;
> +
> + GEN3_IRQ_RESET(uncore, GT);
> if (INTEL_GEN(dev_priv) >= 6)
> - GEN3_IRQ_RESET(GEN6_PM);
> + GEN3_IRQ_RESET(uncore, GEN6_PM);
> }
>
> static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
> {
> + struct intel_uncore *uncore = &dev_priv->uncore;
> +
> if (IS_CHERRYVIEW(dev_priv))
> I915_WRITE(DPINVGTT, DPINVGTT_STATUS_MASK_CHV);
> else
> @@ -3379,12 +3385,14 @@ static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
>
> i9xx_pipestat_irq_reset(dev_priv);
>
> - GEN3_IRQ_RESET(VLV_);
> + GEN3_IRQ_RESET(uncore, VLV_);
> dev_priv->irq_mask = ~0u;
> }
>
> static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
> {
> + struct intel_uncore *uncore = &dev_priv->uncore;
> +
> u32 pipestat_mask;
> u32 enable_mask;
> enum pipe pipe;
> @@ -3409,7 +3417,7 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
>
> dev_priv->irq_mask = ~enable_mask;
>
> - GEN3_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask);
> + GEN3_IRQ_INIT(uncore, VLV_, dev_priv->irq_mask, enable_mask);
> }
>
> /* drm_dma.h hooks
> @@ -3417,8 +3425,9 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
> static void ironlake_irq_reset(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
>
> - GEN3_IRQ_RESET(DE);
> + GEN3_IRQ_RESET(uncore, DE);
> if (IS_GEN(dev_priv, 7))
> I915_WRITE(GEN7_ERR_INT, 0xffffffff);
>
> @@ -3449,15 +3458,18 @@ static void valleyview_irq_reset(struct drm_device *dev)
>
> static void gen8_gt_irq_reset(struct drm_i915_private *dev_priv)
> {
> - GEN8_IRQ_RESET_NDX(GT, 0);
> - GEN8_IRQ_RESET_NDX(GT, 1);
> - GEN8_IRQ_RESET_NDX(GT, 2);
> - GEN8_IRQ_RESET_NDX(GT, 3);
> + struct intel_uncore *uncore = &dev_priv->uncore;
> +
> + GEN8_IRQ_RESET_NDX(uncore, GT, 0);
> + GEN8_IRQ_RESET_NDX(uncore, GT, 1);
> + GEN8_IRQ_RESET_NDX(uncore, GT, 2);
> + GEN8_IRQ_RESET_NDX(uncore, GT, 3);
> }
>
> static void gen8_irq_reset(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
> int pipe;
>
> gen8_master_intr_disable(dev_priv->uncore.regs);
> @@ -3470,11 +3482,11 @@ static void gen8_irq_reset(struct drm_device *dev)
> for_each_pipe(dev_priv, pipe)
> if (intel_display_power_is_enabled(dev_priv,
> POWER_DOMAIN_PIPE(pipe)))
> - GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
> + GEN8_IRQ_RESET_NDX(uncore, DE_PIPE, pipe);
>
> - GEN3_IRQ_RESET(GEN8_DE_PORT_);
> - GEN3_IRQ_RESET(GEN8_DE_MISC_);
> - GEN3_IRQ_RESET(GEN8_PCU_);
> + GEN3_IRQ_RESET(uncore, GEN8_DE_PORT_);
> + GEN3_IRQ_RESET(uncore, GEN8_DE_MISC_);
> + GEN3_IRQ_RESET(uncore, GEN8_PCU_);
>
> if (HAS_PCH_SPLIT(dev_priv))
> ibx_irq_reset(dev_priv);
> @@ -3500,6 +3512,7 @@ static void gen11_gt_irq_reset(struct drm_i915_private *dev_priv)
> static void gen11_irq_reset(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_uncore *uncore = &dev_priv->uncore;
> int pipe;
>
> gen11_master_intr_disable(dev_priv->uncore.regs);
> @@ -3514,21 +3527,23 @@ static void gen11_irq_reset(struct drm_device *dev)
> for_each_pipe(dev_priv, pipe)
> if (intel_display_power_is_enabled(dev_priv,
> POWER_DOMAIN_PIPE(pipe)))
> - GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
> + GEN8_IRQ_RESET_NDX(uncore, DE_PIPE, pipe);
>
> - GEN3_IRQ_RESET(GEN8_DE_PORT_);
> - GEN3_IRQ_RESET(GEN8_DE_MISC_);
> - GEN3_IRQ_RESET(GEN11_DE_HPD_);
> - GEN3_IRQ_RESET(GEN11_GU_MISC_);
> - GEN3_IRQ_RESET(GEN8_PCU_);
> + GEN3_IRQ_RESET(uncore, GEN8_DE_PORT_);
> + GEN3_IRQ_RESET(uncore, GEN8_DE_MISC_);
> + GEN3_IRQ_RESET(uncore, GEN11_DE_HPD_);
> + GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_);
> + GEN3_IRQ_RESET(uncore, GEN8_PCU_);
>
> if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> - GEN3_IRQ_RESET(SDE);
> + GEN3_IRQ_RESET(uncore, SDE);
> }
>
> void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> u8 pipe_mask)
> {
> + struct intel_uncore *uncore = &dev_priv->uncore;
> +
> u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN;
> enum pipe pipe;
>
> @@ -3540,7 +3555,7 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> }
>
> for_each_pipe_masked(dev_priv, pipe, pipe_mask)
> - GEN8_IRQ_INIT_NDX(DE_PIPE, pipe,
> + GEN8_IRQ_INIT_NDX(uncore, DE_PIPE, pipe,
> dev_priv->de_irq_mask[pipe],
> ~dev_priv->de_irq_mask[pipe] | extra_ier);
>
> @@ -3550,6 +3565,7 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
> void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
> u8 pipe_mask)
> {
> + struct intel_uncore *uncore = &dev_priv->uncore;
> enum pipe pipe;
>
> spin_lock_irq(&dev_priv->irq_lock);
> @@ -3560,7 +3576,7 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
> }
>
> for_each_pipe_masked(dev_priv, pipe, pipe_mask)
> - GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
> + GEN8_IRQ_RESET_NDX(uncore, DE_PIPE, pipe);
>
> spin_unlock_irq(&dev_priv->irq_lock);
>
> @@ -3571,13 +3587,14 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
> static void cherryview_irq_reset(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
>
> I915_WRITE(GEN8_MASTER_IRQ, 0);
> POSTING_READ(GEN8_MASTER_IRQ);
>
> gen8_gt_irq_reset(dev_priv);
>
> - GEN3_IRQ_RESET(GEN8_PCU_);
> + GEN3_IRQ_RESET(uncore, GEN8_PCU_);
>
> spin_lock_irq(&dev_priv->irq_lock);
> if (dev_priv->display_irqs_enabled)
> @@ -3862,6 +3879,7 @@ static void ibx_irq_postinstall(struct drm_device *dev)
> static void gen5_gt_irq_postinstall(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
> u32 pm_irqs, gt_irqs;
>
> pm_irqs = gt_irqs = 0;
> @@ -3880,7 +3898,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
> gt_irqs |= GT_BLT_USER_INTERRUPT | GT_BSD_USER_INTERRUPT;
> }
>
> - GEN3_IRQ_INIT(GT, dev_priv->gt_irq_mask, gt_irqs);
> + GEN3_IRQ_INIT(uncore, GT, dev_priv->gt_irq_mask, gt_irqs);
>
> if (INTEL_GEN(dev_priv) >= 6) {
> /*
> @@ -3893,13 +3911,14 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
> }
>
> dev_priv->pm_imr = 0xffffffff;
> - GEN3_IRQ_INIT(GEN6_PM, dev_priv->pm_imr, pm_irqs);
> + GEN3_IRQ_INIT(uncore, GEN6_PM, dev_priv->pm_imr, pm_irqs);
> }
> }
>
> static int ironlake_irq_postinstall(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
> u32 display_mask, extra_mask;
>
> if (INTEL_GEN(dev_priv) >= 7) {
> @@ -3918,7 +3937,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
> }
>
> if (IS_HASWELL(dev_priv)) {
> - gen3_assert_iir_is_zero(&dev_priv->uncore, EDP_PSR_IIR);
> + gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
> intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
> display_mask |= DE_EDP_PSR_INT_HSW;
> }
> @@ -3927,7 +3946,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
>
> ibx_irq_pre_postinstall(dev);
>
> - GEN3_IRQ_INIT(DE, dev_priv->irq_mask, display_mask | extra_mask);
> + GEN3_IRQ_INIT(uncore, DE, dev_priv->irq_mask,
> + display_mask | extra_mask);
>
> gen5_gt_irq_postinstall(dev);
>
> @@ -3997,6 +4017,8 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
>
> static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
> {
> + struct intel_uncore *uncore = &dev_priv->uncore;
> +
> /* These are interrupts we'll toggle with the ring mask register */
> u32 gt_interrupts[] = {
> (GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
> @@ -4017,18 +4039,20 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>
> dev_priv->pm_ier = 0x0;
> dev_priv->pm_imr = ~dev_priv->pm_ier;
> - GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
> - GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
> + GEN8_IRQ_INIT_NDX(uncore, GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
> + GEN8_IRQ_INIT_NDX(uncore, GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
> /*
> * RPS interrupts will get enabled/disabled on demand when RPS itself
> * is enabled/disabled. Same wil be the case for GuC interrupts.
> */
> - GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_imr, dev_priv->pm_ier);
> - GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
> + GEN8_IRQ_INIT_NDX(uncore, GT, 2, dev_priv->pm_imr, dev_priv->pm_ier);
> + GEN8_IRQ_INIT_NDX(uncore, GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
> }
>
> static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> {
> + struct intel_uncore *uncore = &dev_priv->uncore;
> +
> u32 de_pipe_masked = GEN8_PIPE_CDCLK_CRC_DONE;
> u32 de_pipe_enables;
> u32 de_port_masked = GEN8_AUX_CHANNEL_A;
> @@ -4064,7 +4088,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> else if (IS_BROADWELL(dev_priv))
> de_port_enables |= GEN8_PORT_DP_A_HOTPLUG;
>
> - gen3_assert_iir_is_zero(&dev_priv->uncore, EDP_PSR_IIR);
> + gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
> intel_psr_irq_control(dev_priv, dev_priv->psr.debug);
>
> for_each_pipe(dev_priv, pipe) {
> @@ -4072,20 +4096,21 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>
> if (intel_display_power_is_enabled(dev_priv,
> POWER_DOMAIN_PIPE(pipe)))
> - GEN8_IRQ_INIT_NDX(DE_PIPE, pipe,
> + GEN8_IRQ_INIT_NDX(uncore, DE_PIPE, pipe,
> dev_priv->de_irq_mask[pipe],
> de_pipe_enables);
> }
>
> - GEN3_IRQ_INIT(GEN8_DE_PORT_, ~de_port_masked, de_port_enables);
> - GEN3_IRQ_INIT(GEN8_DE_MISC_, ~de_misc_masked, de_misc_masked);
> + GEN3_IRQ_INIT(uncore, GEN8_DE_PORT_, ~de_port_masked, de_port_enables);
> + GEN3_IRQ_INIT(uncore, GEN8_DE_MISC_, ~de_misc_masked, de_misc_masked);
>
> if (INTEL_GEN(dev_priv) >= 11) {
> u32 de_hpd_masked = 0;
> u32 de_hpd_enables = GEN11_DE_TC_HOTPLUG_MASK |
> GEN11_DE_TBT_HOTPLUG_MASK;
>
> - GEN3_IRQ_INIT(GEN11_DE_HPD_, ~de_hpd_masked, de_hpd_enables);
> + GEN3_IRQ_INIT(uncore, GEN11_DE_HPD_, ~de_hpd_masked,
> + de_hpd_enables);
> gen11_hpd_detection_setup(dev_priv);
> } else if (IS_GEN9_LP(dev_priv)) {
> bxt_hpd_detection_setup(dev_priv);
> @@ -4157,6 +4182,7 @@ static void icp_irq_postinstall(struct drm_device *dev)
> static int gen11_irq_postinstall(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_uncore *uncore = &dev_priv->uncore;
> u32 gu_misc_masked = GEN11_GU_MISC_GSE;
>
> if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> @@ -4165,7 +4191,7 @@ static int gen11_irq_postinstall(struct drm_device *dev)
> gen11_gt_irq_postinstall(dev_priv);
> gen8_de_irq_postinstall(dev_priv);
>
> - GEN3_IRQ_INIT(GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
> + GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
>
> I915_WRITE(GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE);
>
> @@ -4195,15 +4221,17 @@ static int cherryview_irq_postinstall(struct drm_device *dev)
> static void i8xx_irq_reset(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
>
> i9xx_pipestat_irq_reset(dev_priv);
>
> - GEN2_IRQ_RESET();
> + GEN2_IRQ_RESET(uncore, );
> }
>
> static int i8xx_irq_postinstall(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
> u16 enable_mask;
>
> I915_WRITE16(EMR, ~(I915_ERROR_PAGE_TABLE |
> @@ -4221,7 +4249,7 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
> I915_MASTER_ERROR_INTERRUPT |
> I915_USER_INTERRUPT;
>
> - GEN2_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
> + GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
>
> /* Interrupt setup is already guaranteed to be single-threaded, this is
> * just to make the assert_spin_locked check happy. */
> @@ -4357,6 +4385,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
> static void i915_irq_reset(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
>
> if (I915_HAS_HOTPLUG(dev_priv)) {
> i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
> @@ -4365,12 +4394,13 @@ static void i915_irq_reset(struct drm_device *dev)
>
> i9xx_pipestat_irq_reset(dev_priv);
>
> - GEN3_IRQ_RESET();
> + GEN3_IRQ_RESET(uncore, );
> }
>
> static int i915_irq_postinstall(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
> u32 enable_mask;
>
> I915_WRITE(EMR, ~(I915_ERROR_PAGE_TABLE |
> @@ -4397,7 +4427,7 @@ static int i915_irq_postinstall(struct drm_device *dev)
> dev_priv->irq_mask &= ~I915_DISPLAY_PORT_INTERRUPT;
> }
>
> - GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
> + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
>
> /* Interrupt setup is already guaranteed to be single-threaded, this is
> * just to make the assert_spin_locked check happy. */
> @@ -4468,18 +4498,20 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
> static void i965_irq_reset(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
>
> i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
> I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
>
> i9xx_pipestat_irq_reset(dev_priv);
>
> - GEN3_IRQ_RESET();
> + GEN3_IRQ_RESET(uncore, );
> }
>
> static int i965_irq_postinstall(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_uncore *uncore = &dev_priv->uncore;
> u32 enable_mask;
> u32 error_mask;
>
> @@ -4517,7 +4549,7 @@ static int i965_irq_postinstall(struct drm_device *dev)
> if (IS_G4X(dev_priv))
> enable_mask |= I915_BSD_USER_INTERRUPT;
>
> - GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask);
> + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
>
> /* Interrupt setup is already guaranteed to be single-threaded, this is
> * just to make the assert_spin_locked check happy. */
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/i915: refactor the IRQ init/reset macros
2019-04-09 18:10 ` Ville Syrjälä
@ 2019-04-10 20:45 ` Paulo Zanoni
0 siblings, 0 replies; 14+ messages in thread
From: Paulo Zanoni @ 2019-04-10 20:45 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Em ter, 2019-04-09 às 21:10 +0300, Ville Syrjälä escreveu:
> On Mon, Apr 08, 2019 at 05:37:27PM -0700, Paulo Zanoni wrote:
> > The whole point of having macros here is for the token pasting
> > necessary to automatically have IMR, IIR and IER selected. We don't
> > really need or want all the inlining that happens as a consequence.
> > The good thing about the current code is that it works regardless of
> > the relative offsets between these registers (they change after gen3).
>
> Did you mean "after gen4"? The DE/GT split happened at ilk, and I
> guess that's when the four registers also got reshuffled for some
> reason. Ignoring the funkyness of vlv/chv or course :)
>
You're right. I'll fix the commit message.
> > One thing which we can do is to split the logic of what we do with
> > imr/ier/iir to functions separate from the macros that pick them.
> > That's what we do in this commit. This allows us to get rid of the
> > gen8 duplicates and also all the inlining:
> >
> > add/remove: 2/0 grow/shrink: 0/21 up/down: 383/-6006 (-5623)
> > Function old new delta
> > gen3_irq_reset - 233 +233
> > gen3_irq_init - 150 +150
> > i8xx_irq_postinstall 459 442 -17
> > gen11_irq_postinstall 804 744 -60
> > ironlake_irq_postinstall 450 353 -97
> > vlv_display_irq_postinstall 348 245 -103
> > i965_irq_postinstall 378 275 -103
> > i915_irq_postinstall 333 228 -105
> > gen8_irq_power_well_post_enable 374 210 -164
> > ironlake_irq_reset 397 218 -179
> > vlv_display_irq_reset 616 433 -183
> > i965_irq_reset 374 180 -194
> > cherryview_irq_reset 379 185 -194
> > i915_irq_reset 407 209 -198
> > ibx_irq_reset 332 133 -199
> > gen5_gt_irq_postinstall 533 332 -201
> > gen8_irq_power_well_pre_disable 434 204 -230
> > gen8_gt_irq_postinstall 469 196 -273
> > gen8_de_irq_postinstall 1200 805 -395
> > gen5_gt_irq_reset 471 76 -395
> > gen8_gt_irq_reset 775 99 -676
> > gen8_irq_reset 1100 333 -767
> > gen11_irq_reset 1959 686 -1273
> > Total: Before=2262051, After=2256428, chg -0.25%
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++-------------
> > 1 file changed, 73 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 6454ddc37f8b..a1e7944fb5d0 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -136,36 +136,45 @@ static const u32 hpd_icp[HPD_NUM_PINS] = {
> > [HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP
> > };
> >
> > -/* IIR can theoretically queue up two events. Be paranoid. */
> > -#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); \
> > - 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 GEN3_IRQ_RESET(type) do { \
> > - I915_WRITE(type##IMR, 0xffffffff); \
> > - POSTING_READ(type##IMR); \
> > - I915_WRITE(type##IER, 0); \
> > - I915_WRITE(type##IIR, 0xffffffff); \
> > - POSTING_READ(type##IIR); \
> > - I915_WRITE(type##IIR, 0xffffffff); \
> > - POSTING_READ(type##IIR); \
> > -} while (0)
> > -
> > -#define GEN2_IRQ_RESET(type) do { \
> > - I915_WRITE16(type##IMR, 0xffff); \
> > - POSTING_READ16(type##IMR); \
> > - I915_WRITE16(type##IER, 0); \
> > - I915_WRITE16(type##IIR, 0xffff); \
> > - POSTING_READ16(type##IIR); \
> > - I915_WRITE16(type##IIR, 0xffff); \
> > - POSTING_READ16(type##IIR); \
> > -} while (0)
> > +static void gen3_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr,
> > + i915_reg_t iir, i915_reg_t ier)
> > +{
> > + I915_WRITE(imr, 0xffffffff);
> > + POSTING_READ(imr);
> > +
> > + I915_WRITE(ier, 0);
> > +
> > + /* IIR can theoretically queue up two events. Be paranoid. */
> > + I915_WRITE(iir, 0xffffffff);
> > + POSTING_READ(iir);
> > + I915_WRITE(iir, 0xffffffff);
> > + POSTING_READ(iir);
> > +}
> > +
> > +static void gen2_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr,
> > + i915_reg_t iir, i915_reg_t ier)
> > +{
> > + I915_WRITE16(imr, 0xffff);
> > + POSTING_READ16(imr);
> > +
> > + I915_WRITE16(ier, 0);
> > +
> > + /* IIR can theoretically queue up two events. Be paranoid. */
> > + I915_WRITE16(iir, 0xffff);
> > + POSTING_READ16(iir);
> > + I915_WRITE16(iir, 0xffff);
> > + POSTING_READ16(iir);
> > +}
> > +
> > +#define GEN8_IRQ_RESET_NDX(type, which) \
> > + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \
> > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
> > +
> > +#define GEN3_IRQ_RESET(type) \
> > + gen3_irq_reset(dev_priv, type##IMR, type##IIR, type##IER)
> > +
> > +#define GEN2_IRQ_RESET(type) \
> > + gen2_irq_reset(dev_priv, type##IMR, type##IIR, type##IER)
> >
> > /*
> > * We should clear IMR at preinstall/uninstall, and just check at postinstall.
> > @@ -202,26 +211,40 @@ static void gen2_assert_iir_is_zero(struct drm_i915_private *dev_priv,
> > POSTING_READ16(reg);
> > }
> >
> > -#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \
> > - gen3_assert_iir_is_zero(dev_priv, GEN8_##type##_IIR(which)); \
> > - I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \
> > - I915_WRITE(GEN8_##type##_IMR(which), (imr_val)); \
> > - POSTING_READ(GEN8_##type##_IMR(which)); \
> > -} while (0)
> > -
> > -#define GEN3_IRQ_INIT(type, imr_val, ier_val) do { \
> > - gen3_assert_iir_is_zero(dev_priv, type##IIR); \
> > - I915_WRITE(type##IER, (ier_val)); \
> > - I915_WRITE(type##IMR, (imr_val)); \
> > - POSTING_READ(type##IMR); \
> > -} while (0)
> > -
> > -#define GEN2_IRQ_INIT(type, imr_val, ier_val) do { \
> > - gen2_assert_iir_is_zero(dev_priv, type##IIR); \
> > - I915_WRITE16(type##IER, (ier_val)); \
> > - I915_WRITE16(type##IMR, (imr_val)); \
> > - POSTING_READ16(type##IMR); \
> > -} while (0)
> > +static void gen3_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr,
> > + i915_reg_t iir, i915_reg_t ier, u32 imr_val,
> > + u32 ier_val)
>
> Maybe we should order these more like this
> foo(imr, imr_value, ier, ier_value, iir) ?
Sure. I'll also try to add a few extra line breaks to the calls to try
to make the arguments a little more readable.
>
> Could make a bit more obvious which values goes to which register. But
> I suppose as long as they're always wrapped in those macros it doesn't
> really matter.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks a lot!
>
> > +{
> > + gen3_assert_iir_is_zero(dev_priv, iir);
> > +
> > + I915_WRITE(ier, ier_val);
> > + I915_WRITE(imr, imr_val);
> > + POSTING_READ(imr);
> > +}
> > +
> > +static void gen2_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr,
> > + i915_reg_t iir, i915_reg_t ier, u32 imr_val,
> > + u32 ier_val)
> > +{
> > + gen2_assert_iir_is_zero(dev_priv, iir);
> > +
> > + I915_WRITE16(ier, ier_val);
> > + I915_WRITE16(imr, imr_val);
> > + POSTING_READ16(imr);
> > +}
> > +
> > +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
> > + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \
> > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> > + imr_val, ier_val)
> > +
> > +#define GEN3_IRQ_INIT(type, imr_val, ier_val) \
> > + gen3_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \
> > + ier_val)
> > +
> > +#define GEN2_IRQ_INIT(type, imr_val, ier_val) \
> > + gen2_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \
> > + ier_val)
> >
> > static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
> > static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
> > --
> > 2.20.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ✗ Fi.CI.CHECKPATCH: warning for IRQ initialization debloat and conversion to uncore
2019-04-09 18:20 ` Ville Syrjälä
@ 2019-04-10 22:53 ` Paulo Zanoni
2019-04-11 11:04 ` Ville Syrjälä
0 siblings, 1 reply; 14+ messages in thread
From: Paulo Zanoni @ 2019-04-10 22:53 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Em ter, 2019-04-09 às 21:20 +0300, Ville Syrjälä escreveu:
> On Tue, Apr 09, 2019 at 10:34:22AM -0700, Paulo Zanoni wrote:
> > Em ter, 2019-04-09 às 00:44 +0000, Patchwork escreveu:
> > > == Series Details ==
> > >
> > > Series: IRQ initialization debloat and conversion to uncore
> > > URL : https://patchwork.freedesktop.org/series/59202/
> > > State : warning
> > >
> > > == Summary ==
> > >
> > > $ dim checkpatch origin/drm-tip
> > > 7f73d1fe31bb drm/i915: refactor the IRQ init/reset macros
> > > -:114: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > > #114: FILE: drivers/gpu/drm/i915/i915_irq.c:169:
> > > +#define GEN8_IRQ_RESET_NDX(type, which) \
> > > + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \
> > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
> > >
> > > -:172: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > > #172: FILE: drivers/gpu/drm/i915/i915_irq.c:236:
> > > +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
> > > + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \
> > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> > > + imr_val, ier_val)
> > >
> > > total: 0 errors, 0 warnings, 2 checks, 135 lines checked
> > > 82160241d80f drm/i915: convert the IRQ initialization functions to intel_uncore
> > > 8c1c76059a41 drm/i915: fully convert the IRQ initialization macros to intel_uncore
> > > -:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > > #24: FILE: drivers/gpu/drm/i915/i915_irq.c:169:
> > > +#define GEN8_IRQ_RESET_NDX(uncore, type, which) \
> > > + gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \
> > > GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
> > >
> > > -:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > > #46: FILE: drivers/gpu/drm/i915/i915_irq.c:236:
> > > +#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \
> > > + gen3_irq_init((uncore), GEN8_##type##_IMR(which), \
> > > GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> > > imr_val, ier_val)
> >
> > The whiches are not really a regression, but OK we can deal with them
> > to make the robots happy.
> >
> > > -:401: ERROR:SPACING: space prohibited before that close parenthesis ')'
> > > #401: FILE: drivers/gpu/drm/i915/i915_irq.c:4228:
> > > + GEN2_IRQ_RESET(uncore, );
> > >
> > > -:416: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> > > #416: FILE: drivers/gpu/drm/i915/i915_irq.c:4252:
> > > + GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
> > > ^
> > >
> > > -:433: ERROR:SPACING: space prohibited before that close parenthesis ')'
> > > #433: FILE: drivers/gpu/drm/i915/i915_irq.c:4397:
> > > + GEN3_IRQ_RESET(uncore, );
> > >
> > > -:448: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> > > #448: FILE: drivers/gpu/drm/i915/i915_irq.c:4430:
> > > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
> > > ^
> > >
> > > -:464: ERROR:SPACING: space prohibited before that close parenthesis ')'
> > > #464: FILE: drivers/gpu/drm/i915/i915_irq.c:4508:
> > > + GEN3_IRQ_RESET(uncore, );
> > >
> > > -:479: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> > > #479: FILE: drivers/gpu/drm/i915/i915_irq.c:4552:
> > > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
> >
> > For these ones I really think the spaces help. I would love to read
> > some opinions. Perhaps some comment like /* paste token here */ would
> > help make the code more readable and could help silence checkpatch.
> > Opinions?
>
> Or maybe rename the registers to eg. I9XX_IIR?
That makes more sense. We use these regs on gen2 too, so I suppose
I8XX_IIR (or GEN2_IIR) would make more sense. OTOH it would break our
current naming rule.
I'll submit v2 soon. Thanks.
>
> > > ^
> > >
> > > total: 6 errors, 0 warnings, 2 checks, 432 lines checked
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ✗ Fi.CI.CHECKPATCH: warning for IRQ initialization debloat and conversion to uncore
2019-04-10 22:53 ` Paulo Zanoni
@ 2019-04-11 11:04 ` Ville Syrjälä
0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2019-04-11 11:04 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Wed, Apr 10, 2019 at 03:53:05PM -0700, Paulo Zanoni wrote:
> Em ter, 2019-04-09 às 21:20 +0300, Ville Syrjälä escreveu:
> > On Tue, Apr 09, 2019 at 10:34:22AM -0700, Paulo Zanoni wrote:
> > > Em ter, 2019-04-09 às 00:44 +0000, Patchwork escreveu:
> > > > == Series Details ==
> > > >
> > > > Series: IRQ initialization debloat and conversion to uncore
> > > > URL : https://patchwork.freedesktop.org/series/59202/
> > > > State : warning
> > > >
> > > > == Summary ==
> > > >
> > > > $ dim checkpatch origin/drm-tip
> > > > 7f73d1fe31bb drm/i915: refactor the IRQ init/reset macros
> > > > -:114: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > > > #114: FILE: drivers/gpu/drm/i915/i915_irq.c:169:
> > > > +#define GEN8_IRQ_RESET_NDX(type, which) \
> > > > + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \
> > > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
> > > >
> > > > -:172: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > > > #172: FILE: drivers/gpu/drm/i915/i915_irq.c:236:
> > > > +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \
> > > > + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \
> > > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> > > > + imr_val, ier_val)
> > > >
> > > > total: 0 errors, 0 warnings, 2 checks, 135 lines checked
> > > > 82160241d80f drm/i915: convert the IRQ initialization functions to intel_uncore
> > > > 8c1c76059a41 drm/i915: fully convert the IRQ initialization macros to intel_uncore
> > > > -:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > > > #24: FILE: drivers/gpu/drm/i915/i915_irq.c:169:
> > > > +#define GEN8_IRQ_RESET_NDX(uncore, type, which) \
> > > > + gen3_irq_reset((uncore), GEN8_##type##_IMR(which), \
> > > > GEN8_##type##_IIR(which), GEN8_##type##_IER(which))
> > > >
> > > > -:46: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'which' - possible side-effects?
> > > > #46: FILE: drivers/gpu/drm/i915/i915_irq.c:236:
> > > > +#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \
> > > > + gen3_irq_init((uncore), GEN8_##type##_IMR(which), \
> > > > GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \
> > > > imr_val, ier_val)
> > >
> > > The whiches are not really a regression, but OK we can deal with them
> > > to make the robots happy.
> > >
> > > > -:401: ERROR:SPACING: space prohibited before that close parenthesis ')'
> > > > #401: FILE: drivers/gpu/drm/i915/i915_irq.c:4228:
> > > > + GEN2_IRQ_RESET(uncore, );
> > > >
> > > > -:416: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> > > > #416: FILE: drivers/gpu/drm/i915/i915_irq.c:4252:
> > > > + GEN2_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
> > > > ^
> > > >
> > > > -:433: ERROR:SPACING: space prohibited before that close parenthesis ')'
> > > > #433: FILE: drivers/gpu/drm/i915/i915_irq.c:4397:
> > > > + GEN3_IRQ_RESET(uncore, );
> > > >
> > > > -:448: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> > > > #448: FILE: drivers/gpu/drm/i915/i915_irq.c:4430:
> > > > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
> > > > ^
> > > >
> > > > -:464: ERROR:SPACING: space prohibited before that close parenthesis ')'
> > > > #464: FILE: drivers/gpu/drm/i915/i915_irq.c:4508:
> > > > + GEN3_IRQ_RESET(uncore, );
> > > >
> > > > -:479: ERROR:SPACING: space prohibited before that ',' (ctx:WxW)
> > > > #479: FILE: drivers/gpu/drm/i915/i915_irq.c:4552:
> > > > + GEN3_IRQ_INIT(uncore, , dev_priv->irq_mask, enable_mask);
> > >
> > > For these ones I really think the spaces help. I would love to read
> > > some opinions. Perhaps some comment like /* paste token here */ would
> > > help make the code more readable and could help silence checkpatch.
> > > Opinions?
> >
> > Or maybe rename the registers to eg. I9XX_IIR?
>
> That makes more sense. We use these regs on gen2 too, so I suppose
> I8XX_IIR (or GEN2_IIR) would make more sense. OTOH it would break our
> current naming rule.
I tend to use i9xx to indicate anything gmch, and sometimes
it even means pre-skl :/ Not the best naming scheme perhaps
but I've not been able to come up with anything better.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-04-11 11:04 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-09 0:37 [PATCH 0/3] IRQ initialization debloat and conversion to uncore Paulo Zanoni
2019-04-09 0:37 ` [PATCH 1/3] drm/i915: refactor the IRQ init/reset macros Paulo Zanoni
2019-04-09 18:10 ` Ville Syrjälä
2019-04-10 20:45 ` Paulo Zanoni
2019-04-09 0:37 ` [PATCH 2/3] drm/i915: convert the IRQ initialization functions to intel_uncore Paulo Zanoni
2019-04-09 0:37 ` [PATCH 3/3] drm/i915: fully convert the IRQ initialization macros " Paulo Zanoni
2019-04-09 20:21 ` Ville Syrjälä
2019-04-09 0:44 ` ✗ Fi.CI.CHECKPATCH: warning for IRQ initialization debloat and conversion to uncore Patchwork
2019-04-09 17:34 ` Paulo Zanoni
2019-04-09 18:20 ` Ville Syrjälä
2019-04-10 22:53 ` Paulo Zanoni
2019-04-11 11:04 ` Ville Syrjälä
2019-04-09 1:07 ` ✓ Fi.CI.BAT: success " Patchwork
2019-04-09 7:32 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.