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

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 25.11.25 um 15:39 schrieb Ludovic.Desroches@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@suse.de>
>> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@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)