diff mbox series

[RFC,5/7] drm/virtio: Ensure that bo's backing store is valid while updating plane

Message ID 20240328083615.2662516-6-vivek.kasireddy@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/virtio: Import scanout buffers from other devices | expand

Commit Message

Vivek Kasireddy March 28, 2024, 8:32 a.m. UTC
To make sure that the imported bo's backing store is valid, we first
pin the associated dmabuf, import the sgt if need be and then unpin
it after the update is complete. Note that we pin/unpin the dmabuf
even when the backing store is valid to ensure that it does not move
when the host update (resource_flush) is in progress.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 drivers/gpu/drm/virtio/virtgpu_plane.c | 56 +++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

Comments

Weifeng Liu April 26, 2024, 6:06 a.m. UTC | #1
On Thu, 2024-03-28 at 01:32 -0700, Vivek Kasireddy wrote:
> To make sure that the imported bo's backing store is valid, we first
> pin the associated dmabuf, import the sgt if need be and then unpin
> it after the update is complete. Note that we pin/unpin the dmabuf
> even when the backing store is valid to ensure that it does not move
> when the host update (resource_flush) is in progress.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_plane.c | 56 +++++++++++++++++++++++++-
>  1 file changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index a72a2dbda031..3ccf88f9addc 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -26,6 +26,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_damage_helper.h>
>  #include <drm/drm_fourcc.h>
> +#include <linux/virtio_dma_buf.h>
>  
>  #include "virtgpu_drv.h"
>  
> @@ -131,6 +132,45 @@ static void virtio_gpu_update_dumb_bo(struct virtio_gpu_device *vgdev,
>  					   objs, NULL);
>  }
>  
> +static bool virtio_gpu_update_dmabuf_bo(struct virtio_gpu_device *vgdev,
> +					struct drm_gem_object *obj)
> +{
> +	struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
> +	struct dma_buf_attachment *attach = obj->import_attach;
> +	struct dma_resv *resv = attach->dmabuf->resv;
> +	struct virtio_gpu_mem_entry *ents = NULL;
> +	unsigned int nents;
> +	int ret;
> +
> +	dma_resv_lock(resv, NULL);
> +
> +	ret = dma_buf_pin(attach);
> +	if (ret) {
> +		dma_resv_unlock(resv);
> +		return false;
> +	}
> +
> +	if (!bo->has_backing) {
> +		if (bo->sgt)
> +			dma_buf_unmap_attachment(attach,
> +						 bo->sgt,
> +						 DMA_BIDIRECTIONAL);
> +
> +		ret = virtgpu_dma_buf_import_sgt(&ents, &nents,
> +						 bo, attach);
> +		if (ret)
> +			goto err_import;
> +
> +		virtio_gpu_object_attach(vgdev, bo, ents, nents);
> +	}
> +	return true;
> +
> +err_import:
> +	dma_buf_unpin(attach);
> +	dma_resv_unlock(resv);
> +	return false;
> +}
> +
>  static void virtio_gpu_resource_flush(struct drm_plane *plane,
>  				      uint32_t x, uint32_t y,
>  				      uint32_t width, uint32_t height)
> @@ -174,7 +214,9 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane,
>  	struct virtio_gpu_device *vgdev = dev->dev_private;
>  	struct virtio_gpu_output *output = NULL;
>  	struct virtio_gpu_object *bo;
> +	struct drm_gem_object *obj;
>  	struct drm_rect rect;
> +	bool updated = false;
>  
>  	if (plane->state->crtc)
>  		output = drm_crtc_to_virtio_gpu_output(plane->state->crtc);
> @@ -196,10 +238,17 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane,
>  	if (!drm_atomic_helper_damage_merged(old_state, plane->state, &rect))
>  		return;
>  
> -	bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
> +	obj = plane->state->fb->obj[0];
> +	bo = gem_to_virtio_gpu_obj(obj);
>  	if (bo->dumb)
>  		virtio_gpu_update_dumb_bo(vgdev, plane->state, &rect);
>  
> +	if (obj->import_attach) {
> +		updated = virtio_gpu_update_dmabuf_bo(vgdev, obj);
Hi Vivek,

It's possible that the objects imported from other devices are used in
other ways apart from being scanned out (e.g., they might act as
texture resources in 3D contexts in the virtio-GPU back-end).  Thus I
think we should find a better way of updating DMA-BUF objects, like
doing so in move_notify callback.

BTW, this patch set is very useful in implementing virtual display for
the case of SR-IOV, especially it supports sharing device local memory
between host and guest.  Thanks for your work and I am really hoping it
gets merged one day!

Best regards,
-Weifeng
> +		if (!updated)
> +			return;
> +	}
> +
>  	if (plane->state->fb != old_state->fb ||
>  	    plane->state->src_w != old_state->src_w ||
>  	    plane->state->src_h != old_state->src_h ||
> @@ -239,6 +288,11 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane,
>  				  rect.y1,
>  				  rect.x2 - rect.x1,
>  				  rect.y2 - rect.y1);
> +
> +	if (obj->import_attach && updated) {
> +		dma_buf_unpin(obj->import_attach);
> +		dma_resv_unlock(obj->import_attach->dmabuf->resv);
> +	}
>  }
>  
>  static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane,
diff mbox series

