Hi Frank,
Thank you for the patch.
On Fri, Aug 08, 2025 at 06:39:07PM -0400, Frank Li wrote:
> CSI2 data type is defined by MIPI Camera Serial Interface 2 Spec Ver4.1.
> See section 9.4.
>
> Add helper function media_bus_fmt_to_csi2_dt() to convert media bus fmt to
> MIPI defined data type and avoid below duplicated static array in each CSI2
> drivers.
>
> {
> .code = MEDIA_BUS_FMT_UYVY8_1X16,
> .data_type = MIPI_CSI2_DT_YUV422_8B,
> }
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/media/v4l2-core/v4l2-common.c | 44 +++++++++++++++++++++++++++++++++++
> include/media/mipi-csi2.h | 3 +++
> 2 files changed, 47 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 3a48b6a55c6e322696b910dd519def4f0b4a58fb..fcc01030beb347499da2a3c8539793d20f6f512c 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -701,6 +701,50 @@ int v4l2_link_freq_to_bitmap(struct device *dev, const u64 *fw_link_freqs,
> }
> EXPORT_SYMBOL_GPL(v4l2_link_freq_to_bitmap);
>
> +int media_bus_fmt_to_csi2_dt(int bus_fmt)
> +{
> + switch (bus_fmt) {
> + case MEDIA_BUS_FMT_UYVY8_1X16:
> + return MIPI_CSI2_DT_YUV422_8B;
> + case MEDIA_BUS_FMT_RGB565_1X16:
> + return MIPI_CSI2_DT_RGB565;
> + case MEDIA_BUS_FMT_BGR888_1X24:
> + return MIPI_CSI2_DT_RGB888;
> + case MEDIA_BUS_FMT_SBGGR8_1X8:
> + case MEDIA_BUS_FMT_SGBRG8_1X8:
> + case MEDIA_BUS_FMT_SGRBG8_1X8:
> + case MEDIA_BUS_FMT_SRGGB8_1X8:
> + case MEDIA_BUS_FMT_Y8_1X8:
> + return MIPI_CSI2_DT_RAW8;
> + case MEDIA_BUS_FMT_SBGGR10_1X10:
> + case MEDIA_BUS_FMT_SGBRG10_1X10:
> + case MEDIA_BUS_FMT_SGRBG10_1X10:
> + case MEDIA_BUS_FMT_SRGGB10_1X10:
> + case MEDIA_BUS_FMT_Y10_1X10:
> + return MIPI_CSI2_DT_RAW10;
> + case MEDIA_BUS_FMT_SBGGR12_1X12:
> + case MEDIA_BUS_FMT_SGBRG12_1X12:
> + case MEDIA_BUS_FMT_SGRBG12_1X12:
> + case MEDIA_BUS_FMT_SRGGB12_1X12:
> + case MEDIA_BUS_FMT_Y12_1X12:
> + return MIPI_CSI2_DT_RAW12;
> + 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_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;
> +
> + default:
> + return MIPI_CSI2_DT_NULL;
I would use 0 here. MIPI_CSI2_DT_NULL is equal to 0x10, and I'm
concerned about driver comparing the return value of the function to 0.
> + }
> +}
> +EXPORT_SYMBOL_GPL(media_bus_fmt_to_csi2_dt);
> +
We have something similar for pixel formats, with the v4l2_format_info
structure and the v4l2_format_info() lookup function. Adding information
about media bus formats to the V4L2 core seems to be a good idea.
Regarding the implementation, I think you should mimick what we do for
the pixel formats, and add the data to an info structure with a lookup
function.
> int media_bus_fmt_to_csi2_bpp(int bus_fmt)
> {
> switch (bus_fmt) {
> diff --git a/include/media/mipi-csi2.h b/include/media/mipi-csi2.h
> index c5b4e8e1ca93803568b1eee85f7f99c3a45a0b6e..35de536b9b65b49ad9e2914437d26d0e4240cf38 100644
> --- a/include/media/mipi-csi2.h
> +++ b/include/media/mipi-csi2.h
> @@ -8,6 +8,8 @@
> #ifndef _MEDIA_MIPI_CSI2_H
> #define _MEDIA_MIPI_CSI2_H
>
> +/* DT value ref to MIPI Camera Serial Interface 2 Spec Ver4.1 section 9.4 */
> +
> /* Short packet data types */
> #define MIPI_CSI2_DT_FS 0x00
> #define MIPI_CSI2_DT_FE 0x01
> @@ -44,6 +46,7 @@
> #define MIPI_CSI2_DT_RAW20 0x2f
> #define MIPI_CSI2_DT_USER_DEFINED(n) (0x30 + (n)) /* 0..7 */
>
> +int media_bus_fmt_to_csi2_dt(int buf_fmt);
> int media_bus_fmt_to_csi2_bpp(int bus_fmt);
>
> #endif /* _MEDIA_MIPI_CSI2_H */
>
>
--
Regards,
Laurent Pinchart