Re: [PATCH 1/4] drm/client: Do not pin in drm_client_buffer_vmap()

[Boris Brezillon <boris.brezillon@collabora.com>]

On Fri,  4 Apr 2025 15:23:31 +0200
Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Pin and vmap are two distict operations. Do not mix them.
> 
> The helper drm_client_buffer_vmap() maps the pages for fbdev-dma
> and fbdev-shmem. In both cases, the vmap operation ensures that
> the pages are available until the vunmap happens. And as the pages
> in DMA or SHMEM areas cannot be moved, there is no reason to call
> pin. Hence remove the pin call.
> 
> Update drm_client_buffer_vunmap() accordingly.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/gpu/drm/drm_client.c | 22 +++++-----------------
>  1 file changed, 5 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index f1de7faf9fb45..154693066a127 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -303,34 +303,23 @@ EXPORT_SYMBOL(drm_client_buffer_vunmap_local);
>   * Returns:
>   *	0 on success, or a negative errno code otherwise.
>   */
> -int
> -drm_client_buffer_vmap(struct drm_client_buffer *buffer,
> -		       struct iosys_map *map_copy)
> +int drm_client_buffer_vmap(struct drm_client_buffer *buffer,
> +			   struct iosys_map *map_copy)
>  {
>  	struct drm_gem_object *gem = buffer->gem;
>  	struct iosys_map *map = &buffer->map;
>  	int ret;
>  
>  	drm_gem_lock(gem);
> -
> -	ret = drm_gem_pin_locked(gem);
> -	if (ret)
> -		goto err_drm_gem_pin_locked;
>  	ret = drm_gem_vmap_locked(gem, map);
> -	if (ret)
> -		goto err_drm_gem_vmap;
> -
>  	drm_gem_unlock(gem);
>  
> +	if (ret)
> +		return ret;
> +
>  	*map_copy = *map;
>  
>  	return 0;
> -
> -err_drm_gem_vmap:
> -	drm_gem_unpin_locked(buffer->gem);
> -err_drm_gem_pin_locked:
> -	drm_gem_unlock(gem);
> -	return ret;
>  }
>  EXPORT_SYMBOL(drm_client_buffer_vmap);
>  
> @@ -349,7 +338,6 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
>  
>  	drm_gem_lock(gem);
>  	drm_gem_vunmap_locked(gem, map);
> -	drm_gem_unpin_locked(gem);
>  	drm_gem_unlock(gem);
>  }
>  EXPORT_SYMBOL(drm_client_buffer_vunmap);