[PATCH] drm/xe: Make irq enabled flag atomic

[Levi, Ilia]

On 12/12/2024 1:29, Rodrigo Vivi wrote:
> On Wed, Dec 11, 2024 at 01:58:08PM -0600, Lucas De Marchi wrote:
>> On Wed, Dec 11, 2024 at 01:13:49PM -0500, Rodrigo Vivi wrote:
>>> On Tue, Dec 10, 2024 at 07:35:06PM +0200, Ilia Levi wrote:
>>>> The irq.enabled flag was protected by a spin lock (irq.lock).
>>>> By making it atomic we no longer need to wait for the spin lock in
>>>> irq handlers. This will become especially useful for MSI-X irq
>>>> handlers to prevent lock contention between different interrupts.
>>>>
>>>> Signed-off-by: Ilia Levi <ilia.levi at intel.com>
>>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>>
>>> pushing soon to drm-xe-next. Thanks for the patch
>>>
>> I just saw the commit in the tree and was wondering... why exactly do we
>> need that flag checked in the irq handlers?  why can't we simply do
>> a) turn off irq
>> b) synchronize_irq()
>>
>> kind of like xe_irq_suspend() is doing, but in the opposite order. It
>> seems this was just copy-pasted over and over or am I missing a
>> synchronization here?
> A very good question. We likely don't need that check there.
> Ilia, perhaps you could give it a try along with your upcoming MSX IRQ
> series?
I think turning the irqs off while a handler is running is problematic.

Specifically, the thread that does suspend will disable interrupts and the handler will re-enable them:
Thread: xe_irq_suspend -> xe_irq_reset -> dg1_irq_reset -> dg1_intr_disable
IRQ handler: dg1_irq_handler -> dg1_intr_enable

What do you think?
>> Lucas De Marchi