On Fri, 24 Apr 2020 14:22:10 +0200, Sasha Levin wrote: > > From: Takashi Iwai <tiwai@xxxxxxx> > > [ Upstream commit c4c8dd6ef807663e42a5f04ea77cd62029eb99fa ] > > The HD-audio controller does system-suspend and resume operations by > directly calling its helpers __azx_runtime_suspend() and > __azx_runtime_resume(). However, in general, we don't have to resume > always the device fully at the system resume; typically, if a device > has been runtime-suspended, we can leave it to runtime resume. > > Usually for achieving this, the driver would call > pm_runtime_force_suspend() and pm_runtime_force_resume() pairs in the > system suspend and resume ops. Unfortunately, this doesn't work for > the resume path in our case. For handling the jack detection at the > system resume, a child codec device may need the (literally) forcibly > resume even if it's been runtime-suspended, and for that, the > controller device must be also resumed even if it's been suspended. > > This patch is an attempt to improve the situation. It replaces the > direct __azx_runtime_suspend()/_resume() calls with with > pm_runtime_force_suspend() and pm_runtime_force_resume() with a slight > trick as we've done for the codec side. More exactly: > > - azx_has_pm_runtime() check is dropped from azx_runtime_suspend() and > azx_runtime_resume(), so that it can be properly executed from the > system-suspend/resume path > > - The WAKEEN handling depends on the card's power state now; it's set > and cleared only for the runtime-suspend > > - azx_resume() checks whether any codec may need the forcible resume > beforehand. If the forcible resume is required, it does temporary > PM refcount up/down for actually triggering the runtime resume. > > - A new helper function, hda_codec_need_resume(), is introduced for > checking whether the codec needs a forcible runtime-resume, and the > existing code is rewritten with that. > > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=207043 > Link: https://lore.kernel.org/r/20200413082034.25166-6-tiwai@xxxxxxx > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> This commit is known to cause a regression, and the fix patch is included in today's pull request. If we apply this, better to wait for the next batch including its fix. thanks, Takashi > --- > include/sound/hda_codec.h | 5 +++++ > sound/pci/hda/hda_codec.c | 2 +- > sound/pci/hda/hda_intel.c | 38 +++++++++++++++++++++++++++----------- > 3 files changed, 33 insertions(+), 12 deletions(-) > > diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h > index 3ee8036f5436d..225154a4f2ed0 100644 > --- a/include/sound/hda_codec.h > +++ b/include/sound/hda_codec.h > @@ -494,6 +494,11 @@ void snd_hda_update_power_acct(struct hda_codec *codec); > static inline void snd_hda_set_power_save(struct hda_bus *bus, int delay) {} > #endif > > +static inline bool hda_codec_need_resume(struct hda_codec *codec) > +{ > + return !codec->relaxed_resume && codec->jacktbl.used; > +} > + > #ifdef CONFIG_SND_HDA_PATCH_LOADER > /* > * patch firmware > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c > index 53e7732ef7520..aed1f8188e662 100644 > --- a/sound/pci/hda/hda_codec.c > +++ b/sound/pci/hda/hda_codec.c > @@ -2951,7 +2951,7 @@ static int hda_codec_runtime_resume(struct device *dev) > static int hda_codec_force_resume(struct device *dev) > { > struct hda_codec *codec = dev_to_hda_codec(dev); > - bool forced_resume = !codec->relaxed_resume && codec->jacktbl.used; > + bool forced_resume = hda_codec_need_resume(codec); > int ret; > > /* The get/put pair below enforces the runtime resume even if the > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c > index aa0be85614b6c..02c6308502b1e 100644 > --- a/sound/pci/hda/hda_intel.c > +++ b/sound/pci/hda/hda_intel.c > @@ -1027,7 +1027,7 @@ static int azx_suspend(struct device *dev) > chip = card->private_data; > bus = azx_bus(chip); > snd_power_change_state(card, SNDRV_CTL_POWER_D3hot); > - __azx_runtime_suspend(chip); > + pm_runtime_force_suspend(dev); > if (bus->irq >= 0) { > free_irq(bus->irq, chip); > bus->irq = -1; > @@ -1044,7 +1044,9 @@ static int azx_suspend(struct device *dev) > static int azx_resume(struct device *dev) > { > struct snd_card *card = dev_get_drvdata(dev); > + struct hda_codec *codec; > struct azx *chip; > + bool forced_resume = false; > > if (!azx_is_pm_ready(card)) > return 0; > @@ -1055,7 +1057,20 @@ static int azx_resume(struct device *dev) > chip->msi = 0; > if (azx_acquire_irq(chip, 1) < 0) > return -EIO; > - __azx_runtime_resume(chip, false); > + > + /* check for the forced resume */ > + list_for_each_codec(codec, &chip->bus) { > + if (hda_codec_need_resume(codec)) { > + forced_resume = true; > + break; > + } > + } > + > + if (forced_resume) > + pm_runtime_get_noresume(dev); > + pm_runtime_force_resume(dev); > + if (forced_resume) > + pm_runtime_put(dev); > snd_power_change_state(card, SNDRV_CTL_POWER_D0); > > trace_azx_resume(chip); > @@ -1102,12 +1117,12 @@ static int azx_runtime_suspend(struct device *dev) > if (!azx_is_pm_ready(card)) > return 0; > chip = card->private_data; > - if (!azx_has_pm_runtime(chip)) > - return 0; > > /* enable controller wake up event */ > - azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) | > - STATESTS_INT_MASK); > + if (snd_power_get_state(card) == SNDRV_CTL_POWER_D0) { > + azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) | > + STATESTS_INT_MASK); > + } > > __azx_runtime_suspend(chip); > trace_azx_runtime_suspend(chip); > @@ -1118,17 +1133,18 @@ static int azx_runtime_resume(struct device *dev) > { > struct snd_card *card = dev_get_drvdata(dev); > struct azx *chip; > + bool from_rt = snd_power_get_state(card) == SNDRV_CTL_POWER_D0; > > if (!azx_is_pm_ready(card)) > return 0; > chip = card->private_data; > - if (!azx_has_pm_runtime(chip)) > - return 0; > - __azx_runtime_resume(chip, true); > + __azx_runtime_resume(chip, from_rt); > > /* disable controller Wake Up event*/ > - azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) & > - ~STATESTS_INT_MASK); > + if (from_rt) { > + azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) & > + ~STATESTS_INT_MASK); > + } > > trace_azx_runtime_resume(chip); > return 0; > -- > 2.20.1 >
