| Message ID | 20250411074347.809-5-ming.qian@oss.nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: imx-jpeg: Fix some motion-jpeg decoding | expand |
On Fri, Apr 11, 2025 at 03:43:43PM +0800, ming.qian@oss.nxp.com wrote: > From: Ming Qian <ming.qian@oss.nxp.com> > > As the first frame in "repeat-mode" is the pattern, the pattern done > interrupt is ignored by the driver. With small resolution bitstreams, > the interrupts might fire too quickly and hardware combine two irqs to > once because irq handle have latency. Thus the driver might miss the > frame decode done interrupt from the first actual frame. > > In order to avoid the driver wait for the frame done interrupt that has > been combined to the pattern done interrupt and been ignored, driver > will check the curr_desc and slot_status registers to figure out if the > decoding of actual frame is finished or not. > > Firstly we check the curr_desc register, > - if it is still pointing to the pattern descriptor, the second actual > frame is not started, we can wait for its frame-done interrupt. > - if the curr_desc has pointed to the frame descriptor, then we check the > ongoing bit of slot_status register. > - if the ongoing bit is set to 1, the decoding of the actual frame is not > finished, we can wait for its frame-done interrupt. > - if the ongoing bit is set to 0, the decoding of the actual frame is > finished, we can't wait for the second interrupt, but mark it as done. > > But there is still a small problem, that the curr_desc and slot_status > registers are not synchronous. curr_desc is updated when the > next_descpt_ptr is loaded, but the ongoing bit of slot_status is set > after the 32 bytes descriptor is loaded, there will be a short time > interval in between, which may cause fake false. Consider read register > is quite slow compared with IP read 32byte from memory, read twice > slot_status can avoid this situation. > > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> Reviewed-by: Frank Li <Frank.Li@nxp.com> > --- > .../media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h | 1 + > .../media/platform/nxp/imx-jpeg/mxc-jpeg.c | 31 ++++++++++++++++++- > 2 files changed, 31 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..4346dcdc9697 100644 > --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c > +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c > @@ -910,6 +910,34 @@ 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; > + > + 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; > + > + slot_status = readl(jpeg->base_reg + MXC_SLOT_OFFSET(ctx->slot, SLOT_STATUS)); > + if (slot_status & SLOT_STATUS_ONGOING) > + return true; > + > + /* > + * The curr_desc register is updated when next_descpt_ptr is loaded, > + * the ongoing bit of slot_status is set when the 32 bytes descriptor is loaded. > + * So there will be a short time interval in between, which may cause fake false. > + * Consider read register is quite slow compared with IP read 32byte from memory, > + * read twice slot_status can avoid this situation. > + */ > + slot_status = readl(jpeg->base_reg + MXC_SLOT_OFFSET(ctx->slot, SLOT_STATUS)); > + 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 +1007,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 >
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..4346dcdc9697 100644 --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c @@ -910,6 +910,34 @@ 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; + + 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; + + slot_status = readl(jpeg->base_reg + MXC_SLOT_OFFSET(ctx->slot, SLOT_STATUS)); + if (slot_status & SLOT_STATUS_ONGOING) + return true; + + /* + * The curr_desc register is updated when next_descpt_ptr is loaded, + * the ongoing bit of slot_status is set when the 32 bytes descriptor is loaded. + * So there will be a short time interval in between, which may cause fake false. + * Consider read register is quite slow compared with IP read 32byte from memory, + * read twice slot_status can avoid this situation. + */ + slot_status = readl(jpeg->base_reg + MXC_SLOT_OFFSET(ctx->slot, SLOT_STATUS)); + 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 +1007,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;