diff mbox series

ALSA: hda/cs8409: Schedule delayed work for jack detect on resume

Message ID 20211124181908.50672-1-vitalyr@opensource.cirrus.com (mailing list archive)
State New, archived
Headers show
Series ALSA: hda/cs8409: Schedule delayed work for jack detect on resume | expand

Commit Message

Vitaly Rodionov Nov. 24, 2021, 6:19 p.m. UTC 
From: Stefan Binding <sbinding@opensource.cirrus.com>

CS42L42 runs jack detect on resume, however this requires unsol
events, and unsol events are ignored whilst the power state is
not set to ON. The power state is set to ON only after the resume
finishes. Schedule a delayed work timer to run jack detect
after the resume call finishes.

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
Signed-off-by: Vitaly Rodionov <vitalyr@opensource.cirrus.com>
---
 sound/pci/hda/patch_cs8409.c | 79 +++++++++++++++++++++++++++++-------
 sound/pci/hda/patch_cs8409.h |  1 +
 2 files changed, 65 insertions(+), 15 deletions(-)

Comments

Takashi Iwai Nov. 25, 2021, 7:16 a.m. UTC | #1 
On Wed, 24 Nov 2021 19:19:08 +0100,
Vitaly Rodionov wrote:
> 
> From: Stefan Binding <sbinding@opensource.cirrus.com>
> 
> CS42L42 runs jack detect on resume, however this requires unsol
> events, and unsol events are ignored whilst the power state is
> not set to ON. The power state is set to ON only after the resume
> finishes.

This statement isn't clear to me: which power state and who ignores?
In the basic design of hda_jack, the all jack states get updated once
after the resume by reading the pin sense.  Doesn't this work in the
case of cs8409 because the code relies on unsol event?

> Schedule a delayed work timer to run jack detect
> after the resume call finishes.

This kind of approach is OK-ish as a last resort workaround, but I
think it'd be cleaner to rewrite the code to use directly snd_jack
stuff like HDMI codec driver without hda_jack stuff, supposing that
all jack states on cs8409 are read rather via i2c.  HDMI codec driver
re-probes jacks at its own resume callback, and issue
snd_jack_report() accordingly, as well as the same check for the
unsol_event.

Or, just faked unsol event handling at the resume callback would be
minimalistic change, I suppose.


thanks,

Takashi

> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> Signed-off-by: Vitaly Rodionov <vitalyr@opensource.cirrus.com>
> ---
>  sound/pci/hda/patch_cs8409.c | 79 +++++++++++++++++++++++++++++-------
>  sound/pci/hda/patch_cs8409.h |  1 +
>  2 files changed, 65 insertions(+), 15 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_cs8409.c b/sound/pci/hda/patch_cs8409.c
> index 31ff11ab868e..88213e95f0b3 100644
> --- a/sound/pci/hda/patch_cs8409.c
> +++ b/sound/pci/hda/patch_cs8409.c
> @@ -634,6 +634,30 @@ static void cs42l42_run_jack_detect(struct sub_codec *cs42l42)
>  	cs8409_i2c_write(cs42l42, 0x1120, 0xc0);
>  }
>  
> +static void cs42l42_run_jack_detect_all(struct hda_codec *codec)
> +{
> +	struct cs8409_spec *spec = codec->spec;
> +	struct sub_codec *cs42l42;
> +	int i;
> +
> +	for (i = 0; i < spec->num_scodecs; i++) {
> +		cs42l42 = spec->scodecs[i];
> +		cs42l42_enable_jack_detect(cs42l42);
> +		if (!cs42l42->hp_jack_in)
> +			cs42l42_run_jack_detect(cs42l42);
> +	}
> +}
> +
> +/*
> + * cs42l42_jack_detect_worker - Worker that retries jack detect
> + */
> +static void cs42l42_jack_detect_worker(struct work_struct *work)
> +{
> +	struct cs8409_spec *spec = container_of(work, struct cs8409_spec, jack_detect_work.work);
> +
> +	cs42l42_run_jack_detect_all(spec->codec);
> +}
> +
>  static int cs42l42_handle_tip_sense(struct sub_codec *cs42l42, unsigned int reg_ts_status)
>  {
>  	int status_changed = 0;
> @@ -749,8 +773,6 @@ static void cs42l42_resume(struct sub_codec *cs42l42)
>  
>  	if (cs42l42->full_scale_vol)
>  		cs8409_i2c_write(cs42l42, 0x2001, 0x01);
> -
> -	cs42l42_enable_jack_detect(cs42l42);
>  }
>  
>  #ifdef CONFIG_PM
> @@ -800,6 +822,7 @@ static void cs8409_free(struct hda_codec *codec)
>  
>  	/* Cancel i2c clock disable timer, and disable clock if left enabled */
>  	cancel_delayed_work_sync(&spec->i2c_clk_work);
> +	cancel_delayed_work_sync(&spec->jack_detect_work);
>  	cs8409_disable_i2c_clock(codec);
>  
>  	snd_hda_gen_free(codec);
> @@ -863,6 +886,7 @@ static int cs8409_cs42l42_suspend(struct hda_codec *codec)
>  
>  	/* Cancel i2c clock disable timer, and disable clock if left enabled */
>  	cancel_delayed_work_sync(&spec->i2c_clk_work);
> +	cancel_delayed_work_sync(&spec->jack_detect_work);
>  	cs8409_disable_i2c_clock(codec);
>  
>  	snd_hda_shutup_pins(codec);
> @@ -970,6 +994,8 @@ void cs8409_cs42l42_fixups(struct hda_codec *codec, const struct hda_fixup *fix,
>  		spec->scodecs[CS8409_CODEC0]->codec = codec;
>  		codec->patch_ops = cs8409_cs42l42_patch_ops;
>  
> +		INIT_DELAYED_WORK(&spec->jack_detect_work, cs42l42_jack_detect_worker);
> +
>  		spec->gen.suppress_auto_mute = 1;
>  		spec->gen.no_primary_hp = 1;
>  		spec->gen.suppress_vmaster = 1;
> @@ -1029,9 +1055,16 @@ void cs8409_cs42l42_fixups(struct hda_codec *codec, const struct hda_fixup *fix,
>  	case HDA_FIXUP_ACT_INIT:
>  		cs8409_cs42l42_hw_init(codec);
>  		spec->init_done = 1;
> -		if (spec->init_done && spec->build_ctrl_done
> -			&& !spec->scodecs[CS8409_CODEC0]->hp_jack_in)
> -			cs42l42_run_jack_detect(spec->scodecs[CS8409_CODEC0]);
> +		if (spec->init_done && spec->build_ctrl_done) {
> +			/* No point in running jack detect until we have fully resumed */
> +			if (codec->core.dev.power.power_state.event != PM_EVENT_ON) {
> +				codec_warn(codec, "Not ready to detect jack, deferring...\n");
> +				schedule_delayed_work(&spec->jack_detect_work, msecs_to_jiffies(25));
> +				return;
> +			} else {
> +				cs42l42_run_jack_detect_all(codec);
> +			}
> +		}
>  		break;
>  	case HDA_FIXUP_ACT_BUILD:
>  		spec->build_ctrl_done = 1;
> @@ -1040,9 +1073,16 @@ void cs8409_cs42l42_fixups(struct hda_codec *codec, const struct hda_fixup *fix,
>  		 * been already plugged in.
>  		 * Run immediately after init.
>  		 */
> -		if (spec->init_done && spec->build_ctrl_done
> -			&& !spec->scodecs[CS8409_CODEC0]->hp_jack_in)
> -			cs42l42_run_jack_detect(spec->scodecs[CS8409_CODEC0]);
> +		if (spec->init_done && spec->build_ctrl_done) {
> +			/* No point in running jack detect until we have fully resumed */
> +			if (codec->core.dev.power.power_state.event != PM_EVENT_ON) {
> +				codec_warn(codec, "Not ready to detect jack, deferring...\n");
> +				schedule_delayed_work(&spec->jack_detect_work, msecs_to_jiffies(25));
> +				return;
> +			} else {
> +				cs42l42_run_jack_detect_all(codec);
> +			}
> +		}
>  		break;
>  	default:
>  		break;
> @@ -1178,7 +1218,6 @@ void dolphin_fixups(struct hda_codec *codec, const struct hda_fixup *fix, int ac
>  {
>  	struct cs8409_spec *spec = codec->spec;
>  	struct snd_kcontrol_new *kctrl;
> -	int i;
>  
>  	switch (action) {
>  	case HDA_FIXUP_ACT_PRE_PROBE:
> @@ -1193,6 +1232,8 @@ void dolphin_fixups(struct hda_codec *codec, const struct hda_fixup *fix, int ac
>  		spec->scodecs[CS8409_CODEC1]->codec = codec;
>  		spec->num_scodecs = 2;
>  
> +		INIT_DELAYED_WORK(&spec->jack_detect_work, cs42l42_jack_detect_worker);
> +
>  		codec->patch_ops = cs8409_dolphin_patch_ops;
>  
>  		/* GPIO 1,5 out, 0,4 in */
> @@ -1237,9 +1278,13 @@ void dolphin_fixups(struct hda_codec *codec, const struct hda_fixup *fix, int ac
>  		dolphin_hw_init(codec);
>  		spec->init_done = 1;
>  		if (spec->init_done && spec->build_ctrl_done) {
> -			for (i = 0; i < spec->num_scodecs; i++) {
> -				if (!spec->scodecs[i]->hp_jack_in)
> -					cs42l42_run_jack_detect(spec->scodecs[i]);
> +			/* No point in running jack detect until we have fully resumed */
> +			if (codec->core.dev.power.power_state.event != PM_EVENT_ON) {
> +				codec_warn(codec, "Not ready to detect jack, deferring...\n");
> +				schedule_delayed_work(&spec->jack_detect_work, msecs_to_jiffies(25));
> +				return;
> +			} else {
> +				cs42l42_run_jack_detect_all(codec);
>  			}
>  		}
>  		break;
> @@ -1251,9 +1296,13 @@ void dolphin_fixups(struct hda_codec *codec, const struct hda_fixup *fix, int ac
>  		 * Run immediately after init.
>  		 */
>  		if (spec->init_done && spec->build_ctrl_done) {
> -			for (i = 0; i < spec->num_scodecs; i++) {
> -				if (!spec->scodecs[i]->hp_jack_in)
> -					cs42l42_run_jack_detect(spec->scodecs[i]);
> +			/* No point in running jack detect until we have fully resumed */
> +			if (codec->core.dev.power.power_state.event != PM_EVENT_ON) {
> +				codec_warn(codec, "Not ready to detect jack, deferring...\n");
> +				schedule_delayed_work(&spec->jack_detect_work, msecs_to_jiffies(25));
> +				return;
> +			} else {
> +				cs42l42_run_jack_detect_all(codec);
>  			}
>  		}
>  		break;
> diff --git a/sound/pci/hda/patch_cs8409.h b/sound/pci/hda/patch_cs8409.h
> index ade2b838590c..632d3ec8322d 100644
> --- a/sound/pci/hda/patch_cs8409.h
> +++ b/sound/pci/hda/patch_cs8409.h
> @@ -330,6 +330,7 @@ struct cs8409_spec {
>  	unsigned int i2c_clck_enabled;
>  	unsigned int dev_addr;
>  	struct delayed_work i2c_clk_work;
> +	struct delayed_work jack_detect_work;
>  
>  	unsigned int playback_started:1;
>  	unsigned int capture_started:1;
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/sound/pci/hda/patch_cs8409.c b/sound/pci/hda/patch_cs8409.c
index 31ff11ab868e..88213e95f0b3 100644
--- a/sound/pci/hda/patch_cs8409.c
+++ b/sound/pci/hda/patch_cs8409.c
@@ -634,6 +634,30 @@  static void cs42l42_run_jack_detect(struct sub_codec *cs42l42)
 	cs8409_i2c_write(cs42l42, 0x1120, 0xc0);
 }
 
+static void cs42l42_run_jack_detect_all(struct hda_codec *codec)
+{
+	struct cs8409_spec *spec = codec->spec;
+	struct sub_codec *cs42l42;
+	int i;
+
+	for (i = 0; i < spec->num_scodecs; i++) {
+		cs42l42 = spec->scodecs[i];
+		cs42l42_enable_jack_detect(cs42l42);
+		if (!cs42l42->hp_jack_in)
+			cs42l42_run_jack_detect(cs42l42);
+	}
+}
+
+/*
+ * cs42l42_jack_detect_worker - Worker that retries jack detect
+ */
+static void cs42l42_jack_detect_worker(struct work_struct *work)
+{
+	struct cs8409_spec *spec = container_of(work, struct cs8409_spec, jack_detect_work.work);
+
+	cs42l42_run_jack_detect_all(spec->codec);
+}
+
 static int cs42l42_handle_tip_sense(struct sub_codec *cs42l42, unsigned int reg_ts_status)
 {
 	int status_changed = 0;
@@ -749,8 +773,6 @@  static void cs42l42_resume(struct sub_codec *cs42l42)
 
 	if (cs42l42->full_scale_vol)
 		cs8409_i2c_write(cs42l42, 0x2001, 0x01);
-
-	cs42l42_enable_jack_detect(cs42l42);
 }
 
 #ifdef CONFIG_PM
@@ -800,6 +822,7 @@  static void cs8409_free(struct hda_codec *codec)
 
 	/* Cancel i2c clock disable timer, and disable clock if left enabled */
 	cancel_delayed_work_sync(&spec->i2c_clk_work);
+	cancel_delayed_work_sync(&spec->jack_detect_work);
 	cs8409_disable_i2c_clock(codec);
 
 	snd_hda_gen_free(codec);
@@ -863,6 +886,7 @@  static int cs8409_cs42l42_suspend(struct hda_codec *codec)
 
 	/* Cancel i2c clock disable timer, and disable clock if left enabled */
 	cancel_delayed_work_sync(&spec->i2c_clk_work);
+	cancel_delayed_work_sync(&spec->jack_detect_work);
 	cs8409_disable_i2c_clock(codec);
 
 	snd_hda_shutup_pins(codec);
@@ -970,6 +994,8 @@  void cs8409_cs42l42_fixups(struct hda_codec *codec, const struct hda_fixup *fix,
 		spec->scodecs[CS8409_CODEC0]->codec = codec;
 		codec->patch_ops = cs8409_cs42l42_patch_ops;
 
+		INIT_DELAYED_WORK(&spec->jack_detect_work, cs42l42_jack_detect_worker);
+
 		spec->gen.suppress_auto_mute = 1;
 		spec->gen.no_primary_hp = 1;
 		spec->gen.suppress_vmaster = 1;
@@ -1029,9 +1055,16 @@  void cs8409_cs42l42_fixups(struct hda_codec *codec, const struct hda_fixup *fix,
 	case HDA_FIXUP_ACT_INIT:
 		cs8409_cs42l42_hw_init(codec);
 		spec->init_done = 1;
-		if (spec->init_done && spec->build_ctrl_done
-			&& !spec->scodecs[CS8409_CODEC0]->hp_jack_in)
-			cs42l42_run_jack_detect(spec->scodecs[CS8409_CODEC0]);
+		if (spec->init_done && spec->build_ctrl_done) {
+			/* No point in running jack detect until we have fully resumed */
+			if (codec->core.dev.power.power_state.event != PM_EVENT_ON) {
+				codec_warn(codec, "Not ready to detect jack, deferring...\n");
+				schedule_delayed_work(&spec->jack_detect_work, msecs_to_jiffies(25));
+				return;
+			} else {
+				cs42l42_run_jack_detect_all(codec);
+			}
+		}
 		break;
 	case HDA_FIXUP_ACT_BUILD:
 		spec->build_ctrl_done = 1;
@@ -1040,9 +1073,16 @@  void cs8409_cs42l42_fixups(struct hda_codec *codec, const struct hda_fixup *fix,
 		 * been already plugged in.
 		 * Run immediately after init.
 		 */
-		if (spec->init_done && spec->build_ctrl_done
-			&& !spec->scodecs[CS8409_CODEC0]->hp_jack_in)
-			cs42l42_run_jack_detect(spec->scodecs[CS8409_CODEC0]);
+		if (spec->init_done && spec->build_ctrl_done) {
+			/* No point in running jack detect until we have fully resumed */
+			if (codec->core.dev.power.power_state.event != PM_EVENT_ON) {
+				codec_warn(codec, "Not ready to detect jack, deferring...\n");
+				schedule_delayed_work(&spec->jack_detect_work, msecs_to_jiffies(25));
+				return;
+			} else {
+				cs42l42_run_jack_detect_all(codec);
+			}
+		}
 		break;
 	default:
 		break;
@@ -1178,7 +1218,6 @@  void dolphin_fixups(struct hda_codec *codec, const struct hda_fixup *fix, int ac
 {
 	struct cs8409_spec *spec = codec->spec;
 	struct snd_kcontrol_new *kctrl;
-	int i;
 
 	switch (action) {
 	case HDA_FIXUP_ACT_PRE_PROBE:
@@ -1193,6 +1232,8 @@  void dolphin_fixups(struct hda_codec *codec, const struct hda_fixup *fix, int ac
 		spec->scodecs[CS8409_CODEC1]->codec = codec;
 		spec->num_scodecs = 2;
 
+		INIT_DELAYED_WORK(&spec->jack_detect_work, cs42l42_jack_detect_worker);
+
 		codec->patch_ops = cs8409_dolphin_patch_ops;
 
 		/* GPIO 1,5 out, 0,4 in */
@@ -1237,9 +1278,13 @@  void dolphin_fixups(struct hda_codec *codec, const struct hda_fixup *fix, int ac
 		dolphin_hw_init(codec);
 		spec->init_done = 1;
 		if (spec->init_done && spec->build_ctrl_done) {
-			for (i = 0; i < spec->num_scodecs; i++) {
-				if (!spec->scodecs[i]->hp_jack_in)
-					cs42l42_run_jack_detect(spec->scodecs[i]);
+			/* No point in running jack detect until we have fully resumed */
+			if (codec->core.dev.power.power_state.event != PM_EVENT_ON) {
+				codec_warn(codec, "Not ready to detect jack, deferring...\n");
+				schedule_delayed_work(&spec->jack_detect_work, msecs_to_jiffies(25));
+				return;
+			} else {
+				cs42l42_run_jack_detect_all(codec);
 			}
 		}
 		break;
@@ -1251,9 +1296,13 @@  void dolphin_fixups(struct hda_codec *codec, const struct hda_fixup *fix, int ac
 		 * Run immediately after init.
 		 */
 		if (spec->init_done && spec->build_ctrl_done) {
-			for (i = 0; i < spec->num_scodecs; i++) {
-				if (!spec->scodecs[i]->hp_jack_in)
-					cs42l42_run_jack_detect(spec->scodecs[i]);
+			/* No point in running jack detect until we have fully resumed */
+			if (codec->core.dev.power.power_state.event != PM_EVENT_ON) {
+				codec_warn(codec, "Not ready to detect jack, deferring...\n");
+				schedule_delayed_work(&spec->jack_detect_work, msecs_to_jiffies(25));
+				return;
+			} else {
+				cs42l42_run_jack_detect_all(codec);
 			}
 		}
 		break;
diff --git a/sound/pci/hda/patch_cs8409.h b/sound/pci/hda/patch_cs8409.h
index ade2b838590c..632d3ec8322d 100644
--- a/sound/pci/hda/patch_cs8409.h
+++ b/sound/pci/hda/patch_cs8409.h
@@ -330,6 +330,7 @@  struct cs8409_spec {
 	unsigned int i2c_clck_enabled;
 	unsigned int dev_addr;
 	struct delayed_work i2c_clk_work;
+	struct delayed_work jack_detect_work;
 
 	unsigned int playback_started:1;
 	unsigned int capture_started:1;