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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to