[alsa-devel] Possible fix for snd-hda-intel model=no-jd failing since ~linux-3.9-rc1
Hi.
This is a bug and suggested temporary fix that I originally posted to the linux-sound mailing list about a month and a half ago. I am grateful to Takashi Awai for informing me after my follow-up inquiry about it today that I should submit it to the alsa-devel mailing list instead. Please feel free to redirect me further if appropriate. I did not notice any contact information for sound/pci/hda in linux-3.16-rc4/MAINATINERS.
Anyhow, here is the bug report and a one line proposed temporary fix.
The "model=no-jd" argument has not initialized the analog audio output jack correctly for me since linux-3.9-rc1 (if I recall correctly) through linux-3.17-rc4, although I have not tried every release candidate in between.
The symptom is that, on a computer with an analog audio output jack that has incorrect jack sense (a hardware bug), audio output is completely muted until I physically replug the cable, even though I specified "model=no-jd" as an argument to the snd-hda-intel kernel module, which is supposed to cause the kernel to ignore the jack sense reported by hardware and just drive the audio output even if the hardware jack sense indicates nothing is plugged in. This problem did not occur until approximate Linux 3.9-rc1.
I have found a few single line workarounds that work, of which my favorite is the following (also attached to this email, in case any mailer subjects this message to reformatting), because it does not add code to anything that gets called frequently.
--- linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c.orig 2014-09-07 16:09:43.000000000 -0700 +++ linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c 2014-09-10 18:41:53.422900040 -0700 @@ -106,6 +106,7 @@ snd_hda_jack_tbl_new(struct hda_codec *c jack->nid = nid; jack->jack_dirty = 1; jack->tag = codec->jacktbl.used; + jack->phantom_jack = codec->no_jack_detect; return jack; } EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
Signed-off-by: Adam J. Richter adam_richter2004@yahoo.com
I consider this a workaround rather than than a perfect fix, because I think the underlying problem seems to be some kind of initialization order issue that I don't fully understand. Basically, by the time jack->phantom_jack was being set, some caller had already called jack_detect_update(), which loaded the incorrect jack sense result from hardware and cleared jack->jack_dirty, so the jack sense would not be set again. At least that is what I think the underlying problem probably is.
Also, if a change like this is to be integrated, I'd like to know if it might be better for the line that I added to be:
jack->phantom_jack = !is_jack_detectable(codec, nid);
...which is what add_jack_kctl does (not sure why add_jack_kctls does not, by the way, at the risk of making a spectacle of my ignorance). My doubt about this approach is that perhaps is_jack_detectable() relies on some initialization that has not occurred at that point.
Anyhow, I'd like to get the process started of either pushing this change upstream or quickly developing a more correct fix. If a more correct fix does not become apparent in the next few days, I would recommend pushing the workaround upstream now until some future code cleanup, so people will no longer be effected by the bug.
Any technical input or advice on how to proceed is welcome. Thanks for taking the time to consider this.
Adam Richter
At Thu, 11 Sep 2014 00:47:19 -0700, Adam Richter wrote:
Hi.
This is a bug and suggested temporary fix that I originally posted to the linux-sound mailing list about a month and a half ago. I am grateful to Takashi Awai for informing me after my follow-up inquiry about it today that I should submit it to the alsa-devel mailing list instead. Please feel free to redirect me further if appropriate. I did not notice any contact information for sound/pci/hda in linux-3.16-rc4/MAINATINERS.
Anyhow, here is the bug report and a one line proposed temporary fix.
The "model=no-jd" argument has not initialized the analog audio output jack correctly for me since linux-3.9-rc1 (if I recall correctly) through linux-3.17-rc4, although I have not tried every release candidate in between.
The symptom is that, on a computer with an analog audio output jack that has incorrect jack sense (a hardware bug), audio output is completely muted until I physically replug the cable, even though I specified "model=no-jd" as an argument to the snd-hda-intel kernel module, which is supposed to cause the kernel to ignore the jack sense reported by hardware and just drive the audio output even if the hardware jack sense indicates nothing is plugged in. This problem did not occur until approximate Linux 3.9-rc1.
I have found a few single line workarounds that work, of which my favorite is the following (also attached to this email, in case any mailer subjects this message to reformatting), because it does not add code to anything that gets called frequently.
--- linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c.orig 2014-09-07 16:09:43.000000000 -0700 +++ linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c 2014-09-10 18:41:53.422900040 -0700 @@ -106,6 +106,7 @@ snd_hda_jack_tbl_new(struct hda_codec *c jack->nid = nid; jack->jack_dirty = 1; jack->tag = codec->jacktbl.used;
- jack->phantom_jack = codec->no_jack_detect; return jack;
} EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
Signed-off-by: Adam J. Richter adam_richter2004@yahoo.com
I consider this a workaround rather than than a perfect fix, because I think the underlying problem seems to be some kind of initialization order issue that I don't fully understand. Basically, by the time jack->phantom_jack was being set, some caller had already called jack_detect_update(), which loaded the incorrect jack sense result from hardware and cleared jack->jack_dirty, so the jack sense would not be set again. At least that is what I think the underlying problem probably is.
Also, if a change like this is to be integrated, I'd like to know if it might be better for the line that I added to be:
jack->phantom_jack = !is_jack_detectable(codec, nid);...which is what add_jack_kctl does (not sure why add_jack_kctls does not, by the way, at the risk of making a spectacle of my ignorance). My doubt about this approach is that perhaps is_jack_detectable() relies on some initialization that has not occurred at that point.
Anyhow, I'd like to get the process started of either pushing this change upstream or quickly developing a more correct fix. If a more correct fix does not become apparent in the next few days, I would recommend pushing the workaround upstream now until some future code cleanup, so people will no longer be effected by the bug.
Any technical input or advice on how to proceed is welcome. Thanks for taking the time to consider this.
Could you give alsa-info.sh output on your machine? Otherwise it's difficult to analyze.
Speaking of your patch: add_jack_kctl() itself has a check of is_jack_detectable() for phantom jacks, so basically this shouldn't make any difference. So we need to check more deeply why this change is really needed.
thanks,
Takashi
Adam Richter --- linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c.orig 2014-09-07 16:09:43.000000000 -0700 +++ linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c 2014-09-10 18:41:53.422900040 -0700 @@ -106,6 +106,7 @@ snd_hda_jack_tbl_new(struct hda_codec *c jack->nid = nid; jack->jack_dirty = 1; jack->tag = codec->jacktbl.used;
- jack->phantom_jack = codec->no_jack_detect; return jack;
} EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new); _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On 2014-09-11 09:47, Adam Richter wrote:
Hi.
Hi Adam,
Interesting problem. Could you submit alsa-info ( http://www.alsa-project.org/alsa-info.sh ) and point us to it? It makes it possible to run the code in an emulator.
This is a bug and suggested temporary fix that I originally posted to the linux-sound mailing list about a month and a half ago. I am grateful to Takashi Awai for informing me after my follow-up inquiry about it today that I should submit it to the alsa-devel mailing list instead. Please feel free to redirect me further if appropriate. I did not notice any contact information for sound/pci/hda in linux-3.16-rc4/MAINATINERS.
Anyhow, here is the bug report and a one line proposed temporary fix.
The "model=no-jd" argument has not initialized the analog audio output jack correctly for me since linux-3.9-rc1 (if I recall correctly) through linux-3.17-rc4, although I have not tried every release candidate in between.
A lot of things changed with 3.9, so that makes sense. Btw, "model=no-jd" works only for specific machines. You can use the hint "jack-detect=0" instead.
The symptom is that, on a computer with an analog audio output jack that has incorrect jack sense (a hardware bug), audio output is completely muted until I physically replug the cable, even though I specified "model=no-jd" as an argument to the snd-hda-intel kernel module, which is supposed to cause the kernel to ignore the jack sense reported by hardware and just drive the audio output even if the hardware jack sense indicates nothing is plugged in. This problem did not occur until approximate Linux 3.9-rc1.
I have found a few single line workarounds that work, of which my favorite is the following (also attached to this email, in case any mailer subjects this message to reformatting), because it does not add code to anything that gets called frequently.
--- linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c.orig 2014-09-07 16:09:43.000000000 -0700 +++ linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c 2014-09-10 18:41:53.422900040 -0700 @@ -106,6 +106,7 @@ snd_hda_jack_tbl_new(struct hda_codec *c jack->nid = nid; jack->jack_dirty = 1; jack->tag = codec->jacktbl.used;
- jack->phantom_jack = codec->no_jack_detect; return jack; } EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
Signed-off-by: Adam J. Richter adam_richter2004@yahoo.com
I consider this a workaround rather than than a perfect fix, because I think the underlying problem seems to be some kind of initialization order issue that I don't fully understand. Basically, by the time jack->phantom_jack was being set, some caller had already called jack_detect_update(), which loaded the incorrect jack sense result from hardware and cleared jack->jack_dirty, so the jack sense would not be set again. At least that is what I think the underlying problem probably is.
Also, if a change like this is to be integrated, I'd like to know if it might be better for the line that I added to be:
jack->phantom_jack = !is_jack_detectable(codec, nid);
Yes, this would probably be better.
I guess that somehow snd_hda_jack_enable_callback gets called for the phantom jack, but I'm not sure exactly how (alsa-info would help).
Nevertheless I'm attaching a patch. I assume it also resolves your problem?
Takashi, what do you think of the attached patch?
...which is what add_jack_kctl does (not sure why add_jack_kctls does not, by the way, at the risk of making a spectacle of my ignorance). My doubt about this approach is that perhaps is_jack_detectable() relies on some initialization that has not occurred at that point.
Anyhow, I'd like to get the process started of either pushing this change upstream or quickly developing a more correct fix. If a more correct fix does not become apparent in the next few days, I would recommend pushing the workaround upstream now until some future code cleanup, so people will no longer be effected by the bug.
Any technical input or advice on how to proceed is welcome. Thanks for taking the time to consider this.
Adam Richter
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
At Thu, 11 Sep 2014 11:07:10 +0200, David Henningsson wrote:
On 2014-09-11 09:47, Adam Richter wrote:
Hi.
Hi Adam,
Interesting problem. Could you submit alsa-info ( http://www.alsa-project.org/alsa-info.sh ) and point us to it? It makes it possible to run the code in an emulator.
This is a bug and suggested temporary fix that I originally posted to the linux-sound mailing list about a month and a half ago. I am grateful to Takashi Awai for informing me after my follow-up inquiry about it today that I should submit it to the alsa-devel mailing list instead. Please feel free to redirect me further if appropriate. I did not notice any contact information for sound/pci/hda in linux-3.16-rc4/MAINATINERS.
Anyhow, here is the bug report and a one line proposed temporary fix.
The "model=no-jd" argument has not initialized the analog audio output jack correctly for me since linux-3.9-rc1 (if I recall correctly) through linux-3.17-rc4, although I have not tried every release candidate in between.
A lot of things changed with 3.9, so that makes sense. Btw, "model=no-jd" works only for specific machines. You can use the hint "jack-detect=0" instead.
The symptom is that, on a computer with an analog audio output jack that has incorrect jack sense (a hardware bug), audio output is completely muted until I physically replug the cable, even though I specified "model=no-jd" as an argument to the snd-hda-intel kernel module, which is supposed to cause the kernel to ignore the jack sense reported by hardware and just drive the audio output even if the hardware jack sense indicates nothing is plugged in. This problem did not occur until approximate Linux 3.9-rc1.
I have found a few single line workarounds that work, of which my favorite is the following (also attached to this email, in case any mailer subjects this message to reformatting), because it does not add code to anything that gets called frequently.
--- linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c.orig 2014-09-07 16:09:43.000000000 -0700 +++ linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c 2014-09-10 18:41:53.422900040 -0700 @@ -106,6 +106,7 @@ snd_hda_jack_tbl_new(struct hda_codec *c jack->nid = nid; jack->jack_dirty = 1; jack->tag = codec->jacktbl.used;
- jack->phantom_jack = codec->no_jack_detect; return jack; } EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
Signed-off-by: Adam J. Richter adam_richter2004@yahoo.com
I consider this a workaround rather than than a perfect fix, because I think the underlying problem seems to be some kind of initialization order issue that I don't fully understand. Basically, by the time jack->phantom_jack was being set, some caller had already called jack_detect_update(), which loaded the incorrect jack sense result from hardware and cleared jack->jack_dirty, so the jack sense would not be set again. At least that is what I think the underlying problem probably is.
Also, if a change like this is to be integrated, I'd like to know if it might be better for the line that I added to be:
jack->phantom_jack = !is_jack_detectable(codec, nid);Yes, this would probably be better.
I guess that somehow snd_hda_jack_enable_callback gets called for the phantom jack, but I'm not sure exactly how (alsa-info would help).
Nevertheless I'm attaching a patch. I assume it also resolves your problem?
Takashi, what do you think of the attached patch?
I'd comment only after I see alsa-info.sh output. The patch above just makes it phantom without changing the name suffix, which looks bogus. So, I prefer fixing in a better way.
Of course, more important to know why it happens and how.
Takashi
At Thu, 11 Sep 2014 11:07:10 +0200, David Henningsson wrote:
On 2014-09-11 09:47, Adam Richter wrote:
Hi.
Hi Adam,
Interesting problem. Could you submit alsa-info ( http://www.alsa-project.org/alsa-info.sh ) and point us to it? It makes it possible to run the code in an emulator.
This is a bug and suggested temporary fix that I originally posted to the linux-sound mailing list about a month and a half ago. I am grateful to Takashi Awai for informing me after my follow-up inquiry about it today that I should submit it to the alsa-devel mailing list instead. Please feel free to redirect me further if appropriate. I did not notice any contact information for sound/pci/hda in linux-3.16-rc4/MAINATINERS.
Anyhow, here is the bug report and a one line proposed temporary fix.
The "model=no-jd" argument has not initialized the analog audio output jack correctly for me since linux-3.9-rc1 (if I recall correctly) through linux-3.17-rc4, although I have not tried every release candidate in between.
A lot of things changed with 3.9, so that makes sense. Btw, "model=no-jd" works only for specific machines. You can use the hint "jack-detect=0" instead.
The symptom is that, on a computer with an analog audio output jack that has incorrect jack sense (a hardware bug), audio output is completely muted until I physically replug the cable, even though I specified "model=no-jd" as an argument to the snd-hda-intel kernel module, which is supposed to cause the kernel to ignore the jack sense reported by hardware and just drive the audio output even if the hardware jack sense indicates nothing is plugged in. This problem did not occur until approximate Linux 3.9-rc1.
I have found a few single line workarounds that work, of which my favorite is the following (also attached to this email, in case any mailer subjects this message to reformatting), because it does not add code to anything that gets called frequently.
--- linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c.orig 2014-09-07 16:09:43.000000000 -0700 +++ linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c 2014-09-10 18:41:53.422900040 -0700 @@ -106,6 +106,7 @@ snd_hda_jack_tbl_new(struct hda_codec *c jack->nid = nid; jack->jack_dirty = 1; jack->tag = codec->jacktbl.used;
- jack->phantom_jack = codec->no_jack_detect; return jack; } EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
Signed-off-by: Adam J. Richter adam_richter2004@yahoo.com
I consider this a workaround rather than than a perfect fix, because I think the underlying problem seems to be some kind of initialization order issue that I don't fully understand. Basically, by the time jack->phantom_jack was being set, some caller had already called jack_detect_update(), which loaded the incorrect jack sense result from hardware and cleared jack->jack_dirty, so the jack sense would not be set again. At least that is what I think the underlying problem probably is.
Also, if a change like this is to be integrated, I'd like to know if it might be better for the line that I added to be:
jack->phantom_jack = !is_jack_detectable(codec, nid);Yes, this would probably be better.
I guess that somehow snd_hda_jack_enable_callback gets called for the phantom jack, but I'm not sure exactly how (alsa-info would help).
Nevertheless I'm attaching a patch. I assume it also resolves your problem?
Takashi, what do you think of the attached patch?
Ah, "attached" was meant *your* attached patch. Let me see...
From e5ce94c6156d1c588f29745ade06d1d76c87a0a1 Mon Sep 17 00:00:00 2001 From: David Henningsson david.henningsson@canonical.com Date: Thu, 11 Sep 2014 11:04:42 +0200 Subject: [PATCH] ALSA: hda - Do not enable unsol events on phantom jacks
To make sure we don't enable unsol events on phantom jacks, we move the logic for determining what a phantom jack is to snd_hda_jack_tbl_new.
Reported-by: Adam Richter adam_richter2004@yahoo.com Signed-off-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_jack.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 9746d73..376ce8f 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -105,6 +105,12 @@ snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid) return NULL; jack->nid = nid; jack->jack_dirty = 1;
- if (get_wcaps_type(get_wcaps(codec, nid)) == AC_WID_PIN) {
unsigned int def_conf = snd_hda_codec_get_pincfg(codec, nid);unsigned int conn = get_defcfg_connect(def_conf);if ((conn != AC_JACK_PORT_COMPLEX) || !is_jack_detectable(codec, nid))jack->phantom_jack = 1;- }
This doesn't look good. The pincfg isn't always reliable at this point. The pin connection check is used in add_jack_kctl() in the current code just because its only caller is snd_hda_jack_add_kctls(), which takes the pincfg explicitly. But snd_hda_jakc_tbl_new() itself is neutral about the pincfg.
And, another problem is that it'll result in a phantom jack secretly without adding a proper name suffix.
In anyway, for catching such a bug, maybe we should put a WARN_ON() at snd_hda_jack_detect_enable_callback() like the patch below.
Takashi
-- diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 9746d73cec52..0645735e196d 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -225,6 +225,8 @@ int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid, return -ENOMEM; if (jack->jack_detect) return 0; /* already registered */ + if (WARN_ON(!is_jack_detectable(codec, nid))) + return 0; jack->jack_detect = 1; if (action) jack->action = action;
On 2014-09-11 11:23, Takashi Iwai wrote:
At Thu, 11 Sep 2014 11:07:10 +0200, David Henningsson wrote:
On 2014-09-11 09:47, Adam Richter wrote:
Hi.
Hi Adam,
Interesting problem. Could you submit alsa-info ( http://www.alsa-project.org/alsa-info.sh ) and point us to it? It makes it possible to run the code in an emulator.
This is a bug and suggested temporary fix that I originally posted to the linux-sound mailing list about a month and a half ago. I am grateful to Takashi Awai for informing me after my follow-up inquiry about it today that I should submit it to the alsa-devel mailing list instead. Please feel free to redirect me further if appropriate. I did not notice any contact information for sound/pci/hda in linux-3.16-rc4/MAINATINERS.
Anyhow, here is the bug report and a one line proposed temporary fix.
The "model=no-jd" argument has not initialized the analog audio output jack correctly for me since linux-3.9-rc1 (if I recall correctly) through linux-3.17-rc4, although I have not tried every release candidate in between.
A lot of things changed with 3.9, so that makes sense. Btw, "model=no-jd" works only for specific machines. You can use the hint "jack-detect=0" instead.
The symptom is that, on a computer with an analog audio output jack that has incorrect jack sense (a hardware bug), audio output is completely muted until I physically replug the cable, even though I specified "model=no-jd" as an argument to the snd-hda-intel kernel module, which is supposed to cause the kernel to ignore the jack sense reported by hardware and just drive the audio output even if the hardware jack sense indicates nothing is plugged in. This problem did not occur until approximate Linux 3.9-rc1.
I have found a few single line workarounds that work, of which my favorite is the following (also attached to this email, in case any mailer subjects this message to reformatting), because it does not add code to anything that gets called frequently.
--- linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c.orig 2014-09-07 16:09:43.000000000 -0700 +++ linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c 2014-09-10 18:41:53.422900040 -0700 @@ -106,6 +106,7 @@ snd_hda_jack_tbl_new(struct hda_codec *c jack->nid = nid; jack->jack_dirty = 1; jack->tag = codec->jacktbl.used;
- jack->phantom_jack = codec->no_jack_detect; return jack; } EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
Signed-off-by: Adam J. Richter adam_richter2004@yahoo.com
I consider this a workaround rather than than a perfect fix, because I think the underlying problem seems to be some kind of initialization order issue that I don't fully understand. Basically, by the time jack->phantom_jack was being set, some caller had already called jack_detect_update(), which loaded the incorrect jack sense result from hardware and cleared jack->jack_dirty, so the jack sense would not be set again. At least that is what I think the underlying problem probably is.
Also, if a change like this is to be integrated, I'd like to know if it might be better for the line that I added to be:
jack->phantom_jack = !is_jack_detectable(codec, nid);Yes, this would probably be better.
I guess that somehow snd_hda_jack_enable_callback gets called for the phantom jack, but I'm not sure exactly how (alsa-info would help).
Nevertheless I'm attaching a patch. I assume it also resolves your problem?
Takashi, what do you think of the attached patch?
Ah, "attached" was meant *your* attached patch. Let me see...
From e5ce94c6156d1c588f29745ade06d1d76c87a0a1 Mon Sep 17 00:00:00 2001 From: David Henningsson david.henningsson@canonical.com Date: Thu, 11 Sep 2014 11:04:42 +0200 Subject: [PATCH] ALSA: hda - Do not enable unsol events on phantom jacks
To make sure we don't enable unsol events on phantom jacks, we move the logic for determining what a phantom jack is to snd_hda_jack_tbl_new.
Reported-by: Adam Richter adam_richter2004@yahoo.com Signed-off-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_jack.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 9746d73..376ce8f 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -105,6 +105,12 @@ snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid) return NULL; jack->nid = nid; jack->jack_dirty = 1;
- if (get_wcaps_type(get_wcaps(codec, nid)) == AC_WID_PIN) {
unsigned int def_conf = snd_hda_codec_get_pincfg(codec, nid);unsigned int conn = get_defcfg_connect(def_conf);if ((conn != AC_JACK_PORT_COMPLEX) || !is_jack_detectable(codec, nid))jack->phantom_jack = 1;- }
This doesn't look good. The pincfg isn't always reliable at this point. The pin connection check is used in add_jack_kctl() in the current code just because its only caller is snd_hda_jack_add_kctls(), which takes the pincfg explicitly. But snd_hda_jakc_tbl_new() itself is neutral about the pincfg.
Are you saying that codec->user_pins/driver_pins/init_pins might not be properly initialized at this point? If so, maybe we should instead warn on somebody calling snd_hda_jack_tbl_new too early?
And, another problem is that it'll result in a phantom jack secretly without adding a proper name suffix.
Are you sure?
snd_hda_jack_add_kctl is called from add_jack_kctl and the hdmi driver. Add_jack_kctl now uses the phantom_jack flag (as initialized by jack_new) to create a proper name, i e, no difference. generic_hdmi_build_jack should probably replace its "is_jack_detectable" call to check the phantom_jack flag for consistency, but for all common cases it's no change there either.
In anyway, for catching such a bug, maybe we should put a WARN_ON() at snd_hda_jack_detect_enable_callback() like the patch below.
Takashi
-- diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 9746d73cec52..0645735e196d 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -225,6 +225,8 @@ int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid, return -ENOMEM; if (jack->jack_detect) return 0; /* already registered */
- if (WARN_ON(!is_jack_detectable(codec, nid)))
return 0;
Remember that enable_callback is sometimes called for non-pin nodes, such as the afg node in the sigmatel driver. Calling is_jack_detectable on non-pin nodes seems wrong to me.
jack->jack_detect = 1; if (action) jack->action = action;
At Thu, 11 Sep 2014 11:42:41 +0200, David Henningsson wrote:
On 2014-09-11 11:23, Takashi Iwai wrote:
At Thu, 11 Sep 2014 11:07:10 +0200, David Henningsson wrote:
On 2014-09-11 09:47, Adam Richter wrote:
Hi.
Hi Adam,
Interesting problem. Could you submit alsa-info ( http://www.alsa-project.org/alsa-info.sh ) and point us to it? It makes it possible to run the code in an emulator.
This is a bug and suggested temporary fix that I originally posted to the linux-sound mailing list about a month and a half ago. I am grateful to Takashi Awai for informing me after my follow-up inquiry about it today that I should submit it to the alsa-devel mailing list instead. Please feel free to redirect me further if appropriate. I did not notice any contact information for sound/pci/hda in linux-3.16-rc4/MAINATINERS.
Anyhow, here is the bug report and a one line proposed temporary fix.
The "model=no-jd" argument has not initialized the analog audio output jack correctly for me since linux-3.9-rc1 (if I recall correctly) through linux-3.17-rc4, although I have not tried every release candidate in between.
A lot of things changed with 3.9, so that makes sense. Btw, "model=no-jd" works only for specific machines. You can use the hint "jack-detect=0" instead.
The symptom is that, on a computer with an analog audio output jack that has incorrect jack sense (a hardware bug), audio output is completely muted until I physically replug the cable, even though I specified "model=no-jd" as an argument to the snd-hda-intel kernel module, which is supposed to cause the kernel to ignore the jack sense reported by hardware and just drive the audio output even if the hardware jack sense indicates nothing is plugged in. This problem did not occur until approximate Linux 3.9-rc1.
I have found a few single line workarounds that work, of which my favorite is the following (also attached to this email, in case any mailer subjects this message to reformatting), because it does not add code to anything that gets called frequently.
--- linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c.orig 2014-09-07 16:09:43.000000000 -0700 +++ linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c 2014-09-10 18:41:53.422900040 -0700 @@ -106,6 +106,7 @@ snd_hda_jack_tbl_new(struct hda_codec *c jack->nid = nid; jack->jack_dirty = 1; jack->tag = codec->jacktbl.used;
- jack->phantom_jack = codec->no_jack_detect; return jack; } EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
Signed-off-by: Adam J. Richter adam_richter2004@yahoo.com
I consider this a workaround rather than than a perfect fix, because I think the underlying problem seems to be some kind of initialization order issue that I don't fully understand. Basically, by the time jack->phantom_jack was being set, some caller had already called jack_detect_update(), which loaded the incorrect jack sense result from hardware and cleared jack->jack_dirty, so the jack sense would not be set again. At least that is what I think the underlying problem probably is.
Also, if a change like this is to be integrated, I'd like to know if it might be better for the line that I added to be:
jack->phantom_jack = !is_jack_detectable(codec, nid);Yes, this would probably be better.
I guess that somehow snd_hda_jack_enable_callback gets called for the phantom jack, but I'm not sure exactly how (alsa-info would help).
Nevertheless I'm attaching a patch. I assume it also resolves your problem?
Takashi, what do you think of the attached patch?
Ah, "attached" was meant *your* attached patch. Let me see...
From e5ce94c6156d1c588f29745ade06d1d76c87a0a1 Mon Sep 17 00:00:00 2001 From: David Henningsson david.henningsson@canonical.com Date: Thu, 11 Sep 2014 11:04:42 +0200 Subject: [PATCH] ALSA: hda - Do not enable unsol events on phantom jacks
To make sure we don't enable unsol events on phantom jacks, we move the logic for determining what a phantom jack is to snd_hda_jack_tbl_new.
Reported-by: Adam Richter adam_richter2004@yahoo.com Signed-off-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_jack.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 9746d73..376ce8f 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -105,6 +105,12 @@ snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid) return NULL; jack->nid = nid; jack->jack_dirty = 1;
- if (get_wcaps_type(get_wcaps(codec, nid)) == AC_WID_PIN) {
unsigned int def_conf = snd_hda_codec_get_pincfg(codec, nid);unsigned int conn = get_defcfg_connect(def_conf);if ((conn != AC_JACK_PORT_COMPLEX) || !is_jack_detectable(codec, nid))jack->phantom_jack = 1;- }
This doesn't look good. The pincfg isn't always reliable at this point. The pin connection check is used in add_jack_kctl() in the current code just because its only caller is snd_hda_jack_add_kctls(), which takes the pincfg explicitly. But snd_hda_jakc_tbl_new() itself is neutral about the pincfg.
Are you saying that codec->user_pins/driver_pins/init_pins might not be properly initialized at this point? If so, maybe we should instead warn on somebody calling snd_hda_jack_tbl_new too early?
No, the hda_jack stuff itself isn't necessarily depending on the pincfg deeply. For example, is_jack_detectable() doesn't check the pin location (although it checks MISC_NO_PRESENCE bit as an exception).
And, another problem is that it'll result in a phantom jack secretly without adding a proper name suffix.
Are you sure?
snd_hda_jack_add_kctl is called from add_jack_kctl and the hdmi driver. Add_jack_kctl now uses the phantom_jack flag (as initialized by jack_new) to create a proper name, i e, no difference. generic_hdmi_build_jack should probably replace its "is_jack_detectable" call to check the phantom_jack flag for consistency, but for all common cases it's no change there either.
Hm, OK, that won't happen. But my point is that it's wrong to check the phantom flag there.
In anyway, for catching such a bug, maybe we should put a WARN_ON() at snd_hda_jack_detect_enable_callback() like the patch below.
Takashi
-- diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 9746d73cec52..0645735e196d 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -225,6 +225,8 @@ int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid, return -ENOMEM; if (jack->jack_detect) return 0; /* already registered */
- if (WARN_ON(!is_jack_detectable(codec, nid)))
return 0;Remember that enable_callback is sometimes called for non-pin nodes, such as the afg node in the sigmatel driver. Calling is_jack_detectable on non-pin nodes seems wrong to me.
OK, then it's an overkill.
Looking at the alsa-info.sh output, the problem is that IDT codec parser creates jack detection for the power-control before the generic parser. There is also a wrong logic in stac_init_power_map()...
An untested fix is below.
Takashi
-- diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index f26ec04a29b5..526b5d39a2cb 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -565,8 +565,8 @@ static void stac_init_power_map(struct hda_codec *codec) if (snd_hda_jack_tbl_get(codec, nid)) continue; if (def_conf == AC_JACK_PORT_COMPLEX && - !(spec->vref_mute_led_nid == nid || - is_jack_detectable(codec, nid))) { + spec->vref_mute_led_nid != nid && + is_jack_detectable(codec, nid)) { snd_hda_jack_detect_enable_callback(codec, nid, STAC_PWR_EVENT, jack_update_power); @@ -4272,11 +4272,17 @@ static int stac_parse_auto_config(struct hda_codec *codec) return err; }
- stac_init_power_map(codec); - return 0; }
+static int stac_build_controls(struct hda_codec *codec) +{ + int err = snd_hda_gen_build_controls(codec); + if (err < 0) + return err; + stac_init_power_map(codec); + return 0; +}
static int stac_init(struct hda_codec *codec) { @@ -4388,7 +4394,7 @@ static int stac_suspend(struct hda_codec *codec) #endif /* CONFIG_PM */
static const struct hda_codec_ops stac_patch_ops = { - .build_controls = snd_hda_gen_build_controls, + .build_controls = stac_build_controls, .build_pcms = snd_hda_gen_build_pcms, .init = stac_init, .free = stac_free,
On 2014-09-11 12:07, Takashi Iwai wrote:
At Thu, 11 Sep 2014 11:42:41 +0200, David Henningsson wrote:
On 2014-09-11 11:23, Takashi Iwai wrote:
At Thu, 11 Sep 2014 11:07:10 +0200, David Henningsson wrote:
On 2014-09-11 09:47, Adam Richter wrote:
Hi.
Hi Adam,
Interesting problem. Could you submit alsa-info ( http://www.alsa-project.org/alsa-info.sh ) and point us to it? It makes it possible to run the code in an emulator.
This is a bug and suggested temporary fix that I originally posted to the linux-sound mailing list about a month and a half ago. I am grateful to Takashi Awai for informing me after my follow-up inquiry about it today that I should submit it to the alsa-devel mailing list instead. Please feel free to redirect me further if appropriate. I did not notice any contact information for sound/pci/hda in linux-3.16-rc4/MAINATINERS.
Anyhow, here is the bug report and a one line proposed temporary fix.
The "model=no-jd" argument has not initialized the analog audio output jack correctly for me since linux-3.9-rc1 (if I recall correctly) through linux-3.17-rc4, although I have not tried every release candidate in between.
A lot of things changed with 3.9, so that makes sense. Btw, "model=no-jd" works only for specific machines. You can use the hint "jack-detect=0" instead.
The symptom is that, on a computer with an analog audio output jack that has incorrect jack sense (a hardware bug), audio output is completely muted until I physically replug the cable, even though I specified "model=no-jd" as an argument to the snd-hda-intel kernel module, which is supposed to cause the kernel to ignore the jack sense reported by hardware and just drive the audio output even if the hardware jack sense indicates nothing is plugged in. This problem did not occur until approximate Linux 3.9-rc1.
I have found a few single line workarounds that work, of which my favorite is the following (also attached to this email, in case any mailer subjects this message to reformatting), because it does not add code to anything that gets called frequently.
--- linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c.orig 2014-09-07 16:09:43.000000000 -0700 +++ linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c 2014-09-10 18:41:53.422900040 -0700 @@ -106,6 +106,7 @@ snd_hda_jack_tbl_new(struct hda_codec *c jack->nid = nid; jack->jack_dirty = 1; jack->tag = codec->jacktbl.used;
- jack->phantom_jack = codec->no_jack_detect; return jack; } EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
Signed-off-by: Adam J. Richter adam_richter2004@yahoo.com
I consider this a workaround rather than than a perfect fix, because I think the underlying problem seems to be some kind of initialization order issue that I don't fully understand. Basically, by the time jack->phantom_jack was being set, some caller had already called jack_detect_update(), which loaded the incorrect jack sense result from hardware and cleared jack->jack_dirty, so the jack sense would not be set again. At least that is what I think the underlying problem probably is.
Also, if a change like this is to be integrated, I'd like to know if it might be better for the line that I added to be:
jack->phantom_jack = !is_jack_detectable(codec, nid);Yes, this would probably be better.
I guess that somehow snd_hda_jack_enable_callback gets called for the phantom jack, but I'm not sure exactly how (alsa-info would help).
Nevertheless I'm attaching a patch. I assume it also resolves your problem?
Takashi, what do you think of the attached patch?
Ah, "attached" was meant *your* attached patch. Let me see...
From e5ce94c6156d1c588f29745ade06d1d76c87a0a1 Mon Sep 17 00:00:00 2001 From: David Henningsson david.henningsson@canonical.com Date: Thu, 11 Sep 2014 11:04:42 +0200 Subject: [PATCH] ALSA: hda - Do not enable unsol events on phantom jacks
To make sure we don't enable unsol events on phantom jacks, we move the logic for determining what a phantom jack is to snd_hda_jack_tbl_new.
Reported-by: Adam Richter adam_richter2004@yahoo.com Signed-off-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_jack.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 9746d73..376ce8f 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -105,6 +105,12 @@ snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid) return NULL; jack->nid = nid; jack->jack_dirty = 1;
- if (get_wcaps_type(get_wcaps(codec, nid)) == AC_WID_PIN) {
unsigned int def_conf = snd_hda_codec_get_pincfg(codec, nid);unsigned int conn = get_defcfg_connect(def_conf);if ((conn != AC_JACK_PORT_COMPLEX) || !is_jack_detectable(codec, nid))jack->phantom_jack = 1;- }
This doesn't look good. The pincfg isn't always reliable at this point. The pin connection check is used in add_jack_kctl() in the current code just because its only caller is snd_hda_jack_add_kctls(), which takes the pincfg explicitly. But snd_hda_jakc_tbl_new() itself is neutral about the pincfg.
Are you saying that codec->user_pins/driver_pins/init_pins might not be properly initialized at this point? If so, maybe we should instead warn on somebody calling snd_hda_jack_tbl_new too early?
No, the hda_jack stuff itself isn't necessarily depending on the pincfg deeply. For example, is_jack_detectable() doesn't check the pin location (although it checks MISC_NO_PRESENCE bit as an exception).
Is there a good reason why the hda_jack stuff should not depend on pincfg?
Also, if we keep the detection in snd_hda_jack_tbl_new, we could change all other calls to is_jack_detectable to !jack->phantom_jack instead.
And, another problem is that it'll result in a phantom jack secretly without adding a proper name suffix.
Are you sure?
snd_hda_jack_add_kctl is called from add_jack_kctl and the hdmi driver. Add_jack_kctl now uses the phantom_jack flag (as initialized by jack_new) to create a proper name, i e, no difference. generic_hdmi_build_jack should probably replace its "is_jack_detectable" call to check the phantom_jack flag for consistency, but for all common cases it's no change there either.
Hm, OK, that won't happen. But my point is that it's wrong to check the phantom flag there.
In anyway, for catching such a bug, maybe we should put a WARN_ON() at snd_hda_jack_detect_enable_callback() like the patch below.
Takashi
-- diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 9746d73cec52..0645735e196d 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -225,6 +225,8 @@ int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid, return -ENOMEM; if (jack->jack_detect) return 0; /* already registered */
- if (WARN_ON(!is_jack_detectable(codec, nid)))
return 0;Remember that enable_callback is sometimes called for non-pin nodes, such as the afg node in the sigmatel driver. Calling is_jack_detectable on non-pin nodes seems wrong to me.
OK, then it's an overkill.
Looking at the alsa-info.sh output, the problem is that IDT codec parser creates jack detection for the power-control before the generic parser. There is also a wrong logic in stac_init_power_map()...
An untested fix is below.
Did you get any alsa-info output from Adam? Maybe he sent it privately to you.
Takashi
-- diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index f26ec04a29b5..526b5d39a2cb 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -565,8 +565,8 @@ static void stac_init_power_map(struct hda_codec *codec) if (snd_hda_jack_tbl_get(codec, nid)) continue; if (def_conf == AC_JACK_PORT_COMPLEX &&
!(spec->vref_mute_led_nid == nid ||is_jack_detectable(codec, nid))) {
spec->vref_mute_led_nid != nid &&is_jack_detectable(codec, nid)) { snd_hda_jack_detect_enable_callback(codec, nid, STAC_PWR_EVENT, jack_update_power);@@ -4272,11 +4272,17 @@ static int stac_parse_auto_config(struct hda_codec *codec) return err; }
- stac_init_power_map(codec);
- return 0; }
+static int stac_build_controls(struct hda_codec *codec) +{
- int err = snd_hda_gen_build_controls(codec);
- if (err < 0)
return err;- stac_init_power_map(codec);
- return 0;
+}
static int stac_init(struct hda_codec *codec) { @@ -4388,7 +4394,7 @@ static int stac_suspend(struct hda_codec *codec) #endif /* CONFIG_PM */
static const struct hda_codec_ops stac_patch_ops = {
- .build_controls = snd_hda_gen_build_controls,
- .build_controls = stac_build_controls, .build_pcms = snd_hda_gen_build_pcms, .init = stac_init, .free = stac_free,
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
At Thu, 11 Sep 2014 12:29:09 +0200, David Henningsson wrote:
On 2014-09-11 12:07, Takashi Iwai wrote:
At Thu, 11 Sep 2014 11:42:41 +0200, David Henningsson wrote:
On 2014-09-11 11:23, Takashi Iwai wrote:
At Thu, 11 Sep 2014 11:07:10 +0200, David Henningsson wrote:
On 2014-09-11 09:47, Adam Richter wrote:
Hi.
Hi Adam,
Interesting problem. Could you submit alsa-info ( http://www.alsa-project.org/alsa-info.sh ) and point us to it? It makes it possible to run the code in an emulator.
This is a bug and suggested temporary fix that I originally posted to the linux-sound mailing list about a month and a half ago. I am grateful to Takashi Awai for informing me after my follow-up inquiry about it today that I should submit it to the alsa-devel mailing list instead. Please feel free to redirect me further if appropriate. I did not notice any contact information for sound/pci/hda in linux-3.16-rc4/MAINATINERS.
Anyhow, here is the bug report and a one line proposed temporary fix.
The "model=no-jd" argument has not initialized the analog audio output jack correctly for me since linux-3.9-rc1 (if I recall correctly) through linux-3.17-rc4, although I have not tried every release candidate in between.
A lot of things changed with 3.9, so that makes sense. Btw, "model=no-jd" works only for specific machines. You can use the hint "jack-detect=0" instead.
The symptom is that, on a computer with an analog audio output jack that has incorrect jack sense (a hardware bug), audio output is completely muted until I physically replug the cable, even though I specified "model=no-jd" as an argument to the snd-hda-intel kernel module, which is supposed to cause the kernel to ignore the jack sense reported by hardware and just drive the audio output even if the hardware jack sense indicates nothing is plugged in. This problem did not occur until approximate Linux 3.9-rc1.
I have found a few single line workarounds that work, of which my favorite is the following (also attached to this email, in case any mailer subjects this message to reformatting), because it does not add code to anything that gets called frequently.
--- linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c.orig 2014-09-07 16:09:43.000000000 -0700 +++ linux-3.17.0-rc4-64bit/sound/pci/hda/hda_jack.c 2014-09-10 18:41:53.422900040 -0700 @@ -106,6 +106,7 @@ snd_hda_jack_tbl_new(struct hda_codec *c jack->nid = nid; jack->jack_dirty = 1; jack->tag = codec->jacktbl.used;
- jack->phantom_jack = codec->no_jack_detect; return jack; } EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
Signed-off-by: Adam J. Richter adam_richter2004@yahoo.com
I consider this a workaround rather than than a perfect fix, because I think the underlying problem seems to be some kind of initialization order issue that I don't fully understand. Basically, by the time jack->phantom_jack was being set, some caller had already called jack_detect_update(), which loaded the incorrect jack sense result from hardware and cleared jack->jack_dirty, so the jack sense would not be set again. At least that is what I think the underlying problem probably is.
Also, if a change like this is to be integrated, I'd like to know if it might be better for the line that I added to be:
jack->phantom_jack = !is_jack_detectable(codec, nid);Yes, this would probably be better.
I guess that somehow snd_hda_jack_enable_callback gets called for the phantom jack, but I'm not sure exactly how (alsa-info would help).
Nevertheless I'm attaching a patch. I assume it also resolves your problem?
Takashi, what do you think of the attached patch?
Ah, "attached" was meant *your* attached patch. Let me see...
From e5ce94c6156d1c588f29745ade06d1d76c87a0a1 Mon Sep 17 00:00:00 2001 From: David Henningsson david.henningsson@canonical.com Date: Thu, 11 Sep 2014 11:04:42 +0200 Subject: [PATCH] ALSA: hda - Do not enable unsol events on phantom jacks
To make sure we don't enable unsol events on phantom jacks, we move the logic for determining what a phantom jack is to snd_hda_jack_tbl_new.
Reported-by: Adam Richter adam_richter2004@yahoo.com Signed-off-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_jack.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 9746d73..376ce8f 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -105,6 +105,12 @@ snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid) return NULL; jack->nid = nid; jack->jack_dirty = 1;
- if (get_wcaps_type(get_wcaps(codec, nid)) == AC_WID_PIN) {
unsigned int def_conf = snd_hda_codec_get_pincfg(codec, nid);unsigned int conn = get_defcfg_connect(def_conf);if ((conn != AC_JACK_PORT_COMPLEX) || !is_jack_detectable(codec, nid))jack->phantom_jack = 1;- }
This doesn't look good. The pincfg isn't always reliable at this point. The pin connection check is used in add_jack_kctl() in the current code just because its only caller is snd_hda_jack_add_kctls(), which takes the pincfg explicitly. But snd_hda_jakc_tbl_new() itself is neutral about the pincfg.
Are you saying that codec->user_pins/driver_pins/init_pins might not be properly initialized at this point? If so, maybe we should instead warn on somebody calling snd_hda_jack_tbl_new too early?
No, the hda_jack stuff itself isn't necessarily depending on the pincfg deeply. For example, is_jack_detectable() doesn't check the pin location (although it checks MISC_NO_PRESENCE bit as an exception).
Is there a good reason why the hda_jack stuff should not depend on pincfg?
Because the pincfg isn't always reliable, as you know ;)
Also, if we keep the detection in snd_hda_jack_tbl_new, we could change all other calls to is_jack_detectable to !jack->phantom_jack instead.
Well, I think is_jack_detectable() is rather a helper to be moved to hda_codec.c, i.e. its functionality is independent from what else in hda_jack.c. In other words, I don't think we want to tie it with hda_jack stuff (at least in the current form of hda_jack.c infrastructure).
And, another problem is that it'll result in a phantom jack secretly without adding a proper name suffix.
Are you sure?
snd_hda_jack_add_kctl is called from add_jack_kctl and the hdmi driver. Add_jack_kctl now uses the phantom_jack flag (as initialized by jack_new) to create a proper name, i e, no difference. generic_hdmi_build_jack should probably replace its "is_jack_detectable" call to check the phantom_jack flag for consistency, but for all common cases it's no change there either.
Hm, OK, that won't happen. But my point is that it's wrong to check the phantom flag there.
In anyway, for catching such a bug, maybe we should put a WARN_ON() at snd_hda_jack_detect_enable_callback() like the patch below.
Takashi
-- diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 9746d73cec52..0645735e196d 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -225,6 +225,8 @@ int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid, return -ENOMEM; if (jack->jack_detect) return 0; /* already registered */
- if (WARN_ON(!is_jack_detectable(codec, nid)))
return 0;Remember that enable_callback is sometimes called for non-pin nodes, such as the afg node in the sigmatel driver. Calling is_jack_detectable on non-pin nodes seems wrong to me.
OK, then it's an overkill.
Looking at the alsa-info.sh output, the problem is that IDT codec parser creates jack detection for the power-control before the generic parser. There is also a wrong logic in stac_init_power_map()...
An untested fix is below.
Did you get any alsa-info output from Adam? Maybe he sent it privately to you.
I guess this was blocked by the mail server due to a too big attachment...
Takashi
At Thu, 11 Sep 2014 12:07:43 +0200, Takashi Iwai wrote:
Looking at the alsa-info.sh output, the problem is that IDT codec parser creates jack detection for the power-control before the generic parser. There is also a wrong logic in stac_init_power_map()...
An untested fix is below.
BTW, a part of the messes is the restriction of hda_jack stuff: currently it supports only one callback per jack. If we can add multiple callbacks and actions to a jack, the code in patch_sigmatel.c would be come much easier. But this is a step after fixing the bug quickly.
Takashi
Takashi
-- diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index f26ec04a29b5..526b5d39a2cb 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -565,8 +565,8 @@ static void stac_init_power_map(struct hda_codec *codec) if (snd_hda_jack_tbl_get(codec, nid)) continue; if (def_conf == AC_JACK_PORT_COMPLEX &&
!(spec->vref_mute_led_nid == nid ||is_jack_detectable(codec, nid))) {
spec->vref_mute_led_nid != nid &&is_jack_detectable(codec, nid)) { snd_hda_jack_detect_enable_callback(codec, nid, STAC_PWR_EVENT, jack_update_power);@@ -4272,11 +4272,17 @@ static int stac_parse_auto_config(struct hda_codec *codec) return err; }
- stac_init_power_map(codec);
- return 0;
}
+static int stac_build_controls(struct hda_codec *codec) +{
- int err = snd_hda_gen_build_controls(codec);
- if (err < 0)
return err;- stac_init_power_map(codec);
- return 0;
+}
static int stac_init(struct hda_codec *codec) { @@ -4388,7 +4394,7 @@ static int stac_suspend(struct hda_codec *codec) #endif /* CONFIG_PM */
static const struct hda_codec_ops stac_patch_ops = {
- .build_controls = snd_hda_gen_build_controls,
- .build_controls = stac_build_controls, .build_pcms = snd_hda_gen_build_pcms, .init = stac_init, .free = stac_free,
Hi, Takashi.
I confirm that your patch, quoted below, eliminated the symptom.
Please feel free to ask me to test other patches related to this problem.
Also, by the way, it appears that linux-3.17-rc4's Intel "high defintiion audio" driver, regardless of whether this patch is applied, no longer shows Linux input subsystem devices for analog audio jack sense, although it does provide such a device for HDMI jack sense. If that's a bug, it's completely separate, but I mention it here in case it's relevant to integrating a fix to this issue.
Thanks to you and David for your prompt and careful attention this bug once I brought it to this mailing list.
Adam Richter
At Thu, 11 Sep 2014 12:07:43 +0200, Takashi Iwai wrote:
-- diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c index f26ec04a29b5..526b5d39a2cb 100644 --- a/sound/pci/hda/patch_sigmatel.c +++ b/sound/pci/hda/patch_sigmatel.c @@ -565,8 +565,8 @@ static void stac_init_power_map(struct hda_codec *codec) if (snd_hda_jack_tbl_get(codec, nid)) continue; if (def_conf == AC_JACK_PORT_COMPLEX && - !(spec->vref_mute_led_nid == nid || - is_jack_detectable(codec, nid))) { + spec->vref_mute_led_nid != nid && + is_jack_detectable(codec, nid)) { snd_hda_jack_detect_enable_callback(codec, nid, STAC_PWR_EVENT, jack_update_power); @@ -4272,11 +4272,17 @@ static int
stac_parse_auto_config(struct hda_codec *codec)
return err; } - stac_init_power_map(codec);
return 0; } +static int stac_build_controls(struct hda_codec *codec) +{ + int err =
snd_hda_gen_build_controls(codec);
+ if (err < 0) + return err; + stac_init_power_map(codec); + return 0; +} static int stac_init(struct hda_codec *codec) { @@ -4388,7 +4394,7 @@ static int stac_suspend(struct hda_codec *codec) #endif /* CONFIG_PM */ static const struct hda_codec_ops stac_patch_ops = { - .build_controls = snd_hda_gen_build_controls, + .build_controls = stac_build_controls, .build_pcms = snd_hda_gen_build_pcms, .init = stac_init, .free = stac_free,
Hi, again, Takashi.
A few minute ago, I wrote a side comment that you should ignore, which was the following:
Also, by the way, it appears that linux-3.17-rc4's Intel "high defintiion audio" driver, regardless of whether this patch is applied, no longer shows Linux input subsystem devices for analog audio jack sense, although it does provide such a device for HDMI jack sense. [...]
In Linux 3.17-rc4, reloading the snd-hda-intel module without "model=no-jd" results in those input devices being created. So, it is not a bug. In fact, I think is completely correct and an improvement that mode=no-jd now causes the jack sense Linux input layer devices not to be created, which should make it easier for userspace to detect and adapt to this situation.
Sorry for the distraction.
Adam Richter
At Thu, 11 Sep 2014 21:30:25 -0700, Adam Richter wrote:
Hi, Takashi.
I confirm that your patch, quoted below, eliminated the symptom.
Please feel free to ask me to test other patches related to this problem.
Also, by the way, it appears that linux-3.17-rc4's Intel "high defintiion audio" driver, regardless of whether this patch is applied, no longer shows Linux input subsystem devices for analog audio jack sense, although it does provide such a device for HDMI jack sense. If that's a bug, it's completely separate, but I mention it here in case it's relevant to integrating a fix to this issue.
Thanks to you and David for your prompt and careful attention this bug once I brought it to this mailing list.
Good to hear. I merged the patch now for-linus branch.
Takashi
participants (3)
-
Adam Richter -
David Henningsson -
Takashi Iwai