Re: [PATCH 1/3] drm/amdgpu: Refuse to create a KMS FB for non-P2P exported dma-bufs

[Christian König <christian.koenig@xxxxxxx>]

Am 22.02.24 um 18:28 schrieb Michel Dänzer:
> From: Michel Dänzer <mdaenzer@xxxxxxxxxx>
>
> Pinning the BO storage to VRAM for scanout would make it inaccessible
> to non-P2P dma-buf importers.

Thinking more about it I don't think we can do this.

Using the BO in a ping/pong fashion for scanout and DMA-buf is actually 
valid, you just can't do both at the same time.

And if I'm not completely mistaken we actually have use cases for this 
at the moment, only as fallback but it would still break existing 
userspace and that is a no-go.

So rejecting things during CS and atomic commit is the best thing we can do.

Regards,
Christian.

> Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/10635
> Signed-off-by: Michel Dänzer <mdaenzer@xxxxxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 38 ++++++++++++++++++---
>   1 file changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index b8fbe97efe1d3..514a5b2159815 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -1255,13 +1255,41 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
>   		return ERR_PTR(-ENOENT);
>   	}
>   
> -	/* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */
>   	bo = gem_to_amdgpu_bo(obj);
>   	domains = amdgpu_display_supported_domains(drm_to_adev(dev), bo->flags);
> -	if (obj->import_attach && !(domains & AMDGPU_GEM_DOMAIN_GTT)) {
> -		drm_dbg_kms(dev, "Cannot create framebuffer from imported dma_buf\n");
> -		drm_gem_object_put(obj);
> -		return ERR_PTR(-EINVAL);
> +	if (!(domains & AMDGPU_GEM_DOMAIN_GTT)) {
> +		bool can_pin = true;
> +
> +		mutex_lock(&file_priv->prime.lock);
> +
> +		/* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */
> +		if (obj->import_attach) {
> +			drm_dbg_kms(dev, "Cannot create framebuffer from imported dma_buf\n");
> +			can_pin = false;
> +		} else if (obj->dma_buf) {
> +			struct dma_buf *dmabuf = obj->dma_buf;
> +			struct dma_buf_attachment *attachment;
> +
> +			dma_resv_lock(dmabuf->resv, NULL);
> +
> +			list_for_each_entry(attachment, &dmabuf->attachments, node) {
> +				if (IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY) && attachment->peer2peer)
> +					continue;
> +
> +				drm_dbg_kms(dev, "Cannot create framebuffer from non-P2P exported dma_buf\n");
> +				can_pin = false;
> +				break;
> +			}
> +
> +			dma_resv_unlock(dmabuf->resv);
> +		}
> +
> +		mutex_unlock(&file_priv->prime.lock);
> +
> +		if (!can_pin) {
> +			drm_gem_object_put(obj);
> +			return ERR_PTR(-EINVAL);
> +		}
>   	}
>   
>   	amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL);