[PATCH 00/17] Rework frame descriptors

[PATCH 00/17] Rework frame descriptors

[Sakari Ailus]

Hi folks,

This smallish set makes frame descriptors dynamically allocated and
implements a single-entry frame descriptor based on the device's format,
using a new helper called v4l2_subdev_get_frame_desc(). All drivers that
do not obtain their frame descriptor from upstream are converted. The
helper also obtains a frame descriptor for the desired type (parallel or
CSI-2) and checks there's at least one entry there. These checks are
removed from drivers that currently perform them. (Some drivers also check
there's exactly a single frame descriptor entry but I think in most cases
this check could be loosened. That could be done after this set.)

On callee side these patches introduce no changes as the number of
pre-allocated memory for 8 frame descriptors remains as-is. The
get_frame_desc() pad op can return more than 8 frame descriptors by
setting the num_entries to the desired number and returning -ENOSPC.

If people prefer using cleanup.h / __free() to release the dynamically
allocated array (I think I'd almost require that), I'll merge the
now-separate __v4l2_subdev_get_frame_desc() into
v4l2_subdev_get_frame_desc().

More formats can be added to df-to-mbus conversion as needed. These are
meant to be initial formats that are enough for typical raw sensors (and
one RGB format, too).

Sakari Ailus (17):
  media: v4l2-common: Add mipi_csi2_dt_for_mbus()
  media: v4l2-subdev: Align frame descriptor error codes with routing
  media: v4l2-subdev: Prepare for changes in getting frame descriptors
  media: v4l2-subdev: Allocate frame descriptors based on the need
  media: v4l2-subdev: Provide a cleanup-friendly get_frame_desc
  media: v4l2-subdev: Change the maximum number of routes
  media: v4l2-subdev: Return dynamically allocated pass-through routes
  media: v4l2-subdev: Always return at least one frame descriptor
  media: bcm2835-unicam: Use v4l2_subdev_get_frame_desc()
  media: nxp: imx8-isi: Use v4l2_subdev_get_frame_desc()
  media: raspberrypi: cfe: Use v4l2_subdev_get_frame_desc()
  media: rzg2l-cru: Use v4l2_subdev_get_frame_desc()
  media: rkisp1: Use v4l2_subdev_get_frame_desc()
  media: exynos4-is: Use v4l2_subdev_get_frame_desc()
  media: ti: cal: Use v4l2_subdev_get_frame_desc()
  media: ipu6: Use v4l2_subdev_get_frame_desc()
  staging: media: ipu7: Use v4l2_subdev_get_frame_desc()

 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c |  23 ++-
 .../media/platform/broadcom/bcm2835-unicam.c  |  20 +--
 .../platform/nxp/imx8-isi/imx8-isi-crossbar.c |  15 +-
 .../media/platform/raspberrypi/rp1-cfe/cfe.c  |  29 ++--
 .../platform/renesas/rzg2l-cru/rzg2l-video.c  |  21 +--
 .../platform/rockchip/rkisp1/rkisp1-isp.c     |  12 +-
 .../samsung/exynos4-is/fimc-capture.c         |  16 +-
 drivers/media/platform/ti/cal/cal-camerarx.c  |  25 ++-
 drivers/media/v4l2-core/v4l2-common.c         |  47 ++++++
 drivers/media/v4l2-core/v4l2-subdev.c         | 156 +++++++++++++++---
 .../staging/media/ipu7/ipu7-isys-csi-phy.c    |  16 +-
 drivers/staging/media/ipu7/ipu7-isys-csi2.c   |  22 ++-
 include/media/mipi-csi2.h                     |   2 +
 include/media/v4l2-subdev.h                   |  76 ++++++++-
 14 files changed, 344 insertions(+), 136 deletions(-)

-- 
2.47.3

[PATCH 01/17] media: v4l2-common: Add mipi_csi2_dt_for_mbus()

[Sakari Ailus]

Add mipi_csi2_dt_for_mbus() for obtaining the MIPI CSI-2 data type (DT)
for a given Media bus pixel code.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 47 +++++++++++++++++++++++++++
 include/media/mipi-csi2.h             |  2 ++
 2 files changed, 49 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index bceafc4e92c8..3f40de563bbd 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -46,6 +46,7 @@
 #include <linux/uaccess.h>
 #include <asm/io.h>
 #include <asm/div64.h>
+#include <media/mipi-csi2.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
@@ -808,3 +809,49 @@ struct clk *__devm_v4l2_sensor_clk_get(struct device *dev, const char *id,
 	return clk_hw->clk;
 }
 EXPORT_SYMBOL_GPL(__devm_v4l2_sensor_clk_get);
+
+int mipi_csi2_dt_for_mbus(u32 code)
+{
+	switch (code) {
+	case MEDIA_BUS_FMT_BGR888_1X24:
+		return MIPI_CSI2_DT_RGB888;
+	case MEDIA_BUS_FMT_Y8_1X8:
+	case MEDIA_BUS_FMT_SBGGR8_1X8:
+	case MEDIA_BUS_FMT_SGBRG8_1X8:
+	case MEDIA_BUS_FMT_SGRBG8_1X8:
+	case MEDIA_BUS_FMT_SRGGB8_1X8:
+		return MIPI_CSI2_DT_RAW8;
+	case MEDIA_BUS_FMT_Y10_1X10:
+	case MEDIA_BUS_FMT_SBGGR10_1X10:
+	case MEDIA_BUS_FMT_SGBRG10_1X10:
+	case MEDIA_BUS_FMT_SGRBG10_1X10:
+	case MEDIA_BUS_FMT_SRGGB10_1X10:
+		return MIPI_CSI2_DT_RAW10;
+	case MEDIA_BUS_FMT_Y12_1X12:
+	case MEDIA_BUS_FMT_SBGGR12_1X12:
+	case MEDIA_BUS_FMT_SGBRG12_1X12:
+	case MEDIA_BUS_FMT_SGRBG12_1X12:
+	case MEDIA_BUS_FMT_SRGGB12_1X12:
+		return MIPI_CSI2_DT_RAW12;
+	case MEDIA_BUS_FMT_Y14_1X14:
+	case MEDIA_BUS_FMT_SBGGR14_1X14:
+	case MEDIA_BUS_FMT_SGBRG14_1X14:
+	case MEDIA_BUS_FMT_SGRBG14_1X14:
+	case MEDIA_BUS_FMT_SRGGB14_1X14:
+		return MIPI_CSI2_DT_RAW14;
+	case MEDIA_BUS_FMT_Y16_1X16:
+	case MEDIA_BUS_FMT_SBGGR16_1X16:
+	case MEDIA_BUS_FMT_SGBRG16_1X16:
+	case MEDIA_BUS_FMT_SGRBG16_1X16:
+	case MEDIA_BUS_FMT_SRGGB16_1X16:
+		return MIPI_CSI2_DT_RAW16;
+	case MEDIA_BUS_FMT_SBGGR20_1X20:
+	case MEDIA_BUS_FMT_SGBRG20_1X20:
+	case MEDIA_BUS_FMT_SGRBG20_1X20:
+	case MEDIA_BUS_FMT_SRGGB20_1X20:
+		return MIPI_CSI2_DT_RAW20;
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL_GPL(mipi_csi2_dt_for_mbus);
diff --git a/include/media/mipi-csi2.h b/include/media/mipi-csi2.h
index 40fc0264250d..0bea79ab66c6 100644
--- a/include/media/mipi-csi2.h
+++ b/include/media/mipi-csi2.h
@@ -44,4 +44,6 @@
 #define MIPI_CSI2_DT_RAW20		0x2f
 #define MIPI_CSI2_DT_USER_DEFINED(n)	(0x30 + (n))	/* 0..7 */
 
+int mipi_csi2_dt_for_mbus(u32 code);
+
 #endif /* _MEDIA_MIPI_CSI2_H */
-- 
2.47.3

Re: [PATCH 01/17] media: v4l2-common: Add mipi_csi2_dt_for_mbus()

[Frank Li]

On Wed, May 13, 2026 at 01:43:42PM +0300, Sakari Ailus wrote:
> Add mipi_csi2_dt_for_mbus() for obtaining the MIPI CSI-2 data type (DT)
> for a given Media bus pixel code.

Thanks, I worked similar version before.
https://lore.kernel.org/linux-media/aP+isGnWmJ4tLXcs@lizhi-Precision-Tower-5810/

strange, I have not find updated one, I remember I posted.
we may need more information, such as bpp.

Frank
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 47 +++++++++++++++++++++++++++
>  include/media/mipi-csi2.h             |  2 ++
>  2 files changed, 49 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index bceafc4e92c8..3f40de563bbd 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -46,6 +46,7 @@
>  #include <linux/uaccess.h>
>  #include <asm/io.h>
>  #include <asm/div64.h>
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-common.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
> @@ -808,3 +809,49 @@ struct clk *__devm_v4l2_sensor_clk_get(struct device *dev, const char *id,
>  	return clk_hw->clk;
>  }
>  EXPORT_SYMBOL_GPL(__devm_v4l2_sensor_clk_get);
> +
> +int mipi_csi2_dt_for_mbus(u32 code)
> +{
> +	switch (code) {
> +	case MEDIA_BUS_FMT_BGR888_1X24:
> +		return MIPI_CSI2_DT_RGB888;
> +	case MEDIA_BUS_FMT_Y8_1X8:
> +	case MEDIA_BUS_FMT_SBGGR8_1X8:
> +	case MEDIA_BUS_FMT_SGBRG8_1X8:
> +	case MEDIA_BUS_FMT_SGRBG8_1X8:
> +	case MEDIA_BUS_FMT_SRGGB8_1X8:
> +		return MIPI_CSI2_DT_RAW8;
> +	case MEDIA_BUS_FMT_Y10_1X10:
> +	case MEDIA_BUS_FMT_SBGGR10_1X10:
> +	case MEDIA_BUS_FMT_SGBRG10_1X10:
> +	case MEDIA_BUS_FMT_SGRBG10_1X10:
> +	case MEDIA_BUS_FMT_SRGGB10_1X10:
> +		return MIPI_CSI2_DT_RAW10;
> +	case MEDIA_BUS_FMT_Y12_1X12:
> +	case MEDIA_BUS_FMT_SBGGR12_1X12:
> +	case MEDIA_BUS_FMT_SGBRG12_1X12:
> +	case MEDIA_BUS_FMT_SGRBG12_1X12:
> +	case MEDIA_BUS_FMT_SRGGB12_1X12:
> +		return MIPI_CSI2_DT_RAW12;
> +	case MEDIA_BUS_FMT_Y14_1X14:
> +	case MEDIA_BUS_FMT_SBGGR14_1X14:
> +	case MEDIA_BUS_FMT_SGBRG14_1X14:
> +	case MEDIA_BUS_FMT_SGRBG14_1X14:
> +	case MEDIA_BUS_FMT_SRGGB14_1X14:
> +		return MIPI_CSI2_DT_RAW14;
> +	case MEDIA_BUS_FMT_Y16_1X16:
> +	case MEDIA_BUS_FMT_SBGGR16_1X16:
> +	case MEDIA_BUS_FMT_SGBRG16_1X16:
> +	case MEDIA_BUS_FMT_SGRBG16_1X16:
> +	case MEDIA_BUS_FMT_SRGGB16_1X16:
> +		return MIPI_CSI2_DT_RAW16;
> +	case MEDIA_BUS_FMT_SBGGR20_1X20:
> +	case MEDIA_BUS_FMT_SGBRG20_1X20:
> +	case MEDIA_BUS_FMT_SGRBG20_1X20:
> +	case MEDIA_BUS_FMT_SRGGB20_1X20:
> +		return MIPI_CSI2_DT_RAW20;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(mipi_csi2_dt_for_mbus);
> diff --git a/include/media/mipi-csi2.h b/include/media/mipi-csi2.h
> index 40fc0264250d..0bea79ab66c6 100644
> --- a/include/media/mipi-csi2.h
> +++ b/include/media/mipi-csi2.h
> @@ -44,4 +44,6 @@
>  #define MIPI_CSI2_DT_RAW20		0x2f
>  #define MIPI_CSI2_DT_USER_DEFINED(n)	(0x30 + (n))	/* 0..7 */
>
> +int mipi_csi2_dt_for_mbus(u32 code);
> +
>  #endif /* _MEDIA_MIPI_CSI2_H */
> --
> 2.47.3
>

Re: [PATCH 01/17] media: v4l2-common: Add mipi_csi2_dt_for_mbus()

[Sakari Ailus]

Hi Frank,

On Wed, May 13, 2026 at 04:46:06PM -0400, Frank Li wrote:
> On Wed, May 13, 2026 at 01:43:42PM +0300, Sakari Ailus wrote:
> > Add mipi_csi2_dt_for_mbus() for obtaining the MIPI CSI-2 data type (DT)
> > for a given Media bus pixel code.
> 
> Thanks, I worked similar version before.
> https://lore.kernel.org/linux-media/aP+isGnWmJ4tLXcs@lizhi-Precision-Tower-5810/
> 
> strange, I have not find updated one, I remember I posted.
> we may need more information, such as bpp.

Ah, I remember this, too! I'd prefer your patch in fact, but it needs some
cleanup first. I'd be interested in seeing the lost updated version of it.
:-)

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 01/17] media: v4l2-common: Add mipi_csi2_dt_for_mbus()

[Frank Li]

On Wed, May 13, 2026 at 11:54:09PM +0300, Sakari Ailus wrote:
> Hi Frank,
>
> On Wed, May 13, 2026 at 04:46:06PM -0400, Frank Li wrote:
> > On Wed, May 13, 2026 at 01:43:42PM +0300, Sakari Ailus wrote:
> > > Add mipi_csi2_dt_for_mbus() for obtaining the MIPI CSI-2 data type (DT)
> > > for a given Media bus pixel code.
> >
> > Thanks, I worked similar version before.
> > https://lore.kernel.org/linux-media/aP+isGnWmJ4tLXcs@lizhi-Precision-Tower-5810/
> >
> > strange, I have not find updated one, I remember I posted.
> > we may need more information, such as bpp.
>
> Ah, I remember this, too! I'd prefer your patch in fact, but it needs some
> cleanup first. I'd be interested in seeing the lost updated version of it.
> :-)

I just post single patch for your reference
https://lore.kernel.org/linux-media/20260513205949.105444-1-Frank.Li@nxp.com/T/#u

you can combine it into this thread, I need more time to work original imx6
stage threads, which is big changes.

Best regards
Frank
>
> --
> Kind regards,
>
> Sakari Ailus

[PATCH 02/17] media: v4l2-subdev: Align frame descriptor error codes with routing

[Sakari Ailus]

__v4l2_subdev_get_frame_desc_passthrough() returns -ENOSPC when there are
too many routes. There's a subtle difference when compared to -E2BIG, but
-E2BIG can be used in this case as well. In practice this was unlikely to
having been ever returned from the kernel as things currently stand.

-ENOSPC can be then repurposed for signalling of running out of entries in
the statically allocated array.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index e9f81b9be9e2..d93ed50255ed 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -2639,7 +2639,7 @@ int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 
 			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
 				dev_dbg(dev, "Frame desc entry limit reached\n");
-				return -ENOSPC;
+				return -E2BIG;
 			}
 
 			fd->entry[fd->num_entries] = *source_entry;
-- 
2.47.3

Re: [PATCH 02/17] media: v4l2-subdev: Align frame descriptor error codes with routing

[Frank Li]

On Wed, May 13, 2026 at 01:43:43PM +0300, Sakari Ailus wrote:
> __v4l2_subdev_get_frame_desc_passthrough() returns -ENOSPC when there are
> too many routes. There's a subtle difference when compared to -E2BIG, but
> -E2BIG can be used in this case as well. In practice this was unlikely to
> having been ever returned from the kernel as things currently stand.
>
> -ENOSPC can be then repurposed for signalling of running out of entries in
> the statically allocated array.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/media/v4l2-core/v4l2-subdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index e9f81b9be9e2..d93ed50255ed 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -2639,7 +2639,7 @@ int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>
>  			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
>  				dev_dbg(dev, "Frame desc entry limit reached\n");
> -				return -ENOSPC;
> +				return -E2BIG;
>  			}
>
>  			fd->entry[fd->num_entries] = *source_entry;
> --
> 2.47.3
>

[PATCH 03/17] media: v4l2-subdev: Prepare for changes in getting frame descriptors

[Sakari Ailus]

Introduce v4l2_subdev_alloc_frame_desc() and v4l2_subdev_free_frame_desc()
to both facilitate implementing drivers that need frame descriptors as
well as prepare for having a larger number of frame descriptors.
If the remote sub-device does not support frame descriptors,
v4l2_subdev_get_frame_desc() creates one (with a single entry)
opportunistically, thus avoiding the need to add frame descriptor support
to sensor drivers the device for which only generates a single stream, or
managing the situation on the caller side.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 59 +++++++++++++++++++++++++++
 include/media/v4l2-subdev.h           | 20 +++++++++
 2 files changed, 79 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index d93ed50255ed..b8acce8f9c33 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -20,6 +20,7 @@
 #include <linux/version.h>
 #include <linux/videodev2.h>
 
+#include <media/mipi-csi2.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-event.h>
@@ -2671,6 +2672,64 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc_passthrough);
 
+int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+			       struct v4l2_mbus_frame_desc *desc)
+{
+	struct v4l2_subdev_format subdev_fmt = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+		.pad = pad,
+	};
+	int ret;
+
+	if (v4l2_subdev_has_op(sd, pad, get_frame_desc)) {
+		unsigned int type = desc->type;
+
+		ret = v4l2_subdev_call(sd, pad, get_frame_desc, pad, desc);
+		if (ret)
+			return ret;
+
+		if (desc->type != type) {
+			dev_dbg(sd->dev,
+				"wrong type of frame descriptor for pad %d (got %u, expected %u)\n",
+				pad, desc->type, type);
+			return -EINVAL;
+		}
+
+		return ret;
+	}
+
+	if (desc->type != V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL &&
+	    desc->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2)
+		return -EINVAL;
+
+	struct v4l2_subdev_state *state =
+		v4l2_subdev_lock_and_get_active_state(sd);
+	ret = v4l2_subdev_call(sd, pad, get_fmt, state, &subdev_fmt);
+	v4l2_subdev_unlock_state(state);
+	if (ret)
+		return ret;
+
+	struct v4l2_mbus_frame_desc_entry entry = {
+		.pixelcode = subdev_fmt.format.code,
+	};
+
+	if (desc->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
+		int dt;
+
+		dt = mipi_csi2_dt_for_mbus(subdev_fmt.format.code);
+		if (dt < 0)
+			return dt;
+
+		entry.bus.csi2.dt = dt;
+	}
+
+	desc->entry[0] = entry;
+	desc->num_entries = 1;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc);
+
 #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
 
 #endif /* CONFIG_MEDIA_CONTROLLER */
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index d256b7ec8f84..c9e74566c85a 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1778,6 +1778,26 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 					   unsigned int pad,
 					   struct v4l2_mbus_frame_desc *fd);
 
+/**
+ * v4l2_subdev_get_frame_desc() - Get a frame descriptor for a pad
+ * @sd: The sub-device
+ * @pad: The number of the pad in @sd from which to obtain the frame descriptor
+ * @desc: A pointer to a frame descriptor, with its type field set
+ *
+ * Obtain a frame descriptor from a sub-device. If the sub-device supports the
+ * get_frame_desc pad operation, its result is returned, just like calling it
+ * directly using v4l2_subdev_call(). If the sub-device driver does not support
+ * it, then a frame descriptor containing a single entry is created using the
+ * information from the sub-device format for types
+ * V4L2_MBUS_FRAME_DESC_TYPE_CSI2 and V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL.
+ *
+ * The caller is required to set @desc->type to the expected bus type.
+ *
+ * Return: %0 on success or negative error code on failure.
+ */
+int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+			       struct v4l2_mbus_frame_desc *desc);
+
 #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
 
 #endif /* CONFIG_MEDIA_CONTROLLER */
-- 
2.47.3

Re: [PATCH 03/17] media: v4l2-subdev: Prepare for changes in getting frame descriptors

[Frank Li]

On Wed, May 13, 2026 at 01:43:44PM +0300, Sakari Ailus wrote:
> Introduce v4l2_subdev_alloc_frame_desc() and v4l2_subdev_free_frame_desc()
> to both facilitate implementing drivers that need frame descriptors as
> well as prepare for having a larger number of frame descriptors.
> If the remote sub-device does not support frame descriptors,
> v4l2_subdev_get_frame_desc() creates one (with a single entry)
> opportunistically, thus avoiding the need to add frame descriptor support
> to sensor drivers the device for which only generates a single stream, or
> managing the situation on the caller side.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/media/v4l2-core/v4l2-subdev.c | 59 +++++++++++++++++++++++++++
>  include/media/v4l2-subdev.h           | 20 +++++++++
>  2 files changed, 79 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index d93ed50255ed..b8acce8f9c33 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -20,6 +20,7 @@
>  #include <linux/version.h>
>  #include <linux/videodev2.h>
>
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-event.h>
> @@ -2671,6 +2672,64 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc_passthrough);
>
> +int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +			       struct v4l2_mbus_frame_desc *desc)
> +{
> +	struct v4l2_subdev_format subdev_fmt = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +		.pad = pad,
> +	};
> +	int ret;
> +
> +	if (v4l2_subdev_has_op(sd, pad, get_frame_desc)) {
> +		unsigned int type = desc->type;
> +
> +		ret = v4l2_subdev_call(sd, pad, get_frame_desc, pad, desc);
> +		if (ret)
> +			return ret;
> +
> +		if (desc->type != type) {
> +			dev_dbg(sd->dev,
> +				"wrong type of frame descriptor for pad %d (got %u, expected %u)\n",
> +				pad, desc->type, type);
> +			return -EINVAL;
> +		}
> +
> +		return ret;
> +	}
> +
> +	if (desc->type != V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL &&
> +	    desc->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2)
> +		return -EINVAL;
> +
> +	struct v4l2_subdev_state *state =
> +		v4l2_subdev_lock_and_get_active_state(sd);
> +	ret = v4l2_subdev_call(sd, pad, get_fmt, state, &subdev_fmt);
> +	v4l2_subdev_unlock_state(state);
> +	if (ret)
> +		return ret;
> +
> +	struct v4l2_mbus_frame_desc_entry entry = {
> +		.pixelcode = subdev_fmt.format.code,
> +	};
> +
> +	if (desc->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
> +		int dt;
> +
> +		dt = mipi_csi2_dt_for_mbus(subdev_fmt.format.code);
> +		if (dt < 0)
> +			return dt;
> +
> +		entry.bus.csi2.dt = dt;
> +	}
> +
> +	desc->entry[0] = entry;
> +	desc->num_entries = 1;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d256b7ec8f84..c9e74566c85a 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1778,6 +1778,26 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  					   unsigned int pad,
>  					   struct v4l2_mbus_frame_desc *fd);
>
> +/**
> + * v4l2_subdev_get_frame_desc() - Get a frame descriptor for a pad
> + * @sd: The sub-device
> + * @pad: The number of the pad in @sd from which to obtain the frame descriptor
> + * @desc: A pointer to a frame descriptor, with its type field set
> + *
> + * Obtain a frame descriptor from a sub-device. If the sub-device supports the
> + * get_frame_desc pad operation, its result is returned, just like calling it
> + * directly using v4l2_subdev_call(). If the sub-device driver does not support
> + * it, then a frame descriptor containing a single entry is created using the
> + * information from the sub-device format for types
> + * V4L2_MBUS_FRAME_DESC_TYPE_CSI2 and V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL.
> + *
> + * The caller is required to set @desc->type to the expected bus type.
> + *
> + * Return: %0 on success or negative error code on failure.
> + */
> +int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +			       struct v4l2_mbus_frame_desc *desc);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> --
> 2.47.3
>

Re: [PATCH 03/17] media: v4l2-subdev: Prepare for changes in getting frame descriptors

[Frank Li]

On Wed, May 13, 2026 at 01:43:44PM +0300, Sakari Ailus wrote:
> Introduce v4l2_subdev_alloc_frame_desc() and v4l2_subdev_free_frame_desc()

Sorry, where v4l2_subdev_alloc_frame_desc() ? do you means
v4l2_subdev_get_frame_desc() ?

Frank
> to both facilitate implementing drivers that need frame descriptors as
> well as prepare for having a larger number of frame descriptors.
> If the remote sub-device does not support frame descriptors,
> v4l2_subdev_get_frame_desc() creates one (with a single entry)
> opportunistically, thus avoiding the need to add frame descriptor support
> to sensor drivers the device for which only generates a single stream, or
> managing the situation on the caller side.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 59 +++++++++++++++++++++++++++
>  include/media/v4l2-subdev.h           | 20 +++++++++
>  2 files changed, 79 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index d93ed50255ed..b8acce8f9c33 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -20,6 +20,7 @@
>  #include <linux/version.h>
>  #include <linux/videodev2.h>
>
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-event.h>
> @@ -2671,6 +2672,64 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc_passthrough);
>
> +int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +			       struct v4l2_mbus_frame_desc *desc)
> +{
> +	struct v4l2_subdev_format subdev_fmt = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +		.pad = pad,
> +	};
> +	int ret;
> +
> +	if (v4l2_subdev_has_op(sd, pad, get_frame_desc)) {
> +		unsigned int type = desc->type;
> +
> +		ret = v4l2_subdev_call(sd, pad, get_frame_desc, pad, desc);
> +		if (ret)
> +			return ret;
> +
> +		if (desc->type != type) {
> +			dev_dbg(sd->dev,
> +				"wrong type of frame descriptor for pad %d (got %u, expected %u)\n",
> +				pad, desc->type, type);
> +			return -EINVAL;
> +		}
> +
> +		return ret;
> +	}
> +
> +	if (desc->type != V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL &&
> +	    desc->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2)
> +		return -EINVAL;
> +
> +	struct v4l2_subdev_state *state =
> +		v4l2_subdev_lock_and_get_active_state(sd);
> +	ret = v4l2_subdev_call(sd, pad, get_fmt, state, &subdev_fmt);
> +	v4l2_subdev_unlock_state(state);
> +	if (ret)
> +		return ret;
> +
> +	struct v4l2_mbus_frame_desc_entry entry = {
> +		.pixelcode = subdev_fmt.format.code,
> +	};
> +
> +	if (desc->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
> +		int dt;
> +
> +		dt = mipi_csi2_dt_for_mbus(subdev_fmt.format.code);
> +		if (dt < 0)
> +			return dt;
> +
> +		entry.bus.csi2.dt = dt;
> +	}
> +
> +	desc->entry[0] = entry;
> +	desc->num_entries = 1;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d256b7ec8f84..c9e74566c85a 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1778,6 +1778,26 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  					   unsigned int pad,
>  					   struct v4l2_mbus_frame_desc *fd);
>
> +/**
> + * v4l2_subdev_get_frame_desc() - Get a frame descriptor for a pad
> + * @sd: The sub-device
> + * @pad: The number of the pad in @sd from which to obtain the frame descriptor
> + * @desc: A pointer to a frame descriptor, with its type field set
> + *
> + * Obtain a frame descriptor from a sub-device. If the sub-device supports the
> + * get_frame_desc pad operation, its result is returned, just like calling it
> + * directly using v4l2_subdev_call(). If the sub-device driver does not support
> + * it, then a frame descriptor containing a single entry is created using the
> + * information from the sub-device format for types
> + * V4L2_MBUS_FRAME_DESC_TYPE_CSI2 and V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL.
> + *
> + * The caller is required to set @desc->type to the expected bus type.
> + *
> + * Return: %0 on success or negative error code on failure.
> + */
> +int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +			       struct v4l2_mbus_frame_desc *desc);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> --
> 2.47.3
>

Re: [PATCH 03/17] media: v4l2-subdev: Prepare for changes in getting frame descriptors

[Jacopo Mondi]

Hi Sakari

On Wed, May 13, 2026 at 01:43:44PM +0300, Sakari Ailus wrote:
> Introduce v4l2_subdev_alloc_frame_desc() and v4l2_subdev_free_frame_desc()
> to both facilitate implementing drivers that need frame descriptors as
> well as prepare for having a larger number of frame descriptors.

As noticed by Frank, this line doesn't seem to match the patch content

> If the remote sub-device does not support frame descriptors,
> v4l2_subdev_get_frame_desc() creates one (with a single entry)
> opportunistically, thus avoiding the need to add frame descriptor support
> to sensor drivers the device for which only generates a single stream, or
> managing the situation on the caller side.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 59 +++++++++++++++++++++++++++
>  include/media/v4l2-subdev.h           | 20 +++++++++
>  2 files changed, 79 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index d93ed50255ed..b8acce8f9c33 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -20,6 +20,7 @@
>  #include <linux/version.h>
>  #include <linux/videodev2.h>
>
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-event.h>
> @@ -2671,6 +2672,64 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc_passthrough);
>
> +int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +			       struct v4l2_mbus_frame_desc *desc)
> +{
> +	struct v4l2_subdev_format subdev_fmt = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +		.pad = pad,
> +	};
> +	int ret;

is it worth being extra paranoid and add an

        if (WARN_ON(!desc))
                return -EINVAL;

here ?

And make immediately sure that desc->type is populated by checking for

        if (desc->type == V4L2_MBUS_FRAME_DESC_TYPE_UNDEFINED)
                return -EINVAL;

or is passing UNDEFINED in supported ?

> +
> +	if (v4l2_subdev_has_op(sd, pad, get_frame_desc)) {
> +		unsigned int type = desc->type;
> +
> +		ret = v4l2_subdev_call(sd, pad, get_frame_desc, pad, desc);
> +		if (ret)
> +			return ret;
> +
> +		if (desc->type != type) {
> +			dev_dbg(sd->dev,
> +				"wrong type of frame descriptor for pad %d (got %u, expected %u)\n",
> +				pad, desc->type, type);
> +			return -EINVAL;
> +		}
> +
> +		return ret;

You can return 0;

> +	}
> +
> +	if (desc->type != V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL &&
> +	    desc->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2)
> +		return -EINVAL;
> +
> +	struct v4l2_subdev_state *state =
> +		v4l2_subdev_lock_and_get_active_state(sd);
> +	ret = v4l2_subdev_call(sd, pad, get_fmt, state, &subdev_fmt);
> +	v4l2_subdev_unlock_state(state);
> +	if (ret)
> +		return ret;
> +
> +	struct v4l2_mbus_frame_desc_entry entry = {
> +		.pixelcode = subdev_fmt.format.code,
> +	};
> +
> +	if (desc->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
> +		int dt;
> +
> +		dt = mipi_csi2_dt_for_mbus(subdev_fmt.format.code);
> +		if (dt < 0)
> +			return dt;
> +
> +		entry.bus.csi2.dt = dt;
> +	}
> +
> +	desc->entry[0] = entry;

Can't you populate desc->entry[0] directly and avoid a copy ?

> +	desc->num_entries = 1;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d256b7ec8f84..c9e74566c85a 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1778,6 +1778,26 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  					   unsigned int pad,
>  					   struct v4l2_mbus_frame_desc *fd);
>
> +/**
> + * v4l2_subdev_get_frame_desc() - Get a frame descriptor for a pad
> + * @sd: The sub-device
           The -remote- sub-device ?

> + * @pad: The number of the pad in @sd from which to obtain the frame descriptor
> + * @desc: A pointer to a frame descriptor, with its type field set
> + *
> + * Obtain a frame descriptor from a sub-device. If the sub-device supports the
> + * get_frame_desc pad operation, its result is returned, just like calling it
> + * directly using v4l2_subdev_call(). If the sub-device driver does not support
> + * it, then a frame descriptor containing a single entry is created using the
> + * information from the sub-device format for types
> + * V4L2_MBUS_FRAME_DESC_TYPE_CSI2 and V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL.
> + *
> + * The caller is required to set @desc->type to the expected bus type.
> + *
> + * Return: %0 on success or negative error code on failure.
> + */
> +int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +			       struct v4l2_mbus_frame_desc *desc);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> --
> 2.47.3
>
>

[PATCH 04/17] media: v4l2-subdev: Allocate frame descriptors based on the need

[Sakari Ailus]

Frame descriptors entries require a small amount of memory per entry (20
bytes), but if the number of entries in a frame descriptor is large, an
unreasonably large amount of memory would need to be allocated in the
stack. Therefore the number of entries has been limited to 8.

Support larger frame descriptors by allocating as much memory as required.
The get_frame_desc() op can now set the num_entries to a number larger
than V4L2_FRAME_BUS_ENTRY_MAX and return -ENOSPC. The caller,
v4l2_subdev_get_frame_desc(), will then allocate memory for that amount of
memory and call the get_frame_desc() op again.

The caller is also responsible for releasing the allocated memory by
calling v4l2_subdev_free_frame_desc().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 49 +++++++++++++++++++++------
 include/media/v4l2-subdev.h           | 44 +++++++++++++++++++-----
 2 files changed, 74 insertions(+), 19 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index b8acce8f9c33..012ff6fb2988 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -63,10 +63,6 @@ static bool v4l2_subdev_enable_streams_api;
 /*
  * Maximum stream ID is 63 for now, as we use u64 bitmask to represent a set
  * of streams.
- *
- * Note that V4L2_FRAME_DESC_ENTRY_MAX is related: V4L2_FRAME_DESC_ENTRY_MAX
- * restricts the total number of streams in a pad, although the stream ID is
- * not restricted.
  */
 #define V4L2_SUBDEV_MAX_STREAM_ID 63
 
@@ -354,6 +350,7 @@ static int call_set_frame_interval(struct v4l2_subdev *sd,
 static int call_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 			       struct v4l2_mbus_frame_desc *fd)
 {
+	unsigned int type;
 	unsigned int i;
 	int ret;
 
@@ -362,16 +359,38 @@ static int call_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 		return -EOPNOTSUPP;
 #endif
 
+	type = fd->type;
 	memset(fd, 0, sizeof(*fd));
+	if (!fd->entry) {
+		fd->entry = fd->entry_mem;
+		fd->len_entries = ARRAY_SIZE(fd->entry_mem);
+	}
 
 	ret = sd->ops->pad->get_frame_desc(sd, pad, fd);
+	if (ret == -ENOSPC) {
+		if (fd->num_entries > V4L2_FRAME_DESC_ENTRY_PREALLOC &&
+		    fd->num_entries < V4L2_FRAME_DESC_ENTRY_MAX) {
+			fd->entry = kzalloc_objs(*fd->entry, fd->num_entries,
+						 GFP_KERNEL);
+			if (!fd->entry)
+				return -ENOMEM;
+
+			fd->len_entries = fd->num_entries;
+			fd->num_entries = 0;
+
+			ret = sd->ops->pad->get_frame_desc(sd, pad, fd);
+		} else {
+			return -E2BIG;
+		}
+	}
 	if (ret)
 		return ret;
 
 	dev_dbg(sd->dev, "Frame descriptor on pad %u, type %s\n", pad,
-		fd->type == V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL ? "parallel" :
-		fd->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2 ? "CSI-2" :
-		"unknown");
+		type == V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL ? "parallel" :
+		type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2 ? "CSI-2" : "unknown");
+
+	fd->type = type;
 
 	for (i = 0; i < fd->num_entries; i++) {
 		struct v4l2_mbus_frame_desc_entry *entry = &fd->entry[i];
@@ -1086,9 +1105,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 		 * descriptor accordingly, with up to one entry per route. Until
 		 * the frame descriptors entries get allocated dynamically,
 		 * limit the number of active routes to
-		 * V4L2_FRAME_DESC_ENTRY_MAX.
+		 * V4L2_FRAME_DESC_ENTRY_PREALLOC.
 		 */
-		if (num_active_routes > V4L2_FRAME_DESC_ENTRY_MAX)
+		if (num_active_routes > V4L2_FRAME_DESC_ENTRY_PREALLOC)
 			return -E2BIG;
 
 		/*
@@ -2638,7 +2657,7 @@ int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 				return -EPIPE;
 			}
 
-			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
+			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_PREALLOC) {
 				dev_dbg(dev, "Frame desc entry limit reached\n");
 				return -E2BIG;
 			}
@@ -2730,6 +2749,16 @@ int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc);
 
+void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc)
+{
+	if (desc->entry != desc->entry_mem)
+		kfree(desc->entry);
+
+	desc->entry = NULL;
+	desc->len_entries = desc->num_entries = 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_free_frame_desc);
+
 #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
 
 #endif /* CONFIG_MEDIA_CONTROLLER */
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index c9e74566c85a..bd97510c7024 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -365,11 +365,13 @@ struct v4l2_mbus_frame_desc_entry {
 	} bus;
 };
 
- /*
-  * If this number is too small, it should be dropped altogether and the
-  * API switched to a dynamic number of frame descriptor entries.
-  */
-#define V4L2_FRAME_DESC_ENTRY_MAX	8
+/* Size of the statically allocated frame descriptor array. */
+#define V4L2_FRAME_DESC_ENTRY_PREALLOC	8
+/*
+ * Maximum number of dynamically allocated frame descriptors. Note that
+ * V4L2_SUBDEV_MAX_STREAM_ID is related to this limit as well.
+ */
+#define V4L2_FRAME_DESC_ENTRY_MAX	64
 
 /**
  * enum v4l2_mbus_frame_desc_type - media bus frame description type
@@ -392,13 +394,17 @@ enum v4l2_mbus_frame_desc_type {
 /**
  * struct v4l2_mbus_frame_desc - media bus data frame description
  * @type: type of the bus (enum v4l2_mbus_frame_desc_type)
- * @entry: frame descriptors array
- * @num_entries: number of entries in @entry array
+ * @entry_mem: memory for the frame descriptors (@entry)
+ * @entry: pointer to the frame descriptors
+ * @num_entries: number of entries in @entry
+ * @len_entries: number of entries allocated for @entry
  */
 struct v4l2_mbus_frame_desc {
 	enum v4l2_mbus_frame_desc_type type;
-	struct v4l2_mbus_frame_desc_entry entry[V4L2_FRAME_DESC_ENTRY_MAX];
+	struct v4l2_mbus_frame_desc_entry entry_mem[V4L2_FRAME_DESC_ENTRY_PREALLOC];
+	struct v4l2_mbus_frame_desc_entry *entry;
 	unsigned short num_entries;
+	unsigned short len_entries;
 };
 
 /**
@@ -780,7 +786,13 @@ struct v4l2_subdev_state {
  * @link_validate: used by the media controller code to check if the links
  *		   that belongs to a pipeline can be used for stream.
  *
- * @get_frame_desc: get the current low level media bus frame parameters.
+ * @get_frame_desc: get the current low level media bus frame parameters. The
+ *		    callback is required to update the num_entries field to the
+ *		    total number of entries in the frame descriptor. The
+ *		    callback shall fill the first entries array up to
+ *		    len_entries, which signifies the number of entries
+ *		    allocated. If num_entries exceeds len_entries, the callback
+ *		    shall return -ENOSPC.
  *
  * @set_frame_desc: set the low level media bus frame parameters, @fd array
  *                  may be adjusted by the subdev driver to device capabilities.
@@ -1793,11 +1805,25 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
  *
  * The caller is required to set @desc->type to the expected bus type.
  *
+ * The entries in the frame descriptor are allocated based on the need. The
+ * caller is required to release the memory of the frame descriptor entries for
+ * each frame descriptor obtained by calling this function using
+ * v4l2_subdev_free_frame_desc().
+ *
  * Return: %0 on success or negative error code on failure.
  */
 int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 			       struct v4l2_mbus_frame_desc *desc);
 
+/**
+ * v4l2_subdev_free_frame_desc() - Release the memory of a frame descriptor
+ * @desc: A pointer to a frame descriptor
+ *
+ * Release the frame descriptor entries in a frame descriptor. The number of
+ * entries in the descriptor are set to 0 again.
+ */
+void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc);
+
 #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
 
 #endif /* CONFIG_MEDIA_CONTROLLER */
-- 
2.47.3

Re: [PATCH 04/17] media: v4l2-subdev: Allocate frame descriptors based on the need

[Jacopo Mondi]

Hi Sakari

On Wed, May 13, 2026 at 01:43:45PM +0300, Sakari Ailus wrote:
> Frame descriptors entries require a small amount of memory per entry (20
> bytes), but if the number of entries in a frame descriptor is large, an
> unreasonably large amount of memory would need to be allocated in the
> stack. Therefore the number of entries has been limited to 8.
>
> Support larger frame descriptors by allocating as much memory as required.
> The get_frame_desc() op can now set the num_entries to a number larger
> than V4L2_FRAME_BUS_ENTRY_MAX and return -ENOSPC. The caller,
> v4l2_subdev_get_frame_desc(), will then allocate memory for that amount of
> memory and call the get_frame_desc() op again.
>
> The caller is also responsible for releasing the allocated memory by
> calling v4l2_subdev_free_frame_desc().
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 49 +++++++++++++++++++++------
>  include/media/v4l2-subdev.h           | 44 +++++++++++++++++++-----
>  2 files changed, 74 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index b8acce8f9c33..012ff6fb2988 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -63,10 +63,6 @@ static bool v4l2_subdev_enable_streams_api;
>  /*
>   * Maximum stream ID is 63 for now, as we use u64 bitmask to represent a set
>   * of streams.
> - *
> - * Note that V4L2_FRAME_DESC_ENTRY_MAX is related: V4L2_FRAME_DESC_ENTRY_MAX
> - * restricts the total number of streams in a pad, although the stream ID is
> - * not restricted.
>   */
>  #define V4L2_SUBDEV_MAX_STREAM_ID 63
>
> @@ -354,6 +350,7 @@ static int call_set_frame_interval(struct v4l2_subdev *sd,
>  static int call_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>  			       struct v4l2_mbus_frame_desc *fd)
>  {
> +	unsigned int type;
>  	unsigned int i;
>  	int ret;
>
> @@ -362,16 +359,38 @@ static int call_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>  		return -EOPNOTSUPP;
>  #endif
>
> +	type = fd->type;
>  	memset(fd, 0, sizeof(*fd));
> +	if (!fd->entry) {
> +		fd->entry = fd->entry_mem;
> +		fd->len_entries = ARRAY_SIZE(fd->entry_mem);
> +	}
>
>  	ret = sd->ops->pad->get_frame_desc(sd, pad, fd);
> +	if (ret == -ENOSPC) {
> +		if (fd->num_entries > V4L2_FRAME_DESC_ENTRY_PREALLOC &&
> +		    fd->num_entries < V4L2_FRAME_DESC_ENTRY_MAX) {

nit: you can invert the condition and save one indentation level

                if (fd->num_entries <= V4L2_FRAME_DESC_ENTRY_PREALLOC ||
                    fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX)
                    return -E2BIG;

> +			fd->entry = kzalloc_objs(*fd->entry, fd->num_entries,
> +						 GFP_KERNEL);
> +			if (!fd->entry)
> +				return -ENOMEM;
> +
> +			fd->len_entries = fd->num_entries;
> +			fd->num_entries = 0;
> +
> +			ret = sd->ops->pad->get_frame_desc(sd, pad, fd);
> +		} else {
> +			return -E2BIG;
> +		}
> +	}
>  	if (ret)
>  		return ret;
>
>  	dev_dbg(sd->dev, "Frame descriptor on pad %u, type %s\n", pad,
> -		fd->type == V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL ? "parallel" :
> -		fd->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2 ? "CSI-2" :
> -		"unknown");
> +		type == V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL ? "parallel" :
> +		type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2 ? "CSI-2" : "unknown");
> +
> +	fd->type = type;

If we end up calling d->ops->pad->get_frame_desc() don't we overwrite
the returned type ?

>
>  	for (i = 0; i < fd->num_entries; i++) {
>  		struct v4l2_mbus_frame_desc_entry *entry = &fd->entry[i];
> @@ -1086,9 +1105,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  		 * descriptor accordingly, with up to one entry per route. Until
>  		 * the frame descriptors entries get allocated dynamically,
>  		 * limit the number of active routes to
> -		 * V4L2_FRAME_DESC_ENTRY_MAX.
> +		 * V4L2_FRAME_DESC_ENTRY_PREALLOC.
>  		 */
> -		if (num_active_routes > V4L2_FRAME_DESC_ENTRY_MAX)
> +		if (num_active_routes > V4L2_FRAME_DESC_ENTRY_PREALLOC)
>  			return -E2BIG;
>
>  		/*
> @@ -2638,7 +2657,7 @@ int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  				return -EPIPE;
>  			}
>
> -			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
> +			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_PREALLOC) {
>  				dev_dbg(dev, "Frame desc entry limit reached\n");
>  				return -E2BIG;
>  			}
> @@ -2730,6 +2749,16 @@ int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc);
>
> +void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc)
> +{

As this might be used as a __cleanup() should we check if desc is
valid ?

> +	if (desc->entry != desc->entry_mem)
> +		kfree(desc->entry);
> +
> +	desc->entry = NULL;
> +	desc->len_entries = desc->num_entries = 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_subdev_free_frame_desc);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index c9e74566c85a..bd97510c7024 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -365,11 +365,13 @@ struct v4l2_mbus_frame_desc_entry {
>  	} bus;
>  };
>
> - /*
> -  * If this number is too small, it should be dropped altogether and the
> -  * API switched to a dynamic number of frame descriptor entries.
> -  */
> -#define V4L2_FRAME_DESC_ENTRY_MAX	8
> +/* Size of the statically allocated frame descriptor array. */
> +#define V4L2_FRAME_DESC_ENTRY_PREALLOC	8
> +/*
> + * Maximum number of dynamically allocated frame descriptors. Note that
> + * V4L2_SUBDEV_MAX_STREAM_ID is related to this limit as well.
> + */
> +#define V4L2_FRAME_DESC_ENTRY_MAX	64
>
>  /**
>   * enum v4l2_mbus_frame_desc_type - media bus frame description type
> @@ -392,13 +394,17 @@ enum v4l2_mbus_frame_desc_type {
>  /**
>   * struct v4l2_mbus_frame_desc - media bus data frame description
>   * @type: type of the bus (enum v4l2_mbus_frame_desc_type)
> - * @entry: frame descriptors array
> - * @num_entries: number of entries in @entry array
> + * @entry_mem: memory for the frame descriptors (@entry)
> + * @entry: pointer to the frame descriptors
> + * @num_entries: number of entries in @entry
> + * @len_entries: number of entries allocated for @entry
>   */
>  struct v4l2_mbus_frame_desc {
>  	enum v4l2_mbus_frame_desc_type type;
> -	struct v4l2_mbus_frame_desc_entry entry[V4L2_FRAME_DESC_ENTRY_MAX];
> +	struct v4l2_mbus_frame_desc_entry entry_mem[V4L2_FRAME_DESC_ENTRY_PREALLOC];
> +	struct v4l2_mbus_frame_desc_entry *entry;
>  	unsigned short num_entries;
> +	unsigned short len_entries;
>  };
>
>  /**
> @@ -780,7 +786,13 @@ struct v4l2_subdev_state {
>   * @link_validate: used by the media controller code to check if the links
>   *		   that belongs to a pipeline can be used for stream.
>   *
> - * @get_frame_desc: get the current low level media bus frame parameters.
> + * @get_frame_desc: get the current low level media bus frame parameters. The
> + *		    callback is required to update the num_entries field to the
> + *		    total number of entries in the frame descriptor. The
> + *		    callback shall fill the first entries array up to
> + *		    len_entries, which signifies the number of entries
> + *		    allocated. If num_entries exceeds len_entries, the callback
> + *		    shall return -ENOSPC.
>   *
>   * @set_frame_desc: set the low level media bus frame parameters, @fd array
>   *                  may be adjusted by the subdev driver to device capabilities.
> @@ -1793,11 +1805,25 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>   *
>   * The caller is required to set @desc->type to the expected bus type.
>   *
> + * The entries in the frame descriptor are allocated based on the need. The
> + * caller is required to release the memory of the frame descriptor entries for

Is it unconditionally true ? If fd->entry is not initialized aren't we
deflecting to the pre-allocated entries in call_get_frame_desc() ?

	if (!fd->entry) {
		fd->entry = fd->entry_mem;
		fd->len_entries = ARRAY_SIZE(fd->entry_mem);
	}

v4l2_subdev_free_frame_desc() shouldn't be called in this case, right ?


> + * each frame descriptor obtained by calling this function using
> + * v4l2_subdev_free_frame_desc().
> + *
>   * Return: %0 on success or negative error code on failure.
>   */
>  int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>  			       struct v4l2_mbus_frame_desc *desc);
>
> +/**
> + * v4l2_subdev_free_frame_desc() - Release the memory of a frame descriptor
> + * @desc: A pointer to a frame descriptor
> + *
> + * Release the frame descriptor entries in a frame descriptor. The number of
> + * entries in the descriptor are set to 0 again.
> + */
> +void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> --
> 2.47.3
>
>

Re: [PATCH 04/17] media: v4l2-subdev: Allocate frame descriptors based on the need

[Jacopo Mondi]

Hi Sakari

On Thu, May 14, 2026 at 12:16:51PM +0200, Jacopo Mondi wrote:
> Hi Sakari
>
> On Wed, May 13, 2026 at 01:43:45PM +0300, Sakari Ailus wrote:
> > Frame descriptors entries require a small amount of memory per entry (20
> > bytes), but if the number of entries in a frame descriptor is large, an
> > unreasonably large amount of memory would need to be allocated in the
> > stack. Therefore the number of entries has been limited to 8.
> >
> > Support larger frame descriptors by allocating as much memory as required.
> > The get_frame_desc() op can now set the num_entries to a number larger
> > than V4L2_FRAME_BUS_ENTRY_MAX and return -ENOSPC. The caller,
> > v4l2_subdev_get_frame_desc(), will then allocate memory for that amount of
> > memory and call the get_frame_desc() op again.
> >
> > The caller is also responsible for releasing the allocated memory by
> > calling v4l2_subdev_free_frame_desc().
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-subdev.c | 49 +++++++++++++++++++++------
> >  include/media/v4l2-subdev.h           | 44 +++++++++++++++++++-----
> >  2 files changed, 74 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index b8acce8f9c33..012ff6fb2988 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -63,10 +63,6 @@ static bool v4l2_subdev_enable_streams_api;
> >  /*
> >   * Maximum stream ID is 63 for now, as we use u64 bitmask to represent a set
> >   * of streams.
> > - *
> > - * Note that V4L2_FRAME_DESC_ENTRY_MAX is related: V4L2_FRAME_DESC_ENTRY_MAX
> > - * restricts the total number of streams in a pad, although the stream ID is
> > - * not restricted.
> >   */
> >  #define V4L2_SUBDEV_MAX_STREAM_ID 63
> >
> > @@ -354,6 +350,7 @@ static int call_set_frame_interval(struct v4l2_subdev *sd,
> >  static int call_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> >  			       struct v4l2_mbus_frame_desc *fd)
> >  {
> > +	unsigned int type;
> >  	unsigned int i;
> >  	int ret;
> >
> > @@ -362,16 +359,38 @@ static int call_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> >  		return -EOPNOTSUPP;
> >  #endif
> >
> > +	type = fd->type;
> >  	memset(fd, 0, sizeof(*fd));
> > +	if (!fd->entry) {
> > +		fd->entry = fd->entry_mem;
> > +		fd->len_entries = ARRAY_SIZE(fd->entry_mem);
> > +	}
> >
> >  	ret = sd->ops->pad->get_frame_desc(sd, pad, fd);
> > +	if (ret == -ENOSPC) {
> > +		if (fd->num_entries > V4L2_FRAME_DESC_ENTRY_PREALLOC &&
> > +		    fd->num_entries < V4L2_FRAME_DESC_ENTRY_MAX) {
>
> nit: you can invert the condition and save one indentation level
>
>                 if (fd->num_entries <= V4L2_FRAME_DESC_ENTRY_PREALLOC ||
>                     fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX)
>                     return -E2BIG;
>
> > +			fd->entry = kzalloc_objs(*fd->entry, fd->num_entries,
> > +						 GFP_KERNEL);
> > +			if (!fd->entry)
> > +				return -ENOMEM;
> > +
> > +			fd->len_entries = fd->num_entries;
> > +			fd->num_entries = 0;
> > +
> > +			ret = sd->ops->pad->get_frame_desc(sd, pad, fd);
> > +		} else {
> > +			return -E2BIG;
> > +		}
> > +	}
> >  	if (ret)
> >  		return ret;
> >
> >  	dev_dbg(sd->dev, "Frame descriptor on pad %u, type %s\n", pad,
> > -		fd->type == V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL ? "parallel" :
> > -		fd->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2 ? "CSI-2" :
> > -		"unknown");
> > +		type == V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL ? "parallel" :
> > +		type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2 ? "CSI-2" : "unknown");
> > +
> > +	fd->type = type;
>
> If we end up calling d->ops->pad->get_frame_desc() don't we overwrite
> the returned type ?
>
> >
> >  	for (i = 0; i < fd->num_entries; i++) {
> >  		struct v4l2_mbus_frame_desc_entry *entry = &fd->entry[i];
> > @@ -1086,9 +1105,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
> >  		 * descriptor accordingly, with up to one entry per route. Until
> >  		 * the frame descriptors entries get allocated dynamically,
> >  		 * limit the number of active routes to
> > -		 * V4L2_FRAME_DESC_ENTRY_MAX.
> > +		 * V4L2_FRAME_DESC_ENTRY_PREALLOC.
> >  		 */
> > -		if (num_active_routes > V4L2_FRAME_DESC_ENTRY_MAX)
> > +		if (num_active_routes > V4L2_FRAME_DESC_ENTRY_PREALLOC)
> >  			return -E2BIG;
> >
> >  		/*
> > @@ -2638,7 +2657,7 @@ int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
> >  				return -EPIPE;
> >  			}
> >
> > -			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
> > +			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_PREALLOC) {
> >  				dev_dbg(dev, "Frame desc entry limit reached\n");
> >  				return -E2BIG;
> >  			}
> > @@ -2730,6 +2749,16 @@ int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc);
> >
> > +void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc)
> > +{
>
> As this might be used as a __cleanup() should we check if desc is
> valid ?
>
> > +	if (desc->entry != desc->entry_mem)
> > +		kfree(desc->entry);

Sorry, in my below comment  ....


> > +
> > +	desc->entry = NULL;
> > +	desc->len_entries = desc->num_entries = 0;
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_subdev_free_frame_desc);
> > +
> >  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
> >
> >  #endif /* CONFIG_MEDIA_CONTROLLER */
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index c9e74566c85a..bd97510c7024 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -365,11 +365,13 @@ struct v4l2_mbus_frame_desc_entry {
> >  	} bus;
> >  };
> >
> > - /*
> > -  * If this number is too small, it should be dropped altogether and the
> > -  * API switched to a dynamic number of frame descriptor entries.
> > -  */
> > -#define V4L2_FRAME_DESC_ENTRY_MAX	8
> > +/* Size of the statically allocated frame descriptor array. */
> > +#define V4L2_FRAME_DESC_ENTRY_PREALLOC	8
> > +/*
> > + * Maximum number of dynamically allocated frame descriptors. Note that
> > + * V4L2_SUBDEV_MAX_STREAM_ID is related to this limit as well.
> > + */
> > +#define V4L2_FRAME_DESC_ENTRY_MAX	64
> >
> >  /**
> >   * enum v4l2_mbus_frame_desc_type - media bus frame description type
> > @@ -392,13 +394,17 @@ enum v4l2_mbus_frame_desc_type {
> >  /**
> >   * struct v4l2_mbus_frame_desc - media bus data frame description
> >   * @type: type of the bus (enum v4l2_mbus_frame_desc_type)
> > - * @entry: frame descriptors array
> > - * @num_entries: number of entries in @entry array
> > + * @entry_mem: memory for the frame descriptors (@entry)
> > + * @entry: pointer to the frame descriptors
> > + * @num_entries: number of entries in @entry
> > + * @len_entries: number of entries allocated for @entry
> >   */
> >  struct v4l2_mbus_frame_desc {
> >  	enum v4l2_mbus_frame_desc_type type;
> > -	struct v4l2_mbus_frame_desc_entry entry[V4L2_FRAME_DESC_ENTRY_MAX];
> > +	struct v4l2_mbus_frame_desc_entry entry_mem[V4L2_FRAME_DESC_ENTRY_PREALLOC];
> > +	struct v4l2_mbus_frame_desc_entry *entry;
> >  	unsigned short num_entries;
> > +	unsigned short len_entries;
> >  };
> >
> >  /**
> > @@ -780,7 +786,13 @@ struct v4l2_subdev_state {
> >   * @link_validate: used by the media controller code to check if the links
> >   *		   that belongs to a pipeline can be used for stream.
> >   *
> > - * @get_frame_desc: get the current low level media bus frame parameters.
> > + * @get_frame_desc: get the current low level media bus frame parameters. The
> > + *		    callback is required to update the num_entries field to the
> > + *		    total number of entries in the frame descriptor. The
> > + *		    callback shall fill the first entries array up to
> > + *		    len_entries, which signifies the number of entries
> > + *		    allocated. If num_entries exceeds len_entries, the callback
> > + *		    shall return -ENOSPC.
> >   *
> >   * @set_frame_desc: set the low level media bus frame parameters, @fd array
> >   *                  may be adjusted by the subdev driver to device capabilities.
> > @@ -1793,11 +1805,25 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
> >   *
> >   * The caller is required to set @desc->type to the expected bus type.
> >   *
> > + * The entries in the frame descriptor are allocated based on the need. The
> > + * caller is required to release the memory of the frame descriptor entries for
>
> Is it unconditionally true ? If fd->entry is not initialized aren't we
> deflecting to the pre-allocated entries in call_get_frame_desc() ?
>
> 	if (!fd->entry) {
> 		fd->entry = fd->entry_mem;
> 		fd->len_entries = ARRAY_SIZE(fd->entry_mem);
> 	}
>
> v4l2_subdev_free_frame_desc() shouldn't be called in this case, right ?

... here I missed this, so it's safe to call
v4l2_subdev_free_frame_desc() unconditionally

>
>
> > + * each frame descriptor obtained by calling this function using
> > + * v4l2_subdev_free_frame_desc().
> > + *
> >   * Return: %0 on success or negative error code on failure.
> >   */
> >  int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> >  			       struct v4l2_mbus_frame_desc *desc);
> >
> > +/**
> > + * v4l2_subdev_free_frame_desc() - Release the memory of a frame descriptor
> > + * @desc: A pointer to a frame descriptor
> > + *
> > + * Release the frame descriptor entries in a frame descriptor. The number of
> > + * entries in the descriptor are set to 0 again.
> > + */
> > +void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc);
> > +
> >  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
> >
> >  #endif /* CONFIG_MEDIA_CONTROLLER */
> > --
> > 2.47.3
> >
> >

Re: [PATCH 04/17] media: v4l2-subdev: Allocate frame descriptors based on the need

[Jacopo Mondi]

Hi Sakari,

   one more comment

On Wed, May 13, 2026 at 01:43:45PM +0300, Sakari Ailus wrote:
> Frame descriptors entries require a small amount of memory per entry (20
> bytes), but if the number of entries in a frame descriptor is large, an
> unreasonably large amount of memory would need to be allocated in the
> stack. Therefore the number of entries has been limited to 8.
>
> Support larger frame descriptors by allocating as much memory as required.
> The get_frame_desc() op can now set the num_entries to a number larger
> than V4L2_FRAME_BUS_ENTRY_MAX and return -ENOSPC. The caller,
> v4l2_subdev_get_frame_desc(), will then allocate memory for that amount of
> memory and call the get_frame_desc() op again.
>
> The caller is also responsible for releasing the allocated memory by
> calling v4l2_subdev_free_frame_desc().
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 49 +++++++++++++++++++++------
>  include/media/v4l2-subdev.h           | 44 +++++++++++++++++++-----
>  2 files changed, 74 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index b8acce8f9c33..012ff6fb2988 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -63,10 +63,6 @@ static bool v4l2_subdev_enable_streams_api;
>  /*
>   * Maximum stream ID is 63 for now, as we use u64 bitmask to represent a set
>   * of streams.
> - *
> - * Note that V4L2_FRAME_DESC_ENTRY_MAX is related: V4L2_FRAME_DESC_ENTRY_MAX
> - * restricts the total number of streams in a pad, although the stream ID is
> - * not restricted.
>   */
>  #define V4L2_SUBDEV_MAX_STREAM_ID 63
>
> @@ -354,6 +350,7 @@ static int call_set_frame_interval(struct v4l2_subdev *sd,
>  static int call_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>  			       struct v4l2_mbus_frame_desc *fd)
>  {
> +	unsigned int type;
>  	unsigned int i;
>  	int ret;
>
> @@ -362,16 +359,38 @@ static int call_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>  		return -EOPNOTSUPP;
>  #endif
>
> +	type = fd->type;
>  	memset(fd, 0, sizeof(*fd));
> +	if (!fd->entry) {
> +		fd->entry = fd->entry_mem;
> +		fd->len_entries = ARRAY_SIZE(fd->entry_mem);
> +	}
>
>  	ret = sd->ops->pad->get_frame_desc(sd, pad, fd);
> +	if (ret == -ENOSPC) {
> +		if (fd->num_entries > V4L2_FRAME_DESC_ENTRY_PREALLOC &&
> +		    fd->num_entries < V4L2_FRAME_DESC_ENTRY_MAX) {
> +			fd->entry = kzalloc_objs(*fd->entry, fd->num_entries,
> +						 GFP_KERNEL);
> +			if (!fd->entry)
> +				return -ENOMEM;
> +
> +			fd->len_entries = fd->num_entries;
> +			fd->num_entries = 0;
> +
> +			ret = sd->ops->pad->get_frame_desc(sd, pad, fd);
> +		} else {
> +			return -E2BIG;
> +		}
> +	}
>  	if (ret)
>  		return ret;
>
>  	dev_dbg(sd->dev, "Frame descriptor on pad %u, type %s\n", pad,
> -		fd->type == V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL ? "parallel" :
> -		fd->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2 ? "CSI-2" :
> -		"unknown");
> +		type == V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL ? "parallel" :
> +		type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2 ? "CSI-2" : "unknown");
> +
> +	fd->type = type;
>
>  	for (i = 0; i < fd->num_entries; i++) {
>  		struct v4l2_mbus_frame_desc_entry *entry = &fd->entry[i];
> @@ -1086,9 +1105,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  		 * descriptor accordingly, with up to one entry per route. Until
>  		 * the frame descriptors entries get allocated dynamically,
>  		 * limit the number of active routes to
> -		 * V4L2_FRAME_DESC_ENTRY_MAX.
> +		 * V4L2_FRAME_DESC_ENTRY_PREALLOC.
>  		 */
> -		if (num_active_routes > V4L2_FRAME_DESC_ENTRY_MAX)
> +		if (num_active_routes > V4L2_FRAME_DESC_ENTRY_PREALLOC)
>  			return -E2BIG;
>
>  		/*
> @@ -2638,7 +2657,7 @@ int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>  				return -EPIPE;
>  			}
>
> -			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
> +			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_PREALLOC) {
>  				dev_dbg(dev, "Frame desc entry limit reached\n");
>  				return -E2BIG;
>  			}
> @@ -2730,6 +2749,16 @@ int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc);
>
> +void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc)
> +{
> +	if (desc->entry != desc->entry_mem)
> +		kfree(desc->entry);
> +
> +	desc->entry = NULL;
> +	desc->len_entries = desc->num_entries = 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_subdev_free_frame_desc);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index c9e74566c85a..bd97510c7024 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -365,11 +365,13 @@ struct v4l2_mbus_frame_desc_entry {
>  	} bus;
>  };
>
> - /*
> -  * If this number is too small, it should be dropped altogether and the
> -  * API switched to a dynamic number of frame descriptor entries.
> -  */
> -#define V4L2_FRAME_DESC_ENTRY_MAX	8
> +/* Size of the statically allocated frame descriptor array. */
> +#define V4L2_FRAME_DESC_ENTRY_PREALLOC	8
> +/*
> + * Maximum number of dynamically allocated frame descriptors. Note that
> + * V4L2_SUBDEV_MAX_STREAM_ID is related to this limit as well.
> + */
> +#define V4L2_FRAME_DESC_ENTRY_MAX	64
>
>  /**
>   * enum v4l2_mbus_frame_desc_type - media bus frame description type
> @@ -392,13 +394,17 @@ enum v4l2_mbus_frame_desc_type {
>  /**
>   * struct v4l2_mbus_frame_desc - media bus data frame description
>   * @type: type of the bus (enum v4l2_mbus_frame_desc_type)
> - * @entry: frame descriptors array
> - * @num_entries: number of entries in @entry array
> + * @entry_mem: memory for the frame descriptors (@entry)
> + * @entry: pointer to the frame descriptors
> + * @num_entries: number of entries in @entry
> + * @len_entries: number of entries allocated for @entry
>   */
>  struct v4l2_mbus_frame_desc {
>  	enum v4l2_mbus_frame_desc_type type;
> -	struct v4l2_mbus_frame_desc_entry entry[V4L2_FRAME_DESC_ENTRY_MAX];
> +	struct v4l2_mbus_frame_desc_entry entry_mem[V4L2_FRAME_DESC_ENTRY_PREALLOC];
> +	struct v4l2_mbus_frame_desc_entry *entry;
>  	unsigned short num_entries;
> +	unsigned short len_entries;

What are the implications of this change on this part of
v4l2_subdev_get_frame_desc() ?

	struct v4l2_subdev_state *state =
		v4l2_subdev_lock_and_get_active_state(sd);
	ret = v4l2_subdev_call(sd, pad, get_fmt, state, &subdev_fmt);
	v4l2_subdev_unlock_state(state);
	if (ret)
		return ret;

	struct v4l2_mbus_frame_desc_entry entry = {
		.pixelcode = subdev_fmt.format.code,
	};

	if (desc->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
		int dt;

		dt = mipi_csi2_dt_for_mbus(subdev_fmt.format.code);
		if (dt < 0)
			return dt;

		entry.bus.csi2.dt = dt;
	}

	desc->entry[0] = entry;

        ^
        |---- here ?

Is there any risk this now access invalid memory ?
Should we do the same thing that you're doing call_get_frame_desc() ?

	fd->entry = fd->entry_mem;

before assigning its first member ?

>  };
>
>  /**
> @@ -780,7 +786,13 @@ struct v4l2_subdev_state {
>   * @link_validate: used by the media controller code to check if the links
>   *		   that belongs to a pipeline can be used for stream.
>   *
> - * @get_frame_desc: get the current low level media bus frame parameters.
> + * @get_frame_desc: get the current low level media bus frame parameters. The
> + *		    callback is required to update the num_entries field to the
> + *		    total number of entries in the frame descriptor. The
> + *		    callback shall fill the first entries array up to
> + *		    len_entries, which signifies the number of entries
> + *		    allocated. If num_entries exceeds len_entries, the callback
> + *		    shall return -ENOSPC.
>   *
>   * @set_frame_desc: set the low level media bus frame parameters, @fd array
>   *                  may be adjusted by the subdev driver to device capabilities.
> @@ -1793,11 +1805,25 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
>   *
>   * The caller is required to set @desc->type to the expected bus type.
>   *
> + * The entries in the frame descriptor are allocated based on the need. The
> + * caller is required to release the memory of the frame descriptor entries for
> + * each frame descriptor obtained by calling this function using
> + * v4l2_subdev_free_frame_desc().
> + *
>   * Return: %0 on success or negative error code on failure.
>   */
>  int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>  			       struct v4l2_mbus_frame_desc *desc);
>
> +/**
> + * v4l2_subdev_free_frame_desc() - Release the memory of a frame descriptor
> + * @desc: A pointer to a frame descriptor
> + *
> + * Release the frame descriptor entries in a frame descriptor. The number of
> + * entries in the descriptor are set to 0 again.
> + */
> +void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc);
> +
>  #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
>  #endif /* CONFIG_MEDIA_CONTROLLER */
> --
> 2.47.3
>

[PATCH 05/17] media: v4l2-subdev: Provide a cleanup-friendly get_frame_desc

[Sakari Ailus]

Provide a cleanup-friendly get_frame_desc() API function that returns a
frame descriptor that can be automatically released using __free().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 36 ++++++++++++++++++++++++---
 include/media/v4l2-subdev.h           | 22 +++++++++++-----
 2 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 012ff6fb2988..a49aaad595af 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -2691,8 +2691,8 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc_passthrough);
 
-int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
-			       struct v4l2_mbus_frame_desc *desc)
+static int __v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+					struct v4l2_mbus_frame_desc *desc)
 {
 	struct v4l2_subdev_format subdev_fmt = {
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
@@ -2747,9 +2747,33 @@ int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 
 	return 0;
 }
+
+struct v4l2_mbus_frame_desc *
+v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+			   enum v4l2_mbus_frame_desc_type type)
+{
+	struct v4l2_mbus_frame_desc *desc;
+	int ret;
+
+	desc = kzalloc_obj(*desc, GFP_KERNEL);
+	if (!desc)
+		return ERR_PTR(-ENOMEM);
+
+	desc->type = type;
+	desc->entry = desc->entry_mem;
+	desc->len_entries = ARRAY_SIZE(desc->entry_mem);
+
+	ret = __v4l2_subdev_get_frame_desc(sd, pad, desc);
+	if (ret) {
+		kfree(desc);
+		return ERR_PTR(ret);
+	}
+
+	return desc;
+}
 EXPORT_SYMBOL_GPL(v4l2_subdev_get_frame_desc);
 
-void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc)
+static void __v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc)
 {
 	if (desc->entry != desc->entry_mem)
 		kfree(desc->entry);
@@ -2757,6 +2781,12 @@ void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc)
 	desc->entry = NULL;
 	desc->len_entries = desc->num_entries = 0;
 }
+
+void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc)
+{
+	__v4l2_subdev_free_frame_desc(desc);
+	kfree(desc);
+}
 EXPORT_SYMBOL_GPL(v4l2_subdev_free_frame_desc);
 
 #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index bd97510c7024..6e40420c2fcc 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -8,6 +8,7 @@
 #ifndef _V4L2_SUBDEV_H
 #define _V4L2_SUBDEV_H
 
+#include <linux/cleanup.h>
 #include <linux/types.h>
 #include <linux/v4l2-subdev.h>
 #include <media/media-entity.h>
@@ -1794,7 +1795,7 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
  * v4l2_subdev_get_frame_desc() - Get a frame descriptor for a pad
  * @sd: The sub-device
  * @pad: The number of the pad in @sd from which to obtain the frame descriptor
- * @desc: A pointer to a frame descriptor, with its type field set
+ * @type: The type of the frame descriptor
  *
  * Obtain a frame descriptor from a sub-device. If the sub-device supports the
  * get_frame_desc pad operation, its result is returned, just like calling it
@@ -1810,20 +1811,29 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
  * each frame descriptor obtained by calling this function using
  * v4l2_subdev_free_frame_desc().
  *
- * Return: %0 on success or negative error code on failure.
+ * Use __free() to release the frame descriptor automatically:
+ *
+ * struct v4l2_mbus_frame_desc *desc __free(v4l2_subdev_free_frame_desc) =
+ *	v4l2_subdev_get_frame_desc(sd, pad, desc);
+ *
+ * Return: The frame descriptor on success or a negative error code on failure.
  */
-int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
-			       struct v4l2_mbus_frame_desc *desc);
+struct v4l2_mbus_frame_desc *
+v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+			   enum v4l2_mbus_frame_desc_type type);
 
 /**
  * v4l2_subdev_free_frame_desc() - Release the memory of a frame descriptor
  * @desc: A pointer to a frame descriptor
  *
- * Release the frame descriptor entries in a frame descriptor. The number of
- * entries in the descriptor are set to 0 again.
+ * Release the frame descriptor entries and the frame descriptor itself.
  */
 void v4l2_subdev_free_frame_desc(struct v4l2_mbus_frame_desc *desc);
 
+DEFINE_FREE(v4l2_subdev_free_frame_desc, struct v4l2_mbus_frame_desc *, \
+	    if (!IS_ERR_OR_NULL(_T))					\
+		    v4l2_subdev_free_frame_desc(_T))
+
 #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
 
 #endif /* CONFIG_MEDIA_CONTROLLER */
-- 
2.47.3

[PATCH 06/17] media: v4l2-subdev: Change the maximum number of routes

[Sakari Ailus]

As the frame descriptors are now allocated dynamically, allow as many
routes that there can be dynamically allocated frame descriptors.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index a49aaad595af..43d8c0f8701f 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1100,14 +1100,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 				num_active_routes++;
 		}
 
-		/*
-		 * Drivers that implement routing need to report a frame
-		 * descriptor accordingly, with up to one entry per route. Until
-		 * the frame descriptors entries get allocated dynamically,
-		 * limit the number of active routes to
-		 * V4L2_FRAME_DESC_ENTRY_PREALLOC.
-		 */
-		if (num_active_routes > V4L2_FRAME_DESC_ENTRY_PREALLOC)
+		if (num_active_routes > V4L2_FRAME_DESC_ENTRY_MAX)
 			return -E2BIG;
 
 		/*
-- 
2.47.3

[PATCH 07/17] media: v4l2-subdev: Return dynamically allocated pass-through routes

[Sakari Ailus]

Count the number of pass-through routes and then return the full table
once enough memory is available for it.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 43d8c0f8701f..9d7f616ab7ea 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -2650,20 +2650,22 @@ int __v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
 				return -EPIPE;
 			}
 
-			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_PREALLOC) {
+			if (fd->num_entries >= V4L2_FRAME_DESC_ENTRY_MAX) {
 				dev_dbg(dev, "Frame desc entry limit reached\n");
 				return -E2BIG;
 			}
 
-			fd->entry[fd->num_entries] = *source_entry;
-
-			fd->entry[fd->num_entries].stream = route->source_stream;
+			if (fd->num_entries < fd->len_entries) {
+				fd->entry[fd->num_entries] = *source_entry;
+				fd->entry[fd->num_entries].stream =
+					route->source_stream;
+			}
 
 			fd->num_entries++;
 		}
 	}
 
-	return 0;
+	return fd->num_entries < fd->len_entries ? 0 : -ENOSPC;
 }
 EXPORT_SYMBOL_GPL(__v4l2_subdev_get_frame_desc_passthrough);
 
-- 
2.47.3

[PATCH 08/17] media: v4l2-subdev: Always return at least one frame descriptor

[Sakari Ailus]

Make v4l2_subdev_get_frame_desc() return at least one frame descriptor
entry or an error. Empty frame descriptors aren't useful for callers so
callers can now omit this check.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 5 +++++
 include/media/v4l2-subdev.h           | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 9d7f616ab7ea..b92ae87fa5d0 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -2709,6 +2709,11 @@ static int __v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad
 			return -EINVAL;
 		}
 
+		if (!desc->num_entries) {
+			dev_dbg(sd->dev, "no frame descriptor entries\n");
+			return -EINVAL;
+		}
+
 		return ret;
 	}
 
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 6e40420c2fcc..8b10dcab5590 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1816,6 +1816,8 @@ int v4l2_subdev_get_frame_desc_passthrough(struct v4l2_subdev *sd,
  * struct v4l2_mbus_frame_desc *desc __free(v4l2_subdev_free_frame_desc) =
  *	v4l2_subdev_get_frame_desc(sd, pad, desc);
  *
+ * The returned frame descriptor will contain at least one entry.
+ *
  * Return: The frame descriptor on success or a negative error code on failure.
  */
 struct v4l2_mbus_frame_desc *
-- 
2.47.3

[PATCH 09/17] media: bcm2835-unicam: Use v4l2_subdev_get_frame_desc()

[Sakari Ailus]

Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
preferred over calling the get_frame_desc() pad operation directly.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../media/platform/broadcom/bcm2835-unicam.c  | 20 +++++++++----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
index 8d28ba0b59a3..2e78b16bf18c 100644
--- a/drivers/media/platform/broadcom/bcm2835-unicam.c
+++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
@@ -888,7 +888,8 @@ static int unicam_get_image_vc_dt(struct unicam_device *unicam,
 				  struct v4l2_subdev_state *state,
 				  u8 *vc, u8 *dt)
 {
-	struct v4l2_mbus_frame_desc fd;
+	struct v4l2_mbus_frame_desc *fd __free(v4l2_subdev_free_frame_desc) =
+		NULL;
 	u32 stream;
 	int ret;
 
@@ -898,17 +899,14 @@ static int unicam_get_image_vc_dt(struct unicam_device *unicam,
 	if (ret)
 		return ret;
 
-	ret = v4l2_subdev_call(unicam->sensor.subdev, pad, get_frame_desc,
-			       unicam->sensor.pad->index, &fd);
-	if (ret)
-		return ret;
-
-	/* Only CSI-2 supports DTs. */
-	if (fd.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2)
-		return -EINVAL;
+	fd = v4l2_subdev_get_frame_desc(unicam->sensor.subdev,
+					unicam->sensor.pad->index,
+					V4L2_MBUS_FRAME_DESC_TYPE_CSI2);
+	if (IS_ERR(fd))
+		return PTR_ERR(fd);
 
-	for (unsigned int i = 0; i < fd.num_entries; ++i) {
-		const struct v4l2_mbus_frame_desc_entry *fde = &fd.entry[i];
+	for (unsigned int i = 0; i < fd->num_entries; ++i) {
+		const struct v4l2_mbus_frame_desc_entry *fde = &fd->entry[i];
 
 		if (fde->stream == stream) {
 			*vc = fde->bus.csi2.vc;
-- 
2.47.3

[PATCH 10/17] media: nxp: imx8-isi: Use v4l2_subdev_get_frame_desc()

[Sakari Ailus]

Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
preferred over calling the get_frame_desc() pad operation directly.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../platform/nxp/imx8-isi/imx8-isi-crossbar.c     | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
index 605a45124103..5bb536465c57 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2022 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/kernel.h>
@@ -32,7 +33,8 @@ static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
 	struct mxc_isi_dev *isi = xbar->isi;
 	const struct mxc_gasket_ops *gasket_ops = isi->pdata->gasket_ops;
 	const struct v4l2_mbus_framefmt *fmt;
-	struct v4l2_mbus_frame_desc fd;
+	struct v4l2_mbus_frame_desc *fd __free(v4l2_subdev_free_frame_desc) =
+		NULL;
 	int ret;
 
 	if (!gasket_ops)
@@ -44,15 +46,16 @@ static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
 	 * to match the configuration of the CSIS.
 	 */
 
-	ret = v4l2_subdev_call(remote_sd, pad, get_frame_desc, remote_pad, &fd);
+	fd = v4l2_subdev_get_fmame_desc(remote_sd, remote_pad,
+					V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL);
 	if (ret) {
 		dev_err(isi->dev,
 			"failed to get frame descriptor from '%s':%u: %d\n",
-			remote_sd->name, remote_pad, ret);
-		return ret;
+			remote_sd->name, remote_pad, PTR_ERR(ret));
+		return PTR_ERR(ret);
 	}
 
-	if (fd.num_entries != 1) {
+	if (fd->num_entries != 1) {
 		dev_err(isi->dev, "invalid frame descriptor for '%s':%u\n",
 			remote_sd->name, remote_pad);
 		return -EINVAL;
@@ -62,7 +65,7 @@ static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
 	if (!fmt)
 		return -EINVAL;
 
-	gasket_ops->enable(isi, &fd, fmt, port);
+	gasket_ops->enable(isi, fd, fmt, port);
 	return 0;
 }
 
-- 
2.47.3

Re: [PATCH 10/17] media: nxp: imx8-isi: Use v4l2_subdev_get_frame_desc()

[Frank Li]

On Wed, May 13, 2026 at 01:43:51PM +0300, Sakari Ailus wrote:
> Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
> preferred over calling the get_frame_desc() pad operation directly.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  .../platform/nxp/imx8-isi/imx8-isi-crossbar.c     | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> index 605a45124103..5bb536465c57 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2022 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>   */
>
> +#include <linux/cleanup.h>
>  #include <linux/device.h>
>  #include <linux/errno.h>
>  #include <linux/kernel.h>
> @@ -32,7 +33,8 @@ static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
>  	struct mxc_isi_dev *isi = xbar->isi;
>  	const struct mxc_gasket_ops *gasket_ops = isi->pdata->gasket_ops;
>  	const struct v4l2_mbus_framefmt *fmt;
> -	struct v4l2_mbus_frame_desc fd;
> +	struct v4l2_mbus_frame_desc *fd __free(v4l2_subdev_free_frame_desc) =
> +		NULL;
>  	int ret;
>
>  	if (!gasket_ops)
> @@ -44,15 +46,16 @@ static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
>  	 * to match the configuration of the CSIS.
>  	 */
>
> -	ret = v4l2_subdev_call(remote_sd, pad, get_frame_desc, remote_pad, &fd);
> +	fd = v4l2_subdev_get_fmame_desc(remote_sd, remote_pad,
> +					V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL);

cleanup.h perfer declear fd here

	struct v4l2_mbus_frame_desc *fd __free(v4l2_subdev_free_frame_desc) =
		v4l2_subdev_get_fmame_desc()


>  	if (ret) {

	ret is not set

	if (IS_ERR(fd))

Frank
>  		dev_err(isi->dev,
>  			"failed to get frame descriptor from '%s':%u: %d\n",
> -			remote_sd->name, remote_pad, ret);
> -		return ret;
> +			remote_sd->name, remote_pad, PTR_ERR(ret));
> +		return PTR_ERR(ret);
>  	}
>
> -	if (fd.num_entries != 1) {
> +	if (fd->num_entries != 1) {
>  		dev_err(isi->dev, "invalid frame descriptor for '%s':%u\n",
>  			remote_sd->name, remote_pad);
>  		return -EINVAL;
> @@ -62,7 +65,7 @@ static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
>  	if (!fmt)
>  		return -EINVAL;
>
> -	gasket_ops->enable(isi, &fd, fmt, port);
> +	gasket_ops->enable(isi, fd, fmt, port);
>  	return 0;
>  }
>
> --
> 2.47.3
>

Re: [PATCH 10/17] media: nxp: imx8-isi: Use v4l2_subdev_get_frame_desc()

[Sakari Ailus]

Hi Frank,

On Wed, May 13, 2026 at 11:58:33PM -0400, Frank Li wrote:
> On Wed, May 13, 2026 at 01:43:51PM +0300, Sakari Ailus wrote:
> > Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
> > preferred over calling the get_frame_desc() pad operation directly.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  .../platform/nxp/imx8-isi/imx8-isi-crossbar.c     | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> > index 605a45124103..5bb536465c57 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> > @@ -5,6 +5,7 @@
> >   * Copyright (c) 2022 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >   */
> >
> > +#include <linux/cleanup.h>
> >  #include <linux/device.h>
> >  #include <linux/errno.h>
> >  #include <linux/kernel.h>
> > @@ -32,7 +33,8 @@ static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
> >  	struct mxc_isi_dev *isi = xbar->isi;
> >  	const struct mxc_gasket_ops *gasket_ops = isi->pdata->gasket_ops;
> >  	const struct v4l2_mbus_framefmt *fmt;
> > -	struct v4l2_mbus_frame_desc fd;
> > +	struct v4l2_mbus_frame_desc *fd __free(v4l2_subdev_free_frame_desc) =
> > +		NULL;
> >  	int ret;
> >
> >  	if (!gasket_ops)
> > @@ -44,15 +46,16 @@ static int mxc_isi_crossbar_gasket_enable(struct mxc_isi_crossbar *xbar,
> >  	 * to match the configuration of the CSIS.
> >  	 */
> >
> > -	ret = v4l2_subdev_call(remote_sd, pad, get_frame_desc, remote_pad, &fd);
> > +	fd = v4l2_subdev_get_fmame_desc(remote_sd, remote_pad,
> > +					V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL);
> 
> cleanup.h perfer declear fd here
> 
> 	struct v4l2_mbus_frame_desc *fd __free(v4l2_subdev_free_frame_desc) =
> 		v4l2_subdev_get_fmame_desc()

That would indeed make sense. I'll take this into account for v2.

-- 
Regards,

Sakari Ailus

[PATCH 11/17] media: raspberrypi: cfe: Use v4l2_subdev_get_frame_desc()

[Sakari Ailus]

Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
preferred over calling the get_frame_desc() pad operation directly.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../media/platform/raspberrypi/rp1-cfe/cfe.c  | 29 +++++++++----------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/raspberrypi/rp1-cfe/cfe.c b/drivers/media/platform/raspberrypi/rp1-cfe/cfe.c
index 8375ed3e97b9..111597c5b031 100644
--- a/drivers/media/platform/raspberrypi/rp1-cfe/cfe.c
+++ b/drivers/media/platform/raspberrypi/rp1-cfe/cfe.c
@@ -6,6 +6,7 @@
  * Copyright (c) 2023-2024 Ideas on Board Oy
  */
 
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
@@ -803,7 +804,8 @@ static int cfe_get_vc_dt_fallback(struct cfe_device *cfe, u8 *vc, u8 *dt)
 static int cfe_get_vc_dt(struct cfe_device *cfe, unsigned int channel, u8 *vc,
 			 u8 *dt)
 {
-	struct v4l2_mbus_frame_desc remote_desc;
+	struct v4l2_mbus_frame_desc *fd __free(v4l2_subdev_free_frame_desc) =
+		NULL;
 	struct v4l2_subdev_state *state;
 	u32 sink_stream;
 	unsigned int i;
@@ -816,34 +818,29 @@ static int cfe_get_vc_dt(struct cfe_device *cfe, unsigned int channel, u8 *vc,
 	if (ret)
 		return ret;
 
-	ret = v4l2_subdev_call(cfe->source_sd, pad, get_frame_desc,
-			       cfe->source_pad, &remote_desc);
-	if (ret == -ENOIOCTLCMD) {
+	fd = v4l2_subdev_get_frame_desc(cfe->source_sd, cfe->source_pad,
+					V4L2_MBUS_FRAME_DESC_TYPE_CSI2);
+	if (PTR_ERR(fd) == -ENOIOCTLCMD) {
 		cfe_dbg(cfe, "source does not support get_frame_desc, use fallback\n");
 		return cfe_get_vc_dt_fallback(cfe, vc, dt);
-	} else if (ret) {
+	} else if (IS_ERR(fd)) {
 		cfe_err(cfe, "Failed to get frame descriptor\n");
-		return ret;
-	}
-
-	if (remote_desc.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
-		cfe_err(cfe, "Frame descriptor does not describe CSI-2 link");
-		return -EINVAL;
+		return PTR_ERR(fd);
 	}
 
-	for (i = 0; i < remote_desc.num_entries; i++) {
-		if (remote_desc.entry[i].stream == sink_stream)
+	for (i = 0; i < fd->num_entries; i++) {
+		if (fd->entry[i].stream == sink_stream)
 			break;
 	}
 
-	if (i == remote_desc.num_entries) {
+	if (i == fd->num_entries) {
 		cfe_err(cfe, "Stream %u not found in remote frame desc\n",
 			sink_stream);
 		return -EINVAL;
 	}
 
-	*vc = remote_desc.entry[i].bus.csi2.vc;
-	*dt = remote_desc.entry[i].bus.csi2.dt;
+	*vc = fd->entry[i].bus.csi2.vc;
+	*dt = fd->entry[i].bus.csi2.dt;
 
 	return 0;
 }
-- 
2.47.3

[PATCH 12/17] media: rzg2l-cru: Use v4l2_subdev_get_frame_desc()

[Sakari Ailus]

Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
preferred over calling the get_frame_desc() pad operation directly.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../platform/renesas/rzg2l-cru/rzg2l-video.c  | 21 ++++---------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
index 162e2ace6931..96c899bb542a 100644
--- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
@@ -11,6 +11,7 @@
  * Copyright (C) 2008 Magnus Damm
  */
 
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/pm_runtime.h>
@@ -411,24 +412,10 @@ static int rzg2l_cru_get_virtual_channel(struct rzg2l_cru_dev *cru)
 	int ret;
 
 	remote_pad = media_pad_remote_pad_unique(&cru->ip.pads[RZG2L_CRU_IP_SINK]);
-	ret = v4l2_subdev_call(cru->ip.remote, pad, get_frame_desc, remote_pad->index, &fd);
-	if (ret < 0 && ret != -ENOIOCTLCMD) {
-		dev_err(cru->dev, "get_frame_desc failed on IP remote subdev\n");
+	ret = v4l2_subdev_get_frame_desc(cru->ip.remote, remote_pad->index,
+					 V4L2_MBUS_FRAME_DESC_TYPE_CSI2);
+	if (ret < 0)
 		return ret;
-	}
-	/* If remote subdev does not implement .get_frame_desc default to VC0. */
-	if (ret == -ENOIOCTLCMD)
-		return 0;
-
-	if (fd.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
-		dev_err(cru->dev, "get_frame_desc returned invalid bus type %d\n", fd.type);
-		return -EINVAL;
-	}
-
-	if (!fd.num_entries) {
-		dev_err(cru->dev, "get_frame_desc returned zero entries\n");
-		return -EINVAL;
-	}
 
 	return fd.entry[0].bus.csi2.vc;
 }
-- 
2.47.3

[PATCH 13/17] media: rkisp1: Use v4l2_subdev_get_frame_desc()

[Sakari Ailus]

Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
preferred over calling the get_frame_desc() pad operation directly.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
index 2311672cedb1..7e94ff2037a6 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -8,6 +8,7 @@
  * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
  */
 
+#include <linux/cleanup.h>
 #include <linux/iopoll.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
@@ -87,7 +88,8 @@ static int rkisp1_gasket_enable(struct rkisp1_device *rkisp1,
 				struct media_pad *source)
 {
 	struct v4l2_subdev *source_sd;
-	struct v4l2_mbus_frame_desc fd;
+	struct v4l2_mbus_frame_desc *fd = __free(v4l2_subdev_free_frame_desc) =
+		NULL;
 	unsigned int dt;
 	u32 mask;
 	u32 val;
@@ -101,8 +103,8 @@ static int rkisp1_gasket_enable(struct rkisp1_device *rkisp1,
 	 */
 
 	source_sd = media_entity_to_v4l2_subdev(source->entity);
-	ret = v4l2_subdev_call(source_sd, pad, get_frame_desc,
-			       source->index, &fd);
+	fd = v4l2_subdev_get_frame_desc(source_sd, source->index,
+					V4L2_MBUS_FRAME_DESC_TYPE_CSI2);
 	if (ret) {
 		dev_err(rkisp1->dev,
 			"failed to get frame descriptor from '%s':%u: %d\n",
@@ -110,13 +112,13 @@ static int rkisp1_gasket_enable(struct rkisp1_device *rkisp1,
 		return ret;
 	}
 
-	if (fd.num_entries != 1) {
+	if (fd->num_entries != 1) {
 		dev_err(rkisp1->dev, "invalid frame descriptor for '%s':%u\n",
 			source_sd->name, 0);
 		return -EINVAL;
 	}
 
-	dt = fd.entry[0].bus.csi2.dt;
+	dt = fd->entry[0].bus.csi2.dt;
 
 	if (rkisp1->gasket_id == 0) {
 		mask = ISP_DEWARP_CONTROL_MIPI_CSI1_HS_POLARITY
-- 
2.47.3

[PATCH 14/17] media: exynos4-is: Use v4l2_subdev_get_frame_desc()

[Sakari Ailus]

Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
preferred over calling the get_frame_desc() pad operation directly.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../platform/samsung/exynos4-is/fimc-capture.c   | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
index d85811f4b8c5..7f463871ea0c 100644
--- a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c
@@ -6,6 +6,7 @@
  * Sylwester Nawrocki <s.nawrocki@samsung.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
@@ -853,18 +854,23 @@ static int fimc_get_sensor_frame_desc(struct v4l2_subdev *sensor,
 				      struct v4l2_plane_pix_format *plane_fmt,
 				      unsigned int num_planes, bool try)
 {
-	struct v4l2_mbus_frame_desc fd = { };
+	struct v4l2_mbus_frame_desc *alloc_fd
+		__free(v4l2_subdev_free_frame_desc) = NULL, fd = { };
 	int i, ret;
 	int pad;
 
 	for (i = 0; i < num_planes; i++)
-		fd.entry[i].length = plane_fmt[i].sizeimage;
+		set_fd.entry[i].length = plane_fmt[i].sizeimage;
 
 	pad = sensor->entity.num_pads - 1;
-	if (try)
+	if (try) {
 		ret = v4l2_subdev_call(sensor, pad, set_frame_desc, pad, &fd);
-	else
-		ret = v4l2_subdev_call(sensor, pad, get_frame_desc, pad, &fd);
+	} else {
+		alloc_fd = v4l2_subdev_get_frame_desc(sensor, pad,
+						      V4L2_MBUS_FRAME_DESC_TYPE_CSI2);
+		ret = PTR_ERR(alloc_fd);
+		fd = *alloc_fd;
+	}
 
 	if (ret < 0)
 		return ret;
-- 
2.47.3

[PATCH 15/17] media: ti: cal: Use v4l2_subdev_get_frame_desc()

[Sakari Ailus]

Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
preferred over calling the get_frame_desc() pad operation directly.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/platform/ti/cal/cal-camerarx.c | 25 +++++++++-----------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
index 00a71dac0ff4..8a81fecc00af 100644
--- a/drivers/media/platform/ti/cal/cal-camerarx.c
+++ b/drivers/media/platform/ti/cal/cal-camerarx.c
@@ -9,6 +9,7 @@
  *	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/mfd/syscon.h>
@@ -872,7 +873,8 @@ static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 				       struct v4l2_mbus_frame_desc *fd)
 {
 	struct cal_camerarx *phy = to_cal_camerarx(sd);
-	struct v4l2_mbus_frame_desc remote_desc;
+	struct v4l2_mbus_frame_desc *remote_desc
+		__free(v4l2_subdev_free_frame_desc) = NULL;
 	const struct media_pad *remote_pad;
 	struct v4l2_subdev_state *state;
 	u32 sink_stream;
@@ -893,24 +895,19 @@ static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 		goto out_unlock;
 	}
 
-	ret = v4l2_subdev_call(phy->source, pad, get_frame_desc,
-			       remote_pad->index, &remote_desc);
-	if (ret)
-		goto out_unlock;
-
-	if (remote_desc.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
-		cal_err(phy->cal,
-			"Frame descriptor does not describe CSI-2 link");
-		ret = -EINVAL;
+	remote_desc = v4l2_subdev_get_frame_desc(phy->source, remote_pad->index,
+						 V4L2_MBUS_FRAME_DESC_TYPE_CSI2);
+	if (IS_ERR(remote_desc)) {
+		ret = PTR_ERR(remote_desc);
 		goto out_unlock;
 	}
 
-	for (i = 0; i < remote_desc.num_entries; i++) {
-		if (remote_desc.entry[i].stream == sink_stream)
+	for (i = 0; i < remote_desc->num_entries; i++) {
+		if (remote_desc->entry[i].stream == sink_stream)
 			break;
 	}
 
-	if (i == remote_desc.num_entries) {
+	if (i == remote_desc->num_entries) {
 		cal_err(phy->cal, "Stream %u not found in remote frame desc\n",
 			sink_stream);
 		ret = -EINVAL;
@@ -919,7 +916,7 @@ static int cal_camerarx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 
 	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
 	fd->num_entries = 1;
-	fd->entry[0] = remote_desc.entry[i];
+	fd->entry[0] = remote_desc->entry[i];
 
 out_unlock:
 	v4l2_subdev_unlock_state(state);
-- 
2.47.3

[PATCH 16/17] media: ipu6: Use v4l2_subdev_get_frame_desc()

[Sakari Ailus]

Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
preferred over calling the get_frame_desc() pad operation directly.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 23 ++++++++-----------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
index 7e539a0c6c92..b47f65d7b668 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
@@ -6,6 +6,7 @@
 #include <linux/atomic.h>
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -600,11 +601,11 @@ int ipu6_isys_csi2_get_remote_desc(u32 source_stream,
 {
 	struct v4l2_mbus_frame_desc_entry *desc_entry = NULL;
 	struct device *dev = &csi2->isys->adev->auxdev.dev;
-	struct v4l2_mbus_frame_desc desc;
+	struct v4l2_mbus_frame_desc *desc __free(v4l2_subdev_free_frame_desc) =
+		NULL;
 	struct v4l2_subdev *source;
 	struct media_pad *pad;
 	unsigned int i;
-	int ret;
 
 	source = media_entity_to_v4l2_subdev(source_entity);
 	if (!source)
@@ -614,18 +615,14 @@ int ipu6_isys_csi2_get_remote_desc(u32 source_stream,
 	if (!pad)
 		return -EPIPE;
 
-	ret = v4l2_subdev_call(source, pad, get_frame_desc, pad->index, &desc);
-	if (ret)
-		return ret;
-
-	if (desc.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
-		dev_err(dev, "Unsupported frame descriptor type\n");
-		return -EINVAL;
-	}
+	desc = v4l2_subdev_get_frame_desc(source, pad->index,
+					  V4L2_MBUS_FRAME_DESC_TYPE_CSI2);
+	if (IS_ERR(desc))
+		return PTR_ERR(desc);
 
-	for (i = 0; i < desc.num_entries; i++) {
-		if (source_stream == desc.entry[i].stream) {
-			desc_entry = &desc.entry[i];
+	for (i = 0; i < desc->num_entries; i++) {
+		if (source_stream == desc->entry[i].stream) {
+			desc_entry = &desc->entry[i];
 			break;
 		}
 	}
-- 
2.47.3

[PATCH 17/17] staging: media: ipu7: Use v4l2_subdev_get_frame_desc()

[Sakari Ailus]

Call v4l2_subdev_get_frame_desc() to obtain the frame descriptor. This is
preferred over calling the get_frame_desc() pad operation directly.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../staging/media/ipu7/ipu7-isys-csi-phy.c    | 16 ++++++--------
 drivers/staging/media/ipu7/ipu7-isys-csi2.c   | 22 +++++++++----------
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/media/ipu7/ipu7-isys-csi-phy.c b/drivers/staging/media/ipu7/ipu7-isys-csi-phy.c
index 3f15af3b4c79..75ac77e1b782 100644
--- a/drivers/staging/media/ipu7/ipu7-isys-csi-phy.c
+++ b/drivers/staging/media/ipu7/ipu7-isys-csi-phy.c
@@ -5,6 +5,7 @@
 
 #include <linux/bitmap.h>
 #include <linux/bug.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/iopoll.h>
@@ -300,7 +301,8 @@ static int ipu7_isys_csi_ctrl_dids_config(struct ipu7_isys_csi2 *csi2, u32 id)
 {
 	struct v4l2_mbus_frame_desc_entry *desc_entry = NULL;
 	struct device *dev = &csi2->isys->adev->auxdev.dev;
-	struct v4l2_mbus_frame_desc desc;
+	struct v4l2_mbus_frame_desc *desc __free(v4l2_subdev_free_frame_desc) =
+		NULL;
 	struct v4l2_subdev *ext_sd;
 	struct media_pad *pad;
 	unsigned int i;
@@ -318,17 +320,13 @@ static int ipu7_isys_csi_ctrl_dids_config(struct ipu7_isys_csi2 *csi2, u32 id)
 		 pad->entity->name))
 		return -ENODEV;
 
-	ret = v4l2_subdev_call(ext_sd, pad, get_frame_desc, pad->index, &desc);
+	desc = v4l2_subdev_get_frame_desc(ext_sd, pad->index,
+					  V4L2_MBUS_FRAME_DESC_TYPE_CSI2);
 	if (ret)
 		return ret;
 
-	if (desc.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
-		dev_warn(dev, "Unsupported frame descriptor type\n");
-		return -EINVAL;
-	}
-
-	for (i = 0; i < desc.num_entries; i++) {
-		desc_entry = &desc.entry[i];
+	for (i = 0; i < desc->num_entries; i++) {
+		desc_entry = &desc->entry[i];
 		if (desc_entry->bus.csi2.vc < IPU7_NR_OF_CSI2_VC) {
 			ret = __dids_config(csi2, id, desc_entry->bus.csi2.vc,
 					    desc_entry->bus.csi2.dt);
diff --git a/drivers/staging/media/ipu7/ipu7-isys-csi2.c b/drivers/staging/media/ipu7/ipu7-isys-csi2.c
index f34eabfe8a98..5a4a53ae1104 100644
--- a/drivers/staging/media/ipu7/ipu7-isys-csi2.c
+++ b/drivers/staging/media/ipu7/ipu7-isys-csi2.c
@@ -6,6 +6,7 @@
 #include <linux/atomic.h>
 #include <linux/bits.h>
 #include <linux/bug.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/io.h>
@@ -491,7 +492,8 @@ int ipu7_isys_csi2_get_remote_desc(u32 source_stream,
 {
 	struct v4l2_mbus_frame_desc_entry *desc_entry = NULL;
 	struct device *dev = &csi2->isys->adev->auxdev.dev;
-	struct v4l2_mbus_frame_desc desc;
+	struct v4l2_mbus_frame_desc *desc __free(v4l2_subdev_free_frame_desc) =
+		NULL;
 	struct v4l2_subdev *source;
 	struct media_pad *pad;
 	unsigned int i;
@@ -505,18 +507,14 @@ int ipu7_isys_csi2_get_remote_desc(u32 source_stream,
 	if (!pad)
 		return -EPIPE;
 
-	ret = v4l2_subdev_call(source, pad, get_frame_desc, pad->index, &desc);
+	desc = v4l2_subdev_get_frame_desc(source, pad->index,
+					  V4L2_MBUS_FRAME_DESC_TYPE_CSI2);
 	if (ret)
 		return ret;
 
-	if (desc.type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
-		dev_err(dev, "Unsupported frame descriptor type\n");
-		return -EINVAL;
-	}
-
-	for (i = 0; i < desc.num_entries; i++) {
-		if (source_stream == desc.entry[i].stream) {
-			desc_entry = &desc.entry[i];
+	for (i = 0; i < desc->num_entries; i++) {
+		if (source_stream == desc->entry[i].stream) {
+			desc_entry = &desc->entry[i];
 			break;
 		}
 	}
@@ -534,8 +532,8 @@ int ipu7_isys_csi2_get_remote_desc(u32 source_stream,
 
 	*entry = *desc_entry;
 
-	for (i = 0; i < desc.num_entries; i++) {
-		if (desc_entry->bus.csi2.vc == desc.entry[i].bus.csi2.vc)
+	for (i = 0; i < desc->num_entries; i++) {
+		if (desc_entry->bus.csi2.vc == desc->entry[i].bus.csi2.vc)
 			(*nr_queues)++;
 	}
 
-- 
2.47.3

Re: [PATCH 00/17] Rework frame descriptors

[Sakari Ailus]

On Wed, May 13, 2026 at 01:43:41PM +0300, Sakari Ailus wrote:
> This smallish set makes frame descriptors dynamically allocated and
> implements a single-entry frame descriptor based on the device's format,
> using a new helper called v4l2_subdev_get_frame_desc(). All drivers that
> do not obtain their frame descriptor from upstream are converted. The
> helper also obtains a frame descriptor for the desired type (parallel or
> CSI-2) and checks there's at least one entry there. These checks are
> removed from drivers that currently perform them. (Some drivers also check
> there's exactly a single frame descriptor entry but I think in most cases
> this check could be loosened. That could be done after this set.)

Please ignore the driver patches for now. There are some issue remaining
which I'll fix for v2.

-- 
Sakari Ailus