[PATCH v2 0/3] media: subdev: v4l2_subdev_get_frame_desc_passthrough improvements

[PATCH v2 0/3] media: subdev: v4l2_subdev_get_frame_desc_passthrough improvements

[Tomi Valkeinen]

Improve v4l2_subdev_get_frame_desc_passthrough() kernel doc, minor
cleanups, and add locked and unlocked versions.

The last patch can be left for later, if there's anything controversial
there, as my need for it (UB953 TPG) depends on internal pads which are
not in upstream.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
Changes in v2:
- Add lockdep assert
- Rename the v4l2_subdev_get_frame_desc_passthrough_locked() to
  __v4l2_subdev_get_frame_desc_passthrough()
- Rebase on linux-media
- Link to v1: https://lore.kernel.org/r/20260312-frame-desc-passthrough-impro-v1-0-30f64d637a3a@ideasonboard.com

---
Tomi Valkeinen (3):
      media: subdev: Improve v4l2_subdev_get_frame_desc_passthrough() kdoc
      media: subdev: Minor v4l2_subdev_get_frame_desc_passthrough() cleanups
      media: subdev: Split v4l2_subdev_get_frame_desc_passthrough() into locked and unlocked

 drivers/media/v4l2-core/v4l2-subdev.c | 58 +++++++++++++++++++----------------
 include/media/v4l2-subdev.h           | 50 +++++++++++++++++++++++++-----
 2 files changed, 74 insertions(+), 34 deletions(-)
---
base-commit: f6390408a846aacc2171c17d88b062e202d84e86
change-id: 20260312-frame-desc-passthrough-impro-6a9ccb32d9a5

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

[PATCH v2 1/3] media: subdev: Improve v4l2_subdev_get_frame_desc_passthrough() kdoc

[Tomi Valkeinen]

Improve the v4l2_subdev_get_frame_desc_passthrough() kernel doc:

- Fix 'v4l2_get_frame_desc' operation to 'get_frame_desc' operation
- Rewrite the body text to be more understandable and specific, and
  specifically mention the frame desc type handling.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 include/media/v4l2-subdev.h | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index e754ed3421c5..23c03ba7f84c 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1724,19 +1724,25 @@ int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable);
 
 /**
  * v4l2_subdev_get_frame_desc_passthrough() - Helper to implement the subdev
- *	v4l2_get_frame_desc operation in simple passthrough cases
+ *	get_frame_desc operation in simple passthrough cases
  * @sd: The subdevice
  * @pad: The source pad index
  * @fd: The mbus frame desc
  *
- * Subdevice drivers that only pass through the streams can use this helper
- * to implement the &v4l2_subdev_pad_ops.v4l2_get_frame_desc operation.
+ * This helper implements get_frame_desc operation for subdevices that pass
+ * streams through without modification. It can be assigned directly as the
+ * .get_frame_desc callback in &v4l2_subdev_pad_ops.
  *
- * The helper will call get_frame_desc on the subdevice's sources, create a new
- * frame desc which contains only the streams on the given source pad. The data
- * for each frame desc entry is copied directly from the data provided from the
- * calls to the subdevice's sources, with the exception of the 'stream' field
- * which is set according to the subdevice's routing table.
+ * The helper iterates over the subdevice's sink pads, calls get_frame_desc on
+ * the remote subdevice connected to each sink pad, and collects the frame desc
+ * entries for streams that are routed to the given source pad according to the
+ * subdevice's routing table. Each entry is copied as-is from the upstream
+ * source, with the exception of the 'stream' field which is remapped to the
+ * source stream ID from the routing table.
+ *
+ * The frame desc type is taken from the first upstream source. If multiple
+ * sink pads are involved and the upstream sources report different frame desc
+ * types, -EPIPE is returned.
  *
  * Return: 0 on success, or a negative error code otherwise.
  */

-- 
2.43.0

