| Message ID | 20240209164825.166800-1-jacopo.mondi@ideasonboard.com |
|---|---|
| Headers | show |
| Series | media: raspberrypi: Add support for PiSP Back End | expand |
Le vendredi 09 février 2024 à 19:28 +0000, Dave Stevenson a écrit : > Hi Nicolas > > On Fri, 9 Feb 2024 at 17:53, Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > > > Le vendredi 09 février 2024 à 17:48 +0100, Jacopo Mondi a écrit : > > > From: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > > > This is the format used by monochrome 12bit image sensors. > > > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > --- > > > .../userspace-api/media/v4l/pixfmt-y12p.rst | 38 +++++++++++++++++++ > > > .../userspace-api/media/v4l/yuv-formats.rst | 1 + > > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > > > include/uapi/linux/videodev2.h | 1 + > > > 4 files changed, 41 insertions(+) > > > create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-y12p.rst > > > > > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-y12p.rst b/Documentation/userspace-api/media/v4l/pixfmt-y12p.rst > > > new file mode 100644 > > > index 000000000000..b2eb4a72724d > > > --- /dev/null > > > +++ b/Documentation/userspace-api/media/v4l/pixfmt-y12p.rst > > > @@ -0,0 +1,38 @@ > > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later > > > + > > > +.. _V4L2-PIX-FMT-Y12P: > > > + > > > +****************************** > > > +V4L2_PIX_FMT_Y12P ('Y12P') > > > +****************************** > > > + > > > +Grey-scale image as a MIPI RAW12 packed array > > > + > > > + > > > +Description > > > +=========== > > > + > > > +This is a packed grey-scale image format with a depth of 12 bits per > > > +pixel. Two consecutive pixels are packed into 3 bytes. The first 2 bytes > > > +contain the 8 high order bits of the pixels, and the 3rd byte contains the 4 > > > +least significants bits of each pixel, in the same order. > > > > This is an interesting arrangement, which make me feel that Y12P is perhaps bit > > too generic ? Perhaps something like: > > > > V4L2_PIX_FMT_Y12_MIPI > > > > That being said, I don't mind if you reserve the nice and short name for the > > first occurrence of such format in 20 years (it might equally be the last). Will > > they do the same for Y10, by storing 4 bytes of higher 8bit of 4 pixels, and > > packing the remaining lower 2 bits packed in the fif bytes ? Cause for this one, > > we'd have confusion, since CODEC usually just place all the bits in order over 5 > > bytes here (which have its own issues of course). > > We already have V4L2_PIX_FMT_Y10P defined for the MIPI packing for > 10bit luma-only. > https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/pixfmt-yuv-luma.html > > This is only extending the definitions for the other bit depths using > the same pattern. > > Adding the P is also the same pattern as for the Bayer formats - > V4L2_PIX_FMT_SRGGB10 for the 10bit unpacked to 16bit, and > V4L2_PIX_FMT_SRGGB10P for the MIPI packed variant. > > I'm not inventing anything new here :) A bit sad, but what can we do, I keep missing patches ;-P Nicolas > > Dave > > > > + > > > +**Byte Order.** > > > +Each cell is one byte. > > > + > > > +.. tabularcolumns:: |p{2.2cm}|p{1.2cm}|p{1.2cm}|p{3.1cm}| > > > + > > > + > > > +.. flat-table:: > > > + :header-rows: 0 > > > + :stub-columns: 0 > > > + :widths: 2 1 1 1 > > > + > > > + > > > + - - start + 0: > > > + - Y'\ :sub:`00high` > > > + - Y'\ :sub:`01high` > > > + - Y'\ :sub:`01low`\ (bits 7--4) > > > + > > > + Y'\ :sub:`00low`\ (bits 3--0) > > > + > > > diff --git a/Documentation/userspace-api/media/v4l/yuv-formats.rst b/Documentation/userspace-api/media/v4l/yuv-formats.rst > > > index 24b34cdfa6fe..7c9ccfdd94cd 100644 > > > --- a/Documentation/userspace-api/media/v4l/yuv-formats.rst > > > +++ b/Documentation/userspace-api/media/v4l/yuv-formats.rst > > > @@ -267,6 +267,7 @@ image. > > > pixfmt-packed-yuv > > > pixfmt-yuv-planar > > > pixfmt-yuv-luma > > > + pixfmt-y12p > > > pixfmt-y8i > > > pixfmt-y12i > > > pixfmt-uv8 > > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > > > index 33076af4dfdb..483498c55899 100644 > > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > > > @@ -1311,6 +1311,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > > > case V4L2_PIX_FMT_Y16_BE: descr = "16-bit Greyscale BE"; break; > > > case V4L2_PIX_FMT_Y10BPACK: descr = "10-bit Greyscale (Packed)"; break; > > > case V4L2_PIX_FMT_Y10P: descr = "10-bit Greyscale (MIPI Packed)"; break; > > > + case V4L2_PIX_FMT_Y12P: descr = "12-bit Greyscale (MIPI Packed)"; break; > > > case V4L2_PIX_FMT_IPU3_Y10: descr = "10-bit greyscale (IPU3 Packed)"; break; > > > case V4L2_PIX_FMT_Y8I: descr = "Interleaved 8-bit Greyscale"; break; > > > case V4L2_PIX_FMT_Y12I: descr = "Interleaved 12-bit Greyscale"; break; > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > > index a8015e5e7fa4..11ebf9b22ccb 100644 > > > --- a/include/uapi/linux/videodev2.h > > > +++ b/include/uapi/linux/videodev2.h > > > @@ -598,6 +598,7 @@ struct v4l2_pix_format { > > > /* Grey bit-packed formats */ > > > #define V4L2_PIX_FMT_Y10BPACK v4l2_fourcc('Y', '1', '0', 'B') /* 10 Greyscale bit-packed */ > > > #define V4L2_PIX_FMT_Y10P v4l2_fourcc('Y', '1', '0', 'P') /* 10 Greyscale, MIPI RAW10 packed */ > > > +#define V4L2_PIX_FMT_Y12P v4l2_fourcc('Y', '1', '2', 'P') /* 12 Greyscale, MIPI RAW12 packed */ > > > #define V4L2_PIX_FMT_IPU3_Y10 v4l2_fourcc('i', 'p', '3', 'y') /* IPU3 packed 10-bit greyscale */ > > > > > > /* Palette formats */ > >
Hi, On 12/02/2024 11:05, Krzysztof Kozlowski wrote: > On 12/02/2024 09:50, Jacopo Mondi wrote: > >>>> +properties: >>>> + compatible: >>>> + const: raspberrypi,pispbe >>> >>> Nothing more specific? No model name, no version? It's quite generic >>> compatible which in general should not be allowed. I would assume that >>> at least version of Pi could denote some sort of a model... unless >>> version is detectable? >>> >> >> The driver matches on a version register and that should be enough to >> handle quirks which are specific to an IP revision in the driver >> itself. >> >> Considering how minimal the integration with the SoC is (one clock, one >> interrupt and one optional iommu reference) even if we'll get future >> revisions of the SoC I don't think there will be any need to match on >> a dedicated compatible for bindings-validation purposes. >> >> However I understand that to be future-proof it's good practice to >> allow a more flexible scheme, so we can have a generic fallback and a >> revision-specific entry. >> >> Would >> >> compatible: >> items: >> - enum: >> - raspberrypi,pipspbe-bcm2712 > > bcm2712 is manufactured by Broadcom, not Raspberry Pi, so it should be > rather Pi model? Indeed, this is something I don't get. If the BE is in the bcm2712, is it not a broadcom IP? Why is raspberrypi in the compatible name at all? Naush, Dave? >> - const: raspberrypi,pispbe >> >> do in this case ? >> >> Also, let's see what RPi think as they are certainly more informed >> than me on what a good revision-specific match could be > > I am fine with auto-detection, though. > > ... > >>>> + >>>> +examples: >>>> + - | >>>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>>> + >>>> + rpi1 { >>> >>> soc { >>> >> >> Are you sure ? This will only ever live in the 'rp1' node. > > What is "rp1" node? Does not look like a generic name. I don't think this is right. RP1 is a separate chip, an IO controller, on raspberrypi 5. BE is not in the RP1. Tomi
Hi Tomi, On Tue, 13 Feb 2024 at 07:28, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > Hi, > > On 12/02/2024 11:05, Krzysztof Kozlowski wrote: > > On 12/02/2024 09:50, Jacopo Mondi wrote: > > > >>>> +properties: > >>>> + compatible: > >>>> + const: raspberrypi,pispbe > >>> > >>> Nothing more specific? No model name, no version? It's quite generic > >>> compatible which in general should not be allowed. I would assume that > >>> at least version of Pi could denote some sort of a model... unless > >>> version is detectable? > >>> > >> > >> The driver matches on a version register and that should be enough to > >> handle quirks which are specific to an IP revision in the driver > >> itself. > >> > >> Considering how minimal the integration with the SoC is (one clock, one > >> interrupt and one optional iommu reference) even if we'll get future > >> revisions of the SoC I don't think there will be any need to match on > >> a dedicated compatible for bindings-validation purposes. > >> > >> However I understand that to be future-proof it's good practice to > >> allow a more flexible scheme, so we can have a generic fallback and a > >> revision-specific entry. > >> > >> Would > >> > >> compatible: > >> items: > >> - enum: > >> - raspberrypi,pipspbe-bcm2712 > > > > bcm2712 is manufactured by Broadcom, not Raspberry Pi, so it should be > > rather Pi model? > > Indeed, this is something I don't get. If the BE is in the bcm2712, is > it not a broadcom IP? Why is raspberrypi in the compatible name at all? > > Naush, Dave? The Backend (and Frontend) IP are both owned solely by Raspberry Pi, and the BE is instantiated on the BCM2712. So I think "raspberry" in the compatible string is correct here. > > >> - const: raspberrypi,pispbe > >> > >> do in this case ? > >> > >> Also, let's see what RPi think as they are certainly more informed > >> than me on what a good revision-specific match could be > > > > I am fine with auto-detection, though. > > > > ... > > > >>>> + > >>>> +examples: > >>>> + - | > >>>> + #include <dt-bindings/interrupt-controller/arm-gic.h> > >>>> + > >>>> + rpi1 { > >>> > >>> soc { > >>> > >> > >> Are you sure ? This will only ever live in the 'rp1' node. > > > > What is "rp1" node? Does not look like a generic name. > > I don't think this is right. RP1 is a separate chip, an IO controller, > on raspberrypi 5. BE is not in the RP1. Oops, missed this. Yes, I think it should be the "soc" node. Regards, Naush > > Tomi >
On Tue, Feb 13, 2024 at 08:35:39AM +0000, Naushir Patuck wrote: > On Tue, 13 Feb 2024 at 07:28, Tomi Valkeinen wrote: > > On 12/02/2024 11:05, Krzysztof Kozlowski wrote: > > > On 12/02/2024 09:50, Jacopo Mondi wrote: > > > > > >>>> +properties: > > >>>> + compatible: > > >>>> + const: raspberrypi,pispbe > > >>> > > >>> Nothing more specific? No model name, no version? It's quite generic > > >>> compatible which in general should not be allowed. I would assume that > > >>> at least version of Pi could denote some sort of a model... unless > > >>> version is detectable? > > >> > > >> The driver matches on a version register and that should be enough to > > >> handle quirks which are specific to an IP revision in the driver > > >> itself. > > >> > > >> Considering how minimal the integration with the SoC is (one clock, one > > >> interrupt and one optional iommu reference) even if we'll get future > > >> revisions of the SoC I don't think there will be any need to match on > > >> a dedicated compatible for bindings-validation purposes. > > >> > > >> However I understand that to be future-proof it's good practice to > > >> allow a more flexible scheme, so we can have a generic fallback and a > > >> revision-specific entry. > > >> > > >> Would > > >> > > >> compatible: > > >> items: > > >> - enum: > > >> - raspberrypi,pipspbe-bcm2712 > > > > > > bcm2712 is manufactured by Broadcom, not Raspberry Pi, so it should be > > > rather Pi model? > > > > Indeed, this is something I don't get. If the BE is in the bcm2712, is > > it not a broadcom IP? Why is raspberrypi in the compatible name at all? > > > > Naush, Dave? > > The Backend (and Frontend) IP are both owned solely by Raspberry Pi, > and the BE is instantiated on the BCM2712. So I think "raspberry" in > the compatible string is correct here. Following what we do with other SoCs, we could have compatible = "brcm,pispbe-bcm2712", "raspberrypi,pispbe"; However, that scheme is mostly used when SoC vendor license IP cores from third parties. Here, while the SoC is indeed manufactured by Broadcom, it's a Raspberry Pi-specific SoC. I don't have a personal preference. > > >> - const: raspberrypi,pispbe > > >> > > >> do in this case ? > > >> > > >> Also, let's see what RPi think as they are certainly more informed > > >> than me on what a good revision-specific match could be > > > > > > I am fine with auto-detection, though. > > > > > > ... > > > > > >>>> + > > >>>> +examples: > > >>>> + - | > > >>>> + #include <dt-bindings/interrupt-controller/arm-gic.h> > > >>>> + > > >>>> + rpi1 { > > >>> > > >>> soc { > > >>> > > >> > > >> Are you sure ? This will only ever live in the 'rp1' node. > > > > > > What is "rp1" node? Does not look like a generic name. > > > > I don't think this is right. RP1 is a separate chip, an IO controller, > > on raspberrypi 5. BE is not in the RP1. > > Oops, missed this. Yes, I think it should be the "soc" node.
Hi Jai On Sat, Feb 10, 2024 at 10:47:43AM +0530, Jai Luthra wrote: > Hi Jacopo, > > Thanks for the patch. > > On Feb 09, 2024 at 17:48:17 +0100, Jacopo Mondi wrote: > > From: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > This is the format used by monochrome 14bit image sensors. > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > .../userspace-api/media/v4l/pixfmt-y14p.rst | 47 +++++++++++++++++++ > > .../userspace-api/media/v4l/yuv-formats.rst | 1 + > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > > include/uapi/linux/videodev2.h | 1 + > > 4 files changed, 50 insertions(+) > > create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-y14p.rst > > > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-y14p.rst b/Documentation/userspace-api/media/v4l/pixfmt-y14p.rst > > new file mode 100644 > > index 000000000000..8e986bc6904f > > --- /dev/null > > +++ b/Documentation/userspace-api/media/v4l/pixfmt-y14p.rst > > @@ -0,0 +1,47 @@ > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later > > + > > +.. _V4L2-PIX-FMT-Y14P: > > + > > +************************** > > +V4L2_PIX_FMT_Y14P ('Y14P') > > +************************** > > + > > +Grey-scale image as a MIPI RAW14 packed array > > + > > + > > +Description > > +=========== > > + > > +This is a packed grey-scale image format with a depth of 14 bits per > > +pixel. Every four consecutive samples are packed into seven bytes. Each > > +of the first four bytes contain the eight high order bits of the pixels, > > +and the three following bytes contains the six least significants bits of > > +each pixel, in the same order. > > + > > +**Byte Order.** > > +Each cell is one byte. > > + > > +.. tabularcolumns:: |p{1.8cm}|p{1.0cm}|p{1.0cm}|p{1.0cm}|p{1.1cm}|p{3.3cm}|p{3.3cm}|p{3.3cm}| > > + > > +.. flat-table:: > > + :header-rows: 0 > > + :stub-columns: 0 > > + :widths: 2 1 1 1 1 3 3 3 > > + > > + > > + - - start + 0: > > + - Y'\ :sub:`00high` > > + - Y'\ :sub:`01high` > > + - Y'\ :sub:`02high` > > + - Y'\ :sub:`03high` > > + - Y'\ :sub:`01low bits 1--0`\ (bits 7--6) > > + > > + Y'\ :sub:`00low bits 5--0`\ (bits 5--0) > > + > > + - Y'\ :sub:`02low bits 3--0`\ (bits 7--4) > > + > > + Y'\ :sub:`01low bits 5--2`\ (bits 3--0) > > + > > + - Y'\ :sub:`03low bits 5--0`\ (bits 7--2) > > + > > + Y'\ :sub:`02low bits 5--4`\ (bits 1--0) > > diff --git a/Documentation/userspace-api/media/v4l/yuv-formats.rst b/Documentation/userspace-api/media/v4l/yuv-formats.rst > > index 7c9ccfdd94cd..6104747d17d4 100644 > > --- a/Documentation/userspace-api/media/v4l/yuv-formats.rst > > +++ b/Documentation/userspace-api/media/v4l/yuv-formats.rst > > @@ -268,6 +268,7 @@ image. > > pixfmt-yuv-planar > > pixfmt-yuv-luma > > pixfmt-y12p > > + pixfmt-y14p > > pixfmt-y8i > > pixfmt-y12i > > pixfmt-uv8 > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > > index 483498c55899..24f52485e59c 100644 > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > > @@ -1312,6 +1312,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > > case V4L2_PIX_FMT_Y10BPACK: descr = "10-bit Greyscale (Packed)"; break; > > case V4L2_PIX_FMT_Y10P: descr = "10-bit Greyscale (MIPI Packed)"; break; > > case V4L2_PIX_FMT_Y12P: descr = "12-bit Greyscale (MIPI Packed)"; break; > > + case V4L2_PIX_FMT_Y14P: descr = "14-bit Greyscale (MIPI Packed)"; break; > > case V4L2_PIX_FMT_IPU3_Y10: descr = "10-bit greyscale (IPU3 Packed)"; break; > > case V4L2_PIX_FMT_Y8I: descr = "Interleaved 8-bit Greyscale"; break; > > case V4L2_PIX_FMT_Y12I: descr = "Interleaved 12-bit Greyscale"; break; > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > index 11ebf9b22ccb..94a0373e8234 100644 > > --- a/include/uapi/linux/videodev2.h > > +++ b/include/uapi/linux/videodev2.h > > @@ -599,6 +599,7 @@ struct v4l2_pix_format { > > #define V4L2_PIX_FMT_Y10BPACK v4l2_fourcc('Y', '1', '0', 'B') /* 10 Greyscale bit-packed */ > > #define V4L2_PIX_FMT_Y10P v4l2_fourcc('Y', '1', '0', 'P') /* 10 Greyscale, MIPI RAW10 packed */ > > #define V4L2_PIX_FMT_Y12P v4l2_fourcc('Y', '1', '2', 'P') /* 12 Greyscale, MIPI RAW12 packed */ > > +#define V4L2_PIX_FMT_Y14P v4l2_fourcc('Y', '1', '4', 'P') /* 14 Greyscale, MIPI RAW12 packed */ > > I guess comment should have RAW14 instead of RAW12? > Indeed, thanks for spotting! Cheers > > #define V4L2_PIX_FMT_IPU3_Y10 v4l2_fourcc('i', 'p', '3', 'y') /* IPU3 packed 10-bit greyscale */ > > > > /* Palette formats */ > > -- > > 2.43.0 > > > > > > -- > Thanks, > Jai > > GPG Fingerprint: 4DE0 D818 E5D5 75E8 D45A AFC5 43DE 91F9 249A 7145
Hello On Mon, Feb 12, 2024 at 11:58:32AM +0000, Dave Stevenson wrote: > On Fri, 9 Feb 2024 at 21:15, Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > > > Le vendredi 09 février 2024 à 19:28 +0000, Dave Stevenson a écrit : > > > Hi Nicolas > > > > > > On Fri, 9 Feb 2024 at 17:53, Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > > > > > > > Le vendredi 09 février 2024 à 17:48 +0100, Jacopo Mondi a écrit : > > > > > From: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > > > > > > > This is the format used by monochrome 12bit image sensors. > > > > > > > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > --- > > > > > .../userspace-api/media/v4l/pixfmt-y12p.rst | 38 +++++++++++++++++++ > > > > > .../userspace-api/media/v4l/yuv-formats.rst | 1 + > > > > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > > > > > include/uapi/linux/videodev2.h | 1 + > > > > > 4 files changed, 41 insertions(+) > > > > > create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-y12p.rst > > > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-y12p.rst b/Documentation/userspace-api/media/v4l/pixfmt-y12p.rst > > > > > new file mode 100644 > > > > > index 000000000000..b2eb4a72724d > > > > > --- /dev/null > > > > > +++ b/Documentation/userspace-api/media/v4l/pixfmt-y12p.rst > > > > > @@ -0,0 +1,38 @@ > > > > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later > > > > > + > > > > > +.. _V4L2-PIX-FMT-Y12P: > > > > > + > > > > > +****************************** > > > > > +V4L2_PIX_FMT_Y12P ('Y12P') > > > > > +****************************** > > > > > + > > > > > +Grey-scale image as a MIPI RAW12 packed array > > > > > + > > > > > + > > > > > +Description > > > > > +=========== > > > > > + > > > > > +This is a packed grey-scale image format with a depth of 12 bits per > > > > > +pixel. Two consecutive pixels are packed into 3 bytes. The first 2 bytes > > > > > +contain the 8 high order bits of the pixels, and the 3rd byte contains the 4 > > > > > +least significants bits of each pixel, in the same order. > > > > > > > > This is an interesting arrangement, which make me feel that Y12P is perhaps bit > > > > too generic ? Perhaps something like: > > > > > > > > V4L2_PIX_FMT_Y12_MIPI > > > > > > > > That being said, I don't mind if you reserve the nice and short name for the > > > > first occurrence of such format in 20 years (it might equally be the last). Will > > > > they do the same for Y10, by storing 4 bytes of higher 8bit of 4 pixels, and > > > > packing the remaining lower 2 bits packed in the fif bytes ? Cause for this one, > > > > we'd have confusion, since CODEC usually just place all the bits in order over 5 > > > > bytes here (which have its own issues of course). > > > > > > We already have V4L2_PIX_FMT_Y10P defined for the MIPI packing for > > > 10bit luma-only. > > > https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/pixfmt-yuv-luma.html > > > > > > This is only extending the definitions for the other bit depths using > > > the same pattern. > > > I now wonder why creating two new files instead of adding Y12P and Y14P to the table of luma-only formats we have in the above mentioned file. In the notes at the bottom of the page we can indeed report that the P variants of the Y10 Y12 and Y14 formats are packed according to the MIPI CSI-2 specification, even if, as Dave said, this is a pretty standard convention for the educated readers of the v4l2 spec. In facts, this is what has been done already for the existing luma formats in: af4f45057695 ("media: doc: pixfmt-yuv: Move all luma-only YUV formats to common file") So I guess it's correct to keep expanding the existing file instead of adding new ones. Sorry for not having realized it earlier. > > > Adding the P is also the same pattern as for the Bayer formats - > > > V4L2_PIX_FMT_SRGGB10 for the 10bit unpacked to 16bit, and > > > V4L2_PIX_FMT_SRGGB10P for the MIPI packed variant. > > > > > > I'm not inventing anything new here :) > > > > A bit sad, but what can we do, I keep missing patches ;-P > > Not recently though. > > V4L2_PIX_FMT_Y10P was added in July 2018. [1] > The use of V4L2_PIX_FMT_Sxxxx10P for Bayer format MIPI 10 bit packing > was added in Dec 2014 [2] > > Dave > > [1] 6e15bec49f36 media: v4l: Add new 10-bit packed grayscale format > [2] 4353e36ee84d [media] v4l: Add packed Bayer raw10 pixel formats > > > Nicolas > > > > > > > > Dave > > > > > > > > + > > > > > +**Byte Order.** > > > > > +Each cell is one byte. > > > > > + > > > > > +.. tabularcolumns:: |p{2.2cm}|p{1.2cm}|p{1.2cm}|p{3.1cm}| > > > > > + > > > > > + > > > > > +.. flat-table:: > > > > > + :header-rows: 0 > > > > > + :stub-columns: 0 > > > > > + :widths: 2 1 1 1 > > > > > + > > > > > + > > > > > + - - start + 0: > > > > > + - Y'\ :sub:`00high` > > > > > + - Y'\ :sub:`01high` > > > > > + - Y'\ :sub:`01low`\ (bits 7--4) > > > > > + > > > > > + Y'\ :sub:`00low`\ (bits 3--0) > > > > > + > > > > > diff --git a/Documentation/userspace-api/media/v4l/yuv-formats.rst b/Documentation/userspace-api/media/v4l/yuv-formats.rst > > > > > index 24b34cdfa6fe..7c9ccfdd94cd 100644 > > > > > --- a/Documentation/userspace-api/media/v4l/yuv-formats.rst > > > > > +++ b/Documentation/userspace-api/media/v4l/yuv-formats.rst > > > > > @@ -267,6 +267,7 @@ image. > > > > > pixfmt-packed-yuv > > > > > pixfmt-yuv-planar > > > > > pixfmt-yuv-luma > > > > > + pixfmt-y12p > > > > > pixfmt-y8i > > > > > pixfmt-y12i > > > > > pixfmt-uv8 > > > > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > > > > > index 33076af4dfdb..483498c55899 100644 > > > > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > > > > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > > > > > @@ -1311,6 +1311,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > > > > > case V4L2_PIX_FMT_Y16_BE: descr = "16-bit Greyscale BE"; break; > > > > > case V4L2_PIX_FMT_Y10BPACK: descr = "10-bit Greyscale (Packed)"; break; > > > > > case V4L2_PIX_FMT_Y10P: descr = "10-bit Greyscale (MIPI Packed)"; break; > > > > > + case V4L2_PIX_FMT_Y12P: descr = "12-bit Greyscale (MIPI Packed)"; break; > > > > > case V4L2_PIX_FMT_IPU3_Y10: descr = "10-bit greyscale (IPU3 Packed)"; break; > > > > > case V4L2_PIX_FMT_Y8I: descr = "Interleaved 8-bit Greyscale"; break; > > > > > case V4L2_PIX_FMT_Y12I: descr = "Interleaved 12-bit Greyscale"; break; > > > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > > > > index a8015e5e7fa4..11ebf9b22ccb 100644 > > > > > --- a/include/uapi/linux/videodev2.h > > > > > +++ b/include/uapi/linux/videodev2.h > > > > > @@ -598,6 +598,7 @@ struct v4l2_pix_format { > > > > > /* Grey bit-packed formats */ > > > > > #define V4L2_PIX_FMT_Y10BPACK v4l2_fourcc('Y', '1', '0', 'B') /* 10 Greyscale bit-packed */ > > > > > #define V4L2_PIX_FMT_Y10P v4l2_fourcc('Y', '1', '0', 'P') /* 10 Greyscale, MIPI RAW10 packed */ > > > > > +#define V4L2_PIX_FMT_Y12P v4l2_fourcc('Y', '1', '2', 'P') /* 12 Greyscale, MIPI RAW12 packed */ > > > > > #define V4L2_PIX_FMT_IPU3_Y10 v4l2_fourcc('i', 'p', '3', 'y') /* IPU3 packed 10-bit greyscale */ > > > > > > > > > > /* Palette formats */ > > > > > >