ALSA: hda/via - fix occasionally no sound with VT1802

Commit Message

tianyucode Nov. 23, 2021, 8:18 a.m. UTC

From: tianyu <[e-mail]>

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.

---
 sound/pci/hda/patch_via.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Patch

diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c
index a29d7e989fe6..d01ea582cf25 100644
--- a/sound/pci/hda/patch_via.c
+++ b/sound/pci/hda/patch_via.c
@@ -1073,8 +1073,10 @@  static int patch_vt2002P(struct hda_codec *codec)
 	snd_hda_pick_fixup(codec, NULL, vt2002p_fixups, via_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
-	if (spec->codec_type == VT1802)
+	if (spec->codec_type == VT1802) {
+		codec->power_save_node = 0;
 		err = snd_hda_add_verbs(codec, vt1802_init_verbs);
+	}
 	else
 		err = snd_hda_add_verbs(codec, vt2002P_init_verbs);
 	if (err < 0)