Re: [PATCH 1/1] ALSA: hda: fix jack detection with Realtek codecs when in D3

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 10/9/20 9:02 AM, 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.

It does make sense to request the codec to resume when the controller 
resumes, we did the same for SoundWire IIRC.

I am still confused on what happens if e.g. the controller resumes and 
remains active, e.g. capturing from the DMIC. The codec would become 
suspended after a while and then we would be back to the same problem, 
wouldn't we?

Or are you saying that this initial resume of the codec is enough?

> 
> 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);
> +		}
>   	}
>   
>   	/* 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..e4ab483db72f 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -849,6 +849,12 @@ static int alc_build_controls(struct hda_codec *codec)
>   static void alc_pre_init(struct hda_codec *codec)
>   {
>   	alc_fill_eapd_coef(codec);
> +
> +	/*
> +	 * if controller is resumed from suspend, while codec remains in D3, codec
> +	 * needs to be woken up once or otherwise jack detection does not work
> +	 */
> +	codec->forced_resume = 1;
>   }
>   
>   #define is_s3_resume(codec) \
>