Re: [PATCH v3 19/19] drm/i915: Get rid of ibx_irq_pre_postinstall()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 28, 2020 at 11:33:23PM +0200, Ville Syrjälä wrote:
From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

ibx_irq_pre_postinstall() looks totally pointless. We can just
init both SDEIMR and SDEIER at the same time before enabling the
master intererupt. It's equally racy as the other order due

master interrupt

to doing all of this from the postinstall stage with the interrupt
handler already in place. That is, safe with MSI but racy with
shared legacy interrupts. Fortunately we should have MSI on all ilk+.

Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>

Lucas De Marchi

---
drivers/gpu/drm/i915/i915_irq.c | 46 ++++++++++++---------------------
1 file changed, 17 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 95268fca2fbc..fdd132e2ec76 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2910,24 +2910,6 @@ static void ibx_irq_reset(struct drm_i915_private *dev_priv)
		I915_WRITE(SERR_INT, 0xffffffff);
}

-/*
- * SDEIER is also touched by the interrupt handler to work around missed PCH
- * interrupts. Hence we can't update it after the interrupt handler is enabled -
- * instead we unconditionally enable all PCH interrupt sources here, but then
- * only unmask them as needed with SDEIMR.
- *
- * This function needs to be called before interrupts are enabled.
- */
-static void ibx_irq_pre_postinstall(struct drm_i915_private *dev_priv)
-{
-	if (HAS_PCH_NOP(dev_priv))
-		return;
-
-	drm_WARN_ON(&dev_priv->drm, I915_READ(SDEIER) != 0);
-	I915_WRITE(SDEIER, 0xffffffff);
-	POSTING_READ(SDEIER);
-}
-
static void vlv_display_irq_reset(struct drm_i915_private *dev_priv)
{
	struct intel_uncore *uncore = &dev_priv->uncore;
@@ -3545,8 +3527,20 @@ static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv)
	bxt_hpd_detection_setup(dev_priv);
}

+/*
+ * SDEIER is also touched by the interrupt handler to work around missed PCH
+ * interrupts. Hence we can't update it after the interrupt handler is enabled -
+ * instead we unconditionally enable all PCH interrupt sources here, but then
+ * only unmask them as needed with SDEIMR.
+ *
+ * Note that we currently do this after installing the interrupt handler,
+ * but before we enable the master interrupt. That should be sufficient
+ * to avoid races with the irq handler, assuming we have MSI. Shared legacy
+ * interrupts could still race.
+ */
static void ibx_irq_postinstall(struct drm_i915_private *dev_priv)
{
+	struct intel_uncore *uncore = &dev_priv->uncore;
	u32 mask;

	if (HAS_PCH_NOP(dev_priv))
@@ -3559,8 +3553,7 @@ static void ibx_irq_postinstall(struct drm_i915_private *dev_priv)
	else
		mask = SDE_GMBUS_CPT;

-	gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR);
-	I915_WRITE(SDEIMR, ~mask);
+	GEN3_IRQ_INIT(uncore, SDE, ~mask, 0xffffffff);
}

static void ilk_irq_postinstall(struct drm_i915_private *dev_priv)
@@ -3593,14 +3586,12 @@ static void ilk_irq_postinstall(struct drm_i915_private *dev_priv)

	dev_priv->irq_mask = ~display_mask;

-	ibx_irq_pre_postinstall(dev_priv);
+	ibx_irq_postinstall(dev_priv);

	gen5_gt_irq_postinstall(&dev_priv->gt);

	GEN3_IRQ_INIT(uncore, DE, dev_priv->irq_mask,
		      display_mask | extra_mask);
-
-	ibx_irq_postinstall(dev_priv);
}

void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
@@ -3725,15 +3716,12 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)

static void gen8_irq_postinstall(struct drm_i915_private *dev_priv)
{
-	if (HAS_PCH_SPLIT(dev_priv))
-		ibx_irq_pre_postinstall(dev_priv);
-
-	gen8_gt_irq_postinstall(&dev_priv->gt);
-	gen8_de_irq_postinstall(dev_priv);
-
	if (HAS_PCH_SPLIT(dev_priv))
		ibx_irq_postinstall(dev_priv);

+	gen8_gt_irq_postinstall(&dev_priv->gt);
+	gen8_de_irq_postinstall(dev_priv);
+
	gen8_master_intr_enable(dev_priv->uncore.regs);
}

--
2.26.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux