On 7/27/20 11:44 AM, Takashi Iwai wrote:

We've received a regression report on Intel HD-audio controller that wakes up immediately after S3 suspend. The bisection leads to the commit c4c8dd6ef807 ("ALSA: hda: Skip controller resume if not needed"). This commit replaces the system-suspend to use pm_runtime_force_suspend() instead of the direct call of __azx_runtime_suspend(). However, by some really mysterious reason, pm_runtime_force_suspend() causes a spurious wakeup (although it calls the same __azx_runtime_suspend() internally).

Right, but __azx_runtime_suspend() is called after enabling a wake-up event, which is fine for pm_runtime but probably not so good for S3?

static int azx_runtime_suspend(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); struct azx *chip;

if (!azx_is_pm_ready(card)) return 0; chip = card->private_data;

/* enable controller wake up event */ 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); return 0; }

As an ugly workaround for now, revert the behavior to call __azx_runtime_suspend() and __azx_runtime_resume() for those old Intel platforms that may exhibit such a problem, while keeping the new standard pm_runtime_force_suspend() and pm_runtime_force_resume() pair for the remaining chips.

Fixes: c4c8dd6ef807 ("ALSA: hda: Skip controller resume if not needed") BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=208649 Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de

---

sound/pci/hda/hda_controller.h | 2 +- sound/pci/hda/hda_intel.c | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/sound/pci/hda/hda_controller.h b/sound/pci/hda/hda_controller.h index fe171685492d..be63ead8161f 100644 --- a/sound/pci/hda/hda_controller.h +++ b/sound/pci/hda/hda_controller.h @@ -41,7 +41,7 @@ /* 24 unused */ #define AZX_DCAPS_COUNT_LPIB_DELAY (1 << 25) /* Take LPIB as delay */ #define AZX_DCAPS_PM_RUNTIME (1 << 26) /* runtime PM support */ -/* 27 unused */ +#define AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP (1 << 27) /* Workaround for spurious wakeups after suspend */ #define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28) /* CORBRP clears itself after reset */ #define AZX_DCAPS_NO_MSI64 (1 << 29) /* Stick to 32-bit MSIs */ #define AZX_DCAPS_SEPARATE_STREAM_TAG (1 << 30) /* capture and playback use separate stream tag */ diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 9ba1fb8f0b7f..fb65450d8de1 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -297,7 +297,8 @@ enum { /* PCH for HSW/BDW; with runtime PM */ /* no i915 binding for this as HSW/BDW has another controller for HDMI */ #define AZX_DCAPS_INTEL_PCH \

• (AZX_DCAPS_INTEL_PCH_BASE | AZX_DCAPS_PM_RUNTIME)

• (AZX_DCAPS_INTEL_PCH_BASE | AZX_DCAPS_PM_RUNTIME |\

• AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP)
  
  

  /* HSW HDMI */ #define AZX_DCAPS_INTEL_HASWELL \

@@ -1026,7 +1027,14 @@ static int azx_suspend(struct device *dev) chip = card->private_data; bus = azx_bus(chip); snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);

• pm_runtime_force_suspend(dev);

• /* An ugly workaround: direct call of __azx_runtime_suspend() and

• * __azx_runtime_resume() for old Intel platforms that suffer from
  

• * spurious wakeups after S3 suspend
  

• */
  

• if (chip->driver_caps & AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP)

• __azx_runtime_suspend(chip);
  

• else

• pm_runtime_force_suspend(dev);
  

  if (bus->irq >= 0) { free_irq(bus->irq, chip); bus->irq = -1;

@@ -1055,7 +1063,10 @@ static int azx_resume(struct device *dev) if (azx_acquire_irq(chip, 1) < 0) return -EIO;

• pm_runtime_force_resume(dev);

• if (chip->driver_caps & AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP)

• __azx_runtime_resume(chip, false);
  

• else

• pm_runtime_force_resume(dev);
  

  snd_power_change_state(card, SNDRV_CTL_POWER_D0);

  trace_azx_resume(chip);