diff mbox

ALSA: hda - repoll jack presence state on resume

Message ID 1471009924-32261-1-git-send-email-perex@perex.cz (mailing list archive)
State New, archived
Headers show

Commit Message

Jaroslav Kysela Aug. 12, 2016, 1:52 p.m. UTC
The jack detection on resume might fail on some hardware
(Lenovo T430s, T440s and X1 Carbon 2016). The codec registers
reports that the jack is not used and the unsolicited event
is not sent on change (or perhaps, it's lost?).

This patch will repoll the jack presence state after
100ms again.

Because the jack status is updated from two workqueues now,
create a new codec->jack_mutex to protect the concurrent
access.

Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 sound/pci/hda/hda_bind.c  | 2 ++
 sound/pci/hda/hda_codec.c | 9 ++++++++-
 sound/pci/hda/hda_codec.h | 1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

Comments

Takashi Iwai Aug. 12, 2016, 2:03 p.m. UTC | #1
On Fri, 12 Aug 2016 15:52:04 +0200,
Jaroslav Kysela wrote:
> 
> The jack detection on resume might fail on some hardware
> (Lenovo T430s, T440s and X1 Carbon 2016). The codec registers
> reports that the jack is not used and the unsolicited event
> is not sent on change (or perhaps, it's lost?).
> 
> This patch will repoll the jack presence state after
> 100ms again.

Does this happen with the runtime PM, too?  If the runtime PM doesn't
suffer, such a delay can be skipped for runtime resume, at least.
100ms isn't so short for the runtime PM, after all.

> Because the jack status is updated from two workqueues now,
> create a new codec->jack_mutex to protect the concurrent
> access.

IMO, it's better to move mutex_lock call in hda_jack.c.

And the similar situation may happen with the jackpoll option, could
you split this mutex protection change as a separate fix?


thanks,

Takashi


> Signed-off-by: Jaroslav Kysela <perex@perex.cz>
> ---
>  sound/pci/hda/hda_bind.c  | 2 ++
>  sound/pci/hda/hda_codec.c | 9 ++++++++-
>  sound/pci/hda/hda_codec.h | 1 +
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/hda_bind.c b/sound/pci/hda/hda_bind.c
> index 6efadbf..81a9284 100644
> --- a/sound/pci/hda/hda_bind.c
> +++ b/sound/pci/hda/hda_bind.c
> @@ -42,8 +42,10 @@ static void hda_codec_unsol_event(struct hdac_device *dev, unsigned int ev)
>  {
>  	struct hda_codec *codec = container_of(dev, struct hda_codec, core);
>  
> +	mutex_lock(&codec->jack_mutex);
>  	if (codec->patch_ops.unsol_event)
>  		codec->patch_ops.unsol_event(codec, ev);
> +	mutex_unlock(&codec->jack_mutex);
>  }
>  
>  /**
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index 9913be8..4339e98 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -592,8 +592,10 @@ static void hda_jackpoll_work(struct work_struct *work)
>  	struct hda_codec *codec =
>  		container_of(work, struct hda_codec, jackpoll_work.work);
>  
> +	mutex_lock(&codec->jack_mutex);
>  	snd_hda_jack_set_dirty_all(codec);
>  	snd_hda_jack_poll_all(codec);
> +	mutex_unlock(&codec->jack_mutex);
>  
>  	if (!codec->jackpoll_interval)
>  		return;
> @@ -837,6 +839,7 @@ int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
>  	codec->addr = codec_addr;
>  	mutex_init(&codec->spdif_mutex);
>  	mutex_init(&codec->control_mutex);
> +	mutex_init(&codec->jack_mutex);
>  	snd_array_init(&codec->mixers, sizeof(struct hda_nid_item), 32);
>  	snd_array_init(&codec->nids, sizeof(struct hda_nid_item), 32);
>  	snd_array_init(&codec->init_pins, sizeof(struct hda_pincfg), 16);
> @@ -3002,8 +3005,12 @@ static void hda_call_codec_resume(struct hda_codec *codec)
>  
>  	if (codec->jackpoll_interval)
>  		hda_jackpoll_work(&codec->jackpoll_work.work);
> -	else
> +	else {
>  		snd_hda_jack_report_sync(codec);
> +		/* Some hw requires a little time to settle things */
> +		schedule_delayed_work(&codec->jackpoll_work,
> +					msecs_to_jiffies(100));
> +	}
>  	atomic_dec(&codec->core.in_pm);
>  }
>  
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index 373fcad..6a4a631 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -213,6 +213,7 @@ struct hda_codec {
>  
>  	struct mutex spdif_mutex;
>  	struct mutex control_mutex;
> +	struct mutex jack_mutex;
>  	struct snd_array spdif_out;
>  	unsigned int spdif_in_enable;	/* SPDIF input enable? */
>  	const hda_nid_t *slave_dig_outs; /* optional digital out slave widgets */
> -- 
> 2.5.5
>
diff mbox

Patch

diff --git a/sound/pci/hda/hda_bind.c b/sound/pci/hda/hda_bind.c
index 6efadbf..81a9284 100644
--- a/sound/pci/hda/hda_bind.c
+++ b/sound/pci/hda/hda_bind.c
@@ -42,8 +42,10 @@  static void hda_codec_unsol_event(struct hdac_device *dev, unsigned int ev)
 {
 	struct hda_codec *codec = container_of(dev, struct hda_codec, core);
 
+	mutex_lock(&codec->jack_mutex);
 	if (codec->patch_ops.unsol_event)
 		codec->patch_ops.unsol_event(codec, ev);
+	mutex_unlock(&codec->jack_mutex);
 }
 
 /**
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 9913be8..4339e98 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -592,8 +592,10 @@  static void hda_jackpoll_work(struct work_struct *work)
 	struct hda_codec *codec =
 		container_of(work, struct hda_codec, jackpoll_work.work);
 
+	mutex_lock(&codec->jack_mutex);
 	snd_hda_jack_set_dirty_all(codec);
 	snd_hda_jack_poll_all(codec);
+	mutex_unlock(&codec->jack_mutex);
 
 	if (!codec->jackpoll_interval)
 		return;
@@ -837,6 +839,7 @@  int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
 	codec->addr = codec_addr;
 	mutex_init(&codec->spdif_mutex);
 	mutex_init(&codec->control_mutex);
+	mutex_init(&codec->jack_mutex);
 	snd_array_init(&codec->mixers, sizeof(struct hda_nid_item), 32);
 	snd_array_init(&codec->nids, sizeof(struct hda_nid_item), 32);
 	snd_array_init(&codec->init_pins, sizeof(struct hda_pincfg), 16);
@@ -3002,8 +3005,12 @@  static void hda_call_codec_resume(struct hda_codec *codec)
 
 	if (codec->jackpoll_interval)
 		hda_jackpoll_work(&codec->jackpoll_work.work);
-	else
+	else {
 		snd_hda_jack_report_sync(codec);
+		/* Some hw requires a little time to settle things */
+		schedule_delayed_work(&codec->jackpoll_work,
+					msecs_to_jiffies(100));
+	}
 	atomic_dec(&codec->core.in_pm);
 }
 
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 373fcad..6a4a631 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -213,6 +213,7 @@  struct hda_codec {
 
 	struct mutex spdif_mutex;
 	struct mutex control_mutex;
+	struct mutex jack_mutex;
 	struct snd_array spdif_out;
 	unsigned int spdif_in_enable;	/* SPDIF input enable? */
 	const hda_nid_t *slave_dig_outs; /* optional digital out slave widgets */