From: Hui Wang <hui.wang@canonical.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: 1478497@bugs.launchpad.net, alsa-devel@alsa-project.org,
woodrow.shen@canonical.com, david.henningsson@canonical.com,
stable@vger.kernel.org
Subject: Re: [alsa-devel] [PATCH] ALSA: hda - Add pin quirk for the headset mic jack detection on Dell laptop
Date: Fri, 31 Jul 2015 09:54:55 +0800 [thread overview]
Message-ID: <55BAD56F.2060309@canonical.com> (raw)
In-Reply-To: <s5hfv45rbnj.wl-tiwai@suse.de>
On 07/30/2015 09:57 PM, Takashi Iwai wrote:
> On Thu, 30 Jul 2015 12:11:00 +0200,
> Hui Wang wrote:
>> On 07/27/2015 06:52 PM, Takashi Iwai wrote:
>>> On Mon, 27 Jul 2015 12:34:31 +0200,
>>> woodrow.shen@canonical.com wrote:
>>>> From: Woodrow Shen <woodrow.shen@canonical.com>
>>>>
>>>> The new Dell laptop with codec 256 can't detect headset mic when headset was
>>>> inserted on the machine. From alsa-info, we check init_pin_configs and need to
>>>> define the new register value for pin 0x1d & 0x1e because the original macro
>>>> ALC256_STANDARD_PINS can't match pin definition. Also, the macro ALC256_STANDARD_PINS
>>>> is simplified by removing them. This makes headset mic works on laptop.
>>>>
>>>> Codec: Realtek ALC256
>>>> Vendor Id: 0x10ec0256
>>>> Subsystem Id: 0x102806f2
>>>>
>>>> BugLink: https://bugs.launchpad.net/bugs/1478497
>>>> Signed-off-by: Woodrow Shen <woodrow.shen@canonical.com>
>>> I applied this as is now. But the code there is already messy, and I
>>> wonder whether we should treat all 0x4xxxxxxx equally. So far, all
>>> pincfgs are regarded as a kind of fingerprint. But, judging from the
>>> actual values, the difference of 0x4xxxxxxx might be just a leftover
>>> of wastes by BIOS programmers.
>>>
>>> I don't mind any other way, but a sane cleanup would be appreciated.
>>>
>>>
>>> thanks,
>>>
>>> Takashi
>> Something like below?
>>
>> remove all 0x4xxxxxxx from pin quirk table
>> rewrite the pin_config_match()
>> - first, compare all pins in a quirk table with the corresponding pin
>> on the machine, if all pins get a matching,
>> - second, compare all pins on the machine with the corresponding pin
>> in the quirk table
>> if the pin is non-0x4xxxxxxx, it needs match a corresponding
>> pin in the quirk table, otherwise, it return false
>> if the pin is 0x4xxxxxxx, the corresponding pin in the quirk
>> table is 0x4xxxxxxxx as well, it matches; if the corresponding pin in
>> the quirk table is non-0x4xxxxxxx, it return false
>> if the pin is 0x4xxxxxxx, and can't find the corresponding pin
>> in the quirk table, it matches.
> Yes, something like that. But I think the first loop can be
> reduced.
If the first loop is removed, there is some situation the second one
can't handle.
e.g.
a quirk table has:
{0x12, 0x9xxxxxxx},
{0x14, 0x9xxxxxxx},
{0x21, 0x02xxxxxx}
while a machine only has
{0x12, 0x9xxxxxxx},
{0x21, 0x02xxxxxx}
and the NID 0x14 on the machine is a [Vendor Defined Widget] instead of
a [pin complex].
In this situation, only the first loop can detect they are not matching.
Regards,
Hui.
>
> thanks,
>
> Takashi
>
>>
>>
>>
>> diff --git a/sound/pci/hda/hda_auto_parser.c
>> b/sound/pci/hda/hda_auto_parser.c
>> index 03b7399..2639fc3 100644
>> --- a/sound/pci/hda/hda_auto_parser.c
>> +++ b/sound/pci/hda/hda_auto_parser.c
>> @@ -887,11 +887,38 @@ EXPORT_SYMBOL_GPL(snd_hda_apply_fixup);
>> static bool pin_config_match(struct hda_codec *codec,
>> const struct hda_pintbl *pins)
>> {
>> - for (; pins->nid; pins++) {
>> - u32 def_conf = snd_hda_codec_get_pincfg(codec, pins->nid);
>> - if (pins->val != def_conf)
>> + const struct hda_pintbl *t_pins = pins;
>> + int i;
>> +
>> + for (; t_pins->nid; t_pins++) {
>> + u32 def_conf = snd_hda_codec_get_pincfg(codec, t_pins->nid);
>> + if (t_pins->val != def_conf)
>> + return false;
>> + }
>> +
>> + for (i = 0; i < codec->init_pins.used; i++) {
>> + struct hda_pincfg *pin =
>> snd_array_elem(&codec->init_pins, i);
>> + hda_nid_t nid = pin->nid;
>> + u32 cfg = pin->cfg;
>> + int found;
>> +
>> + t_pins = pins;
>> + found = 0;
>> + for (; t_pins->nid; t_pins++) {
>> + if (t_pins->nid == nid) {
>> + found = 1;
>> + if (t_pins->val == cfg)
>> + break;
>> + else if ((cfg & 0xf0000000) ==
>> 0x40000000 && (t_pins->val & 0xf0000000) == 0x40000000)
>> + break;
>> + else
>> + return false;
>> + }
>> + }
>> + if (!found && (cfg & 0xf0000000) != 0x40000000)
>> return false;
>> }
>> +
>> return true;
>> }
>>
>> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
>> index 18ae17e..0fda442 100644
>> --- a/sound/pci/hda/patch_realtek.c
>> +++ b/sound/pci/hda/patch_realtek.c
>> @@ -5386,399 +5386,207 @@ static const struct hda_model_fixup
>> alc269_fixup_models[] = {
>> {}
>> };
>>
>> -#define ALC255_STANDARD_PINS \
>> - {0x18, 0x411111f0}, \
>> - {0x19, 0x411111f0}, \
>> - {0x1a, 0x411111f0}, \
>> - {0x1b, 0x411111f0}, \
>> - {0x1e, 0x411111f0}
>> -
>> #define ALC256_STANDARD_PINS \
>> {0x12, 0x90a60140}, \
>> {0x14, 0x90170110}, \
>> - {0x19, 0x411111f0}, \
>> - {0x1a, 0x411111f0}, \
>> - {0x1b, 0x411111f0}, \
>> {0x21, 0x02211020}
>>
>> #define ALC282_STANDARD_PINS \
>> - {0x14, 0x90170110}, \
>> - {0x18, 0x411111f0}, \
>> - {0x1a, 0x411111f0}, \
>> - {0x1b, 0x411111f0}, \
>> - {0x1e, 0x411111f0}
>> -
>> -#define ALC288_STANDARD_PINS \
>> - {0x17, 0x411111f0}, \
>> - {0x18, 0x411111f0}, \
>> - {0x19, 0x411111f0}, \
>> - {0x1a, 0x411111f0}, \
>> - {0x1e, 0x411111f0}
>> + {0x14, 0x90170110}
>>
>> #define ALC290_STANDARD_PINS \
>> - {0x12, 0x99a30130}, \
>> - {0x13, 0x40000000}, \
>> - {0x16, 0x411111f0}, \
>> - {0x17, 0x411111f0}, \
>> - {0x19, 0x411111f0}, \
>> - {0x1b, 0x411111f0}, \
>> - {0x1e, 0x411111f0}
>> + {0x12, 0x99a30130}
>>
>> #define ALC292_STANDARD_PINS \
>> {0x14, 0x90170110}, \
>> - {0x15, 0x0221401f}, \
>> - {0x1a, 0x411111f0}, \
>> - {0x1b, 0x411111f0}, \
>> - {0x1d, 0x40700001}
>> -
>> -#define ALC298_STANDARD_PINS \
>> - {0x18, 0x411111f0}, \
>> - {0x19, 0x411111f0}, \
>> - {0x1a, 0x411111f0}, \
>> - {0x1e, 0x411111f0}, \
>> - {0x1f, 0x411111f0}
>> + {0x15, 0x0221401f}
>>
>> static const struct snd_hda_pin_quirk alc269_pin_fixup_tbl[] = {
>> SND_HDA_PIN_QUIRK(0x10ec0255, 0x1028, "Dell",
>> ALC255_FIXUP_DELL2_MIC_NO_PRESENCE,
>> - ALC255_STANDARD_PINS,
>> - {0x12, 0x40300000},
>> {0x14, 0x90170110},
>> - {0x17, 0x411111f0},
>> - {0x1d, 0x40538029},
>> {0x21, 0x02211020}),
>> SND_HDA_PIN_QUIRK(0x10ec0255, 0x1028, "Dell",
>> ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
>> - ALC255_STANDARD_PINS,
>> {0x12, 0x90a60140},
>> {0x14, 0x90170110},
>> - {0x17, 0x40000000},
>> - {0x1d, 0x40700001},
>> {0x21, 0x02211020}),
>> SND_HDA_PIN_QUIRK(0x10ec0255, 0x1028, "Dell",
>> ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
>> - ALC255_STANDARD_PINS,
>> {0x12, 0x90a60160},
>> {0x14, 0x90170120},
>> - {0x17, 0x40000000},
>> - {0x1d, 0x40700001},
>> {0x21, 0x02211030}),
>> SND_HDA_PIN_QUIRK(0x10ec0255, 0x1028, "Dell",
>> ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
>> - {0x12, 0x40000000},
>> {0x14, 0x90170130},
>> - {0x17, 0x411111f0},
>> - {0x18, 0x411111f0},
>> - {0x19, 0x411111f0},
>> - {0x1a, 0x411111f0},
>> {0x1b, 0x01014020},
>> - {0x1d, 0x4054c029},
>> - {0x1e, 0x411111f0},
>> {0x21, 0x0221103f}),
>> SND_HDA_PIN_QUIRK(0x10ec0255, 0x1028, "Dell",
>> ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
>> - {0x12, 0x40000000},
>> {0x14, 0x90170150},
>> - {0x17, 0x411111f0},
>> - {0x18, 0x411111f0},
>> - {0x19, 0x411111f0},
>> - {0x1a, 0x411111f0},
>> {0x1b, 0x02011020},
>> - {0x1d, 0x4054c029},
>> - {0x1e, 0x411111f0},
>> {0x21, 0x0221105f}),
>> ..........
>>
>>
>>
>>>> ---
>>>> sound/pci/hda/patch_realtek.c | 15 +++++++++++----
>>>> 1 file changed, 11 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
>>>> index d94c0e3..4ae877c 100644
>>>> --- a/sound/pci/hda/patch_realtek.c
>>>> +++ b/sound/pci/hda/patch_realtek.c
>>>> @@ -5398,8 +5398,6 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
>>>> {0x19, 0x411111f0}, \
>>>> {0x1a, 0x411111f0}, \
>>>> {0x1b, 0x411111f0}, \
>>>> - {0x1d, 0x40700001}, \
>>>> - {0x1e, 0x411111f0}, \
>>>> {0x21, 0x02211020}
>>>>
>>>> #define ALC282_STANDARD_PINS \
>>>> @@ -5556,10 +5554,19 @@ static const struct snd_hda_pin_quirk alc269_pin_fixup_tbl[] = {
>>>> {0x21, 0x02211030}),
>>>> SND_HDA_PIN_QUIRK(0x10ec0256, 0x1028, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
>>>> ALC256_STANDARD_PINS,
>>>> - {0x13, 0x40000000}),
>>>> + {0x13, 0x40000000},
>>>> + {0x1d, 0x40700001},
>>>> + {0x1e, 0x411111f0}),
>>>> SND_HDA_PIN_QUIRK(0x10ec0256, 0x1028, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
>>>> ALC256_STANDARD_PINS,
>>>> - {0x13, 0x411111f0}),
>>>> + {0x13, 0x411111f0},
>>>> + {0x1d, 0x40700001},
>>>> + {0x1e, 0x411111f0}),
>>>> + SND_HDA_PIN_QUIRK(0x10ec0256, 0x1028, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
>>>> + ALC256_STANDARD_PINS,
>>>> + {0x13, 0x411111f0},
>>>> + {0x1d, 0x4077992d},
>>>> + {0x1e, 0x411111ff}),
>>>> SND_HDA_PIN_QUIRK(0x10ec0280, 0x103c, "HP", ALC280_FIXUP_HP_GPIO4,
>>>> {0x12, 0x90a60130},
>>>> {0x13, 0x40000000},
>>>> --
>>>> 2.1.4
>>>>
>>> _______________________________________________
>>> Alsa-devel mailing list
>>> Alsa-devel@alsa-project.org
>>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
next prev parent reply other threads:[~2015-07-31 1:54 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-27 10:34 [alsa-devel][PATCH] ALSA: hda - Add pin quirk for the headset mic jack detection on Dell laptop woodrow.shen
2015-07-27 10:52 ` Takashi Iwai
2015-07-30 10:11 ` [alsa-devel] [PATCH] " Hui Wang
2015-07-30 13:57 ` Takashi Iwai
2015-07-31 1:54 ` Hui Wang [this message]
2015-08-02 7:17 ` Takashi Iwai
2015-08-03 1:11 ` Hui Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55BAD56F.2060309@canonical.com \
--to=hui.wang@canonical.com \
--cc=1478497@bugs.launchpad.net \
--cc=alsa-devel@alsa-project.org \
--cc=david.henningsson@canonical.com \
--cc=stable@vger.kernel.org \
--cc=tiwai@suse.de \
--cc=woodrow.shen@canonical.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.