This is a note to let you know that I've just added the patch titled ALSA: hda - Fix unexpected resume through regmap code path to the 4.5-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: alsa-hda-fix-unexpected-resume-through-regmap-code-path.patch and it can be found in the queue-4.5 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable@xxxxxxxxxxxxxxx> know about it. >From fc4f000bf8c0cbf38f44de6bd5e225574e572ed4 Mon Sep 17 00:00:00 2001 From: Takashi Iwai <tiwai@xxxxxxx> Date: Fri, 4 Mar 2016 11:34:18 +0100 Subject: ALSA: hda - Fix unexpected resume through regmap code path From: Takashi Iwai <tiwai@xxxxxxx> commit fc4f000bf8c0cbf38f44de6bd5e225574e572ed4 upstream. HD-audio driver has a mechanism to trigger the runtime resume automatically at accessing the verbs. This auto-resume, however, causes the mutex deadlock when invoked from the regmap handler since the regmap keeps the mutex while auto-resuming. For avoiding that, there is some tricky check in the HDA regmap handler to return -EAGAIN error to back-off when the codec is powered down. Then the caller of regmap r/w will retry after properly turning on the codec power. This works in most cases, but there seems a slight race between the codec power check and the actual on-demand auto-resume trigger. This resulted in the lockdep splat, eventually leading to a real deadlock. This patch tries to address the race window by getting the runtime PM refcount at the check time using pm_runtime_get_if_in_use(). With this call, we can keep the power on only when the codec has been already turned on, and back off if not. For keeping the code consistency, the code touching the runtime PM is stored in hdac_device.c although it's used only locally in hdac_regmap.c. Reported-by: Jiri Slaby <jslaby@xxxxxxx> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- include/sound/hdaudio.h | 2 + sound/hda/hdac_device.c | 16 +++++++++++ sound/hda/hdac_regmap.c | 69 ++++++++++++++++++++++++++++++++---------------- 3 files changed, 64 insertions(+), 23 deletions(-) --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -168,11 +168,13 @@ int snd_hdac_power_up(struct hdac_device int snd_hdac_power_down(struct hdac_device *codec); int snd_hdac_power_up_pm(struct hdac_device *codec); int snd_hdac_power_down_pm(struct hdac_device *codec); +int snd_hdac_keep_power_up(struct hdac_device *codec); #else static inline int snd_hdac_power_up(struct hdac_device *codec) { return 0; } static inline int snd_hdac_power_down(struct hdac_device *codec) { return 0; } static inline int snd_hdac_power_up_pm(struct hdac_device *codec) { return 0; } static inline int snd_hdac_power_down_pm(struct hdac_device *codec) { return 0; } +static inline int snd_hdac_keep_power_up(struct hdac_device *codec) { return 0; } #endif /* --- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -611,6 +611,22 @@ int snd_hdac_power_up_pm(struct hdac_dev } EXPORT_SYMBOL_GPL(snd_hdac_power_up_pm); +/* like snd_hdac_power_up_pm(), but only increment the pm count when + * already powered up. Returns -1 if not powered up, 1 if incremented + * or 0 if unchanged. Only used in hdac_regmap.c + */ +int snd_hdac_keep_power_up(struct hdac_device *codec) +{ + if (!atomic_inc_not_zero(&codec->in_pm)) { + int ret = pm_runtime_get_if_in_use(&codec->dev); + if (!ret) + return -1; + if (ret < 0) + return 0; + } + return 1; +} + /** * snd_hdac_power_down_pm - power down the codec * @codec: the codec object --- a/sound/hda/hdac_regmap.c +++ b/sound/hda/hdac_regmap.c @@ -21,13 +21,16 @@ #include <sound/hdaudio.h> #include <sound/hda_regmap.h> -#ifdef CONFIG_PM -#define codec_is_running(codec) \ - (atomic_read(&(codec)->in_pm) || \ - !pm_runtime_suspended(&(codec)->dev)) -#else -#define codec_is_running(codec) true -#endif +static int codec_pm_lock(struct hdac_device *codec) +{ + return snd_hdac_keep_power_up(codec); +} + +static void codec_pm_unlock(struct hdac_device *codec, int lock) +{ + if (lock == 1) + snd_hdac_power_down_pm(codec); +} #define get_verb(reg) (((reg) >> 8) & 0xfff) @@ -238,20 +241,28 @@ static int hda_reg_read(void *context, u struct hdac_device *codec = context; int verb = get_verb(reg); int err; + int pm_lock = 0; - if (!codec_is_running(codec) && verb != AC_VERB_GET_POWER_STATE) - return -EAGAIN; + if (verb != AC_VERB_GET_POWER_STATE) { + pm_lock = codec_pm_lock(codec); + if (pm_lock < 0) + return -EAGAIN; + } reg |= (codec->addr << 28); - if (is_stereo_amp_verb(reg)) - return hda_reg_read_stereo_amp(codec, reg, val); - if (verb == AC_VERB_GET_PROC_COEF) - return hda_reg_read_coef(codec, reg, val); + if (is_stereo_amp_verb(reg)) { + err = hda_reg_read_stereo_amp(codec, reg, val); + goto out; + } + if (verb == AC_VERB_GET_PROC_COEF) { + err = hda_reg_read_coef(codec, reg, val); + goto out; + } if ((verb & 0x700) == AC_VERB_SET_AMP_GAIN_MUTE) reg &= ~AC_AMP_FAKE_MUTE; err = snd_hdac_exec_verb(codec, reg, 0, val); if (err < 0) - return err; + goto out; /* special handling for asymmetric reads */ if (verb == AC_VERB_GET_POWER_STATE) { if (*val & AC_PWRST_ERROR) @@ -259,7 +270,9 @@ static int hda_reg_read(void *context, u else /* take only the actual state */ *val = (*val >> 4) & 0x0f; } - return 0; + out: + codec_pm_unlock(codec, pm_lock); + return err; } static int hda_reg_write(void *context, unsigned int reg, unsigned int val) @@ -267,6 +280,7 @@ static int hda_reg_write(void *context, struct hdac_device *codec = context; unsigned int verb; int i, bytes, err; + int pm_lock = 0; if (codec->caps_overwriting) return 0; @@ -275,14 +289,21 @@ static int hda_reg_write(void *context, reg |= (codec->addr << 28); verb = get_verb(reg); - if (!codec_is_running(codec) && verb != AC_VERB_SET_POWER_STATE) - return codec->lazy_cache ? 0 : -EAGAIN; + if (verb != AC_VERB_SET_POWER_STATE) { + pm_lock = codec_pm_lock(codec); + if (pm_lock < 0) + return codec->lazy_cache ? 0 : -EAGAIN; + } - if (is_stereo_amp_verb(reg)) - return hda_reg_write_stereo_amp(codec, reg, val); + if (is_stereo_amp_verb(reg)) { + err = hda_reg_write_stereo_amp(codec, reg, val); + goto out; + } - if (verb == AC_VERB_SET_PROC_COEF) - return hda_reg_write_coef(codec, reg, val); + if (verb == AC_VERB_SET_PROC_COEF) { + err = hda_reg_write_coef(codec, reg, val); + goto out; + } switch (verb & 0xf00) { case AC_VERB_SET_AMP_GAIN_MUTE: @@ -319,10 +340,12 @@ static int hda_reg_write(void *context, reg |= (verb + i) << 8 | ((val >> (8 * i)) & 0xff); err = snd_hdac_exec_verb(codec, reg, 0, NULL); if (err < 0) - return err; + goto out; } - return 0; + out: + codec_pm_unlock(codec, pm_lock); + return err; } static const struct regmap_config hda_regmap_cfg = { Patches currently in stable-queue which might be from tiwai@xxxxxxx are queue-4.5/alsa-hda-apply-reboot-d3-fix-for-cx20724-codec-too.patch queue-4.5/alsa-intel8x0-add-clock-quirk-entry-for-ad1981b-on-ibm-thinkpad-x41.patch queue-4.5/alsa-hda-add-new-gpu-codec-id-0x10de0082-to-snd-hda.patch queue-4.5/alsa-hda-limit-i915-hdmi-binding-only-for-hsw-and-later.patch queue-4.5/alsa-hda-fix-unexpected-resume-through-regmap-code-path.patch queue-4.5/alsa-hda-fix-missing-eld-update-at-unplugging.patch queue-4.5/alsa-hda-really-restrict-i915-notifier-to-hsw.patch queue-4.5/alsa-usb-audio-add-microsoft-hd-5001-to-quirks.patch queue-4.5/alsa-hda-workaround-for-unbalanced-i915-power-refcount-by-concurrent-probe.patch queue-4.5/alsa-usb-audio-fix-null-dereference-in-create_fixed_stream_quirk.patch queue-4.5/alsa-hda-fix-spurious-kernel-warning-on-baytrail-hdmi.patch queue-4.5/alsa-usb-audio-minor-code-cleanup-in-create_fixed_stream_quirk.patch queue-4.5/alsa-pcm-avoid-bug-string-for-warnings-again.patch queue-4.5/alsa-usb-audio-fix-double-free-in-error-paths-after-snd_usb_add_audio_stream-call.patch queue-4.5/alsa-hda-fix-unconditional-gpio-toggle-via-automute.patch queue-4.5/alsa-hda-don-t-handle-eld-notify-from-invalid-port.patch queue-4.5/alsa-usb-audio-add-sanity-checks-for-endpoint-accesses.patch queue-4.5/alsa-hda-fix-the-mic-mute-button-and-led-problem-for-a-lenovo-aio.patch queue-4.5/alsa-hda-fix-forgotten-hdmi-monitor_present-update.patch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html