[Intel-gfx] [PATCH v3 19/19] drm/i915: Get rid of ibx_irq_pre_postinstall()

[Lucas De Marchi]

On Wed, Oct 28, 2020 at 11:33:23PM +0200, Ville Syrjälä wrote:
>From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
>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 at linux.intel.com>

Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

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 at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx