[PATCH] media: imx219: Report streams using frame descriptors

[PATCH] media: imx219: Report streams using frame descriptors

[Tomi Valkeinen]

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Implement the .get_frame_desc() subdev operation to report information
about streams to the connected CSI-2 receiver. This is required to let
the CSI-2 receiver driver know about virtual channels and data types for
each stream.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
[tomi.valkeinen: picked from "Generic line based metadata support, internal pads" series]
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
This patch that adds .get_frame_desc() support to imx219 driver has been
circulating for a few years, and is currently posted in "[PATCH v12
00/86] Generic line based metadata support, internal pads" series.

However, as some bridge drivers require modern drivers that support
.get_frame_desc, specifically ds90ub960.c, let's pick the patch and
queue it separately from the huge metadata series.
---
 drivers/media/i2c/imx219.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 223d3753cc93..7829ddc115a0 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -23,6 +23,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 
+#include <media/mipi-csi2.h>
 #include <media/v4l2-cci.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
@@ -661,6 +662,24 @@ static void imx219_free_controls(struct imx219 *imx219)
  * Subdev operations
  */
 
+static unsigned int imx219_format_bpp(u32 code)
+{
+	switch (code) {
+	case MEDIA_BUS_FMT_SRGGB8_1X8:
+	case MEDIA_BUS_FMT_SGRBG8_1X8:
+	case MEDIA_BUS_FMT_SGBRG8_1X8:
+	case MEDIA_BUS_FMT_SBGGR8_1X8:
+		return 8;
+
+	case MEDIA_BUS_FMT_SRGGB10_1X10:
+	case MEDIA_BUS_FMT_SGRBG10_1X10:
+	case MEDIA_BUS_FMT_SGBRG10_1X10:
+	case MEDIA_BUS_FMT_SBGGR10_1X10:
+	default:
+		return 10;
+	}
+}
+
 static int imx219_set_framefmt(struct imx219 *imx219,
 			       struct v4l2_subdev_state *state)
 {
@@ -969,6 +988,30 @@ static int imx219_get_selection(struct v4l2_subdev *sd,
 	return -EINVAL;
 }
 
+static int imx219_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+				 struct v4l2_mbus_frame_desc *fd)
+{
+	const struct v4l2_mbus_framefmt *fmt;
+	struct v4l2_subdev_state *state;
+	u32 code;
+
+	state = v4l2_subdev_lock_and_get_active_state(sd);
+	fmt = v4l2_subdev_state_get_format(state, 0);
+	code = fmt->code;
+	v4l2_subdev_unlock_state(state);
+
+	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+	fd->num_entries = 1;
+
+	fd->entry[0].pixelcode = code;
+	fd->entry[0].stream = 0;
+	fd->entry[0].bus.csi2.vc = 0;
+	fd->entry[0].bus.csi2.dt = imx219_format_bpp(code) == 8 ?
+		MIPI_CSI2_DT_RAW8 : MIPI_CSI2_DT_RAW10;
+
+	return 0;
+}
+
 static int imx219_init_state(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_state *state)
 {
@@ -995,6 +1038,7 @@ static const struct v4l2_subdev_pad_ops imx219_pad_ops = {
 	.set_fmt = imx219_set_pad_format,
 	.get_selection = imx219_get_selection,
 	.enum_frame_size = imx219_enum_frame_size,
+	.get_frame_desc = imx219_get_frame_desc,
 	.enable_streams = imx219_enable_streams,
 	.disable_streams = imx219_disable_streams,
 };

---
base-commit: 06cb687a5132fcffe624c0070576ab852ac6b568
change-id: 20260611-imx219-frame-desc-9cc223b1fbd5

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

Re: [PATCH] media: imx219: Report streams using frame descriptors

[Sakari Ailus]

Moi,

On Thu, Jun 11, 2026 at 12:13:02PM +0300, Tomi Valkeinen wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Implement the .get_frame_desc() subdev operation to report information
> about streams to the connected CSI-2 receiver. This is required to let
> the CSI-2 receiver driver know about virtual channels and data types for
> each stream.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> [tomi.valkeinen: picked from "Generic line based metadata support, internal pads" series]
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> This patch that adds .get_frame_desc() support to imx219 driver has been
> circulating for a few years, and is currently posted in "[PATCH v12
> 00/86] Generic line based metadata support, internal pads" series.
> 
> However, as some bridge drivers require modern drivers that support
> .get_frame_desc, specifically ds90ub960.c, let's pick the patch and
> queue it separately from the huge metadata series.

I've been recently working on
<URL:https://lore.kernel.org/linux-media/20260518164318.3367888-1-sakari.ailus@linux.intel.com/>.
In other words, drivers that have a single stream don't need this. We could
probably extend that further by making use of the routing information but I
think that should be left for later.

-- 
Terveisin,

Sakari Ailus

Re: [PATCH] media: imx219: Report streams using frame descriptors

[Tomi Valkeinen]

Hi,

On 11/06/2026 12:24, Sakari Ailus wrote:
> Moi,
> 
> On Thu, Jun 11, 2026 at 12:13:02PM +0300, Tomi Valkeinen wrote:
>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> Implement the .get_frame_desc() subdev operation to report information
>> about streams to the connected CSI-2 receiver. This is required to let
>> the CSI-2 receiver driver know about virtual channels and data types for
>> each stream.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>> [tomi.valkeinen: picked from "Generic line based metadata support, internal pads" series]
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> This patch that adds .get_frame_desc() support to imx219 driver has been
>> circulating for a few years, and is currently posted in "[PATCH v12
>> 00/86] Generic line based metadata support, internal pads" series.
>>
>> However, as some bridge drivers require modern drivers that support
>> .get_frame_desc, specifically ds90ub960.c, let's pick the patch and
>> queue it separately from the huge metadata series.
> 
> I've been recently working on
> <URL:https://lore.kernel.org/linux-media/20260518164318.3367888-1-sakari.ailus@linux.intel.com/>.
> In other words, drivers that have a single stream don't need this. We could

Thanks, I had missed that. I like the idea of a helper that does the 
fallback mechanism. But I wonder about the need for dynamic alloc, which 
complicates the series. In any case, we can drop this series and 
continue the discussion on your series.

  Tomi

Re: [PATCH] media: imx219: Report streams using frame descriptors

[Laurent Pinchart]

On Thu, Jun 11, 2026 at 04:06:38PM +0300, Tomi Valkeinen wrote:
> On 11/06/2026 12:24, Sakari Ailus wrote:
> > On Thu, Jun 11, 2026 at 12:13:02PM +0300, Tomi Valkeinen wrote:
> >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>
> >> Implement the .get_frame_desc() subdev operation to report information
> >> about streams to the connected CSI-2 receiver. This is required to let
> >> the CSI-2 receiver driver know about virtual channels and data types for
> >> each stream.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >> [tomi.valkeinen: picked from "Generic line based metadata support, internal pads" series]
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >> ---
> >> This patch that adds .get_frame_desc() support to imx219 driver has been
> >> circulating for a few years, and is currently posted in "[PATCH v12
> >> 00/86] Generic line based metadata support, internal pads" series.
> >>
> >> However, as some bridge drivers require modern drivers that support
> >> .get_frame_desc, specifically ds90ub960.c, let's pick the patch and
> >> queue it separately from the huge metadata series.
> > 
> > I've been recently working on
> > <URL:https://lore.kernel.org/linux-media/20260518164318.3367888-1-sakari.ailus@linux.intel.com/>.
> > In other words, drivers that have a single stream don't need this. We could
> 
> Thanks, I had missed that. I like the idea of a helper that does the 
> fallback mechanism. But I wonder about the need for dynamic alloc, which 
> complicates the series. In any case, we can drop this series and 
> continue the discussion on your series.

Maybe we can merge the fallback implementation separately from the
dynamic allocation if the latter requires more work ? I'm also not a fan
of the dynamic allocation, at least in the way it's implemented in the
proposed series. I'm wondering if we could build the frame descriptors
at stream enable time and store it in state structures instead.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: imx219: Report streams using frame descriptors

[Sakari Ailus]

Hi Laurent,

On Thu, Jun 11, 2026 at 04:10:58PM +0300, Laurent Pinchart wrote:
> On Thu, Jun 11, 2026 at 04:06:38PM +0300, Tomi Valkeinen wrote:
> > On 11/06/2026 12:24, Sakari Ailus wrote:
> > > On Thu, Jun 11, 2026 at 12:13:02PM +0300, Tomi Valkeinen wrote:
> > >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >>
> > >> Implement the .get_frame_desc() subdev operation to report information
> > >> about streams to the connected CSI-2 receiver. This is required to let
> > >> the CSI-2 receiver driver know about virtual channels and data types for
> > >> each stream.
> > >>
> > >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > >> [tomi.valkeinen: picked from "Generic line based metadata support, internal pads" series]
> > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > >> ---
> > >> This patch that adds .get_frame_desc() support to imx219 driver has been
> > >> circulating for a few years, and is currently posted in "[PATCH v12
> > >> 00/86] Generic line based metadata support, internal pads" series.
> > >>
> > >> However, as some bridge drivers require modern drivers that support
> > >> .get_frame_desc, specifically ds90ub960.c, let's pick the patch and
> > >> queue it separately from the huge metadata series.
> > > 
> > > I've been recently working on
> > > <URL:https://lore.kernel.org/linux-media/20260518164318.3367888-1-sakari.ailus@linux.intel.com/>.
> > > In other words, drivers that have a single stream don't need this. We could
> > 
> > Thanks, I had missed that. I like the idea of a helper that does the 
> > fallback mechanism. But I wonder about the need for dynamic alloc, which 
> > complicates the series. In any case, we can drop this series and 
> > continue the discussion on your series.
> 
> Maybe we can merge the fallback implementation separately from the
> dynamic allocation if the latter requires more work ? I'm also not a fan
> of the dynamic allocation, at least in the way it's implemented in the
> proposed series. I'm wondering if we could build the frame descriptors
> at stream enable time and store it in state structures instead.

I've changed the allocation to take place in the get_frame_desc() callback,
I'll post the next version soonish.

