Re: [PATCH v4 03/25] drm/gem-dma: Compute dumb-buffer sizes with drm_mode_size_dumb()

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Hi

Am 12.06.25 um 10:43 schrieb Tomi Valkeinen:
> Hi,
>
> On 11/03/2025 17:47, Thomas Zimmermann wrote:
> > Call drm_mode_size_dumb() to compute dumb-buffer scanline pitch and
> > buffer size. Align the pitch to a multiple of 8.
> >
> > Push the current calculation into the only direct caller imx. Imx's
> > hardware requires the framebuffer width to be aligned to 8. The
> > driver's current approach is actually incorrect, as it only guarantees
> > this implicitly and requires bpp to be a multiple of 8 already. A
> > later commit will fix this problem by aligning the scanline pitch
> > such that an aligned width still fits into each scanline's memory.
> >
> > A number of other drivers are build on top of gem-dma helpers and
> > implement their own dumb-buffer allocation. These drivers invoke
> > drm_gem_dma_dumb_create_internal(), which is not affected by this
> > commit.
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > ---
> >   drivers/gpu/drm/drm_gem_dma_helper.c     | 7 +++++--
> >   drivers/gpu/drm/imx/ipuv3/imx-drm-core.c | 2 ++
> >   2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c 
> > b/drivers/gpu/drm/drm_gem_dma_helper.c
> > index b7f033d4352a..49be9b033610 100644
> > --- a/drivers/gpu/drm/drm_gem_dma_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_dma_helper.c
> > @@ -20,6 +20,7 @@
> >   #include <drm/drm.h>
> >   #include <drm/drm_device.h>
> >   #include <drm/drm_drv.h>
> > +#include <drm/drm_dumb_buffers.h>
> >   #include <drm/drm_gem_dma_helper.h>
> >   #include <drm/drm_vma_manager.h>
> >   
> > @@ -304,9 +305,11 @@ int drm_gem_dma_dumb_create(struct drm_file *file_priv,
> >                             struct drm_mode_create_dumb *args)
> >   {
> >         struct drm_gem_dma_object *dma_obj;
> > +       int ret;
> >   
> > -	args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> > -       args->size = args->pitch * args->height;
> > +       ret = drm_mode_size_dumb(drm, args, SZ_8, 0);
> > +       if (ret)
> > +               return ret;
> >   
> >   	dma_obj = drm_gem_dma_create_with_handle(file_priv, drm, args->size,
> >                                                  &args->handle);
> > diff --git a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c 
> > b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
> > index ec5fd9a01f1e..e7025df7b978 100644
> > --- a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
> > +++ b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
> > @@ -145,6 +145,8 @@ static int imx_drm_dumb_create(struct drm_file *file_priv,
> >         int ret;
> >   
> >   	args->width = ALIGN(width, 8);
> > +       args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> > +       args->size = args->pitch * args->height;
> >   
> >   	ret = drm_gem_dma_dumb_create(file_priv, drm, args);
> >         if (ret)
> Won't the pitch and size just be overwritten by the
> drm_gem_dma_dumb_create() call?
Right, looks like it. Thanks for looking over this. The call to 
drm_gem_dma_dumb_create() needs to be changed 
drm_gem_dma_dumb_create_internal(). The latter doesn't modify the 
arguments besides some sanity checks. BTW patch 10 cleans up imx 
entirely. Best regards Thomas
>   Tomi
--
--
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)