All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] ALSA: hda - Add fixup_forced flag
@ 2014-05-26  8:22 Hui Wang
  2014-05-26  8:22 ` [PATCH 2/5] ALSA: hda - Add a new quirk match based on default pin configuration Hui Wang
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Hui Wang @ 2014-05-26  8:22 UTC (permalink / raw)
  To: tiwai, alsa-devel; +Cc: hui.wang, david.henningsson

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/5] ALSA: hda - Add a new quirk match based on default pin configuration
  2014-05-26  8:22 [PATCH 1/5] ALSA: hda - Add fixup_forced flag Hui Wang
@ 2014-05-26  8:22 ` Hui Wang
  2014-05-26  8:22 ` [PATCH 3/5] ALSA: hda - get subvendor from codec rather than pci_dev Hui Wang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Hui Wang @ 2014-05-26  8:22 UTC (permalink / raw)
  To: tiwai, alsa-devel; +Cc: hui.wang, david.henningsson

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/5] ALSA: hda - get subvendor from codec rather than pci_dev
  2014-05-26  8:22 [PATCH 1/5] ALSA: hda - Add fixup_forced flag Hui Wang
  2014-05-26  8:22 ` [PATCH 2/5] ALSA: hda - Add a new quirk match based on default pin configuration Hui Wang
@ 2014-05-26  8:22 ` Hui Wang
  2014-05-26  8:22 ` [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing Hui Wang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Hui Wang @ 2014-05-26  8:22 UTC (permalink / raw)
  To: tiwai, alsa-devel; +Cc: hui.wang, david.henningsson

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing
  2014-05-26  8:22 [PATCH 1/5] ALSA: hda - Add fixup_forced flag Hui Wang
  2014-05-26  8:22 ` [PATCH 2/5] ALSA: hda - Add a new quirk match based on default pin configuration Hui Wang
  2014-05-26  8:22 ` [PATCH 3/5] ALSA: hda - get subvendor from codec rather than pci_dev Hui Wang
@ 2014-05-26  8:22 ` Hui Wang
  2014-05-26 10:11   ` David Henningsson
  2014-05-26  8:22 ` [PATCH 5/5] ALSA: hda - add an instance to use snd_hda_pick_pin_fixup Hui Wang
  2014-05-26  9:10 ` [PATCH 1/5] ALSA: hda - Add fixup_forced flag Takashi Iwai
  4 siblings, 1 reply; 16+ messages in thread
From: Hui Wang @ 2014-05-26  8:22 UTC (permalink / raw)
  To: tiwai, alsa-devel; +Cc: hui.wang, david.henningsson

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/5] ALSA: hda - add an instance to use snd_hda_pick_pin_fixup
  2014-05-26  8:22 [PATCH 1/5] ALSA: hda - Add fixup_forced flag Hui Wang
                   ` (2 preceding siblings ...)
  2014-05-26  8:22 ` [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing Hui Wang
@ 2014-05-26  8:22 ` Hui Wang
  2014-05-26 10:14   ` David Henningsson
  2014-05-26  9:10 ` [PATCH 1/5] ALSA: hda - Add fixup_forced flag Takashi Iwai
  4 siblings, 1 reply; 16+ messages in thread
From: Hui Wang @ 2014-05-26  8:22 UTC (permalink / raw)
  To: tiwai, alsa-devel; +Cc: hui.wang, david.henningsson

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);
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] ALSA: hda - Add fixup_forced flag
  2014-05-26  8:22 [PATCH 1/5] ALSA: hda - Add fixup_forced flag Hui Wang
                   ` (3 preceding siblings ...)
  2014-05-26  8:22 ` [PATCH 5/5] ALSA: hda - add an instance to use snd_hda_pick_pin_fixup Hui Wang
@ 2014-05-26  9:10 ` Takashi Iwai
  4 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2014-05-26  9:10 UTC (permalink / raw)
  To: Hui Wang; +Cc: alsa-devel, david.henningsson

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
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing
  2014-05-26  8:22 ` [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing Hui Wang
@ 2014-05-26 10:11   ` David Henningsson
  2014-05-27  2:25     ` Alex Hung
  0 siblings, 1 reply; 16+ messages in thread
From: David Henningsson @ 2014-05-26 10:11 UTC (permalink / raw)
  To: Hui Wang, tiwai, alsa-devel, alex.hung

(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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] ALSA: hda - add an instance to use snd_hda_pick_pin_fixup
  2014-05-26  8:22 ` [PATCH 5/5] ALSA: hda - add an instance to use snd_hda_pick_pin_fixup Hui Wang
@ 2014-05-26 10:14   ` David Henningsson
  2014-05-27  0:31     ` Hui Wang
  0 siblings, 1 reply; 16+ messages in thread
From: David Henningsson @ 2014-05-26 10:14 UTC (permalink / raw)
  To: Hui Wang, tiwai, alsa-devel



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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] ALSA: hda - add an instance to use snd_hda_pick_pin_fixup
  2014-05-26 10:14   ` David Henningsson
@ 2014-05-27  0:31     ` Hui Wang
  0 siblings, 0 replies; 16+ messages in thread
