* Re: [PATCH] drm/gem-shmem: Do not map s/g table by default
2025-06-30 14:34 [PATCH] drm/gem-shmem: Do not map s/g table by default Thomas Zimmermann
@ 2025-07-01 16:20 ` Zenghui Yu
2025-07-02 7:10 ` Christian König
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Zenghui Yu @ 2025-07-01 16:20 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: christian.koenig, oushixiong, louis.chauvet, hamohammed.sa,
simona, melissa.srw, airlied, jfalempe, maarten.lankhorst,
mripard, sean, dri-devel, José Expósito
On 2025/6/30 22:34, Thomas Zimmermann wrote:
> The vast majority of drivers that use GEM-SHMEM helpers do not use
> an s/g table for imported buffers; specifically all drivers that use
> DRM_GEM_SHMEM_DRIVER_OPS. Therefore convert the initializer macro
> to DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT and remove the latter. This
> helps to avoid swiotbl errors, such as seen with some Aspeed systems
>
> ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots)
>
> The error is caused by the system's limited DMA capabilities and can
> happen with any GEM-SHMEM-based driver. It results in a performance
> penalty.
>
> In the case of vgem and vkms, the devices do not support DMA at all,
> which can result in failure to map the buffer object into the kernel's
> address space. [1][2] Avoiding the s/g table fixes this problem.
>
> The other drivers based on GEM-SHMEM, imagination, lima, panfrost,
> panthor, v3d and virtio, use the s/g table of imported buffers. Neither
> driver uses the default initializer, so they won't be affected by
> this change.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reported-by: Zenghui Yu <zenghui.yu@linux.dev>
> Closes: https://lore.kernel.org/dri-devel/6d22bce3-4533-4cfa-96ba-64352b715741@linux.dev/ # [1]
> Reported-by: José Expósito <jose.exposito89@gmail.com>
> Closes: https://lore.kernel.org/dri-devel/20250311172054.2903-1-jose.exposito89@gmail.com/ # [2]
Tested-by: Zenghui Yu <zenghui.yu@linux.dev>
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/gem-shmem: Do not map s/g table by default
2025-06-30 14:34 [PATCH] drm/gem-shmem: Do not map s/g table by default Thomas Zimmermann
2025-07-01 16:20 ` Zenghui Yu
@ 2025-07-02 7:10 ` Christian König
2025-07-03 8:40 ` Louis Chauvet
2025-07-07 13:50 ` Thomas Zimmermann
3 siblings, 0 replies; 5+ messages in thread
From: Christian König @ 2025-07-02 7:10 UTC (permalink / raw)
To: Thomas Zimmermann, oushixiong, louis.chauvet, zenghui.yu,
hamohammed.sa, simona, melissa.srw, airlied, jfalempe,
maarten.lankhorst, mripard, sean
Cc: dri-devel, José Expósito
On 30.06.25 16:34, Thomas Zimmermann wrote:
> The vast majority of drivers that use GEM-SHMEM helpers do not use
> an s/g table for imported buffers; specifically all drivers that use
> DRM_GEM_SHMEM_DRIVER_OPS. Therefore convert the initializer macro
> to DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT and remove the latter. This
> helps to avoid swiotbl errors, such as seen with some Aspeed systems
>
> ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots)
>
> The error is caused by the system's limited DMA capabilities and can
> happen with any GEM-SHMEM-based driver. It results in a performance
> penalty.
>
> In the case of vgem and vkms, the devices do not support DMA at all,
> which can result in failure to map the buffer object into the kernel's
> address space. [1][2] Avoiding the s/g table fixes this problem.
>
> The other drivers based on GEM-SHMEM, imagination, lima, panfrost,
> panthor, v3d and virtio, use the s/g table of imported buffers. Neither
> driver uses the default initializer, so they won't be affected by
> this change.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reported-by: Zenghui Yu <zenghui.yu@linux.dev>
> Closes: https://lore.kernel.org/dri-devel/6d22bce3-4533-4cfa-96ba-64352b715741@linux.dev/ # [1]
> Reported-by: José Expósito <jose.exposito89@gmail.com>
> Closes: https://lore.kernel.org/dri-devel/20250311172054.2903-1-jose.exposito89@gmail.com/ # [2]
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/ast/ast_drv.c | 2 +-
> drivers/gpu/drm/udl/udl_drv.c | 2 +-
> include/drm/drm_gem_shmem_helper.h | 18 +++---------------
> 3 files changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 054acda41909..6fbf62a99c48 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -64,7 +64,7 @@ static const struct drm_driver ast_driver = {
> .minor = DRIVER_MINOR,
> .patchlevel = DRIVER_PATCHLEVEL,
>
> - DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT,
> + DRM_GEM_SHMEM_DRIVER_OPS,
> DRM_FBDEV_SHMEM_DRIVER_OPS,
> };
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index ce5ae7cacb90..1922988625eb 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -57,7 +57,7 @@ static const struct drm_driver driver = {
>
> /* GEM hooks */
> .fops = &udl_driver_fops,
> - DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT,
> + DRM_GEM_SHMEM_DRIVER_OPS,
> DRM_FBDEV_SHMEM_DRIVER_OPS,
>
> .name = DRIVER_NAME,
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index 35f7466dca84..92f5db84b9c2 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -293,23 +293,11 @@ struct drm_gem_object *drm_gem_shmem_prime_import_no_map(struct drm_device *dev,
> /**
> * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations
> *
> - * This macro provides a shortcut for setting the shmem GEM operations in
> - * the &drm_driver structure.
> + * This macro provides a shortcut for setting the shmem GEM operations
> + * in the &drm_driver structure. Drivers that do not require an s/g table
> + * for imported buffers should use this.
> */
> #define DRM_GEM_SHMEM_DRIVER_OPS \
> - .gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \
> - .dumb_create = drm_gem_shmem_dumb_create
> -
> -/**
> - * DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT - shmem GEM operations
> - * without mapping sg_table on
> - * imported buffer.
> - *
> - * This macro provides a shortcut for setting the shmem GEM operations in
> - * the &drm_driver structure for drivers that do not require a sg_table on
> - * imported buffers.
> - */
> -#define DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT \
> .gem_prime_import = drm_gem_shmem_prime_import_no_map, \
> .dumb_create = drm_gem_shmem_dumb_create
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/gem-shmem: Do not map s/g table by default
2025-06-30 14:34 [PATCH] drm/gem-shmem: Do not map s/g table by default Thomas Zimmermann
2025-07-01 16:20 ` Zenghui Yu
2025-07-02 7:10 ` Christian König
@ 2025-07-03 8:40 ` Louis Chauvet
2025-07-07 13:50 ` Thomas Zimmermann
3 siblings, 0 replies; 5+ messages in thread
From: Louis Chauvet @ 2025-07-03 8:40 UTC (permalink / raw)
To: Thomas Zimmermann, christian.koenig, oushixiong, zenghui.yu,
hamohammed.sa, simona, melissa.srw, airlied, jfalempe,
maarten.lankhorst, mripard, sean
Cc: dri-devel, José Expósito
Le 30/06/2025 à 16:34, Thomas Zimmermann a écrit :
> The vast majority of drivers that use GEM-SHMEM helpers do not use
> an s/g table for imported buffers; specifically all drivers that use
> DRM_GEM_SHMEM_DRIVER_OPS. Therefore convert the initializer macro
> to DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT and remove the latter. This
> helps to avoid swiotbl errors, such as seen with some Aspeed systems
>
> ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots)
>
> The error is caused by the system's limited DMA capabilities and can
> happen with any GEM-SHMEM-based driver. It results in a performance
> penalty.
>
> In the case of vgem and vkms, the devices do not support DMA at all,
> which can result in failure to map the buffer object into the kernel's
> address space. [1][2] Avoiding the s/g table fixes this problem.
>
> The other drivers based on GEM-SHMEM, imagination, lima, panfrost,
> panthor, v3d and virtio, use the s/g table of imported buffers. Neither
> driver uses the default initializer, so they won't be affected by
> this change.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
> Reported-by: Zenghui Yu <zenghui.yu@linux.dev>
> Closes: https://lore.kernel.org/dri-devel/6d22bce3-4533-4cfa-96ba-64352b715741@linux.dev/ # [1]
> Reported-by: José Expósito <jose.exposito89@gmail.com>
> Closes: https://lore.kernel.org/dri-devel/20250311172054.2903-1-jose.exposito89@gmail.com/ # [2]
> ---
> drivers/gpu/drm/ast/ast_drv.c | 2 +-
> drivers/gpu/drm/udl/udl_drv.c | 2 +-
> include/drm/drm_gem_shmem_helper.h | 18 +++---------------
> 3 files changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 054acda41909..6fbf62a99c48 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -64,7 +64,7 @@ static const struct drm_driver ast_driver = {
> .minor = DRIVER_MINOR,
> .patchlevel = DRIVER_PATCHLEVEL,
>
> - DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT,
> + DRM_GEM_SHMEM_DRIVER_OPS,
> DRM_FBDEV_SHMEM_DRIVER_OPS,
> };
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index ce5ae7cacb90..1922988625eb 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -57,7 +57,7 @@ static const struct drm_driver driver = {
>
> /* GEM hooks */
> .fops = &udl_driver_fops,
> - DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT,
> + DRM_GEM_SHMEM_DRIVER_OPS,
> DRM_FBDEV_SHMEM_DRIVER_OPS,
>
> .name = DRIVER_NAME,
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index 35f7466dca84..92f5db84b9c2 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -293,23 +293,11 @@ struct drm_gem_object *drm_gem_shmem_prime_import_no_map(struct drm_device *dev,
> /**
> * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations
> *
> - * This macro provides a shortcut for setting the shmem GEM operations in
> - * the &drm_driver structure.
> + * This macro provides a shortcut for setting the shmem GEM operations
> + * in the &drm_driver structure. Drivers that do not require an s/g table
> + * for imported buffers should use this.
> */
> #define DRM_GEM_SHMEM_DRIVER_OPS \
> - .gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \
> - .dumb_create = drm_gem_shmem_dumb_create
> -
> -/**
> - * DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT - shmem GEM operations
> - * without mapping sg_table on
> - * imported buffer.
> - *
> - * This macro provides a shortcut for setting the shmem GEM operations in
> - * the &drm_driver structure for drivers that do not require a sg_table on
> - * imported buffers.
> - */
> -#define DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT \
> .gem_prime_import = drm_gem_shmem_prime_import_no_map, \
> .dumb_create = drm_gem_shmem_dumb_create
>
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/gem-shmem: Do not map s/g table by default
2025-06-30 14:34 [PATCH] drm/gem-shmem: Do not map s/g table by default Thomas Zimmermann
` (2 preceding siblings ...)
2025-07-03 8:40 ` Louis Chauvet
@ 2025-07-07 13:50 ` Thomas Zimmermann
3 siblings, 0 replies; 5+ messages in thread
From: Thomas Zimmermann @ 2025-07-07 13:50 UTC (permalink / raw)
To: christian.koenig, oushixiong, louis.chauvet, zenghui.yu,
hamohammed.sa, simona, melissa.srw, airlied, jfalempe,
maarten.lankhorst, mripard, sean
Cc: dri-devel, José Expósito
Merged
Am 30.06.25 um 16:34 schrieb Thomas Zimmermann:
> The vast majority of drivers that use GEM-SHMEM helpers do not use
> an s/g table for imported buffers; specifically all drivers that use
> DRM_GEM_SHMEM_DRIVER_OPS. Therefore convert the initializer macro
> to DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT and remove the latter. This
> helps to avoid swiotbl errors, such as seen with some Aspeed systems
>
> ast 0000:07:00.0: swiotlb buffer is full (sz: 3145728 bytes), total 32768 (slots), used 0 (slots)
>
> The error is caused by the system's limited DMA capabilities and can
> happen with any GEM-SHMEM-based driver. It results in a performance
> penalty.
>
> In the case of vgem and vkms, the devices do not support DMA at all,
> which can result in failure to map the buffer object into the kernel's
> address space. [1][2] Avoiding the s/g table fixes this problem.
>
> The other drivers based on GEM-SHMEM, imagination, lima, panfrost,
> panthor, v3d and virtio, use the s/g table of imported buffers. Neither
> driver uses the default initializer, so they won't be affected by
> this change.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reported-by: Zenghui Yu <zenghui.yu@linux.dev>
> Closes: https://lore.kernel.org/dri-devel/6d22bce3-4533-4cfa-96ba-64352b715741@linux.dev/ # [1]
> Reported-by: José Expósito <jose.exposito89@gmail.com>
> Closes: https://lore.kernel.org/dri-devel/20250311172054.2903-1-jose.exposito89@gmail.com/ # [2]
> ---
> drivers/gpu/drm/ast/ast_drv.c | 2 +-
> drivers/gpu/drm/udl/udl_drv.c | 2 +-
> include/drm/drm_gem_shmem_helper.h | 18 +++---------------
> 3 files changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 054acda41909..6fbf62a99c48 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -64,7 +64,7 @@ static const struct drm_driver ast_driver = {
> .minor = DRIVER_MINOR,
> .patchlevel = DRIVER_PATCHLEVEL,
>
> - DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT,
> + DRM_GEM_SHMEM_DRIVER_OPS,
> DRM_FBDEV_SHMEM_DRIVER_OPS,
> };
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index ce5ae7cacb90..1922988625eb 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -57,7 +57,7 @@ static const struct drm_driver driver = {
>
> /* GEM hooks */
> .fops = &udl_driver_fops,
> - DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT,
> + DRM_GEM_SHMEM_DRIVER_OPS,
> DRM_FBDEV_SHMEM_DRIVER_OPS,
>
> .name = DRIVER_NAME,
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index 35f7466dca84..92f5db84b9c2 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -293,23 +293,11 @@ struct drm_gem_object *drm_gem_shmem_prime_import_no_map(struct drm_device *dev,
> /**
> * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations
> *
> - * This macro provides a shortcut for setting the shmem GEM operations in
> - * the &drm_driver structure.
> + * This macro provides a shortcut for setting the shmem GEM operations
> + * in the &drm_driver structure. Drivers that do not require an s/g table
> + * for imported buffers should use this.
> */
> #define DRM_GEM_SHMEM_DRIVER_OPS \
> - .gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \
> - .dumb_create = drm_gem_shmem_dumb_create
> -
> -/**
> - * DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT - shmem GEM operations
> - * without mapping sg_table on
> - * imported buffer.
> - *
> - * This macro provides a shortcut for setting the shmem GEM operations in
> - * the &drm_driver structure for drivers that do not require a sg_table on
> - * imported buffers.
> - */
> -#define DRM_GEM_SHMEM_DRIVER_OPS_NO_MAP_SGT \
> .gem_prime_import = drm_gem_shmem_prime_import_no_map, \
> .dumb_create = drm_gem_shmem_dumb_create
>
--
--
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)
^ permalink raw reply [flat|nested] 5+ messages in thread