[PATCH v2 1/6] drm/client: Support unmapping of DRM client buffers

[Noralf Trønnes]

Den 05.07.2019 11.26, skrev Thomas Zimmermann:
> DRM clients, such as the fbdev emulation, have their buffer objects
> mapped by default. Mapping a buffer implicitly prevents its relocation.
> Hence, the buffer may permanently consume video memory while it's
> allocated. This is a problem for drivers of low-memory devices, such as
> ast, mgag200 or older framebuffer hardware, which will then not have
> enough memory to display other content (e.g., X11).
> 
> This patch introduces drm_client_buffer_vmap() and _vunmap(). Internal
> DRM clients can use these functions to unmap and remap buffer objects
> as needed.
> 
> There's no reference counting for vmap operations. Callers are expected
> to either keep buffers mapped (as it is now), or call vmap and vunmap
> in pairs around code that accesses the mapped memory.
> 
> v2:
> 	* remove several duplicated NULL-pointer checks
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
>  drivers/gpu/drm/drm_client.c | 67 ++++++++++++++++++++++++++++++------
>  include/drm/drm_client.h     |  3 ++
>  2 files changed, 60 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 410572f14257..66d8d645ac79 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -281,6 +281,43 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u
>  
>  	buffer->gem = obj;
>  
> +	vaddr = drm_client_buffer_vmap(buffer);
> +	if (IS_ERR(vaddr)) {
> +		ret = PTR_ERR(vaddr);
> +		goto err_delete;
> +	}
> +
> +	return buffer;
> +
> +err_delete:
> +	drm_client_buffer_delete(buffer);
> +
> +	return ERR_PTR(ret);
> +}
> +
> +/**
> + * drm_client_buffer_vmap - Map DRM client buffer into address space
> + * @buffer: DRM client buffer
> + *
> + * This function maps a client buffer into kernel address space. If the
> + * buffer is already mapped, it returns the mapping's address.
> + *
> + * Client buffer mappings are not ref'counted. Each call to
> + * drm_client_buffer_vmap() should be followed by a call to
> + * drm_client_buffer_vunmap(); or the client buffer should be mapped
> + * throughout its lifetime. The latter is the default.
> + *
> + * Returns:
> + *	The mapped memory's address
> + */
> +void *
> +drm_client_buffer_vmap(struct drm_client_buffer *buffer)

I prefer to have this on one line.

> +{
> +	void *vaddr;
> +
> +	if (buffer->vaddr)
> +		return buffer->vaddr;
> +
>  	/*
>  	 * FIXME: The dependency on GEM here isn't required, we could
>  	 * convert the driver handle to a dma-buf instead and use the
> @@ -289,21 +326,31 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u
>  	 * fd_install step out of the driver backend hooks, to make that
>  	 * final step optional for internal users.
>  	 */
> -	vaddr = drm_gem_vmap(obj);
> -	if (IS_ERR(vaddr)) {
> -		ret = PTR_ERR(vaddr);
> -		goto err_delete;
> -	}
> +	vaddr = drm_gem_vmap(buffer->gem);
> +	if (IS_ERR(vaddr))
> +		return vaddr;
>  
>  	buffer->vaddr = vaddr;
>  
> -	return buffer;
> -
> -err_delete:
> -	drm_client_buffer_delete(buffer);
> +	return vaddr;
> +}
> +EXPORT_SYMBOL(drm_client_buffer_vmap);
>  
> -	return ERR_PTR(ret);
> +/**
> + * drm_client_buffer_vunmap - Unmap DRM client buffer
> + * @buffer: DRM client buffer
> + *
> + * This function removes a client buffer's memory mmapping. This

s/mmapping/mapping/

> + * function is only required by clients that manage their buffers
> + * by themselves. By default, DRM client buffers are mapped throughout
> + * their entire lifetime.
> + */
> +void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> +{
> +	drm_gem_vunmap(buffer->gem, buffer->vaddr);
> +	buffer->vaddr = NULL;
>  }
> +EXPORT_SYMBOL(drm_client_buffer_vunmap);
>  
>  static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer)
>  {
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 72d51d1e9dd9..e1db1d9da0bf 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -149,6 +149,9 @@ struct drm_client_buffer {
>  struct drm_client_buffer *
>  drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format);
>  void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
> +void *
> +drm_client_buffer_vmap(struct drm_client_buffer *buffer);

Prefer to have this on one line.

Reviewed-by: Noralf Trønnes <noralf at tronnes.org>

> +void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
>  
>  int drm_client_modeset_create(struct drm_client_dev *client);
>  void drm_client_modeset_free(struct drm_client_dev *client);
>