Matthew Auld
2025-Jan-09 16:05 UTC
[PATCH v2 23/25] drm/xe: Compute dumb-buffer sizes with drm_mode_size_dumb()
On 09/01/2025 14:57, 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. Align the > buffer size according to hardware requirements. > > Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de> > Cc: Lucas De Marchi <lucas.demarchi at intel.com> > Cc: "Thomas Hellstr?m" <thomas.hellstrom at linux.intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com> > --- > drivers/gpu/drm/xe/xe_bo.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c > index e6c896ad5602..d75e3c39ab14 100644 > --- a/drivers/gpu/drm/xe/xe_bo.c > +++ b/drivers/gpu/drm/xe/xe_bo.c > @@ -8,6 +8,7 @@ > #include <linux/dma-buf.h> > > #include <drm/drm_drv.h> > +#include <drm/drm_dumb_buffers.h> > #include <drm/drm_gem_ttm_helper.h> > #include <drm/drm_managed.h> > #include <drm/ttm/ttm_device.h> > @@ -2535,14 +2536,13 @@ int xe_bo_dumb_create(struct drm_file *file_priv, > struct xe_device *xe = to_xe_device(dev); > struct xe_bo *bo; > uint32_t handle; > - int cpp = DIV_ROUND_UP(args->bpp, 8); > int err; > u32 page_size = max_t(u32, PAGE_SIZE, > xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K); > > - args->pitch = ALIGN(args->width * cpp, 64); > - args->size = ALIGN(mul_u32_u32(args->pitch, args->height), > - page_size); > + err = drm_mode_size_dumb(dev, args, SZ_64, page_size);AFAICT this looks to change the behaviour, where u64 size was technically possible and was allowed given that args->size is u64, but this helper is limiting the size to u32. Is that intentional? If so, we should probably make that clear in the commit message.> + if (err) > + return err; > > bo = xe_bo_create_user(xe, NULL, NULL, args->size, > DRM_XE_GEM_CPU_CACHING_WC,
Thomas Zimmermann
2025-Jan-09 16:26 UTC
[PATCH v2 23/25] drm/xe: Compute dumb-buffer sizes with drm_mode_size_dumb()
Hi Am 09.01.25 um 17:05 schrieb Matthew Auld:> On 09/01/2025 14:57, 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. Align the >> buffer size according to hardware requirements. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de> >> Cc: Lucas De Marchi <lucas.demarchi at intel.com> >> Cc: "Thomas Hellstr?m" <thomas.hellstrom at linux.intel.com> >> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com> >> --- >> ? drivers/gpu/drm/xe/xe_bo.c | 8 ++++---- >> ? 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c >> index e6c896ad5602..d75e3c39ab14 100644 >> --- a/drivers/gpu/drm/xe/xe_bo.c >> +++ b/drivers/gpu/drm/xe/xe_bo.c >> @@ -8,6 +8,7 @@ >> ? #include <linux/dma-buf.h> >> ? ? #include <drm/drm_drv.h> >> +#include <drm/drm_dumb_buffers.h> >> ? #include <drm/drm_gem_ttm_helper.h> >> ? #include <drm/drm_managed.h> >> ? #include <drm/ttm/ttm_device.h> >> @@ -2535,14 +2536,13 @@ int xe_bo_dumb_create(struct drm_file >> *file_priv, >> ????? struct xe_device *xe = to_xe_device(dev); >> ????? struct xe_bo *bo; >> ????? uint32_t handle; >> -??? int cpp = DIV_ROUND_UP(args->bpp, 8); >> ????? int err; >> ????? u32 page_size = max_t(u32, PAGE_SIZE, >> ????????? xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K ? SZ_64K : SZ_4K); >> ? -??? args->pitch = ALIGN(args->width * cpp, 64); >> -??? args->size = ALIGN(mul_u32_u32(args->pitch, args->height), >> -?????????????? page_size); >> +??? err = drm_mode_size_dumb(dev, args, SZ_64, page_size); > > AFAICT this looks to change the behaviour, where u64 size was > technically possible and was allowed given that args->size is u64, but > this helper is limiting the size to u32. Is that intentional? If so, > we should probably make that clear in the commit message.That's an interesting observation; thanks. The ioctl's internal checks have always limited the size to 32 bit. [1] I think it is not supposed to be larger than that. We can change the helper to support 64-bit sizes as well. Having said that, is there any use case? Dumb buffers are for software rendering only. Allocating more than a few dozen MiB seems like a mistake. Maybe we should rather limit the allowed allocation size instead? Best regards Thomas [1] https://elixir.bootlin.com/linux/v6.12.6/source/drivers/gpu/drm/drm_dumb_buffers.c#L82> >> +??? if (err) >> +??????? return err; >> ? ????? bo = xe_bo_create_user(xe, NULL, NULL, args->size, >> ???????????????????? DRM_XE_GEM_CPU_CACHING_WC, >-- -- 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)