Re: [PATCH 1/2] drm/virtio: Defer the host dumb buffer creation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23/08/2024 09:04, Gerd Hoffmann wrote:
+static void virtio_gpu_deferred_create(struct virtio_gpu_object *bo,
+				       struct virtio_gpu_device *vgdev,
+				       const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	struct virtio_gpu_object_params params = { 0 };
+
+	params.format = virtio_gpu_translate_format(mode_cmd->pixel_format);
+	params.width = mode_cmd->width;
+	params.height = mode_cmd->height;
+	params.size = params.height * params.width * 4;
+	params.size = ALIGN(params.size, PAGE_SIZE);
+	params.dumb = true;

I'd suggest to simply store the complete virtio_gpu_object_params struct
in virtio_gpu_object instead of re-creating it here.

The struct params is much bigger than the struct virtio_gpu_object, so I though it would waste too much memory. Using a pointer would add an alloc/free pair, and a potential source of memleak. And as we have the required parameters in struct drm_mode_fb_cmd2, I found it better this way.


diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 64c236169db88..8d1e8dcfa8c15 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -93,6 +93,9 @@ struct virtio_gpu_object {
  	bool dumb;
  	bool created;
  	bool host3d_blob, guest_blob;
+	bool deferred;
+	struct virtio_gpu_mem_entry *ents;
+	unsigned int nents;
  	uint32_t blob_mem, blob_flags;

Put them into a new block separated by newline, add a comment saying
those are needed for virtio_gpu_deferred_create?
Yes, I will do that in v2


@@ -229,9 +231,14 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
  						  objs, fence);
  		virtio_gpu_object_attach(vgdev, bo, ents, nents);
  	} else {
-		virtio_gpu_cmd_create_resource(vgdev, bo, params,
-					       objs, fence);
-		virtio_gpu_object_attach(vgdev, bo, ents, nents);
+		/* Fence is used only with blob or virgl */
+		WARN_ONCE(fence != NULL, "deferred doesn't support fence\n");

Additionally check for param->dumb to take the deferred code path?  That
should make sure there is no fence.

I think I can also duplicate virtio_gpu_object_create(), and have one version only for deferred dumb buffer. I will see if that doesn't make too much code duplication.


take care,
   Gerd


Thanks for the review,

--

Jocelyn





[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux