Re: [PATCH] ALSA: hda/via

* Re: [PATCH] ALSA: hda/via - fix occasionally no sound with VT1802
       [not found] <tencent_F4C3EF827E2193D1C7323B9F51D53CA91D0A@qq.com>
@ 2021-11-23  8:25 ` Takashi Iwai
  2021-11-23  8:39   ` =?gb18030?B?u9i4tKO6IFtQQVRDSF0gQUxTQTogaGRhL3ZpYSAtIGZpeCBvY2Nhc2lvbmFsbHkgbm8gc291bmQgd2l0aCBWVDE4MDI=?= =?gb18030?B?taXH+tGtu7c=?=
  0 siblings, 1 reply; 2+ messages in thread
From: Takashi Iwai @ 2021-11-23  8:25 UTC (permalink / raw)
  To: tianyucode; +Cc: alsa-devel, tianyu, tiwai

On Tue, 23 Nov 2021 09:07:14 +0100,
tianyucode wrote:
> 
> From: tianyu <tianyu@kylinos.cn>
> 
> Hi guys, I'm a new guy for audio driver.
> 
> I have encountered a problem. When using the VT1802 codec,
> there will be no sound occasionally. After checking the initialization process,
> I found that the cause of the problem is that the power state does not have
> hda_set_power_state(codec, AC_PWRST_D0).
> 
> 1. The power state is set to AC_PWRST_D0 during the first initialization：
> 
> azx_probe_continue
> 	azx_probe_codecs
> 		snd_hda_codec_new
> 			snd_hda_codec_device_new
> 				hda_set_power_state(codec, AC_PWRST_D0);
> 
> 2. Then, the power state is set to AC_PWRST_D3 in VT1802, but not set in realtek (ALC269).
> 
> azx_probe_continue
> 	azx_codec_configure
> 		…
> 		patch_vt2002
> 			via_parse_auto_config
> 				snd_hda_gen_parse_auto_config
> 					add_all_pin_power_ctls
> 						add_pin_power_ctls
> 							set_path_power
> 
> static void add_all_pin_power_ctls(struct hda_codec *codec, bool on)
> {
> 	/**/
> 	if (!codec->power_save_node)
> 		return;
> 
> 	add_pin_power_ctls(codec, cfg->line_outs, cfg->line_out_pins, on);
> 	/**/
> }
> 
> The power_save_node is set to 0 in patch_alc269 and it returns here,
> but it is set to 1 in VT1802.
> 
> patch_vt2002P
> 	via_new_spec
> 		codec->power_save_node = 1;
> 	via_parse_auto_config
> 		snd_hda_gen_parse_auto_config
> 		codec->power_save_node = 0;//it set to 0 here,but add_all_pin_power_ctls has been done
> 
> patch_alc269
> 	codec->power_save_node = 0;
> 
> 3. Next there is a suspend and resume process, the resume process will also set the
> power state to AC_PWRST_D0, but I think there is a problem with this process.
> 
> (1)suspend:
> 
> azx_probe_continue
> 	snd_card_register
> 		..
> 		snd_hda_codec_dev_register
> 			snd_hda_codec_register
> 				snd_hda_power_down
> 					pm_runtime_put_autosuspend
> 					__pm_runtime_suspend(dev, RPM_GET_PUT | RPM_ASYNC | RPM_AUTO);
> 						rpm_suspend
> 
> static int rpm_suspend(struct device *dev, int rpmflags)
> {
> 	/**/
> 	if (rpmflags & RPM_ASYNC) {
> 		dev->power.request = (rpmflags & RPM_AUTO) ?
> 		    RPM_REQ_AUTOSUSPEND : RPM_REQ_SUSPEND;
> 		if (!dev->power.request_pending) {
> 			dev->power.request_pending = true;
> 			queue_work(pm_wq, &dev->power.work);
> 		}
> 		goto out;
> 	}
> 
> 	__update_runtime_status(dev, RPM_SUSPENDING);
> 
> 	/**/
> 	retval = rpm_callback(callback, dev);
> 	__update_runtime_status(dev, RPM_SUSPENDED);
> 	/**/
> }
> 
> The first call to rpm_suspend has the RPM_ASYNC flag, so queue_work is called,
> and the work queue is used to enter rpm_suspend
> 
> pm_runtime_init
> 	INIT_WORK(&dev->power.work, pm_runtime_work);
> 
> (2)resume:
> 
> azx_probe_continue
> 	set_default_power_save
> 		snd_hda_set_power_save(&chip->bus, val * 1000);
> 			codec_set_power_save(c, delay);
> 
> int val = power_save;
> static int power_save = CONFIG_SND_HDA_POWER_SAVE_DEFAULT;//It is 0 in my .config
> 
> static void codec_set_power_save(struct hda_codec *codec, int delay)
> {
> 	struct device *dev = hda_codec_dev(codec);
> 	if (delay == 0 && codec->auto_runtime_pm)
> 		delay = 3000;
> 
> 	if (delay > 0) {
> 		pm_runtime_set_autosuspend_delay(dev, delay);
> 		pm_runtime_use_autosuspend(dev);
> 		pm_runtime_allow(dev);
> 		if (!pm_runtime_suspended(dev))
> 			pm_runtime_mark_last_busy(dev);
> 	} else {
> 		pm_runtime_dont_use_autosuspend(dev);
> 		pm_runtime_forbid(dev);
> 	}
> }
> 
> pm_runtime_forbid
> 	rpm_resume(dev, 0);
> 
> static int rpm_resume(struct device *dev, int rpmflags)
> {
> 	/**/
> 	if (dev->power.runtime_status == RPM_ACTIVE) {
> 		retval = 1;
> 		goto out;
> 	}
> 
> 	/**/
> 	_update_runtime_status(dev, RPM_RESUMING);
> 	retval = rpm_callback(callback, dev);
> 	/**/
> 	__update_runtime_status(dev, RPM_ACTIVE);
> 	/**/
> }
> 
> The callback functions of suspend and resume are as follows, which set the power state:
> 
> hda_call_codec_suspend
> 	state = hda_set_power_state(codec, AC_PWRST_D3);
> 
> hda_call_codec_resume
> 	hda_set_power_state(codec, AC_PWRST_D0);
> 
> You can see that the resume function relies on dev->power.runtime_status,
> and the status is set in rpm_suspend. The operation of rpm_suspend depends
> on the scheduling of the work queue. I added print debugging on my machine,
> and occasionally there will be pm_runtime_work in Run after rpm_resume.
> At this time, the suspend and resume processes are not performed correctly.
> In VT1802, the power_state is still D3, and the machine has no sound.
> 
> 4. I searched the Internet and did not find the relevant modification,
> but found the commit 317d9313925c (ALSA: hda/realtek-Set default power save node to 0).
> 
> Does VIA need to be modified like this?
> 
> Since I am not familiar with ALSA and runtime pm, so come here to consult.

For VIA codecs, there is an additional fixup table to turn off the
power_save_node.  Add an entry with the PCI SSID of your device to
vt2002p_fixups[] table with VIA_FIXUP_POWER_SAVE.

thanks,

Takashi

^ permalink raw reply	[flat|nested] 2+ messages in thread