diff mbox series

drm/virtio: Create Dumb BOs as guest Blobs (v3)

Message ID 20210413052614.2486768-1-vivek.kasireddy@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/virtio: Create Dumb BOs as guest Blobs (v3) | expand

Commit Message

Kasireddy, Vivek April 13, 2021, 5:26 a.m. UTC
If support for Blob resources is available, then dumb BOs created
by the driver can be considered as guest Blobs.

v2: Don't skip transfer and flush commands as part of plane update
as the device may have created a shared mapping. (Gerd)

v3: Don't create dumb BOs as Guest blobs if Virgl is enabled. (Gurchetan)

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 drivers/gpu/drm/virtio/virtgpu_gem.c    | 8 ++++++++
 drivers/gpu/drm/virtio/virtgpu_object.c | 3 +++
 2 files changed, 11 insertions(+)

Comments

Tina Zhang April 14, 2021, 6:36 a.m. UTC | #1
Hi Gerd,

Speaking of the modifier, we notice that the virtio-gpu driver's mode_config.allow_fb_modifiers = false, which means virtio-gpu doesn't support modifier. With mode_config.allow_fb_modifiers=false, the DRM Modifier API would fail. 

So, do you know if there is any concern about letting virito-gpu allow modifiers? Thanks.

BR,
Tina

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> Vivek Kasireddy
> Sent: Tuesday, April 13, 2021 1:26 PM
> To: dri-devel@lists.freedesktop.org
> Cc: Kasireddy, Vivek <vivek.kasireddy@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>
> Subject: [PATCH] drm/virtio: Create Dumb BOs as guest Blobs (v3)
> 
> If support for Blob resources is available, then dumb BOs created by the
> driver can be considered as guest Blobs.
> 
> v2: Don't skip transfer and flush commands as part of plane update as the
> device may have created a shared mapping. (Gerd)
> 
> v3: Don't create dumb BOs as Guest blobs if Virgl is enabled. (Gurchetan)
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_gem.c    | 8 ++++++++
>  drivers/gpu/drm/virtio/virtgpu_object.c | 3 +++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c
> b/drivers/gpu/drm/virtio/virtgpu_gem.c
> index 8502400b2f9c..2de61b63ef91 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
> @@ -64,6 +64,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file
> *file_priv,  {
>  	struct drm_gem_object *gobj;
>  	struct virtio_gpu_object_params params = { 0 };
> +	struct virtio_gpu_device *vgdev = dev->dev_private;
>  	int ret;
>  	uint32_t pitch;
> 
> @@ -79,6 +80,13 @@ int virtio_gpu_mode_dumb_create(struct drm_file
> *file_priv,
>  	params.height = args->height;
>  	params.size = args->size;
>  	params.dumb = true;
> +
> +	if (vgdev->has_resource_blob && !vgdev->has_virgl_3d) {
> +		params.blob_mem = VIRTGPU_BLOB_MEM_GUEST;
> +		params.blob_flags = VIRTGPU_BLOB_FLAG_USE_SHAREABLE;
> +		params.blob = true;
> +	}
> +
>  	ret = virtio_gpu_gem_create(file_priv, dev, &params, &gobj,
>  				    &args->handle);
>  	if (ret)
> diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
> b/drivers/gpu/drm/virtio/virtgpu_object.c
> index 4ff1ec28e630..f648b0e24447 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_object.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_object.c
> @@ -254,6 +254,9 @@ int virtio_gpu_object_create(struct virtio_gpu_device
> *vgdev,
>  	}
> 
>  	if (params->blob) {
> +		if (params->blob_mem == VIRTGPU_BLOB_MEM_GUEST)
> +			bo->guest_blob = true;
> +
>  		virtio_gpu_cmd_resource_create_blob(vgdev, bo, params,
>  						    ents, nents);
>  	} else if (params->virgl) {
> --
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Gerd Hoffmann April 14, 2021, 9:29 a.m. UTC | #2
On Wed, Apr 14, 2021 at 06:36:55AM +0000, Zhang, Tina wrote:
> Hi Gerd,
> 
> Speaking of the modifier, we notice that the virtio-gpu driver's
> mode_config.allow_fb_modifiers = false, which means virtio-gpu doesn't
> support modifier. With mode_config.allow_fb_modifiers=false, the DRM
> Modifier API would fail. 
> 
> So, do you know if there is any concern about letting virito-gpu allow
> modifiers? Thanks.

Well, virtio-gpu must also provide a list of modifiers then.  We need
some way for virtio-gpu to figure which modifiers are supported by the
host and which are not.  Otherwise we could list LINEAR only which
doesn't buy us much ;)

Not sure whenever virglrenderer allows that already (via
VIRGL_CCMD*QUERY* or via virgl caps).  If not we could define a new
modifiers capability for that, which could then be used for both virgl
and non-virgl mode.

take care,
  Gerd
Gurchetan Singh April 14, 2021, 11:31 p.m. UTC | #3
On Mon, Apr 12, 2021 at 10:36 PM Vivek Kasireddy <vivek.kasireddy@intel.com>
wrote:

> If support for Blob resources is available, then dumb BOs created
> by the driver can be considered as guest Blobs.
>
> v2: Don't skip transfer and flush commands as part of plane update
> as the device may have created a shared mapping. (Gerd)
>
> v3: Don't create dumb BOs as Guest blobs if Virgl is enabled. (Gurchetan)
>

I think it is a good start and advances QEMU blobs.  Improvements are
always possible, but may be made at a future time.

Acked-by: Gurchetan Singh <gurchetansingh@chromium.org>


>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_gem.c    | 8 ++++++++
>  drivers/gpu/drm/virtio/virtgpu_object.c | 3 +++
>  2 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c
> b/drivers/gpu/drm/virtio/virtgpu_gem.c
> index 8502400b2f9c..2de61b63ef91 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
> @@ -64,6 +64,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file
> *file_priv,
>  {
>         struct drm_gem_object *gobj;
>         struct virtio_gpu_object_params params = { 0 };
> +       struct virtio_gpu_device *vgdev = dev->dev_private;
>         int ret;
>         uint32_t pitch;
>
> @@ -79,6 +80,13 @@ int virtio_gpu_mode_dumb_create(struct drm_file
> *file_priv,
>         params.height = args->height;
>         params.size = args->size;
>         params.dumb = true;
> +
> +       if (vgdev->has_resource_blob && !vgdev->has_virgl_3d) {
> +               params.blob_mem = VIRTGPU_BLOB_MEM_GUEST;
> +               params.blob_flags = VIRTGPU_BLOB_FLAG_USE_SHAREABLE;
> +               params.blob = true;
> +       }
> +
>         ret = virtio_gpu_gem_create(file_priv, dev, &params, &gobj,
>                                     &args->handle);
>         if (ret)
> diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
> b/drivers/gpu/drm/virtio/virtgpu_object.c
> index 4ff1ec28e630..f648b0e24447 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_object.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_object.c
> @@ -254,6 +254,9 @@ int virtio_gpu_object_create(struct virtio_gpu_device
> *vgdev,
>         }
>
>         if (params->blob) {
> +               if (params->blob_mem == VIRTGPU_BLOB_MEM_GUEST)
> +                       bo->guest_blob = true;
> +
>                 virtio_gpu_cmd_resource_create_blob(vgdev, bo, params,
>                                                     ents, nents);
>         } else if (params->virgl) {
> --
> 2.26.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Gerd Hoffmann April 15, 2021, 9:07 a.m. UTC | #4
On Wed, Apr 14, 2021 at 04:31:45PM -0700, Gurchetan Singh wrote:
> On Mon, Apr 12, 2021 at 10:36 PM Vivek Kasireddy <vivek.kasireddy@intel.com>
> wrote:
> 
> > If support for Blob resources is available, then dumb BOs created
> > by the driver can be considered as guest Blobs.
> >
> > v2: Don't skip transfer and flush commands as part of plane update
> > as the device may have created a shared mapping. (Gerd)
> >
> > v3: Don't create dumb BOs as Guest blobs if Virgl is enabled. (Gurchetan)
> >
> 
> I think it is a good start and advances QEMU blobs.  Improvements are
> always possible, but may be made at a future time.
> 
> Acked-by: Gurchetan Singh <gurchetansingh@chromium.org>

Agree.  Future improvements (like maybe use HOST3D blobs for virgl=on)
can easily go on top of this.

Pushed to drm-misc-next.

thanks,
  Gerd
diff mbox series

Patch

diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 8502400b2f9c..2de61b63ef91 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -64,6 +64,7 @@  int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 {
 	struct drm_gem_object *gobj;
 	struct virtio_gpu_object_params params = { 0 };
+	struct virtio_gpu_device *vgdev = dev->dev_private;
 	int ret;
 	uint32_t pitch;
 
@@ -79,6 +80,13 @@  int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 	params.height = args->height;
 	params.size = args->size;
 	params.dumb = true;
+
+	if (vgdev->has_resource_blob && !vgdev->has_virgl_3d) {
+		params.blob_mem = VIRTGPU_BLOB_MEM_GUEST;
+		params.blob_flags = VIRTGPU_BLOB_FLAG_USE_SHAREABLE;
+		params.blob = true;
+	}
+
 	ret = virtio_gpu_gem_create(file_priv, dev, &params, &gobj,
 				    &args->handle);
 	if (ret)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 4ff1ec28e630..f648b0e24447 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -254,6 +254,9 @@  int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
 	}
 
 	if (params->blob) {
+		if (params->blob_mem == VIRTGPU_BLOB_MEM_GUEST)
+			bo->guest_blob = true;
+
 		virtio_gpu_cmd_resource_create_blob(vgdev, bo, params,
 						    ents, nents);
 	} else if (params->virgl) {