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@xxxxxxx>
Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Cc: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxx/
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
