With cmwq, there's no reason for nouveau to use a dedicated workqueue.
Drop dev_priv->wq and use system_wq instead.  Each work item is sync
flushed when the containing structure is unregistered/destroyed.

Note that this change also makes sure that nv50_gpio_handler is not
freed while the contained work item is still running.

Signed-off-by: Tejun Heo <t...@kernel.org>
Cc: David Airlie <airl...@linux.ie>
Cc: dri-devel@lists.freedesktop.org
---
Here's a patch on top of the current linus#master.  It's much simpler
than before.  The only concern is that it adds flush_work_sync() call
which might sleep to unregistration paths.  AFAICS, this seems safe,
right?  If this looks okay to you, please feel free to route it
through the drm tree.

Thank you.

 drivers/gpu/drm/nouveau/nouveau_drv.h   |    1 -
 drivers/gpu/drm/nouveau/nouveau_state.c |   10 +---------
 drivers/gpu/drm/nouveau/nv50_display.c  |    5 ++++-
 drivers/gpu/drm/nouveau/nv50_gpio.c     |   11 ++++++++---
 4 files changed, 13 insertions(+), 14 deletions(-)

Index: work/drivers/gpu/drm/nouveau/nouveau_drv.h
===================================================================
--- work.orig/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ work/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -652,7 +652,6 @@ struct drm_nouveau_private {
        /* interrupt handling */
        void (*irq_handler[32])(struct drm_device *);
        bool msi_enabled;
-       struct workqueue_struct *wq;
        struct work_struct irq_work;
 
        struct list_head vbl_waiting;
Index: work/drivers/gpu/drm/nouveau/nouveau_state.c
===================================================================
--- work.orig/drivers/gpu/drm/nouveau/nouveau_state.c
+++ work/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -929,12 +929,6 @@ int nouveau_load(struct drm_device *dev,
        NV_DEBUG(dev, "vendor: 0x%X device: 0x%X class: 0x%X\n",
                 dev->pci_vendor, dev->pci_device, dev->pdev->class);
 
-       dev_priv->wq = create_workqueue("nouveau");
-       if (!dev_priv->wq) {
-               ret = -EINVAL;
-               goto err_priv;
-       }
-
        /* resource 0 is mmio regs */
        /* resource 1 is linear FB */
        /* resource 2 is RAMIN (mmio regs + 0x1000000) */
@@ -947,7 +941,7 @@ int nouveau_load(struct drm_device *dev,
                NV_ERROR(dev, "Unable to initialize the mmio mapping. "
                         "Please report your setup to " DRIVER_EMAIL "\n");
                ret = -EINVAL;
-               goto err_wq;
+               goto err_priv;
        }
        NV_DEBUG(dev, "regs mapped ok at 0x%llx\n",
                                        (unsigned long long)mmio_start_offs);
@@ -1054,8 +1048,6 @@ err_ramin:
        iounmap(dev_priv->ramin);
 err_mmio:
        iounmap(dev_priv->mmio);
-err_wq:
-       destroy_workqueue(dev_priv->wq);
 err_priv:
        kfree(dev_priv);
        dev->dev_private = NULL;
Index: work/drivers/gpu/drm/nouveau/nv50_display.c
===================================================================
--- work.orig/drivers/gpu/drm/nouveau/nv50_display.c
+++ work/drivers/gpu/drm/nouveau/nv50_display.c
@@ -345,12 +345,15 @@ int nv50_display_create(struct drm_devic
 void
 nv50_display_destroy(struct drm_device *dev)
 {
+       struct drm_nouveau_private *dev_priv = dev->dev_private;
+
        NV_DEBUG_KMS(dev, "\n");
 
        drm_mode_config_cleanup(dev);
 
        nv50_display_disable(dev);
        nouveau_irq_unregister(dev, 26);
+       flush_work_sync(&dev_priv->irq_work);
 }
 
 static u16
@@ -836,7 +839,7 @@ nv50_display_isr(struct drm_device *dev)
                if (clock) {
                        nv_wr32(dev, NV03_PMC_INTR_EN_0, 0);
                        if (!work_pending(&dev_priv->irq_work))
-                               queue_work(dev_priv->wq, &dev_priv->irq_work);
+                               schedule_work(&dev_priv->irq_work);
                        delayed |= clock;
                        intr1 &= ~clock;
                }
Index: work/drivers/gpu/drm/nouveau/nv50_gpio.c
===================================================================
--- work.orig/drivers/gpu/drm/nouveau/nv50_gpio.c
+++ work/drivers/gpu/drm/nouveau/nv50_gpio.c
@@ -137,6 +137,7 @@ nv50_gpio_irq_unregister(struct drm_devi
        struct nv50_gpio_priv *priv = pgpio->priv;
        struct nv50_gpio_handler *gpioh, *tmp;
        struct dcb_gpio_entry *gpio;
+       LIST_HEAD(tofree);
        unsigned long flags;
 
        gpio = nouveau_bios_gpio_entry(dev, tag);
@@ -149,10 +150,14 @@ nv50_gpio_irq_unregister(struct drm_devi
                    gpioh->handler != handler ||
                    gpioh->data != data)
                        continue;
-               list_del(&gpioh->head);
-               kfree(gpioh);
+               list_move(&gpioh->head, &tofree);
        }
        spin_unlock_irqrestore(&priv->lock, flags);
+
+       list_for_each_entry_safe(gpioh, tmp, &tofree, head) {
+               flush_work_sync(&gpioh->work);
+               kfree(gpioh);
+       }
 }
 
 bool
@@ -293,7 +298,7 @@ nv50_gpio_isr(struct drm_device *dev)
                        continue;
                gpioh->inhibit = true;
 
-               queue_work(dev_priv->wq, &gpioh->work);
+               schedule_work(&gpioh->work);
        }
        spin_unlock(&priv->lock);
 }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to