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

[Takashi Iwai]

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 at realtek.com>
> Co-developed-by: Kai-Heng Feng <kai.heng.feng at canonical.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng at canonical.com>
> Signed-off-by: Kai Vehmanen <kai.vehmanen at 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