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] [thread-next>] [day] [month] [year] [list]
Message-ID: <cfa4e4ebd9015a291e513fc22ec70821976e56ce.camel@ndufresne.ca>
Date: Mon, 06 Jan 2025 15:52:40 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Dave Stevenson <dave.stevenson@...pberrypi.com>, Robert Mader
	 <robert.mader@...labora.com>
Cc: 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

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.

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.. */
> > 
>