diff mbox series

[v2] ALSA: hda: fix jack detection with Realtek codecs when in D3

Message ID 20201012102704.794423-1-kai.vehmanen@linux.intel.com
State Accepted
Commit a6e7d0a4bdb02a7a3ffe0b44aaa8842b7efdd056
Headers show
Series [v2] ALSA: hda: fix jack detection with Realtek codecs when in D3 | expand

Commit Message

Kai Vehmanen Oct. 12, 2020, 10:27 a.m. UTC 
In case HDA controller becomes active, but codec is runtime suspended,
jack detection is not successful and no interrupt is raised. This has
been observed with multiple Realtek codecs and HDA controllers from
different vendors. Bug does not occur if both codec and controller are
active, or both are in suspend. Bug can be easily hit on desktop systems
with no built-in speaker.

The problem can be fixed by powering up the codec once after every
controller runtime resume. Even if codec goes back to suspend later, the
jack detection will continue to work. Add a flag to 'hda_codec' to
describe codecs that require this flow from the controller driver.
Modify __azx_runtime_resume() to use pm_request_resume() to make the
intent clearer.

Mark all Realtek codecs with the new forced_resume flag.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=209379
Cc: Kailang Yang <kailang@realtek.com>
Co-developed-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 include/sound/hda_codec.h     |  1 +
 sound/pci/hda/hda_intel.c     | 14 ++++++++------
 sound/pci/hda/patch_realtek.c |  1 +
 3 files changed, 10 insertions(+), 6 deletions(-)

v1->v2:
- simplified the for_each_codec block as per feedback from Takashi
- clarified commit message that bug occurs when controller
  is resumed (feedback from Pierre)
- moved forced_resume setting from alc_pre_init to 
  alc_alloc_spec which seems more appropriate and removed 
  the inline comments as those seem out-of-place in patch_realtek (other
  drivers may use the flag as well)

Comments

Takashi Iwai Oct. 12, 2020, 11:01 a.m. UTC | #1 
On Mon, 12 Oct 2020 12:27:04 +0200,
Kai Vehmanen wrote:
> 
> In case HDA controller becomes active, but codec is runtime suspended,
> jack detection is not successful and no interrupt is raised. This has
> been observed with multiple Realtek codecs and HDA controllers from
> different vendors. Bug does not occur if both codec and controller are
> active, or both are in suspend. Bug can be easily hit on desktop systems
> with no built-in speaker.
> 
> The problem can be fixed by powering up the codec once after every
> controller runtime resume. Even if codec goes back to suspend later, the
> jack detection will continue to work. Add a flag to 'hda_codec' to
> describe codecs that require this flow from the controller driver.
> Modify __azx_runtime_resume() to use pm_request_resume() to make the
> intent clearer.
> 
> Mark all Realtek codecs with the new forced_resume flag.
> 
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=209379
> Cc: Kailang Yang <kailang@realtek.com>
> Co-developed-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>

This deserves for Cc-to-stable.  I applied with it now.
Thanks!


Takashi
diff mbox series

Patch

diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index 0fea49bfc5e8..73827b7d17e0 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -253,6 +253,7 @@  struct hda_codec {
 	unsigned int force_pin_prefix:1; /* Add location prefix */
 	unsigned int link_down_at_suspend:1; /* link down at runtime suspend */
 	unsigned int relaxed_resume:1;	/* don't resume forcibly for jack */
+	unsigned int forced_resume:1; /* forced resume for jack */
 	unsigned int mst_no_extra_pcms:1; /* no backup PCMs for DP-MST */
 
 #ifdef CONFIG_PM
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 61e495187b1a..749b88090970 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1002,12 +1002,14 @@  static void __azx_runtime_resume(struct azx *chip, bool from_rt)
 	azx_init_pci(chip);
 	hda_intel_init_chip(chip, true);
 
-	if (status && from_rt) {
-		list_for_each_codec(codec, &chip->bus)
-			if (!codec->relaxed_resume &&
-			    (status & (1 << codec->addr)))
-				schedule_delayed_work(&codec->jackpoll_work,
-						      codec->jackpoll_interval);
+	if (from_rt) {
+		list_for_each_codec(codec, &chip->bus) {
+			if (codec->relaxed_resume)
+				continue;
+
+			if (codec->forced_resume || (status & (1 << codec->addr)))
+				pm_request_resume(hda_codec_dev(codec));
+		}
 	}
 
 	/* power down again for link-controlled chips */
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index f89506dffd5b..f2398721ac1e 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -1150,6 +1150,7 @@  static int alc_alloc_spec(struct hda_codec *codec, hda_nid_t mixer_nid)
 	codec->single_adc_amp = 1;
 	/* FIXME: do we need this for all Realtek codec models? */
 	codec->spdif_status_reset = 1;
+	codec->forced_resume = 1;
 	codec->patch_ops = alc_patch_ops;
 
 	err = alc_codec_rename_from_preset(codec);