Re: [PATCH 08/10] drm/panthor: Automatically enable interrupts in panthor_fw_wait_acks()

[Boris Brezillon <boris.brezillon@collabora.com>]

On Fri, 1 May 2026 15:20:17 +0100
Steven Price <steven.price@arm.com> wrote:

> On 29/04/2026 10:38, Boris Brezillon wrote:
> > Rather than assuming an interrupt is always expected for request
> > acks, temporarily enable the relevant interrupts when the polling-wait
> > failed. This should hopefully reduce the number of interrupts the CPU
> > has to process.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>  
> 
> It seems to work, although I'm lightly uneasy about this because I'm not
> entirely sure whether the FW will immediately see the updates to
> ack_irq_mask and therefore whether there's a possibility to miss an
> event and be stuck waiting for the timeout.
> 
> Memory models are not my strong point, OpenAI tells me the sequence
> should be something like:
> 
>   scoped_guard(spinlock_irqsave, lock) {
>   	u32 ack_irq_mask = READ_ONCE(*ack_irq_mask_ptr);
> 
>   	WRITE_ONCE(*ack_irq_mask_ptr, ack_irq_mask | req_mask);
>   }

Is this really needed? In which situation would the compiler/CPU decide
to re-order this read_update_modify sequence?

> 
>   /*
>    * The FW interface can be mapped write-combine/Normal-NC.

I'm not too sure I see what the non-cached property has to do with it.
If it was cached we would still need this memory barrier, and in
addition, we'd need a cache flush if the FW is not IO-coherent.

>Make sure the
>    * IRQ mask update is visible to the FW before sleeping waiting for
> the IRQ.
>    */
>   wmb();
> 
> Which seems plausible. But I've long ago learnt that plausible doesn't
> mean much when dealing with memory models!

Yeah, I'm not too sure. I was honestly expecting the spinlock guard to
act as a memory barrier already, but maybe it's not enough.