[alsa-devel] [PATCH] ALSA: hda - call runtime_resume after S3 and S4 for each codec

[Hui Wang]

On 2019/3/13 下午7:19, Takashi Iwai wrote:
> On Sat, 09 Mar 2019 16:19:47 +0100,
> Hui Wang wrote:
>> Recently we found the audio jack detection doesn't work after suspend
>> on many machines with Realtek codec. Sometimes the audio selection
>> dialogue didn't show up after users plugged headhphone/headset into
>> the headset jack, sometimes after uses plugged headphone/headset, then
>> click the sound icon on the upper-right corner of gnome-desktop, it
>> also showed the speaker rather than the headphone.
>>
>> The root cause is that before suspend, the codec already call the
>> runtime_suspend since this codec is not used by any apps, then in
>> resume, it will not call runtime_resume for this codec. But for some
>> realtek codec (so far, alc236, alc255 and alc891) with the specific
>> BIOS, if it doesn't run runtime_resume after suspend, all codec
>> functions including jack detection stop working anymore.
>>
>> This problem existed for a long time, but it was not exposed, that is
>> because if users is playing sound or recording sound, and at the same
>> time they run suspend,  the runtime_suspend will be called in
>> pm_suspend and runtime_resume will be called in pm_resume; if audio
>> codec is not used by any apps and users run suspend, the
>> runtime_resume will not be called in pm_resume, then codec stops
>> working, but if users play sound or open sound-setting to check
>> audio device, this will trigger calling to runtime_resume (
>> via snd_hda_power_up), then the codec starts working again before
>> users notice this problem.
>>
>> Since we don't know how many codec and BIOS combinations have this
>> problem, to fix it, let the driver call runtime_resume for all codecs
>> in pm_resume, maybe for some codecs, this is not needed, but it is
>> harmless. After a codec is runtime resumed, if it is not used by any
>> apps, it will be runtime suspended soon and furthermore we don't run
>> suspend frequently, this change will not add much power consumption.
>>
>> Signed-off-by: Hui Wang <hui.wang at canonical.com>
>> ---
>>  include/sound/hda_codec.h |  3 +++
>>  sound/pci/hda/hda_codec.c | 26 +++++++++++++++++++++++---
>>  2 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
>> index cc7c8d42d4fd..a4e26d1d18bc 100644
>> --- a/include/sound/hda_codec.h
>> +++ b/include/sound/hda_codec.h
>> @@ -262,6 +262,9 @@ struct hda_codec {
>>  	unsigned int auto_runtime_pm:1; /* enable automatic codec runtime pm */
>>  	unsigned int force_pin_prefix:1; /* Add location prefix */
>>  	unsigned int link_down_at_suspend:1; /* link down at runtime suspend */
>> +#ifdef CONFIG_PM_SLEEP
>> +	unsigned int already_rt_suspend:1; /* already runtime suspend before sleep */
>> +#endif
>>  #ifdef CONFIG_PM
>>  	unsigned long power_on_acct;
>>  	unsigned long power_off_acct;
>> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
>> index 5f2005098a60..7c2bbe25adde 100644
>> --- a/sound/pci/hda/hda_codec.c
>> +++ b/sound/pci/hda/hda_codec.c
>> @@ -2939,34 +2939,54 @@ static int hda_codec_runtime_resume(struct device *dev)
>>  #endif /* CONFIG_PM */
>>  
>>  #ifdef CONFIG_PM_SLEEP
>> +static int hda_codec_force_resume(struct device *dev)
>> +{
>> +	struct hda_codec *codec = dev_to_hda_codec(dev);
>> +
>> +	if (codec->already_rt_suspend) {
>> +		int ret;
>> +
>> +		pm_runtime_get_noresume(dev);
>> +		ret = pm_runtime_force_resume(dev);
>> +		pm_runtime_put(dev);
>> +		return ret;
>> +	} else
>> +		return pm_runtime_force_resume(dev);
>> +}
> I don't think we need codec->already_rt_suspend flag check.  And it
> deserves a comment.  i.e. hda_codec_force_resume() will be something
> like:
>
> static int hda_codec_force_resume(struct device *dev)
> {
> 	int ret;
>
> 	/* The get/put pair below enforces the runtime resume even if the
> 	 * device hasn't been used at suspend time.  This trick is needed to
> 	 * update the jack state change during the sleep.
> 	 */
> 	pm_runtime_get_noresume(dev);
> 	ret = pm_runtime_force_resume(dev);
> 	pm_runtime_put(dev);
> 	return ret;
> }
>
>
> Could you check whether this works?
>
>
> thanks,
>
> Takashi

Got it, will test it this weekend or next week after I get the hardware.

Thanks,

Hui.


> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>