Thomas Zimmermann
2025-Nov-25 15:03 UTC
[PATCH v6 03/25] drm/gem-dma: Compute dumb-buffer sizes with drm_mode_size_dumb()
Hi Am 25.11.25 um 15:39 schrieb Ludovic.Desroches at microchip.com:> On 8/21/25 10:17, 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.I missed a chance to explain it here. :/>> >> 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. >> >> v5: >> - avoid reset of arguments (Tomi) >> >> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de> >> Reviewed-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com> >> --- >> drivers/gpu/drm/drm_gem_dma_helper.c | 7 +++++-- >> drivers/gpu/drm/imx/ipuv3/imx-drm-core.c | 4 +++- >> 2 files changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c b/drivers/gpu/drm/drm_gem_dma_helper.c >> index 4f0320df858f..ab1a70b1d6f1 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; > Hi, > > Was it intentional for this change to alter the pitch?Most hardware does not need the pitch to be of a certain alignment. But these buffers are possibly shared with other hardware, which sometimes needs alignment to certain values. Using SZ_8 improves compatible with that hardware.> > The alignment requirement has been updated?from 8-bit alignment to > 64-bit alignment. Since the pitch is expressed in bytes, we should pass > SZ_1 instead of SZ_8 for hw_patch_align. > > For example, for an 850?480 framebuffer at 16 bpp, the pitch should be > 1700 bytes. With the new alignment, the pitch becomes 1704 bytes.Many display modes have an 8-byte alignment in their width. 850 pixels is somewhat of an exception.> > Please let me know if you?d like me to submit a fix.Do you see a bug with your hardware? Unless this creates a real problem, I'd like to keep it as it is now. Best regards Thomas> > Regards, > Ludovic > > >> >> 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..af4a30311e18 100644 >> --- a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c >> +++ b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c >> @@ -145,8 +145,10 @@ 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); >> + ret = drm_gem_dma_dumb_create_internal(file_priv, drm, args); >> if (ret) >> return ret; >>-- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstr. 146, 90461 N?rnberg, Germany, www.suse.com GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG N?rnberg)