[Intel-xe] [PATCH v2] drm/xe/irq: Clear GFX_MSTR_IRQ as part of IRQ reset

[Lucas De Marchi]

On Wed, Sep 20, 2023 at 04:45:35PM -0300, Gustavo Sousa wrote:
>Starting with Xe_LP+, GFX_MSTR_IRQ contains status bits that have W1C
>behavior. If we do not properly reset them, we would miss delivery of
>interrupts if a pending bit is set when enabling IRQs.
>
>As an example, the display part of our probe routine contains paths
>where we wait for vblank interrupts. If a display interrupt was already
>pending when enabling IRQs, we would time out waiting for the vblank.
>
>That in fact happened recently when modprobing Xe on a Lunar Lake with a
>specific configuration; and that's how we found out we were missing this
>step in the IRQ enabling logic.
>
>Fix the issue by clearing GFX_MSTR_IRQ as part of the IRQ reset.
>
>v2:
>  - Make resetting GFX_MSTR_IRQ be the last step to avoid bit
>    re-latching. (Ville)
>
>BSpec: 50875, 54028, 62357
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Matt Roper <matthew.d.roper at intel.com>
>Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>---
> drivers/gpu/drm/xe/xe_irq.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>index ccb934f8fa34..cb03f40df272 100644
>--- a/drivers/gpu/drm/xe/xe_irq.c
>+++ b/drivers/gpu/drm/xe/xe_irq.c
>@@ -519,6 +519,13 @@ static void dg1_irq_reset(struct xe_tile *tile)
> 	mask_and_disable(tile, PCU_IRQ_OFFSET);
> }
>
>+static void dg1_irq_reset_mstr(struct xe_tile *tile)
>+{
>+	struct xe_gt *mmio = tile->primary_gt;
>+
>+	xe_mmio_write32(mmio, GFX_MSTR_IRQ, ~0);
>+}
>+
> static void xe_irq_reset(struct xe_device *xe)
> {
> 	struct xe_tile *tile;
>@@ -534,6 +541,15 @@ static void xe_irq_reset(struct xe_device *xe)
> 	tile = xe_device_get_root_tile(xe);
> 	mask_and_disable(tile, GU_MISC_IRQ_OFFSET);
> 	xe_display_irq_reset(xe);
>+
>+	/*
>+	 * The tile's top-level status register should be the last one
>+	 * to be reset to avoid possible bit re-latching from lower
>+	 * level interrupts.
>+	 */
>+	for_each_tile(tile, xe, id)
>+		if (GRAPHICS_VERx100(xe) >= 1210)

Better with the version checked swapped:
	
	if (GRAPHICS_VERx100(xe) >= 1210)
		for_each_tile(tile, xe, id)
			dg1_irq_reset_mstr(tile);

The code in the beginning of this function is about choosing between 2
versions of the function, which is not the case here

Lucas De Marchi

>+			dg1_irq_reset_mstr(tile);
> }
>
> static void xe_irq_postinstall(struct xe_device *xe)
>-- 
>2.42.0
>