Re: [PATCH v2 3/3] media: imx-jpeg: Check decoding is ongoing for motion-jpeg

["Ming Qian(OSS)" <ming.qian@oss.nxp.com>]

Hi Frank,

On 2025/3/28 22:45, Frank Li wrote:
> On Fri, Mar 28, 2025 at 02:30:52PM +0800, ming.qian@oss.nxp.com wrote:
>> From: Ming Qian <ming.qian@oss.nxp.com>
>>
>> To support decoding motion-jpeg without DHT, driver will try to decode a
>> pattern jpeg before actual jpeg frame by use of linked descriptors
>> (This is called "repeat mode"), then the DHT in the pattern jpeg can be
>> used for decoding the motion-jpeg.
>>
>> In other words, 2 frame done interrupts will be triggered, driver will
>> ignore the first interrupt,
> 
> Does any field in linked descriptors to control if issue irq? Generally
> you needn't enable first descriptors's irq and only enable second one.
> 

Unfortunately, we cannot configure interrupts for each descriptor.
So we can't only enable the second irq.


>> and wait for the second interrupt.
>> If the resolution is small, and the 2 interrupts may be too close,
> 
> It also possible two irqs combine to 1 irqs. If I understand correct, your
> irq handle only handle one descriptors per irq.
> 
> Another words,
> 
> If second irq already happen just before 1,
> 
> 1. dec_ret = readl(reg + MXC_SLOT_OFFSET(slot, SLOT_STATUS));
> 2. writel(dec_ret, reg + MXC_SLOT_OFFSET(slot, SLOT_STATUS)); /* w1c */
> 
> Does your driver wait forever because no second irq happen?
> 
> Frank

Before this patch, the answer is yes, the driver will wait 2 seconds
until timeout.
In fact, this is the problem that this patch wants to avoid.
Now I think there are 3 cases for motion-jpeg decoding:
1. The second irq happens before the first irq status check, the 
on-going check
help to hanle this case.
2. The second irq happens after the first irq status check, but before
on-going check, this on-going check can help handle it, fisnish the
current decoding and reset the jpeg decoder.
3. The second irq happens after the on-going check, this is the normal
process before. No additional processing required.

Thanks,
Ming

>>
>> when driver is handling the first interrupt, two frames are done, then
>> driver will fail to wait for the second interrupt.
>>
>> In such case, driver can check whether the decoding is still ongoing,
>> if not, just done the current decoding.
>>
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>> ---
>>   .../media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h |  1 +
>>   .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 20 ++++++++++++++++++-
>>   2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
>> index d579c804b047..adb93e977be9 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
>> @@ -89,6 +89,7 @@
>>   /* SLOT_STATUS fields for slots 0..3 */
>>   #define SLOT_STATUS_FRMDONE			(0x1 << 3)
>>   #define SLOT_STATUS_ENC_CONFIG_ERR		(0x1 << 8)
>> +#define SLOT_STATUS_ONGOING			(0x1 << 31)
>>
>>   /* SLOT_IRQ_EN fields TBD */
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index 45705c606769..e6bb45633a19 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -910,6 +910,23 @@ static u32 mxc_jpeg_get_plane_size(struct mxc_jpeg_q_data *q_data, u32 plane_no)
>>   	return size;
>>   }
>>
>> +static bool mxc_dec_is_ongoing(struct mxc_jpeg_ctx *ctx)
>> +{
>> +	struct mxc_jpeg_dev *jpeg = ctx->mxc_jpeg;
>> +	u32 curr_desc;
>> +	u32 slot_status;
>> +
>> +	slot_status = readl(jpeg->base_reg + MXC_SLOT_OFFSET(ctx->slot, SLOT_STATUS));
>> +	curr_desc = readl(jpeg->base_reg + MXC_SLOT_OFFSET(ctx->slot, SLOT_CUR_DESCPT_PTR));
>> +
>> +	if (curr_desc == jpeg->slot_data.cfg_desc_handle)
>> +		return true;
>> +	if (slot_status & SLOT_STATUS_ONGOING)
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>>   static irqreturn_t mxc_jpeg_dec_irq(int irq, void *priv)
>>   {
>>   	struct mxc_jpeg_dev *jpeg = priv;
>> @@ -979,7 +996,8 @@ static irqreturn_t mxc_jpeg_dec_irq(int irq, void *priv)
>>   		mxc_jpeg_enc_mode_go(dev, reg, mxc_jpeg_is_extended_sequential(q_data->fmt));
>>   		goto job_unlock;
>>   	}
>> -	if (jpeg->mode == MXC_JPEG_DECODE && jpeg_src_buf->dht_needed) {
>> +	if (jpeg->mode == MXC_JPEG_DECODE && jpeg_src_buf->dht_needed &&
>> +	    mxc_dec_is_ongoing(ctx)) {
>>   		jpeg_src_buf->dht_needed = false;
>>   		dev_dbg(dev, "Decoder DHT cfg finished. Start decoding...\n");
>>   		goto job_unlock;
>> --
>> 2.43.0-rc1
>>