Re: [PATCH v1 5/6] media: rkisp1: Give buffers back instead of dropping in bypass mode

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Wed, Feb 04, 2026 at 11:25:05AM +0000, Isaac Scott wrote:
> In the data mode used for YUV passthrough, falling VSYNC events are used
> to determine when a buffer is complete. This means there is no 'Frame
> End' signal.
> 
> Previously, all buffers would be dropped when bypass mode was active.
> Instead of dropping every frame, we should return the buffer to user
> space if it is marked as complete by a falling VSYNC signal.
> 
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>  drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> index 867cdddf9f89..2753be39ab33 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> @@ -832,6 +832,11 @@ irqreturn_t rkisp1_capture_isr(int irq, void *ctx)
>  	for (i = 0; i < dev_count; ++i) {
>  		struct rkisp1_capture *cap = &rkisp1->capture_devs[i];
>  
> +		if (rkisp1->in_bypass) {
> +			rkisp1_handle_buffer(cap);
> +			continue;
> +		}
> +

You're completing the buffer if *any* interrupt bit is set, this doesn't
seem right. At the moment we may not enable interrupts other than the
one you expect, but that may change later, and this will misbehave.
You're also not handling the stopping logic, which also seems wrong.

Finally, what interrupt bit do you expect to see ? There's no VSYNC
interrupt in the RKISP1_CIF_MI_MIS register. Are you relying on the fact
that your platform has a single interrupt line for the three interrupt
sources (CSI2, ISP and MI), and react to the VSYNC interrupt from the
ISP interrupt here ? That's not right either, not only will it not work
on platforms that have three separate interrupt lines, but it will also
cause rkisp1_handle_buffer() to be called for *any* ISP interrupt.

I'm surprised that the frame end interrupt doesn't fire. It's part of
the MI, not the ISP, so it shouldn't be affected by bypass mode. You
should investigate that.

>  		if (!(status & RKISP1_CIF_MI_FRAME(cap)))
>  			continue;
>  		if (!cap->is_stopping) {

-- 
Regards,

Laurent Pinchart