Patch

diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index a72a2dbda031..3ccf88f9addc 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -26,6 +26,7 @@ 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_fourcc.h>
+#include <linux/virtio_dma_buf.h>
 
 #include "virtgpu_drv.h"
 
@@ -131,6 +132,45 @@  static void virtio_gpu_update_dumb_bo(struct virtio_gpu_device *vgdev,
 					   objs, NULL);
 }
 
+static bool virtio_gpu_update_dmabuf_bo(struct virtio_gpu_device *vgdev,
+					struct drm_gem_object *obj)
+{
+	struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
+	struct dma_buf_attachment *attach = obj->import_attach;
+	struct dma_resv *resv = attach->dmabuf->resv;
+	struct virtio_gpu_mem_entry *ents = NULL;
+	unsigned int nents;
+	int ret;
+
+	dma_resv_lock(resv, NULL);
+
+	ret = dma_buf_pin(attach);
+	if (ret) {
+		dma_resv_unlock(resv);
+		return false;
+	}
+
+	if (!bo->has_backing) {
+		if (bo->sgt)
+			dma_buf_unmap_attachment(attach,
+						 bo->sgt,
+						 DMA_BIDIRECTIONAL);
+
+		ret = virtgpu_dma_buf_import_sgt(&ents, &nents,
+						 bo, attach);
+		if (ret)
+			goto err_import;
+
+		virtio_gpu_object_attach(vgdev, bo, ents, nents);
+	}
+	return true;
+
+err_import:
+	dma_buf_unpin(attach);
+	dma_resv_unlock(resv);
+	return false;
+}
+
 static void virtio_gpu_resource_flush(struct drm_plane *plane,
 				      uint32_t x, uint32_t y,
 				      uint32_t width, uint32_t height)
@@ -174,7 +214,9 @@  static void virtio_gpu_primary_plane_update(struct drm_plane *plane,
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct virtio_gpu_output *output = NULL;
 	struct virtio_gpu_object *bo;
+	struct drm_gem_object *obj;
 	struct drm_rect rect;
+	bool updated = false;
 
 	if (plane->state->crtc)
 		output = drm_crtc_to_virtio_gpu_output(plane->state->crtc);
@@ -196,10 +238,17 @@  static void virtio_gpu_primary_plane_update(struct drm_plane *plane,
 	if (!drm_atomic_helper_damage_merged(old_state, plane->state, &rect))
 		return;
 
-	bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
+	obj = plane->state->fb->obj[0];
+	bo = gem_to_virtio_gpu_obj(obj);
 	if (bo->dumb)
 		virtio_gpu_update_dumb_bo(vgdev, plane->state, &rect);
 
+	if (obj->import_attach) {
+		updated = virtio_gpu_update_dmabuf_bo(vgdev, obj);
+		if (!updated)
+			return;
+	}
+
 	if (plane->state->fb != old_state->fb ||
 	    plane->state->src_w != old_state->src_w ||
 	    plane->state->src_h != old_state->src_h ||
@@ -239,6 +288,11 @@  static void virtio_gpu_primary_plane_update(struct drm_plane *plane,
 				  rect.y1,
 				  rect.x2 - rect.x1,
 				  rect.y2 - rect.y1);
+
+	if (obj->import_attach && updated) {
+		dma_buf_unpin(obj->import_attach);
+		dma_resv_unlock(obj->import_attach->dmabuf->resv);
+	}
 }
 
 static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane,