[alsa-devel] [PATCH 1/2] ALSA: hdmi - poll eld at resume time
Hdmi driver may not receive intrinsic event from gfx side when it's in runtime suspend mode. There's no ELD info when exit from runtime suspend. This patch avoid missing ELD info. Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com> --- sound/pci/hda/patch_hdmi.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 7803ddd..cb8ac66 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1868,12 +1868,33 @@ static void generic_hdmi_free(struct hda_codec *codec) kfree(spec); } +#ifdef CONFIG_PM +static int generic_hdmi_resume(struct hda_codec *codec) +{ + struct hdmi_spec *spec = codec->spec; + int pin_idx; + + generic_hdmi_init(codec); + snd_hda_codec_resume_amp(codec); + snd_hda_codec_resume_cache(codec); + + for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { + struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); + hdmi_present_sense(per_pin, 1); + } + return 0; +} +#endif + static const struct hda_codec_ops generic_hdmi_patch_ops = { .init = generic_hdmi_init, .free = generic_hdmi_free, .build_pcms = generic_hdmi_build_pcms, .build_controls = generic_hdmi_build_controls, .unsol_event = hdmi_unsol_event, +#ifdef CONFIG_PM + .resume = generic_hdmi_resume, +#endif }; -- 1.8.1.2
Hi Takashi, The two patches fix below issues: 1) controller/codec enter suspend mode, insert monitor, gfx side update eld info, but eld# has no valid eld info in audio driver side. 2) hdmi monitor connected, eld# info keep valid, controller/codec enter suspend mode, remove hdmi monitor, eld# still exist. thanks --xingchao
-----Original Message----- From: Wang Xingchao [mailto:xingchao.wang@linux.intel.com] Sent: Monday, June 24, 2013 7:45 PM To: tiwai@suse.de Cc: alsa-devel@alsa-project.org; Wang, Xingchao; Wang Xingchao Subject: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
Hdmi driver may not receive intrinsic event from gfx side when it's in runtime suspend mode. There's no ELD info when exit from runtime suspend. This patch avoid missing ELD info.
Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com> --- sound/pci/hda/patch_hdmi.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 7803ddd..cb8ac66 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1868,12 +1868,33 @@ static void generic_hdmi_free(struct hda_codec *codec) kfree(spec); }
+#ifdef CONFIG_PM +static int generic_hdmi_resume(struct hda_codec *codec) { + struct hdmi_spec *spec = codec->spec; + int pin_idx; + + generic_hdmi_init(codec); + snd_hda_codec_resume_amp(codec); + snd_hda_codec_resume_cache(codec); + + for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { + struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); + hdmi_present_sense(per_pin, 1); + } + return 0; +} +#endif + static const struct hda_codec_ops generic_hdmi_patch_ops = { .init = generic_hdmi_init, .free = generic_hdmi_free, .build_pcms = generic_hdmi_build_pcms, .build_controls = generic_hdmi_build_controls, .unsol_event = hdmi_unsol_event, +#ifdef CONFIG_PM + .resume = generic_hdmi_resume, +#endif };
-- 1.8.1.2
At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
Hdmi driver may not receive intrinsic event from gfx side when it's in runtime suspend mode. There's no ELD info when exit from runtime suspend. This patch avoid missing ELD info.
hda_call_codec_resume() sets the jack detection all dirty, thus each jack detection callback should be called at resume. Didn't it work as expected? Takashi
Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com> --- sound/pci/hda/patch_hdmi.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 7803ddd..cb8ac66 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1868,12 +1868,33 @@ static void generic_hdmi_free(struct hda_codec *codec) kfree(spec); }
+#ifdef CONFIG_PM +static int generic_hdmi_resume(struct hda_codec *codec) +{ + struct hdmi_spec *spec = codec->spec; + int pin_idx; + + generic_hdmi_init(codec); + snd_hda_codec_resume_amp(codec); + snd_hda_codec_resume_cache(codec); + + for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { + struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); + hdmi_present_sense(per_pin, 1); + } + return 0; +} +#endif + static const struct hda_codec_ops generic_hdmi_patch_ops = { .init = generic_hdmi_init, .free = generic_hdmi_free, .build_pcms = generic_hdmi_build_pcms, .build_controls = generic_hdmi_build_controls, .unsol_event = hdmi_unsol_event, +#ifdef CONFIG_PM + .resume = generic_hdmi_resume, +#endif };
-- 1.8.1.2
At Mon, 24 Jun 2013 07:45:24 -0400, Wang Xingchao wrote:
when controller/codec in runtime suspended mode, monitor hotplug would not trigger unsolicited event. This patch tries to power up codec and hdmi driver would probe ELD info again.
I don't know whether this is the wanted behavior. The proc file is supposed to read the driver's status, not to trigger anything. Takashi
Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com> --- sound/pci/hda/hda_eld.c | 6 ++++++ sound/pci/hda/hda_local.h | 1 + 2 files changed, 7 insertions(+)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index d0d7ac1..914712a 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -482,6 +482,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, struct snd_info_buffer *buffer) { struct hdmi_eld *eld = entry->private_data; + struct hda_codec *codec = eld->codec; struct parsed_hdmi_eld *e = &eld->info; char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE]; int i; @@ -500,11 +501,14 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, [4 ... 7] = "reserved" };
+ snd_hda_power_up(codec); + mutex_lock(&eld->lock); snd_iprintf(buffer, "monitor_present\t\t%d\n", eld->monitor_present); snd_iprintf(buffer, "eld_valid\t\t%d\n", eld->eld_valid); if (!eld->eld_valid) { mutex_unlock(&eld->lock); + snd_hda_power_down(codec); return; } snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name); @@ -529,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, for (i = 0; i < e->sad_count; i++) hdmi_print_sad_info(i, e->sad + i, buffer); mutex_unlock(&eld->lock); + snd_hda_power_down(codec); }
static void hdmi_write_eld_info(struct snd_info_entry *entry, @@ -614,6 +619,7 @@ int snd_hda_eld_proc_new(struct hda_codec *codec, struct hdmi_eld *eld, entry->c.text.write = hdmi_write_eld_info; entry->mode |= S_IWUSR; eld->proc_entry = entry; + eld->codec = codec;
return 0; } diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index e0bf753..1aaf8b6 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -747,6 +747,7 @@ struct hdmi_eld { #ifdef CONFIG_PROC_FS struct snd_info_entry *proc_entry; #endif + struct hda_codec *codec; };
int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid); -- 1.8.1.2
when controller/codec in runtime suspended mode, monitor hotplug would not trigger unsolicited event. This patch tries to power up codec and hdmi driver would probe ELD info again. Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com> --- sound/pci/hda/hda_eld.c | 6 ++++++ sound/pci/hda/hda_local.h | 1 + 2 files changed, 7 insertions(+) diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index d0d7ac1..914712a 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -482,6 +482,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, struct snd_info_buffer *buffer) { struct hdmi_eld *eld = entry->private_data; + struct hda_codec *codec = eld->codec; struct parsed_hdmi_eld *e = &eld->info; char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE]; int i; @@ -500,11 +501,14 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, [4 ... 7] = "reserved" }; + snd_hda_power_up(codec); + mutex_lock(&eld->lock); snd_iprintf(buffer, "monitor_present\t\t%d\n", eld->monitor_present); snd_iprintf(buffer, "eld_valid\t\t%d\n", eld->eld_valid); if (!eld->eld_valid) { mutex_unlock(&eld->lock); + snd_hda_power_down(codec); return; } snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name); @@ -529,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, for (i = 0; i < e->sad_count; i++) hdmi_print_sad_info(i, e->sad + i, buffer); mutex_unlock(&eld->lock); + snd_hda_power_down(codec); } static void hdmi_write_eld_info(struct snd_info_entry *entry, @@ -614,6 +619,7 @@ int snd_hda_eld_proc_new(struct hda_codec *codec, struct hdmi_eld *eld, entry->c.text.write = hdmi_write_eld_info; entry->mode |= S_IWUSR; eld->proc_entry = entry; + eld->codec = codec; return 0; } diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index e0bf753..1aaf8b6 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -747,6 +747,7 @@ struct hdmi_eld { #ifdef CONFIG_PROC_FS struct snd_info_entry *proc_entry; #endif + struct hda_codec *codec; }; int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid); -- 1.8.1.2
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:36 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
At Mon, 24 Jun 2013 07:45:24 -0400, Wang Xingchao wrote:
when controller/codec in runtime suspended mode, monitor hotplug would not trigger unsolicited event. This patch tries to power up codec and hdmi driver would probe ELD info again.
I don't know whether this is the wanted behavior. The proc file is supposed to read the driver's status, not to trigger anything.
I explained in another mail what the patch fix. In the real case, when HDMI monitor removed, Eld# still hold monitor information because the codec/controller is in suspend mode atm and has no chance to refresh eld. Even when controller/codec waken up, it will not refresh eld# as gfx side handled hdmi hotplut when audio driver suspended. Then user will always see eld valid info but in fact the monitor is removed already, obviously it's a bug. Thanks --xingchao
Takashi
Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com> --- sound/pci/hda/hda_eld.c | 6 ++++++ sound/pci/hda/hda_local.h | 1 + 2 files changed, 7 insertions(+)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index d0d7ac1..914712a 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -482,6 +482,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, struct snd_info_buffer *buffer) { struct hdmi_eld *eld = entry->private_data; + struct hda_codec *codec = eld->codec; struct parsed_hdmi_eld *e = &eld->info; char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE]; int i; @@ -500,11 +501,14 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, [4 ... 7] = "reserved" };
+ snd_hda_power_up(codec); + mutex_lock(&eld->lock); snd_iprintf(buffer, "monitor_present\t\t%d\n", eld->monitor_present); snd_iprintf(buffer, "eld_valid\t\t%d\n", eld->eld_valid); if (!eld->eld_valid) { mutex_unlock(&eld->lock); + snd_hda_power_down(codec); return; } snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name); @@ -529,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, for (i = 0; i < e->sad_count; i++) hdmi_print_sad_info(i, e->sad + i, buffer); mutex_unlock(&eld->lock); + snd_hda_power_down(codec); }
static void hdmi_write_eld_info(struct snd_info_entry *entry, @@ -614,6 +619,7 @@ int snd_hda_eld_proc_new(struct hda_codec *codec, struct hdmi_eld *eld, entry->c.text.write = hdmi_write_eld_info; entry->mode |= S_IWUSR; eld->proc_entry = entry; + eld->codec = codec;
return 0; } diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index e0bf753..1aaf8b6 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -747,6 +747,7 @@ struct hdmi_eld { #ifdef CONFIG_PROC_FS struct snd_info_entry *proc_entry; #endif + struct hda_codec *codec; };
int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid); -- 1.8.1.2
At Mon, 24 Jun 2013 11:56:21 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:36 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
At Mon, 24 Jun 2013 07:45:24 -0400, Wang Xingchao wrote:
when controller/codec in runtime suspended mode, monitor hotplug would not trigger unsolicited event. This patch tries to power up codec and hdmi driver would probe ELD info again.
I don't know whether this is the wanted behavior. The proc file is supposed to read the driver's status, not to trigger anything.
I explained in another mail what the patch fix. In the real case, when HDMI monitor removed, Eld# still hold monitor information because the codec/controller is in suspend mode atm and has no chance to refresh eld.
And this is no bug, per se. The proc file shows the driver's current status, not the hardware's raw status.
Even when controller/codec waken up, it will not refresh eld# as gfx side hand led hdmi hotplut when audio driver suspended.
... and this is actually the bug. But, it's still unclear why the jack detection callback isn't called at resume. Takashi
Then user will always see eld valid info but in fact the monitor is removed already, obviously it's a bug.
Thanks --xingchao
Takashi
Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com> --- sound/pci/hda/hda_eld.c | 6 ++++++ sound/pci/hda/hda_local.h | 1 + 2 files changed, 7 insertions(+)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index d0d7ac1..914712a 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -482,6 +482,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, struct snd_info_buffer *buffer) { struct hdmi_eld *eld = entry->private_data; + struct hda_codec *codec = eld->codec; struct parsed_hdmi_eld *e = &eld->info; char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE]; int i; @@ -500,11 +501,14 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, [4 ... 7] = "reserved" };
+ snd_hda_power_up(codec); + mutex_lock(&eld->lock); snd_iprintf(buffer, "monitor_present\t\t%d\n", eld->monitor_present); snd_iprintf(buffer, "eld_valid\t\t%d\n", eld->eld_valid); if (!eld->eld_valid) { mutex_unlock(&eld->lock); + snd_hda_power_down(codec); return; } snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name); @@ -529,6 +533,7 @@ static void hdmi_print_eld_info(struct snd_info_entry *entry, for (i = 0; i < e->sad_count; i++) hdmi_print_sad_info(i, e->sad + i, buffer); mutex_unlock(&eld->lock); + snd_hda_power_down(codec); }
static void hdmi_write_eld_info(struct snd_info_entry *entry, @@ -614,6 +619,7 @@ int snd_hda_eld_proc_new(struct hda_codec *codec, struct hdmi_eld *eld, entry->c.text.write = hdmi_write_eld_info; entry->mode |= S_IWUSR; eld->proc_entry = entry; + eld->codec = codec;
return 0; } diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index e0bf753..1aaf8b6 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -747,6 +747,7 @@ struct hdmi_eld { #ifdef CONFIG_PROC_FS struct snd_info_entry *proc_entry; #endif + struct hda_codec *codec; };
int snd_hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid); -- 1.8.1.2
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:33 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
Hdmi driver may not receive intrinsic event from gfx side when it's in runtime suspend mode. There's no ELD info when exit from runtime suspend. This patch avoid missing ELD info.
hda_call_codec_resume() sets the jack detection all dirty, thus each jack detection callback should be called at resume. Didn't it work as expected?
I would double check that. In my test, it doesnot work as expected. --xingchao
Takashi
Signed-off-by: Wang Xingchao <xingchao.wang@linux.intel.com> --- sound/pci/hda/patch_hdmi.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 7803ddd..cb8ac66 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1868,12 +1868,33 @@ static void generic_hdmi_free(struct hda_codec
*codec)
kfree(spec); }
+#ifdef CONFIG_PM +static int generic_hdmi_resume(struct hda_codec *codec) { + struct hdmi_spec *spec = codec->spec; + int pin_idx; + + generic_hdmi_init(codec); + snd_hda_codec_resume_amp(codec); + snd_hda_codec_resume_cache(codec); + + for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { + struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); + hdmi_present_sense(per_pin, 1); + } + return 0; +} +#endif + static const struct hda_codec_ops generic_hdmi_patch_ops = { .init = generic_hdmi_init, .free = generic_hdmi_free, .build_pcms = generic_hdmi_build_pcms, .build_controls = generic_hdmi_build_controls, .unsol_event = hdmi_unsol_event, +#ifdef CONFIG_PM + .resume = generic_hdmi_resume, +#endif };
-- 1.8.1.2
On 06/24/2013 02:00 PM, Takashi Iwai wrote:
At Mon, 24 Jun 2013 11:56:21 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:36 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
At Mon, 24 Jun 2013 07:45:24 -0400, Wang Xingchao wrote:
when controller/codec in runtime suspended mode, monitor hotplug would not trigger unsolicited event.
And here's the original problem IMO: If you can't detect plug/unplug in runtime suspend mode, the runtime suspend mode is broken. Userspace relies on getting notification when a HDMI/DP connection is plugged/unplugged. If no unsol event is triggered, how is userspace notified? This patch tries to power up codec and
hdmi driver would probe ELD info again.
I don't know whether this is the wanted behavior. The proc file is supposed to read the driver's status, not to trigger anything.
I explained in another mail what the patch fix. In the real case, when HDMI monitor removed, Eld# still hold monitor information because the codec/controller is in suspend mode atm and has no chance to refresh eld.
And this is no bug, per se. The proc file shows the driver's current status, not the hardware's raw status.
Well, the codec proc file shows the hardware's raw status (which includes powering up the codec), perhaps it would be more consistent if the eld proc file did the same?
Even when controller/codec waken up, it will not refresh eld# as gfx side hand led hdmi hotplut when audio driver suspended.
... and this is actually the bug. But, it's still unclear why the jack detection callback isn't called at resume.
Well, I haven't double checked, but is there anyone actually calling it? Just setting a jack to dirty does not cause any action in itself. And btw, even if the proc file might show old information, we also have the ELD bytes ctrl, which IMO should always correctly return 0 if there's nothing connected. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
At Mon, 24 Jun 2013 12:19:42 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:33 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
Hdmi driver may not receive intrinsic event from gfx side when it's in runtime suspend mode. There's no ELD info when exit from runtime suspend. This patch avoid missing ELD info.
hda_call_codec_resume() sets the jack detection all dirty, thus each jack detection callback should be called at resume. Didn't it work as expected?
I would double check that. In my test, it doesnot work as expected.
OK, I found the problem. patch_hdmi.c enables the jack detection stuff without the callback, so the resume code triggers the check of jack detection but only updates the kcontrols. How about the patch below instead? Takashi --- diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 8428763..3059d69 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -950,14 +950,10 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, int pin_idx, * Unsolicited events */ -static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll); - static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) { - struct hdmi_spec *spec = codec->spec; int tag = res >> AC_UNSOL_RES_TAG_SHIFT; int pin_nid; - int pin_idx; struct hda_jack_tbl *jack; jack = snd_hda_jack_tbl_get_from_tag(codec, tag); @@ -970,12 +966,6 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) "HDMI hot plug event: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n", codec->addr, pin_nid, !!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV)); - - pin_idx = pin_nid_to_pin_index(spec, pin_nid); - if (pin_idx < 0) - return; - - hdmi_present_sense(get_pin(spec, pin_idx), 1); snd_hda_jack_report_sync(codec); } @@ -1358,6 +1348,14 @@ static void hdmi_repoll_eld(struct work_struct *work) hdmi_present_sense(per_pin, per_pin->repoll_count); } +static void hdmi_jack_detect_cb(struct hda_codec *codec, struct hda_jack_tbl *jack) +{ + struct hdmi_spec *spec = codec->spec; + int pin_idx = pin_nid_to_pin_index(spec, jack->nid); + if (pin_idx >= 0) + hdmi_present_sense(get_pin(spec, pin_idx), 1); +} + static void intel_haswell_fixup_connect_list(struct hda_codec *codec, hda_nid_t nid); @@ -1827,7 +1825,8 @@ static int generic_hdmi_init(struct hda_codec *codec) hda_nid_t pin_nid = per_pin->pin_nid; hdmi_init_pin(codec, pin_nid); - snd_hda_jack_detect_enable(codec, pin_nid, pin_nid); + snd_hda_jack_detect_enable_callback(codec, pin_nid, pin_nid, + hdmi_jack_detect_cb); } return 0; }
At Mon, 24 Jun 2013 14:47:01 +0200, David Henningsson wrote:
On 06/24/2013 02:00 PM, Takashi Iwai wrote:
At Mon, 24 Jun 2013 11:56:21 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:36 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
At Mon, 24 Jun 2013 07:45:24 -0400, Wang Xingchao wrote:
when controller/codec in runtime suspended mode, monitor hotplug would not trigger unsolicited event.
And here's the original problem IMO: If you can't detect plug/unplug in runtime suspend mode, the runtime suspend mode is broken. Userspace relies on getting notification when a HDMI/DP connection is plugged/unplugged. If no unsol event is triggered, how is userspace notified?
It can't, so far. The same thing happens for any other jack detections when the power-saving is turned on. There is a low power mode that allows the jack detection, but this is different from the aggressive power-saving with runtime D3. In theory, the audio driver can be resumed forcibly before unsolicited event is sent, once if we have a layer controlling the power of both graphics and audio devices. This is the thing to be done in future, together with the runtime PM control of the graphics side.
This patch tries to power up codec and
hdmi driver would probe ELD info again.
I don't know whether this is the wanted behavior. The proc file is supposed to read the driver's status, not to trigger anything.
I explained in another mail what the patch fix. In the real case, when HDMI monitor removed, Eld# still hold monitor information because the codec/controller is in suspend mode atm and has no chance to refresh eld.
And this is no bug, per se. The proc file shows the driver's current status, not the hardware's raw status.
Well, the codec proc file shows the hardware's raw status (which includes powering up the codec), perhaps it would be more consistent if the eld proc file did the same?
Well, the concept is different. The codec proc file shows the codec raw status, so it is. The eld proc file shows the content of the data the driver currently contains. It's good so for debugging purpose; otherwise the driver status would change just by reading this proc file.
Even when controller/codec waken up, it will not refresh eld# as gfx side hand led hdmi hotplut when audio driver suspended.
... and this is actually the bug. But, it's still unclear why the jack detection callback isn't called at resume.
Well, I haven't double checked, but is there anyone actually calling it? Just setting a jack to dirty does not cause any action in itself.
The culprit was found, see my previous reply.
And btw, even if the proc file might show old information, we also have the ELD bytes ctrl, which IMO should always correctly return 0 if there's nothing connected.
Right. thanks, Takashi
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 8:50 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 12:19:42 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:33 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
Hdmi driver may not receive intrinsic event from gfx side when it's in runtime suspend mode. There's no ELD info when exit from runtime suspend. This patch avoid missing ELD info.
hda_call_codec_resume() sets the jack detection all dirty, thus each jack detection callback should be called at resume. Didn't it work as
expected?
I would double check that. In my test, it doesnot work as expected.
OK, I found the problem. patch_hdmi.c enables the jack detection stuff without the callback, so the resume code triggers the check of jack detection but only updates the kcontrols.
You patch did not resolve the issue. I added some debug log, the callback wasnot called at all. I will continue to check it. Thanks --xingchao
How about the patch below instead?
Takashi
--- diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 8428763..3059d69 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -950,14 +950,10 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, int pin_idx, * Unsolicited events */
-static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll); - static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) { - struct hdmi_spec *spec = codec->spec; int tag = res >> AC_UNSOL_RES_TAG_SHIFT; int pin_nid; - int pin_idx; struct hda_jack_tbl *jack;
jack = snd_hda_jack_tbl_get_from_tag(codec, tag); @@ -970,12 +966,6 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) "HDMI hot plug event: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n", codec->addr, pin_nid, !!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV)); - - pin_idx = pin_nid_to_pin_index(spec, pin_nid); - if (pin_idx < 0) - return; - - hdmi_present_sense(get_pin(spec, pin_idx), 1); snd_hda_jack_report_sync(codec); }
@@ -1358,6 +1348,14 @@ static void hdmi_repoll_eld(struct work_struct *work) hdmi_present_sense(per_pin, per_pin->repoll_count); }
+static void hdmi_jack_detect_cb(struct hda_codec *codec, struct +hda_jack_tbl *jack) { + struct hdmi_spec *spec = codec->spec; + int pin_idx = pin_nid_to_pin_index(spec, jack->nid); + if (pin_idx >= 0) + hdmi_present_sense(get_pin(spec, pin_idx), 1); } + static void intel_haswell_fixup_connect_list(struct hda_codec *codec, hda_nid_t nid);
@@ -1827,7 +1825,8 @@ static int generic_hdmi_init(struct hda_codec *codec) hda_nid_t pin_nid = per_pin->pin_nid;
hdmi_init_pin(codec, pin_nid); - snd_hda_jack_detect_enable(codec, pin_nid, pin_nid); + snd_hda_jack_detect_enable_callback(codec, pin_nid, pin_nid, + hdmi_jack_detect_cb); } return 0; }
At Tue, 25 Jun 2013 04:54:05 +0000, Wang, Xingchao wrote:
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 8:50 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 12:19:42 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:33 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
Hdmi driver may not receive intrinsic event from gfx side when it's in runtime suspend mode. There's no ELD info when exit from runtime suspend. This patch avoid missing ELD info.
hda_call_codec_resume() sets the jack detection all dirty, thus each jack detection callback should be called at resume. Didn't it work as
expected?
I would double check that. In my test, it doesnot work as expected.
OK, I found the problem. patch_hdmi.c enables the jack detection stuff without the callback, so the resume code triggers the check of jack detection but only updates the kcontrols.
You patch did not resolve the issue. I added some debug log, the callback wasnot called at all.
Even if you unplugged while runtime suspend? The callback is called only when the plug status (i.e. the jack detection state) change is detected at the resume time -- i.e. the state the driver holds differs from the state at the resume. Takashi
I will continue to check it.
Thanks --xingchao
How about the patch below instead?
Takashi
--- diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 8428763..3059d69 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -950,14 +950,10 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, int pin_idx, * Unsolicited events */
-static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll); - static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) { - struct hdmi_spec *spec = codec->spec; int tag = res >> AC_UNSOL_RES_TAG_SHIFT; int pin_nid; - int pin_idx; struct hda_jack_tbl *jack;
jack = snd_hda_jack_tbl_get_from_tag(codec, tag); @@ -970,12 +966,6 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) "HDMI hot plug event: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n", codec->addr, pin_nid, !!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV)); - - pin_idx = pin_nid_to_pin_index(spec, pin_nid); - if (pin_idx < 0) - return; - - hdmi_present_sense(get_pin(spec, pin_idx), 1); snd_hda_jack_report_sync(codec); }
@@ -1358,6 +1348,14 @@ static void hdmi_repoll_eld(struct work_struct *work) hdmi_present_sense(per_pin, per_pin->repoll_count); }
+static void hdmi_jack_detect_cb(struct hda_codec *codec, struct +hda_jack_tbl *jack) { + struct hdmi_spec *spec = codec->spec; + int pin_idx = pin_nid_to_pin_index(spec, jack->nid); + if (pin_idx >= 0) + hdmi_present_sense(get_pin(spec, pin_idx), 1); } + static void intel_haswell_fixup_connect_list(struct hda_codec *codec, hda_nid_t nid);
@@ -1827,7 +1825,8 @@ static int generic_hdmi_init(struct hda_codec *codec) hda_nid_t pin_nid = per_pin->pin_nid;
hdmi_init_pin(codec, pin_nid); - snd_hda_jack_detect_enable(codec, pin_nid, pin_nid); + snd_hda_jack_detect_enable_callback(codec, pin_nid, pin_nid, + hdmi_jack_detect_cb); } return 0; }
-----Original Message----- From: Wang, Xingchao Sent: Tuesday, June 25, 2013 12:54 PM To: 'Takashi Iwai' Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: RE: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 8:50 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 12:19:42 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:33 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
Hdmi driver may not receive intrinsic event from gfx side when it's in runtime suspend mode. There's no ELD info when exit from runtime suspend. This patch avoid missing ELD info.
hda_call_codec_resume() sets the jack detection all dirty, thus each jack detection callback should be called at resume. Didn't it work as
expected?
I would double check that. In my test, it doesnot work as expected.
OK, I found the problem. patch_hdmi.c enables the jack detection stuff without the callback, so the resume code triggers the check of jack detection but only updates the kcontrols.
You patch did not resolve the issue. I added some debug log, the callback wasnot called at all. I will continue to check it.
Seems jackpoll_interval be 0 by default, and the callback was not called. I think this would be a common issue not limited to Haswell, so what about call had_jackpoll_work() in snd_hda_codec_resume()? Thanks --xingchao
Thanks --xingchao
How about the patch below instead?
Takashi
--- diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 8428763..3059d69 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -950,14 +950,10 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, int pin_idx, * Unsolicited events */
-static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll); - static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) { - struct hdmi_spec *spec = codec->spec; int tag = res >> AC_UNSOL_RES_TAG_SHIFT; int pin_nid; - int pin_idx; struct hda_jack_tbl *jack;
jack = snd_hda_jack_tbl_get_from_tag(codec, tag); @@ -970,12 +966,6 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) "HDMI hot plug event: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n", codec->addr, pin_nid, !!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV)); - - pin_idx = pin_nid_to_pin_index(spec, pin_nid); - if (pin_idx < 0) - return; - - hdmi_present_sense(get_pin(spec, pin_idx), 1); snd_hda_jack_report_sync(codec); }
@@ -1358,6 +1348,14 @@ static void hdmi_repoll_eld(struct work_struct *work) hdmi_present_sense(per_pin, per_pin->repoll_count); }
+static void hdmi_jack_detect_cb(struct hda_codec *codec, struct +hda_jack_tbl *jack) { + struct hdmi_spec *spec = codec->spec; + int pin_idx = pin_nid_to_pin_index(spec, jack->nid); + if (pin_idx >= 0) + hdmi_present_sense(get_pin(spec, pin_idx), 1); } + static void intel_haswell_fixup_connect_list(struct hda_codec *codec, hda_nid_t nid);
@@ -1827,7 +1825,8 @@ static int generic_hdmi_init(struct hda_codec *codec) hda_nid_t pin_nid = per_pin->pin_nid;
hdmi_init_pin(codec, pin_nid); - snd_hda_jack_detect_enable(codec, pin_nid, pin_nid); + snd_hda_jack_detect_enable_callback(codec, pin_nid, pin_nid, + hdmi_jack_detect_cb); } return 0; }
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, June 25, 2013 2:07 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Tue, 25 Jun 2013 04:54:05 +0000, Wang, Xingchao wrote:
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 8:50 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 12:19:42 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:33 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote:
Hdmi driver may not receive intrinsic event from gfx side when it's in runtime suspend mode. There's no ELD info when exit from runtime suspend. This patch avoid missing ELD info.
hda_call_codec_resume() sets the jack detection all dirty, thus each jack detection callback should be called at resume. Didn't it work as
expected?
I would double check that. In my test, it doesnot work as expected.
OK, I found the problem. patch_hdmi.c enables the jack detection stuff without the callback, so the resume code triggers the check of jack detection but only updates the kcontrols.
You patch did not resolve the issue. I added some debug log, the callback wasnot called at all.
Even if you unplugged while runtime suspend?
Yes, the controller/codec suspended in runtime already.
The callback is called only when the plug status (i.e. the jack detection state) change is detected at the resume time -- i.e. the state the driver holds differs from the state at the resume.
Do you assume jackpoll_interval be non-zero? If so the hda_jackpoll_work will continue to run periodically. IMO the callback just need be called only once at resume time. Thanks --xingchao
At Tue, 25 Jun 2013 06:34:49 +0000, Wang, Xingchao wrote:
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, June 25, 2013 2:07 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Tue, 25 Jun 2013 04:54:05 +0000, Wang, Xingchao wrote:
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 8:50 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 12:19:42 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:33 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote: > > Hdmi driver may not receive intrinsic event from gfx side when > it's in runtime suspend mode. There's no ELD info when exit > from runtime suspend. This patch avoid missing ELD info.
hda_call_codec_resume() sets the jack detection all dirty, thus each jack detection callback should be called at resume. Didn't it work as
expected?
I would double check that. In my test, it doesnot work as expected.
OK, I found the problem. patch_hdmi.c enables the jack detection stuff without the callback, so the resume code triggers the check of jack detection but only updates the kcontrols.
You patch did not resolve the issue. I added some debug log, the callback wasnot called at all.
Even if you unplugged while runtime suspend?
Yes, the controller/codec suspended in runtime already.
The callback is called only when the plug status (i.e. the jack detection state) change is detected at the resume time -- i.e. the state the driver holds differs from the state at the resume.
Do you assume jackpoll_interval be non-zero? If so the hda_jackpoll_work will continue to run periodically. IMO the callback just need be called only once at resume time.
Hmm, the problem is that the callback updater isn't called in the resume path as default. The oneliner below will fix it. But, looking through all changes, maybe your first patch is easier to apply as is now. I'll take it. We need a bit more cleanups over init and resume codes after all. The init callback is called before calling build_controls and build_pcm, and the very same init callback is called also in the resume path (if the resume callback is undefined). This causes the confusion, too. In patch_hdmi.c, the init changes only the pin control and amp, and can't touch anything else, because it cannot call any pin-related events at that point since the necessary resources are added in the later point, generic_hdmi_build_controls(). thanks, Takashi
Thanks --xingchao
At Tue, 25 Jun 2013 09:06:32 +0200, Takashi Iwai wrote:
At Tue, 25 Jun 2013 06:34:49 +0000, Wang, Xingchao wrote:
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, June 25, 2013 2:07 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Tue, 25 Jun 2013 04:54:05 +0000, Wang, Xingchao wrote:
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 8:50 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 12:19:42 +0000, Wang, Xingchao wrote:
> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Monday, June 24, 2013 7:33 PM > To: Wang Xingchao > Cc: alsa-devel@alsa-project.org; Wang, Xingchao > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time > > At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote: > > > > Hdmi driver may not receive intrinsic event from gfx side when > > it's in runtime suspend mode. There's no ELD info when exit > > from runtime suspend. This patch avoid missing ELD info. > > hda_call_codec_resume() sets the jack detection all dirty, thus > each jack detection callback should be called at resume. Didn't > it work as
expected?
I would double check that. In my test, it doesnot work as expected.
OK, I found the problem. patch_hdmi.c enables the jack detection stuff without the callback, so the resume code triggers the check of jack detection but only updates the kcontrols.
You patch did not resolve the issue. I added some debug log, the callback wasnot called at all.
Even if you unplugged while runtime suspend?
Yes, the controller/codec suspended in runtime already.
The callback is called only when the plug status (i.e. the jack detection state) change is detected at the resume time -- i.e. the state the driver holds differs from the state at the resume.
Do you assume jackpoll_interval be non-zero? If so the hda_jackpoll_work will continue to run periodically. IMO the callback just need be called only once at resume time.
Hmm, the problem is that the callback updater isn't called in the resume path as default. The oneliner below will fix it.
Forgot to attach... Takashi --- diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 35090b3..86d4709 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3942,6 +3942,7 @@ static void hda_call_codec_resume(struct hda_codec *codec) codec->patch_ops.init(codec); snd_hda_codec_resume_amp(codec); snd_hda_codec_resume_cache(codec); + snd_hda_jack_poll_all(codec); } if (codec->jackpoll_interval)
On 06/24/2013 03:00 PM, Takashi Iwai wrote:
At Mon, 24 Jun 2013 14:47:01 +0200, David Henningsson wrote:
On 06/24/2013 02:00 PM, Takashi Iwai wrote:
At Mon, 24 Jun 2013 11:56:21 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:36 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
At Mon, 24 Jun 2013 07:45:24 -0400, Wang Xingchao wrote:
when controller/codec in runtime suspended mode, monitor hotplug would not trigger unsolicited event.
And here's the original problem IMO: If you can't detect plug/unplug in runtime suspend mode, the runtime suspend mode is broken. Userspace relies on getting notification when a HDMI/DP connection is plugged/unplugged. If no unsol event is triggered, how is userspace notified?
It can't, so far. The same thing happens for any other jack detections when the power-saving is turned on.
There is a low power mode that allows the jack detection, but this is different from the aggressive power-saving with runtime D3.
If "aggressive power-saving with runtime D3" is the same as AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs connected to a Lynx point controller. It looks like userspace have problems getting notifications for e g headphone insertion on Lynx point controllers, so this is not only an HDMI/DP problem? Trying to read up a little on this, there seem to be an option to set the WAKEEN register to have jack detection working even when the controller is in D3. (refer HDA specification 4.5.9.2: Codec Wake From System S0, Controller D3.) But it seems we do not (yet) use this feature. Is this something that could/should be implemented to fix the jack detection problems that seems to be happening otherwise? -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
At Tue, 25 Jun 2013 09:45:04 +0200, David Henningsson wrote:
On 06/24/2013 03:00 PM, Takashi Iwai wrote:
At Mon, 24 Jun 2013 14:47:01 +0200, David Henningsson wrote:
On 06/24/2013 02:00 PM, Takashi Iwai wrote:
At Mon, 24 Jun 2013 11:56:21 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 7:36 PM To: Wang Xingchao Cc: alsa-devel@alsa-project.org; Wang, Xingchao Subject: Re: [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
At Mon, 24 Jun 2013 07:45:24 -0400, Wang Xingchao wrote: > > when controller/codec in runtime suspended mode, monitor hotplug would > not trigger unsolicited event.
And here's the original problem IMO: If you can't detect plug/unplug in runtime suspend mode, the runtime suspend mode is broken. Userspace relies on getting notification when a HDMI/DP connection is plugged/unplugged. If no unsol event is triggered, how is userspace notified?
It can't, so far. The same thing happens for any other jack detections when the power-saving is turned on.
There is a low power mode that allows the jack detection, but this is different from the aggressive power-saving with runtime D3.
If "aggressive power-saving with runtime D3" is the same as AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs connected to a Lynx point controller.
It looks like userspace have problems getting notifications for e g headphone insertion on Lynx point controllers, so this is not only an HDMI/DP problem?
Yes.
Trying to read up a little on this, there seem to be an option to set the WAKEEN register to have jack detection working even when the controller is in D3. (refer HDA specification 4.5.9.2: Codec Wake From System S0, Controller D3.) But it seems we do not (yet) use this feature. Is this something that could/should be implemented to fix the jack detection problems that seems to be happening otherwise?
It sounds feasible, at least for traditional jack detection of analog pins. But I'm not sure whether this would help for the Intel graphics case. Just need testing. Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, June 25, 2013 3:09 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Tue, 25 Jun 2013 09:06:32 +0200, Takashi Iwai wrote:
At Tue, 25 Jun 2013 06:34:49 +0000, Wang, Xingchao wrote:
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, June 25, 2013 2:07 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Tue, 25 Jun 2013 04:54:05 +0000, Wang, Xingchao wrote:
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, June 24, 2013 8:50 PM To: Wang, Xingchao Cc: alsa-devel@alsa-project.org; Wang Xingchao Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume time
At Mon, 24 Jun 2013 12:19:42 +0000, Wang, Xingchao wrote: > > > > > -----Original Message----- > > From: Takashi Iwai [mailto:tiwai@suse.de] > > Sent: Monday, June 24, 2013 7:33 PM > > To: Wang Xingchao > > Cc: alsa-devel@alsa-project.org; Wang, Xingchao > > Subject: Re: [PATCH 1/2] ALSA: hdmi - poll eld at resume > > time > > > > At Mon, 24 Jun 2013 07:45:23 -0400, Wang Xingchao wrote: > > > > > > Hdmi driver may not receive intrinsic event from gfx > > > side when it's in runtime suspend mode. There's no ELD > > > info when exit from runtime suspend. This patch avoid missing
ELD info.
> > > > hda_call_codec_resume() sets the jack detection all dirty, > > thus each jack detection callback should be called at > > resume. Didn't it work as expected? > > I would double check that. In my test, it doesnot work as expected.
OK, I found the problem. patch_hdmi.c enables the jack detection stuff without the callback, so the resume code triggers the check of jack detection but only updates the kcontrols.
You patch did not resolve the issue. I added some debug log, the callback wasnot called at all.
Even if you unplugged while runtime suspend?
Yes, the controller/codec suspended in runtime already.
The callback is called only when the plug status (i.e. the jack detection state) change is detected at the resume time -- i.e. the state the driver holds differs from the state at the resume.
Do you assume jackpoll_interval be non-zero? If so the hda_jackpoll_work will continue to run periodically. IMO the callback just need be called only once at resume time.
Hmm, the problem is that the callback updater isn't called in the resume path as default. The oneliner below will fix it.
Forgot to attach...
Yes, it works if call snd_hda_jack_poll_all(codec) directly, but sometimes read pin sense value is 0xc000,0000 even unplug monitor, and eld# will keep valid all the time. Although sometimes it works(the ELD info refreshed correctly), you have to wake up controller/codec before check eld info(cat codec# or play a piece of audio). Thanks --xingchao
Takashi
--- diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 35090b3..86d4709 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3942,6 +3942,7 @@ static void hda_call_codec_resume(struct hda_codec *codec) codec->patch_ops.init(codec); snd_hda_codec_resume_amp(codec); snd_hda_codec_resume_cache(codec); + snd_hda_jack_poll_all(codec); }
if (codec->jackpoll_interval)
On 06/25/2013 09:55 AM, Takashi Iwai wrote:
At Tue, 25 Jun 2013 09:45:04 +0200, David Henningsson wrote:
There is a low power mode that allows the jack detection, but this is different from the aggressive power-saving with runtime D3.
If "aggressive power-saving with runtime D3" is the same as AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs connected to a Lynx point controller.
It looks like userspace have problems getting notifications for e g headphone insertion on Lynx point controllers, so this is not only an HDMI/DP problem?
Yes.
Trying to read up a little on this, there seem to be an option to set the WAKEEN register to have jack detection working even when the controller is in D3. (refer HDA specification 4.5.9.2: Codec Wake From System S0, Controller D3.) But it seems we do not (yet) use this feature. Is this something that could/should be implemented to fix the jack detection problems that seems to be happening otherwise?
It sounds feasible, at least for traditional jack detection of analog pins. But I'm not sure whether this would help for the Intel graphics case. Just need testing.
Xingchao, what are your thoughts about using WAKEEN to wakeup both Lynx point and Haswell HDMI? -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
Hi David,
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David Henningsson Sent: Tuesday, June 25, 2013 5:02 PM To: Wang, Xingchao Cc: Takashi Iwai; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
On 06/25/2013 09:55 AM, Takashi Iwai wrote:
At Tue, 25 Jun 2013 09:45:04 +0200, David Henningsson wrote:
There is a low power mode that allows the jack detection, but this is different from the aggressive power-saving with runtime D3.
If "aggressive power-saving with runtime D3" is the same as AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs connected to a Lynx point controller.
It looks like userspace have problems getting notifications for e g headphone insertion on Lynx point controllers, so this is not only an HDMI/DP problem?
Yes.
Trying to read up a little on this, there seem to be an option to set the WAKEEN register to have jack detection working even when the controller is in D3. (refer HDA specification 4.5.9.2: Codec Wake From System S0, Controller D3.) But it seems we do not (yet) use this feature. Is this something that could/should be implemented to fix the jack detection problems that seems to be happening otherwise?
It sounds feasible, at least for traditional jack detection of analog pins. But I'm not sure whether this would help for the Intel graphics case. Just need testing.
Xingchao, what are your thoughts about using WAKEEN to wakeup both Lynx point and Haswell HDMI?
That's okay for me, I would do some test on that. do you have some test cases? That would help me verify them when enable WAKEEN feature. Thanks --xingchao
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On 06/25/2013 11:33 AM, Wang, Xingchao wrote:
Hi David,
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David Henningsson Sent: Tuesday, June 25, 2013 5:02 PM To: Wang, Xingchao Cc: Takashi Iwai; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
On 06/25/2013 09:55 AM, Takashi Iwai wrote:
At Tue, 25 Jun 2013 09:45:04 +0200, David Henningsson wrote:
There is a low power mode that allows the jack detection, but this is different from the aggressive power-saving with runtime D3.
If "aggressive power-saving with runtime D3" is the same as AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs connected to a Lynx point controller.
It looks like userspace have problems getting notifications for e g headphone insertion on Lynx point controllers, so this is not only an HDMI/DP problem?
Yes.
Trying to read up a little on this, there seem to be an option to set the WAKEEN register to have jack detection working even when the controller is in D3. (refer HDA specification 4.5.9.2: Codec Wake From System S0, Controller D3.) But it seems we do not (yet) use this feature. Is this something that could/should be implemented to fix the jack detection problems that seems to be happening otherwise?
It sounds feasible, at least for traditional jack detection of analog pins. But I'm not sure whether this would help for the Intel graphics case. Just need testing.
Xingchao, what are your thoughts about using WAKEEN to wakeup both Lynx point and Haswell HDMI?
That's okay for me, I would do some test on that. do you have some test cases? That would help me verify them when enable WAKEEN feature.
Since we still enable the legacy jack feature through /dev/input/event*, the easiest way to test would probably to run evtest (which is in the Ubuntu repositories). Find the correct /dev/input/event file by checking dmesg (or just trying them one by one), then run "sudo evtest /dev/input/<filename>". Now check if you get events correctly on HDMI/DP/Headphone/Mic/etc plug and unplug, even if the controller is runtime suspended. Also note that WAKEEN should probably be disabled during system S3, because we don't want to wake up the entire computer just because somebody unplugs his headphone, right? -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
Hi Takashi,
-----Original Message----- From: Wang, Xingchao Sent: Tuesday, June 25, 2013 4:30 PM To: Takashi Iwai Cc: alsa-devel@alsa-project.org; Wang Xingchao
[snip]
Do you assume jackpoll_interval be non-zero? If so the hda_jackpoll_work
will continue to run periodically.
IMO the callback just need be called only once at resume time.
Hmm, the problem is that the callback updater isn't called in the resume path as default. The oneliner below will fix it.
Forgot to attach...
Yes, it works if call snd_hda_jack_poll_all(codec) directly, but sometimes read pin sense value is 0xc000,0000 even unplug monitor, and eld# will keep valid all the time. Although sometimes it works(the ELD info refreshed correctly), you have to wake up controller/codec before check eld info(cat codec# or play a piece of audio).
The fix is not enough. When hotplug event happen and codec/controller in active state, hdmi_intrinsic_event() will be called. In such scenario, jack event would be reported but not handled(hdmi_present_sense() was moved to callback). So there's no eld info updated, have to trigger codec_resume to update the eld info. If App depends on ELD# info to check external monitor status, it may cause confuse as eld# under /proc donot Provide realtime information. Thanks --xingchao
Thanks --xingchao
Takashi
--- diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 35090b3..86d4709 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3942,6 +3942,7 @@ static void hda_call_codec_resume(struct hda_codec *codec) codec->patch_ops.init(codec); snd_hda_codec_resume_amp(codec); snd_hda_codec_resume_cache(codec); + snd_hda_jack_poll_all(codec); }
if (codec->jackpoll_interval)
Hi David/Takashi, Here's some update on this topic. I used evtest to monitor hotplug input event for Headphone, it doesnot report the hotplug event when audio controller/codec in runtime suspend mode. if it's during audio playback(both controller and codec are active), the events could be monitored correctly. However even the controller/codec in runtime suspend mode, the hotplug event was not missed when waken up from suspend mode. After exit from suspend mode, the hotplug event would be reported asap. So userspace will not receive the event notification from driver when controller/codec in suspend mode, but it will get them once audio controller/codec become active. I think it's acceptable for audio playback functinality, it will not harm audio routing in fact. i'm not sure there's other potential risk, i.e. user space will show/hide the devices in UI according to the event, in that case , user will never see the Headphone device for playback before audio controller/codec was waken up in another way. Meanwhile i tested attached patch to monitor WAKEEN events, it doesnot work well as Spec said. it would not wake up audio controller/codec when plug in/out headphone in runtime suspend mode, and the status register always be 0. thanks --xingchao 2013/6/25 David Henningsson <david.henningsson@canonical.com>:
On 06/25/2013 11:33 AM, Wang, Xingchao wrote:
Hi David,
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David Henningsson Sent: Tuesday, June 25, 2013 5:02 PM To: Wang, Xingchao Cc: Takashi Iwai; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
On 06/25/2013 09:55 AM, Takashi Iwai wrote:
At Tue, 25 Jun 2013 09:45:04 +0200, David Henningsson wrote:
There is a low power mode that allows the jack detection, but this is different from the aggressive power-saving with runtime D3.
If "aggressive power-saving with runtime D3" is the same as AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs connected to a Lynx point controller.
It looks like userspace have problems getting notifications for e g headphone insertion on Lynx point controllers, so this is not only an HDMI/DP problem?
Yes.
Trying to read up a little on this, there seem to be an option to set the WAKEEN register to have jack detection working even when the controller is in D3. (refer HDA specification 4.5.9.2: Codec Wake From System S0, Controller D3.) But it seems we do not (yet) use this feature. Is this something that could/should be implemented to fix the jack detection problems that seems to be happening otherwise?
It sounds feasible, at least for traditional jack detection of analog pins. But I'm not sure whether this would help for the Intel graphics case. Just need testing.
Xingchao, what are your thoughts about using WAKEEN to wakeup both Lynx point and Haswell HDMI?
That's okay for me, I would do some test on that. do you have some test cases? That would help me verify them when enable WAKEEN feature.
Since we still enable the legacy jack feature through /dev/input/event*, the easiest way to test would probably to run evtest (which is in the Ubuntu repositories). Find the correct /dev/input/event file by checking dmesg (or just trying them one by one), then run "sudo evtest /dev/input/<filename>".
Now check if you get events correctly on HDMI/DP/Headphone/Mic/etc plug and unplug, even if the controller is runtime suspended.
Also note that WAKEEN should probably be disabled during system S3, because we don't want to wake up the entire computer just because somebody unplugs his headphone, right?
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On 07/12/2013 08:13 AM, Wang Xingchao wrote:
Hi David/Takashi,
Here's some update on this topic. I used evtest to monitor hotplug input event for Headphone, it doesnot report the hotplug event when audio controller/codec in runtime suspend mode. if it's during audio playback(both controller and codec are active), the events could be monitored correctly. However even the controller/codec in runtime suspend mode, the hotplug event was not missed when waken up from suspend mode. After exit from suspend mode, the hotplug event would be reported asap. So userspace will not receive the event notification from driver when controller/codec in suspend mode, but it will get them once audio controller/codec become active. I think it's acceptable for audio playback functinality, it will not harm audio routing in fact. i'm not sure there's other potential risk, i.e. user space will show/hide the devices in UI according to the event, in that case , user will never see the Headphone device for playback before audio controller/codec was waken up in another way.
Meanwhile i tested attached patch to monitor WAKEEN events, it doesnot work well as Spec said. it would not wake up audio controller/codec when plug in/out headphone in runtime suspend mode, and the status register always be 0.
Hi Xingchao, Maybe you're giving up too easily on the WAKEEN stuff? I can spot a few places where your patch is incomplete: - First, WAKEEN and STATETS are word registers, you should probably use azx_writew when you access them - Second, and this is definitely a problem, azx_runtime_suspend calls azx_stop_chip -> azx_int_disable. So we disable interrupts, that's why we don't get them from the wake events. - Third, first in azx_interrupt, we check for chip->pci->dev.power.runtime_status - maybe this is no longer true if the device is in D3? Do you know how and if Windows has solved this?
thanks --xingchao
2013/6/25 David Henningsson <david.henningsson@canonical.com>:
On 06/25/2013 11:33 AM, Wang, Xingchao wrote:
Hi David,
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of David Henningsson Sent: Tuesday, June 25, 2013 5:02 PM To: Wang, Xingchao Cc: Takashi Iwai; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
On 06/25/2013 09:55 AM, Takashi Iwai wrote:
At Tue, 25 Jun 2013 09:45:04 +0200, David Henningsson wrote:
> > There is a low power mode that allows the jack detection, but this > is different from the aggressive power-saving with runtime D3.
If "aggressive power-saving with runtime D3" is the same as AZX_DCAPS_PM_RUNTIME, this is also enabled for analog codecs connected to a Lynx point controller.
It looks like userspace have problems getting notifications for e g headphone insertion on Lynx point controllers, so this is not only an HDMI/DP problem?
Yes.
Trying to read up a little on this, there seem to be an option to set the WAKEEN register to have jack detection working even when the controller is in D3. (refer HDA specification 4.5.9.2: Codec Wake From System S0, Controller D3.) But it seems we do not (yet) use this feature. Is this something that could/should be implemented to fix the jack detection problems that seems to be happening otherwise?
It sounds feasible, at least for traditional jack detection of analog pins. But I'm not sure whether this would help for the Intel graphics case. Just need testing.
Xingchao, what are your thoughts about using WAKEEN to wakeup both Lynx point and Haswell HDMI?
That's okay for me, I would do some test on that. do you have some test cases? That would help me verify them when enable WAKEEN feature.
Since we still enable the legacy jack feature through /dev/input/event*, the easiest way to test would probably to run evtest (which is in the Ubuntu repositories). Find the correct /dev/input/event file by checking dmesg (or just trying them one by one), then run "sudo evtest /dev/input/<filename>".
Now check if you get events correctly on HDMI/DP/Headphone/Mic/etc plug and unplug, even if the controller is runtime suspended.
Also note that WAKEEN should probably be disabled during system S3, because we don't want to wake up the entire computer just because somebody unplugs his headphone, right?
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Monday, July 15, 2013 12:29 PM To: Wang Xingchao Cc: Takashi Iwai; Wang, Xingchao; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
On 07/12/2013 08:13 AM, Wang Xingchao wrote:
Hi David/Takashi,
Here's some update on this topic. I used evtest to monitor hotplug input event for Headphone, it doesnot report the hotplug event when audio controller/codec in runtime suspend mode. if it's during audio playback(both controller and codec are active), the events could be monitored correctly. However even the controller/codec in runtime suspend mode, the hotplug event was not missed when waken up from suspend mode. After exit from suspend mode, the hotplug event would be reported asap. So userspace will not receive the event notification from driver when controller/codec in suspend mode, but it will get them once audio controller/codec become active. I think it's acceptable for audio playback functinality, it will not harm audio routing in fact. i'm not sure there's other potential risk, i.e. user space will show/hide the devices in UI according to the event, in that case , user will never see the Headphone device for playback before audio controller/codec was waken up in another way.
Meanwhile i tested attached patch to monitor WAKEEN events, it doesnot work well as Spec said. it would not wake up audio controller/codec when plug in/out headphone in runtime suspend mode, and the status register always be 0.
Hi Xingchao,
Maybe you're giving up too easily on the WAKEEN stuff? I can spot a few places where your patch is incomplete:
- First, WAKEEN and STATETS are word registers, you should probably use azx_writew when you access them
Thanks for clarify this. We support 8 codec at most, so I think it's safe to use writeb/readb here.
- Second, and this is definitely a problem, azx_runtime_suspend calls azx_stop_chip -> azx_int_disable. So we disable interrupts, that's why we don't get them from the wake events.
[Wang, Xingchao] yes, moreover I think it's not enough only enabling "controller interrupt" bits in this case, PCI interrupt bits must be anbled too. I'm still checking the setting in runtime_suspend.
- Third, first in azx_interrupt, we check for chip->pci->dev.power.runtime_status - maybe this is no longer true if the device is in D3?
[Wang, Xingchao] yes, it's a risk here. Just move the lines before the "if" condition check.
Do you know how and if Windows has solved this?
[Wang, Xingchao] I have wrote emails for windows team guys, has no response yet. I will update here when I have update. thanks --xingchao
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Wang, Xingchao Sent: Monday, July 15, 2013 4:54 PM To: David Henningsson; Wang Xingchao Cc: Takashi Iwai; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Monday, July 15, 2013 12:29 PM To: Wang Xingchao Cc: Takashi Iwai; Wang, Xingchao; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
On 07/12/2013 08:13 AM, Wang Xingchao wrote:
Hi David/Takashi,
Here's some update on this topic. I used evtest to monitor hotplug input event for Headphone, it doesnot report the hotplug event when audio controller/codec in runtime suspend mode. if it's during audio playback(both controller and codec are active), the events could be monitored correctly. However even the controller/codec in runtime suspend mode, the hotplug event was not missed when waken up from suspend mode. After exit from suspend mode, the hotplug event would be reported asap. So userspace will not receive the event notification from driver when controller/codec in suspend mode, but it will get them once audio controller/codec become active. I think it's acceptable for audio playback functinality, it will not harm audio routing in fact. i'm not sure there's other potential risk, i.e. user space will show/hide the devices in UI according to the event, in that case , user will never see the Headphone device for playback before audio controller/codec was waken up in another way.
Meanwhile i tested attached patch to monitor WAKEEN events, it doesnot work well as Spec said. it would not wake up audio controller/codec when plug in/out headphone in runtime suspend mode, and the status register always be 0.
Hi Xingchao,
Maybe you're giving up too easily on the WAKEEN stuff? I can spot a few places where your patch is incomplete:
- First, WAKEEN and STATETS are word registers, you should probably use azx_writew when you access them
If the controller is active while codec suspended, "evtest" can catch headphone hotplug event. I made some changes in azx_runtime_suspend(), which in brief leave interrupt enabled, then "evtest" cannot catch input event. Also I added some debug log shows, when controller enter suspend mode, the WAKEEN bits were set(value 0x3), after headphone hotplug, Audio controller was not waken up. When audio controller exit, in azx_runtime_resume(), the debug shows WAKEEN bits value no change, STATETS has value 0. The controller did not detect changes from codec side, so it donot set the bits in STATETS. I read the spec, and found something different: - Controller should be in reset state while In D3?(Chapter 4.5.9.2 in HD-A spec.) Azx_runtime-suspend() did not reset the controller. - codec will use power state change request on the link to let controller know. Not sure it's hardware operation or software layer. - codec discovery ( chapter 4.3) "When the link is enabled by the assertion of CRST, the codecs will detect the de-assertion of the RESET# signal and request a status change and enumeration by the controller. As the controller hardware detects these requests, it will provide the codecs with their unique addresses and set the controller STATESTS bits to indicate that a Status Change event was detected on the appropriate SDATA_INx signals. " I'm afraid something was not configured correctly, i.e. should let controller in reset state when enter runtime suspend mode? Thanks --xingchao
Thanks for clarify this. We support 8 codec at most, so I think it's safe to use writeb/readb here.
- Second, and this is definitely a problem, azx_runtime_suspend calls azx_stop_chip -> azx_int_disable. So we disable interrupts, that's why we don't get them from the wake events.
[Wang, Xingchao] yes, moreover I think it's not enough only enabling "controller interrupt" bits in this case, PCI interrupt bits must be anbled too. I'm still checking the setting in runtime_suspend.
- Third, first in azx_interrupt, we check for chip->pci->dev.power.runtime_status - maybe this is no longer true if the device is in D3?
[Wang, Xingchao] yes, it's a risk here. Just move the lines before the "if" condition check.
Do you know how and if Windows has solved this?
[Wang, Xingchao] I have wrote emails for windows team guys, has no response yet. I will update here when I have update.
thanks --xingchao _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
-----Original Message----- From: Wang, Xingchao Sent: Wednesday, July 17, 2013 6:15 PM To: Wang, Xingchao; David Henningsson Cc: Takashi Iwai; alsa-devel@alsa-project.org Subject: RE: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Wang, Xingchao Sent: Monday, July 15, 2013 4:54 PM To: David Henningsson; Wang Xingchao Cc: Takashi Iwai; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Monday, July 15, 2013 12:29 PM To: Wang Xingchao Cc: Takashi Iwai; Wang, Xingchao; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
On 07/12/2013 08:13 AM, Wang Xingchao wrote:
Hi David/Takashi,
Here's some update on this topic. I used evtest to monitor hotplug input event for Headphone, it doesnot report the hotplug event when audio controller/codec in runtime suspend mode. if it's during audio playback(both controller and codec are active), the events could be monitored correctly. However even the controller/codec in runtime suspend mode, the hotplug event was not missed when waken up from suspend mode. After exit from suspend mode, the hotplug event would be reported asap. So userspace will not receive the event notification from driver when controller/codec in suspend mode, but it will get them once audio controller/codec become active. I think it's acceptable for audio playback functinality, it will not harm audio routing in fact. i'm not sure there's other potential risk, i.e. user space will show/hide the devices in UI according to the event, in that case , user will never see the Headphone device for playback before audio controller/codec was waken up in another way.
Meanwhile i tested attached patch to monitor WAKEEN events, it doesnot work well as Spec said. it would not wake up audio controller/codec when plug in/out headphone in runtime suspend mode, and the status register always be 0.
Hi Xingchao,
Maybe you're giving up too easily on the WAKEEN stuff? I can spot a few places where your patch is incomplete:
- First, WAKEEN and STATETS are word registers, you should probably use azx_writew when you access them
If the controller is active while codec suspended, "evtest" can catch headphone hotplug event. I made some changes in azx_runtime_suspend(), which in brief leave interrupt enabled, then "evtest" cannot catch input event. Also I added some debug log shows, when controller enter suspend mode, the WAKEEN bits were set(value 0x3), after headphone hotplug, Audio controller was not waken up. When audio controller exit, in azx_runtime_resume(), the debug shows WAKEEN bits value no change, STATETS has value 0. The controller did not detect changes from codec side, so it donot set the bits in STATETS.
I read the spec, and found something different: - Controller should be in reset state while In D3?(Chapter 4.5.9.2 in HD-A spec.) Azx_runtime-suspend() did not reset the controller. - codec will use power state change request on the link to let controller know. Not sure it's hardware operation or software layer. - codec discovery ( chapter 4.3) "When the link is enabled by the assertion of CRST, the codecs will detect the de-assertion of the RESET# signal and request a status change and enumeration by the controller. As the controller hardware detects these requests, it will provide the codecs with their unique addresses and set the controller STATESTS bits to indicate that a Status Change event was detected on the appropriate SDATA_INx signals. " I'm afraid something was not configured correctly, i.e. should let controller in reset state when enter runtime suspend mode?
After some test, I found the controller should be in "reset" state, and CIE interrupt should be enabled, Otherwise the wake-up event from codec could not wake up system. I'm writing a patch to fix this. --xingchao
Thanks --xingchao
Thanks for clarify this. We support 8 codec at most, so I think it's safe to use writeb/readb here.
- Second, and this is definitely a problem, azx_runtime_suspend calls azx_stop_chip -> azx_int_disable. So we disable interrupts, that's why we don't get them from the wake events.
[Wang, Xingchao] yes, moreover I think it's not enough only enabling "controller interrupt" bits in this case, PCI interrupt bits must be anbled too. I'm still checking the setting in runtime_suspend.
- Third, first in azx_interrupt, we check for chip->pci->dev.power.runtime_status - maybe this is no longer true chip->pci->if the device is in D3?
[Wang, Xingchao] yes, it's a risk here. Just move the lines before the "if" condition check.
Do you know how and if Windows has solved this?
[Wang, Xingchao] I have wrote emails for windows team guys, has no response yet. I will update here when I have update.
thanks --xingchao _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
At Thu, 18 Jul 2013 05:47:30 +0000, Wang, Xingchao wrote:
-----Original Message----- From: Wang, Xingchao Sent: Wednesday, July 17, 2013 6:15 PM To: Wang, Xingchao; David Henningsson Cc: Takashi Iwai; alsa-devel@alsa-project.org Subject: RE: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Wang, Xingchao Sent: Monday, July 15, 2013 4:54 PM To: David Henningsson; Wang Xingchao Cc: Takashi Iwai; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Monday, July 15, 2013 12:29 PM To: Wang Xingchao Cc: Takashi Iwai; Wang, Xingchao; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 2/2] ALSA: hda - get realtime ELD info when codec suspended
On 07/12/2013 08:13 AM, Wang Xingchao wrote:
Hi David/Takashi,
Here's some update on this topic. I used evtest to monitor hotplug input event for Headphone, it doesnot report the hotplug event when audio controller/codec in runtime suspend mode. if it's during audio playback(both controller and codec are active), the events could be monitored correctly. However even the controller/codec in runtime suspend mode, the hotplug event was not missed when waken up from suspend mode. After exit from suspend mode, the hotplug event would be reported asap. So userspace will not receive the event notification from driver when controller/codec in suspend mode, but it will get them once audio controller/codec become active. I think it's acceptable for audio playback functinality, it will not harm audio routing in fact. i'm not sure there's other potential risk, i.e. user space will show/hide the devices in UI according to the event, in that case , user will never see the Headphone device for playback before audio controller/codec was waken up in another way.
Meanwhile i tested attached patch to monitor WAKEEN events, it doesnot work well as Spec said. it would not wake up audio controller/codec when plug in/out headphone in runtime suspend mode, and the status register always be 0.
Hi Xingchao,
Maybe you're giving up too easily on the WAKEEN stuff? I can spot a few places where your patch is incomplete:
- First, WAKEEN and STATETS are word registers, you should probably use azx_writew when you access them
If the controller is active while codec suspended, "evtest" can catch headphone hotplug event. I made some changes in azx_runtime_suspend(), which in brief leave interrupt enabled, then "evtest" cannot catch input event. Also I added some debug log shows, when controller enter suspend mode, the WAKEEN bits were set(value 0x3), after headphone hotplug, Audio controller was not waken up. When audio controller exit, in azx_runtime_resume(), the debug shows WAKEEN bits value no change, STATETS has value 0. The controller did not detect changes from codec side, so it donot set the bits in STATETS.
I read the spec, and found something different: - Controller should be in reset state while In D3?(Chapter 4.5.9.2 in HD-A spec.) Azx_runtime-suspend() did not reset the controller. - codec will use power state change request on the link to let controller know. Not sure it's hardware operation or software layer. - codec discovery ( chapter 4.3) "When the link is enabled by the assertion of CRST, the codecs will detect the de-assertion of the RESET# signal and request a status change and enumeration by the controller. As the controller hardware detects these requests, it will provide the codecs with their unique addresses and set the controller STATESTS bits to indicate that a Status Change event was detected on the appropriate SDATA_INx signals. " I'm afraid something was not configured correctly, i.e. should let controller in reset state when enter runtime suspend mode?
After some test, I found the controller should be in "reset" state, and CIE interrupt should be enabled, Otherwise the wake-up event from codec could not wake up system.
I'm writing a patch to fix this.
Great, thanks for checking it. Takashi
--xingchao
Thanks --xingchao
Thanks for clarify this. We support 8 codec at most, so I think it's safe to use writeb/readb here.
- Second, and this is definitely a problem, azx_runtime_suspend calls azx_stop_chip -> azx_int_disable. So we disable interrupts, that's why we don't get them from the wake events.
[Wang, Xingchao] yes, moreover I think it's not enough only enabling "controller interrupt" bits in this case, PCI interrupt bits must be anbled too. I'm still checking the setting in runtime_suspend.
- Third, first in azx_interrupt, we check for chip->pci->dev.power.runtime_status - maybe this is no longer true chip->pci->if the device is in D3?
[Wang, Xingchao] yes, it's a risk here. Just move the lines before the "if" condition check.
Do you know how and if Windows has solved this?
[Wang, Xingchao] I have wrote emails for windows team guys, has no response yet. I will update here when I have update.
thanks --xingchao _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (5)
-
David Henningsson -
Takashi Iwai -
Wang Xingchao -
Wang Xingchao -
Wang, Xingchao