Re: [PATCH v2 3/7] media: uapi: add MEDIA_BUS_FMT_BGR666_1X24_CPADHI

[Laurent Pinchart]

Hi Jörg,

On Tue, Nov 29, 2022 at 03:52:45PM +0100, Jörg Quinten wrote:
> Hi Laurent,
> 
> looks like linux/Documentation/userspace-api/media/v4l/subdev-formats.rst
> doesn't correlate at all to the arrangement and numbering in
> linux/include/uapi/linux/media-bus-format.h .

Looking at the RGB group in include/uapi/linux/media-bus-format.h in the
mainline kernel, if you exclude the LVDS formats that are documented
separately, and the MEDIA_BUS_FMT_RGB888_3X8_DELTA format that appears
to be undocumented, the formats are ordered in the same way as in
subdev-formats.rst (see
https://linuxtv.org/downloads/v4l-dvb-apis/userspace-api/v4l/subdev-formats.html#v4l2-mbus-pixelcode for an HTML version).

> Which sources do you want me to check?
> 
> Looking at https://github.com/raspberrypi/linux/tree/rpi-6.1.y btw.
> 
> Rgds
> Joerg
> 
> 
> 
> 
> Am Di., 29. Nov. 2022 um 13:38 Uhr schrieb Laurent Pinchart <
> laurent.pinchart@xxxxxxxxxxxxxxxx>:
> 
> > Hi Maxime and Joerg,
> >
> > Thank you for the patch.
> >
> > On Thu, Oct 20, 2022 at 10:30:47AM +0200, Maxime Ripard wrote:
> > > From: Joerg Quinten <aBUGSworstnightmare@xxxxxxxxx>
> > >
> > > Add the BGR666 format MEDIA_BUS_FMT_BGR666_1X24_CPADHI supported by the
> > > RaspberryPi.
> > >
> > > Signed-off-by: Joerg Quinten <aBUGSworstnightmare@xxxxxxxxx>
> > > Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
> > > ---
> > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37
> > ++++++++++++++++++++++
> > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > index 68f8d7d37984..604a30e2f890 100644
> > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > @@ -1023,6 +1023,43 @@ The following tables list existing packed RGB
> > formats.
> > >        - g\ :sub:`2`
> > >        - g\ :sub:`1`
> > >        - g\ :sub:`0`
> > > +    * .. _MEDIA-BUS-FMT-BGR666-1X24_CPADHI:
> >
> > I would move this just below MEDIA_BUS_FMT_RGB565_1X24_CPADHI. Actually,
> > could you check 1/7 and 2/7 to make sure the formats are sorted in the
> > documentation in the same order as in the header ?
> >
> > > +
> > > +      - MEDIA_BUS_FMT_BGR666_1X24_CPADHI
> > > +      - 0x1024
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      - 0
> > > +      - 0
> > > +      - b\ :sub:`5`
> > > +      - b\ :sub:`4`
> > > +      - b\ :sub:`3`
> > > +      - b\ :sub:`2`
> > > +      - b\ :sub:`1`
> > > +      - b\ :sub:`0`
> > > +      - 0
> > > +      - 0
> > > +      - g\ :sub:`5`
> > > +      - g\ :sub:`4`
> > > +      - g\ :sub:`3`
> > > +      - g\ :sub:`2`
> > > +      - g\ :sub:`1`
> > > +      - g\ :sub:`0`
> > > +      - 0
> > > +      - 0
> > > +      - r\ :sub:`5`
> > > +      - r\ :sub:`4`
> > > +      - r\ :sub:`3`
> > > +      - r\ :sub:`2`
> > > +      - r\ :sub:`1`
> > > +      - r\ :sub:`0`
> > >      * .. _MEDIA-BUS-FMT-RGB565-1X24_CPADHI:
> > >
> > >        - MEDIA_BUS_FMT_RGB565_1X24_CPADHI
> > > diff --git a/include/uapi/linux/media-bus-format.h
> > b/include/uapi/linux/media-bus-format.h
> > > index 2ee0b38c0a71..d4228d038b54 100644
> > > --- a/include/uapi/linux/media-bus-format.h
> > > +++ b/include/uapi/linux/media-bus-format.h
> > > @@ -34,7 +34,7 @@
> > >
> > >  #define MEDIA_BUS_FMT_FIXED                  0x0001
> > >
> > > -/* RGB - next is     0x1024 */
> > > +/* RGB - next is     0x1025 */
> > >  #define MEDIA_BUS_FMT_RGB444_1X12            0x1016
> > >  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE    0x1001
> > >  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE    0x1002
> > > @@ -49,6 +49,7 @@
> > >  #define MEDIA_BUS_FMT_BGR666_1X18            0x1023
> > >  #define MEDIA_BUS_FMT_RGB666_1X18            0x1009
> > >  #define MEDIA_BUS_FMT_RBG888_1X24            0x100e
> > > +#define MEDIA_BUS_FMT_BGR666_1X24_CPADHI     0x1024
> > >  #define MEDIA_BUS_FMT_RGB666_1X24_CPADHI     0x1015
> > >  #define MEDIA_BUS_FMT_RGB666_1X7X3_SPWG              0x1010
> > >  #define MEDIA_BUS_FMT_BGR888_1X24            0x1013
> > >

-- 
Regards,

Laurent Pinchart