Re: [PATCH] ALSA: hda/cs8409: Schedule delayed work for jack detect on resume
From: Takashi Iwai
Date: Thu Nov 25 2021 - 02:18:41 EST
On Wed, 24 Nov 2021 19:19:08 +0100,
Vitaly Rodionov wrote:
>
> From: Stefan Binding <sbinding@xxxxxxxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Vitaly Rodionov <vitalyr@xxxxxxxxxxxxxxxxxxxxx>
> ---
> 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
>