* [PATCH 08/32] drm/nouveau: use system_wq instead of dev_priv->wq [not found] <1294062595-30097-1-git-send-email-tj@kernel.org> @ 2011-01-03 13:49 ` Tejun Heo 2011-01-05 1:07 ` Ben Skeggs 2011-01-03 13:49 ` [PATCH 09/32] drm/radeon: " Tejun Heo 1 sibling, 1 reply; 11+ messages in thread From: Tejun Heo @ 2011-01-03 13:49 UTC (permalink / raw) To: linux-kernel; +Cc: Tejun Heo, dri-devel With cmwq, there's no reason for nouveau to use a dedicated workqueue. Drop dev_priv->wq and use system_wq instead. Because nouveau_irq_uninstall() may be called from unsleepable context, the work items can't be flushed from there. Instead, init and flush from nouveau_load/unload(). Signed-off-by: Tejun Heo <tj@kernel.org> Cc: David Airlie <airlied@linux.ie> Cc: dri-devel@lists.freedesktop.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/gpu/drm/nouveau/nouveau_drv.h | 1 - drivers/gpu/drm/nouveau/nouveau_irq.c | 9 --------- drivers/gpu/drm/nouveau/nouveau_state.c | 19 ++++++++++--------- drivers/gpu/drm/nouveau/nv50_display.c | 4 ++-- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h index 1c7db64..2ecf875 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drv.h +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h @@ -580,7 +580,6 @@ struct drm_nouveau_private { struct nouveau_bo *vga_ram; - struct workqueue_struct *wq; struct work_struct irq_work; struct work_struct hpd_work; diff --git a/drivers/gpu/drm/nouveau/nouveau_irq.c b/drivers/gpu/drm/nouveau/nouveau_irq.c index 7bfd9e6..7d05a06 100644 --- a/drivers/gpu/drm/nouveau/nouveau_irq.c +++ b/drivers/gpu/drm/nouveau/nouveau_irq.c @@ -52,17 +52,8 @@ static int nouveau_ratelimit(void) void nouveau_irq_preinstall(struct drm_device *dev) { - struct drm_nouveau_private *dev_priv = dev->dev_private; - /* Master disable */ nv_wr32(dev, NV03_PMC_INTR_EN_0, 0); - - if (dev_priv->card_type >= NV_50) { - INIT_WORK(&dev_priv->irq_work, nv50_display_irq_handler_bh); - INIT_WORK(&dev_priv->hpd_work, nv50_display_irq_hotplug_bh); - spin_lock_init(&dev_priv->hpd_state.lock); - INIT_LIST_HEAD(&dev_priv->vbl_waiting); - } } int diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c index 049f755..2eea6ea 100644 --- a/drivers/gpu/drm/nouveau/nouveau_state.c +++ b/drivers/gpu/drm/nouveau/nouveau_state.c @@ -839,17 +839,17 @@ int nouveau_load(struct drm_device *dev, unsigned long flags) dev->dev_private = dev_priv; dev_priv->dev = dev; + /* the followings are used only by >= NV_50 */ + INIT_WORK(&dev_priv->irq_work, nv50_display_irq_handler_bh); + INIT_WORK(&dev_priv->hpd_work, nv50_display_irq_hotplug_bh); + spin_lock_init(&dev_priv->hpd_state.lock); + INIT_LIST_HEAD(&dev_priv->vbl_waiting); + dev_priv->flags = flags & NOUVEAU_FLAGS; 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) */ @@ -862,7 +862,7 @@ int nouveau_load(struct drm_device *dev, unsigned long flags) 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); @@ -969,8 +969,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; @@ -992,6 +990,9 @@ int nouveau_unload(struct drm_device *dev) engine->display.destroy(dev); nouveau_card_takedown(dev); + flush_work_sync(&dev_priv->irq_work); + flush_work_sync(&dev_priv->hpd_work); + iounmap(dev_priv->mmio); iounmap(dev_priv->ramin); diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index f624c61..3d569da 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -1110,7 +1110,7 @@ nv50_display_irq_handler(struct drm_device *dev) dev_priv->hpd_state.hpd1_bits |= hpd1_bits; spin_unlock(&dev_priv->hpd_state.lock); - queue_work(dev_priv->wq, &dev_priv->hpd_work); + schedule_work(&dev_priv->hpd_work); } while (nv_rd32(dev, NV50_PMC_INTR_0) & NV50_PMC_INTR_0_DISPLAY) { @@ -1139,7 +1139,7 @@ nv50_display_irq_handler(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; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 08/32] drm/nouveau: use system_wq instead of dev_priv->wq 2011-01-03 13:49 ` [PATCH 08/32] drm/nouveau: use system_wq instead of dev_priv->wq Tejun Heo @ 2011-01-05 1:07 ` Ben Skeggs 2011-01-05 1:16 ` Ben Skeggs 0 siblings, 1 reply; 11+ messages in thread From: Ben Skeggs @ 2011-01-05 1:07 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-kernel, dri-devel On Mon, 2011-01-03 at 14:49 +0100, Tejun Heo wrote: > With cmwq, there's no reason for nouveau to use a dedicated workqueue. > Drop dev_priv->wq and use system_wq instead. > > Because nouveau_irq_uninstall() may be called from unsleepable > context, the work items can't be flushed from there. Instead, init > and flush from nouveau_load/unload(). Ehh, ok, why not! I'll push this through the nouveau tree, and it'll get to Dave from there. Thanks! Ben. > > Signed-off-by: Tejun Heo <tj@kernel.org> > Cc: David Airlie <airlied@linux.ie> > Cc: dri-devel@lists.freedesktop.org > --- > Only compile tested. Please feel free to take it into the subsystem > tree or simply ack - I'll route it through the wq tree. > > Thanks. > > drivers/gpu/drm/nouveau/nouveau_drv.h | 1 - > drivers/gpu/drm/nouveau/nouveau_irq.c | 9 --------- > drivers/gpu/drm/nouveau/nouveau_state.c | 19 ++++++++++--------- > drivers/gpu/drm/nouveau/nv50_display.c | 4 ++-- > 4 files changed, 12 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h > index 1c7db64..2ecf875 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drv.h > +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h > @@ -580,7 +580,6 @@ struct drm_nouveau_private { > > struct nouveau_bo *vga_ram; > > - struct workqueue_struct *wq; > struct work_struct irq_work; > struct work_struct hpd_work; > > diff --git a/drivers/gpu/drm/nouveau/nouveau_irq.c b/drivers/gpu/drm/nouveau/nouveau_irq.c > index 7bfd9e6..7d05a06 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_irq.c > +++ b/drivers/gpu/drm/nouveau/nouveau_irq.c > @@ -52,17 +52,8 @@ static int nouveau_ratelimit(void) > void > nouveau_irq_preinstall(struct drm_device *dev) > { > - struct drm_nouveau_private *dev_priv = dev->dev_private; > - > /* Master disable */ > nv_wr32(dev, NV03_PMC_INTR_EN_0, 0); > - > - if (dev_priv->card_type >= NV_50) { > - INIT_WORK(&dev_priv->irq_work, nv50_display_irq_handler_bh); > - INIT_WORK(&dev_priv->hpd_work, nv50_display_irq_hotplug_bh); > - spin_lock_init(&dev_priv->hpd_state.lock); > - INIT_LIST_HEAD(&dev_priv->vbl_waiting); > - } > } > > int > diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c > index 049f755..2eea6ea 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_state.c > +++ b/drivers/gpu/drm/nouveau/nouveau_state.c > @@ -839,17 +839,17 @@ int nouveau_load(struct drm_device *dev, unsigned long flags) > dev->dev_private = dev_priv; > dev_priv->dev = dev; > > + /* the followings are used only by >= NV_50 */ > + INIT_WORK(&dev_priv->irq_work, nv50_display_irq_handler_bh); > + INIT_WORK(&dev_priv->hpd_work, nv50_display_irq_hotplug_bh); > + spin_lock_init(&dev_priv->hpd_state.lock); > + INIT_LIST_HEAD(&dev_priv->vbl_waiting); > + > dev_priv->flags = flags & NOUVEAU_FLAGS; > > 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) */ > @@ -862,7 +862,7 @@ int nouveau_load(struct drm_device *dev, unsigned long flags) > 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); > @@ -969,8 +969,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; > @@ -992,6 +990,9 @@ int nouveau_unload(struct drm_device *dev) > engine->display.destroy(dev); > nouveau_card_takedown(dev); > > + flush_work_sync(&dev_priv->irq_work); > + flush_work_sync(&dev_priv->hpd_work); > + > iounmap(dev_priv->mmio); > iounmap(dev_priv->ramin); > > diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c > index f624c61..3d569da 100644 > --- a/drivers/gpu/drm/nouveau/nv50_display.c > +++ b/drivers/gpu/drm/nouveau/nv50_display.c > @@ -1110,7 +1110,7 @@ nv50_display_irq_handler(struct drm_device *dev) > dev_priv->hpd_state.hpd1_bits |= hpd1_bits; > spin_unlock(&dev_priv->hpd_state.lock); > > - queue_work(dev_priv->wq, &dev_priv->hpd_work); > + schedule_work(&dev_priv->hpd_work); > } > > while (nv_rd32(dev, NV50_PMC_INTR_0) & NV50_PMC_INTR_0_DISPLAY) { > @@ -1139,7 +1139,7 @@ nv50_display_irq_handler(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; > } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 08/32] drm/nouveau: use system_wq instead of dev_priv->wq 2011-01-05 1:07 ` Ben Skeggs @ 2011-01-05 1:16 ` Ben Skeggs 2011-01-06 17:29 ` Tejun Heo 0 siblings, 1 reply; 11+ messages in thread From: Ben Skeggs @ 2011-01-05 1:16 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-kernel, dri-devel On Wed, 2011-01-05 at 11:07 +1000, Ben Skeggs wrote: > On Mon, 2011-01-03 at 14:49 +0100, Tejun Heo wrote: > > With cmwq, there's no reason for nouveau to use a dedicated workqueue. > > Drop dev_priv->wq and use system_wq instead. > > > > Because nouveau_irq_uninstall() may be called from unsleepable > > context, the work items can't be flushed from there. Instead, init > > and flush from nouveau_load/unload(). > Ehh, ok, why not! I'll push this through the nouveau tree, and it'll > get to Dave from there. On second thoughts, this won't apply on top of current nouveau code that's queued for 2.6.38. Can you rebase on top of Dave's drm-next tree please. Ben. > > Thanks! > Ben. > > > > Signed-off-by: Tejun Heo <tj@kernel.org> > > Cc: David Airlie <airlied@linux.ie> > > Cc: dri-devel@lists.freedesktop.org > > --- > > Only compile tested. Please feel free to take it into the subsystem > > tree or simply ack - I'll route it through the wq tree. > > > > Thanks. > > > > drivers/gpu/drm/nouveau/nouveau_drv.h | 1 - > > drivers/gpu/drm/nouveau/nouveau_irq.c | 9 --------- > > drivers/gpu/drm/nouveau/nouveau_state.c | 19 ++++++++++--------- > > drivers/gpu/drm/nouveau/nv50_display.c | 4 ++-- > > 4 files changed, 12 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h > > index 1c7db64..2ecf875 100644 > > --- a/drivers/gpu/drm/nouveau/nouveau_drv.h > > +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h > > @@ -580,7 +580,6 @@ struct drm_nouveau_private { > > > > struct nouveau_bo *vga_ram; > > > > - struct workqueue_struct *wq; > > struct work_struct irq_work; > > struct work_struct hpd_work; > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_irq.c b/drivers/gpu/drm/nouveau/nouveau_irq.c > > index 7bfd9e6..7d05a06 100644 > > --- a/drivers/gpu/drm/nouveau/nouveau_irq.c > > +++ b/drivers/gpu/drm/nouveau/nouveau_irq.c > > @@ -52,17 +52,8 @@ static int nouveau_ratelimit(void) > > void > > nouveau_irq_preinstall(struct drm_device *dev) > > { > > - struct drm_nouveau_private *dev_priv = dev->dev_private; > > - > > /* Master disable */ > > nv_wr32(dev, NV03_PMC_INTR_EN_0, 0); > > - > > - if (dev_priv->card_type >= NV_50) { > > - INIT_WORK(&dev_priv->irq_work, nv50_display_irq_handler_bh); > > - INIT_WORK(&dev_priv->hpd_work, nv50_display_irq_hotplug_bh); > > - spin_lock_init(&dev_priv->hpd_state.lock); > > - INIT_LIST_HEAD(&dev_priv->vbl_waiting); > > - } > > } > > > > int > > diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c > > index 049f755..2eea6ea 100644 > > --- a/drivers/gpu/drm/nouveau/nouveau_state.c > > +++ b/drivers/gpu/drm/nouveau/nouveau_state.c > > @@ -839,17 +839,17 @@ int nouveau_load(struct drm_device *dev, unsigned long flags) > > dev->dev_private = dev_priv; > > dev_priv->dev = dev; > > > > + /* the followings are used only by >= NV_50 */ > > + INIT_WORK(&dev_priv->irq_work, nv50_display_irq_handler_bh); > > + INIT_WORK(&dev_priv->hpd_work, nv50_display_irq_hotplug_bh); > > + spin_lock_init(&dev_priv->hpd_state.lock); > > + INIT_LIST_HEAD(&dev_priv->vbl_waiting); > > + > > dev_priv->flags = flags & NOUVEAU_FLAGS; > > > > 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) */ > > @@ -862,7 +862,7 @@ int nouveau_load(struct drm_device *dev, unsigned long flags) > > 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); > > @@ -969,8 +969,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; > > @@ -992,6 +990,9 @@ int nouveau_unload(struct drm_device *dev) > > engine->display.destroy(dev); > > nouveau_card_takedown(dev); > > > > + flush_work_sync(&dev_priv->irq_work); > > + flush_work_sync(&dev_priv->hpd_work); > > + > > iounmap(dev_priv->mmio); > > iounmap(dev_priv->ramin); > > > > diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c > > index f624c61..3d569da 100644 > > --- a/drivers/gpu/drm/nouveau/nv50_display.c > > +++ b/drivers/gpu/drm/nouveau/nv50_display.c > > @@ -1110,7 +1110,7 @@ nv50_display_irq_handler(struct drm_device *dev) > > dev_priv->hpd_state.hpd1_bits |= hpd1_bits; > > spin_unlock(&dev_priv->hpd_state.lock); > > > > - queue_work(dev_priv->wq, &dev_priv->hpd_work); > > + schedule_work(&dev_priv->hpd_work); > > } > > > > while (nv_rd32(dev, NV50_PMC_INTR_0) & NV50_PMC_INTR_0_DISPLAY) { > > @@ -1139,7 +1139,7 @@ nv50_display_irq_handler(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; > > } > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 08/32] drm/nouveau: use system_wq instead of dev_priv->wq 2011-01-05 1:16 ` Ben Skeggs @ 2011-01-06 17:29 ` Tejun Heo 2011-01-26 16:49 ` [PATCH UPDATED " Tejun Heo 0 siblings, 1 reply; 11+ messages in thread From: Tejun Heo @ 2011-01-06 17:29 UTC (permalink / raw) To: Ben Skeggs; +Cc: linux-kernel, dri-devel Hello, On Wed, Jan 05, 2011 at 11:16:05AM +1000, Ben Skeggs wrote: > On Wed, 2011-01-05 at 11:07 +1000, Ben Skeggs wrote: > > On Mon, 2011-01-03 at 14:49 +0100, Tejun Heo wrote: > > > With cmwq, there's no reason for nouveau to use a dedicated workqueue. > > > Drop dev_priv->wq and use system_wq instead. > > > > > > Because nouveau_irq_uninstall() may be called from unsleepable > > > context, the work items can't be flushed from there. Instead, init > > > and flush from nouveau_load/unload(). > > Ehh, ok, why not! I'll push this through the nouveau tree, and it'll > > get to Dave from there. > > On second thoughts, this won't apply on top of current nouveau code > that's queued for 2.6.38. Can you rebase on top of Dave's drm-next tree > please. We already missed this merge window, so I'll refresh the patch once the window is closed and resend. Thank you. -- tejun ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH UPDATED 08/32] drm/nouveau: use system_wq instead of dev_priv->wq 2011-01-06 17:29 ` Tejun Heo @ 2011-01-26 16:49 ` Tejun Heo 2011-02-01 10:41 ` Tejun Heo 0 siblings, 1 reply; 11+ messages in thread From: Tejun Heo @ 2011-01-26 16:49 UTC (permalink / raw) To: Ben Skeggs; +Cc: linux-kernel, dri-devel 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 <tj@kernel.org> Cc: David Airlie <airlied@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); } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH UPDATED 08/32] drm/nouveau: use system_wq instead of dev_priv->wq 2011-01-26 16:49 ` [PATCH UPDATED " Tejun Heo @ 2011-02-01 10:41 ` Tejun Heo 2011-02-04 1:53 ` Ben Skeggs 0 siblings, 1 reply; 11+ messages in thread From: Tejun Heo @ 2011-02-01 10:41 UTC (permalink / raw) To: Ben Skeggs; +Cc: linux-kernel, dri-devel On Wed, Jan 26, 2011 at 05:49:18PM +0100, Tejun Heo wrote: > 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 <tj@kernel.org> > Cc: David Airlie <airlied@linux.ie> > Cc: dri-devel@lists.freedesktop.org Ping. Can you please put this through the drm tree? Thank you. -- tejun ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH UPDATED 08/32] drm/nouveau: use system_wq instead of dev_priv->wq 2011-02-01 10:41 ` Tejun Heo @ 2011-02-04 1:53 ` Ben Skeggs 2011-02-04 11:03 ` Tejun Heo 0 siblings, 1 reply; 11+ messages in thread From: Ben Skeggs @ 2011-02-04 1:53 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-kernel, dri-devel On Tue, 2011-02-01 at 11:41 +0100, Tejun Heo wrote: > On Wed, Jan 26, 2011 at 05:49:18PM +0100, Tejun Heo wrote: > > 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 <tj@kernel.org> > > Cc: David Airlie <airlied@linux.ie> > > Cc: dri-devel@lists.freedesktop.org > > Ping. Can you please put this through the drm tree? Hey Tejun, Thanks for this again. I've just made some changes to the nv50 display bottom half handling that these will conflict with, they'll hit the nouveau tree next week. I'll push the still-relevant bits of your nouveau patch through the nouveau tree for the 2.6.39-rc1 merge window. That ok? Thanks, Ben. > > Thank you. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH UPDATED 08/32] drm/nouveau: use system_wq instead of dev_priv->wq 2011-02-04 1:53 ` Ben Skeggs @ 2011-02-04 11:03 ` Tejun Heo 0 siblings, 0 replies; 11+ messages in thread From: Tejun Heo @ 2011-02-04 11:03 UTC (permalink / raw) To: Ben Skeggs; +Cc: linux-kernel, dri-devel Hello, On Fri, Feb 04, 2011 at 11:53:25AM +1000, Ben Skeggs wrote: > Thanks for this again. I've just made some changes to the nv50 display > bottom half handling that these will conflict with, they'll hit the > nouveau tree next week. Eh, so it doesn't apply again? :-) > I'll push the still-relevant bits of your nouveau patch through the > nouveau tree for the 2.6.39-rc1 merge window. That ok? Yeah, sure, but if you tell me against which tree I should generate patch, I can also just redo the patch. Thanks. -- tejun ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 09/32] drm/radeon: use system_wq instead of dev_priv->wq [not found] <1294062595-30097-1-git-send-email-tj@kernel.org> 2011-01-03 13:49 ` [PATCH 08/32] drm/nouveau: use system_wq instead of dev_priv->wq Tejun Heo @ 2011-01-03 13:49 ` Tejun Heo 2011-01-05 0:21 ` Alex Deucher 1 sibling, 1 reply; 11+ messages in thread From: Tejun Heo @ 2011-01-03 13:49 UTC (permalink / raw) To: linux-kernel; +Cc: Tejun Heo, dri-devel With cmwq, there's no reason for radeon to use a dedicated workqueue. Drop dev_priv->wq and use system_wq instead. Because radeon_driver_irq_uninstall_kms() may be called from unsleepable context, the work items can't be flushed from there. Instead, init and flush from radeon_irq_kms_init/fini(). While at it, simplify canceling/flushing of rdev->pm.dynpm_idle_work. Always initialize and sync cancel instead of being unnecessarily smart about it. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: David Airlie <airlied@linux.ie> Cc: dri-devel@lists.freedesktop.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/gpu/drm/radeon/evergreen.c | 2 +- drivers/gpu/drm/radeon/r100.c | 2 +- drivers/gpu/drm/radeon/r600.c | 2 +- drivers/gpu/drm/radeon/radeon.h | 1 - drivers/gpu/drm/radeon/radeon_device.c | 6 ---- drivers/gpu/drm/radeon/radeon_irq_kms.c | 5 ++- drivers/gpu/drm/radeon/radeon_pm.c | 47 ++++++++++-------------------- drivers/gpu/drm/radeon/rs600.c | 2 +- 8 files changed, 23 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 7b337c3..6540adb 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2516,7 +2516,7 @@ restart_ih: if (wptr != rdev->ih.wptr) goto restart_ih; if (queue_hotplug) - queue_work(rdev->wq, &rdev->hotplug_work); + schedule_work(&rdev->hotplug_work); rdev->ih.rptr = rptr; WREG32(IH_RB_RPTR, rdev->ih.rptr); spin_unlock_irqrestore(&rdev->ih.lock, flags); diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c index 8e10aa9..1d15748 100644 --- a/drivers/gpu/drm/radeon/r100.c +++ b/drivers/gpu/drm/radeon/r100.c @@ -622,7 +622,7 @@ int r100_irq_process(struct radeon_device *rdev) /* reset gui idle ack. the status bit is broken */ rdev->irq.gui_idle_acked = false; if (queue_hotplug) - queue_work(rdev->wq, &rdev->hotplug_work); + schedule_work(&rdev->hotplug_work); if (rdev->msi_enabled) { switch (rdev->family) { case CHIP_RS400: diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index 9c92db7..f927cd4 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -3419,7 +3419,7 @@ restart_ih: if (wptr != rdev->ih.wptr) goto restart_ih; if (queue_hotplug) - queue_work(rdev->wq, &rdev->hotplug_work); + schedule_work(&rdev->hotplug_work); rdev->ih.rptr = rptr; WREG32(IH_RB_RPTR, rdev->ih.rptr); spin_unlock_irqrestore(&rdev->ih.lock, flags); diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 3a70957..ba233a8 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1095,7 +1095,6 @@ struct radeon_device { struct r700_vram_scratch vram_scratch; int msi_enabled; /* msi enabled */ struct r600_ih ih; /* r6/700 interrupt ring */ - struct workqueue_struct *wq; struct work_struct hotplug_work; int num_crtc; /* number of crtcs */ struct mutex dc_hw_i2c_mutex; /* display controller hw i2c mutex */ diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index 501966a..4bc0012 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -704,11 +704,6 @@ int radeon_device_init(struct radeon_device *rdev, init_waitqueue_head(&rdev->irq.vblank_queue); init_waitqueue_head(&rdev->irq.idle_queue); - /* setup workqueue */ - rdev->wq = create_workqueue("radeon"); - if (rdev->wq == NULL) - return -ENOMEM; - /* Set asic functions */ r = radeon_asic_init(rdev); if (r) @@ -806,7 +801,6 @@ void radeon_device_fini(struct radeon_device *rdev) /* evict vram memory */ radeon_bo_evict_vram(rdev); radeon_fini(rdev); - destroy_workqueue(rdev->wq); vga_switcheroo_unregister_client(rdev->pdev); vga_client_register(rdev->pdev, NULL, NULL, NULL); if (rdev->rio_mem) diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c index a108c7e..33b9d21 100644 --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c @@ -64,8 +64,6 @@ void radeon_driver_irq_preinstall_kms(struct drm_device *dev) struct radeon_device *rdev = dev->dev_private; unsigned i; - INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); - /* Disable *all* interrupts */ rdev->irq.sw_int = false; rdev->irq.gui_idle = false; @@ -110,6 +108,8 @@ int radeon_irq_kms_init(struct radeon_device *rdev) { int r = 0; + INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); + spin_lock_init(&rdev->irq.sw_lock); r = drm_vblank_init(rdev->ddev, rdev->num_crtc); if (r) { @@ -148,6 +148,7 @@ void radeon_irq_kms_fini(struct radeon_device *rdev) if (rdev->msi_enabled) pci_disable_msi(rdev->pdev); } + flush_work_sync(&rdev->hotplug_work); } void radeon_irq_kms_sw_irq_get(struct radeon_device *rdev) diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index 8c9b2ef..845f295 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -405,20 +405,13 @@ static ssize_t radeon_set_pm_method(struct device *dev, rdev->pm.dynpm_planned_action = DYNPM_ACTION_DEFAULT; mutex_unlock(&rdev->pm.mutex); } else if (strncmp("profile", buf, strlen("profile")) == 0) { - bool flush_wq = false; - mutex_lock(&rdev->pm.mutex); - if (rdev->pm.pm_method == PM_METHOD_DYNPM) { - cancel_delayed_work(&rdev->pm.dynpm_idle_work); - flush_wq = true; - } /* disable dynpm */ rdev->pm.dynpm_state = DYNPM_STATE_DISABLED; rdev->pm.dynpm_planned_action = DYNPM_ACTION_NONE; rdev->pm.pm_method = PM_METHOD_PROFILE; mutex_unlock(&rdev->pm.mutex); - if (flush_wq) - flush_workqueue(rdev->wq); + cancel_delayed_work_sync(&rdev->pm.dynpm_idle_work); } else { DRM_ERROR("invalid power method!\n"); goto fail; @@ -520,18 +513,14 @@ static void radeon_hwmon_fini(struct radeon_device *rdev) void radeon_pm_suspend(struct radeon_device *rdev) { - bool flush_wq = false; - mutex_lock(&rdev->pm.mutex); if (rdev->pm.pm_method == PM_METHOD_DYNPM) { - cancel_delayed_work(&rdev->pm.dynpm_idle_work); if (rdev->pm.dynpm_state == DYNPM_STATE_ACTIVE) rdev->pm.dynpm_state = DYNPM_STATE_SUSPENDED; - flush_wq = true; } mutex_unlock(&rdev->pm.mutex); - if (flush_wq) - flush_workqueue(rdev->wq); + + cancel_delayed_work_sync(&rdev->pm.dynpm_idle_work); } void radeon_pm_resume(struct radeon_device *rdev) @@ -546,8 +535,8 @@ void radeon_pm_resume(struct radeon_device *rdev) if (rdev->pm.pm_method == PM_METHOD_DYNPM && rdev->pm.dynpm_state == DYNPM_STATE_SUSPENDED) { rdev->pm.dynpm_state = DYNPM_STATE_ACTIVE; - queue_delayed_work(rdev->wq, &rdev->pm.dynpm_idle_work, - msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); + schedule_delayed_work(&rdev->pm.dynpm_idle_work, + msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); } mutex_unlock(&rdev->pm.mutex); radeon_pm_compute_clocks(rdev); @@ -581,6 +570,9 @@ int radeon_pm_init(struct radeon_device *rdev) ret = radeon_hwmon_init(rdev); if (ret) return ret; + + INIT_DELAYED_WORK(&rdev->pm.dynpm_idle_work, radeon_dynpm_idle_work_handler); + if (rdev->pm.num_power_states > 1) { /* where's the best place to put these? */ ret = device_create_file(rdev->dev, &dev_attr_power_profile); @@ -594,8 +586,6 @@ int radeon_pm_init(struct radeon_device *rdev) rdev->acpi_nb.notifier_call = radeon_acpi_event; register_acpi_notifier(&rdev->acpi_nb); #endif - INIT_DELAYED_WORK(&rdev->pm.dynpm_idle_work, radeon_dynpm_idle_work_handler); - if (radeon_debugfs_pm_init(rdev)) { DRM_ERROR("Failed to register debugfs file for PM!\n"); } @@ -609,25 +599,20 @@ int radeon_pm_init(struct radeon_device *rdev) void radeon_pm_fini(struct radeon_device *rdev) { if (rdev->pm.num_power_states > 1) { - bool flush_wq = false; - mutex_lock(&rdev->pm.mutex); if (rdev->pm.pm_method == PM_METHOD_PROFILE) { rdev->pm.profile = PM_PROFILE_DEFAULT; radeon_pm_update_profile(rdev); radeon_pm_set_clocks(rdev); } else if (rdev->pm.pm_method == PM_METHOD_DYNPM) { - /* cancel work */ - cancel_delayed_work(&rdev->pm.dynpm_idle_work); - flush_wq = true; /* reset default clocks */ rdev->pm.dynpm_state = DYNPM_STATE_DISABLED; rdev->pm.dynpm_planned_action = DYNPM_ACTION_DEFAULT; radeon_pm_set_clocks(rdev); } mutex_unlock(&rdev->pm.mutex); - if (flush_wq) - flush_workqueue(rdev->wq); + + cancel_delayed_work_sync(&rdev->pm.dynpm_idle_work); device_remove_file(rdev->dev, &dev_attr_power_profile); device_remove_file(rdev->dev, &dev_attr_power_method); @@ -686,12 +671,12 @@ void radeon_pm_compute_clocks(struct radeon_device *rdev) radeon_pm_get_dynpm_state(rdev); radeon_pm_set_clocks(rdev); - queue_delayed_work(rdev->wq, &rdev->pm.dynpm_idle_work, - msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); + schedule_delayed_work(&rdev->pm.dynpm_idle_work, + msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); } else if (rdev->pm.dynpm_state == DYNPM_STATE_PAUSED) { rdev->pm.dynpm_state = DYNPM_STATE_ACTIVE; - queue_delayed_work(rdev->wq, &rdev->pm.dynpm_idle_work, - msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); + schedule_delayed_work(&rdev->pm.dynpm_idle_work, + msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); DRM_DEBUG_DRIVER("radeon: dynamic power management activated\n"); } } else { /* count == 0 */ @@ -796,8 +781,8 @@ static void radeon_dynpm_idle_work_handler(struct work_struct *work) radeon_pm_set_clocks(rdev); } - queue_delayed_work(rdev->wq, &rdev->pm.dynpm_idle_work, - msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); + schedule_delayed_work(&rdev->pm.dynpm_idle_work, + msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); } mutex_unlock(&rdev->pm.mutex); ttm_bo_unlock_delayed_workqueue(&rdev->mman.bdev, resched); diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c index f1c6e02..2d707d4 100644 --- a/drivers/gpu/drm/radeon/rs600.c +++ b/drivers/gpu/drm/radeon/rs600.c @@ -634,7 +634,7 @@ int rs600_irq_process(struct radeon_device *rdev) /* reset gui idle ack. the status bit is broken */ rdev->irq.gui_idle_acked = false; if (queue_hotplug) - queue_work(rdev->wq, &rdev->hotplug_work); + schedule_work(&rdev->hotplug_work); if (rdev->msi_enabled) { switch (rdev->family) { case CHIP_RS600: -- 1.7.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 09/32] drm/radeon: use system_wq instead of dev_priv->wq 2011-01-03 13:49 ` [PATCH 09/32] drm/radeon: " Tejun Heo @ 2011-01-05 0:21 ` Alex Deucher 2011-01-06 4:31 ` Dave Airlie 0 siblings, 1 reply; 11+ messages in thread From: Alex Deucher @ 2011-01-05 0:21 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-kernel, dri-devel On Mon, Jan 3, 2011 at 8:49 AM, Tejun Heo <tj@kernel.org> wrote: > With cmwq, there's no reason for radeon to use a dedicated workqueue. > Drop dev_priv->wq and use system_wq instead. > > Because radeon_driver_irq_uninstall_kms() may be called from > unsleepable context, the work items can't be flushed from there. > Instead, init and flush from radeon_irq_kms_init/fini(). > > While at it, simplify canceling/flushing of rdev->pm.dynpm_idle_work. > Always initialize and sync cancel instead of being unnecessarily smart > about it. > > Signed-off-by: Tejun Heo <tj@kernel.org> > Cc: David Airlie <airlied@linux.ie> > Cc: dri-devel@lists.freedesktop.org > --- > Only compile tested. Please feel free to take it into the subsystem > tree or simply ack - I'll route it through the wq tree. Patch looks good to me. I'm not sure what's the best way to send this upstream. I'm working on some irq changes in the same area now, so I'd prefer if we pushed it through Dave's tree, but I can handle it either way. Acked-by: Alex Deucher <alexdeucher@gmail.com> Alex > > Thanks. > > drivers/gpu/drm/radeon/evergreen.c | 2 +- > drivers/gpu/drm/radeon/r100.c | 2 +- > drivers/gpu/drm/radeon/r600.c | 2 +- > drivers/gpu/drm/radeon/radeon.h | 1 - > drivers/gpu/drm/radeon/radeon_device.c | 6 ---- > drivers/gpu/drm/radeon/radeon_irq_kms.c | 5 ++- > drivers/gpu/drm/radeon/radeon_pm.c | 47 ++++++++++-------------------- > drivers/gpu/drm/radeon/rs600.c | 2 +- > 8 files changed, 23 insertions(+), 44 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c > index 7b337c3..6540adb 100644 > --- a/drivers/gpu/drm/radeon/evergreen.c > +++ b/drivers/gpu/drm/radeon/evergreen.c > @@ -2516,7 +2516,7 @@ restart_ih: > if (wptr != rdev->ih.wptr) > goto restart_ih; > if (queue_hotplug) > - queue_work(rdev->wq, &rdev->hotplug_work); > + schedule_work(&rdev->hotplug_work); > rdev->ih.rptr = rptr; > WREG32(IH_RB_RPTR, rdev->ih.rptr); > spin_unlock_irqrestore(&rdev->ih.lock, flags); > diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c > index 8e10aa9..1d15748 100644 > --- a/drivers/gpu/drm/radeon/r100.c > +++ b/drivers/gpu/drm/radeon/r100.c > @@ -622,7 +622,7 @@ int r100_irq_process(struct radeon_device *rdev) > /* reset gui idle ack. the status bit is broken */ > rdev->irq.gui_idle_acked = false; > if (queue_hotplug) > - queue_work(rdev->wq, &rdev->hotplug_work); > + schedule_work(&rdev->hotplug_work); > if (rdev->msi_enabled) { > switch (rdev->family) { > case CHIP_RS400: > diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c > index 9c92db7..f927cd4 100644 > --- a/drivers/gpu/drm/radeon/r600.c > +++ b/drivers/gpu/drm/radeon/r600.c > @@ -3419,7 +3419,7 @@ restart_ih: > if (wptr != rdev->ih.wptr) > goto restart_ih; > if (queue_hotplug) > - queue_work(rdev->wq, &rdev->hotplug_work); > + schedule_work(&rdev->hotplug_work); > rdev->ih.rptr = rptr; > WREG32(IH_RB_RPTR, rdev->ih.rptr); > spin_unlock_irqrestore(&rdev->ih.lock, flags); > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index 3a70957..ba233a8 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -1095,7 +1095,6 @@ struct radeon_device { > struct r700_vram_scratch vram_scratch; > int msi_enabled; /* msi enabled */ > struct r600_ih ih; /* r6/700 interrupt ring */ > - struct workqueue_struct *wq; > struct work_struct hotplug_work; > int num_crtc; /* number of crtcs */ > struct mutex dc_hw_i2c_mutex; /* display controller hw i2c mutex */ > diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c > index 501966a..4bc0012 100644 > --- a/drivers/gpu/drm/radeon/radeon_device.c > +++ b/drivers/gpu/drm/radeon/radeon_device.c > @@ -704,11 +704,6 @@ int radeon_device_init(struct radeon_device *rdev, > init_waitqueue_head(&rdev->irq.vblank_queue); > init_waitqueue_head(&rdev->irq.idle_queue); > > - /* setup workqueue */ > - rdev->wq = create_workqueue("radeon"); > - if (rdev->wq == NULL) > - return -ENOMEM; > - > /* Set asic functions */ > r = radeon_asic_init(rdev); > if (r) > @@ -806,7 +801,6 @@ void radeon_device_fini(struct radeon_device *rdev) > /* evict vram memory */ > radeon_bo_evict_vram(rdev); > radeon_fini(rdev); > - destroy_workqueue(rdev->wq); > vga_switcheroo_unregister_client(rdev->pdev); > vga_client_register(rdev->pdev, NULL, NULL, NULL); > if (rdev->rio_mem) > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c > index a108c7e..33b9d21 100644 > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c > @@ -64,8 +64,6 @@ void radeon_driver_irq_preinstall_kms(struct drm_device *dev) > struct radeon_device *rdev = dev->dev_private; > unsigned i; > > - INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); > - > /* Disable *all* interrupts */ > rdev->irq.sw_int = false; > rdev->irq.gui_idle = false; > @@ -110,6 +108,8 @@ int radeon_irq_kms_init(struct radeon_device *rdev) > { > int r = 0; > > + INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); > + > spin_lock_init(&rdev->irq.sw_lock); > r = drm_vblank_init(rdev->ddev, rdev->num_crtc); > if (r) { > @@ -148,6 +148,7 @@ void radeon_irq_kms_fini(struct radeon_device *rdev) > if (rdev->msi_enabled) > pci_disable_msi(rdev->pdev); > } > + flush_work_sync(&rdev->hotplug_work); > } > > void radeon_irq_kms_sw_irq_get(struct radeon_device *rdev) > diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c > index 8c9b2ef..845f295 100644 > --- a/drivers/gpu/drm/radeon/radeon_pm.c > +++ b/drivers/gpu/drm/radeon/radeon_pm.c > @@ -405,20 +405,13 @@ static ssize_t radeon_set_pm_method(struct device *dev, > rdev->pm.dynpm_planned_action = DYNPM_ACTION_DEFAULT; > mutex_unlock(&rdev->pm.mutex); > } else if (strncmp("profile", buf, strlen("profile")) == 0) { > - bool flush_wq = false; > - > mutex_lock(&rdev->pm.mutex); > - if (rdev->pm.pm_method == PM_METHOD_DYNPM) { > - cancel_delayed_work(&rdev->pm.dynpm_idle_work); > - flush_wq = true; > - } > /* disable dynpm */ > rdev->pm.dynpm_state = DYNPM_STATE_DISABLED; > rdev->pm.dynpm_planned_action = DYNPM_ACTION_NONE; > rdev->pm.pm_method = PM_METHOD_PROFILE; > mutex_unlock(&rdev->pm.mutex); > - if (flush_wq) > - flush_workqueue(rdev->wq); > + cancel_delayed_work_sync(&rdev->pm.dynpm_idle_work); > } else { > DRM_ERROR("invalid power method!\n"); > goto fail; > @@ -520,18 +513,14 @@ static void radeon_hwmon_fini(struct radeon_device *rdev) > > void radeon_pm_suspend(struct radeon_device *rdev) > { > - bool flush_wq = false; > - > mutex_lock(&rdev->pm.mutex); > if (rdev->pm.pm_method == PM_METHOD_DYNPM) { > - cancel_delayed_work(&rdev->pm.dynpm_idle_work); > if (rdev->pm.dynpm_state == DYNPM_STATE_ACTIVE) > rdev->pm.dynpm_state = DYNPM_STATE_SUSPENDED; > - flush_wq = true; > } > mutex_unlock(&rdev->pm.mutex); > - if (flush_wq) > - flush_workqueue(rdev->wq); > + > + cancel_delayed_work_sync(&rdev->pm.dynpm_idle_work); > } > > void radeon_pm_resume(struct radeon_device *rdev) > @@ -546,8 +535,8 @@ void radeon_pm_resume(struct radeon_device *rdev) > if (rdev->pm.pm_method == PM_METHOD_DYNPM > && rdev->pm.dynpm_state == DYNPM_STATE_SUSPENDED) { > rdev->pm.dynpm_state = DYNPM_STATE_ACTIVE; > - queue_delayed_work(rdev->wq, &rdev->pm.dynpm_idle_work, > - msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); > + schedule_delayed_work(&rdev->pm.dynpm_idle_work, > + msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); > } > mutex_unlock(&rdev->pm.mutex); > radeon_pm_compute_clocks(rdev); > @@ -581,6 +570,9 @@ int radeon_pm_init(struct radeon_device *rdev) > ret = radeon_hwmon_init(rdev); > if (ret) > return ret; > + > + INIT_DELAYED_WORK(&rdev->pm.dynpm_idle_work, radeon_dynpm_idle_work_handler); > + > if (rdev->pm.num_power_states > 1) { > /* where's the best place to put these? */ > ret = device_create_file(rdev->dev, &dev_attr_power_profile); > @@ -594,8 +586,6 @@ int radeon_pm_init(struct radeon_device *rdev) > rdev->acpi_nb.notifier_call = radeon_acpi_event; > register_acpi_notifier(&rdev->acpi_nb); > #endif > - INIT_DELAYED_WORK(&rdev->pm.dynpm_idle_work, radeon_dynpm_idle_work_handler); > - > if (radeon_debugfs_pm_init(rdev)) { > DRM_ERROR("Failed to register debugfs file for PM!\n"); > } > @@ -609,25 +599,20 @@ int radeon_pm_init(struct radeon_device *rdev) > void radeon_pm_fini(struct radeon_device *rdev) > { > if (rdev->pm.num_power_states > 1) { > - bool flush_wq = false; > - > mutex_lock(&rdev->pm.mutex); > if (rdev->pm.pm_method == PM_METHOD_PROFILE) { > rdev->pm.profile = PM_PROFILE_DEFAULT; > radeon_pm_update_profile(rdev); > radeon_pm_set_clocks(rdev); > } else if (rdev->pm.pm_method == PM_METHOD_DYNPM) { > - /* cancel work */ > - cancel_delayed_work(&rdev->pm.dynpm_idle_work); > - flush_wq = true; > /* reset default clocks */ > rdev->pm.dynpm_state = DYNPM_STATE_DISABLED; > rdev->pm.dynpm_planned_action = DYNPM_ACTION_DEFAULT; > radeon_pm_set_clocks(rdev); > } > mutex_unlock(&rdev->pm.mutex); > - if (flush_wq) > - flush_workqueue(rdev->wq); > + > + cancel_delayed_work_sync(&rdev->pm.dynpm_idle_work); > > device_remove_file(rdev->dev, &dev_attr_power_profile); > device_remove_file(rdev->dev, &dev_attr_power_method); > @@ -686,12 +671,12 @@ void radeon_pm_compute_clocks(struct radeon_device *rdev) > radeon_pm_get_dynpm_state(rdev); > radeon_pm_set_clocks(rdev); > > - queue_delayed_work(rdev->wq, &rdev->pm.dynpm_idle_work, > - msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); > + schedule_delayed_work(&rdev->pm.dynpm_idle_work, > + msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); > } else if (rdev->pm.dynpm_state == DYNPM_STATE_PAUSED) { > rdev->pm.dynpm_state = DYNPM_STATE_ACTIVE; > - queue_delayed_work(rdev->wq, &rdev->pm.dynpm_idle_work, > - msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); > + schedule_delayed_work(&rdev->pm.dynpm_idle_work, > + msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); > DRM_DEBUG_DRIVER("radeon: dynamic power management activated\n"); > } > } else { /* count == 0 */ > @@ -796,8 +781,8 @@ static void radeon_dynpm_idle_work_handler(struct work_struct *work) > radeon_pm_set_clocks(rdev); > } > > - queue_delayed_work(rdev->wq, &rdev->pm.dynpm_idle_work, > - msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); > + schedule_delayed_work(&rdev->pm.dynpm_idle_work, > + msecs_to_jiffies(RADEON_IDLE_LOOP_MS)); > } > mutex_unlock(&rdev->pm.mutex); > ttm_bo_unlock_delayed_workqueue(&rdev->mman.bdev, resched); > diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c > index f1c6e02..2d707d4 100644 > --- a/drivers/gpu/drm/radeon/rs600.c > +++ b/drivers/gpu/drm/radeon/rs600.c > @@ -634,7 +634,7 @@ int rs600_irq_process(struct radeon_device *rdev) > /* reset gui idle ack. the status bit is broken */ > rdev->irq.gui_idle_acked = false; > if (queue_hotplug) > - queue_work(rdev->wq, &rdev->hotplug_work); > + schedule_work(&rdev->hotplug_work); > if (rdev->msi_enabled) { > switch (rdev->family) { > case CHIP_RS600: > -- > 1.7.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 09/32] drm/radeon: use system_wq instead of dev_priv->wq 2011-01-05 0:21 ` Alex Deucher @ 2011-01-06 4:31 ` Dave Airlie 0 siblings, 0 replies; 11+ messages in thread From: Dave Airlie @ 2011-01-06 4:31 UTC (permalink / raw) To: Alex Deucher; +Cc: Tejun Heo, linux-kernel, dri-devel On Tue, 2011-01-04 at 19:21 -0500, Alex Deucher wrote: > On Mon, Jan 3, 2011 at 8:49 AM, Tejun Heo <tj@kernel.org> wrote: > > With cmwq, there's no reason for radeon to use a dedicated workqueue. > > Drop dev_priv->wq and use system_wq instead. > > > > Because radeon_driver_irq_uninstall_kms() may be called from > > unsleepable context, the work items can't be flushed from there. > > Instead, init and flush from radeon_irq_kms_init/fini(). > > > > While at it, simplify canceling/flushing of rdev->pm.dynpm_idle_work. > > Always initialize and sync cancel instead of being unnecessarily smart > > about it. > > > > Signed-off-by: Tejun Heo <tj@kernel.org> > > Cc: David Airlie <airlied@linux.ie> > > Cc: dri-devel@lists.freedesktop.org > > --- > > Only compile tested. Please feel free to take it into the subsystem > > tree or simply ack - I'll route it through the wq tree. > > Patch looks good to me. I'm not sure what's the best way to send this > upstream. I'm working on some irq changes in the same area now, so > I'd prefer if we pushed it through Dave's tree, but I can handle it > either way. I'll pull it into my tree for merging to Linus. Dave. > > Acked-by: Alex Deucher <alexdeucher@gmail.com> > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-02-04 11:03 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1294062595-30097-1-git-send-email-tj@kernel.org>
2011-01-03 13:49 ` [PATCH 08/32] drm/nouveau: use system_wq instead of dev_priv->wq Tejun Heo
2011-01-05 1:07 ` Ben Skeggs
2011-01-05 1:16 ` Ben Skeggs
2011-01-06 17:29 ` Tejun Heo
2011-01-26 16:49 ` [PATCH UPDATED " Tejun Heo
2011-02-01 10:41 ` Tejun Heo
2011-02-04 1:53 ` Ben Skeggs
2011-02-04 11:03 ` Tejun Heo
2011-01-03 13:49 ` [PATCH 09/32] drm/radeon: " Tejun Heo
2011-01-05 0:21 ` Alex Deucher
2011-01-06 4:31 ` Dave Airlie
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).