[PATCH 0/2] media: synopsys: hdmirx: Report frame drops

[PATCH 0/2] media: synopsys: hdmirx: Report frame drops

[Nicolas Dufresne]

Frame drops are common problem with live captures. This is most of
the time due to buffer starvation introduced by userspace. The
v4l2_buffer.sequence allows detecting gaps (drops), which allows
userspace to report it. The new HDMI receiver driver on RK3588
did not report these properly.

With this in place, drops will be warned by GStreamer notably, with a
trace similar to:

WARN [...]:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:03.063493047

A QoS message is also sent to the application, which can be used to
gather statistics.

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
Nicolas Dufresne (2):
      media: synopsys: hdmirx: Renamed frame_idx to sequence
      media: synopsys: hdmirx: Count dropped frames

 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
---
base-commit: 9ddc3d6c16ea2587898a315f20f7b8fbd791dc1b
change-id: 20250410-rk3588-hdmirx-sequence-a470b604d176

Best regards,
-- 
Nicolas Dufresne <nicolas.dufresne@collabora.com>

[PATCH 1/2] media: synopsys: hdmirx: Renamed frame_idx to sequence

[Nicolas Dufresne]

This variable is used to fill the v4l2_buffer.sequence, let's name it
the exact same way to reduce confusion.

No functional changes.

Fixes: 7b59b132ad439 ("media: platform: synopsys: Add support for HDMI input driver")
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index 3d2913de9a86c6a4e66562727388b4326365048a..f5b3f5010ede55bde28756da326a434cc9245492 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -114,7 +114,7 @@ struct hdmirx_stream {
 	spinlock_t vbq_lock; /* to lock video buffer queue */
 	bool stopping;
 	wait_queue_head_t wq_stopped;
-	u32 frame_idx;
+	u32 sequence;
 	u32 line_flag_int_cnt;
 	u32 irq_stat;
 };
@@ -1540,7 +1540,7 @@ static int hdmirx_start_streaming(struct vb2_queue *queue, unsigned int count)
 	int line_flag;
 
 	mutex_lock(&hdmirx_dev->stream_lock);
-	stream->frame_idx = 0;
+	stream->sequence = 0;
 	stream->line_flag_int_cnt = 0;
 	stream->curr_buf = NULL;
 	stream->next_buf = NULL;
@@ -1948,7 +1948,7 @@ static void dma_idle_int_handler(struct snps_hdmirx_dev *hdmirx_dev,
 
 			if (vb_done) {
 				vb_done->vb2_buf.timestamp = ktime_get_ns();
-				vb_done->sequence = stream->frame_idx;
+				vb_done->sequence = stream->sequence;
 
 				if (bt->interlaced)
 					vb_done->field = V4L2_FIELD_INTERLACED_TB;
@@ -1956,8 +1956,8 @@ static void dma_idle_int_handler(struct snps_hdmirx_dev *hdmirx_dev,
 					vb_done->field = V4L2_FIELD_NONE;
 
 				hdmirx_vb_done(stream, vb_done);
-				stream->frame_idx++;
-				if (stream->frame_idx == 30)
+				stream->sequence++;
+				if (stream->sequence == 30)
 					v4l2_dbg(1, debug, v4l2_dev,
 						 "rcv frames\n");
 			}

-- 
2.48.1

Re: [PATCH 1/2] media: synopsys: hdmirx: Renamed frame_idx to sequence

[Dmitry Osipenko]

10.04.2025 23:43, Nicolas Dufresne пишет:
> This variable is used to fill the v4l2_buffer.sequence, let's name it
> the exact same way to reduce confusion.
> 
> No functional changes.
> 
> Fixes: 7b59b132ad439 ("media: platform: synopsys: Add support for HDMI input driver")
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
>  drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> index 3d2913de9a86c6a4e66562727388b4326365048a..f5b3f5010ede55bde28756da326a434cc9245492 100644
> --- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> @@ -114,7 +114,7 @@ struct hdmirx_stream {
>  	spinlock_t vbq_lock; /* to lock video buffer queue */
>  	bool stopping;
>  	wait_queue_head_t wq_stopped;
> -	u32 frame_idx;
> +	u32 sequence;
>  	u32 line_flag_int_cnt;
>  	u32 irq_stat;
>  };
> @@ -1540,7 +1540,7 @@ static int hdmirx_start_streaming(struct vb2_queue *queue, unsigned int count)
>  	int line_flag;
>  
>  	mutex_lock(&hdmirx_dev->stream_lock);
> -	stream->frame_idx = 0;
> +	stream->sequence = 0;
>  	stream->line_flag_int_cnt = 0;
>  	stream->curr_buf = NULL;
>  	stream->next_buf = NULL;
> @@ -1948,7 +1948,7 @@ static void dma_idle_int_handler(struct snps_hdmirx_dev *hdmirx_dev,
>  
>  			if (vb_done) {
>  				vb_done->vb2_buf.timestamp = ktime_get_ns();
> -				vb_done->sequence = stream->frame_idx;
> +				vb_done->sequence = stream->sequence;
>  
>  				if (bt->interlaced)
>  					vb_done->field = V4L2_FIELD_INTERLACED_TB;
> @@ -1956,8 +1956,8 @@ static void dma_idle_int_handler(struct snps_hdmirx_dev *hdmirx_dev,
>  					vb_done->field = V4L2_FIELD_NONE;
>  
>  				hdmirx_vb_done(stream, vb_done);
> -				stream->frame_idx++;
> -				if (stream->frame_idx == 30)
> +				stream->sequence++;
> +				if (stream->sequence == 30)
>  					v4l2_dbg(1, debug, v4l2_dev,
>  						 "rcv frames\n");
>  			}
> 

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

-- 
Best regards,
Dmitry

[PATCH 2/2] media: synopsys: hdmirx: Count dropped frames

[Nicolas Dufresne]

The sequence number communicate the lost frames to userspace. For this
reason, it must be incremented even when a frame is skipped. This allows
userspace such as GStreamer to report the loss.

Fixes: 7b59b132ad439 ("media: platform: synopsys: Add support for HDMI input driver")
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index f5b3f5010ede55bde28756da326a434cc9245492..7af6765532e33239f4260b29ea82b31494b66213 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -1956,10 +1956,6 @@ static void dma_idle_int_handler(struct snps_hdmirx_dev *hdmirx_dev,
 					vb_done->field = V4L2_FIELD_NONE;
 
 				hdmirx_vb_done(stream, vb_done);
-				stream->sequence++;
-				if (stream->sequence == 30)
-					v4l2_dbg(1, debug, v4l2_dev,
-						 "rcv frames\n");
 			}
 
 			stream->curr_buf = NULL;
@@ -1971,6 +1967,10 @@ static void dma_idle_int_handler(struct snps_hdmirx_dev *hdmirx_dev,
 			v4l2_dbg(3, debug, v4l2_dev,
 				 "%s: next_buf NULL, skip vb_done\n", __func__);
 		}
+
+		stream->sequence++;
+		if (stream->sequence == 30)
+			v4l2_dbg(1, debug, v4l2_dev, "rcv frames\n");
 	}
 
 DMA_IDLE_OUT:

-- 
2.48.1

Re: [PATCH 2/2] media: synopsys: hdmirx: Count dropped frames

[Dmitry Osipenko]

10.04.2025 23:43, Nicolas Dufresne пишет:
> The sequence number communicate the lost frames to userspace. For this
> reason, it must be incremented even when a frame is skipped. This allows
> userspace such as GStreamer to report the loss.
> 
> Fixes: 7b59b132ad439 ("media: platform: synopsys: Add support for HDMI input driver")
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
>  drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> index f5b3f5010ede55bde28756da326a434cc9245492..7af6765532e33239f4260b29ea82b31494b66213 100644
> --- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> @@ -1956,10 +1956,6 @@ static void dma_idle_int_handler(struct snps_hdmirx_dev *hdmirx_dev,
>  					vb_done->field = V4L2_FIELD_NONE;
>  
>  				hdmirx_vb_done(stream, vb_done);
> -				stream->sequence++;
> -				if (stream->sequence == 30)
> -					v4l2_dbg(1, debug, v4l2_dev,
> -						 "rcv frames\n");
>  			}
>  
>  			stream->curr_buf = NULL;
> @@ -1971,6 +1967,10 @@ static void dma_idle_int_handler(struct snps_hdmirx_dev *hdmirx_dev,
>  			v4l2_dbg(3, debug, v4l2_dev,
>  				 "%s: next_buf NULL, skip vb_done\n", __func__);
>  		}
> +
> +		stream->sequence++;
> +		if (stream->sequence == 30)
> +			v4l2_dbg(1, debug, v4l2_dev, "rcv frames\n");
>  	}
>  
>  DMA_IDLE_OUT:
> 

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

-- 
Best regards,
Dmitry