[PATCH v2 02/25] drm/dumb-buffers: Provide helper to set pitch and size
Andy Yan
andyshrk at 163.com
Fri Jan 10 01:49:34 UTC 2025
Hi Thomas,
At 2025-01-09 22:56:56, "Thomas Zimmermann" <tzimmermann at suse.de> wrote:
>Add drm_modes_size_dumb(), a helper to calculate the dumb-buffer
>scanline pitch and allocation size. Implementations of struct
>drm_driver.dumb_create can call the new helper for their size
>computations. There's currently quite a bit of code duplication
>among DRM's memory managers. Each calculates scanline pitch and
>buffer size from the given arguments, but the implementations are
>inconsistent in how they treat alignment and format support. Later
>patches will unify this code on top of drm_mode_size_dumb() as
>much as possible.
>
>drm_mode_size_dumb() uses existing 4CC format helpers to interpret the
>given color mode. This makes the dumb-buffer interface behave similar
>the kernel's video= parameter. Again, current per-driver implementations
>likely have subtle differences or bugs in how they support color modes.
>
>Future directions: one bug is present in the current input validation
>in drm_mode_create_dumb(). The dumb-buffer overflow tests round up any
>given bits-per-pixel value to a multiple of 8. So even one-bit formats,
>such as DRM_FORMAT_C1, require 8 bits per pixel. While not common,
>low-end displays use such formats; with a possible overcommitment of
>memory. At some point, the validation logic in drm_mode_size_dumb() is
>supposed to replace the erronous code.
>
>Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>---
> drivers/gpu/drm/drm_dumb_buffers.c | 93 ++++++++++++++++++++++++++++++
> include/drm/drm_dumb_buffers.h | 14 +++++
> 2 files changed, 107 insertions(+)
> create mode 100644 include/drm/drm_dumb_buffers.h
>
>diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
>index 9916aaf5b3f2..fd39720bd617 100644
>--- a/drivers/gpu/drm/drm_dumb_buffers.c
>+++ b/drivers/gpu/drm/drm_dumb_buffers.c
>@@ -25,6 +25,8 @@
>
> #include <drm/drm_device.h>
> #include <drm/drm_drv.h>
>+#include <drm/drm_dumb_buffers.h>
>+#include <drm/drm_fourcc.h>
> #include <drm/drm_gem.h>
> #include <drm/drm_mode.h>
>
>@@ -57,6 +59,97 @@
> * a hardware-specific ioctl to allocate suitable buffer objects.
> */
>
>+static int drm_mode_align_dumb(struct drm_mode_create_dumb *args,
>+ unsigned long pitch_align,
>+ unsigned long size_align)
>+{
>+ u32 pitch = args->pitch;
>+ u32 size;
>+
>+ if (!pitch)
>+ return -EINVAL;
>+
>+ if (pitch_align)
>+ pitch = roundup(pitch, pitch_align);
>+
>+ /* overflow checks for 32bit size calculations */
>+ if (args->height > U32_MAX / pitch)
>+ return -EINVAL;
>+
>+ if (!size_align)
>+ size_align = PAGE_SIZE;
>+ else if (!IS_ALIGNED(size_align, PAGE_SIZE))
>+ return -EINVAL;
>+
>+ size = ALIGN(args->height * pitch, size_align);
>+ if (!size)
>+ return -EINVAL;
>+
>+ args->pitch = pitch;
>+ args->size = size;
>+
>+ return 0;
>+}
>+
>+/**
>+ * drm_mode_size_dumb - Calculates the scanline and buffer sizes for dumb buffers
>+ * @dev: DRM device
>+ * @args: Parameters for the dumb buffer
>+ * @pitch_align: Scanline alignment in bytes
>+ * @size_align: Buffer-size alignment in bytes
>+ *
>+ * The helper drm_mode_size_dumb() calculates the size of the buffer
>+ * allocation and the scanline size for a dumb buffer. Callers have to
>+ * set the buffers width, height and color mode in the argument @arg.
>+ * The helper validates the correctness of the input and tests for
>+ * possible overflows. If successful, it returns the dumb buffer's
>+ * required scanline pitch and size in &args.
>+ *
>+ * The parameter @pitch_align allows the driver to specifies an
>+ * alignment for the scanline pitch, if the hardware requires any. The
>+ * calculated pitch will be a multiple of the alignment. The parameter
>+ * @size_align allows to specify an alignment for buffer sizes. The
>+ * returned size is always a multiple of PAGE_SIZE.
>+ *
>+ * Returns:
>+ * Zero on success, or a negative error code otherwise.
>+ */
>+int drm_mode_size_dumb(struct drm_device *dev,
>+ struct drm_mode_create_dumb *args,
>+ unsigned long pitch_align,
>+ unsigned long size_align)
>+{
>+ u32 fourcc;
>+ const struct drm_format_info *info;
>+ u64 pitch;
>+
>+ /*
>+ * The scanline pitch depends on the buffer width and the color
>+ * format. The latter is specified as a color-mode constant for
>+ * which we first have to find the corresponding color format.
>+ *
>+ * Different color formats can have the same color-mode constant.
>+ * For example XRGB8888 and BGRX8888 both have a color mode of 32.
>+ * It is possible to use different formats for dumb-buffer allocation
>+ * and rendering as long as all involved formats share the same
>+ * color-mode constant.
>+ */
>+ fourcc = drm_driver_color_mode_format(dev, args->bpp);
This will return -EINVAL with bpp drm_mode_legacy_fb_format doesn't support,
such as(NV15, NV20, NV30, bpp is 10)[0]
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);
}
}
[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
More information about the Spice-devel
mailing list