| 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 |
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, ¶ms, &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
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
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, ¶ms, &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 >
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 --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, ¶ms, &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) {
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(+)