Re: [PATCH v2 1/3] media: subdev: Improve v4l2_subdev_get_frame_desc_passthrough() kdoc

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Mar 17, 2026 at 02:09:40PM +0200, Tomi Valkeinen wrote:
> Improve the v4l2_subdev_get_frame_desc_passthrough() kernel doc:
> 
> - Fix 'v4l2_get_frame_desc' operation to 'get_frame_desc' operation
> - Rewrite the body text to be more understandable and specific, and
>   specifically mention the frame desc type handling.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  include/media/v4l2-subdev.h | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index e754ed3421c5..23c03ba7f84c 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1724,19 +1724,25 @@ int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable);
>  
>  /**
>   * v4l2_subdev_get_frame_desc_passthrough() - Helper to implement the subdev
> - *	v4l2_get_frame_desc operation in simple passthrough cases
> + *	get_frame_desc operation in simple passthrough cases
>   * @sd: The subdevice
>   * @pad: The source pad index
>   * @fd: The mbus frame desc
>   *
> - * Subdevice drivers that only pass through the streams can use this helper
> - * to implement the &v4l2_subdev_pad_ops.v4l2_get_frame_desc operation.
> + * This helper implements get_frame_desc operation for subdevices that pass
> + * streams through without modification. It can be assigned directly as the
> + * .get_frame_desc callback in &v4l2_subdev_pad_ops.
>   *
> - * The helper will call get_frame_desc on the subdevice's sources, create a new
> - * frame desc which contains only the streams on the given source pad. The data
> - * for each frame desc entry is copied directly from the data provided from the
> - * calls to the subdevice's sources, with the exception of the 'stream' field
> - * which is set according to the subdevice's routing table.
> + * The helper iterates over the subdevice's sink pads, calls get_frame_desc on
> + * the remote subdevice connected to each sink pad, and collects the frame desc
> + * entries for streams that are routed to the given source pad according to the
> + * subdevice's routing table. Each entry is copied as-is from the upstream
> + * source, with the exception of the 'stream' field which is remapped to the
> + * source stream ID from the routing table.
> + *
> + * The frame desc type is taken from the first upstream source. If multiple
> + * sink pads are involved and the upstream sources report different frame desc
> + * types, -EPIPE is returned.
>   *
>   * Return: 0 on success, or a negative error code otherwise.
>   */
> 

-- 
Regards,

Laurent Pinchart

[PATCH v2 2/3] media: subdev: Minor v4l2_subdev_get_frame_desc_passthrough() cleanups

[Tomi Valkeinen]

Minor code cleanups, no functional change.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 9efd14d4026f..2757378c628a 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -2549,14 +2549,13 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 					   unsigned int pad,
 					   struct v4l2_mbus_frame_desc *fd)
 {
-	const struct media_pad *pads = sd->entity.pads;
 	struct media_pad *local_sink_pad;
 	struct v4l2_subdev_route *route;
 	struct v4l2_subdev_state *state;
 	struct device *dev = sd->dev;
 	int ret = 0;
 
-	if (WARN_ON(!(pads[pad].flags & MEDIA_PAD_FL_SOURCE)))
+	if (WARN_ON(!(sd->entity.pads[pad].flags & MEDIA_PAD_FL_SOURCE)))
 		return -EINVAL;
 
 	state = v4l2_subdev_lock_and_get_active_state(sd);
@@ -2577,7 +2576,6 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 			struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
 			struct media_pad *remote_source_pad;
 			struct v4l2_subdev *remote_sd;
-			unsigned int i;
 
 			if (route->source_pad != pad ||
 			    route->sink_pad != local_sink_pad->index)
@@ -2622,7 +2620,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 				}
 			}
 
