media: mali-c55: Fix frame sequence numbers on dual-pipe hardware

PATCH In review David Carlier 3 participants 4 messages Sep 30 – Nov 10 1 message not in the archive

Subsystems (3)

Stable branch Media (V4L/DVB) Arm Mali-C55 ISP driver

lore

Share & tools

lore

David Carlier

author

2026-09-30 17:46 UTC #0 lore raw

PATCH v2 1/1 media: mali-c55: Fix frame sequence numbers on dual-pipe hardware

The single frame_sequence counter in struct mali_c55_isp is incremented in mali_c55_set_plane_done(), which runs once per completed buffer per capture device rather than once per frame. Hardware fitted with the downscale pipe always hits this: mali_c55_pipeline_ready() refuses to start the ISP unless both the full-resolution and the downscale queues are streaming, so the counter advances twice per frame.

Each video node then reports sequence numbers 0, 2, 4, ..., which userspace reads as a dropped frame between every pair of frames, and the two pipes never number the same frame alike, contrary to Documentation/admin-guide/media/mali-c55.rst. The V4L2_EVENT_FRAME_SYNC event and the statistics and parameters buffers only read the counter, so their sequence numbers stop identifying a frame too.

Increment the counter once per frame, when the ISP start interrupt is handled, and stamp each capture buffer when mali_c55_set_next_buffer() programs it, as it is written out during the following frame. Stamping at completion would be off by one whenever DONE(n) and START(n+1) are handled in the same interrupt, since ISP_START is processed first. Hold the counter at UINT_MAX while the ISP is stopped, as the first buffers are programmed before it starts, so the first frame gets sequence zero.

Fixes d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")

Cc [email protected]

Assisted-by Claude:claude-opus-5

Signed-off-by David Carlier <[email protected]>

---
v2:
- Stamp the buffer sequence in mali_c55_set_next_buffer() instead of at completion, which broke when START(n+1) and DONE(n) are handled together (Jacopo)
- Reset the counter when the ISP stops instead of when it starts

v1: https://lore.kernel.org/r/[email protected]

---
 drivers/media/platform/arm/mali-c55/mali-c55-capture.c | 5 +++--
 drivers/media/platform/arm/mali-c55/mali-c55-common.h  | 4 ++++
 drivers/media/platform/arm/mali-c55/mali-c55-core.c    | 1 +
 drivers/media/platform/arm/mali-c55/mali-c55-isp.c     | 3 ++-
 4 files changed, 10 insertions(+), 3 deletions(-)

drivers/media/platform/arm/mali-c55/mali-c55-capture.c +3 -2

index ff01553026fb..fa073ce4b2bd 100644
@@ -413,6 +413,9 @@ void mali_c55_set_next_buffer(struct mali_c55_cap_dev *cap_dev)
 		return;
 	}
 
