On Fri, 09 Oct 2020 16:02:27 +0200, Kai Vehmanen wrote:
In case HDA controller is 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, the jack detection will now work. Add a flag to 'hda_codec' to describe codecs that require this flow from the controller driver. Mark all Realtek codecs with this 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 | 8 ++++++-- sound/pci/hda/patch_realtek.c | 6 ++++++ 3 files changed, 13 insertions(+), 2 deletions(-)
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..cfc073c992e7 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -1002,12 +1002,16 @@ 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 (from_rt) {
list_for_each_codec(codec, &chip->bus) {
if (codec->forced_resume && pm_runtime_suspended(hda_codec_dev(codec)))
pm_request_resume(hda_codec_dev(codec));
if (!codec->relaxed_resume && (status & (1 << codec->addr))) schedule_delayed_work(&codec->jackpoll_work, codec->jackpoll_interval);
}}
Basically both pm_request_resume() and the jackpoll_work do the equivalent task, hence no need to do twice differently. Actually pm_request_resume() looks like a better choice as it's clearer about what it does, so let's replace with it.
Also, the pm_runtime_suspend() can be skipped here; the codec must have been suspended at this moment because of the pm-dependency.
So, it'll be like:
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)); } }
Could you check whether this still works?
thanks,
Takashi