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