[PATCH] media: i2c: imx219: Implement .get_frame_desc()

[PATCH] media: i2c: imx219: Implement .get_frame_desc()

[Mattijs Korpershoek]

From: Vaishnav Achath <vaishnav.a@ti.com>

The next subdev in the media graph may want to enquire information such
as bus format, virtual channel, bus data type to route the stream from
this sensor correctly.

Add support for sharing this information using the .get_frame_desc()
callback.

Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
Signed-off-by: Jai Luthra <j-luthra@ti.com>
Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
This has been tested on top of linus/master based on commit
3d6d817622b0 ("Merge tag 'scsi-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi")

I used a AM69-SK with the Arducam FPD V3Link[1] 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 [2]

This is based on a patch [3] from TI's public vendor tree.

I've tried to polish the patch a bit with the following changes:
* Use existing imx219_get_format_bpp() instead of open coding it
* Use MIPI_CSI2_DT_RAW{,10} instead of magic numbers
* Don't memset(*fd) since already handled by the core
* Simplify frame_desc entries by removing fd->num_entries++
* Add new imx219_get_data_type_by_code() helper and use it
* Add error handling for v4l2_subdev_state_get_format()
* Don't hard-code pad number (is always 0)
* Remove 'ret' variable

[1] https://www.arducam.com/arducam-v3link-camera-kit-for-ti-development-boards.html
[2] 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
[3] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit?id=4268e58970c119e8dda8ad951f329d267eacc7a7&dt=2
---
 drivers/media/i2c/imx219.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 223d3753cc93..fc237a2dba60 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -23,11 +23,13 @@
 #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>
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-mediabus.h>
+#include <media/v4l2-subdev.h>
 
 /* Chip ID */
 #define IMX219_REG_CHIP_ID		CCI_REG16(0x0000)
@@ -429,6 +431,24 @@ static inline u32 imx219_get_rate_factor(struct v4l2_subdev_state *state)
 	return (bin_h & bin_v) == IMX219_BINNING_X2_ANALOG ? 2 : 1;
 }
 
+static u8 imx219_get_data_type_by_code(__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 MIPI_CSI2_DT_RAW8;
+
+	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 MIPI_CSI2_DT_RAW10;
+	}
+}
+
 /* -----------------------------------------------------------------------------
  * Controls
  */
@@ -539,6 +559,37 @@ static unsigned long imx219_get_pixel_rate(struct imx219 *imx219)
 	return (imx219->lanes == 2) ? IMX219_PIXEL_RATE : IMX219_PIXEL_RATE_4LANE;
 }
 
+static int imx219_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+				 struct v4l2_mbus_frame_desc *fd)
+{
+	struct v4l2_mbus_framefmt *format;
+	struct v4l2_subdev_state *state;
+	u32 bpp;
+
+	if (pad != 0)
+		return -EINVAL;
+
+	state = v4l2_subdev_lock_and_get_active_state(sd);
+	if (!state)
+		return -EINVAL;
+
+	format = v4l2_subdev_state_get_format(state, pad);
+	bpp = imx219_get_format_bpp(format);
+
+	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+	fd->num_entries = 1;
+	fd->entry[0].pixelcode = format->code;
+	fd->entry[0].stream = 0;
+	fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
+	fd->entry[0].length = (format->width * format->height * bpp) / 8;
+	fd->entry[0].bus.csi2.vc = 0;
+	fd->entry[0].bus.csi2.dt = imx219_get_data_type_by_code(format->code);
+
+	v4l2_subdev_unlock_state(state);
+
+	return 0;
+}
+
 /* Initialize control handlers */
 static int imx219_init_controls(struct imx219 *imx219)
 {
@@ -994,6 +1045,7 @@ static const struct v4l2_subdev_pad_ops imx219_pad_ops = {
 	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = imx219_set_pad_format,
 	.get_selection = imx219_get_selection,
+	.get_frame_desc = imx219_get_frame_desc,
 	.enum_frame_size = imx219_enum_frame_size,
 	.enable_streams = imx219_enable_streams,
 	.disable_streams = imx219_disable_streams,

---
base-commit: 3d6d817622b0a9721e3cc404df3469171582be13
change-id: 20260813-imx219-frame-desc-35e9cbb004cd

Best regards,
--  
Mattijs Korpershoek <mkorpershoek@kernel.org>

Re: [PATCH] media: i2c: imx219: Implement .get_frame_desc()

[Yemike Abhilash Chandra]

Hi Mattijs,
Thanks for the patch.

On 13/08/26 20:41, Mattijs Korpershoek wrote:
> From: Vaishnav Achath <vaishnav.a@ti.com>
> 
> The next subdev in the media graph may want to enquire information such
> as bus format, virtual channel, bus data type to route the stream from
> this sensor correctly.
> 
> Add support for sharing this information using the .get_frame_desc()
> callback.
> 
> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
> Signed-off-by: Jai Luthra <j-luthra@ti.com>
> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
> ---

FYI, similar variant of this is already posted by Tomi recently [1].

On that patch, quoting Sakari [2]:

"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."

I don't really know the status of that series. I will let Sakari to comment.

Thanks and Regards,
Yemike Abhilash Chandra


[1]: 
https://lore.kernel.org/all/20260611-imx219-frame-desc-v1-1-fe7e975bca6e@ideasonboard.com/
[2]: https://lore.kernel.org/all/aip-xwYlKT1d3N0S@kekkonen.localdomain/#t


> This has been tested on top of linus/master based on commit
> 3d6d817622b0 ("Merge tag 'scsi-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi")
> 
> I used a AM69-SK with the Arducam FPD V3Link[1] 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 [2]
> 
> This is based on a patch [3] from TI's public vendor tree.
> 
> I've tried to polish the patch a bit with the following changes:
> * Use existing imx219_get_format_bpp() instead of open coding it
> * Use MIPI_CSI2_DT_RAW{,10} instead of magic numbers
> * Don't memset(*fd) since already handled by the core
> * Simplify frame_desc entries by removing fd->num_entries++
> * Add new imx219_get_data_type_by_code() helper and use it
> * Add error handling for v4l2_subdev_state_get_format()
> * Don't hard-code pad number (is always 0)
> * Remove 'ret' variable
> 
> [1] https://www.arducam.com/arducam-v3link-camera-kit-for-ti-development-boards.html
> [2] 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
> [3] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit?id=4268e58970c119e8dda8ad951f329d267eacc7a7&dt=2
> ---
>   drivers/media/i2c/imx219.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index 223d3753cc93..fc237a2dba60 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -23,11 +23,13 @@
>   #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>
>   #include <media/v4l2-fwnode.h>
>   #include <media/v4l2-mediabus.h>
> +#include <media/v4l2-subdev.h>
>   
>   /* Chip ID */
>   #define IMX219_REG_CHIP_ID		CCI_REG16(0x0000)
> @@ -429,6 +431,24 @@ static inline u32 imx219_get_rate_factor(struct v4l2_subdev_state *state)
>   	return (bin_h & bin_v) == IMX219_BINNING_X2_ANALOG ? 2 : 1;
>   }
>   
> +static u8 imx219_get_data_type_by_code(__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 MIPI_CSI2_DT_RAW8;
> +
> +	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 MIPI_CSI2_DT_RAW10;
> +	}
> +}
> +
>   /* -----------------------------------------------------------------------------
>    * Controls
>    */
> @@ -539,6 +559,37 @@ static unsigned long imx219_get_pixel_rate(struct imx219 *imx219)
>   	return (imx219->lanes == 2) ? IMX219_PIXEL_RATE : IMX219_PIXEL_RATE_4LANE;
>   }
>   
> +static int imx219_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +				 struct v4l2_mbus_frame_desc *fd)
> +{
> +	struct v4l2_mbus_framefmt *format;
> +	struct v4l2_subdev_state *state;
> +	u32 bpp;
> +
> +	if (pad != 0)
> +		return -EINVAL;
> +
> +	state = v4l2_subdev_lock_and_get_active_state(sd);
> +	if (!state)
> +		return -EINVAL;
> +
> +	format = v4l2_subdev_state_get_format(state, pad);
> +	bpp = imx219_get_format_bpp(format);
> +
> +	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +	fd->num_entries = 1;
> +	fd->entry[0].pixelcode = format->code;
> +	fd->entry[0].stream = 0;
> +	fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
> +	fd->entry[0].length = (format->width * format->height * bpp) / 8;
> +	fd->entry[0].bus.csi2.vc = 0;
> +	fd->entry[0].bus.csi2.dt = imx219_get_data_type_by_code(format->code);
> +
> +	v4l2_subdev_unlock_state(state);
> +
> +	return 0;
> +}
> +
>   /* Initialize control handlers */
>   static int imx219_init_controls(struct imx219 *imx219)
>   {
> @@ -994,6 +1045,7 @@ static const struct v4l2_subdev_pad_ops imx219_pad_ops = {
>   	.get_fmt = v4l2_subdev_get_fmt,
>   	.set_fmt = imx219_set_pad_format,
>   	.get_selection = imx219_get_selection,
> +	.get_frame_desc = imx219_get_frame_desc,
>   	.enum_frame_size = imx219_enum_frame_size,
>   	.enable_streams = imx219_enable_streams,
>   	.disable_streams = imx219_disable_streams,
> 
> ---
> base-commit: 3d6d817622b0a9721e3cc404df3469171582be13
> change-id: 20260813-imx219-frame-desc-35e9cbb004cd
> 
> Best regards,
> --
> Mattijs Korpershoek <mkorpershoek@kernel.org>
> 

Re: [PATCH] media: i2c: imx219: Implement .get_frame_desc()

[Sakari Ailus]

Hi Yamike, Mattijs,

On Thu, Aug 13, 2026 at 09:12:03PM +0530, Yemike Abhilash Chandra wrote:
> Hi Mattijs,
> Thanks for the patch.
> 
> On 13/08/26 20:41, Mattijs Korpershoek wrote:
> > From: Vaishnav Achath <vaishnav.a@ti.com>
> > 
> > The next subdev in the media graph may want to enquire information such
> > as bus format, virtual channel, bus data type to route the stream from
> > this sensor correctly.
> > 
> > Add support for sharing this information using the .get_frame_desc()
> > callback.
> > 
> > Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
> > Signed-off-by: Jai Luthra <j-luthra@ti.com>
> > Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> > Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
> > ---
> 
> FYI, similar variant of this is already posted by Tomi recently [1].
> 
> On that patch, quoting Sakari [2]:
> 
> "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."
> 
> I don't really know the status of that series. I will let Sakari to comment.

Good question. I think it'd be nice to have it merged. Laurent's opinion
was the frame descriptors should be part of sub-device state. I don't
really disagree, but moving them there is again an overhaul of that
patchset. If the set isn't merged in the near future, we'll see a large
number of driver specific implementations of both different frame
descriptor functions in sensor drivers as well as receiver drivers
preparing for sensor drivers not supporting get_frame_desc().

The current implementation has issues especially in cases where frame
descriptor has dependencies to upstream frame descriptors. There aren't
many drivers that do that though, but in these cases it has potential for
re-requesting the same frame descriptors rather many times. I wonder if
that's something that could be addressed later though.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH] media: i2c: imx219: Implement .get_frame_desc()

[Mattijs Korpershoek]

Hi Yemike,

On Thu, Aug 13, 2026 at 21:12, Yemike Abhilash Chandra <y-abhilashchandra@ti.com> wrote:

> Hi Mattijs,
> Thanks for the patch.
>
> On 13/08/26 20:41, Mattijs Korpershoek wrote:
>> From: Vaishnav Achath <vaishnav.a@ti.com>
>> 
>> The next subdev in the media graph may want to enquire information such
>> as bus format, virtual channel, bus data type to route the stream from
>> this sensor correctly.
>> 
>> Add support for sharing this information using the .get_frame_desc()
>> callback.
>> 
>> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
>> Signed-off-by: Jai Luthra <j-luthra@ti.com>
>> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>> ---
>
> FYI, similar variant of this is already posted by Tomi recently [1].

Oh, I completely missed this. I did search on lore, but not good enough,
apparently.
Thanks for the pointer.

I'm happy to drop my patch then.

>
> On that patch, quoting Sakari [2]:
>
> "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."
>
> I don't really know the status of that series. I will let Sakari to comment.

Thanks, I'll have a look!

>
> Thanks and Regards,
> Yemike Abhilash Chandra
>
>
> [1]: 
> https://lore.kernel.org/all/20260611-imx219-frame-desc-v1-1-fe7e975bca6e@ideasonboard.com/
> [2]: https://lore.kernel.org/all/aip-xwYlKT1d3N0S@kekkonen.localdomain/#t
>
>