-			for (i = 0; i < source_fd.num_entries; i++) {
+			for (unsigned int i = 0; i < source_fd.num_entries; i++) {
 				if (source_fd.entry[i].stream == route->sink_stream) {
 					source_entry = &source_fd.entry[i];
 					break;
@@ -2630,7 +2628,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 			}
 
 			if (!source_entry) {
-				dev_dbg(sd->dev,
+				dev_dbg(dev,
 					"Failed to find stream %u from source frame desc\n",
 					route->sink_stream);
 				ret = -EPIPE;
@@ -2638,7 +2636,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 			}
 
 			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
-				dev_dbg(sd->dev, "Frame desc entry limit reached\n");
+				dev_dbg(dev, "Frame desc entry limit reached\n");
 				ret = -ENOSPC;
 				goto out_unlock;
 			}

-- 
2.43.0

Re: [PATCH v2 2/3] media: subdev: Minor v4l2_subdev_get_frame_desc_passthrough() cleanups

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Mar 17, 2026 at 02:09:41PM +0200, Tomi Valkeinen wrote:
> Minor code cleanups, no functional change.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 9efd14d4026f..2757378c628a 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -2549,14 +2549,13 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  					   unsigned int pad,
>  					   struct v4l2_mbus_frame_desc *fd)
>  {
> -	const struct media_pad *pads = sd->entity.pads;
>  	struct media_pad *local_sink_pad;
>  	struct v4l2_subdev_route *route;
>  	struct v4l2_subdev_state *state;
>  	struct device *dev = sd->dev;
>  	int ret = 0;
>  
> -	if (WARN_ON(!(pads[pad].flags & MEDIA_PAD_FL_SOURCE)))
> +	if (WARN_ON(!(sd->entity.pads[pad].flags & MEDIA_PAD_FL_SOURCE)))
>  		return -EINVAL;
>  
>  	state = v4l2_subdev_lock_and_get_active_state(sd);
> @@ -2577,7 +2576,6 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  			struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
>  			struct media_pad *remote_source_pad;
>  			struct v4l2_subdev *remote_sd;
> -			unsigned int i;
>  
>  			if (route->source_pad != pad ||
>  			    route->sink_pad != local_sink_pad->index)
> @@ -2622,7 +2620,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  				}
>  			}
>  
> -			for (i = 0; i < source_fd.num_entries; i++) {
> +			for (unsigned int i = 0; i < source_fd.num_entries; i++) {
>  				if (source_fd.entry[i].stream == route->sink_stream) {
>  					source_entry = &source_fd.entry[i];
>  					break;
> @@ -2630,7 +2628,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  			}
>  
>  			if (!source_entry) {
> -				dev_dbg(sd->dev,
> +				dev_dbg(dev,
>  					"Failed to find stream %u from source frame desc\n",
>  					route->sink_stream);
>  				ret = -EPIPE;
> @@ -2638,7 +2636,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  			}
>  
>  			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
> -				dev_dbg(sd->dev, "Frame desc entry limit reached\n");
> +				dev_dbg(dev, "Frame desc entry limit reached\n");
>  				ret = -ENOSPC;
>  				goto out_unlock;
>  			}
> 

-- 
Regards,

Laurent Pinchart

[PATCH v2 3/3] media: subdev: Split v4l2_subdev_get_frame_desc_passthrough() into locked and unlocked

[Tomi Valkeinen]

The recently added v4l2_subdev_get_frame_desc_passthrough() can be used
directly as an implementation for .get_frame_desc subdev op. However, in
some cases the drivers may want to add some customizations, while the
bulk of the work is still identical to what
v4l2_subdev_get_frame_desc_passthrough() does. Current locking scheme
makes this impossible to do properly.

Split v4l2_subdev_get_frame_desc_passthrough() into two functions:

__v4l2_subdev_get_frame_desc_passthrough(), which takes a locked subdev
state as a parameter, instead of locking and getting the active state
internally. Other than that, it does the same as
v4l2_subdev_get_frame_desc_passthrough() used to do.

v4l2_subdev_get_frame_desc_passthrough(), which locks the active state
and calls __v4l2_subdev_get_frame_desc_passthrough().

In other words, v4l2_subdev_get_frame_desc_passthrough() works as
before, but drivers can now alternatively add custom .get_frame_desc
code and call v4l2_subdev_get_frame_desc_passthrough().

An example use case is with DS90UB953 serializer: in normal use the
serializer passes through everything, but when test-pattern-generator
(TPG) is used, an internal TPG source is used. After this commit, the
UB953 get_frame_desc() can lock the state, look at the routing table to
see if we're in normal or TPG mode, then either call
__v4l2_subdev_get_frame_desc_passthrough() if in normal mode, or
construct a TPG frame desc if in TPG mode.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 48 ++++++++++++++++++++---------------
 include/media/v4l2-subdev.h           | 38 +++++++++++++++++++++++----
 2 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 2757378c628a..b73ccd1c83d2 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -2545,21 +2545,21 @@ int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable)
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_s_stream_helper);
 
-int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
-					   unsigned int pad,
-					   struct v4l2_mbus_frame_desc *fd)
+int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
+					     struct v4l2_subdev_state *state,
+					     unsigned int pad,
+					     struct v4l2_mbus_frame_desc *fd)
 {
 	struct media_pad *local_sink_pad;
 	struct v4l2_subdev_route *route;
-	struct v4l2_subdev_state *state;
 	struct device *dev = sd->dev;
 	int ret = 0;
 
+	lockdep_assert_held(state->lock);
+
 	if (WARN_ON(!(sd->entity.pads[pad].flags & MEDIA_PAD_FL_SOURCE)))
 		return -EINVAL;
 
-	state = v4l2_subdev_lock_and_get_active_state(sd);
-
 	/* Iterate over sink pads */
 	media_entity_for_each_pad(&sd->entity, local_sink_pad) {
 		struct v4l2_mbus_frame_desc source_fd;
@@ -2586,15 +2586,12 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 				if (!remote_source_pad) {
 					dev_dbg(dev, "Failed to find remote pad for sink pad %u\n",
 						local_sink_pad->index);
-					ret = -EINVAL;
-					goto out_unlock;
+					return -EINVAL;
 				}
 
 				remote_sd = media_entity_to_v4l2_subdev(remote_source_pad->entity);
-				if (!remote_sd) {
-					ret = -EINVAL;
-					goto out_unlock;
-				}
+				if (!remote_sd)
+					return -EINVAL;
 
 				ret = v4l2_subdev_call(remote_sd, pad,
 						       get_frame_desc,
@@ -2604,7 +2601,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 					dev_err(dev,
 						"Failed to get frame desc from remote subdev %s\n",
 						remote_sd->name);
-					goto out_unlock;
+					return ret;
 				}
 
 				have_source_fd = true;
@@ -2615,8 +2612,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 					dev_err(dev,
 						"Frame desc type mismatch: %u != %u\n",
 						fd->type, source_fd.type);
-					ret = -EPIPE;
-					goto out_unlock;
+					return -EPIPE;
 				}
 			}
 
@@ -2631,14 +2627,12 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 				dev_dbg(dev,
 					"Failed to find stream %u from source frame desc\n",
 					route->sink_stream);
-				ret = -EPIPE;
-				goto out_unlock;
+				return -EPIPE;
 			}
 
 			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
 				dev_dbg(dev, "Frame desc entry limit reached\n");
-				ret = -ENOSPC;
-				goto out_unlock;
+				return -ENOSPC;
 			}
 
 			fd->entry[fd->num_entries] = *source_entry;
@@ -2649,7 +2643,21 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 		}
 	}
 
-out_unlock:
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__v4l2_subdev_get_frame_desc_passthrough);
+
+int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
+					   unsigned int pad,
+					   struct v4l2_mbus_frame_desc *fd)
+{
+	struct v4l2_subdev_state *state;
+	int ret;
+
+	state = v4l2_subdev_lock_and_get_active_state(sd);
+
+	ret = __v4l2_subdev_get_frame_desc_passthrough(sd, state, pad, fd);
+
 	v4l2_subdev_unlock_state(state);
 
 	return ret;
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 23c03ba7f84c..d256b7ec8f84 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1723,15 +1723,15 @@ int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad,
 int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable);
 
 /**
- * v4l2_subdev_get_frame_desc_passthrough() - Helper to implement the subdev
- *	get_frame_desc operation in simple passthrough cases
+ * __v4l2_subdev_get_frame_desc_passthrough - Helper to implement the
+ *	subdev get_frame_desc operation in simple passthrough cases
  * @sd: The subdevice
+ * @state: The locked subdevice active state
  * @pad: The source pad index
  * @fd: The mbus frame desc
  *
- * This helper implements get_frame_desc operation for subdevices that pass
- * streams through without modification. It can be assigned directly as the
- * .get_frame_desc callback in &v4l2_subdev_pad_ops.
+ * This helper implements the get_frame_desc operation for subdevices that pass
+ * streams through without modification.
  *
  * The helper iterates over the subdevice's sink pads, calls get_frame_desc on
  * the remote subdevice connected to each sink pad, and collects the frame desc
@@ -1744,6 +1744,34 @@ int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable);
  * sink pads are involved and the upstream sources report different frame desc
  * types, -EPIPE is returned.
  *
+ * The caller must hold the subdevice's active state lock. This variant is
+ * intended for drivers that need to perform additional work around the
+ * passthrough frame descriptor collection. Drivers that do not need any
+ * customization should use v4l2_subdev_get_frame_desc_passthrough() instead.
+ *
+ * Return: 0 on success, or a negative error code otherwise.
+ */
+int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
+					     struct v4l2_subdev_state *state,
+					     unsigned int pad,
+					     struct v4l2_mbus_frame_desc *fd);
+
+/**
+ * v4l2_subdev_get_frame_desc_passthrough() - Helper to implement the subdev
+ *	get_frame_desc operation in simple passthrough cases
+ * @sd: The subdevice
+ * @pad: The source pad index
+ * @fd: The mbus frame desc
+ *
+ * This function locks the subdevice's active state, calls
+ * __v4l2_subdev_get_frame_desc_passthrough(), and unlocks the state.
+ *
+ * This function can be assigned directly as the .get_frame_desc callback in
+ * &v4l2_subdev_pad_ops for subdevices that pass streams through without
+ * modification. Drivers that need to perform additional work should use
+ * __v4l2_subdev_get_frame_desc_passthrough() in their custom
+ * .get_frame_desc implementation instead.
+ *
  * Return: 0 on success, or a negative error code otherwise.
  */
 int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,

-- 
2.43.0

Re: [PATCH v2 3/3] media: subdev: Split v4l2_subdev_get_frame_desc_passthrough() into locked and unlocked

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Mar 17, 2026 at 02:09:42PM +0200, Tomi Valkeinen wrote:
> The recently added v4l2_subdev_get_frame_desc_passthrough() can be used
> directly as an implementation for .get_frame_desc subdev op. However, in
> some cases the drivers may want to add some customizations, while the
> bulk of the work is still identical to what
> v4l2_subdev_get_frame_desc_passthrough() does. Current locking scheme
> makes this impossible to do properly.
> 
> Split v4l2_subdev_get_frame_desc_passthrough() into two functions:
> 
> __v4l2_subdev_get_frame_desc_passthrough(), which takes a locked subdev
> state as a parameter, instead of locking and getting the active state
> internally. Other than that, it does the same as
> v4l2_subdev_get_frame_desc_passthrough() used to do.
> 
> v4l2_subdev_get_frame_desc_passthrough(), which locks the active state
> and calls __v4l2_subdev_get_frame_desc_passthrough().
> 
> In other words, v4l2_subdev_get_frame_desc_passthrough() works as
> before, but drivers can now alternatively add custom .get_frame_desc
> code and call v4l2_subdev_get_frame_desc_passthrough().

This looks fine (and quite standard) to me.

> An example use case is with DS90UB953 serializer: in normal use the
> serializer passes through everything, but when test-pattern-generator
> (TPG) is used, an internal TPG source is used. After this commit, the
> UB953 get_frame_desc() can lock the state, look at the routing table to
> see if we're in normal or TPG mode, then either call
> __v4l2_subdev_get_frame_desc_passthrough() if in normal mode, or
> construct a TPG frame desc if in TPG mode.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 48 ++++++++++++++++++++---------------
>  include/media/v4l2-subdev.h           | 38 +++++++++++++++++++++++----
>  2 files changed, 61 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 2757378c628a..b73ccd1c83d2 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -2545,21 +2545,21 @@ int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable)
>  }
>  EXPORT_SYMBOL_GPL(v4l2_subdev_s_stream_helper);
>  
> -int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
> -					   unsigned int pad,
> -					   struct v4l2_mbus_frame_desc *fd)
> +int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
> +					     struct v4l2_subdev_state *state,
> +					     unsigned int pad,
> +					     struct v4l2_mbus_frame_desc *fd)
>  {
>  	struct media_pad *local_sink_pad;
>  	struct v4l2_subdev_route *route;
> -	struct v4l2_subdev_state *state;
>  	struct device *dev = sd->dev;
>  	int ret = 0;
>  
> +	lockdep_assert_held(state->lock);
> +
>  	if (WARN_ON(!(sd->entity.pads[pad].flags & MEDIA_PAD_FL_SOURCE)))
>  		return -EINVAL;
>  
> -	state = v4l2_subdev_lock_and_get_active_state(sd);
> -
>  	/* Iterate over sink pads */
>  	media_entity_for_each_pad(&sd->entity, local_sink_pad) {
>  		struct v4l2_mbus_frame_desc source_fd;
> @@ -2586,15 +2586,12 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  				if (!remote_source_pad) {
>  					dev_dbg(dev, "Failed to find remote pad for sink pad %u\n",
>  						local_sink_pad->index);
> -					ret = -EINVAL;
> -					goto out_unlock;
> +					return -EINVAL;
>  				}
>  
>  				remote_sd = media_entity_to_v4l2_subdev(remote_source_pad->entity);
> -				if (!remote_sd) {
> -					ret = -EINVAL;
> -					goto out_unlock;
> -				}
> +				if (!remote_sd)
> +					return -EINVAL;
>  
>  				ret = v4l2_subdev_call(remote_sd, pad,
>  						       get_frame_desc,
> @@ -2604,7 +2601,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  					dev_err(dev,
>  						"Failed to get frame desc from remote subdev %s\n",
>  						remote_sd->name);
> -					goto out_unlock;
> +					return ret;
>  				}
>  
>  				have_source_fd = true;
> @@ -2615,8 +2612,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  					dev_err(dev,
>  						"Frame desc type mismatch: %u != %u\n",
>  						fd->type, source_fd.type);
> -					ret = -EPIPE;
> -					goto out_unlock;
> +					return -EPIPE;
>  				}
>  			}
>  
> @@ -2631,14 +2627,12 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  				dev_dbg(dev,
>  					"Failed to find stream %u from source frame desc\n",
>  					route->sink_stream);
> -				ret = -EPIPE;
> -				goto out_unlock;
> +				return -EPIPE;
>  			}
>  
>  			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
>  				dev_dbg(dev, "Frame desc entry limit reached\n");
> -				ret = -ENOSPC;
> -				goto out_unlock;
> +				return -ENOSPC;
>  			}
>  
>  			fd->entry[fd->num_entries] = *source_entry;
> @@ -2649,7 +2643,21 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  		}
>  	}
>  
> -out_unlock:
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(__v4l2_subdev_get_frame_desc_passthrough);
> +
> +int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
> +					   unsigned int pad,
> +					   struct v4l2_mbus_frame_desc *fd)
> +{
> +	struct v4l2_subdev_state *state;
> +	int ret;
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> +	ret = __v4l2_subdev_get_frame_desc_passthrough(sd, state, pad, fd);
> +
>  	v4l2_subdev_unlock_state(state);
>  
>  	return ret;
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 23c03ba7f84c..d256b7ec8f84 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1723,15 +1723,15 @@ int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad,
>  int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable);
>  
>  /**
> - * v4l2_subdev_get_frame_desc_passthrough() - Helper to implement the subdev
> - *	get_frame_desc operation in simple passthrough cases
> + * __v4l2_subdev_get_frame_desc_passthrough - Helper to implement the
> + *	subdev get_frame_desc operation in simple passthrough cases
>   * @sd: The subdevice
> + * @state: The locked subdevice active state
>   * @pad: The source pad index
>   * @fd: The mbus frame desc
>   *
> - * This helper implements get_frame_desc operation for subdevices that pass
> - * streams through without modification. It can be assigned directly as the
> - * .get_frame_desc callback in &v4l2_subdev_pad_ops.
> + * This helper implements the get_frame_desc operation for subdevices that pass

Nitpicking, the insertion of "the" belongs to a previous patch.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> + * streams through without modification.
>   *
>   * The helper iterates over the subdevice's sink pads, calls get_frame_desc on
>   * the remote subdevice connected to each sink pad, and collects the frame desc
> @@ -1744,6 +1744,34 @@ int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable);
>   * sink pads are involved and the upstream sources report different frame desc
>   * types, -EPIPE is returned.
>   *
> + * The caller must hold the subdevice's active state lock. This variant is
> + * intended for drivers that need to perform additional work around the
> + * passthrough frame descriptor collection. Drivers that do not need any
> + * customization should use v4l2_subdev_get_frame_desc_passthrough() instead.
> + *
> + * Return: 0 on success, or a negative error code otherwise.
> + */
> +int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
> +					     struct v4l2_subdev_state *state,
> +					     unsigned int pad,
> +					     struct v4l2_mbus_frame_desc *fd);
> +
> +/**
> + * v4l2_subdev_get_frame_desc_passthrough() - Helper to implement the subdev
> + *	get_frame_desc operation in simple passthrough cases
> + * @sd: The subdevice
> + * @pad: The source pad index
> + * @fd: The mbus frame desc
> + *
> + * This function locks the subdevice's active state, calls
> + * __v4l2_subdev_get_frame_desc_passthrough(), and unlocks the state.
> + *
> + * This function can be assigned directly as the .get_frame_desc callback in
> + * &v4l2_subdev_pad_ops for subdevices that pass streams through without
> + * modification. Drivers that need to perform additional work should use
> + * __v4l2_subdev_get_frame_desc_passthrough() in their custom
> + * .get_frame_desc implementation instead.
> + *
>   * Return: 0 on success, or a negative error code otherwise.
>   */
>  int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
> 

-- 
Regards,

Laurent Pinchart