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

Isaac Scott posted 6 patches 4 days, 22 hours ago
There is a newer version of this series
[PATCH v1 5/6] media: rkisp1: Give buffers back instead of dropping in bypass mode
Posted by Isaac Scott 4 days, 22 hours ago
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;
+		}
+
 		if (!(status & RKISP1_CIF_MI_FRAME(cap)))
 			continue;
 		if (!cap->is_stopping) {
-- 
2.43.0
Re: [PATCH v1 5/6] media: rkisp1: Give buffers back instead of dropping in bypass mode
Posted by Laurent Pinchart 4 days, 18 hours ago
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
Re: [PATCH v1 5/6] media: rkisp1: Give buffers back instead of dropping in bypass mode
Posted by Isaac Scott 4 days, 17 hours ago
Hi Laurent,

Quoting Laurent Pinchart (2026-02-04 15:40:04)
> 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.

You're right, I think I should get a frame end interrupt. I think what
has happened is during development part I was getting no frame end
interrupts, so I made this change, but it actually turned out to be an
inform_size_err if I recall correctly.

Although it may also be incorrect, I have checked whether I actually do
not get a frame end interrupt by dropping this patch, and it seems I do.
(I must have had my FPGA configured to output the wrong width / height
at the time).

I'll drop this in v2 as it doesn't seem necessary.

Thanks very much,
Isaac

> 
> >               if (!(status & RKISP1_CIF_MI_FRAME(cap)))
> >                       continue;
> >               if (!cap->is_stopping) {
> 
> -- 
> Regards,
> 
> Laurent Pinchart