Hi Maintainers, Pinging on this again.
Tomi On 22/10/2025 10:33, Tomi Valkeinen wrote: > 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, >
