| Message ID | 20240516122102.16379-15-yunfei.dong@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: mediatek: add driver to support secure video decoder | expand |
Hi Yunfei, W dniu 16.05.2024 o 14:20, Yunfei Dong pisze: > Define one uncompressed capture format V4L2_PIX_FMT_MS21 in order to > support one plane memory. The buffer size is luma + chroma, luma is > stored at the start and chrome is stored at the end. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 8 ++++++++ > drivers/media/v4l2-core/v4l2-common.c | 2 ++ > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > include/uapi/linux/videodev2.h | 1 + > 4 files changed, 12 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > index 886ba7b08d6b..6ec899649d50 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > @@ -295,6 +295,14 @@ please make a proposal on the linux-media mailing list. > - Compressed format used by Nuvoton NPCM video driver. This format is > defined in Remote Framebuffer Protocol (RFC 6143, chapter 7.7.4 Hextile > Encoding). > + * .. _V4L2-PIX-FMT-MS21: > + > + - ``V4L2_PIX_FMT_MS21`` > + - 'MS21' > + - This format has one plane, luma and chroma are stored in a contiguous Maybe s/one/single ? > + memory. Luma pixel in 16x32 tiles at the start, chroma pixel in 16x16 maybe the word "pixel" is reduntant here? What else than pixels could tile sizes mean? Any padding between luma and chroma? > + tiles at the end. The image height must be aligned with 32 and the image > + width must be aligned with 16. Maybe aligned to? > .. raw:: latex > > \normalsize > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > index 4165c815faef..5ae54cf48dc7 100644 > --- a/drivers/media/v4l2-core/v4l2-common.c > +++ b/drivers/media/v4l2-core/v4l2-common.c > @@ -271,6 +271,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format) > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2, > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > + { .format = V4L2_PIX_FMT_MS21, pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2, > + .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > > /* YUV planar formats */ > { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 }, > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index 4c76d17b4629..3a68f2b9e7a4 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1529,6 +1529,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > case V4L2_PIX_FMT_MT2110T: descr = "Mediatek 10bit Tile Mode"; break; > case V4L2_PIX_FMT_MT2110R: descr = "Mediatek 10bit Raster Mode"; break; > case V4L2_PIX_FMT_HEXTILE: descr = "Hextile Compressed Format"; break; > + case V4L2_PIX_FMT_MS21: descr = "MediaTek One Plane Format"; break; s/One/Single ? Regards, Andrzej > default: > if (fmt->description[0]) > return; > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 89eb1a3c6555..7aff2f2c8f9c 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -800,6 +800,7 @@ struct v4l2_pix_format { > #define V4L2_PIX_FMT_MM21 v4l2_fourcc('M', 'M', '2', '1') /* Mediatek 8-bit block mode, two non-contiguous planes */ > #define V4L2_PIX_FMT_MT2110T v4l2_fourcc('M', 'T', '2', 'T') /* Mediatek 10-bit block tile mode */ > #define V4L2_PIX_FMT_MT2110R v4l2_fourcc('M', 'T', '2', 'R') /* Mediatek 10-bit block raster mode */ > +#define V4L2_PIX_FMT_MS21 v4l2_fourcc('M', 'S', '2', '1') /* MediaTek 8-bit block mode with one plane */ > #define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar Greyscale 10-bit and Depth 16-bit */ > #define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit packed depth confidence information */ > #define V4L2_PIX_FMT_HI240 v4l2_fourcc('H', 'I', '2', '4') /* BTTV 8-bit dithered RGB */
Hi, I'm having second thoughts, please see inline, W dniu 22.05.2024 o 14:26, Andrzej Pietrasiewicz pisze: > Hi Yunfei, > > W dniu 16.05.2024 o 14:20, Yunfei Dong pisze: >> Define one uncompressed capture format V4L2_PIX_FMT_MS21 in order to >> support one plane memory. The buffer size is luma + chroma, luma is >> stored at the start and chrome is stored at the end. >> >> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> >> --- >> Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 8 ++++++++ >> drivers/media/v4l2-core/v4l2-common.c | 2 ++ >> drivers/media/v4l2-core/v4l2-ioctl.c | 1 + >> include/uapi/linux/videodev2.h | 1 + >> 4 files changed, 12 insertions(+) >> >> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >> index 886ba7b08d6b..6ec899649d50 100644 >> --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >> +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >> @@ -295,6 +295,14 @@ please make a proposal on the linux-media mailing list. >> - Compressed format used by Nuvoton NPCM video driver. This format is >> defined in Remote Framebuffer Protocol (RFC 6143, chapter 7.7.4 Hextile >> Encoding). >> + * .. _V4L2-PIX-FMT-MS21: >> + >> + - ``V4L2_PIX_FMT_MS21`` >> + - 'MS21' >> + - This format has one plane, luma and chroma are stored in a contiguous > > Maybe s/one/single ? > >> + memory. Luma pixel in 16x32 tiles at the start, chroma pixel in 16x16 > > maybe the word "pixel" is reduntant here? What else than pixels could tile sizes mean? > Any padding between luma and chroma? > >> + tiles at the end. The image height must be aligned with 32 and the image >> + width must be aligned with 16. > > Maybe aligned to? > >> .. raw:: latex >> \normalsize >> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c >> index 4165c815faef..5ae54cf48dc7 100644 >> --- a/drivers/media/v4l2-core/v4l2-common.c >> +++ b/drivers/media/v4l2-core/v4l2-common.c >> @@ -271,6 +271,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format) >> .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, >> { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2, >> .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, >> + { .format = V4L2_PIX_FMT_MS21, pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2, >> + .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, >> /* YUV planar formats */ >> { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 }, >> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c >> index 4c76d17b4629..3a68f2b9e7a4 100644 >> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >> @@ -1529,6 +1529,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) >> case V4L2_PIX_FMT_MT2110T: descr = "Mediatek 10bit Tile Mode"; break; >> case V4L2_PIX_FMT_MT2110R: descr = "Mediatek 10bit Raster Mode"; break; >> case V4L2_PIX_FMT_HEXTILE: descr = "Hextile Compressed Format"; break; >> + case V4L2_PIX_FMT_MS21: descr = "MediaTek One Plane Format"; break; > > s/One/Single ? > On the other hand "single" would be [in this case incorrectly] associated with single-planar API, which would be totally confusing. Still, the reality you are trying to model is complex: you use MPLANE, yet there's a single plane in case of secure playback. Regards, Andrzej > Regards, > > Andrzej > >> default: >> if (fmt->description[0]) >> return; >> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >> index 89eb1a3c6555..7aff2f2c8f9c 100644 >> --- a/include/uapi/linux/videodev2.h >> +++ b/include/uapi/linux/videodev2.h >> @@ -800,6 +800,7 @@ struct v4l2_pix_format { >> #define V4L2_PIX_FMT_MM21 v4l2_fourcc('M', 'M', '2', '1') /* Mediatek 8-bit block mode, two non-contiguous planes */ >> #define V4L2_PIX_FMT_MT2110T v4l2_fourcc('M', 'T', '2', 'T') /* Mediatek 10-bit block tile mode */ >> #define V4L2_PIX_FMT_MT2110R v4l2_fourcc('M', 'T', '2', 'R') /* Mediatek 10-bit block raster mode */ >> +#define V4L2_PIX_FMT_MS21 v4l2_fourcc('M', 'S', '2', '1') /* MediaTek 8-bit block mode with one plane */ >> #define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar Greyscale 10-bit and Depth 16-bit */ >> #define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit packed depth confidence information */ >> #define V4L2_PIX_FMT_HI240 v4l2_fourcc('H', 'I', '2', '4') /* BTTV 8-bit dithered RGB */ >
On Thu, May 23, 2024 at 6:14 PM Andrzej Pietrasiewicz <andrzej.p@collabora.com> wrote: > > Hi, > > I'm having second thoughts, please see inline, > > W dniu 22.05.2024 o 14:26, Andrzej Pietrasiewicz pisze: > > Hi Yunfei, > > > > W dniu 16.05.2024 o 14:20, Yunfei Dong pisze: > >> Define one uncompressed capture format V4L2_PIX_FMT_MS21 in order to > >> support one plane memory. The buffer size is luma + chroma, luma is > >> stored at the start and chrome is stored at the end. > >> > >> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > >> --- > >> Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 8 ++++++++ > >> drivers/media/v4l2-core/v4l2-common.c | 2 ++ > >> drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > >> include/uapi/linux/videodev2.h | 1 + > >> 4 files changed, 12 insertions(+) > >> > >> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > >> index 886ba7b08d6b..6ec899649d50 100644 > >> --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > >> +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > >> @@ -295,6 +295,14 @@ please make a proposal on the linux-media mailing list. > >> - Compressed format used by Nuvoton NPCM video driver. This format is > >> defined in Remote Framebuffer Protocol (RFC 6143, chapter 7.7.4 Hextile > >> Encoding). > >> + * .. _V4L2-PIX-FMT-MS21: > >> + > >> + - ``V4L2_PIX_FMT_MS21`` > >> + - 'MS21' > >> + - This format has one plane, luma and chroma are stored in a contiguous > > > > Maybe s/one/single ? > > > >> + memory. Luma pixel in 16x32 tiles at the start, chroma pixel in 16x16 > > > > maybe the word "pixel" is reduntant here? What else than pixels could tile sizes mean? > > Any padding between luma and chroma? > > > >> + tiles at the end. The image height must be aligned with 32 and the image > >> + width must be aligned with 16. > > > > Maybe aligned to? > > > >> .. raw:: latex > >> \normalsize > >> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > >> index 4165c815faef..5ae54cf48dc7 100644 > >> --- a/drivers/media/v4l2-core/v4l2-common.c > >> +++ b/drivers/media/v4l2-core/v4l2-common.c > >> @@ -271,6 +271,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format) > >> .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > >> { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2, > >> .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > >> + { .format = V4L2_PIX_FMT_MS21, pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2, > >> + .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > >> /* YUV planar formats */ > >> { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 }, > >> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > >> index 4c76d17b4629..3a68f2b9e7a4 100644 > >> --- a/drivers/media/v4l2-core/v4l2-ioctl.c > >> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > >> @@ -1529,6 +1529,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > >> case V4L2_PIX_FMT_MT2110T: descr = "Mediatek 10bit Tile Mode"; break; > >> case V4L2_PIX_FMT_MT2110R: descr = "Mediatek 10bit Raster Mode"; break; > >> case V4L2_PIX_FMT_HEXTILE: descr = "Hextile Compressed Format"; break; > >> + case V4L2_PIX_FMT_MS21: descr = "MediaTek One Plane Format"; break; > > > > s/One/Single ? > > > > On the other hand "single" would be [in this case incorrectly] associated with > single-planar API, which would be totally confusing. > > Still, the reality you are trying to model is complex: you use > MPLANE, yet there's a single plane in case of secure playback. I don't think that's a problem though. The NV12 format (seen above in the diff context) has the same attributes: 1 contiguous memory plane containing two component planes. And it's perfectly fine for MPLANE drivers to support these formats, Hantro being one of them that decodes to NV12. If a decoder can decode into discontiguous buffers, it surely can decode into contiguous ones; just set the second/third buffer address to the correct offset into the first buffer based on the format layout. In retrospect we probably should have named "MM21" as "MM21M" to denote that it is multi-memory-plane, and this "MS21" would have been "MM21", having a single contiguous memory plane. And the MPLANE driver supporting "single memory plane" oddity gets resolved if the API rework ever gets finished. Regards, ChenYu > Regards, > > Andrzej > > > > Regards, > > > > Andrzej > > > >> default: > >> if (fmt->description[0]) > >> return; > >> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > >> index 89eb1a3c6555..7aff2f2c8f9c 100644 > >> --- a/include/uapi/linux/videodev2.h > >> +++ b/include/uapi/linux/videodev2.h > >> @@ -800,6 +800,7 @@ struct v4l2_pix_format { > >> #define V4L2_PIX_FMT_MM21 v4l2_fourcc('M', 'M', '2', '1') /* Mediatek 8-bit block mode, two non-contiguous planes */ > >> #define V4L2_PIX_FMT_MT2110T v4l2_fourcc('M', 'T', '2', 'T') /* Mediatek 10-bit block tile mode */ > >> #define V4L2_PIX_FMT_MT2110R v4l2_fourcc('M', 'T', '2', 'R') /* Mediatek 10-bit block raster mode */ > >> +#define V4L2_PIX_FMT_MS21 v4l2_fourcc('M', 'S', '2', '1') /* MediaTek 8-bit block mode with one plane */ > >> #define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar Greyscale 10-bit and Depth 16-bit */ > >> #define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit packed depth confidence information */ > >> #define V4L2_PIX_FMT_HI240 v4l2_fourcc('H', 'I', '2', '4') /* BTTV 8-bit dithered RGB */ > > >
Hi, On Thu, May 16, 2024 at 8:21 PM Yunfei Dong <yunfei.dong@mediatek.com> wrote: > > Define one uncompressed capture format V4L2_PIX_FMT_MS21 in order to > support one plane memory. The buffer size is luma + chroma, luma is > stored at the start and chrome is stored at the end. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 8 ++++++++ > drivers/media/v4l2-core/v4l2-common.c | 2 ++ > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > include/uapi/linux/videodev2.h | 1 + > 4 files changed, 12 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > index 886ba7b08d6b..6ec899649d50 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > @@ -295,6 +295,14 @@ please make a proposal on the linux-media mailing list. > - Compressed format used by Nuvoton NPCM video driver. This format is > defined in Remote Framebuffer Protocol (RFC 6143, chapter 7.7.4 Hextile > Encoding). > + * .. _V4L2-PIX-FMT-MS21: > + > + - ``V4L2_PIX_FMT_MS21`` > + - 'MS21' > + - This format has one plane, luma and chroma are stored in a contiguous > + memory. Luma pixel in 16x32 tiles at the start, chroma pixel in 16x16 > + tiles at the end. The image height must be aligned with 32 and the image > + width must be aligned with 16. > .. raw:: latex > > \normalsize > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > index 4165c815faef..5ae54cf48dc7 100644 > --- a/drivers/media/v4l2-core/v4l2-common.c > +++ b/drivers/media/v4l2-core/v4l2-common.c > @@ -271,6 +271,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format) > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2, > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > + { .format = V4L2_PIX_FMT_MS21, pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2, ^ This is missing a '.' before 'pixel_enc', which breaks the build. ChenYu > + .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > > /* YUV planar formats */ > { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 }, > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index 4c76d17b4629..3a68f2b9e7a4 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1529,6 +1529,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > case V4L2_PIX_FMT_MT2110T: descr = "Mediatek 10bit Tile Mode"; break; > case V4L2_PIX_FMT_MT2110R: descr = "Mediatek 10bit Raster Mode"; break; > case V4L2_PIX_FMT_HEXTILE: descr = "Hextile Compressed Format"; break; > + case V4L2_PIX_FMT_MS21: descr = "MediaTek One Plane Format"; break; > default: > if (fmt->description[0]) > return; > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 89eb1a3c6555..7aff2f2c8f9c 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -800,6 +800,7 @@ struct v4l2_pix_format { > #define V4L2_PIX_FMT_MM21 v4l2_fourcc('M', 'M', '2', '1') /* Mediatek 8-bit block mode, two non-contiguous planes */ > #define V4L2_PIX_FMT_MT2110T v4l2_fourcc('M', 'T', '2', 'T') /* Mediatek 10-bit block tile mode */ > #define V4L2_PIX_FMT_MT2110R v4l2_fourcc('M', 'T', '2', 'R') /* Mediatek 10-bit block raster mode */ > +#define V4L2_PIX_FMT_MS21 v4l2_fourcc('M', 'S', '2', '1') /* MediaTek 8-bit block mode with one plane */ > #define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar Greyscale 10-bit and Depth 16-bit */ > #define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit packed depth confidence information */ > #define V4L2_PIX_FMT_HI240 v4l2_fourcc('H', 'I', '2', '4') /* BTTV 8-bit dithered RGB */ > -- > 2.25.1 >
Hi, Le jeudi 23 mai 2024 à 18:36 +0800, Chen-Yu Tsai a écrit : > On Thu, May 23, 2024 at 6:14 PM Andrzej Pietrasiewicz > <andrzej.p@collabora.com> wrote: > > > > Hi, > > > > I'm having second thoughts, please see inline, > > > > W dniu 22.05.2024 o 14:26, Andrzej Pietrasiewicz pisze: > > > Hi Yunfei, > > > > > > W dniu 16.05.2024 o 14:20, Yunfei Dong pisze: > > > > Define one uncompressed capture format V4L2_PIX_FMT_MS21 in order to > > > > support one plane memory. The buffer size is luma + chroma, luma is > > > > stored at the start and chrome is stored at the end. > > > > > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > > > --- > > > > Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 8 ++++++++ > > > > drivers/media/v4l2-core/v4l2-common.c | 2 ++ > > > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > > > > include/uapi/linux/videodev2.h | 1 + > > > > 4 files changed, 12 insertions(+) > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > > > > index 886ba7b08d6b..6ec899649d50 100644 > > > > --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > > > > +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > > > > @@ -295,6 +295,14 @@ please make a proposal on the linux-media mailing list. > > > > - Compressed format used by Nuvoton NPCM video driver. This format is > > > > defined in Remote Framebuffer Protocol (RFC 6143, chapter 7.7.4 Hextile > > > > Encoding). > > > > + * .. _V4L2-PIX-FMT-MS21: > > > > + > > > > + - ``V4L2_PIX_FMT_MS21`` > > > > + - 'MS21' > > > > + - This format has one plane, luma and chroma are stored in a contiguous > > > > > > Maybe s/one/single ? > > > > > > > + memory. Luma pixel in 16x32 tiles at the start, chroma pixel in 16x16 > > > > > > maybe the word "pixel" is reduntant here? What else than pixels could tile sizes mean? > > > Any padding between luma and chroma? > > > > > > > + tiles at the end. The image height must be aligned with 32 and the image > > > > + width must be aligned with 16. > > > > > > Maybe aligned to? > > > > > > > .. raw:: latex > > > > \normalsize > > > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > > > > index 4165c815faef..5ae54cf48dc7 100644 > > > > --- a/drivers/media/v4l2-core/v4l2-common.c > > > > +++ b/drivers/media/v4l2-core/v4l2-common.c > > > > @@ -271,6 +271,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format) > > > > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > > > > { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2, > > > > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > > > > + { .format = V4L2_PIX_FMT_MS21, pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2, > > > > + .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > > > > /* YUV planar formats */ > > > > { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 }, > > > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > > > > index 4c76d17b4629..3a68f2b9e7a4 100644 > > > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > > > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > > > > @@ -1529,6 +1529,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > > > > case V4L2_PIX_FMT_MT2110T: descr = "Mediatek 10bit Tile Mode"; break; > > > > case V4L2_PIX_FMT_MT2110R: descr = "Mediatek 10bit Raster Mode"; break; > > > > case V4L2_PIX_FMT_HEXTILE: descr = "Hextile Compressed Format"; break; > > > > + case V4L2_PIX_FMT_MS21: descr = "MediaTek One Plane Format"; break; > > > > > > s/One/Single ? > > > > > > > On the other hand "single" would be [in this case incorrectly] associated with > > single-planar API, which would be totally confusing. > > > > Still, the reality you are trying to model is complex: you use > > MPLANE, yet there's a single plane in case of secure playback. > > I don't think that's a problem though. The NV12 format (seen above in > the diff context) has the same attributes: 1 contiguous memory plane > containing two component planes. > > And it's perfectly fine for MPLANE drivers to support these formats, > Hantro being one of them that decodes to NV12. If a decoder can decode > into discontiguous buffers, it surely can decode into contiguous ones; > just set the second/third buffer address to the correct offset into > the first buffer based on the format layout. There is some complexity and limitations not disclose here, but in 99% of the cases I agree. > > In retrospect we probably should have named "MM21" as "MM21M" to denote > that it is multi-memory-plane, and this "MS21" would have been "MM21", > having a single contiguous memory plane. > > And the MPLANE driver supporting "single memory plane" oddity gets > resolved if the API rework ever gets finished. Not clear what you are proposing here, since its too late to rename MM21. I agree with you there is no problem adding single allocation version of MM21 to save on secure zones, and this is intented to be support by the MPLANE API too. Though, on the other side, I'm thinking its an internal limitation that we must have multiple allocation when using planar formats. We could certainly export twice the same dmabuf, in that capture case, data_offset would be used with its original design (driver provided). In fact, if we remove this internal limitation, we could stop the proliferation of duplicated pixel formats in V4L2 which is quite a bad design error in my opinion. Nicolas > > > Regards, > ChenYu > > > Regards, > > > > Andrzej > > > > > > > Regards, > > > > > > Andrzej > > > > > > > default: > > > > if (fmt->description[0]) > > > > return; > > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > > > index 89eb1a3c6555..7aff2f2c8f9c 100644 > > > > --- a/include/uapi/linux/videodev2.h > > > > +++ b/include/uapi/linux/videodev2.h > > > > @@ -800,6 +800,7 @@ struct v4l2_pix_format { > > > > #define V4L2_PIX_FMT_MM21 v4l2_fourcc('M', 'M', '2', '1') /* Mediatek 8-bit block mode, two non-contiguous planes */ > > > > #define V4L2_PIX_FMT_MT2110T v4l2_fourcc('M', 'T', '2', 'T') /* Mediatek 10-bit block tile mode */ > > > > #define V4L2_PIX_FMT_MT2110R v4l2_fourcc('M', 'T', '2', 'R') /* Mediatek 10-bit block raster mode */ > > > > +#define V4L2_PIX_FMT_MS21 v4l2_fourcc('M', 'S', '2', '1') /* MediaTek 8-bit block mode with one plane */ > > > > #define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar Greyscale 10-bit and Depth 16-bit */ > > > > #define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit packed depth confidence information */ > > > > #define V4L2_PIX_FMT_HI240 v4l2_fourcc('H', 'I', '2', '4') /* BTTV 8-bit dithered RGB */ > > > > >
Hi Andrzej, Thanks for your help to review this patch. On Wed, 2024-05-22 at 14:26 +0200, Andrzej Pietrasiewicz wrote: > Hi Yunfei, > > W dniu 16.05.2024 o 14:20, Yunfei Dong pisze: > > Define one uncompressed capture format V4L2_PIX_FMT_MS21 in order > > to > > support one plane memory. The buffer size is luma + chroma, luma is > > stored at the start and chrome is stored at the end. > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > --- > > Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 8 > > ++++++++ > > drivers/media/v4l2-core/v4l2-common.c | 2 ++ > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > > include/uapi/linux/videodev2.h | 1 + > > 4 files changed, 12 insertions(+) > > > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt- > > reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt- > > reserved.rst > > index 886ba7b08d6b..6ec899649d50 100644 > > --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > > +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > > @@ -295,6 +295,14 @@ please make a proposal on the linux-media > > mailing list. > > - Compressed format used by Nuvoton NPCM video driver. This > > format is > > defined in Remote Framebuffer Protocol (RFC 6143, chapter > > 7.7.4 Hextile > > Encoding). > > + * .. _V4L2-PIX-FMT-MS21: > > + > > + - ``V4L2_PIX_FMT_MS21`` > > + - 'MS21' > > + - This format has one plane, luma and chroma are stored in a > > contiguous > > Maybe s/one/single ? > > > + memory. Luma pixel in 16x32 tiles at the start, chroma > > pixel in 16x16 > > maybe the word "pixel" is reduntant here? What else than pixels could > tile sizes mean? > Any padding between luma and chroma? > The secure memory only has one plane: Luma data + Chrome data. Luma data is in order of 16x32 block, chrome is in order of 16x16 block. Width and height is aligned with fixed value. I need to re-write the description again. Best Regards, Yunfei Dong > > + tiles at the end. The image height must be aligned with 32 > > and the image > > + width must be aligned with 16. > > Maybe aligned to? > > > .. raw:: latex > > > > \normalsize > > diff --git a/drivers/media/v4l2-core/v4l2-common.c > > b/drivers/media/v4l2-core/v4l2-common.c > > index 4165c815faef..5ae54cf48dc7 100644 > > --- a/drivers/media/v4l2-core/v4l2-common.c > > +++ b/drivers/media/v4l2-core/v4l2-common.c > > @@ -271,6 +271,8 @@ const struct v4l2_format_info > > *v4l2_format_info(u32 format) > > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 > > }}, > > { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = > > V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, > > 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2, > > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 > > }}, > > + { .format = V4L2_PIX_FMT_MS21, pixel_enc = > > V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, > > 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2, > > + .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 > > }}, > > > > /* YUV planar formats */ > > { .format = V4L2_PIX_FMT_NV12, .pixel_enc = > > V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, > > 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 }, > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c > > b/drivers/media/v4l2-core/v4l2-ioctl.c > > index 4c76d17b4629..3a68f2b9e7a4 100644 > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > > @@ -1529,6 +1529,7 @@ static void v4l_fill_fmtdesc(struct > > v4l2_fmtdesc *fmt) > > case V4L2_PIX_FMT_MT2110T: descr = "Mediatek 10bit > > Tile Mode"; break; > > case V4L2_PIX_FMT_MT2110R: descr = "Mediatek 10bit > > Raster Mode"; break; > > case V4L2_PIX_FMT_HEXTILE: descr = "Hextile Compressed > > Format"; break; > > + case V4L2_PIX_FMT_MS21: descr = "MediaTek > > One Plane Format"; break; > > s/One/Single ? > > Regards, > > Andrzej > > > default: > > if (fmt->description[0]) > > return; > > diff --git a/include/uapi/linux/videodev2.h > > b/include/uapi/linux/videodev2.h > > index 89eb1a3c6555..7aff2f2c8f9c 100644 > > --- a/include/uapi/linux/videodev2.h > > +++ b/include/uapi/linux/videodev2.h > > @@ -800,6 +800,7 @@ struct v4l2_pix_format { > > #define V4L2_PIX_FMT_MM21 v4l2_fourcc('M', 'M', '2', '1') /* > > Mediatek 8-bit block mode, two non-contiguous planes */ > > #define V4L2_PIX_FMT_MT2110T v4l2_fourcc('M', 'T', '2', 'T') /* > > Mediatek 10-bit block tile mode */ > > #define V4L2_PIX_FMT_MT2110R v4l2_fourcc('M', 'T', '2', 'R') /* > > Mediatek 10-bit block raster mode */ > > +#define V4L2_PIX_FMT_MS21 v4l2_fourcc('M', 'S', '2', '1') /* > > MediaTek 8-bit block mode with one plane */ > > #define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* > > Intel Planar Greyscale 10-bit and Depth 16-bit */ > > #define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* > > Intel 4-bit packed depth confidence information */ > > #define V4L2_PIX_FMT_HI240 v4l2_fourcc('H', 'I', '2', '4') /* > > BTTV 8-bit dithered RGB */ > >
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst index 886ba7b08d6b..6ec899649d50 100644 --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst @@ -295,6 +295,14 @@ please make a proposal on the linux-media mailing list. - Compressed format used by Nuvoton NPCM video driver. This format is defined in Remote Framebuffer Protocol (RFC 6143, chapter 7.7.4 Hextile Encoding). + * .. _V4L2-PIX-FMT-MS21: + + - ``V4L2_PIX_FMT_MS21`` + - 'MS21' + - This format has one plane, luma and chroma are stored in a contiguous + memory. Luma pixel in 16x32 tiles at the start, chroma pixel in 16x16 + tiles at the end. The image height must be aligned with 32 and the image + width must be aligned with 16. .. raw:: latex \normalsize diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index 4165c815faef..5ae54cf48dc7 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -271,6 +271,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format) .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2, .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, + { .format = V4L2_PIX_FMT_MS21, pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2, + .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, /* YUV planar formats */ { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 }, diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 4c76d17b4629..3a68f2b9e7a4 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -1529,6 +1529,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) case V4L2_PIX_FMT_MT2110T: descr = "Mediatek 10bit Tile Mode"; break; case V4L2_PIX_FMT_MT2110R: descr = "Mediatek 10bit Raster Mode"; break; case V4L2_PIX_FMT_HEXTILE: descr = "Hextile Compressed Format"; break; + case V4L2_PIX_FMT_MS21: descr = "MediaTek One Plane Format"; break; default: if (fmt->description[0]) return; diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 89eb1a3c6555..7aff2f2c8f9c 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -800,6 +800,7 @@ struct v4l2_pix_format { #define V4L2_PIX_FMT_MM21 v4l2_fourcc('M', 'M', '2', '1') /* Mediatek 8-bit block mode, two non-contiguous planes */ #define V4L2_PIX_FMT_MT2110T v4l2_fourcc('M', 'T', '2', 'T') /* Mediatek 10-bit block tile mode */ #define V4L2_PIX_FMT_MT2110R v4l2_fourcc('M', 'T', '2', 'R') /* Mediatek 10-bit block raster mode */ +#define V4L2_PIX_FMT_MS21 v4l2_fourcc('M', 'S', '2', '1') /* MediaTek 8-bit block mode with one plane */ #define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar Greyscale 10-bit and Depth 16-bit */ #define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit packed depth confidence information */ #define V4L2_PIX_FMT_HI240 v4l2_fourcc('H', 'I', '2', '4') /* BTTV 8-bit dithered RGB */
Define one uncompressed capture format V4L2_PIX_FMT_MS21 in order to support one plane memory. The buffer size is luma + chroma, luma is stored at the start and chrome is stored at the end. Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> --- Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 8 ++++++++ drivers/media/v4l2-core/v4l2-common.c | 2 ++ drivers/media/v4l2-core/v4l2-ioctl.c | 1 + include/uapi/linux/videodev2.h | 1 + 4 files changed, 12 insertions(+)