Thomas Zimmermann
2025-Jan-13 07:52 UTC
[PATCH v2 02/25] drm/dumb-buffers: Provide helper to set pitch and size
Hi Am 13.01.25 um 04:53 schrieb Andy Yan: [...]>> Thanks for taking a look. That NV-related code at [0] is a 'somewhat >> non-idiomatic use' of the UAPI. The dumb-buffer interface really just >> supports a single plane. The fix would be a new ioctl that takes a DRM >> 4cc constant and returns a buffer handle/pitch/size for each plane. But >> that's separate series throughout the various components. > So is there a standard way to create buffer for NV-related format now ?I don't know, but it doesn't look like there is. As I outlined, a new dumb-buffer interface seems required.> With a quick search, I can see many user space use dumb-buffer for NV-releated > buffer alloc: > > [0]https://github.com/tomba/kmsxx/blob/master/kms%2B%2B/src/pixelformats.cpp > [1]https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/lib/igt_fb.c?ref_type=heads > [2]https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/gst-plugins-bad/sys/kms/gstkmsutils.c?ref_type=heads#L116 > >> There's also code XRGB16161616F. This is a viable format for the UAPI, >> but seems not very useful in practice. >> >>> And there are also some AFBC based format with bpp can't be handled here, see: >>> static __u32 drm_gem_afbc_get_bpp(struct drm_device *dev, >>> const struct drm_mode_fb_cmd2 *mode_cmd) >>> { >>> const struct drm_format_info *info; >>> >>> info = drm_get_format_info(dev, mode_cmd); >>> >>> switch (info->format) { >>> case DRM_FORMAT_YUV420_8BIT: >>> return 12; >>> case DRM_FORMAT_YUV420_10BIT: >>> return 15; >>> case DRM_FORMAT_VUY101010: >>> return 30; >>> default: >>> return drm_format_info_bpp(info, 0); >>> } >>> } >> Same problem here. These YUV formats are multi-planar and there should >> be no dumb buffers for them. > These afbc based format are one plane, see:Apologies. I confused them with other YUV formats.> > /* > * 1-plane YUV 4:2:0 > * In these formats, the component ordering is specified (Y, followed by U > * then V), but the exact Linear layout is undefined. > * These formats can only be used with a non-Linear modifier. > */ > #define DRM_FORMAT_YUV420_8BIT fourcc_code('Y', 'U', '0', '8') > #define DRM_FORMAT_YUV420_10BIT fourcc_code('Y', 'U', '1', '0') > >> As we still have to support these all use cases, I've modified the new >> helper to fallback to computing the pitch from the given bpp value. >> That's what drivers currently do. Could you please apply the attached >> patch on top of the series and report back the result of the test? You >> should see a kernel warning about the unknown color mode, but allocation >> should succeed. > Yes, the attached patch works for my test case.Thanks for testing. I'll include the changes in the patch' next iteration. Best regards Thomas> >> Best regards >> Thomas >> >>> >>> [0]https://gitlab.freedesktop.org/mesa/drm/-/blob/main/tests/modetest/buffers.c?ref_type=heads#L159 >>> >>> This introduce a modetest failure on rockchip platform: >>> # modetest -M rockchip -s 70 at 68:1920x1080 -P 32 at 68:1920x1080 at NV30 >>> setting mode 1920x1080-60.00Hz on connectors 70, crtc 68 >>> testing 1920x1080 at NV30 overlay plane 32 >>> failed to create dumb buffer: Invalid argument >>> >>> I think other platform with bpp can't handler by drm_mode_legacy_fb_format will >>> also see this kind of failure: >>> >>> >>> >>>> + if (fourcc == DRM_FORMAT_INVALID) >>>> + return -EINVAL; >>>> + info = drm_format_info(fourcc); >>>> + if (!info) >>>> + return -EINVAL; >>>> + pitch = drm_format_info_min_pitch(info, 0, args->width); >>>> + if (!pitch || pitch > U32_MAX) >>>> + return -EINVAL; >>>> + >>>> + args->pitch = pitch; >>>> + >>>> + return drm_mode_align_dumb(args, pitch_align, size_align); >>>> +} >>>> +EXPORT_SYMBOL(drm_mode_size_dumb); >>>> + >>>> int drm_mode_create_dumb(struct drm_device *dev, >>>> struct drm_mode_create_dumb *args, >>>> struct drm_file *file_priv) >>>> diff --git a/include/drm/drm_dumb_buffers.h b/include/drm/drm_dumb_buffers.h >>>> new file mode 100644 >>>> index 000000000000..6fe36004b19d >>>> --- /dev/null >>>> +++ b/include/drm/drm_dumb_buffers.h >>>> @@ -0,0 +1,14 @@ >>>> +/* SPDX-License-Identifier: MIT */ >>>> + >>>> +#ifndef __DRM_DUMB_BUFFERS_H__ >>>> +#define __DRM_DUMB_BUFFERS_H__ >>>> + >>>> +struct drm_device; >>>> +struct drm_mode_create_dumb; >>>> + >>>> +int drm_mode_size_dumb(struct drm_device *dev, >>>> + struct drm_mode_create_dumb *args, >>>> + unsigned long pitch_align, >>>> + unsigned long size_align); >>>> + >>>> +#endif >>>> -- >>>> 2.47.1 >>>> >>>> >>>> _______________________________________________ >>>> Linux-rockchip mailing list >>>> Linux-rockchip at lists.infradead.org >>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip >> -- >> -- >> Thomas Zimmermann >> Graphics Driver Developer >> SUSE Software Solutions Germany GmbH >> Frankenstrasse 146, 90461 Nuernberg, Germany >> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman >> HRB 36809 (AG Nuernberg)-- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Seemingly Similar Threads
- [PATCH v2 02/25] drm/dumb-buffers: Provide helper to set pitch and size
- [PATCH v2 00/25] drm/dumb-buffers: Fix and improve buffer-size calculation
- [PATCH v2 13/25] drm/msm: Compute dumb-buffer sizes with drm_mode_size_dumb()
- [PATCH v2 10/25] drm/imx/ipuv3: Compute dumb-buffer sizes with drm_mode_size_dumb()
- [PATCH v2 25/25] drm/xlnx: Compute dumb-buffer sizes with drm_mode_size_dumb()