| Message ID | 20241220-media-rpi-hevc-dec-v1-3-0ebcc04ed42e@raspberrypi.com |
|---|---|
| State | New |
| Headers | show |
| Series | Raspberry Pi HEVC decoder driver | expand |
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@collabora.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@raspberrypi.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. > 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.. */ >
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@collabora.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@raspberrypi.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.. */ > > >
On Mon, 6 Jan 2025 at 20:52, Nicolas Dufresne <nicolas@ndufresne.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@collabora.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@raspberrypi.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.. */ > > > > > >
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') + /* 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.. */
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@raspberrypi.com> --- drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++ include/uapi/linux/videodev2.h | 5 +++++ 2 files changed, 7 insertions(+)