Re: [PATCH] media: cedrus: Remove global IRQ spin lock from the driver

From: Maxime Ripard
Date: Fri Nov 16 2018 - 05:47:41 EST


On Thu, Nov 15, 2018 at 03:39:55PM +0100, Paul Kocialkowski wrote:
> We initially introduced a spin lock to ensure that the VPU registers
> are not accessed concurrently between our setup function and IRQ
> handler. Because the V4L2 M2M API only allows one job to run at a time
> and our jobs are completed following the IRQ handler, there is actually
> no chance of an interrupt happening while programming the VPU registers.

That's not entirely true. There's no chance that the interrupt
signaling the end of the frame decoding can happen.

However, spurious interrupts can happen at any point in time as soon
as you have the interrupts enabled.

> In addition, holding a spin lock is problematic when doing more than
> simply configuring the registers in the setup operation. H.265 support
> currently involves a CMA allocation in that step, which is not
> compatible with an atomic context.

That's not really true either. Any allocation can be done with
GFP_ATOMIC or GFP_NOWAIT, and be compatible with an atomic
context. Whether it's something we want is a different story :)

And since the h265 code isn't upstream, I'm not really sure it's
relevant to mention it here.

> As a result, remove the global IRQ spin lock.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx>

Acked-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Attachment: signature.asc
Description: PGP signature