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

[Gustavo Sousa]

Quoting Ville Syrjälä (2023-09-20 17:00:07-03:00)
>On Wed, Sep 20, 2023 at 04:53:52PM -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.
>> 
>> Avoid the potential issue by clearing GFX_MSTR_IRQ as part of the IRQ
>> reset.
>> 
>> v2:
>>   - Move logic from gen11_gt_irq_reset() to dg1_irq_reset(). (Matt)
>> 
>> BSpec: 50875, 54028
>> Cc: Matt Roper <matthew.d.roper at intel.com>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 1bfcfbe6e30b..8130f043693b 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -751,6 +751,8 @@ static void dg1_irq_reset(struct drm_i915_private *dev_priv)
>>  
>>          GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_);
>>          GEN3_IRQ_RESET(uncore, GEN8_PCU_);
>> +
>> +        intel_uncore_write(uncore, GEN11_GFX_MSTR_IRQ, ~0);
>
>Did you confirm that it's not double buffered?

Ah, sorry... I forgot to mention on the original thread where you asked:

  - BSpec 50875 and 66434 (for Xe2) does not say that GFX_MSTR_IRQ is
    double buffered. In fact they mention the fact that display IIR
    registers are double buffered and require multiple writes to fully
    clear, but does not say the same about GFX_MSTR_IRQ.

  - Also, BSpec 54028 does not mention that the register is double
    buffered.

Based on the above, I'm assuming it is not double buffered.

Should I check other sources? Is there some sort of runtime check that
can be done?

--
Gustavo Sousa

>
>>  }
>>  
>>  static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
>> -- 
>> 2.42.0
>
>-- 
>Ville Syrjälä
>Intel