[alsa-devel] [PATCH 1/5] ALSA: hda - Add fixup_forced flag
From: David Henningsson david.henningsson@canonical.com
The "fixup_forced" flag will indicate whether a specific fixup (or nofixup) has been set by the user, to override the driver's default. This flag will help future patches.
Signed-off-by: David Henningsson david.henningsson@canonical.com --- sound/pci/hda/hda_auto_parser.c | 9 ++++++--- sound/pci/hda/hda_codec.h | 1 + 2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c index 90d2fda..36961ab 100644 --- a/sound/pci/hda/hda_auto_parser.c +++ b/sound/pci/hda/hda_auto_parser.c @@ -852,15 +852,17 @@ void snd_hda_pick_fixup(struct hda_codec *codec, if (codec->modelname && !strcmp(codec->modelname, "nofixup")) { codec->fixup_list = NULL; codec->fixup_id = -1; + codec->fixup_forced = 1; return; }
if (codec->modelname && models) { while (models->name) { if (!strcmp(codec->modelname, models->name)) { - id = models->id; - name = models->name; - break; + codec->fixup_id = models->id; + codec->fixup_name = models->name; + codec->fixup_forced = 1; + return; } models++; } @@ -889,6 +891,7 @@ void snd_hda_pick_fixup(struct hda_codec *codec, } }
+ codec->fixup_forced = 0; codec->fixup_id = id; if (id >= 0) { codec->fixup_list = fixlist; diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index a423313..5825aa1 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -402,6 +402,7 @@ struct hda_codec {
/* fix-up list */ int fixup_id; + unsigned int fixup_forced:1; /* fixup explicitly set by user */ const struct hda_fixup *fixup_list; const char *fixup_name;
From: David Henningsson david.henningsson@canonical.com
Normally, we match on pci ssid only. This works but needs new code for every machine. To catch more machines in the same quirk, let's add a new type of quirk, where we match on 1) PCI Subvendor ID (i e, not device, just vendor) 2) Codec ID 3) Pin configuration default
If all these three match, we could be reasonably certain that the quirk should apply to the machine even though it might not be the exact same device.
Signed-off-by: David Henningsson david.henningsson@canonical.com --- sound/pci/hda/hda_auto_parser.c | 37 +++++++++++++++++++++++++++++++++++++ sound/pci/hda/hda_local.h | 14 ++++++++++++++ 2 files changed, 51 insertions(+)
diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c index 36961ab..a142753 100644 --- a/sound/pci/hda/hda_auto_parser.c +++ b/sound/pci/hda/hda_auto_parser.c @@ -839,6 +839,43 @@ void snd_hda_apply_fixup(struct hda_codec *codec, int action) } 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) + return false; + } + return true; +} + +void snd_hda_pick_pin_fixup(struct hda_codec *codec, + const struct snd_hda_pin_quirk *pin_quirk, + const struct hda_fixup *fixlist) +{ + const struct snd_hda_pin_quirk *pq; + + if (codec->fixup_forced) + return; + + for (pq = pin_quirk; pq->subvendor; pq++) { + if (codec->bus->pci->subsystem_vendor != pq->subvendor) + continue; + if (codec->vendor_id != pq->codec) + continue; + if (pin_config_match(codec, pq->pins)) { + codec->fixup_id = pq->value; +#ifdef CONFIG_SND_DEBUG_VERBOSE + codec->fixup_name = pq->name; +#endif + codec->fixup_list = fixlist; + return; + } + } +} +EXPORT_SYMBOL_GPL(snd_hda_pick_pin_fixup); + void snd_hda_pick_fixup(struct hda_codec *codec, const struct hda_model_fixup *models, const struct snd_pci_quirk *quirk, diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index e51d155..ebd1fa6 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -407,6 +407,16 @@ struct hda_fixup { } v; };
+struct snd_hda_pin_quirk { + unsigned int codec; /* Codec vendor/device ID */ + unsigned short subvendor; /* PCI subvendor ID */ + const struct hda_pintbl *pins; /* list of matching pins */ +#ifdef CONFIG_SND_DEBUG_VERBOSE + const char *name; +#endif + int value; /* quirk value */ +}; + /* fixup types */ enum { HDA_FIXUP_INVALID, @@ -434,6 +444,10 @@ void snd_hda_pick_fixup(struct hda_codec *codec, const struct hda_model_fixup *models, const struct snd_pci_quirk *quirk, const struct hda_fixup *fixlist); +void snd_hda_pick_pin_fixup(struct hda_codec *codec, + const struct snd_hda_pin_quirk *pin_quirk, + const struct hda_fixup *fixlist); +
/* * unsolicited event handler
It is safer for non-pci situation.
Signed-off-by: Hui Wang hui.wang@canonical.com --- sound/pci/hda/hda_auto_parser.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c index a142753..b684c6e 100644 --- a/sound/pci/hda/hda_auto_parser.c +++ b/sound/pci/hda/hda_auto_parser.c @@ -860,7 +860,7 @@ void snd_hda_pick_pin_fixup(struct hda_codec *codec, return;
for (pq = pin_quirk; pq->subvendor; pq++) { - if (codec->bus->pci->subsystem_vendor != pq->subvendor) + if ((codec->subsystem_id & 0xffff0000) != (pq->subvendor << 16)) continue; if (codec->vendor_id != pq->codec) continue;
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.
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;
(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;
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.
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
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.
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
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.
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
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
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
On 05/27/2014 02:53 PM, Takashi Iwai wrote:
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
OK, agree to revert it.
Regards, Hui.
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
On 05/27/2014 02:53 PM, Takashi Iwai wrote:
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
I will write a reverting patch within this week, and send it out accompany with the patch moving the some existing machines from old quirk matching table to new quirk matching table.
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
Just two members in the alc269_pin_fixup_tbl[] can cover more than 10 Dell laptop models.
Signed-off-by: Hui Wang hui.wang@canonical.com --- sound/pci/hda/patch_realtek.c | 47 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index c0b16de..992949c 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -4725,8 +4725,6 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x1028, 0x061f, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x0629, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x062c, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE), - SND_PCI_QUIRK(0x1028, 0x062e, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE), - SND_PCI_QUIRK(0x1028, 0x0632, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x0638, "Dell Inspiron 5439", ALC290_FIXUP_MONO_SPEAKERS_HSJACK), SND_PCI_QUIRK(0x1028, 0x063e, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x063f, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE), @@ -4924,6 +4922,50 @@ static const struct hda_model_fixup alc269_fixup_models[] = { {} };
+static const struct snd_hda_pin_quirk alc269_pin_fixup_tbl[] = { + { + .codec = 0x10ec0293, + .subvendor = 0x1028, +#ifdef CONFIG_SND_DEBUG_VERBOSE + .name = "Dell", +#endif + .pins = (const struct hda_pintbl[]) { + {0x12, 0x40000000}, + {0x13, 0x90a60140}, + {0x14, 0x90170110}, + {0x15, 0x0221401f}, + {0x16, 0x21014020}, + {0x18, 0x411111f0}, + {0x19, 0x21a19030}, + {0x1a, 0x411111f0}, + {0x1b, 0x411111f0}, + {0x1d, 0x40700001}, + {0x1e, 0x411111f0}, + }, + .value = ALC269_FIXUP_DELL1_MIC_NO_PRESENCE, + }, + { + .codec = 0x10ec0255, + .subvendor = 0x1028, +#ifdef CONFIG_SND_DEBUG_VERBOSE + .name = "Dell", +#endif + .pins = (const struct hda_pintbl[]) { + {0x12, 0x90a60140}, + {0x14, 0x90170110}, + {0x17, 0x40000000}, + {0x18, 0x411111f0}, + {0x19, 0x411111f0}, + {0x1a, 0x411111f0}, + {0x1b, 0x411111f0}, + {0x1d, 0x40700001}, + {0x1e, 0x411111f0}, + {0x21, 0x02211020}, + }, + .value = ALC255_FIXUP_DELL1_MIC_NO_PRESENCE, + }, + {} +};
static void alc269_fill_coef(struct hda_codec *codec) { @@ -4985,6 +5027,7 @@ static int patch_alc269(struct hda_codec *codec)
snd_hda_pick_fixup(codec, alc269_fixup_models, alc269_fixup_tbl, alc269_fixups); + snd_hda_pick_pin_fixup(codec, alc269_pin_fixup_tbl, alc269_fixups); snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
alc_auto_parse_customize_define(codec);
On 2014-05-26 10:22, Hui Wang wrote:
Just two members in the alc269_pin_fixup_tbl[] can cover more than 10 Dell laptop models.
Shouldn't then 10 lines be removed from alc269_fixup_tbl? Right now only two lines are removed.
Signed-off-by: Hui Wang hui.wang@canonical.com
sound/pci/hda/patch_realtek.c | 47 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index c0b16de..992949c 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -4725,8 +4725,6 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x1028, 0x061f, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x0629, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x062c, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE),
- SND_PCI_QUIRK(0x1028, 0x062e, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE),
- SND_PCI_QUIRK(0x1028, 0x0632, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x0638, "Dell Inspiron 5439", ALC290_FIXUP_MONO_SPEAKERS_HSJACK), SND_PCI_QUIRK(0x1028, 0x063e, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x063f, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE),
@@ -4924,6 +4922,50 @@ static const struct hda_model_fixup alc269_fixup_models[] = { {} };
+static const struct snd_hda_pin_quirk alc269_pin_fixup_tbl[] = {
- {
.codec = 0x10ec0293,.subvendor = 0x1028,+#ifdef CONFIG_SND_DEBUG_VERBOSE
.name = "Dell",+#endif
.pins = (const struct hda_pintbl[]) {{0x12, 0x40000000},{0x13, 0x90a60140},{0x14, 0x90170110},{0x15, 0x0221401f},{0x16, 0x21014020},{0x18, 0x411111f0},{0x19, 0x21a19030},{0x1a, 0x411111f0},{0x1b, 0x411111f0},{0x1d, 0x40700001},{0x1e, 0x411111f0},},.value = ALC269_FIXUP_DELL1_MIC_NO_PRESENCE,- },
- {
.codec = 0x10ec0255,.subvendor = 0x1028,+#ifdef CONFIG_SND_DEBUG_VERBOSE
.name = "Dell",+#endif
.pins = (const struct hda_pintbl[]) {{0x12, 0x90a60140},{0x14, 0x90170110},{0x17, 0x40000000},{0x18, 0x411111f0},{0x19, 0x411111f0},{0x1a, 0x411111f0},{0x1b, 0x411111f0},{0x1d, 0x40700001},{0x1e, 0x411111f0},{0x21, 0x02211020},},.value = ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,- },
- {}
+};
static void alc269_fill_coef(struct hda_codec *codec) { @@ -4985,6 +5027,7 @@ static int patch_alc269(struct hda_codec *codec)
snd_hda_pick_fixup(codec, alc269_fixup_models, alc269_fixup_tbl, alc269_fixups);
snd_hda_pick_pin_fixup(codec, alc269_pin_fixup_tbl, alc269_fixups); snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
alc_auto_parse_customize_define(codec);
On 05/26/2014 06:14 PM, David Henningsson wrote:
On 2014-05-26 10:22, Hui Wang wrote:
Just two members in the alc269_pin_fixup_tbl[] can cover more than 10 Dell laptop models.
Shouldn't then 10 lines be removed from alc269_fixup_tbl? Right now only two lines are removed.
I mean the 10 new Dell laptops which have not been in the Linux kernel yet.
Please see a new bug #1321179, there are 7 laptops in this bug, In the past, we need to add 7 lines to fix them, but now all of them can be fixed by this patch.
Removing the existing lines is the next step plan.
Signed-off-by: Hui Wang hui.wang@canonical.com
sound/pci/hda/patch_realtek.c | 47 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index c0b16de..992949c 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -4725,8 +4725,6 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x1028, 0x061f, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x0629, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x062c, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE),
- SND_PCI_QUIRK(0x1028, 0x062e, "Dell",
ALC269_FIXUP_DELL1_MIC_NO_PRESENCE),
- SND_PCI_QUIRK(0x1028, 0x0632, "Dell",
ALC255_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x0638, "Dell Inspiron 5439", ALC290_FIXUP_MONO_SPEAKERS_HSJACK), SND_PCI_QUIRK(0x1028, 0x063e, "Dell", ALC269_FIXUP_DELL1_MIC_NO_PRESENCE), SND_PCI_QUIRK(0x1028, 0x063f, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE), @@ -4924,6 +4922,50 @@ static const struct hda_model_fixup alc269_fixup_models[] = { {} };
+static const struct snd_hda_pin_quirk alc269_pin_fixup_tbl[] = {
- {
.codec = 0x10ec0293,.subvendor = 0x1028,+#ifdef CONFIG_SND_DEBUG_VERBOSE
.name = "Dell",+#endif
.pins = (const struct hda_pintbl[]) {{0x12, 0x40000000},{0x13, 0x90a60140},{0x14, 0x90170110},{0x15, 0x0221401f},{0x16, 0x21014020},{0x18, 0x411111f0},{0x19, 0x21a19030},{0x1a, 0x411111f0},{0x1b, 0x411111f0},{0x1d, 0x40700001},{0x1e, 0x411111f0},},.value = ALC269_FIXUP_DELL1_MIC_NO_PRESENCE,- },
- {
.codec = 0x10ec0255,.subvendor = 0x1028,+#ifdef CONFIG_SND_DEBUG_VERBOSE
.name = "Dell",+#endif
.pins = (const struct hda_pintbl[]) {{0x12, 0x90a60140},{0x14, 0x90170110},{0x17, 0x40000000},{0x18, 0x411111f0},{0x19, 0x411111f0},{0x1a, 0x411111f0},{0x1b, 0x411111f0},{0x1d, 0x40700001},{0x1e, 0x411111f0},{0x21, 0x02211020},},.value = ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,- },
- {}
+};
static void alc269_fill_coef(struct hda_codec *codec) { @@ -4985,6 +5027,7 @@ static int patch_alc269(struct hda_codec *codec)
snd_hda_pick_fixup(codec, alc269_fixup_models, alc269_fixup_tbl, alc269_fixups);
snd_hda_pick_pin_fixup(codec, alc269_pin_fixup_tbl, alc269_fixups); snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
alc_auto_parse_customize_define(codec);
At Mon, 26 May 2014 16:22:40 +0800, Hui Wang wrote:
From: David Henningsson david.henningsson@canonical.com
The "fixup_forced" flag will indicate whether a specific fixup (or nofixup) has been set by the user, to override the driver's default. This flag will help future patches.
Signed-off-by: David Henningsson david.henningsson@canonical.com
In general, when you submit a patch, *you* need to sign off the patch in addition. It's no big problem in this case since I already got the same patches from David, though.
In anyway, I applied all patches now. Thanks.
Takashi
sound/pci/hda/hda_auto_parser.c | 9 ++++++--- sound/pci/hda/hda_codec.h | 1 + 2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c index 90d2fda..36961ab 100644 --- a/sound/pci/hda/hda_auto_parser.c +++ b/sound/pci/hda/hda_auto_parser.c @@ -852,15 +852,17 @@ void snd_hda_pick_fixup(struct hda_codec *codec, if (codec->modelname && !strcmp(codec->modelname, "nofixup")) { codec->fixup_list = NULL; codec->fixup_id = -1;
codec->fixup_forced = 1;return; }
if (codec->modelname && models) { while (models->name) { if (!strcmp(codec->modelname, models->name)) {
id = models->id;name = models->name;break;
codec->fixup_id = models->id;codec->fixup_name = models->name;codec->fixup_forced = 1;return; } models++;@@ -889,6 +891,7 @@ void snd_hda_pick_fixup(struct hda_codec *codec, } }
- codec->fixup_forced = 0; codec->fixup_id = id; if (id >= 0) { codec->fixup_list = fixlist;
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index a423313..5825aa1 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -402,6 +402,7 @@ struct hda_codec {
/* fix-up list */ int fixup_id;
- unsigned int fixup_forced:1; /* fixup explicitly set by user */ const struct hda_fixup *fixup_list; const char *fixup_name;
-- 1.8.1.2
participants (4)
-
Alex Hung -
David Henningsson -
Hui Wang -
Takashi Iwai