diff mbox series

[v3,09/19] drm/virtio: implement blob resources: probe for host visible region

Message ID 20200917000838.735-9-gurchetansingh@chromium.org (mailing list archive)
State New, archived
Headers show
Series [v3,01/19] drm/virtio: blob prep: refactor getting pages and attaching backing | expand

Commit Message

Gurchetan Singh Sept. 17, 2020, 12:08 a.m. UTC 
From: Gerd Hoffmann <kraxel@redhat.com>

The availability of the host visible region means host 3D
allocations can be directly mapped in the guest.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Co-developed-by: Gurchetan Singh <gurchetansingh@chromium.org>
Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
Acked-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/gpu/drm/virtio/virtgpu_debugfs.c |  5 +++++
 drivers/gpu/drm/virtio/virtgpu_drv.h     |  2 ++
 drivers/gpu/drm/virtio/virtgpu_kms.c     | 27 ++++++++++++++++++++++--
 3 files changed, 32 insertions(+), 2 deletions(-)

Comments

Gerd Hoffmann Sept. 17, 2020, 9:40 a.m. UTC | #1 
Hi,

> +		if (!devm_request_mem_region(&vgdev->vdev->dev,
> +					     vgdev->host_visible_region.addr,
> +					     vgdev->host_visible_region.len,
> +					     dev_name(&vgdev->vdev->dev))) {
> +			DRM_ERROR("Could not reserve host visible region\n");
> +			goto err_vqs;
> +		}

> +	if (vgdev->has_host_visible) {
> +		devm_release_mem_region(&vgdev->vdev->dev,
> +					vgdev->host_visible_region.addr,
> +					vgdev->host_visible_region.len);
> +	}

Hmm. isn't it the point of the managed apis that the release happens
automatically?  I think you don't need the devm_release_mem_region
call (it doesn't break things though).

take care,
  Gerd
Gurchetan Singh Sept. 17, 2020, 10:27 p.m. UTC | #2 
On Thu, Sep 17, 2020 at 2:40 AM Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
>
> > +             if (!devm_request_mem_region(&vgdev->vdev->dev,
> > +
> vgdev->host_visible_region.addr,
> > +
> vgdev->host_visible_region.len,
> > +                                          dev_name(&vgdev->vdev->dev)))
> {
> > +                     DRM_ERROR("Could not reserve host visible
> region\n");
> > +                     goto err_vqs;
> > +             }
>
> > +     if (vgdev->has_host_visible) {
> > +             devm_release_mem_region(&vgdev->vdev->dev,
> > +                                     vgdev->host_visible_region.addr,
> > +                                     vgdev->host_visible_region.len);
> > +     }
>
> Hmm. isn't it the point of the managed apis that the release happens
> automatically?  I think you don't need the devm_release_mem_region
> call (it doesn't break things though).
>

Ack, good catch.  Fixed.


>
> take care,
>   Gerd
>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/virtio/virtgpu_debugfs.c b/drivers/gpu/drm/virtio/virtgpu_debugfs.c
index 6b9b8376613f0..a2cdd267914ac 100644
--- a/drivers/gpu/drm/virtio/virtgpu_debugfs.c
+++ b/drivers/gpu/drm/virtio/virtgpu_debugfs.c
@@ -52,6 +52,11 @@  static int virtio_gpu_features(struct seq_file *m, void *data)
 	virtio_add_bool(m, "blob resources", vgdev->has_resource_blob);
 	virtio_add_int(m, "cap sets", vgdev->num_capsets);
 	virtio_add_int(m, "scanouts", vgdev->num_scanouts);
+	if (vgdev->host_visible_region.len) {
+		seq_printf(m, "%-16s : 0x%lx +0x%lx\n", "host visible region",
+			   (unsigned long)vgdev->host_visible_region.addr,
+			   (unsigned long)vgdev->host_visible_region.len);
+	}
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index b53478a6a3c08..391637f0b362d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -209,6 +209,8 @@  struct virtio_gpu_device {
 	bool has_indirect;
 	bool has_resource_assign_uuid;
 	bool has_resource_blob;
+	bool has_host_visible;
+	struct virtio_shm_region host_visible_region;
 
 	struct work_struct config_changed_work;
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 0678e56100dae..6239cc984958f 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -155,11 +155,27 @@  int virtio_gpu_init(struct drm_device *dev)
 	if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_RESOURCE_BLOB)) {
 		vgdev->has_resource_blob = true;
 	}
+	if (virtio_get_shm_region(vgdev->vdev, &vgdev->host_visible_region,
+				  VIRTIO_GPU_SHM_ID_HOST_VISIBLE)) {
+		if (!devm_request_mem_region(&vgdev->vdev->dev,
+					     vgdev->host_visible_region.addr,
+					     vgdev->host_visible_region.len,
+					     dev_name(&vgdev->vdev->dev))) {
+			DRM_ERROR("Could not reserve host visible region\n");
+			goto err_vqs;
+		}
+
+		DRM_INFO("Host memory window: 0x%lx +0x%lx\n",
+			 (unsigned long)vgdev->host_visible_region.addr,
+			 (unsigned long)vgdev->host_visible_region.len);
+		vgdev->has_host_visible = true;
+	}
 
-	DRM_INFO("features: %cvirgl %cedid %cresource_blob\n",
+	DRM_INFO("features: %cvirgl %cedid %cresource_blob %chost_visible\n",
 		 vgdev->has_virgl_3d    ? '+' : '-',
 		 vgdev->has_edid        ? '+' : '-',
-		 vgdev->has_resource_blob ? '+' : '-');
+		 vgdev->has_resource_blob ? '+' : '-',
+		 vgdev->has_host_visible ? '+' : '-');
 
 	ret = virtio_find_vqs(vgdev->vdev, 2, vqs, callbacks, names, NULL);
 	if (ret) {
@@ -247,6 +263,13 @@  void virtio_gpu_release(struct drm_device *dev)
 	virtio_gpu_free_vbufs(vgdev);
 	virtio_gpu_cleanup_cap_cache(vgdev);
 	kfree(vgdev->capsets);
+
+	if (vgdev->has_host_visible) {
+		devm_release_mem_region(&vgdev->vdev->dev,
+					vgdev->host_visible_region.addr,
+					vgdev->host_visible_region.len);
+	}
+
 	kfree(vgdev);
 }