* [PATCH 1/4] drm/bochs: add drm_driver.release callback.
[not found] <20191220115935.15152-1-kraxel@redhat.com>
@ 2019-12-20 11:59 ` Gerd Hoffmann
2019-12-20 11:59 ` [PATCH 2/4] drm/cirrus: " Gerd Hoffmann
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2019-12-20 11:59 UTC (permalink / raw)
To: dri-devel
Cc: Gurchetan Singh, Gerd Hoffmann, David Airlie, Daniel Vetter,
open list:DRM DRIVER FOR BOCHS VIRTUAL GPU, open list
From: Gurchetan Singh <gurchetansingh@chromium.org>
Move bochs_unload call from bochs_remove() to the new bochs_release()
callback. Also call drm_dev_unregister() first in bochs_remove().
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
drivers/gpu/drm/bochs/bochs_drv.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bochs/bochs_drv.c b/drivers/gpu/drm/bochs/bochs_drv.c
index 10460878414e..87ee1eb21a4d 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -60,6 +60,11 @@ static int bochs_load(struct drm_device *dev)
DEFINE_DRM_GEM_FOPS(bochs_fops);
+static void bochs_release(struct drm_device *dev)
+{
+ bochs_unload(dev);
+}
+
static struct drm_driver bochs_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
.fops = &bochs_fops,
@@ -69,6 +74,7 @@ static struct drm_driver bochs_driver = {
.major = 1,
.minor = 0,
DRM_GEM_VRAM_DRIVER,
+ .release = bochs_release,
};
/* ---------------------------------------------------------------------- */
@@ -148,9 +154,8 @@ static void bochs_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- drm_atomic_helper_shutdown(dev);
drm_dev_unregister(dev);
- bochs_unload(dev);
+ drm_atomic_helper_shutdown(dev);
drm_dev_put(dev);
}
--
2.18.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] drm/cirrus: add drm_driver.release callback.
[not found] <20191220115935.15152-1-kraxel@redhat.com>
2019-12-20 11:59 ` [PATCH 1/4] drm/bochs: add drm_driver.release callback Gerd Hoffmann
@ 2019-12-20 11:59 ` Gerd Hoffmann
2019-12-20 11:59 ` [PATCH 3/4] drm/virtio: " Gerd Hoffmann
2019-12-20 11:59 ` [PATCH 4/4] drm/qxl: " Gerd Hoffmann
3 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2019-12-20 11:59 UTC (permalink / raw)
To: dri-devel
Cc: Gerd Hoffmann, Dave Airlie, David Airlie, Daniel Vetter,
open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE, open list
Move final cleanups from cirrus_pci_remove() to the new callback.
Add drm_atomic_helper_shutdown() call to cirrus_pci_remove().
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
drivers/gpu/drm/cirrus/cirrus.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 248c9f765c45..a4ca66f9a423 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -510,6 +510,16 @@ static void cirrus_mode_config_init(struct cirrus_device *cirrus)
/* ------------------------------------------------------------------ */
+static void cirrus_release(struct drm_device *dev)
+{
+ struct cirrus_device *cirrus = dev->dev_private;
+
+ drm_mode_config_cleanup(dev);
+ iounmap(cirrus->mmio);
+ iounmap(cirrus->vram);
+ kfree(cirrus);
+}
+
DEFINE_DRM_GEM_FOPS(cirrus_fops);
static struct drm_driver cirrus_driver = {
@@ -523,6 +533,7 @@ static struct drm_driver cirrus_driver = {
.fops = &cirrus_fops,
DRM_GEM_SHMEM_DRIVER_OPS,
+ .release = cirrus_release,
};
static int cirrus_pci_probe(struct pci_dev *pdev,
@@ -604,14 +615,10 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
static void cirrus_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- struct cirrus_device *cirrus = dev->dev_private;
drm_dev_unregister(dev);
- drm_mode_config_cleanup(dev);
- iounmap(cirrus->mmio);
- iounmap(cirrus->vram);
+ drm_atomic_helper_shutdown(dev);
drm_dev_put(dev);
- kfree(cirrus);
pci_release_regions(pdev);
}
--
2.18.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] drm/virtio: add drm_driver.release callback.
[not found] <20191220115935.15152-1-kraxel@redhat.com>
2019-12-20 11:59 ` [PATCH 1/4] drm/bochs: add drm_driver.release callback Gerd Hoffmann
2019-12-20 11:59 ` [PATCH 2/4] drm/cirrus: " Gerd Hoffmann
@ 2019-12-20 11:59 ` Gerd Hoffmann
2019-12-20 11:59 ` [PATCH 4/4] drm/qxl: " Gerd Hoffmann
3 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2019-12-20 11:59 UTC (permalink / raw)
To: dri-devel
Cc: Gerd Hoffmann, David Airlie, Daniel Vetter,
open list:VIRTIO GPU DRIVER, open list
Split virtio_gpu_deinit(), move the drm shutdown and release to
virtio_gpu_release(). Also free vbufs in case we can't queue them.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 1 +
drivers/gpu/drm/virtio/virtgpu_drv.c | 4 ++++
drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +++++
drivers/gpu/drm/virtio/virtgpu_vq.c | 8 ++++++--
4 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 7e69c06e168e..227f7c07e61c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -216,6 +216,7 @@ extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
/* virtio_kms.c */
int virtio_gpu_init(struct drm_device *dev);
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);
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 8cf27af3ad53..664a741a3b0b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -31,6 +31,7 @@
#include <linux/pci.h>
#include <drm/drm.h>
+#include <drm/drm_atomic_helper.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
@@ -136,6 +137,7 @@ static void virtio_gpu_remove(struct virtio_device *vdev)
struct drm_device *dev = vdev->priv;
drm_dev_unregister(dev);
+ drm_atomic_helper_shutdown(dev);
virtio_gpu_deinit(dev);
drm_dev_put(dev);
}
@@ -214,4 +216,6 @@ static struct drm_driver driver = {
.major = DRIVER_MAJOR,
.minor = DRIVER_MINOR,
.patchlevel = DRIVER_PATCHLEVEL,
+
+ .release = virtio_gpu_release,
};
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 2f5773e43557..387f05a9cc18 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -237,6 +237,11 @@ void virtio_gpu_deinit(struct drm_device *dev)
flush_work(&vgdev->config_changed_work);
vgdev->vdev->config->reset(vgdev->vdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
+}
+
+void virtio_gpu_release(struct drm_device *dev)
+{
+ struct virtio_gpu_device *vgdev = dev->dev_private;
virtio_gpu_modeset_fini(vgdev);
virtio_gpu_free_vbufs(vgdev);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5914e79d3429..619c6108d07e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -319,8 +319,10 @@ static bool virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
bool notify = false;
int ret;
- if (!vgdev->vqs_ready)
+ if (!vgdev->vqs_ready) {
+ free_vbuf(vgdev, vbuf);
return notify;
+ }
sg_init_one(&vcmd, vbuf->buf, vbuf->size);
sgs[outcnt + incnt] = &vcmd;
@@ -447,8 +449,10 @@ static void virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
int ret;
int outcnt;
- if (!vgdev->vqs_ready)
+ if (!vgdev->vqs_ready) {
+ free_vbuf(vgdev, vbuf);
return;
+ }
sg_init_one(&ccmd, vbuf->buf, vbuf->size);
sgs[0] = &ccmd;
--
2.18.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] drm/qxl: add drm_driver.release callback.
[not found] <20191220115935.15152-1-kraxel@redhat.com>
` (2 preceding siblings ...)
2019-12-20 11:59 ` [PATCH 3/4] drm/virtio: " Gerd Hoffmann
@ 2019-12-20 11:59 ` Gerd Hoffmann
2019-12-20 12:09 ` [Spice-devel] " Frediano Ziglio
3 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2019-12-20 11:59 UTC (permalink / raw)
To: dri-devel
Cc: Gerd Hoffmann, Dave Airlie, David Airlie, Daniel Vetter,
open list:DRM DRIVER FOR QXL VIRTUAL GPU,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, open list
Move final cleanups to qxl_drm_release() callback.
Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
Reorder calls in qxl_device_fini(). Cleaning up gem & ttm
might trigger qxl commands, so we should do that before
releaseing command rings.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
drivers/gpu/drm/qxl/qxl_drv.c | 21 ++++++++++++++-------
drivers/gpu/drm/qxl/qxl_kms.c | 8 ++++----
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 1d601f57a6ba..8044363ba0f2 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -34,6 +34,7 @@
#include <linux/pci.h>
#include <drm/drm.h>
+#include <drm/drm_atomic_helper.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
#include <drm/drm_modeset_helper.h>
@@ -132,21 +133,25 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
return ret;
}
+static void qxl_drm_release(struct drm_device *dev)
+{
+ struct qxl_device *qdev = dev->dev_private;
+
+ qxl_modeset_fini(qdev);
+ qxl_device_fini(qdev);
+ dev->dev_private = NULL;
+ kfree(qdev);
+}
+
static void
qxl_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- struct qxl_device *qdev = dev->dev_private;
drm_dev_unregister(dev);
-
- qxl_modeset_fini(qdev);
- qxl_device_fini(qdev);
+ drm_atomic_helper_shutdown(dev);
if (is_vga(pdev))
vga_put(pdev, VGA_RSRC_LEGACY_IO);
-
- dev->dev_private = NULL;
- kfree(qdev);
drm_dev_put(dev);
}
@@ -279,6 +284,8 @@ static struct drm_driver qxl_driver = {
.major = 0,
.minor = 1,
.patchlevel = 0,
+
+ .release = qxl_drm_release,
};
static int __init qxl_init(void)
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index bfc1631093e9..70b20ee4741a 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -299,12 +299,12 @@ void qxl_device_fini(struct qxl_device *qdev)
{
qxl_bo_unref(&qdev->current_release_bo[0]);
qxl_bo_unref(&qdev->current_release_bo[1]);
- flush_work(&qdev->gc_work);
- qxl_ring_free(qdev->command_ring);
- qxl_ring_free(qdev->cursor_ring);
- qxl_ring_free(qdev->release_ring);
qxl_gem_fini(qdev);
qxl_bo_fini(qdev);
+ flush_work(&qdev->gc_work);
+ qxl_ring_free(qdev->command_ring);
+ qxl_ring_free(qdev->cursor_ring);
+ qxl_ring_free(qdev->release_ring);
io_mapping_free(qdev->surface_mapping);
io_mapping_free(qdev->vram_mapping);
iounmap(qdev->ram_header);
--
2.18.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Spice-devel] [PATCH 4/4] drm/qxl: add drm_driver.release callback.
2019-12-20 11:59 ` [PATCH 4/4] drm/qxl: " Gerd Hoffmann
@ 2019-12-20 12:09 ` Frediano Ziglio
2019-12-20 13:00 ` Gerd Hoffmann
0 siblings, 1 reply; 6+ messages in thread
From: Frediano Ziglio @ 2019-12-20 12:09 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: dri-devel, David Airlie, open list,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Daniel Vetter,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie
>
> Move final cleanups to qxl_drm_release() callback.
Can you explain in the commit why this is better or preferable?
> Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
I suppose this is to replace the former manual cleanup calls,
which were moved to qxl_drm_release, I think this could be
added in the commit message ("why"), I don't see much value
in describing "how" this was done.
>
> Reorder calls in qxl_device_fini(). Cleaning up gem & ttm
> might trigger qxl commands, so we should do that before
> releaseing command rings.
Typo: releaseing -> releasing
Why not putting this in a separate commit? Was this behaviour
changed? It does not seem so to me.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 21 ++++++++++++++-------
> drivers/gpu/drm/qxl/qxl_kms.c | 8 ++++----
> 2 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 1d601f57a6ba..8044363ba0f2 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -34,6 +34,7 @@
> #include <linux/pci.h>
>
> #include <drm/drm.h>
> +#include <drm/drm_atomic_helper.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> #include <drm/drm_modeset_helper.h>
> @@ -132,21 +133,25 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> return ret;
> }
>
> +static void qxl_drm_release(struct drm_device *dev)
> +{
> + struct qxl_device *qdev = dev->dev_private;
> +
> + qxl_modeset_fini(qdev);
> + qxl_device_fini(qdev);
> + dev->dev_private = NULL;
> + kfree(qdev);
> +}
> +
> static void
> qxl_pci_remove(struct pci_dev *pdev)
> {
> struct drm_device *dev = pci_get_drvdata(pdev);
> - struct qxl_device *qdev = dev->dev_private;
>
> drm_dev_unregister(dev);
> -
> - qxl_modeset_fini(qdev);
> - qxl_device_fini(qdev);
> + drm_atomic_helper_shutdown(dev);
> if (is_vga(pdev))
> vga_put(pdev, VGA_RSRC_LEGACY_IO);
> -
> - dev->dev_private = NULL;
> - kfree(qdev);
> drm_dev_put(dev);
> }
>
> @@ -279,6 +284,8 @@ static struct drm_driver qxl_driver = {
> .major = 0,
> .minor = 1,
> .patchlevel = 0,
> +
> + .release = qxl_drm_release,
> };
>
> static int __init qxl_init(void)
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index bfc1631093e9..70b20ee4741a 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -299,12 +299,12 @@ void qxl_device_fini(struct qxl_device *qdev)
> {
> qxl_bo_unref(&qdev->current_release_bo[0]);
> qxl_bo_unref(&qdev->current_release_bo[1]);
> - flush_work(&qdev->gc_work);
> - qxl_ring_free(qdev->command_ring);
> - qxl_ring_free(qdev->cursor_ring);
> - qxl_ring_free(qdev->release_ring);
> qxl_gem_fini(qdev);
> qxl_bo_fini(qdev);
> + flush_work(&qdev->gc_work);
> + qxl_ring_free(qdev->command_ring);
> + qxl_ring_free(qdev->cursor_ring);
> + qxl_ring_free(qdev->release_ring);
> io_mapping_free(qdev->surface_mapping);
> io_mapping_free(qdev->vram_mapping);
> iounmap(qdev->ram_header);
Frediano
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Spice-devel] [PATCH 4/4] drm/qxl: add drm_driver.release callback.
2019-12-20 12:09 ` [Spice-devel] " Frediano Ziglio
@ 2019-12-20 13:00 ` Gerd Hoffmann
0 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2019-12-20 13:00 UTC (permalink / raw)
To: Frediano Ziglio
Cc: dri-devel, David Airlie, open list,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Daniel Vetter,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie
On Fri, Dec 20, 2019 at 07:09:20AM -0500, Frediano Ziglio wrote:
> >
> > Move final cleanups to qxl_drm_release() callback.
>
> Can you explain in the commit why this is better or preferable?
It gets called when the drm device refcount goes down to zero.
It's needed for a proper cleanup in the correct order.
> > Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
>
> I suppose this is to replace the former manual cleanup calls,
> which were moved to qxl_drm_release, I think this could be
> added in the commit message ("why"), I don't see much value
> in describing "how" this was done.
The call is part of the shutdown sequence for atomic drm drivers
and wasn't present in qxl for some reason.
> > Reorder calls in qxl_device_fini(). Cleaning up gem & ttm
> > might trigger qxl commands, so we should do that before
> > releaseing command rings.
>
> Typo: releaseing -> releasing
> Why not putting this in a separate commit? Was this behaviour
> changed? It does not seem so to me.
Yes, I can make that a separate commit.
No, behavior didn't change. qxl_device_fini() is simply broken
without this.
cheers,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread