| Message ID | 20241122084152.1841419-2-naush@raspberrypi.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: bcm2835-unicam: Upstreaming various improvements | expand |
Hi Naush On Fri, Nov 22, 2024 at 08:41:48AM +0000, Naushir Patuck wrote: > Ensure that the frame sequence counter is incremented only if a previous > frame start interrupt has occurred, or a frame start + frame end has > occurred simultaneously. > > This corresponds the sequence number with the actual number of frames > produced by the sensor, not the number of frame buffers dequeued back > to userland. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> Thanks, looks good to me Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > .../media/platform/broadcom/bcm2835-unicam.c | 22 +++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c > index 3aed0e493c81..36fb186a0421 100644 > --- a/drivers/media/platform/broadcom/bcm2835-unicam.c > +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c > @@ -199,6 +199,7 @@ struct unicam_device { > /* subdevice async notifier */ > struct v4l2_async_notifier notifier; > unsigned int sequence; > + bool frame_started; > > /* Sensor node */ > struct { > @@ -742,6 +743,8 @@ static irqreturn_t unicam_isr(int irq, void *dev) > * buffer forever. > */ > if (fe) { > + bool inc_seq = unicam->frame_started; > + > /* > * Ensure we have swapped buffers already as we can't > * stop the peripheral. If no buffer is available, use a > @@ -761,11 +764,24 @@ static irqreturn_t unicam_isr(int irq, void *dev) > * + FS + LS). In this case, we cannot signal the buffer > * as complete, as the HW will reuse that buffer. > */ > - if (node->cur_frm && node->cur_frm != node->next_frm) > + if (node->cur_frm && node->cur_frm != node->next_frm) { > unicam_process_buffer_complete(node, sequence); > + inc_seq = true; > + } > node->cur_frm = node->next_frm; > } > - unicam->sequence++; > + > + /* > + * Increment the sequence number conditionally on either a FS > + * having already occurred, or in the FE + FS condition as > + * caught in the FE handler above. This ensures the sequence > + * number corresponds to the frames generated by the sensor, not > + * the frames dequeued to userland. > + */ > + if (inc_seq) { > + unicam->sequence++; > + unicam->frame_started = false; > + } > } > > if (ista & UNICAM_FSI) { > @@ -795,6 +811,7 @@ static irqreturn_t unicam_isr(int irq, void *dev) > } > > unicam_queue_event_sof(unicam); > + unicam->frame_started = true; > } > > /* > @@ -1413,6 +1430,7 @@ static int unicam_sd_enable_streams(struct v4l2_subdev *sd, > if (unicam->pipe.nodes & BIT(UNICAM_METADATA_NODE)) > unicam_start_metadata(unicam); > > + unicam->frame_started = false; > unicam_start_rx(unicam, state); > } > > -- > 2.34.1 > >
diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c index 3aed0e493c81..36fb186a0421 100644 --- a/drivers/media/platform/broadcom/bcm2835-unicam.c +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c @@ -199,6 +199,7 @@ struct unicam_device { /* subdevice async notifier */ struct v4l2_async_notifier notifier; unsigned int sequence; + bool frame_started; /* Sensor node */ struct { @@ -742,6 +743,8 @@ static irqreturn_t unicam_isr(int irq, void *dev) * buffer forever. */ if (fe) { + bool inc_seq = unicam->frame_started; + /* * Ensure we have swapped buffers already as we can't * stop the peripheral. If no buffer is available, use a @@ -761,11 +764,24 @@ static irqreturn_t unicam_isr(int irq, void *dev) * + FS + LS). In this case, we cannot signal the buffer * as complete, as the HW will reuse that buffer. */ - if (node->cur_frm && node->cur_frm != node->next_frm) + if (node->cur_frm && node->cur_frm != node->next_frm) { unicam_process_buffer_complete(node, sequence); + inc_seq = true; + } node->cur_frm = node->next_frm; } - unicam->sequence++; + + /* + * Increment the sequence number conditionally on either a FS + * having already occurred, or in the FE + FS condition as + * caught in the FE handler above. This ensures the sequence + * number corresponds to the frames generated by the sensor, not + * the frames dequeued to userland. + */ + if (inc_seq) { + unicam->sequence++; + unicam->frame_started = false; + } } if (ista & UNICAM_FSI) { @@ -795,6 +811,7 @@ static irqreturn_t unicam_isr(int irq, void *dev) } unicam_queue_event_sof(unicam); + unicam->frame_started = true; } /* @@ -1413,6 +1430,7 @@ static int unicam_sd_enable_streams(struct v4l2_subdev *sd, if (unicam->pipe.nodes & BIT(UNICAM_METADATA_NODE)) unicam_start_metadata(unicam); + unicam->frame_started = false; unicam_start_rx(unicam, state); }
Ensure that the frame sequence counter is incremented only if a previous frame start interrupt has occurred, or a frame start + frame end has occurred simultaneously. This corresponds the sequence number with the actual number of frames produced by the sensor, not the number of frame buffers dequeued back to userland. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- .../media/platform/broadcom/bcm2835-unicam.c | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)