[alsa-devel] [PATCH] ALSA: HDA: Fix jack creation for codecs with front and rear Line In
If a codec has both a front and a rear Line In, two controls both named "Line Jack" will be created, which causes parsing to fail. While a long term solution might be to name the jacks differently, this extra check is consistent with what is already being done in many auto-parsers, and will also protect against other cases when two inputs have the same label.
BugLink: https://bugs.launchpad.net/bugs/923409 Signed-off-by: David Henningsson david.henningsson@canonical.com --- sound/pci/hda/hda_jack.c | 24 +++++++++++++++--------- 1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index d8a35da..9d819c4 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -282,7 +282,8 @@ int snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid, EXPORT_SYMBOL_HDA(snd_hda_jack_add_kctl);
static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid, - const struct auto_pin_cfg *cfg) + const struct auto_pin_cfg *cfg, + char *lastname, int *lastidx) { unsigned int def_conf, conn; char name[44]; @@ -298,6 +299,10 @@ static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid, return 0;
snd_hda_get_pin_label(codec, nid, cfg, name, sizeof(name), &idx); + if (!strcmp(name, lastname) && idx == *lastidx) + idx++; + strncpy(lastname, name, 44); + *lastidx = idx; err = snd_hda_jack_add_kctl(codec, nid, name, idx); if (err < 0) return err; @@ -311,41 +316,42 @@ int snd_hda_jack_add_kctls(struct hda_codec *codec, const struct auto_pin_cfg *cfg) { const hda_nid_t *p; - int i, err; + int i, err, lastidx = 0; + char lastname[44] = "";
for (i = 0, p = cfg->line_out_pins; i < cfg->line_outs; i++, p++) { - err = add_jack_kctl(codec, *p, cfg); + err = add_jack_kctl(codec, *p, cfg, lastname, &lastidx); if (err < 0) return err; } for (i = 0, p = cfg->hp_pins; i < cfg->hp_outs; i++, p++) { if (*p == *cfg->line_out_pins) /* might be duplicated */ break; - err = add_jack_kctl(codec, *p, cfg); + err = add_jack_kctl(codec, *p, cfg, lastname, &lastidx); if (err < 0) return err; } for (i = 0, p = cfg->speaker_pins; i < cfg->speaker_outs; i++, p++) { if (*p == *cfg->line_out_pins) /* might be duplicated */ break; - err = add_jack_kctl(codec, *p, cfg); + err = add_jack_kctl(codec, *p, cfg, lastname, &lastidx); if (err < 0) return err; } for (i = 0; i < cfg->num_inputs; i++) { - err = add_jack_kctl(codec, cfg->inputs[i].pin, cfg); + err = add_jack_kctl(codec, cfg->inputs[i].pin, cfg, lastname, &lastidx); if (err < 0) return err; } for (i = 0, p = cfg->dig_out_pins; i < cfg->dig_outs; i++, p++) { - err = add_jack_kctl(codec, *p, cfg); + err = add_jack_kctl(codec, *p, cfg, lastname, &lastidx); if (err < 0) return err; } - err = add_jack_kctl(codec, cfg->dig_in_pin, cfg); + err = add_jack_kctl(codec, cfg->dig_in_pin, cfg, lastname, &lastidx); if (err < 0) return err; - err = add_jack_kctl(codec, cfg->mono_out_pin, cfg); + err = add_jack_kctl(codec, cfg->mono_out_pin, cfg, lastname, &lastidx); if (err < 0) return err; return 0;
2012/1/31, David Henningsson david.henningsson@canonical.com:
If a codec has both a front and a rear Line In, two controls both named "Line Jack" will be created, which causes parsing to fail. While a long term solution might be to name the jacks differently, this extra check is consistent with what is already being done in many auto-parsers, and will also protect against other cases when two inputs have the same label.
BugLink: https://bugs.launchpad.net/bugs/923409 Signed-off-by: David Henningsson david.henningsson@canonical.com
I have doubt since most of ad1988 6stack-dig quirks have two Line In ( one is ext front and the other is int ATAPI and the "Input Source" controls are broken
On 01/31/2012 09:29 AM, Raymond Yau wrote:
2012/1/31, David Henningssondavid.henningsson@canonical.com:
If a codec has both a front and a rear Line In, two controls both named "Line Jack" will be created, which causes parsing to fail. While a long term solution might be to name the jacks differently, this extra check is consistent with what is already being done in many auto-parsers, and will also protect against other cases when two inputs have the same label.
BugLink: https://bugs.launchpad.net/bugs/923409 Signed-off-by: David Henningssondavid.henningsson@canonical.com
I have doubt since most of ad1988 6stack-dig quirks have two Line In ( one is ext front and the other is int ATAPI and the "Input Source" controls are broken
Well, it won't fix any "Input Source" controls, of course. Only jack creation later, assuming you have your autocfg struct set up correctly.
2012/1/31, David Henningsson david.henningsson@canonical.com:
On 01/31/2012 09:29 AM, Raymond Yau wrote:
2012/1/31, David Henningssondavid.henningsson@canonical.com:
If a codec has both a front and a rear Line In, two controls both named "Line Jack" will be created, which causes parsing to fail. While a long term solution might be to name the jacks differently, this extra check is consistent with what is already being done in many auto-parsers, and will also protect against other cases when two inputs have the same label.
BugLink: https://bugs.launchpad.net/bugs/923409 Signed-off-by: David Henningssondavid.henningsson@canonical.com
I have doubt since most of ad1988 6stack-dig quirks have two Line In ( one is ext front and the other is int ATAPI and the "Input Source" controls are broken
Well, it won't fix any "Input Source" controls, of course. Only jack creation later, assuming you have your autocfg struct set up correctly.
There is another bug in snd_hda_parse_def_pincfg() when bios assigned two Line Out at ext front and rear 0x14 and 0x1b
autoconfig: line_outs=1 (0x14/0x0/0x0/0x0/0x0) type:line speaker_outs=0 (0x0/0x0/0x0/0x0/0x0) hp_outs=0 (0x0/0x0/0x0/0x0/0x0) mono: mono_out=0x0 dig-out=0x1e/0x0 inputs: Rear Mic=0x18 Front Mic=0x19 Line=0x1a
At Tue, 31 Jan 2012 09:04:15 +0100, David Henningsson wrote:
If a codec has both a front and a rear Line In, two controls both named "Line Jack" will be created, which causes parsing to fail. While a long term solution might be to name the jacks differently, this extra check is consistent with what is already being done in many auto-parsers, and will also protect against other cases when two inputs have the same label.
BugLink: https://bugs.launchpad.net/bugs/923409 Signed-off-by: David Henningsson david.henningsson@canonical.com
Thanks, applied now.
BTW, any chance to change apport to gather alsa-info.sh output? It'd be a great help, so that we can check it via hda-emu.
Takashi
sound/pci/hda/hda_jack.c | 24 +++++++++++++++--------- 1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index d8a35da..9d819c4 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -282,7 +282,8 @@ int snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid, EXPORT_SYMBOL_HDA(snd_hda_jack_add_kctl);
static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid,
const struct auto_pin_cfg *cfg)
const struct auto_pin_cfg *cfg,char *lastname, int *lastidx){ unsigned int def_conf, conn; char name[44]; @@ -298,6 +299,10 @@ static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid, return 0;
snd_hda_get_pin_label(codec, nid, cfg, name, sizeof(name), &idx);
- if (!strcmp(name, lastname) && idx == *lastidx)
idx++;- strncpy(lastname, name, 44);
- *lastidx = idx; err = snd_hda_jack_add_kctl(codec, nid, name, idx); if (err < 0) return err;
@@ -311,41 +316,42 @@ int snd_hda_jack_add_kctls(struct hda_codec *codec, const struct auto_pin_cfg *cfg) { const hda_nid_t *p;
- int i, err;
int i, err, lastidx = 0;
char lastname[44] = "";
for (i = 0, p = cfg->line_out_pins; i < cfg->line_outs; i++, p++) {
err = add_jack_kctl(codec, *p, cfg);
err = add_jack_kctl(codec, *p, cfg, lastname, &lastidx);
err = add_jack_kctl(codec, *p, cfg);
err = add_jack_kctl(codec, *p, cfg, lastname, &lastidx);
err = add_jack_kctl(codec, *p, cfg);
err = add_jack_kctl(codec, *p, cfg, lastname, &lastidx);
err = add_jack_kctl(codec, cfg->inputs[i].pin, cfg);
err = add_jack_kctl(codec, cfg->inputs[i].pin, cfg, lastname, &lastidx);
err = add_jack_kctl(codec, *p, cfg);
err = add_jack_kctl(codec, *p, cfg, lastname, &lastidx);
- err = add_jack_kctl(codec, cfg->dig_in_pin, cfg);
- err = add_jack_kctl(codec, cfg->dig_in_pin, cfg, lastname, &lastidx); if (err < 0) return err;
- err = add_jack_kctl(codec, cfg->mono_out_pin, cfg);
- err = add_jack_kctl(codec, cfg->mono_out_pin, cfg, lastname, &lastidx); if (err < 0) return err; return 0;
-- 1.7.8.3
On 01/31/2012 03:17 PM, Takashi Iwai wrote:
At Tue, 31 Jan 2012 09:04:15 +0100, David Henningsson wrote:
If a codec has both a front and a rear Line In, two controls both named "Line Jack" will be created, which causes parsing to fail. While a long term solution might be to name the jacks differently, this extra check is consistent with what is already being done in many auto-parsers, and will also protect against other cases when two inputs have the same label.
BugLink: https://bugs.launchpad.net/bugs/923409 Signed-off-by: David Henningssondavid.henningsson@canonical.com
Thanks, applied now.
BTW, any chance to change apport to gather alsa-info.sh output? It'd be a great help, so that we can check it via hda-emu.
I have asked for this particular bug reporter to supply alsa-info as well, but may I ask you...do you have some kind of regression test script, and if so, are they public? Or could it a part of hda-emu?
It would be nice if you could run "make tests" or something from the hda-emu source directory, and it would parse all codecs and report error if parsing fails (or pm fails, etc).
At Tue, 31 Jan 2012 16:28:14 +0100, David Henningsson wrote:
On 01/31/2012 03:17 PM, Takashi Iwai wrote:
At Tue, 31 Jan 2012 09:04:15 +0100, David Henningsson wrote:
If a codec has both a front and a rear Line In, two controls both named "Line Jack" will be created, which causes parsing to fail. While a long term solution might be to name the jacks differently, this extra check is consistent with what is already being done in many auto-parsers, and will also protect against other cases when two inputs have the same label.
BugLink: https://bugs.launchpad.net/bugs/923409 Signed-off-by: David Henningssondavid.henningsson@canonical.com
Thanks, applied now.
BTW, any chance to change apport to gather alsa-info.sh output? It'd be a great help, so that we can check it via hda-emu.
I have asked for this particular bug reporter to supply alsa-info as well, but may I ask you...do you have some kind of regression test script, and if so, are they public? Or could it a part of hda-emu?
The checks I often do are scripts like below. Run for all files and check the difference before and after the patch.
It would be nice if you could run "make tests" or something from the hda-emu source directory, and it would parse all codecs and report error if parsing fails (or pm fails, etc).
Yeah, any patch is welcome ;)
Takashi
On 01/31/2012 04:40 PM, Takashi Iwai wrote:
At Tue, 31 Jan 2012 16:28:14 +0100, David Henningsson wrote:
On 01/31/2012 03:17 PM, Takashi Iwai wrote:
At Tue, 31 Jan 2012 09:04:15 +0100, David Henningsson wrote:
If a codec has both a front and a rear Line In, two controls both named "Line Jack" will be created, which causes parsing to fail. While a long term solution might be to name the jacks differently, this extra check is consistent with what is already being done in many auto-parsers, and will also protect against other cases when two inputs have the same label.
BugLink: https://bugs.launchpad.net/bugs/923409 Signed-off-by: David Henningssondavid.henningsson@canonical.com
Thanks, applied now.
BTW, any chance to change apport to gather alsa-info.sh output?
This has now been provided: http://www.alsa-project.org/db/?f=f4a8312f34573a24f60011109e18f0c7371129e0
It'd be a great help, so that we can check it via hda-emu.
I have asked for this particular bug reporter to supply alsa-info as well, but may I ask you...do you have some kind of regression test script, and if so, are they public? Or could it a part of hda-emu?
The checks I often do are scripts like below. Run for all files and check the difference before and after the patch.
It would be nice if you could run "make tests" or something from the hda-emu source directory, and it would parse all codecs and report error if parsing fails (or pm fails, etc).
Yeah, any patch is welcome ;)
Thanks for the small test scripts! It's good to be on the same page. Hopefully I get around some day to write a "make tests", but not right now unfortunately.
I quickly ran test2 and noticed that - We have quite a lot of work to do before all errors are fixed! - The error I got from hda-emu for the codec above, 'Control element Line Jack:0 already exists!' was already present for one of the codecs.
At Wed, 01 Feb 2012 09:19:25 +0100, David Henningsson wrote:
On 01/31/2012 04:40 PM, Takashi Iwai wrote:
At Tue, 31 Jan 2012 16:28:14 +0100, David Henningsson wrote:
On 01/31/2012 03:17 PM, Takashi Iwai wrote:
At Tue, 31 Jan 2012 09:04:15 +0100, David Henningsson wrote:
If a codec has both a front and a rear Line In, two controls both named "Line Jack" will be created, which causes parsing to fail. While a long term solution might be to name the jacks differently, this extra check is consistent with what is already being done in many auto-parsers, and will also protect against other cases when two inputs have the same label.
BugLink: https://bugs.launchpad.net/bugs/923409 Signed-off-by: David Henningssondavid.henningsson@canonical.com
Thanks, applied now.
BTW, any chance to change apport to gather alsa-info.sh output?
This has now been provided: http://www.alsa-project.org/db/?f=f4a8312f34573a24f60011109e18f0c7371129e0
It'd be a great help, so that we can check it via hda-emu.
I have asked for this particular bug reporter to supply alsa-info as well, but may I ask you...do you have some kind of regression test script, and if so, are they public? Or could it a part of hda-emu?
The checks I often do are scripts like below. Run for all files and check the difference before and after the patch.
It would be nice if you could run "make tests" or something from the hda-emu source directory, and it would parse all codecs and report error if parsing fails (or pm fails, etc).
Yeah, any patch is welcome ;)
Thanks for the small test scripts! It's good to be on the same page. Hopefully I get around some day to write a "make tests", but not right now unfortunately.
I quickly ran test2 and noticed that
- We have quite a lot of work to do before all errors are fixed!
Many errors come from the static quirks and they are harmless. You can pass -mauto to test scripts for checking the auto-parser.
- The error I got from hda-emu for the codec above, 'Control element
Line Jack:0 already exists!' was already present for one of the codecs.
What a shame...
thanks,
Takashi
participants (3)
-
David Henningsson -
Raymond Yau -
Takashi Iwai