Hi DRM maintainers, On 01/10/2025 16:22, Tomi Valkeinen wrote: > Add new DRM pixel formats and add support for those in the Xilinx zynqmp > display driver.
Could someone merge this one, or at least the patches 1 to 6 (I can merge the xilinx patches separately). Tomi > All other formats except XVUY2101010 are already supported in upstream > gstreamer, but gstreamer's kmssink does not have the support yet, as it > obviously cannot support the formats without kernel having the formats. > > Xilinx has support for these formats in their BSP kernel, and Xilinx has > a branch here, adding the support to gstreamer kmssink: > > https://github.com/Xilinx/gst-plugins-bad.git xlnx-rebase-v1.18.5 > > New formats added: > > DRM_FORMAT_Y8 > - 8-bit Y-only > - fourcc: "GREY" > - gstreamer: GRAY8 > > DRM_FORMAT_Y10_P32 > - 10-bit Y-only, three pixels packed into 32-bits > - fourcc: "YPA4" > - gstreamer: GRAY10_LE32 > > DRM_FORMAT_XV15 > - Like NV12, but with 10-bit components > - fourcc: "XV15" > - gstreamer: NV12_10LE32 > > DRM_FORMAT_XV20 > - Like NV16, but with 10-bit components > - fourcc: "XV20" > - gstreamer: NV16_10LE32 > > DRM_FORMAT_X403 > - 10-bit planar 4:4:4, with three samples packed into 32-bits > - fourcc: "X403" > - gstreamer: Y444_10LE32 > > XVUY2101010 > - 10-bit 4:4:4, one pixel in 32 bits > - fourcc: "XY30" > > Some notes: > > I know the 8-bit greyscale format has been discussed before, and the > guidance was to use DRM_FORMAT_R8. While I'm not totally against that, I > would argue that adding DRM_FORMAT_Y8 makes sense, as: > > 1) We can mark it as 'is_yuv' in the drm_format_info, and this can help > the drivers handle e.g. full/limited range. Probably some hardware > handles grayscale as a value used for all RGB components, in which case > R8 makes sense, but when the hardware handles the Y-only pixels as YCbCr, > where Cb and Cr are "neutral", it makes more sense to consider the > format as an YUV format rather than RGB. > > 2) We can have the same fourcc as in v4l2. While not strictly necessary, > it's a constant source of confusion when the fourccs differ. > > 3) It (possibly) makes more sense for the user to use Y8/GREY format > instead of R8, as, in my experience, the documentation usually refers > to gray(scale) format or Y-only format. > > As we add new Y-only formats, it makes sense to have similar terms, so > we need to adjust the Y10_P32 format name accordingly. > > I have made some adjustments to the formats compared to the Xilinx's > branch. E.g. The DRM_FORMAT_Y10_P32 format in Xilinx's kmssink uses > fourcc "Y10 ", and DRM_FORMAT_Y10. I didn't like those, as the format is > a packed format, three 10-bit pixels in a 32-bit container, and I think > Y10 means a 10-bit pixel in a 16-bit container. > > Generally speaking, if someone has good ideas for the format define > names or fourccs, speak up, as it's not easy to invent good names =). > That said, keeping them the same as in the Xilinx trees will, of course, > be slightly easier for the users of Xilinx platforms. > > I made WIP additions to modetest to support most of these formats, > partially based on Xilinx's code: > > https://github.com/tomba/libdrm.git xilinx > > A few thoughts about that: > > modetest uses bo_create_dumb(), and as highlighted in recent discussions > in the kernel list [1], dumb buffers are only for RGB formats. They may > work for non-RGB formats, but that's platform specific. None of the > formats I add here are RGB formats. Do we want to go this way with > modetest? > > I also feel that the current structure of modetest is not well suited to > more complicated formats. Both the buffer allocation is a bit more > difficult (see "Add virtual_width and pixels_per_container"), and the > drawing is complicated (see, e.g., "Add support for DRM_FORMAT_XV15 & > DRM_FORMAT_XV20"). > > I have recently added support for these Xilinx formats to both kms++ [2] and > pykms/pixutils [3][4] (WIP), and it's not been easy... But I have to say I > think I like the template based version in kms++. That won't work in > modetest, of course, but a non-templated version might be implementable, > but probably much slower. > > In any case, I slighly feel it's not worth merging the modetest patches > I have for these formats: they complicate the code quite a bit, break > the RGB-only rule, and I'm not sure if there really are (m)any users. If > we want to add support to modetest, I think a bigger rewrite of the test > pattern code might be in order. > > [1] > https://lore.kernel.org/all/20250109150310.219442-26-tzimmermann%40suse.de/ > [2] [email protected]:tomba/kmsxx.git xilinx > [3] [email protected]:tomba/pykms.git xilinx > [4] [email protected]:tomba/pixutils.git xilinx > > Signed-off-by: Tomi Valkeinen <[email protected]> > --- > Changes in v6: > - Added tags for reviews > - Rebased on v6.17 > - Link to v5: > https://lore.kernel.org/r/[email protected] > > Changes in v5: > - Add comment about Y-only formats, clarifying how the display pipeline > handles them (they're handled as YCbCr, with Cb and Cr as "neutral") > - Clarify X403 format in the patch description > - Set unused Y-only CSC offsets to 0 (instead of 0x1800). > - Add R-bs > - Link to v4: > https://lore.kernel.org/r/[email protected] > > Changes in v4: > - Reformat the drm_format_info entries a bit > - Calculate block size only once in drm_format_info_bpp() > - Declare local variables in separate lines > - Add review tags > - Fix commit message referring to Y10_LE32 (should be Y10_P32) > - Link to v3: > https://lore.kernel.org/r/[email protected] > > Changes in v3: > - Drop "drm: xlnx: zynqmp: Fix max dma segment size". It is already > pushed. > - Add XVUY2101010 format. > - Rename DRM_FORMAT_Y10_LE32 to DRM_FORMAT_Y10_P32. > - Link to v2: > https://lore.kernel.org/r/[email protected] > > Changes in v2: > - I noticed V4L2 already has fourcc Y10P, referring to MIPI-style packed > Y10 format. So I changed Y10_LE32 fourcc to YPA4. If logic has any > relevance here, P means packed, A means 10, 4 means "in 4 bytes". > - Added tags to "Fix max dma segment size" patch > - Updated description for "Add warning for bad bpp" > - Link to v1: > https://lore.kernel.org/r/[email protected] > > --- > Tomi Valkeinen (11): > drm/fourcc: Add warning for bad bpp > drm/fourcc: Add DRM_FORMAT_XV15/XV20 > drm/fourcc: Add DRM_FORMAT_Y8 > drm/fourcc: Add DRM_FORMAT_Y10_P32 > drm/fourcc: Add DRM_FORMAT_X403 > drm/fourcc: Add DRM_FORMAT_XVUY2101010 > drm: xlnx: zynqmp: Use drm helpers when calculating buffer sizes > drm: xlnx: zynqmp: Add support for XV15 & XV20 > drm: xlnx: zynqmp: Add support for Y8 and Y10_P32 > drm: xlnx: zynqmp: Add support for X403 > drm: xlnx: zynqmp: Add support for XVUY2101010 > > drivers/gpu/drm/drm_fourcc.c | 28 ++++++++++++++++--- > drivers/gpu/drm/xlnx/zynqmp_disp.c | 55 > +++++++++++++++++++++++++++++++++++--- > include/uapi/drm/drm_fourcc.h | 29 ++++++++++++++++++++ > 3 files changed, 105 insertions(+), 7 deletions(-) > --- > base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a > change-id: 20241120-xilinx-formats-f71901621833 > > Best regards,
