| Message ID | 20250115-xilinx-formats-v2-4-160327ca652a@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm: Add new pixel formats for Xilinx Zynqmp | expand |
Hi Tomi, On Wed, Jan 15, 2025 at 10:04 AM Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > Add Y10_LE32, a 10 bit greyscale format, with 3 pixels packed into > 32-bit container. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Thanks for your patch! > --- a/include/uapi/drm/drm_fourcc.h > +++ b/include/uapi/drm/drm_fourcc.h > @@ -408,6 +408,7 @@ extern "C" { > /* Greyscale formats */ > > #define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit Y-only */ > +#define DRM_FORMAT_Y10_LE32 fourcc_code('Y', 'P', 'A', '4') /* [31:0] x:Y2:Y1:Y0 2:10:10:10 little endian */ R10_LE32? Or R10_PA4? Does LE32 have a meaning? My first guess just reading the subject was wrong ("little endian 32-bit" ;-) Gr{oetje,eeting}s, Geert
Hi, On 15/01/2025 12:33, Geert Uytterhoeven wrote: > Hi Tomi, > > On Wed, Jan 15, 2025 at 10:04 AM Tomi Valkeinen > <tomi.valkeinen@ideasonboard.com> wrote: >> Add Y10_LE32, a 10 bit greyscale format, with 3 pixels packed into >> 32-bit container. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > Thanks for your patch! > >> --- a/include/uapi/drm/drm_fourcc.h >> +++ b/include/uapi/drm/drm_fourcc.h >> @@ -408,6 +408,7 @@ extern "C" { >> /* Greyscale formats */ >> >> #define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit Y-only */ >> +#define DRM_FORMAT_Y10_LE32 fourcc_code('Y', 'P', 'A', '4') /* [31:0] x:Y2:Y1:Y0 2:10:10:10 little endian */ > > R10_LE32? Or R10_PA4? Can we discuss the "R" vs "Y" question under the cover letter? There's some more context about it in there. I took the "LE32" from Gstreamer's format. Maybe it's a bit pointless. I don't know if it makes sense to add the fourcc to the DRM format name. The fourcc is very limited. Rather, we could, say, have DRM_FORMAT_Y10_PACKED_32 (or "R", if you insist =). > > Does LE32 have a meaning? My first guess just reading the subject > was wrong ("little endian 32-bit" ;-) I'm not sure I follow. It's little-endian. The pixel group/unit is a 32-bit number, where the leftmost pixel on the screen is in bits 9-0, and the padding is in bits 31-30, and stored in memory as little-endian. Tomi
Hi Tomi, On Wed, Jan 15, 2025 at 12:11 PM Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > On 15/01/2025 12:33, Geert Uytterhoeven wrote: > > On Wed, Jan 15, 2025 at 10:04 AM Tomi Valkeinen > > <tomi.valkeinen@ideasonboard.com> wrote: > >> Add Y10_LE32, a 10 bit greyscale format, with 3 pixels packed into > >> 32-bit container. > >> > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > > > Thanks for your patch! > > > >> --- a/include/uapi/drm/drm_fourcc.h > >> +++ b/include/uapi/drm/drm_fourcc.h > >> @@ -408,6 +408,7 @@ extern "C" { > >> /* Greyscale formats */ > >> > >> #define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit Y-only */ > >> +#define DRM_FORMAT_Y10_LE32 fourcc_code('Y', 'P', 'A', '4') /* [31:0] x:Y2:Y1:Y0 2:10:10:10 little endian */ > > > > R10_LE32? Or R10_PA4? > > Can we discuss the "R" vs "Y" question under the cover letter? There's > some more context about it in there. Sorry, hadn't read the cover letter. I got attracted by "Y8" and "Y10". > I took the "LE32" from Gstreamer's format. Maybe it's a bit pointless. > > I don't know if it makes sense to add the fourcc to the DRM format name. > The fourcc is very limited. Rather, we could, say, have > DRM_FORMAT_Y10_PACKED_32 (or "R", if you insist =). > > > Does LE32 have a meaning? My first guess just reading the subject > > was wrong ("little endian 32-bit" ;-) > > I'm not sure I follow. It's little-endian. The pixel group/unit is a > 32-bit number, where the leftmost pixel on the screen is in bits 9-0, > and the padding is in bits 31-30, and stored in memory as little-endian. Ah, the "LE" applies to the pixels inside each word. DRM formats stored in memory are always little-endian, unless the DRM_FORMAT_BIG_ENDIAN bit is set, which is what I was hinting at... Gr{oetje,eeting}s, Geert
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index d721d9fdbe98..6048e0a191dc 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -327,6 +327,10 @@ const struct drm_format_info *__drm_format_info(u32 format) .num_planes = 2, .char_per_block = { 4, 8, 0 }, .block_w = { 3, 3, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true }, + { .format = DRM_FORMAT_Y10_LE32, .depth = 0, + .num_planes = 1, .char_per_block = { 4, 0, 0 }, + .block_w = { 3, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 1, + .vsub = 1, .is_yuv = true }, }; unsigned int i; diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index f79ee3b93f09..03be2aa40265 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -408,6 +408,7 @@ extern "C" { /* Greyscale formats */ #define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8-bit Y-only */ +#define DRM_FORMAT_Y10_LE32 fourcc_code('Y', 'P', 'A', '4') /* [31:0] x:Y2:Y1:Y0 2:10:10:10 little endian */ /* * Format Modifiers:
Add Y10_LE32, a 10 bit greyscale format, with 3 pixels packed into 32-bit container. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- drivers/gpu/drm/drm_fourcc.c | 4 ++++ include/uapi/drm/drm_fourcc.h | 1 + 2 files changed, 5 insertions(+)