On 11/25/25 16:03, Thomas Zimmermann wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know > the content is safe > > Hi > > Am 25.11.25 um 15:39 schrieb [email protected]: >> 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. :/
I was wondering if it was 8-bits or 8-bytes. > >>> >>> 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 <[email protected]> >>> Reviewed-by: Tomi Valkeinen <[email protected]> >>> --- >>> 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. Indeed, but overlay-type planes are often used to display windows that are not the same size as the screen, and therefore potentially not on 8-bytes aligned. > >> >> 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. Yes, that’s how I noticed this change. I have an application that requests a framebuffer larger than my screen, in this case 850x480 in RGB565 for an 800x480 display. So I have a pitch of 1700 bytes. Now it’s using a pitch of 1704, which shifts my lines. Regards, Ludovic > > 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) > >
