Ubuntu
linux package

[Lenovo X200s] Headphone Jacks state error

Bug #1060729 reported by Meng Zhuo
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
linux (Ubuntu)
Fix Released
Undecided
David Henningsson

Bug Description

My Lenovo X200s has only one "Green Color Headphone Jack"
http://www.thinkwiki.org/wiki/Category:X200s

But in amixer and I quoted David Henningsson:
https://bugs.freedesktop.org/show_bug.cgi?id=51354#c6
>>>
-------------
numid=23,iface=CARD,name='Headphone Jack',index=1
  ; type=BOOLEAN,access=r-------,values=1
  : values=on
-------------
The problem is that 'Headphone Jack' has an index > 0. This is done by ALSA when there are two different headphone jacks and has no other way to distinguish them. If there is no other 'Headphone Jack', without an index, it's a bug in ALSA.
 >>>
Therefor I filed this bug report.

Tags: quantal
Revision history for this message
Meng Zhuo (mengzhuo1203) wrote :
Revision history for this message
David Henningsson (diwic) wrote :

AlsaInfo extracted from the attachment.

Revision history for this message
David Henningsson (diwic) wrote : [PATCH] ALSA: hda - avoid unneccesary indices on "Headphone Jack" controls

In case there is one "Headphone Jack" and one "Dock Headphone Jack",
one of them will get an index, even though that is not needed.
This patch fixes that issue.

BugLink: https://bugs.launchpad.net/bugs/1060729
Signed-off-by: David Henningsson <email address hidden>
---
 sound/pci/hda/hda_auto_parser.c | 49 +++++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 12 deletions(-)

Alsa-info at: https://launchpadlibrarian.net/118211803/AlsaInfo

diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c
index a98e25e..4ec6dc8 100644
--- a/sound/pci/hda/hda_auto_parser.c
+++ b/sound/pci/hda/hda_auto_parser.c
@@ -498,6 +498,38 @@ static const char *check_output_sfx(hda_nid_t nid, const hda_nid_t *pins,
  return channel_sfx[i];
 }

