search for: pm_runtime_put_sync

...untime_status_suspending(dev))) { > > ret = pm_runtime_get_sync(dev); > > if (ret < 0 && ret != -EACCES) > > return ret; > > } > > > > But here's the catch: This only works for an *async* runtime suspend. > > It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc, > > because then the runtime suspend is executed in the context of the caller, > > not in the context of dev->power.work. > > > > So it's not a full solution, but hopefully something that gets you > > going. I'm not really fami...

...n. Indeed, that'll be a prerequisite before we can merge power domain support. I do have a couple of local patches that add very rudimentary runtime PM for various drivers. For starters we could probably just do the pm_runtime_enable(...); pm_runtime_get_sync(...) in the ->probe() and pm_runtime_put_sync(...); pm_runtime_disable(...); in the ->remove() callbacks for those drivers. That's by no means optimal but should get us pretty close to what we do now and still support the generic power domains. Thierry -------------- next part -------------- A non-text attachment was scrubbed... Name...

...i2c->subdev.device->dev; if (!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) { ret = pm_runtime_get_sync(dev); if (ret < 0 && ret != -EACCES) return ret; } But here's the catch: This only works for an *async* runtime suspend. It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc, because then the runtime suspend is executed in the context of the caller, not in the context of dev->power.work. So it's not a full solution, but hopefully something that gets you going. I'm not really familiar with the code paths leading to nvkm_i2c_aux_a...

...@@ -360,6 +360,8 @@ nouveau_display_hpd_work(struct work_struct *work) pm_runtime_get_sync(drm->dev->dev); drm_helper_hpd_irq_event(drm->dev); + /* enable polling for external displays */ + drm_kms_helper_poll_enable(drm->dev); pm_runtime_mark_last_busy(drm->dev->dev); pm_runtime_put_sync(drm->dev->dev); @@ -413,10 +415,6 @@ nouveau_display_init(struct drm_device *dev) if (ret) return ret; - /* enable polling for external displays */ - if (!dev->mode_config.poll_enabled) - drm_kms_helper_poll_enable(dev); - /* enable hotplug interrupts */ list_for_each_entry(con...

As it turns out, on cards that actually have CRTCs on them we're already calling drm_kms_helper_poll_enable(drm_dev) from nouveau_display_resume() before we call it in nouveau_pmops_runtime_resume(). This leads us to accidentally trying to enable polling twice, which results in a potential deadlock between the RPM locks and drm_dev->mode_config.mutex if we end up trying to enable polling

...schedule_work(hpd_work) # <--- ... nouveau_display_hpd_work pm_runtime_get_sync drm_helper_hpd_irq_event pm_runtime_mark_last_busy pm_runtime_put_sync I'm curious about that "schedule_work(hpd_work)" near the end because no other drivers seem to use schedule_work() in the runtime_resume path, and I don't know how that synchronizes with the shutdown process. I don't see anything that waits for nouveau_display_hpd_work() to...

...0x81 +#endif + +static void +nouveau_display_acpi_work(struct work_struct *work) +{ + struct nouveau_drm *drm = container_of(work, typeof(*drm), acpi_work); + + pm_runtime_get_sync(drm->dev->dev); + + drm_helper_hpd_irq_event(drm->dev); + + pm_runtime_mark_last_busy(drm->dev->dev); + pm_runtime_put_sync(drm->dev->dev); +} + +static int +nouveau_display_acpi_ntfy(struct notifier_block *nb, unsigned long val, + void *data) +{ + struct nouveau_drm *drm = container_of(nb, typeof(*drm), acpi_nb); + struct acpi_bus_event *info = data; + + if (!strcmp(info->device_class, ACPI_VIDEO_CLASS)) {...

...e we can merge power domain > support. I do have a couple of local patches that add very rudimentary > runtime PM for various drivers. For starters we could probably just do > the > > pm_runtime_enable(...); > pm_runtime_get_sync(...) > > in the ->probe() and > > pm_runtime_put_sync(...); > pm_runtime_disable(...); > > in the ->remove() callbacks for those drivers. That's by no means > optimal but should get us pretty close to what we do now and still > support the generic power domains. Cool. Could you send me the patches? Thanks, Vince > > Thie...

...@@ -355,8 +355,6 @@ nouveau_display_hpd_work(struct work_struct *work) pm_runtime_get_sync(drm->dev->dev); drm_helper_hpd_irq_event(drm->dev); - /* enable polling for external displays */ - drm_kms_helper_poll_enable(drm->dev); pm_runtime_mark_last_busy(drm->dev->dev); pm_runtime_put_sync(drm->dev->dev); @@ -411,6 +409,11 @@ nouveau_display_init(struct drm_device *dev) if (ret) return ret; + /* enable connector detection and polling for connectors without HPD + * support + */ + drm_kms_helper_poll_enable(dev); + /* enable hotplug interrupts */ drm_connector_list_i...

...gt; > > ret = pm_runtime_get_sync(dev); > > > if (ret < 0 && ret != -EACCES) > > > return ret; > > > } > > > > > > But here's the catch: This only works for an *async* runtime suspend. > > > It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc, > > > because then the runtime suspend is executed in the context of the caller, > > > not in the context of dev->power.work. > > > > > > So it's not a full solution, but hopefully something that gets you > > > going....

