linux-kernel - Re: [PATCH 3/7] media: ioctl: Add pixel formats NV12MT_COL128 and NV12MT_10

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAPY8ntAxkdyngsCFfYNGNv6ObJU1PCMPvr_61R14a2H+dbZm2Q@mail.gmail.com>
Date: Tue, 7 Jan 2025 16:28:54 +0000
From: Dave Stevenson <dave.stevenson@...pberrypi.com>
To: Nicolas Dufresne <nicolas@...fresne.ca>
Cc: Robert Mader <robert.mader@...labora.com>, Sakari Ailus <sakari.ailus@...ux.intel.com>, 
	Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Florian Fainelli <florian.fainelli@...adcom.com>, 
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, John Cox <john.cox@...pberrypi.com>, 
	Dom Cobley <dom@...pberrypi.com>, review list <kernel-list@...pberrypi.com>, 
	Ezequiel Garcia <ezequiel@...guardiasur.com.ar>, John Cox <jc@...esim.co.uk>, 
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-rpi-kernel@...ts.infradead.org, 
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 3/7] media: ioctl: Add pixel formats NV12MT_COL128 and NV12MT_10_COL128

On Mon, 6 Jan 2025 at 20:52, Nicolas Dufresne <nicolas@...fresne.ca> wrote:
>
> Le jeudi 02 janvier 2025 à 12:52 +0000, Dave Stevenson a écrit :
> > Hi Robert
> >
> > Resending this reply as replying from my phone before the Christmas
> > break sent it as HTML :-(
> >
> > On Sat, 21 Dec 2024 at 17:38, Robert Mader <robert.mader@...labora.com> wrote:
> > >
> > > Hi, thanks for the patch.
> > >
> > > On 20.12.24 17:21, Dave Stevenson wrote:
> > >
> > > Add V4L2_PIXFMT_NV12MT_COL128 and V4L2_PIXFMT_NV12MT_10_COL128
> > > to describe the Raspberry Pi HEVC decoder NV12 multiplanar formats.
> > >
> > > Signed-off-by: Dave Stevenson <dave.stevenson@...pberrypi.com>
> > > ---
> > >  drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
> > >  include/uapi/linux/videodev2.h       | 5 +++++
> > >  2 files changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > index 0304daa8471d..e510e375a871 100644
> > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > @@ -1377,7 +1377,9 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> > >   case V4L2_PIX_FMT_NV16M: descr = "Y/UV 4:2:2 (N-C)"; break;
> > >   case V4L2_PIX_FMT_NV61M: descr = "Y/VU 4:2:2 (N-C)"; break;
> > >   case V4L2_PIX_FMT_NV12MT: descr = "Y/UV 4:2:0 (64x32 MB, N-C)"; break;
> > > + case V4L2_PIX_FMT_NV12MT_COL128: descr = "Y/CbCr 4:2:0 (128b cols)"; break;
> > >   case V4L2_PIX_FMT_NV12MT_16X16: descr = "Y/UV 4:2:0 (16x16 MB, N-C)"; break;
> > > + case V4L2_PIX_FMT_NV12MT_10_COL128: descr = "10-bit Y/CbCr 4:2:0 (128b cols)"; break;
> > >   case V4L2_PIX_FMT_P012M: descr = "12-bit Y/UV 4:2:0 (N-C)"; break;
> > >   case V4L2_PIX_FMT_YUV420M: descr = "Planar YUV 4:2:0 (N-C)"; break;
> > >   case V4L2_PIX_FMT_YVU420M: descr = "Planar YVU 4:2:0 (N-C)"; break;
> > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > > index e7c4dce39007..f8f97aa6a616 100644
> > > --- a/include/uapi/linux/videodev2.h
> > > +++ b/include/uapi/linux/videodev2.h
> > > @@ -687,6 +687,11 @@ struct v4l2_pix_format {
> > >  #define V4L2_PIX_FMT_NV12MT_16X16 v4l2_fourcc('V', 'M', '1', '2') /* 12  Y/CbCr 4:2:0 16x16 tiles */
> > >  #define V4L2_PIX_FMT_NV12M_8L128      v4l2_fourcc('N', 'A', '1', '2') /* Y/CbCr 4:2:0 8x128 tiles */
> > >  #define V4L2_PIX_FMT_NV12M_10BE_8L128 v4l2_fourcc_be('N', 'T', '1', '2') /* Y/CbCr 4:2:0 10-bit 8x128 tiles */
> > > +#define V4L2_PIX_FMT_NV12MT_COL128 v4l2_fourcc('N', 'c', '1', '2') /* 12  Y/CbCr 4:2:0 128 pixel wide column */
> > > +#define V4L2_PIX_FMT_NV12MT_10_COL128 v4l2_fourcc('N', 'c', '3', '0')
> > >
> > > Should these be upper-case Cs instead? So they compatible with the previously used downstream values?
> >
> > No, this is deliberate.
> >
> > Downstream was using a single planar format, with extra complexity for
> > determining the chroma offset per column, and weird handling required
> > on bytesperline.
> > Having had discussions, switching to a multiplanar format (hence MT in
> > the name) removes those complexities and means we don't need to do
> > anything weird in the v4l2 format definitions.
> >
> > Reusing NC12 and NC30 fourccs will give us grief over backwards
> > compatibility, hence lower case for the new version.
> >
> > I have a patch on [1] that adds back in NC12 and NC30 for downstream
> > just so we don't break existing users, but see no point in upstreaming
> > that.
>
> Yes, I think its fair to avoid incompatibility there. Are there matching Mesa
> patches coming, since without that we are back to square one, where the format
> remains unusable. NC12 have a matching (mainline) DRM_FORMAT_NV12
> DRM_FORMAT_MOD_BROADCOM_SAND128 pair. I believe a new modifier is needed and
> will serve both Nc12/30.

The current DRM_FORMAT_MOD_BROADCOM_SAND128 taking the column height
as the parameter is apparently contrary to how DRM modifiers are meant
to be used. You're not allowed to have a genuine runtime parameter in
there, and all potential values for parameters have to be listed out
in the in_formats blob.

I have a patch on [1] to add DRM_FORMAT_MOD_BROADCOM_SAND128(0) as a
modifier which then takes the height passed in to addFB2 to be the
column height. With luma and chroma now in independent buffers via the
multi-planar API, that solves the problem, and we don't need a new
modifier. I will submit that to dri-devel in the next week or so.

I've pinged Igalia to sort the equivalent Mesa patch for me.

 Dave

[1] https://github.com/6by9/linux/commits/rpi-6.12.y-hevc_dec patch
"drm/vc4: Add algorithmic handling for SAND". I can't give a hash as I
rebase that branch.


> Nicolas
>
> >
> > > P.S.: Coincidentally Gstreamer just landed support for NC12 in https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7355 and there is also a link to experimental NC30 patches. So happy to see this series appear upstream :)
> >
> > Ooh, nice. I'll give it a test when I'm back in the office.
> >
> > Dave
> >
> > [1] https://github.com/6by9/linux/commits/rpi-6.12.y-hevc_dec/
> >
> > > + /* Y/CbCr 4:2:0 10bpc, 3x10 packed as 4 bytes in
> > > + * a 128 bytes / 96 pixel wide column */
> > > +
> > >
> > >  /* Bayer formats - see http://www.siliconimaging.com/RGB%20Bayer.htm */
> > >  #define V4L2_PIX_FMT_SBGGR8  v4l2_fourcc('B', 'A', '8', '1') /*  8  BGBG.. GRGR.. */
> > >
> >
>