Re: [PATCH v3 1/3] drm: Introduce DRM_MODE_DUMB_KERNEL_MAP flag

[Thomas Zimmermann]

Hi

Am 26.03.26 um 11:01 schrieb Chen-Yu Tsai:
> From: Rob Herring <robh@xxxxxxxxxx>
>
> Introduce a new flag, DRM_MODE_DUMB_KERNEL_MAP, for struct
> drm_mode_create_dumb. This flag is for internal kernel use to indicate
> if dumb buffer allocation needs a kernel mapping. This is needed only for
> GEM DMA where creating a kernel mapping or not has to be decided at
> allocation time because creating a mapping on demand (with vmap()) is not
> guaranteed to work.

I still don't understand what you're trying to achieve. As I pointed 
out, ever driver's memory manager potentially requires a vmap. Passing 
around flags will not solve that problem. If vmap is not possible, the 
driver should not provide the vmap callbacks in the first place.

Best regards
Thomas

> Several drivers are using reimplementing the GEM DMA helpers because
> they distinguish between kernel and userspace allocations to create a
> kernel mapping or not. Adding a flag allows migrating these drivers
> to the helpers while preserving their existing behavior. These include
> exynos, rockchip, and previously mediatek.
>
> Update the callers of drm_mode_dumb_create() to set
> drm_mode_dumb_create.flags to appropriate defaults. Currently, flags can
> be set to anything by userspace, but is unused within the kernel. Let's
> force flags to zero (no kernel mapping) for userspace callers by default.
> For in kernel clients, set DRM_MODE_DUMB_KERNEL_MAP by default. Drivers
> can override this as needed.
>
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> [wenst@xxxxxxxxxxxx: Emit warning (once) if args->flags is not zero]
> [wenst@xxxxxxxxxxxx: Moved flag def. to include/drm/drm_dumb_buffers.h]
> Signed-off-by: Chen-Yu Tsai <wenst@xxxxxxxxxxxx>
> ---
> Changes since v2:
> - Switched to drm_warn_once()
> - Moved flag definition from include/uapi/ to include/drm/drm_dumb_buffers.h
> - Reworded commit message
>
> Changes since v1:
> - Emit warning if args->flags is not zero
> ---
>   drivers/gpu/drm/drm_client.c       | 2 ++
>   drivers/gpu/drm/drm_dumb_buffers.c | 4 ++++
>   include/drm/drm_dumb_buffers.h     | 3 +++
>   3 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 46c465bce98c..3d3e61823cc1 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -14,6 +14,7 @@
>   #include <drm/drm_client_event.h>
>   #include <drm/drm_device.h>
>   #include <drm/drm_drv.h>
> +#include <drm/drm_dumb_buffers.h>
>   #include <drm/drm_file.h>
>   #include <drm/drm_fourcc.h>
>   #include <drm/drm_framebuffer.h>
> @@ -404,6 +405,7 @@ drm_client_buffer_create_dumb(struct drm_client_dev *client, u32 width, u32 heig
>   	dumb_args.width = width;
>   	dumb_args.height = height;
>   	dumb_args.bpp = drm_format_info_bpp(info, 0);
> +	dumb_args.flags = DRM_MODE_DUMB_KERNEL_MAP;
>   	ret = drm_mode_create_dumb(dev, &dumb_args, client->file);
>   	if (ret)
>   		return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index e2b62e5fb891..60f4c2d08641 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -233,6 +233,10 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev,
>   	struct drm_mode_create_dumb *args = data;
>   	int err;
>   
> +	if (args->flags)
> +		drm_warn_once(dev, "drm_mode_create_dumb.flags is not zero.\n");
> +	args->flags = 0;
> +
>   	err = drm_mode_create_dumb(dev, args, file_priv);
>   	if (err) {
>   		args->handle = 0;
> diff --git a/include/drm/drm_dumb_buffers.h b/include/drm/drm_dumb_buffers.h
> index 1f3a8236fb3d..4657e44533f4 100644
> --- a/include/drm/drm_dumb_buffers.h
> +++ b/include/drm/drm_dumb_buffers.h
> @@ -6,6 +6,9 @@
>   struct drm_device;
>   struct drm_mode_create_dumb;
>   
> +/* drm_mode_create_dumb flags for internal use */
> +#define DRM_MODE_DUMB_KERNEL_MAP	(1<<0)
> +
>   int drm_mode_size_dumb(struct drm_device *dev,
>   		       struct drm_mode_create_dumb *args,
>   		       unsigned long hw_pitch_align,

--
--
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)