From: Hui Wang @ 2014-05-27  0:31 UTC (permalink / raw)
  To: David Henningsson; +Cc: tiwai, alsa-devel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing
  2014-05-26 10:11   ` David Henningsson
@ 2014-05-27  2:25     ` Alex Hung
  2014-05-27  4:41       ` David Henningsson
  0 siblings, 1 reply; 16+ messages in thread
From: Alex Hung @ 2014-05-27  2:25 UTC (permalink / raw)
  To: David Henningsson; +Cc: Hui Wang, Takashi Iwai, alsa-devel

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



-- 
Cheers,
Alex Hung

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing
  2014-05-27  2:25     ` Alex Hung
@ 2014-05-27  4:41       ` David Henningsson
  2014-05-27  6:40         ` Hui Wang
  0 siblings, 1 reply; 16+ messages in thread
From: David Henningsson @ 2014-05-27  4:41 UTC (permalink / raw)
  To: Alex Hung; +Cc: Hui Wang, Takashi Iwai, alsa-devel



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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing
  2014-05-27  4:41       ` David Henningsson
@ 2014-05-27  6:40         ` Hui Wang
  2014-05-27  6:47           ` David Henningsson
  0 siblings, 1 reply; 16+ messages in thread
From: Hui Wang @ 2014-05-27  6:40 UTC (permalink / raw)
  To: David Henningsson; +Cc: Takashi Iwai, Alex Hung, alsa-devel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing
  2014-05-27  6:40         ` Hui Wang
@ 2014-05-27  6:47           ` David Henningsson
  2014-05-27  6:53             ` Takashi Iwai
  0 siblings, 1 reply; 16+ messages in thread
From: David Henningsson @ 2014-05-27  6:47 UTC (permalink / raw)
  To: Hui Wang; +Cc: Takashi Iwai, Alex Hung, alsa-devel



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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing
  2014-05-27  6:47           ` David Henningsson
@ 2014-05-27  6:53             ` Takashi Iwai
  2014-05-27  6:55               ` Hui Wang
  2014-05-27  9:24               ` Hui Wang
  0 siblings, 2 replies; 16+ messages in thread
From: Takashi Iwai @ 2014-05-27  6:53 UTC (permalink / raw)
  To: David Henningsson; +Cc: Hui Wang, Alex Hung, alsa-devel

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
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing
  2014-05-27  6:53             ` Takashi Iwai
@ 2014-05-27  6:55               ` Hui Wang
  2014-05-27  9:24               ` Hui Wang
  1 sibling, 0 replies; 16+ messages in thread
From: Hui Wang @ 2014-05-27  6:55 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Alex Hung, alsa-devel, David Henningsson

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing
  2014-05-27  6:53             ` Takashi Iwai
  2014-05-27  6:55               ` Hui Wang
@ 2014-05-27  9:24               ` Hui Wang
  1 sibling, 0 replies; 16+ messages in thread
From: Hui Wang @ 2014-05-27  9:24 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Alex Hung, alsa-devel, David Henningsson

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-05-27  9:24 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-26  8:22 [PATCH 1/5] ALSA: hda - Add fixup_forced flag Hui Wang
2014-05-26  8:22 ` [PATCH 2/5] ALSA: hda - Add a new quirk match based on default pin configuration Hui Wang
2014-05-26  8:22 ` [PATCH 3/5] ALSA: hda - get subvendor from codec rather than pci_dev Hui Wang
2014-05-26  8:22 ` [PATCH 4/5] ALSA: hda - drop def association and sequence from pinconf comparing Hui Wang
2014-05-26 10:11   ` David Henningsson
2014-05-27  2:25     ` Alex Hung
2014-05-27  4:41       ` David Henningsson
2014-05-27  6:40         ` Hui Wang
2014-05-27  6:47           ` David Henningsson
2014-05-27  6:53             ` Takashi Iwai
2014-05-27  6:55               ` Hui Wang
2014-05-27  9:24               ` Hui Wang
2014-05-26  8:22 ` [PATCH 5/5] ALSA: hda - add an instance to use snd_hda_pick_pin_fixup Hui Wang
2014-05-26 10:14   ` David Henningsson
2014-05-27  0:31     ` Hui Wang
2014-05-26  9:10 ` [PATCH 1/5] ALSA: hda - Add fixup_forced flag Takashi Iwai

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.