On Mon Sep 22 20:19:57 2025 +0200, Stefan Klug wrote:
> On the rkisp1 (in my case on a NXP i.MX8 M Plus) the ISP interrupt
> handler is sometimes called with RKISP1_CIF_ISP_V_START (start of frame)
> and RKISP1_CIF_ISP_FRAME (end of frame) being set at the same time. In
> commit 8524fa22fd2f ("media: staging: rkisp1: isp: add a warning and
> debugfs var for irq delay") a warning was added for that. There are two
> cases where this condition can occur:
>
> 1. The v-sync and the frame-end belong to the same frame. This means,
> the irq was heavily delayed and the warning is likely appropriate.
>
> 2. The v-sync belongs to the next frame. This can happen if the vertical
> blanking between two frames is very short.
>
> The current code always handles case 1 although case 2 is in my
> experience the more common case and happens in regular usage. This leads
> to incorrect sequence numbers on stats and params buffers which in turn
> breaks the regulation in user space. Fix that by adding a frame_active
> flag to distinguish between these cases and handle the start of frame
> either at the beginning or at the end of the rkisp1_isp_isr().
>
> Signed-off-by: Stefan Klug <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Reviewed-by: Kieran Bingham <[email protected]>
> Reviewed-by: Jacopo Mondi <[email protected]>
> Link:
> https://patch.msgid.link/[email protected]
> Signed-off-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Hans Verkuil <[email protected]>
Patch committed.
Thanks,
Hans Verkuil
.../media/platform/rockchip/rkisp1/rkisp1-common.h | 1 +
.../media/platform/rockchip/rkisp1/rkisp1-isp.c | 27 ++++++++++++++++++----
2 files changed, 23 insertions(+), 5 deletions(-)
---
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index 6028ecdd23de..5e6a4d5f6fd1 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -227,6 +227,7 @@ struct rkisp1_isp {
struct media_pad pads[RKISP1_ISP_PAD_MAX];
const struct rkisp1_mbus_info *sink_fmt;
__u32 frame_sequence;
+ bool frame_active;
};
/*
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
index 147d33eb6ee0..2311672cedb1 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -965,6 +965,7 @@ static int rkisp1_isp_s_stream(struct v4l2_subdev *sd, int
enable)
}
isp->frame_sequence = -1;
+ isp->frame_active = false;
sd_state = v4l2_subdev_lock_and_get_active_state(sd);
@@ -1086,12 +1087,15 @@ void rkisp1_isp_unregister(struct rkisp1_device *rkisp1)
* Interrupt handlers
*/
-static void rkisp1_isp_queue_event_sof(struct rkisp1_isp *isp)
+static void rkisp1_isp_sof(struct rkisp1_isp *isp)
{
struct v4l2_event event = {
.type = V4L2_EVENT_FRAME_SYNC,
};
+ isp->frame_sequence++;
+ isp->frame_active = true;
+
event.u.frame_sync.frame_sequence = isp->frame_sequence;
v4l2_event_queue(isp->sd.devnode, &event);
}
@@ -1111,15 +1115,20 @@ irqreturn_t rkisp1_isp_isr(int irq, void *ctx)
rkisp1_write(rkisp1, RKISP1_CIF_ISP_ICR, status);
- /* Vertical sync signal, starting generating new frame */
- if (status & RKISP1_CIF_ISP_V_START) {
- rkisp1->isp.frame_sequence++;
- rkisp1_isp_queue_event_sof(&rkisp1->isp);
+ /*
+ * Vertical sync signal, starting new frame. Defer handling of vsync
+ * after RKISP1_CIF_ISP_FRAME if the previous frame was not completed
+ * yet.
+ */
+ if (status & RKISP1_CIF_ISP_V_START && !rkisp1->isp.frame_active) {
+ status &= ~RKISP1_CIF_ISP_V_START;
+ rkisp1_isp_sof(&rkisp1->isp);
if (status & RKISP1_CIF_ISP_FRAME) {
WARN_ONCE(1, "irq delay is too long, buffers might not
be in sync\n");
rkisp1->debug.irq_delay++;
}
}
+
if (status & RKISP1_CIF_ISP_PIC_SIZE_ERROR) {
/* Clear pic_size_error */
isp_err = rkisp1_read(rkisp1, RKISP1_CIF_ISP_ERR);
@@ -1138,6 +1147,7 @@ irqreturn_t rkisp1_isp_isr(int irq, void *ctx)
if (status & RKISP1_CIF_ISP_FRAME) {
u32 isp_ris;
+ rkisp1->isp.frame_active = false;
rkisp1->debug.complete_frames++;
/* New frame from the sensor received */
@@ -1152,5 +1162,12 @@ irqreturn_t rkisp1_isp_isr(int irq, void *ctx)
rkisp1_params_isr(rkisp1);
}
+ /*
+ * Deferred handling of vsync if RKISP1_CIF_ISP_V_START and
+ * RKISP1_CIF_ISP_FRAME occurred in the same irq.
+ */
+ if (status & RKISP1_CIF_ISP_V_START)
+ rkisp1_isp_sof(&rkisp1->isp);
+
return IRQ_HANDLED;
}
_______________________________________________
linuxtv-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]