+	/* The buffer is written out during the frame following this one. */
+	buf->vb.sequence = cap_dev->mali_c55->isp.frame_sequence + 1;
+
 	pix_mp = &cap_dev->format.format;
 
 	mali_c55_cap_dev_update_bits(cap_dev, MALI_C55_REG_Y_WRITER_MODE,

@@ -457,7 +460,6 @@ void mali_c55_set_next_buffer(struct mali_c55_cap_dev *cap_dev)
 void mali_c55_set_plane_done(struct mali_c55_cap_dev *cap_dev,
 			     enum mali_c55_planes plane)
 {
-	struct mali_c55_isp *isp = &cap_dev->mali_c55->isp;
 	struct mali_c55_buffer *buf;
 
 	scoped_guard(spinlock, &cap_dev->buffers.processing_lock) {

@@ -476,7 +478,6 @@ void mali_c55_set_plane_done(struct mali_c55_cap_dev *cap_dev,
 
 	/* If the other plane is also done... */
 	buf->vb.vb2_buf.timestamp = ktime_get_boottime_ns();
-	buf->vb.sequence = isp->frame_sequence++;
 	vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
 }

drivers/media/platform/arm/mali-c55/mali-c55-common.h +4 -0

index 13a3e9dc4243..50f2b7a85f62 100644
@@ -76,6 +76,10 @@ struct mali_c55_isp {
 	struct media_pad *remote_src;
 	/* Mutex to guard vb2 start/stop streaming */
 	struct mutex capture_lock;
+	/*
+	 * Sequence of the frame being processed, incremented at SOF. Held at
+	 * UINT_MAX while stopped so the first frame gets sequence 0.
+	 */
 	unsigned int frame_sequence;
 };

drivers/media/platform/arm/mali-c55/mali-c55-core.c +1 -0

index fb81141d1653..be562156b295 100644
@@ -573,6 +573,7 @@ static irqreturn_t mali_c55_isr(int irq, void *context)
 	for_each_set_bit(i, &interrupt_status, MALI_C55_NUM_IRQ_BITS) {
 		switch (i) {
 		case MALI_C55_IRQ_ISP_START:
+			mali_c55->isp.frame_sequence++;
 			mali_c55_isp_queue_event_sof(mali_c55);
 
 			mali_c55_set_next_buffer(&mali_c55->cap_devs[MALI_C55_CAP_DEV_FR]);

drivers/media/platform/arm/mali-c55/mali-c55-isp.c +2 -1

index e128adf6ee37..dd3bce287a3d 100644
@@ -342,7 +342,6 @@ static int mali_c55_isp_enable_streams(struct v4l2_subdev *sd,
 
 	src_sd = media_entity_to_v4l2_subdev(isp->remote_src->entity);
 
-	isp->frame_sequence = 0;
 	ret = mali_c55_isp_start(mali_c55, state);
 	if (ret) {
 		dev_err(mali_c55->dev, "Failed to start ISP\n");

@@ -380,6 +379,7 @@ static int mali_c55_isp_disable_streams(struct v4l2_subdev *sd,
 	isp->remote_src = NULL;
 
 	mali_c55_isp_stop(mali_c55);
+	isp->frame_sequence = UINT_MAX;
 
 	return 0;
 }

@@ -584,6 +584,7 @@ int mali_c55_register_isp(struct mali_c55 *mali_c55)
 	int ret;
 
 	isp->mali_c55 = mali_c55;
+	isp->frame_sequence = UINT_MAX;
 
 	v4l2_subdev_init(sd, &mali_c55_isp_ops);
 	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;

base-commit: 95f76f51937fdfb0fc1e14cae606b1ef574a56f3

Signature · 1 line

2.55.0

Dan Scally

maintainer

2026-10-07 10:29 UTC #1 lore raw

Hi David

Quoted 22 lines · David Carlier

The single frame_sequence counter in struct mali_c55_isp is incremented in mali_c55_set_plane_done(), which runs once per completed buffer per capture device rather than once per frame. Hardware fitted with the downscale pipe always hits this: mali_c55_pipeline_ready() refuses to start the ISP unless both the full-resolution and the downscale queues are streaming, so the counter advances twice per frame.
Each video node then reports sequence numbers 0, 2, 4, ..., which userspace reads as a dropped frame between every pair of frames, and the two pipes never number the same frame alike, contrary to Documentation/admin-guide/media/mali-c55.rst. The V4L2_EVENT_FRAME_SYNC event and the statistics and parameters buffers only read the counter, so their sequence numbers stop identifying a frame too.
Increment the counter once per frame, when the ISP start interrupt is handled, and stamp each capture buffer when mali_c55_set_next_buffer() programs it, as it is written out during the following frame. Stamping at completion would be off by one whenever DONE(n) and START(n+1) are handled in the same interrupt, since ISP_START is processed first. Hold the counter at UINT_MAX while the ISP is stopped, as the first buffers are programmed before it starts, so the first frame gets sequence zero.

Quoted 5 lines

Fixes d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
Cc [email protected]
Assisted-by Claude:claude-opus-5
Signed-off-by David Carlier <[email protected]>
---

I think this approach is fine. In v1 Jacopo said that he'd rather handle it with per-output counters though; @Jacopo, do you mean there that you'd rather an independent sequence counter for the FR and DS outputs?

Thanks
Dan

Quoted 101 lines

v2:
- Stamp the buffer sequence in mali_c55_set_next_buffer() instead of at completion, which broke when START(n+1) and DONE(n) are handled together (Jacopo)
- Reset the counter when the ISP stops instead of when it starts

v1: https://lore.kernel.org/r/[email protected]

---
  drivers/media/platform/arm/mali-c55/mali-c55-capture.c | 5 +++--
  drivers/media/platform/arm/mali-c55/mali-c55-common.h  | 4 ++++
  drivers/media/platform/arm/mali-c55/mali-c55-core.c    | 1 +
  drivers/media/platform/arm/mali-c55/mali-c55-isp.c     | 3 ++-
  4 files changed, 10 insertions(+), 3 deletions(-)

drivers/media/platform/arm/mali-c55/mali-c55-capture.c +3 -2

index ff01553026fb..fa073ce4b2bd 100644
@@ -413,6 +413,9 @@ void mali_c55_set_next_buffer(struct mali_c55_cap_dev *cap_dev)
  		return;
  	}
 
+	/* The buffer is written out during the frame following this one. */
+	buf->vb.sequence = cap_dev->mali_c55->isp.frame_sequence + 1;
+
  	pix_mp = &cap_dev->format.format;
 
  	mali_c55_cap_dev_update_bits(cap_dev, MALI_C55_REG_Y_WRITER_MODE,

@@ -457,7 +460,6 @@ void mali_c55_set_next_buffer(struct mali_c55_cap_dev *cap_dev)
  void mali_c55_set_plane_done(struct mali_c55_cap_dev *cap_dev,
  			     enum mali_c55_planes plane)
  {
-	struct mali_c55_isp *isp = &cap_dev->mali_c55->isp;
  	struct mali_c55_buffer *buf;
 
  	scoped_guard(spinlock, &cap_dev->buffers.processing_lock) {

@@ -476,7 +478,6 @@ void mali_c55_set_plane_done(struct mali_c55_cap_dev *cap_dev,
 
  	/* If the other plane is also done... */
  	buf->vb.vb2_buf.timestamp = ktime_get_boottime_ns();
-	buf->vb.sequence = isp->frame_sequence++;
  	vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
  }

drivers/media/platform/arm/mali-c55/mali-c55-common.h +4 -0

index 13a3e9dc4243..50f2b7a85f62 100644
@@ -76,6 +76,10 @@ struct mali_c55_isp {
  	struct media_pad *remote_src;
  	/* Mutex to guard vb2 start/stop streaming */
  	struct mutex capture_lock;
+	/*
+	 * Sequence of the frame being processed, incremented at SOF. Held at
+	 * UINT_MAX while stopped so the first frame gets sequence 0.
+	 */
  	unsigned int frame_sequence;
  };

drivers/media/platform/arm/mali-c55/mali-c55-core.c +1 -0

index fb81141d1653..be562156b295 100644
@@ -573,6 +573,7 @@ static irqreturn_t mali_c55_isr(int irq, void *context)
  	for_each_set_bit(i, &interrupt_status, MALI_C55_NUM_IRQ_BITS) {
  		switch (i) {
  		case MALI_C55_IRQ_ISP_START:
+			mali_c55->isp.frame_sequence++;
  			mali_c55_isp_queue_event_sof(mali_c55);
 
  			mali_c55_set_next_buffer(&mali_c55->cap_devs[MALI_C55_CAP_DEV_FR]);

drivers/media/platform/arm/mali-c55/mali-c55-isp.c +2 -1

index e128adf6ee37..dd3bce287a3d 100644
@@ -342,7 +342,6 @@ static int mali_c55_isp_enable_streams(struct v4l2_subdev *sd,
 
  	src_sd = media_entity_to_v4l2_subdev(isp->remote_src->entity);
 
-	isp->frame_sequence = 0;
  	ret = mali_c55_isp_start(mali_c55, state);
  	if (ret) {
  		dev_err(mali_c55->dev, "Failed to start ISP\n");

@@ -380,6 +379,7 @@ static int mali_c55_isp_disable_streams(struct v4l2_subdev *sd,
  	isp->remote_src = NULL;
 
  	mali_c55_isp_stop(mali_c55);
+	isp->frame_sequence = UINT_MAX;
 
  	return 0;
  }

@@ -584,6 +584,7 @@ int mali_c55_register_isp(struct mali_c55 *mali_c55)
  	int ret;
 
  	isp->mali_c55 = mali_c55;
+	isp->frame_sequence = UINT_MAX;
 
  	v4l2_subdev_init(sd, &mali_c55_isp_ops);
  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;

base-commit: 95f76f51937fdfb0fc1e14cae606b1ef574a56f3

Jacopo Mondi

maintainer Reviewed-by on v2 1/1

2026-11-10 13:22 UTC #2 lore raw

In reply to David Carlier

Hi David

Quoted 10 lines · David Carlier

The single frame_sequence counter in struct mali_c55_isp is incremented in mali_c55_set_plane_done(), which runs once per completed buffer per capture device rather than once per frame. Hardware fitted with the downscale pipe always hits this: mali_c55_pipeline_ready() refuses to start the ISP unless both the full-resolution and the downscale queues are streaming, so the counter advances twice per frame.
Each video node then reports sequence numbers 0, 2, 4, ..., which userspace reads as a dropped frame between every pair of frames, and the two pipes never number the same frame alike, contrary to

Sorry to ask, this is not meant to be detrimental to the value of this patch, but in these days I feel like I should ask:

- is this an error found running the system with an FR/DS pipeline
- is this a report from an LLM

Mostly I'm curious to know as I can't test on a system with a DS pipe fitted, knowing you actually tested this would make me more confident in accepting the patch

Quoted 109 lines

Documentation/admin-guide/media/mali-c55.rst. The V4L2_EVENT_FRAME_SYNC event and the statistics and parameters buffers only read the counter, so their sequence numbers stop identifying a frame too.
Increment the counter once per frame, when the ISP start interrupt is handled, and stamp each capture buffer when mali_c55_set_next_buffer() programs it, as it is written out during the following frame. Stamping at completion would be off by one whenever DONE(n) and START(n+1) are handled in the same interrupt, since ISP_START is processed first. Hold the counter at UINT_MAX while the ISP is stopped, as the first buffers are programmed before it starts, so the first frame gets sequence zero.
Fixes d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
Cc [email protected]
Assisted-by Claude:claude-opus-5
Signed-off-by David Carlier <[email protected]>
---
v2:
- Stamp the buffer sequence in mali_c55_set_next_buffer() instead of at completion, which broke when START(n+1) and DONE(n) are handled together (Jacopo)
- Reset the counter when the ISP stops instead of when it starts
v1: https://lore.kernel.org/r/[email protected]
---
 drivers/media/platform/arm/mali-c55/mali-c55-capture.c | 5 +++--
 drivers/media/platform/arm/mali-c55/mali-c55-common.h  | 4 ++++
 drivers/media/platform/arm/mali-c55/mali-c55-core.c    | 1 +
 drivers/media/platform/arm/mali-c55/mali-c55-isp.c     | 3 ++-
 4 files changed, 10 insertions(+), 3 deletions(-)
drivers/media/platform/arm/mali-c55/mali-c55-capture.c +3 -2
index ff01553026fb..fa073ce4b2bd 100644
@@ -413,6 +413,9 @@ void mali_c55_set_next_buffer(struct mali_c55_cap_dev *cap_dev)
 		return;
 	}
 
+	/* The buffer is written out during the frame following this one. */
+	buf->vb.sequence = cap_dev->mali_c55->isp.frame_sequence + 1;
+
 	pix_mp = &cap_dev->format.format;
 
 	mali_c55_cap_dev_update_bits(cap_dev, MALI_C55_REG_Y_WRITER_MODE,
@@ -457,7 +460,6 @@ void mali_c55_set_next_buffer(struct mali_c55_cap_dev *cap_dev)
 void mali_c55_set_plane_done(struct mali_c55_cap_dev *cap_dev,
 			     enum mali_c55_planes plane)
 {
-	struct mali_c55_isp *isp = &cap_dev->mali_c55->isp;
 	struct mali_c55_buffer *buf;
 
 	scoped_guard(spinlock, &cap_dev->buffers.processing_lock) {
@@ -476,7 +478,6 @@ void mali_c55_set_plane_done(struct mali_c55_cap_dev *cap_dev,
 
 	/* If the other plane is also done... */
 	buf->vb.vb2_buf.timestamp = ktime_get_boottime_ns();
-	buf->vb.sequence = isp->frame_sequence++;
 	vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
 }
 
drivers/media/platform/arm/mali-c55/mali-c55-common.h +4 -0
index 13a3e9dc4243..50f2b7a85f62 100644
@@ -76,6 +76,10 @@ struct mali_c55_isp {
 	struct media_pad *remote_src;
 	/* Mutex to guard vb2 start/stop streaming */
 	struct mutex capture_lock;
+	/*
+	 * Sequence of the frame being processed, incremented at SOF. Held at
+	 * UINT_MAX while stopped so the first frame gets sequence 0.
+	 */
 	unsigned int frame_sequence;
 };
 
drivers/media/platform/arm/mali-c55/mali-c55-core.c +1 -0
index fb81141d1653..be562156b295 100644
@@ -573,6 +573,7 @@ static irqreturn_t mali_c55_isr(int irq, void *context)
 	for_each_set_bit(i, &interrupt_status, MALI_C55_NUM_IRQ_BITS) {
 		switch (i) {
 		case MALI_C55_IRQ_ISP_START:
+			mali_c55->isp.frame_sequence++;
 			mali_c55_isp_queue_event_sof(mali_c55);
 
 			mali_c55_set_next_buffer(&mali_c55->cap_devs[MALI_C55_CAP_DEV_FR]);
drivers/media/platform/arm/mali-c55/mali-c55-isp.c +1 -1
index e128adf6ee37..dd3bce287a3d 100644
@@ -342,7 +342,6 @@ static int mali_c55_isp_enable_streams(struct v4l2_subdev *sd,
 
 	src_sd = media_entity_to_v4l2_subdev(isp->remote_src->entity);
 
-	isp->frame_sequence = 0;
 	ret = mali_c55_isp_start(mali_c55, state);
 	if (ret) {
 		dev_err(mali_c55->dev, "Failed to start ISP\n");
@@ -380,6 +379,7 @@ static int mali_c55_isp_disable_streams(struct v4l2_subdev *sd,
 	isp->remote_src = NULL;
 
 	mali_c55_isp_stop(mali_c55);
+	isp->frame_sequence = UINT_MAX;
 
 	return 0;
 }

drivers/media/platform/arm/mali-c55/mali-c55-isp.c int mali_c55_register_isp(struct mali_c55 *mali_c55) · line 587

Quoted 4 lines

drivers/media/platform/arm/mali-c55/mali-c55-isp.c +1 -0

@@ -584,6 +584,7 @@ int mali_c55_register_isp(struct mali_c55 *mali_c55)
 	int ret;
 
 	isp->mali_c55 = mali_c55;
+	isp->frame_sequence = UINT_MAX;

The patch looks ok to me:

Reviewed-by Jacopo Mondi <[email protected]>

but I wish we could test it.

Knowing the current frame counter stamping is broken already makes it easier for me to collect this patch.

Quoted 8 lines

drivers/media/platform/arm/mali-c55/mali-c55-isp.c +0 -0
 	v4l2_subdev_init(sd, &mali_c55_isp_ops);
 	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
 
base-commit: 95f76f51937fdfb0fc1e14cae606b1ef574a56f3 --
2.55.0

A message that is not in the archive (sent off-list or not yet received): CA+XhMqz--zg_rR8_TqziuPXLRDDP1Y1xcNV7WS9_004AfaBtuA@mail.gmail.com

Jacopo Mondi

maintainer

2026-11-10 14:12 UTC #4 lore raw

Hi David

Quoted 22 lines · David CARLIER

Hi,
Quoted 17 lines · Jacopo Mondi
Hi David
Quoted 10 lines · David Carlier
The single frame_sequence counter in struct mali_c55_isp is incremented in mali_c55_set_plane_done(), which runs once per completed buffer per capture device rather than once per frame. Hardware fitted with the downscale pipe always hits this: mali_c55_pipeline_ready() refuses to start the ISP unless both the full-resolution and the downscale queues are streaming, so the counter advances twice per frame.
Each video node then reports sequence numbers 0, 2, 4, ..., which userspace reads as a dropped frame between every pair of frames, and the two pipes never number the same frame alike, contrary to
Sorry to ask, this is not meant to be detrimental to the value of this patch, but in these days I feel like I should ask:

Quoted 8 lines

Quoted 3 lines
- is this an error found running the system with an FR/DS pipeline
- is this a report from an LLM
I honestly use LLM to find this issue.
I paid attention however if the increment in mali_c55_set_plane_done() and above all mali_c55_pipeline_ready() requires the queues when MALI_C55_GPS_DS_PIPE_FITTED is set.

Thanks, as said in the previous reply, we know the current handling of frame sequence is broken, which makes it easier for me to accept a patch not tested on real hw.

Honestly, commit messages reporting such a precise analysis of an issue suggest the patch comes from from a real device, not from analysis, and I wonder if in 6 months, if we ever find an issue with this patch we'll remember it hasn't actually been tested.

It's a general concern regarding the increasing amount of LLM generated fixes which come from code analysis rather than actual testing.

Usage of LLM is now not required to be disclosed, but I wonder if for our own safety we shouldn't clearly say "This issue has been found by an LLM inspecting the code and not tested on a device." on all these automated patches.

Quoted 153 lines

Cheers.
Quoted 150 lines
Mostly I'm curious to know as I can't test on a system with a DS pipe fitted, knowing you actually tested this would make me more confident in accepting the patch
Quoted 24 lines
Documentation/admin-guide/media/mali-c55.rst. The V4L2_EVENT_FRAME_SYNC event and the statistics and parameters buffers only read the counter, so their sequence numbers stop identifying a frame too.
Increment the counter once per frame, when the ISP start interrupt is handled, and stamp each capture buffer when mali_c55_set_next_buffer() programs it, as it is written out during the following frame. Stamping at completion would be off by one whenever DONE(n) and START(n+1) are handled in the same interrupt, since ISP_START is processed first. Hold the counter at UINT_MAX while the ISP is stopped, as the first buffers are programmed before it starts, so the first frame gets sequence zero.
Fixes d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
Cc [email protected]
Assisted-by Claude:claude-opus-5
Signed-off-by David Carlier <[email protected]>
---
v2:
- Stamp the buffer sequence in mali_c55_set_next_buffer() instead of at completion, which broke when START(n+1) and DONE(n) are handled together (Jacopo)
- Reset the counter when the ISP stops instead of when it starts
v1: https://lore.kernel.org/r/[email protected]
Review
Quoted 8 lines
---
 drivers/media/platform/arm/mali-c55/mali-c55-capture.c | 5 +++--
 drivers/media/platform/arm/mali-c55/mali-c55-common.h  | 4 ++++
 drivers/media/platform/arm/mali-c55/mali-c55-core.c    | 1 +
 drivers/media/platform/arm/mali-c55/mali-c55-isp.c     | 3 ++-
 4 files changed, 10 insertions(+), 3 deletions(-)
(quoted diff) +0 -0
b/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
drivers/media/platform/arm/mali-c55/mali-c55-capture.c void mali_c55_set_next_buffer(struct
Quoted 4 lines
index ff01553026fb..fa073ce4b2bd 100644
drivers/media/platform/arm/mali-c55/mali-c55-capture.c +0 -0
@@ -413,6 +413,9 @@ void mali_c55_set_next_buffer(struct
mali_c55_cap_dev *cap_dev)
drivers/media/platform/arm/mali-c55/mali-c55-capture.c void mali_c55_set_next_buffer(struct · line 416
Quoted 4 lines
drivers/media/platform/arm/mali-c55/mali-c55-capture.c +1 -0
              return;
      }
 
+     /* The buffer is written out during the frame following this one.
*/
drivers/media/platform/arm/mali-c55/mali-c55-capture.c void mali_c55_set_next_buffer(struct
Quoted 6 lines
drivers/media/platform/arm/mali-c55/mali-c55-capture.c +2 -0
+     buf->vb.sequence = cap_dev->mali_c55->isp.frame_sequence + 1;
+
      pix_mp = &cap_dev->format.format;
 
      mali_c55_cap_dev_update_bits(cap_dev, MALI_C55_REG_Y_WRITER_MODE,
@@ -457,7 +460,6 @@ void mali_c55_set_next_buffer(struct
mali_c55_cap_dev *cap_dev)
drivers/media/platform/arm/mali-c55/mali-c55-capture.c void mali_c55_set_plane_done(struct mali_c55_cap_dev
Quoted 8 lines
drivers/media/platform/arm/mali-c55/mali-c55-capture.c +0 -1
 void mali_c55_set_plane_done(struct mali_c55_cap_dev *cap_dev,
                           enum mali_c55_planes plane)
 {
-     struct mali_c55_isp *isp = &cap_dev->mali_c55->isp;
      struct mali_c55_buffer *buf;
 
      scoped_guard(spinlock, &cap_dev->buffers.processing_lock) {
@@ -476,7 +478,6 @@ void mali_c55_set_plane_done(struct mali_c55_cap_dev
*cap_dev,
Review
Quoted 8 lines
drivers/media/platform/arm/mali-c55/mali-c55-capture.c +0 -1
      /* If the other plane is also done... */
      buf->vb.vb2_buf.timestamp = ktime_get_boottime_ns();
-     buf->vb.sequence = isp->frame_sequence++;
      vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
 }
 
(quoted diff) +0 -0
b/drivers/media/platform/arm/mali-c55/mali-c55-common.h
drivers/media/platform/arm/mali-c55/mali-c55-common.h struct mali_c55_isp { · line 80
Quoted 9 lines
index 13a3e9dc4243..50f2b7a85f62 100644
drivers/media/platform/arm/mali-c55/mali-c55-common.h +2 -0
@@ -76,6 +76,10 @@ struct mali_c55_isp {
      struct media_pad *remote_src;
      /* Mutex to guard vb2 start/stop streaming */
      struct mutex capture_lock;
+     /*
+      * Sequence of the frame being processed, incremented at SOF. Held
at
Review
Quoted 6 lines
drivers/media/platform/arm/mali-c55/mali-c55-common.h +2 -0
+      * UINT_MAX while stopped so the first frame gets sequence 0.
+      */
      unsigned int frame_sequence;
 };
 
(quoted diff) +0 -0
b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
drivers/media/platform/arm/mali-c55/mali-c55-core.c static irqreturn_t mali_c55_isr(int irq, void
Quoted 4 lines
index fb81141d1653..be562156b295 100644
drivers/media/platform/arm/mali-c55/mali-c55-core.c +0 -0
@@ -573,6 +573,7 @@ static irqreturn_t mali_c55_isr(int irq, void
*context)
drivers/media/platform/arm/mali-c55/mali-c55-core.c static irqreturn_t mali_c55_isr(int irq, void *context)
Quoted 7 lines
drivers/media/platform/arm/mali-c55/mali-c55-core.c +1 -0
      for_each_set_bit(i, &interrupt_status, MALI_C55_NUM_IRQ_BITS) {
              switch (i) {
              case MALI_C55_IRQ_ISP_START:
+                     mali_c55->isp.frame_sequence++;
                      mali_c55_isp_queue_event_sof(mali_c55);
 
 
mali_c55_set_next_buffer(&mali_c55->cap_devs[MALI_C55_CAP_DEV_FR]);
Review
Quoted 1 line
(quoted diff) +0 -0
b/drivers/media/platform/arm/mali-c55/mali-c55-isp.c
drivers/media/platform/arm/mali-c55/mali-c55-isp.c static int mali_c55_isp_enable_streams(struct
Quoted 4 lines
index e128adf6ee37..dd3bce287a3d 100644
drivers/media/platform/arm/mali-c55/mali-c55-isp.c +0 -0
@@ -342,7 +342,6 @@ static int mali_c55_isp_enable_streams(struct
v4l2_subdev *sd,
drivers/media/platform/arm/mali-c55/mali-c55-isp.c static int mali_c55_isp_disable_streams(struct
Quoted 8 lines
drivers/media/platform/arm/mali-c55/mali-c55-isp.c +0 -1
      src_sd = media_entity_to_v4l2_subdev(isp->remote_src->entity);
 
-     isp->frame_sequence = 0;
      ret = mali_c55_isp_start(mali_c55, state);
      if (ret) {
              dev_err(mali_c55->dev, "Failed to start ISP\n");
@@ -380,6 +379,7 @@ static int mali_c55_isp_disable_streams(struct
v4l2_subdev *sd,
drivers/media/platform/arm/mali-c55/mali-c55-isp.c int mali_c55_register_isp(struct mali_c55 *mali_c55) · line 587
Quoted 12 lines
drivers/media/platform/arm/mali-c55/mali-c55-isp.c +2 -0
      isp->remote_src = NULL;
 
      mali_c55_isp_stop(mali_c55);
+     isp->frame_sequence = UINT_MAX;
 
      return 0;
 }
@@ -584,6 +584,7 @@ int mali_c55_register_isp(struct mali_c55 *mali_c55)
      int ret;
 
      isp->mali_c55 = mali_c55;
+     isp->frame_sequence = UINT_MAX;
The patch looks ok to me:
Reviewed-by Jacopo Mondi <[email protected]>
but I wish we could test it.
Knowing the current frame counter stamping is broken already makes it easier for me to collect this patch.
drivers/media/platform/arm/mali-c55/mali-c55-isp.c int mali_c55_register_isp(struct mali_c55 *mali_c55) · line 589
Quoted 3 lines
drivers/media/platform/arm/mali-c55/mali-c55-isp.c +0 -0
      v4l2_subdev_init(sd, &mali_c55_isp_ops);
      sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
V4L2_SUBDEV_FL_HAS_EVENTS;
Quoted 5 lines
base-commit: 95f76f51937fdfb0fc1e14cae606b1ef574a56f3 --
2.55.0