| Message ID | 1456508397-14193-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 26 Feb 2016 18:39:57 +0100, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > azx_probe_continue() uses pm_runtime_put_noidle() to drop the rpm > usage_count, which means that if it's the last reference the > autosuspend of the controller won't actually happen. So if the codecs > autosuspend before the azx_probe_continue() drops the last > reference we'll fail to autosuspend the controller. This does happen > in practice, but not every time. As can be seen in [1] the controller > autosuspend attempt fails due to the usage_count when suspending the > codecs. A bit later we see the the contoller usage_count dropping to > zero without further attempts at autosuspend. > > Fix the problem by using pm_runtime_put_autosuspend() instead, which > will kick off the autosuspend of the controller even if the codecs > are already asleep. As can be seen in [2] the controller autosuspend > still fails while suspending the codecs, but later on we see another > autosuspend attempt after dropping the usage_count to 0. > > I was also a bit worried that there might still be a race between the > controller autosuspend and the rest of the code in azx_probe_continue(). > So I also tried replacing the the put_noidle() with put_sync_suspend(). > No explosions occurred, so I'm somewhat satisfied that there are no > serious problems in this area. A good catch! The change should be safe, as this is the point where already all initialization has been done. I queued it to for-next branch, since this is no super-urgent issue to be fixed in 4.5. thanks, Takashi
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index e5240cb3749f..2624cfe98884 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -2145,7 +2145,7 @@ static int azx_probe_continue(struct azx *chip) azx_add_card_list(chip); snd_hda_set_power_save(&chip->bus, power_save * 1000); if (azx_has_pm_runtime(chip) || hda->use_vga_switcheroo) - pm_runtime_put_noidle(&pci->dev); + pm_runtime_put_autosuspend(&pci->dev); out_free: if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL