Thomas Zimmermann
2025-Jan-09  14:57 UTC
[PATCH v2 25/25] drm/xlnx: Compute dumb-buffer sizes with drm_mode_size_dumb()
Call drm_mode_size_dumb() to compute dumb-buffer scanline pitch and
buffer size. Align the pitch according to hardware requirements.
Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
---
 drivers/gpu/drm/xlnx/zynqmp_kms.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c
b/drivers/gpu/drm/xlnx/zynqmp_kms.c
index b47463473472..7ea0cd4f71d3 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
@@ -19,6 +19,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_dumb_buffers.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_fbdev_dma.h>
 #include <drm/drm_fourcc.h>
@@ -363,10 +364,12 @@ static int zynqmp_dpsub_dumb_create(struct drm_file
*file_priv,
 				    struct drm_mode_create_dumb *args)
 {
 	struct zynqmp_dpsub *dpsub = to_zynqmp_dpsub(drm);
-	unsigned int pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
+	int ret;
 
 	/* Enforce the alignment constraints of the DMA engine. */
-	args->pitch = ALIGN(pitch, dpsub->dma_align);
+	ret = drm_mode_size_dumb(drm, args, dpsub->dma_align, 0);
+	if (ret)
+		return ret;
 
 	return drm_gem_dma_dumb_create_internal(file_priv, drm, args);
 }
-- 
2.47.1
Tomi Valkeinen
2025-Jan-15  10:13 UTC
[PATCH v2 25/25] drm/xlnx: Compute dumb-buffer sizes with drm_mode_size_dumb()
Hi! On 09/01/2025 16:57, Thomas Zimmermann wrote:> Call drm_mode_size_dumb() to compute dumb-buffer scanline pitch and > buffer size. Align the pitch according to hardware requirements. > > Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de> > Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com> > Cc: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com> > --- > drivers/gpu/drm/xlnx/zynqmp_kms.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c > index b47463473472..7ea0cd4f71d3 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c > @@ -19,6 +19,7 @@ > #include <drm/drm_crtc.h> > #include <drm/drm_device.h> > #include <drm/drm_drv.h> > +#include <drm/drm_dumb_buffers.h> > #include <drm/drm_encoder.h> > #include <drm/drm_fbdev_dma.h> > #include <drm/drm_fourcc.h> > @@ -363,10 +364,12 @@ static int zynqmp_dpsub_dumb_create(struct drm_file *file_priv, > struct drm_mode_create_dumb *args) > { > struct zynqmp_dpsub *dpsub = to_zynqmp_dpsub(drm); > - unsigned int pitch = DIV_ROUND_UP(args->width * args->bpp, 8); > + int ret; > > /* Enforce the alignment constraints of the DMA engine. */ > - args->pitch = ALIGN(pitch, dpsub->dma_align); > + ret = drm_mode_size_dumb(drm, args, dpsub->dma_align, 0); > + if (ret) > + return ret; > > return drm_gem_dma_dumb_create_internal(file_priv, drm, args); > }I have some trouble with this one. I have sent a series to add some pixel formats: https://lore.kernel.org/all/20250115-xilinx-formats-v2-0-160327ca652a at ideasonboard.com/ Let's look at XV15. It's similar to NV12, but 10 bits per component, and some packing and padding. First plane: 3 pixels in a 32 bit group Second plane: 3 pixels in a 64 bit group, 2x2 subsampled So, on average, a pixel on the first plane takes 32 / 3 = 10.666... bits on a line. That's not a usable number for the DRM_IOCTL_MODE_CREATE_DUMB ioctl. What I did was to use the pixel group size as "bpp" for DRM_IOCTL_MODE_CREATE_DUMB. So, e.g., for 720 x 576: Stride for first plane: 720 * (32 / 3) / 8 = 960 bytes Stride for second plane: 720 / 2 * (64 / 3) / 8 = 960 bytes First plane: 720 / 3 = 240 pixel groups Second plane: 720 / 2 / 3 = 120 pixel groups So I allocated the two planes with: 240 x 576 with 32 bitspp 120 x 288 with 64 bitspp This worked, and if I look at the DRM_IOCTL_MODE_CREATE_DUMB in the docs, I can't right away see anything there that says my tactic was not allowed. The above doesn't work anymore with this patch, as the code calls drm_driver_color_mode_format(), which fails for 64 bitspp. It feels a bit odd that DRM_IOCTL_MODE_CREATE_DUMB will try to guess the RGB fourcc for a dumb buffer allocation. So, what to do here? Am I doing something silly? What's the correct way to allocate the buffers for XV15? Should I just use 32 bitspp for the plane 2 too, and double the width (this works)? Is DRM_IOCTL_MODE_CREATE_DUMB only meant for simple RGB formats? The xilinx driver can, of course, just not use drm_mode_size_dumb(). But if so, I guess the limitations of drm_mode_size_dumb() should be documented. Do we need a new dumb-alloc ioctl that takes the format and plane number as parameters? Or alternatively a simpler dumb-alloc that doesn't have width and bpp, but instead takes a stride and height as parameters? I think those would be easier for the userspace to use, instead of trying to adjust the parameters to be suitable for the kernel. Tomi
Reasonably Related Threads
- [PATCH v2 25/25] drm/xlnx: Compute dumb-buffer sizes with drm_mode_size_dumb()
- [PATCH v2 25/25] drm/xlnx: Compute dumb-buffer sizes with drm_mode_size_dumb()
- [PATCH v2 25/25] drm/xlnx: Compute dumb-buffer sizes with drm_mode_size_dumb()
- [PATCH v2 00/25] drm/dumb-buffers: Fix and improve buffer-size calculation
- [PATCH v2 20/21] drm/xlnx: Initialize DRM driver instance with CMA helper macro