Re: [PATCH 1/6] media: Define MIPI CSI-2 data types in a shared header file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent

Quoting Laurent Pinchart (2022-01-23 16:08:52)
> There are many CSI-2-related drivers in the media subsystem that come
> with their own macros to handle the CSI-2 data types (or just hardcode
> the numerical values). Provide a shared header with definitions for
> those data types that driver can use.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
>  include/media/mipi-csi2.h | 45 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 include/media/mipi-csi2.h
> 
> diff --git a/include/media/mipi-csi2.h b/include/media/mipi-csi2.h
> new file mode 100644
> index 000000000000..392794e5badd
> --- /dev/null
> +++ b/include/media/mipi-csi2.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * MIPI CSI-2 Data Types
> + *
> + * Copyright (C) 2022 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> + */
> +
> +#ifndef _MEDIA_MIPI_CSI2_H
> +#define _MEDIA_MIPI_CSI2_H
> +
> +/* Short packet data types */
> +#define MIPI_CSI2_DT_FS                        0x00
> +#define MIPI_CSI2_DT_FE                        0x01
> +#define MIPI_CSI2_DT_LS                        0x02
> +#define MIPI_CSI2_DT_LE                        0x03
> +#define MIPI_CSI2_DT_GENERIC_SHORT(n)  (0x08 + (n))    /* 0..7 */
> +
> +/* Long packet data types */
> +#define MIPI_CSI2_DT_NULL              0x10
> +#define MIPI_CSI2_DT_BLANKING          0x11
> +#define MIPI_CSI2_DT_EMBEDDED_8B       0x12
> +#define MIPI_CSI2_DT_YUV420_8B         0x18
> +#define MIPI_CSI2_DT_YUV420_10B                0x19
> +#define MIPI_CSI2_DT_YUV420_8B_LEGACY  0x1a
> +#define MIPI_CSI2_DT_YUV420_8B_CS      0x1c
> +#define MIPI_CSI2_DT_YUV420_10B_CS     0x1d
> +#define MIPI_CSI2_DT_YUV422_8B         0x1e
> +#define MIPI_CSI2_DT_YUV422_10B                0x1f
> +#define MIPI_CSI2_DT_RGB444            0x20
> +#define MIPI_CSI2_DT_RGB555            0x21
> +#define MIPI_CSI2_DT_RGB565            0x22
> +#define MIPI_CSI2_DT_RGB666            0x23
> +#define MIPI_CSI2_DT_RGB888            0x24
> +#define MIPI_CSI2_DT_RAW24             0x27
> +#define MIPI_CSI2_DT_RAW6              0x28
> +#define MIPI_CSI2_DT_RAW7              0x29
> +#define MIPI_CSI2_DT_RAW8              0x2a
> +#define MIPI_CSI2_DT_RAW10             0x2b
> +#define MIPI_CSI2_DT_RAW12             0x2c
> +#define MIPI_CSI2_DT_RAW14             0x2d
> +#define MIPI_CSI2_DT_RAW16             0x2e
> +#define MIPI_CSI2_DT_RAW20             0x2f
> +#define MIPI_CSI2_DT_USER_DEFINED(n)   (0x30 + (n))    /* 0..7 */

I don't have an easy way to validate those values right now so as with
Niklas I'll leave those to your judgement, and Pratyush's review.

Also along side Pratyush's comment, I concur that the mapping tables too
could be common, but I suspect that's an even bigger topic as maybe that
falls into the trap of also being common to DRM formats...

And finally, are these defines in a location that can be accessible from
device tree? Or would it have to be further duplicated there still?

For instance, the bindings for the Xilinx CSI2 RX explicitly list DT
values to specify as the xlnx,csi-pxl-format which I think should also
come from this common header definition.

For the patches here so far, I can't see anything stark that is wrong
so for the series:


Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>

as further extending this to the device tree bindings can be done on
top.


> +
> +#endif /* _MEDIA_MIPI_CSI2_H */
> -- 
> Regards,
> 
> Laurent Pinchart
>




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux