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

[Gustavo Sousa]

Quoting Matt Roper (2023-09-19 18:15:06-03:00)
>On Tue, Sep 19, 2023 at 11:41:10AM -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.
>> 
>> BSpec: 50875, 54028, 62357
>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_irq.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>> index ccb934f8fa34..3746e9204e48 100644
>> --- a/drivers/gpu/drm/xe/xe_irq.c
>> +++ b/drivers/gpu/drm/xe/xe_irq.c
>> @@ -456,6 +456,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
>>  
>>  static void gt_irq_reset(struct xe_tile *tile)
>
>As noted on the i915 patch, the bits in this register correspond to
>tiles rather than GTs.  And in Xe we do a much better job of handling
>the two separately.  You probably want to move this to dg1_irq_reset()

gt_irq_reset() seems to end up being called for each tile, but, yeah,
dg1_irq_reset() looks like a much better and natural place to do this.
Thanks!

>and only reset the bit that corresponds to the specific tile that was
>passed as a parameter.

Not sure I follow here. Shouldn't the bits in that register already be
specific to the tile?

I think the per-tile bits are in MSTR_TILE_INTR (0x190008), and they are
already handled by dg1_intr_disable().

--
Gustavo Sousa

>
>
>Matt
>
>>  {
>> +        struct xe_device *xe = tile_to_xe(tile);
>>          struct xe_gt *mmio = tile->primary_gt;
>>  
>>          u32 ccs_mask = xe_hw_engine_mask_per_class(tile->primary_gt,
>> @@ -463,6 +464,9 @@ static void gt_irq_reset(struct xe_tile *tile)
>>          u32 bcs_mask = xe_hw_engine_mask_per_class(tile->primary_gt,
>>                                                     XE_ENGINE_CLASS_COPY);
>>  
>> +        if (GRAPHICS_VERx100(xe) >= 1210)
>> +                xe_mmio_write32(mmio, GFX_MSTR_IRQ, ~0);
>> +
>>          /* Disable RCS, BCS, VCS and VECS class engines. */
>>          xe_mmio_write32(mmio, RENDER_COPY_INTR_ENABLE, 0);
>>          xe_mmio_write32(mmio, VCS_VECS_INTR_ENABLE, 0);
>> -- 
>> 2.42.0
>> 
>
>-- 
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation