Re: [PATCH v6 00/11] drm: Add new pixel formats for Xilinx Zynqmp

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

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] git@github.com:tomba/kmsxx.git xilinx
>> [3] git@github.com:tomba/pykms.git xilinx
>> [4] git@github.com:tomba/pixutils.git xilinx
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> Changes in v6:
>> - Added tags for reviews
>> - Rebased on v6.17
>> - Link to v5: https://lore.kernel.org/r/20250425-xilinx-formats-v5-0-c74263231630@ideasonboard.com
>>
>> 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/20250326-xilinx-formats-v4-0-322a300c6d72@ideasonboard.com
>>
>> 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/20250212-xilinx-formats-v3-0-90d0fe106995@ideasonboard.com
>>
>> 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/20250115-xilinx-formats-v2-0-160327ca652a@ideasonboard.com
>>
>> 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/20241204-xilinx-formats-v1-0-0bf2c5147db1@ideasonboard.com
>>
>> ---
>> 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,
>