Re: [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing

[David Henningsson <david.henningsson@canonical.com>]

On 2014-05-27 08:40, Hui Wang wrote:
> On 05/27/2014 12:41 PM, David Henningsson wrote:
>>
>>
>> On 2014-05-27 04:25, Alex Hung wrote:
>>> Hi David,
>>>
>>> BIOS today implements verbtable which is provided by codec vendor
>>> based on hardware design, and it is indeed not uncommon that the
>>> verbtable includes used pin only and leaves unused pins untouched.
>>
>> Sure, but those unused pins would then have the same default value
>> that the codec initializes it with.
>>
>> Also, it wouldn't be uncommon for BIOS (or codec vendors) to use the
>> same verbtable for several machines if they share the same audio
>> hardware.
>>
>> But none of this explains why anyone would just change def association
>> and sequence value between machines? It makes no sense.
>
> So far, I met one example for this case:
>
> the Dell laptops with the same 0x10ec0255 codec,
>
> On some machines:
> pin 0x12, 0x90a60170
> pin 0x14, 0x90170120
> pin 0x21, 0x02211030
>
> On another machine:
> pin 0x12, 0x90a60140
> pin 0x14, 0x90170110
> pin 0x21, 0x02211020
>
>
> The def config of the rest pins are same.

If there is only one example (and only two different options), I think 
we should revert this patch and use two different pin-matching quirks 
instead.

After all, ignoring the def assoc/sequence values also means a greater 
risk of catching unwanted machines. Better err on the more careful side.

This is IMO, what do others think?

>
>
> Regards,
> Hui.
>>
>>>
>>> On Mon, May 26, 2014 at 6:11 PM, David Henningsson
>>> <david.henningsson@canonical.com> wrote:
>>>> (Add Alex Hung to CC)
>>>>
>>>> On 2014-05-26 10:22, Hui Wang wrote:
>>>>>
>>>>> A lot a machine have the same codec, but they have different default
>>>>> pinconf setting just because the def association and sequence is
>>>>> different, as a result they can't share a hda_pintbl[], to overcome
>>>>> it, we don't compare def association and sequence in the pinconf
>>>>> matching.
>>>>
>>>>
>>>> Uhm, really? Alex, does this seem reasonable from a BIOS
>>>> perspective, i e,
>>>> that BIOS people normally would set def association and sequence
>>>> different
>>>> while leaving everything else unchanged?
>>>>
>>>>>
>>>>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
>>>>> ---
>>>>>    sound/pci/hda/hda_auto_parser.c | 3 ++-
>>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/sound/pci/hda/hda_auto_parser.c
>>>>> b/sound/pci/hda/hda_auto_parser.c
>>>>> index b684c6e..3cf9137 100644
>>>>> --- a/sound/pci/hda/hda_auto_parser.c
>>>>> +++ b/sound/pci/hda/hda_auto_parser.c
>>>>> @@ -844,7 +844,8 @@ static bool pin_config_match(struct hda_codec
>>>>> *codec,
>>>>>    {
>>>>>          for (; pins->nid; pins++) {
>>>>>                  u32 def_conf = snd_hda_codec_get_pincfg(codec,
>>>>> pins->nid);
>>>>> -               if (pins->val != def_conf)
>>>>> +               u32 mask = 0xffffff00;
>>>>> +               if ((pins->val & mask) != (def_conf & mask))
>>>>>                          return false;
>>>>>          }
>>>>>          return true;
>>>>>
>>>>
>>>> --
>>>> David Henningsson, Canonical Ltd.
>>>> https://launchpad.net/~diwic
>>>
>>>
>>>
>>
>

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