dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Virtio-GPU suspend and resume
@ 2025-07-02 22:24 dongwon.kim
  2025-07-02 22:24 ` [PATCH v3 1/2] drm/virtio: Freeze and restore hooks to support " dongwon.kim
  2025-07-02 22:24 ` [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects dongwon.kim
  0 siblings, 2 replies; 11+ messages in thread
From: dongwon.kim @ 2025-07-02 22:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Dmitry Osipenko, Vivek Kasireddy

From: Dongwon Kim <dongwon.kim@intel.com>

This patch series introduces a freeze and restore mechanism for
the virtio-gpu driver:

First patch adds `virtgpu_freeze` and `virtgpu_restore` functions.
These functions handle the deletion of virtio queues before suspension and
their recreation during the restoration process.

Second patch implements a mechanism for restoring `virtio_gpu_object` instances.
This is necessary because the host (QEMU) deletes all associated resources during
the virtio-gpu reset, which occurs as part of the restoration process.

These changes ensure that the virtio-gpu driver can properly handle suspend and
resume scenarios without resource loss.

v2: 10ms sleep is added in virtgpu_freeze to avoid the situation
    the driver is locked up during resumption.

v3: Plain 10ms delay (v2) is replaced with wait calls which wait until
    the virtio queue is empty.
    (Dmitry Osipenko)

Dongwon Kim (2):
  drm/virtio: Freeze and restore hooks to support suspend and resume
  drm/virtio: Implement save and restore for virtio_gpu_objects

 drivers/gpu/drm/virtio/virtgpu_drv.c    | 67 ++++++++++++++++++++++-
 drivers/gpu/drm/virtio/virtgpu_drv.h    | 11 ++++
 drivers/gpu/drm/virtio/virtgpu_kms.c    | 24 ++++++---
 drivers/gpu/drm/virtio/virtgpu_object.c | 72 +++++++++++++++++++++++++
 4 files changed, 167 insertions(+), 7 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/2] drm/virtio: Freeze and restore hooks to support suspend and resume
  2025-07-02 22:24 [PATCH v3 0/2] Virtio-GPU suspend and resume dongwon.kim
@ 2025-07-02 22:24 ` dongwon.kim
  2025-07-09 11:21   ` Dmitry Osipenko
  2025-07-09 11:21   ` Dmitry Osipenko
  2025-07-02 22:24 ` [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects dongwon.kim
  1 sibling, 2 replies; 11+ messages in thread
From: dongwon.kim @ 2025-07-02 22:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Dmitry Osipenko, Vivek Kasireddy

From: Dongwon Kim <dongwon.kim@intel.com>

virtio device needs to delete before VM suspend happens
then reinitialize all virtqueues again upon resume

v2: 10ms sleep is added in virtgpu_freeze to avoid the situation
    the driver is locked up during resumption.

v3: Plain 10ms delay (v2) is replaced with wait calls which wait until
    the virtio queue is empty.
    (Dmitry Osipenko)

Suggested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 drivers/gpu/drm/virtio/virtgpu_drv.c | 61 +++++++++++++++++++++++++++-
 drivers/gpu/drm/virtio/virtgpu_drv.h |  1 +
 drivers/gpu/drm/virtio/virtgpu_kms.c | 23 ++++++++---
 3 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index e32e680c7197..bcdd6e37db3e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -163,6 +163,60 @@ static unsigned int features[] = {
 	VIRTIO_GPU_F_RESOURCE_BLOB,
 	VIRTIO_GPU_F_CONTEXT_INIT,
 };
+
+#ifdef CONFIG_PM_SLEEP
+static int virtgpu_freeze(struct virtio_device *vdev)
+{
+	struct drm_device *dev = vdev->priv;
+	struct virtio_gpu_device *vgdev = dev->dev_private;
+	int error;
+
+	error = drm_mode_config_helper_suspend(dev);
+	if (error) {
+		DRM_ERROR("suspend error %d\n", error);
+		return error;
+	}
+
+	flush_work(&vgdev->obj_free_work);
+	flush_work(&vgdev->ctrlq.dequeue_work);
+	flush_work(&vgdev->cursorq.dequeue_work);
+	flush_work(&vgdev->config_changed_work);
+
+	wait_event(vgdev->ctrlq.ack_queue,
+		   vgdev->ctrlq.vq->num_free == vgdev->ctrlq.vq->num_max);
+
+	wait_event(vgdev->cursorq.ack_queue,
+		   vgdev->cursorq.vq->num_free == vgdev->cursorq.vq->num_max);
+
+	vdev->config->del_vqs(vdev);
+
+	return 0;
+}
+
+static int virtgpu_restore(struct virtio_device *vdev)
+{
+	struct drm_device *dev = vdev->priv;
+	struct virtio_gpu_device *vgdev = dev->dev_private;
+	int error;
+
+	error = virtio_gpu_find_vqs(vgdev);
+	if (error) {
+		DRM_ERROR("failed to find virt queues\n");
+		return error;
+	}
+
+	virtio_device_ready(vdev);
+
+	error = drm_mode_config_helper_resume(dev);
+	if (error) {
+		DRM_ERROR("resume error %d\n", error);
+		return error;
+	}
+
+	return 0;
+}
+#endif
+
 static struct virtio_driver virtio_gpu_driver = {
 	.feature_table = features,
 	.feature_table_size = ARRAY_SIZE(features),
@@ -171,7 +225,11 @@ static struct virtio_driver virtio_gpu_driver = {
 	.probe = virtio_gpu_probe,
 	.remove = virtio_gpu_remove,
 	.shutdown = virtio_gpu_shutdown,
-	.config_changed = virtio_gpu_config_changed
+	.config_changed = virtio_gpu_config_changed,
+#ifdef CONFIG_PM_SLEEP
+	.freeze = virtgpu_freeze,
+	.restore = virtgpu_restore,
+#endif
 };
 
 static int __init virtio_gpu_driver_init(void)
@@ -193,6 +251,7 @@ static int __init virtio_gpu_driver_init(void)
 
 	ret = register_virtio_driver(&virtio_gpu_driver);
 
+	printk("**dw_debug:: virtio-init\n");
 	if (pdev) {
 		if (pci_is_vga(pdev))
 			vga_put(pdev,
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index f17660a71a3e..1279f998c8e0 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -300,6 +300,7 @@ void virtio_gpu_deinit(struct drm_device *dev);
 void virtio_gpu_release(struct drm_device *dev);
 int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file);
 void virtio_gpu_driver_postclose(struct drm_device *dev, struct drm_file *file);
+int virtio_gpu_find_vqs(struct virtio_gpu_device *vgdev);
 
 /* virtgpu_gem.c */
 int virtio_gpu_gem_object_open(struct drm_gem_object *obj,
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 7dfb2006c561..6c1af77ea083 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -114,15 +114,28 @@ static void virtio_gpu_get_capsets(struct virtio_gpu_device *vgdev,
 	vgdev->num_capsets = num_capsets;
 }
 
-int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
+int virtio_gpu_find_vqs(struct virtio_gpu_device *vgdev)
 {
 	struct virtqueue_info vqs_info[] = {
 		{ "control", virtio_gpu_ctrl_ack },
 		{ "cursor", virtio_gpu_cursor_ack },
 	};
-	struct virtio_gpu_device *vgdev;
-	/* this will expand later */
 	struct virtqueue *vqs[2];
+	int ret;
+
+	ret = virtio_find_vqs(vgdev->vdev, 2, vqs, vqs_info, NULL);
+	if (ret)
+		return ret;
+
+	vgdev->ctrlq.vq = vqs[0];
+	vgdev->cursorq.vq = vqs[1];
+
+	return 0;
+}
+
+int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
+{
+	struct virtio_gpu_device *vgdev;
 	u32 num_scanouts, num_capsets;
 	int ret = 0;
 
@@ -206,13 +219,11 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
 	DRM_INFO("features: %ccontext_init\n",
 		 vgdev->has_context_init ? '+' : '-');
 
-	ret = virtio_find_vqs(vgdev->vdev, 2, vqs, vqs_info, NULL);
+	ret = virtio_gpu_find_vqs(vgdev);
 	if (ret) {
 		DRM_ERROR("failed to find virt queues\n");
 		goto err_vqs;
 	}
-	vgdev->ctrlq.vq = vqs[0];
-	vgdev->cursorq.vq = vqs[1];
 	ret = virtio_gpu_alloc_vbufs(vgdev);
 	if (ret) {
 		DRM_ERROR("failed to alloc vbufs\n");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects
  2025-07-02 22:24 [PATCH v3 0/2] Virtio-GPU suspend and resume dongwon.kim
  2025-07-02 22:24 ` [PATCH v3 1/2] drm/virtio: Freeze and restore hooks to support " dongwon.kim
@ 2025-07-02 22:24 ` dongwon.kim
  2025-07-09 11:23   ` Dmitry Osipenko
  2025-07-09 13:26   ` Dmitry Osipenko
  1 sibling, 2 replies; 11+ messages in thread
From: dongwon.kim @ 2025-07-02 22:24 UTC (permalink / raw)
  To: dri-devel; +Cc: Dmitry Osipenko, Vivek Kasireddy

From: Dongwon Kim <dongwon.kim@intel.com>

Host KVM/QEMU loses all graphics resources submitted by the guest OS
upon resumption from sleep or hibernation. This results in invalid
resource errors when the guest OS attempts to interact with the host
regarding those resources.

To address this issue, the virtio-gpu driver now resubmits all existing
resources upon resumption. A linked list has been introduced to maintain
references to all created `virtio_gpu_object` instances and their parameters.

Whenever a new object is created and sent to the host, it is added to this
list. During the `.resume` function, all backed-up objects are re-sent to
the host using the 'create resource' virtio GPU command, ensuring the
resources are restored seamlessly.

v2: - Attach backing is done if bo->attached was set before

Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 drivers/gpu/drm/virtio/virtgpu_drv.c    |  6 +++
 drivers/gpu/drm/virtio/virtgpu_drv.h    | 10 ++++
 drivers/gpu/drm/virtio/virtgpu_kms.c    |  1 +
 drivers/gpu/drm/virtio/virtgpu_object.c | 72 +++++++++++++++++++++++++
 4 files changed, 89 insertions(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index bcdd6e37db3e..3f0c84696f30 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -207,6 +207,12 @@ static int virtgpu_restore(struct virtio_device *vdev)
 
 	virtio_device_ready(vdev);
 
+	error = virtio_gpu_object_restore_all(vgdev);
+	if (error) {
+		DRM_ERROR("Failed to recover objects\n");
+		return error;
+	}
+
 	error = drm_mode_config_helper_resume(dev);
 	if (error) {
 		DRM_ERROR("resume error %d\n", error);
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 1279f998c8e0..55f836378237 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -126,6 +126,12 @@ struct virtio_gpu_object_array {
 	struct drm_gem_object *objs[] __counted_by(total);
 };
 
+struct virtio_gpu_object_restore {
+	struct virtio_gpu_object *bo;
+	struct virtio_gpu_object_params params;
+	struct list_head node;
+};
+
 struct virtio_gpu_vbuffer;
 struct virtio_gpu_device;
 
@@ -265,6 +271,7 @@ struct virtio_gpu_device {
 	struct work_struct obj_free_work;
 	spinlock_t obj_free_lock;
 	struct list_head obj_free_list;
+	struct list_head obj_restore;
 
 	struct virtio_gpu_drv_capset *capsets;
 	uint32_t num_capsets;
@@ -479,6 +486,9 @@ bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo);
 
 int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
 			       uint32_t *resid);
+
+int virtio_gpu_object_restore_all(struct virtio_gpu_device *vgdev);
+
 /* virtgpu_prime.c */
 int virtio_gpu_resource_assign_uuid(struct virtio_gpu_device *vgdev,
 				    struct virtio_gpu_object *bo);
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 6c1af77ea083..17d182737910 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -162,6 +162,7 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
 	vgdev->fence_drv.context = dma_fence_context_alloc(1);
 	spin_lock_init(&vgdev->fence_drv.lock);
 	INIT_LIST_HEAD(&vgdev->fence_drv.fences);
+	INIT_LIST_HEAD(&vgdev->obj_restore);
 	INIT_LIST_HEAD(&vgdev->cap_cache);
 	INIT_WORK(&vgdev->config_changed_work,
 		  virtio_gpu_config_changed_work_func);
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 5517cff8715c..15c2598187ed 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -61,6 +61,38 @@ static void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t
 	}
 }
 
+static void virtio_gpu_object_add_restore_list(struct virtio_gpu_device *vgdev,
+					       struct virtio_gpu_object *bo,
+					       struct virtio_gpu_object_params *params)
+{
+	struct virtio_gpu_object_restore *new;
+
+	new = kvmalloc(sizeof(*new), GFP_KERNEL);
+	if (!new) {
+		DRM_ERROR("Fail to allocate virtio_gpu_object_restore");
+		return;
+	}
+
+	new->bo = bo;
+	memcpy(&new->params, params, sizeof(*params));
+
+	list_add_tail(&new->node, &vgdev->obj_restore);
+}
+
+static void virtio_gpu_object_del_restore_list(struct virtio_gpu_device *vgdev,
+					       struct virtio_gpu_object *bo)
+{
+	struct virtio_gpu_object_restore *curr, *tmp;
+
+	list_for_each_entry_safe(curr, tmp, &vgdev->obj_restore, node) {
+		if (bo == curr->bo) {
+			list_del(&curr->node);
+			kvfree(curr);
+			break;
+		}
+	}
+}
+
 void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
 {
 	struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
@@ -84,6 +116,7 @@ void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
 		drm_gem_object_release(&bo->base.base);
 		kfree(bo);
 	}
+	virtio_gpu_object_del_restore_list(vgdev, bo);
 }
 
 static void virtio_gpu_free_object(struct drm_gem_object *obj)
@@ -257,8 +290,11 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
 					       objs, fence);
 		virtio_gpu_object_attach(vgdev, bo, ents, nents);
 	}
+	/* add submitted object to restore list */
+	virtio_gpu_object_add_restore_list(vgdev, bo, params);
 
 	*bo_ptr = bo;
+
 	return 0;
 
 err_put_objs:
@@ -271,3 +307,39 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
 	drm_gem_shmem_free(shmem_obj);
 	return ret;
 }
+
+int virtio_gpu_object_restore_all(struct virtio_gpu_device *vgdev)
+{
+	struct virtio_gpu_object_restore *curr, *tmp;
+	struct virtio_gpu_mem_entry *ents;
+	unsigned int nents;
+	int ret;
+
+	list_for_each_entry_safe(curr, tmp, &vgdev->obj_restore, node) {
+		ret = virtio_gpu_object_shmem_init(vgdev, curr->bo, &ents, &nents);
+		if (ret)
+			break;
+
+		if (curr->params.blob) {
+			virtio_gpu_cmd_resource_create_blob(vgdev, curr->bo, &curr->params,
+							    ents, nents);
+		} else if (curr->params.virgl) {
+			virtio_gpu_cmd_resource_create_3d(vgdev, curr->bo, &curr->params,
+							  NULL, NULL);
+
+			if (curr->bo->attached) {
+				curr->bo->attached = false;
+				virtio_gpu_object_attach(vgdev, curr->bo, ents, nents);
+			}
+		} else {
+			virtio_gpu_cmd_create_resource(vgdev, curr->bo, &curr->params,
+						       NULL, NULL);
+			if (curr->bo->attached) {
+				curr->bo->attached = false;
+				virtio_gpu_object_attach(vgdev, curr->bo, ents, nents);
+			}
+		}
+	}
+
+	return ret;
+}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] drm/virtio: Freeze and restore hooks to support suspend and resume
  2025-07-02 22:24 ` [PATCH v3 1/2] drm/virtio: Freeze and restore hooks to support " dongwon.kim
@ 2025-07-09 11:21   ` Dmitry Osipenko
  2025-07-09 11:21   ` Dmitry Osipenko
  1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2025-07-09 11:21 UTC (permalink / raw)
  To: dongwon.kim, dri-devel; +Cc: Vivek Kasireddy

On 7/3/25 01:24, dongwon.kim@intel.com wrote:
> +static int virtgpu_freeze(struct virtio_device *vdev)
> +{
> +	struct drm_device *dev = vdev->priv;
> +	struct virtio_gpu_device *vgdev = dev->dev_private;
> +	int error;
> +
> +	error = drm_mode_config_helper_suspend(dev);
> +	if (error) {
> +		DRM_ERROR("suspend error %d\n", error);
> +		return error;
> +	}
> +
> +	flush_work(&vgdev->obj_free_work);
> +	flush_work(&vgdev->ctrlq.dequeue_work);
> +	flush_work(&vgdev->cursorq.dequeue_work);
> +	flush_work(&vgdev->config_changed_work);
> +
> +	wait_event(vgdev->ctrlq.ack_queue,
> +		   vgdev->ctrlq.vq->num_free == vgdev->ctrlq.vq->num_max);
> +
> +	wait_event(vgdev->cursorq.ack_queue,
> +		   vgdev->cursorq.vq->num_free == vgdev->cursorq.vq->num_max);

Should be more correct to first do a wait_event() and then flush_work()
to not race with the work completion.



-- 
Best regards,
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] drm/virtio: Freeze and restore hooks to support suspend and resume
  2025-07-02 22:24 ` [PATCH v3 1/2] drm/virtio: Freeze and restore hooks to support " dongwon.kim
  2025-07-09 11:21   ` Dmitry Osipenko
@ 2025-07-09 11:21   ` Dmitry Osipenko
  1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2025-07-09 11:21 UTC (permalink / raw)
  To: dongwon.kim, dri-devel; +Cc: Vivek Kasireddy

On 7/3/25 01:24, dongwon.kim@intel.com wrote:
>  static int __init virtio_gpu_driver_init(void)
> @@ -193,6 +251,7 @@ static int __init virtio_gpu_driver_init(void)
>  
>  	ret = register_virtio_driver(&virtio_gpu_driver);
>  
> +	printk("**dw_debug:: virtio-init\n");

Leftover printk

-- 
Best regards,
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects
  2025-07-02 22:24 ` [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects dongwon.kim
@ 2025-07-09 11:23   ` Dmitry Osipenko
  2025-07-09 13:26   ` Dmitry Osipenko
  1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2025-07-09 11:23 UTC (permalink / raw)
  To: dongwon.kim, dri-devel; +Cc: Vivek Kasireddy

On 7/3/25 01:24, dongwon.kim@intel.com wrote:
> @@ -257,8 +290,11 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
>  					       objs, fence);
>  		virtio_gpu_object_attach(vgdev, bo, ents, nents);
>  	}
> +	/* add submitted object to restore list */
> +	virtio_gpu_object_add_restore_list(vgdev, bo, params);

The list should be protected with a mutex

-- 
Best regards,
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects
  2025-07-02 22:24 ` [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects dongwon.kim
  2025-07-09 11:23   ` Dmitry Osipenko
@ 2025-07-09 13:26   ` Dmitry Osipenko
  2025-07-09 15:34     ` Kim, Dongwon
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2025-07-09 13:26 UTC (permalink / raw)
  To: dongwon.kim, dri-devel; +Cc: Vivek Kasireddy

On 7/3/25 01:24, dongwon.kim@intel.com wrote:
> +int virtio_gpu_object_restore_all(struct virtio_gpu_device *vgdev)
> +{
> +	struct virtio_gpu_object_restore *curr, *tmp;
> +	struct virtio_gpu_mem_entry *ents;
> +	unsigned int nents;
> +	int ret;
> +
> +	list_for_each_entry_safe(curr, tmp, &vgdev->obj_restore, node) {
> +		ret = virtio_gpu_object_shmem_init(vgdev, curr->bo, &ents, &nents);
> +		if (ret)
> +			break;
> +
> +		if (curr->params.blob) {
> +			virtio_gpu_cmd_resource_create_blob(vgdev, curr->bo, &curr->params,
> +							    ents, nents);
> +		} else if (curr->params.virgl) {
> +			virtio_gpu_cmd_resource_create_3d(vgdev, curr->bo, &curr->params,
> +							  NULL, NULL);
> +
> +			if (curr->bo->attached) {
> +				curr->bo->attached = false;
> +				virtio_gpu_object_attach(vgdev, curr->bo, ents, nents);
> +			}
> +		} else {
> +			virtio_gpu_cmd_create_resource(vgdev, curr->bo, &curr->params,
> +						       NULL, NULL);
> +			if (curr->bo->attached) {
> +				curr->bo->attached = false;
> +				virtio_gpu_object_attach(vgdev, curr->bo, ents, nents);
> +			}
> +		}
> +	}

So, back to the old question I posted on v1 about GPU reset... we need
to re-create BOs after resume because QEMU destroyed these BOs on
VirtIO-GPU reset that happens on suspend. This should be a wrong
behaiviour to begin with. We're suspending machine, hence the host
resources shouldn't disappear on resume. Can we avoid GPU resetting on
suspend?

The VQ freezing part is good to me, it pauses VirtIO-GPU gracefully on
suspend. But resetting GPU shouldn't happen and needs to be fixed, IMO.
Not doing reset should also make QEMU suspend/resume work for 3d contexts.

-- 
Best regards,
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects
  2025-07-09 13:26   ` Dmitry Osipenko
@ 2025-07-09 15:34     ` Kim, Dongwon
  2025-07-10 21:42       ` Dmitry Osipenko
  0 siblings, 1 reply; 11+ messages in thread
From: Kim, Dongwon @ 2025-07-09 15:34 UTC (permalink / raw)
  To: Dmitry Osipenko, dri-devel@lists.freedesktop.org; +Cc: Kasireddy, Vivek

Hi Dmitry,

I thought about what you are saying - avoiding GPU reset and it would work with normal
sleep and restore (s3) but the problem I saw was hibernation scenario (s4). In this case, QEMU
process will be terminated after guest hibernation and this actually destroys all the resources
anyway. So some sort of recreation seemed to be required from my point of view.

> -----Original Message-----
> From: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Sent: Wednesday, July 9, 2025 6:26 AM
> To: Kim, Dongwon <dongwon.kim@intel.com>; dri-
> devel@lists.freedesktop.org
> Cc: Kasireddy, Vivek <vivek.kasireddy@intel.com>
> Subject: Re: [PATCH v3 2/2] drm/virtio: Implement save and restore for
> virtio_gpu_objects
> 
> On 7/3/25 01:24, dongwon.kim@intel.com wrote:
> > +int virtio_gpu_object_restore_all(struct virtio_gpu_device *vgdev) {
> > +	struct virtio_gpu_object_restore *curr, *tmp;
> > +	struct virtio_gpu_mem_entry *ents;
> > +	unsigned int nents;
> > +	int ret;
> > +
> > +	list_for_each_entry_safe(curr, tmp, &vgdev->obj_restore, node) {
> > +		ret = virtio_gpu_object_shmem_init(vgdev, curr->bo, &ents,
> &nents);
> > +		if (ret)
> > +			break;
> > +
> > +		if (curr->params.blob) {
> > +			virtio_gpu_cmd_resource_create_blob(vgdev, curr-
> >bo, &curr->params,
> > +							    ents, nents);
> > +		} else if (curr->params.virgl) {
> > +			virtio_gpu_cmd_resource_create_3d(vgdev, curr->bo,
> &curr->params,
> > +							  NULL, NULL);
> > +
> > +			if (curr->bo->attached) {
> > +				curr->bo->attached = false;
> > +				virtio_gpu_object_attach(vgdev, curr->bo,
> ents, nents);
> > +			}
> > +		} else {
> > +			virtio_gpu_cmd_create_resource(vgdev, curr->bo,
> &curr->params,
> > +						       NULL, NULL);
> > +			if (curr->bo->attached) {
> > +				curr->bo->attached = false;
> > +				virtio_gpu_object_attach(vgdev, curr->bo,
> ents, nents);
> > +			}
> > +		}
> > +	}
> 
> So, back to the old question I posted on v1 about GPU reset... we need to re-
> create BOs after resume because QEMU destroyed these BOs on VirtIO-GPU
> reset that happens on suspend. This should be a wrong behaiviour to begin
> with. We're suspending machine, hence the host resources shouldn't
> disappear on resume. Can we avoid GPU resetting on suspend?
> 
> The VQ freezing part is good to me, it pauses VirtIO-GPU gracefully on
> suspend. But resetting GPU shouldn't happen and needs to be fixed, IMO.
> Not doing reset should also make QEMU suspend/resume work for 3d
> contexts.
> 
> --
> Best regards,
> Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects
  2025-07-09 15:34     ` Kim, Dongwon
@ 2025-07-10 21:42       ` Dmitry Osipenko
  2025-08-20 17:37         ` Kim, Dongwon
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2025-07-10 21:42 UTC (permalink / raw)
  To: Kim, Dongwon, dri-devel@lists.freedesktop.org; +Cc: Kasireddy, Vivek

On 7/9/25 18:34, Kim, Dongwon wrote:
> Hi Dmitry,
> 
> I thought about what you are saying - avoiding GPU reset and it would work with normal
> sleep and restore (s3) but the problem I saw was hibernation scenario (s4). In this case, QEMU
> process will be terminated after guest hibernation and this actually destroys all the resources
> anyway. So some sort of recreation seemed to be required from my point of view.

Can we add proper hibernation support/handling to the virtio-gpu driver?
I.e. restoring shmem buffers based on S4 PM_POST_HIBERNATION event and
not resetting GPU on S3. Entering S4 then should be prohibited if 3d
contexts feature enabled.

-- 
Best regards,
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects
  2025-07-10 21:42       ` Dmitry Osipenko
@ 2025-08-20 17:37         ` Kim, Dongwon
  2025-08-24 15:09           ` Dmitry Osipenko
  0 siblings, 1 reply; 11+ messages in thread
From: Kim, Dongwon @ 2025-08-20 17:37 UTC (permalink / raw)
  To: Dmitry Osipenko, dri-devel@lists.freedesktop.org; +Cc: Kasireddy, Vivek

Hi Dmitry,

Sorry, I have been busy in handling some other high priority tasks so couldn't follow up
with this. Yeah, so to confirm, you are saying this object save/restore mechanism
should only be used for S4 case (in the PM notifier for virtio-gpu driver) and handle
S3 case by not resetting virtio-gpu from QEMU?

I will take a look at that option.

Thanks!

> -----Original Message-----
> From: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Sent: Thursday, July 10, 2025 2:43 PM
> To: Kim, Dongwon <dongwon.kim@intel.com>; dri-devel@lists.freedesktop.org
> Cc: Kasireddy, Vivek <vivek.kasireddy@intel.com>
> Subject: Re: [PATCH v3 2/2] drm/virtio: Implement save and restore for
> virtio_gpu_objects
> 
> On 7/9/25 18:34, Kim, Dongwon wrote:
> > Hi Dmitry,
> >
> > I thought about what you are saying - avoiding GPU reset and it would
> > work with normal sleep and restore (s3) but the problem I saw was
> > hibernation scenario (s4). In this case, QEMU process will be
> > terminated after guest hibernation and this actually destroys all the resources
> anyway. So some sort of recreation seemed to be required from my point of
> view.
> 
> Can we add proper hibernation support/handling to the virtio-gpu driver?
> I.e. restoring shmem buffers based on S4 PM_POST_HIBERNATION event and
> not resetting GPU on S3. Entering S4 then should be prohibited if 3d contexts
> feature enabled.
> 
> --
> Best regards,
> Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects
  2025-08-20 17:37         ` Kim, Dongwon
@ 2025-08-24 15:09           ` Dmitry Osipenko
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2025-08-24 15:09 UTC (permalink / raw)
  To: Kim, Dongwon, dri-devel@lists.freedesktop.org; +Cc: Kasireddy, Vivek

Hi,

On 8/20/25 20:37, Kim, Dongwon wrote:
> Hi Dmitry,
> 
> Sorry, I have been busy in handling some other high priority tasks so couldn't follow up
> with this. Yeah, so to confirm, you are saying this object save/restore mechanism
> should only be used for S4 case (in the PM notifier for virtio-gpu driver) and handle
> S3 case by not resetting virtio-gpu from QEMU?
> 
> I will take a look at that option.

Thanks, sounds correct. Save/restore for S4. No reset for S3.

-- 
Best regards,
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-08-24 15:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-02 22:24 [PATCH v3 0/2] Virtio-GPU suspend and resume dongwon.kim
2025-07-02 22:24 ` [PATCH v3 1/2] drm/virtio: Freeze and restore hooks to support " dongwon.kim
2025-07-09 11:21   ` Dmitry Osipenko
2025-07-09 11:21   ` Dmitry Osipenko
2025-07-02 22:24 ` [PATCH v3 2/2] drm/virtio: Implement save and restore for virtio_gpu_objects dongwon.kim
2025-07-09 11:23   ` Dmitry Osipenko
2025-07-09 13:26   ` Dmitry Osipenko
2025-07-09 15:34     ` Kim, Dongwon
2025-07-10 21:42       ` Dmitry Osipenko
2025-08-20 17:37         ` Kim, Dongwon
2025-08-24 15:09           ` Dmitry Osipenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).