| Message ID | 20210203131615.1714021-6-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/qxl: fix driver shutdown issues. | expand |
Am 03.02.21 um 14:16 schrieb Gerd Hoffmann: > Reorganize qxl_device_fini() a bit. > Add missing unpin() calls. > > Count releases. Add wait queue for releases. That way > qxl_device_fini() can easily wait until everything is > ready for proper shutdown. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/qxl/qxl_drv.h | 2 ++ > drivers/gpu/drm/qxl/qxl_cmd.c | 1 + > drivers/gpu/drm/qxl/qxl_irq.c | 1 + > drivers/gpu/drm/qxl/qxl_kms.c | 22 ++++++++++++++++++++-- > drivers/gpu/drm/qxl/qxl_release.c | 2 ++ > 5 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h > index 01354b43c413..6dd57cfb2e7c 100644 > --- a/drivers/gpu/drm/qxl/qxl_drv.h > +++ b/drivers/gpu/drm/qxl/qxl_drv.h > @@ -214,6 +214,8 @@ struct qxl_device { > spinlock_t release_lock; > struct idr release_idr; > uint32_t release_seqno; > + atomic_t release_count; > + wait_queue_head_t release_event; > spinlock_t release_idr_lock; > struct mutex async_io_mutex; > unsigned int last_sent_io_cmd; > diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c > index 54e3c3a97440..7e22a81bfb36 100644 > --- a/drivers/gpu/drm/qxl/qxl_cmd.c > +++ b/drivers/gpu/drm/qxl/qxl_cmd.c > @@ -254,6 +254,7 @@ int qxl_garbage_collect(struct qxl_device *qdev) > } > } > > + wake_up_all(&qdev->release_event); > DRM_DEBUG_DRIVER("%d\n", i); > > return i; > diff --git a/drivers/gpu/drm/qxl/qxl_irq.c b/drivers/gpu/drm/qxl/qxl_irq.c > index ddf6588a2a38..d312322cacd1 100644 > --- a/drivers/gpu/drm/qxl/qxl_irq.c > +++ b/drivers/gpu/drm/qxl/qxl_irq.c > @@ -87,6 +87,7 @@ int qxl_irq_init(struct qxl_device *qdev) > init_waitqueue_head(&qdev->display_event); > init_waitqueue_head(&qdev->cursor_event); > init_waitqueue_head(&qdev->io_cmd_event); > + init_waitqueue_head(&qdev->release_event); > INIT_WORK(&qdev->client_monitors_config_work, > qxl_client_monitors_config_work_func); > atomic_set(&qdev->irq_received, 0); > diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c > index 4a60a52ab62e..616aea948863 100644 > --- a/drivers/gpu/drm/qxl/qxl_kms.c > +++ b/drivers/gpu/drm/qxl/qxl_kms.c > @@ -286,8 +286,26 @@ int qxl_device_init(struct qxl_device *qdev, > > void qxl_device_fini(struct qxl_device *qdev) > { > - qxl_bo_unref(&qdev->current_release_bo[0]); > - qxl_bo_unref(&qdev->current_release_bo[1]); > + int cur_idx; > + > + for (cur_idx = 0; cur_idx < 3; cur_idx++) { > + if (!qdev->current_release_bo[cur_idx]) > + continue; > + qxl_bo_unpin(qdev->current_release_bo[cur_idx]); > + qxl_bo_unref(&qdev->current_release_bo[cur_idx]); > + qdev->current_release_bo_offset[cur_idx] = 0; > + qdev->current_release_bo[cur_idx] = NULL; > + } > + > + /* > + * Ask host to release resources (+fill release ring), > + * then wait for the release actually happening. > + */ > + qxl_io_notify_oom(qdev); > + wait_event_timeout(qdev->release_event, > + atomic_read(&qdev->release_count) == 0, > + HZ); > + > qxl_gem_fini(qdev); > qxl_bo_fini(qdev); > flush_work(&qdev->gc_work); > diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c > index 28013fd1f8ea..43a5436853b7 100644 > --- a/drivers/gpu/drm/qxl/qxl_release.c > +++ b/drivers/gpu/drm/qxl/qxl_release.c > @@ -196,6 +196,7 @@ qxl_release_free(struct qxl_device *qdev, > qxl_release_free_list(release); > kfree(release); > } > + atomic_dec(&qdev->release_count); > } > > static int qxl_release_bo_alloc(struct qxl_device *qdev, > @@ -344,6 +345,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev, unsigned long size, > *rbo = NULL; > return idr_ret; > } > + atomic_inc(&qdev->release_count); > > mutex_lock(&qdev->release_mutex); > if (qdev->current_release_bo_offset[cur_idx] + 1 >= releases_per_bo[cur_idx]) { >
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index 01354b43c413..6dd57cfb2e7c 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -214,6 +214,8 @@ struct qxl_device { spinlock_t release_lock; struct idr release_idr; uint32_t release_seqno; + atomic_t release_count; + wait_queue_head_t release_event; spinlock_t release_idr_lock; struct mutex async_io_mutex; unsigned int last_sent_io_cmd; diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c index 54e3c3a97440..7e22a81bfb36 100644 --- a/drivers/gpu/drm/qxl/qxl_cmd.c +++ b/drivers/gpu/drm/qxl/qxl_cmd.c @@ -254,6 +254,7 @@ int qxl_garbage_collect(struct qxl_device *qdev) } } + wake_up_all(&qdev->release_event); DRM_DEBUG_DRIVER("%d\n", i); return i; diff --git a/drivers/gpu/drm/qxl/qxl_irq.c b/drivers/gpu/drm/qxl/qxl_irq.c index ddf6588a2a38..d312322cacd1 100644 --- a/drivers/gpu/drm/qxl/qxl_irq.c +++ b/drivers/gpu/drm/qxl/qxl_irq.c @@ -87,6 +87,7 @@ int qxl_irq_init(struct qxl_device *qdev) init_waitqueue_head(&qdev->display_event); init_waitqueue_head(&qdev->cursor_event); init_waitqueue_head(&qdev->io_cmd_event); + init_waitqueue_head(&qdev->release_event); INIT_WORK(&qdev->client_monitors_config_work, qxl_client_monitors_config_work_func); atomic_set(&qdev->irq_received, 0); diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index 4a60a52ab62e..616aea948863 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -286,8 +286,26 @@ int qxl_device_init(struct qxl_device *qdev, void qxl_device_fini(struct qxl_device *qdev) { - qxl_bo_unref(&qdev->current_release_bo[0]); - qxl_bo_unref(&qdev->current_release_bo[1]); + int cur_idx; + + for (cur_idx = 0; cur_idx < 3; cur_idx++) { + if (!qdev->current_release_bo[cur_idx]) + continue; + qxl_bo_unpin(qdev->current_release_bo[cur_idx]); + qxl_bo_unref(&qdev->current_release_bo[cur_idx]); + qdev->current_release_bo_offset[cur_idx] = 0; + qdev->current_release_bo[cur_idx] = NULL; + } + + /* + * Ask host to release resources (+fill release ring), + * then wait for the release actually happening. + */ + qxl_io_notify_oom(qdev); + wait_event_timeout(qdev->release_event, + atomic_read(&qdev->release_count) == 0, + HZ); + qxl_gem_fini(qdev); qxl_bo_fini(qdev); flush_work(&qdev->gc_work); diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c index 28013fd1f8ea..43a5436853b7 100644 --- a/drivers/gpu/drm/qxl/qxl_release.c +++ b/drivers/gpu/drm/qxl/qxl_release.c @@ -196,6 +196,7 @@ qxl_release_free(struct qxl_device *qdev, qxl_release_free_list(release); kfree(release); } + atomic_dec(&qdev->release_count); } static int qxl_release_bo_alloc(struct qxl_device *qdev, @@ -344,6 +345,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev, unsigned long size, *rbo = NULL; return idr_ret; } + atomic_inc(&qdev->release_count); mutex_lock(&qdev->release_mutex); if (qdev->current_release_bo_offset[cur_idx] + 1 >= releases_per_bo[cur_idx]) {
Reorganize qxl_device_fini() a bit. Add missing unpin() calls. Count releases. Add wait queue for releases. That way qxl_device_fini() can easily wait until everything is ready for proper shutdown. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- drivers/gpu/drm/qxl/qxl_drv.h | 2 ++ drivers/gpu/drm/qxl/qxl_cmd.c | 1 + drivers/gpu/drm/qxl/qxl_irq.c | 1 + drivers/gpu/drm/qxl/qxl_kms.c | 22 ++++++++++++++++++++-- drivers/gpu/drm/qxl/qxl_release.c | 2 ++ 5 files changed, 26 insertions(+), 2 deletions(-)