At Wed, 29 Apr 2015 12:28:59 +0100, Jonathan McDowell wrote:
On Tue, Apr 28, 2015 at 04:43:00PM +0200, Takashi Iwai wrote:
At Tue, 28 Apr 2015 15:05:20 +0200, Takashi Iwai wrote:
At Tue, 28 Apr 2015 13:35:18 +0100, Jonathan McDowell wrote:
On Tue, Apr 28, 2015 at 02:00:17PM +0200, Takashi Iwai wrote:
At Tue, 28 Apr 2015 12:21:57 +0100, Jonathan McDowell wrote:
Having upgraded to 4.1-rc1 from 4.0 I'm now hearing audio crackles at regular intervals. I'm fairly sure this is due to the HDA power save as once audio is playing things are fine, it's just when starting to play audio that I hear the crackle.
System is a Dell Latitude E7240. I haven't tried a bisect yet but will attempt to find some time to do so in the next few days. It looks like there have been some changes in sound/hda/ between 4.0 + 4.1-rc1 so I'll concentrate on those first.
There are lots of code changes and enhancements wrt power saving in 4.1, and bisection won't help so much, I'm afraid.
First off, check the device status while you hear crackles. Is the codec in runtime suspend (aka power save)? This can be seen in /sys/bus/hdaudio/devices/*/power/runtime_status. Then check the runtime status of the controller, too, found in /sys/class/sound/card?/device/power/runtime_status.
The cracking is definitely happening with the transition from suspended to active:
/sys/class/sound/card0/device/power/runtime_status:suspended /sys/class/sound/card1/device/power/runtime_status:active /sys/bus/hdaudio/devices/hdaudioC0D0/power/runtime_status:suspended /sys/bus/hdaudio/devices/hdaudioC1D0/power/runtime_status:active
card1 + C1D0 were suspended, I hit delete in a terminal to force a terminal bell, got the crackle and the state changed to active as above.
Ah, so it's a click noise that happens only once per runtime PM transition? I thought it were constant crackling noises from your description ("regular intervals").
In anyway, please give alsa-info.sh output on both 4.0 and 4.1.
Also, does the click noise occur only at powering up, and not at powering down?
The click is only on the transition from suspended to active; not constantly during playback and not when transitioning from active to suspended. It doesn't appear 4.0 is doing the suspending at all; there are no /sys/bus/hdaudio/devices/*/power/runtime_status files and the card runtime_status is always active. alsa-info for 4.0 + 4.1-rc1 attached.
OK, thanks for clarification.
One patch you can try (with or without power_save_node disablement) is below, it squashes the verb sequences at (runtime) PM resume as we did for 4.0. Let me know if this changes the behavior.
Takashi
--- diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index 2a8aa9dfb83d..8e47d9cc369f 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -79,6 +79,7 @@ struct hdac_device { bool lazy_cache:1; /* don't wake up for writes */ bool caps_overwriting:1; /* caps overwrite being in process */ bool cache_coef:1; /* cache COEF read/write too */ + bool cache_only:1; /* don't write actually registers */ };
/* device/driver type used for matching */ diff --git a/sound/hda/hdac_regmap.c b/sound/hda/hdac_regmap.c index 7371e0c3926f..c5b3dcbbacdd 100644 --- a/sound/hda/hdac_regmap.c +++ b/sound/hda/hdac_regmap.c @@ -265,6 +265,9 @@ static int hda_reg_write(void *context, unsigned int reg, unsigned int val) unsigned int verb; int i, bytes, err;
+ if (codec->cache_only) + return 0; + reg &= ~0x00080000U; /* drop GET bit */ reg |= (codec->addr << 28); verb = get_verb(reg); diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index e70a7fb393dd..8f79d649975b 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3156,28 +3156,29 @@ static void hda_call_codec_resume(struct hda_codec *codec) { atomic_inc(&codec->core.in_pm);
- if (codec->core.regmap) - regcache_mark_dirty(codec->core.regmap); - codec->power_jiffies = jiffies;
hda_set_power_state(codec, AC_PWRST_D0); restore_shutup_pins(codec); hda_exec_init_verbs(codec); snd_hda_jack_set_dirty_all(codec); + if (codec->core.regmap) { + regcache_mark_dirty(codec->core.regmap); + codec->core.cache_only = true; + } if (codec->patch_ops.resume) codec->patch_ops.resume(codec); - else { - if (codec->patch_ops.init) - codec->patch_ops.init(codec); - if (codec->core.regmap) - regcache_sync(codec->core.regmap); - } + else if (codec->patch_ops.init) + codec->patch_ops.init(codec);
if (codec->jackpoll_interval) hda_jackpoll_work(&codec->jackpoll_work.work); else snd_hda_jack_report_sync(codec); + if (codec->core.regmap) { + codec->core.cache_only = false; + regcache_sync(codec->core.regmap); + } atomic_dec(&codec->core.in_pm); }
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 3d2597b7037b..844d5e6ae1a1 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -5780,8 +5780,6 @@ int snd_hda_gen_init(struct hda_codec *codec) /* call init functions of standard auto-mute helpers */ update_automute_all(codec);
- regcache_sync(codec->core.regmap); - if (spec->vmaster_mute.sw_kctl && spec->vmaster_mute.hook) snd_hda_sync_vmaster_hook(&spec->vmaster_mute);
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 5f44f60a6389..e4a774ddb1fc 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2211,7 +2211,6 @@ static int generic_hdmi_resume(struct hda_codec *codec) int pin_idx;
codec->patch_ops.init(codec); - regcache_sync(codec->core.regmap);
for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index b18b9c67b262..34219aa6a40e 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -799,7 +799,6 @@ static int alc_resume(struct hda_codec *codec) if (!spec->no_depop_delay) msleep(150); /* to avoid pop noise */ codec->patch_ops.init(codec); - regcache_sync(codec->core.regmap); hda_call_check_power_status(codec, 0x01); return 0; } @@ -3059,7 +3058,6 @@ static int alc269_resume(struct hda_codec *codec) msleep(200); }
- regcache_sync(codec->core.regmap); hda_call_check_power_status(codec, 0x01);
/* on some machine, the BIOS will clear the codec gpio data when enter