+static const char *check_output_pfx(struct hda_codec *codec, hda_nid_t nid)
+{
+ unsigned int def_conf = snd_hda_codec_get_pincfg(codec, nid);
+ int attr = snd_hda_get_input_pin_attr(def_conf);
+
+ /* check the location */
+ switch (attr) {
+ case INPUT_PIN_ATTR_DOCK:
+ return "Dock ";
+ case INPUT_PIN_ATTR_FRONT:
+ return "Front ";
+ }
+ return "";
+}
+
+static int get_hp_label_index(struct hda_codec *codec, hda_nid_t nid,
+ const hda_nid_t *pins, int num_pins)
+{
+ int i, j, idx = 0;
+
+ const char *pfx = check_output_pfx(codec, nid);
+
+ i = find_idx_in_nid_list(nid, pins, num_pins);
+ if (i < 0)
+ return -1;
+ for (j = 0; j < i; j++)
+ if (pfx == check_output_pfx(codec, pins[j]))
+ idx++;
+
+ return idx;
+}
+
 static int fill_audio_out_name(struct hda_codec *codec, hda_nid_t nid,
           const struct auto_pin_cfg *cfg,
           const char *name, char *label, int maxlen,
@@ -505,20 +537,13 @@ static int fill_audio_out_name(struct hda_codec *codec, hda_nid_t nid,
 {
  unsigned int def_conf = snd_hda_codec_get_pincfg(codec, nid);
  int attr = snd_hda_get_input_pin_attr(def_conf);
- const char *pfx = "", *sfx = "";
+ const char *pfx, *sfx = "";

  /* handle as a speaker if it's a fixed line-out */
  if (!strcmp(name, "Line Out") && attr == INPUT_PIN_ATTR_INT)
   name = "Speaker";
- /* check the location */
- switch (attr) {
- case INPUT_PIN_ATTR_DOCK:
- pfx = "Dock ";
- break;
- case INPUT_PIN_ATTR_FRONT:
- pfx = "Front ";
- break;
- }
+ pfx = check_output_pfx(codec, nid);
+
  if (cfg) {
   /* try to give a unique suffix if needed */
   sfx = check_output_sfx(nid, cfg->line_out_pins, cfg->line_outs,
@@ -528,8 +553,8 @@ static int fill_audio_out_name(struct hda_codec *codec, hda_nid_t nid,
             indexp);
   if (!sfx) {
    /* don't add channel suffix for Headphone controls */
- int idx = find_idx_in_nid_list(nid, cfg->hp_pins,
- cfg->hp_outs);
+ int idx = get_hp_label_index(codec, nid, cfg->hp_pins,
+ cfg->hp_outs);
    if (idx >= 0)
     *indexp = idx;
    sfx = "";
--
1.7.9.5

Revision history for this message
David Henningsson (diwic) wrote :

Thanks! I've been able to come up with a patch to correct this error, and sent it to upstream. Let's hope it's accepted!

Changed in alsa-driver (Ubuntu):
assignee: nobody → David Henningsson (diwic)
status: New → In Progress
summary: - Headphone Jacks state error
+ [Lenovo X200s] Headphone Jacks state error
Revision history for this message
Poncho (poncho) wrote :

I can confirm that your patch fixes the pulseaudio jack detection on my Elitebook 8540w running gentoo.

Thanks.

Revision history for this message
Meng Zhuo (mengzhuo1203) wrote :

Where should this patch apply to?
I can't find "hda_auto_parser.c" in the source code by running "apt-get source alsa-driver"

Revision history for this message
Takashi Iwai (tiwai) wrote :

At Wed, 3 Oct 2012 11:12:53 +0200,
David Henningsson wrote:
>
> In case there is one "Headphone Jack" and one "Dock Headphone Jack",
> one of them will get an index, even though that is not needed.
> This patch fixes that issue.
>
> BugLink: https://bugs.launchpad.net/bugs/1060729
> Signed-off-by: David Henningsson <email address hidden>

Applied. This one doesn't need to go to stable, right?

Takashi

> ---
> sound/pci/hda/hda_auto_parser.c | 49 +++++++++++++++++++++++++++++----------
> 1 file changed, 37 insertions(+), 12 deletions(-)
>
> Alsa-info at: https://launchpadlibrarian.net/118211803/AlsaInfo
>
> diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c
> index a98e25e..4ec6dc8 100644
> --- a/sound/pci/hda/hda_auto_parser.c
> +++ b/sound/pci/hda/hda_auto_parser.c
> @@ -498,6 +498,38 @@ static const char *check_output_sfx(hda_nid_t nid, const hda_nid_t *pins,
> return channel_sfx[i];
> }
>
> +static const char *check_output_pfx(struct hda_codec *codec, hda_nid_t nid)
> +{
> + unsigned int def_conf = snd_hda_codec_get_pincfg(codec, nid);
> + int attr = snd_hda_get_input_pin_attr(def_conf);
> +
> + /* check the location */
> + switch (attr) {
> + case INPUT_PIN_ATTR_DOCK:
> + return "Dock ";
> + case INPUT_PIN_ATTR_FRONT:
> + return "Front ";
> + }
> + return "";
> +}
> +
> +static int get_hp_label_index(struct hda_codec *codec, hda_nid_t nid,
> + const hda_nid_t *pins, int num_pins)
> +{
> + int i, j, idx = 0;
> +
> + const char *pfx = check_output_pfx(codec, nid);
> +
> + i = find_idx_in_nid_list(nid, pins, num_pins);
> + if (i < 0)
> + return -1;
> + for (j = 0; j < i; j++)
> + if (pfx == check_output_pfx(codec, pins[j]))
> + idx++;
> +
> + return idx;
> +}
> +
> static int fill_audio_out_name(struct hda_codec *codec, hda_nid_t nid,
> const struct auto_pin_cfg *cfg,
> const char *name, char *label, int maxlen,
> @@ -505,20 +537,13 @@ static int fill_audio_out_name(struct hda_codec *codec, hda_nid_t nid,
> {
> unsigned int def_conf = snd_hda_codec_get_pincfg(codec, nid);
> int attr = snd_hda_get_input_pin_attr(def_conf);
> - const char *pfx = "", *sfx = "";
> + const char *pfx, *sfx = "";
>
> /* handle as a speaker if it's a fixed line-out */
> if (!strcmp(name, "Line Out") && attr == INPUT_PIN_ATTR_INT)
> name = "Speaker";
> - /* check the location */
> - switch (attr) {
> - case INPUT_PIN_ATTR_DOCK:
> - pfx = "Dock ";
> - break;
> - case INPUT_PIN_ATTR_FRONT:
> - pfx = "Front ";
> - break;
> - }
> + pfx = check_output_pfx(codec, nid);
> +
> if (cfg) {
> /* try to give a unique suffix if needed */
> sfx = check_output_sfx(nid, cfg->line_out_pins, cfg->line_outs,
> @@ -528,8 +553,8 @@ static int fill_audio_out_name(struct hda_codec *codec, hda_nid_t nid,
> indexp);
> if (!sfx) {
> /* don't add channel suffix for Headphone controls */
> - int idx = find_idx_in_nid_list(nid, cfg->hp_pins,
> - cfg->hp_outs);
> + int idx = get_hp_label_index(codec, nid, cfg->hp_pins,
> + cfg->hp_outs);
> if (idx >= 0)
> *indexp = idx;
> sfx = "";
> --
> 1.7.9.5
>

Revision history for this message
David Henningsson (diwic) wrote :

@Meng, now that Takashi has applied it, from tomorrow on you should - hopefully - be able to test a fixed version this way: https://wiki.ubuntu.com/Audio/UpgradingAlsa/DKMS

Revision history for this message
David Henningsson (diwic) wrote :

On 10/08/2012 10:18 AM, Takashi Iwai wrote:
> At Wed, 3 Oct 2012 11:12:53 +0200,
> David Henningsson wrote:
>>
>> In case there is one "Headphone Jack" and one "Dock Headphone Jack",
>> one of them will get an index, even though that is not needed.
>> This patch fixes that issue.
>>
>> BugLink: https://bugs.launchpad.net/bugs/1060729
>> Signed-off-by: David Henningsson <email address hidden>
>
> Applied. This one doesn't need to go to stable, right?

Good question. I guess it depends on how we look upon kcontrol indices
in general.

PulseAudio never picks anything up if it has an index, therefore it
becomes a real problem/bug for users. (Not sure if one could argue that
this is a problem in PulseAudio rather than the kernel. Haven't thought
that through.)

OTOH, if the reasoning is that you would break custom made scripts that
depend on this index, i e it becomes a change of behaviour.

To be pragmatic about it, the chances that people are depending on this
index being non-zero are probably very low, so I don't mind sending this
one to stable.

--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Revision history for this message
Meng Zhuo (mengzhuo1203) wrote :

@diwic
DKMS encounter some errors while I am tring to install it, is it depend on a specific kernel?

Revision history for this message
David Henningsson (diwic) wrote :

@Meng,

DKMS make.log for alsa-hda-0.201210170702~precise1 for kernel 3.5.0-17-generic (x86_64)

Yes, it looks like you're trying to install the precise DKMS package on top of a quantal kernel. I don't understand the language of the rest of the log so I don't know what error you get.

I have now updated the daily builds so they build for quantal as well. When the builds have completed, you could try the quantal builds to see if they work better.

Revision history for this message
David Henningsson (diwic) wrote :

Fixed in Linux 3.7+, and 3.8 went into Ubuntu 13.04.

affects: alsa-driver (Ubuntu) → linux (Ubuntu)
Changed in linux (Ubuntu):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.