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@xxxxxxxxxxx> > Co-developed-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > Signed-off-by: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx> > --- > 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
