Re: [PATCH] media: imx-jpeg: Fix JPEG encoder ready race condition

[Marek Vasut <marek.vasut@mailbox.org>]

On 8/21/25 9:06 AM, Ming Qian(OSS) wrote:
> Hi Marek,

Hi,

> On 8/21/2025 12:29 AM, Marek Vasut wrote:
>> [You don't often get email from marek.vasut@mailbox.org. Learn why 
>> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> The current mxc_jpeg_job_ready() implementation works for JPEG decode
>> side of this IP, it does not work at all for the JPEG encode side. The
>> JPEG encode side does not change ctx->source_change at all, therefore
>> the mxc_jpeg_source_change() always returns right away and the encode
>> side somehow works.
>>
>> However, this is susceptible to a race condition between 
>> mxc_jpeg_dec_irq()
>> and mxc_jpeg_start_streaming(), where mxc_jpeg_start_streaming() might
>> start decoding another frame before mxc_jpeg_dec_irq() indicates 
>> completion
>> of encoding of current frame. Add new state, MXC_JPEG_ENC_DONE, which is
>> set in three locations, first when streaming starts to indicate the 
>> encoder
>> is ready to start processing a frame, second in mxc_jpeg_dec_irq() 
>> when the
>> encoder finishes encoding current frame, and third in 
>> mxc_jpeg_dec_irq() in
>> case of an error so the encoder can proceed with encoding another frame.
>>
>> Update mxc_jpeg_job_ready() to check whether the encoder is in this new
>> MXC_JPEG_ENC_DONE state before reporting the encoder is ready to encode
>> new frame.
>>
> 
> The jpeg encoder and jpeg decoder are 2 different devices, so they won't
> compete with each other.

I know.

> For encoder, we always want mxc_jpeg_job_ready() to return true.

For encoder, mxc_jpeg_job_ready() currently returns !ctx->source_change 
, which is only set by decoder side of the driver. This seems wrong.

> And v4l2_m2m has ensured that there will be no race condition between
> mxc_jpeg_dec_irq() and mxc_jpeg_start_streaming().
> 
> After v4l2_m2m_job_finish() is called in the mxc_jpeg_dec_irq(),
> then it is possible to start encoding the next frame.
This part would be true if the IRQ function couldn't be called by 
anything except end of configuration or end of encoding, but it can be 
triggered by other things, like AXI READ error IRQ.