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

[Takashi Iwai <tiwai@suse.de>]

At Tue, 27 May 2014 08:47:33 +0200,
David Henningsson wrote:
> 
> 
> 
> 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?

I'm for the safer side, too.


Takashi

> 
> >
> >
> > 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
>