...on_accel_restore(drm->dev); + drm_fb_helper_set_suspend(&drm->fbcon->helper, state); + if (state != FBINFO_STATE_RUNNING) + nouveau_fbcon_accel_save_disable(drm->dev); + console_unlock(); + + if (state == FBINFO_STATE_RUNNING) { + pm_runtime_mark_last_busy(drm->dev->dev); + pm_runtime_put_sync(drm->dev->dev); + } +} + void nouveau_fbcon_set_suspend(struct drm_device *dev, int state) { struct nouveau_drm *drm = nouveau_drm(dev); - if (drm->fbcon) { - console_lock(); - if (state == FBINFO_STATE_RUNNING) - nouveau_fbcon_accel_restore(dev); - drm_fb_helper_set_suspend(&amp...

While working on refactoring how we handle connector hotplugging/probing + a bunch of misc DP stuff, I found a bunch of runtime_pm errors that were mostly introduced by me at one point. Oops. A lot of these aren't triggered consistently on a lot of systems, so it's not terribly surprising these got swept under the rug. Lyude Paul (9): drm/nouveau/kms: Handle -EINPROGRESS in

...(!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) { > ret = pm_runtime_get_sync(dev); > if (ret < 0 && ret != -EACCES) > return ret; > } > > But here's the catch: This only works for an *async* runtime suspend. > It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc, > because then the runtime suspend is executed in the context of the caller, > not in the context of dev->power.work. > > So it's not a full solution, but hopefully something that gets you > going. I'm not really familiar with the code paths...

...ding(dev))) { > ret = pm_runtime_get_sync(dev); > if (ret < 0 && ret != -EACCES) > return ret; > } > > But here's the catch: This only works for an *async* runtime suspend. > It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc, > because then the runtime suspend is executed in the context of the caller, > not in the context of dev->power.work. > > So it's not a full solution, but hopefully something that gets you > going. I'm not really familiar with the code paths l...

...+366,7 @@ void sun8i_ce_cipher_exit(struct crypto_tfm *tfm) { struct sun8i_cipher_tfm_ctx *op = crypto_tfm_ctx(tfm); - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); crypto_free_sync_skcipher(op->fallback_tfm); pm_runtime_put_sync_suspend(op->ce->dev); } @@ -391,10 +388,7 @@ int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, dev_dbg(ce->dev, "ERROR: Invalid keylen %u\n", keylen); return -EINVAL; } - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->...

...+366,7 @@ void sun8i_ce_cipher_exit(struct crypto_tfm *tfm) { struct sun8i_cipher_tfm_ctx *op = crypto_tfm_ctx(tfm); - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op->key); - } + kfree_sensitive(op->key); crypto_free_sync_skcipher(op->fallback_tfm); pm_runtime_put_sync_suspend(op->ce->dev); } @@ -391,14 +388,11 @@ int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm, const u8 *key, dev_dbg(ce->dev, "ERROR: Invalid keylen %u\n", keylen); return -EINVAL; } - if (op->key) { - memzero_explicit(op->key, op->keylen); - kfree(op-&gt...

...work) # <--- > ... > nouveau_display_hpd_work > pm_runtime_get_sync > drm_helper_hpd_irq_event > pm_runtime_mark_last_busy > pm_runtime_put_sync > > I'm curious about that "schedule_work(hpd_work)" near the end because > no other drivers seem to use schedule_work() in the runtime_resume > path, and I don't know how that synchronizes with the shutdown > process. I don't see anything that waits for >...

On Wed, Jan 07, 2015 at 02:27:10PM +0100, Thierry Reding wrote: > > Yeah. I plan to have the information of all the clock client of the > > partitions and > > the memory clients be defined statically in c source, e.g. pmc-tegra124.c. > > All modules can declare which domain they belong to in DT. One domain can > > be really power gated only when no module is awake.

...acpi_work(struct work_struct *work) > +{ > + struct nouveau_drm *drm = container_of(work, typeof(*drm), acpi_work); > + > + pm_runtime_get_sync(drm->dev->dev); > + > + drm_helper_hpd_irq_event(drm->dev); > + > + pm_runtime_mark_last_busy(drm->dev->dev); > + pm_runtime_put_sync(drm->dev->dev); Nothing depends on the device being suspended immediately, I guess you can drop _sync and also use: pm_runtime_put_autosuspend Kind regards, Peter > +} > + > +static int > +nouveau_display_acpi_ntfy(struct notifier_block *nb, unsigned long val, > + v...

Latest version of https://patchwork.freedesktop.org/series/46815/ , with one small change re: ilia Lyude Paul (5): drm/nouveau: Fix bogus drm_kms_helper_poll_enable() placement drm/nouveau: Remove duplicate poll_enable() in pmops_runtime_suspend() drm/nouveau: Fix deadlock with fb_helper with async RPM requests drm/nouveau: Use pm_runtime_get_noresume() in connector_detect()