I think in the long run indeed this should be stored in the state. Without
this series we'll have all receiver drivers implement a fallback or
implement get_frame_desc() callback in all sub-device drivers which is
something I'd like to avoid.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH] media: imx219: Report streams using frame descriptors

[Mattijs Korpershoek]

Hi Tomi,

Thank you for the patch.

On Thu, Jun 11, 2026 at 12:13, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Implement the .get_frame_desc() subdev operation to report information
> about streams to the connected CSI-2 receiver. This is required to let
> the CSI-2 receiver driver know about virtual channels and data types for
> each stream.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> [tomi.valkeinen: picked from "Generic line based metadata support, internal pads" series]
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> This patch that adds .get_frame_desc() support to imx219 driver has been
> circulating for a few years, and is currently posted in "[PATCH v12
> 00/86] Generic line based metadata support, internal pads" series.
>
> However, as some bridge drivers require modern drivers that support
> .get_frame_desc, specifically ds90ub960.c, let's pick the patch and
> queue it separately from the huge metadata series.
> ---
>  drivers/media/i2c/imx219.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)

Note: I understand that this is not the preferred solution because there
is [1]. I still found value on testing this so:

Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

Details:
This is the only patch I needed on top of linux/master to get camera
working on a TI AM69-SK.

I used a AM69-SK with the Arducam FPD V3Link[2] using
the following device tree overlays:
  ti/k3-am68-sk-v3link-fusion.dtbo ti/k3-v3link-imx219-0-0.dtbo

See TI's documentation about this [3]

[1] https://lore.kernel.org/linux-media/20260518164318.3367888-1-sakari.ailus@linux.intel.com/
[2] https://www.arducam.com/arducam-v3link-camera-kit-for-ti-development-boards.html
[3]
https://software-dl.ti.com/jacinto7/esd/processor-sdk-linux-am69/11_00_10_01/exports/docs/linux/Foundational_Components/Kernel/Kernel_Drivers/Camera/CSI2RX.html

Some review comment below.

>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index 223d3753cc93..7829ddc115a0 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -23,6 +23,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
>  
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-cci.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
> @@ -661,6 +662,24 @@ static void imx219_free_controls(struct imx219 *imx219)
>   * Subdev operations
>   */
>  
> +static unsigned int imx219_format_bpp(u32 code)
> +{
> +	switch (code) {
> +	case MEDIA_BUS_FMT_SRGGB8_1X8:
> +	case MEDIA_BUS_FMT_SGRBG8_1X8:
> +	case MEDIA_BUS_FMT_SGBRG8_1X8:
> +	case MEDIA_BUS_FMT_SBGGR8_1X8:
> +		return 8;
> +
> +	case MEDIA_BUS_FMT_SRGGB10_1X10:
> +	case MEDIA_BUS_FMT_SGRBG10_1X10:
> +	case MEDIA_BUS_FMT_SGBRG10_1X10:
> +	case MEDIA_BUS_FMT_SBGGR10_1X10:
> +	default:
> +		return 10;
> +	}
> +}
> +

Do we need this function when we already have imx219_get_format_bpp()
which does the same thing?

>  static int imx219_set_framefmt(struct imx219 *imx219,
>  			       struct v4l2_subdev_state *state)
>  {
> @@ -969,6 +988,30 @@ static int imx219_get_selection(struct v4l2_subdev *sd,
>  	return -EINVAL;
>  }
>  
> +static int imx219_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +				 struct v4l2_mbus_frame_desc *fd)
> +{
> +	const struct v4l2_mbus_framefmt *fmt;
> +	struct v4l2_subdev_state *state;
> +	u32 code;
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);

Do we need error handling here? v4l2_subdev_lock_and_get_active_state()
can return NULL.

> +	fmt = v4l2_subdev_state_get_format(state, 0);
> +	code = fmt->code;
> +	v4l2_subdev_unlock_state(state);
> +
> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +	fd->num_entries = 1;
> +
> +	fd->entry[0].pixelcode = code;
> +	fd->entry[0].stream = 0;
> +	fd->entry[0].bus.csi2.vc = 0;
> +	fd->entry[0].bus.csi2.dt = imx219_format_bpp(code) == 8 ?
> +		MIPI_CSI2_DT_RAW8 : MIPI_CSI2_DT_RAW10;
> +
> +	return 0;
> +}
> +
>  static int imx219_init_state(struct v4l2_subdev *sd,
>  			     struct v4l2_subdev_state *state)
>  {
> @@ -995,6 +1038,7 @@ static const struct v4l2_subdev_pad_ops imx219_pad_ops = {
>  	.set_fmt = imx219_set_pad_format,
>  	.get_selection = imx219_get_selection,
>  	.enum_frame_size = imx219_enum_frame_size,
> +	.get_frame_desc = imx219_get_frame_desc,
>  	.enable_streams = imx219_enable_streams,
>  	.disable_streams = imx219_disable_streams,
>  };
>
> ---
> base-commit: 06cb687a5132fcffe624c0070576ab852ac6b568
> change-id: 20260611-imx219-frame-desc-9cc223b1fbd5
>
> Best regards,
> --  
> Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>