[alsa-devel] [PATCH v2 1/2] ALSA: hda - Add a new match function for only undef configurations

[Takashi Iwai]

On Thu, 15 Aug 2019 12:14:14 +0200,
Hui Wang wrote:
> 
> With the existing pintbl, we already have many entries in it. it is
> better to figure out a new match to reduce the size of the pintbl.
> 
> For example, there are over 10 entries in the pintbl for:
> 0x10ec0255, 0x1028, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE
> 
> If we define a new tbl like below, and with the new adding match
> function, we can remove those over 10 entries:
> SND_HDA_PIN_QUIRK(0x10ec0255, 0x1028, "Dell", ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
> 	{0x19, 0x40000000},
> 	{0x1a, 0x40000000},),
> 
> Here we put 0x19 and 0x1a in the tbl just because these two pins are
> undefined on Dell laptops with the codec alc255, and these two pins
> will be overwritten by ALC255_FIXUP_DELL1_MIC_NO_PRESENCE.
> 
> In summary: the new match will check vendor id and codec id first,
> then check the pin_cfg defined in the tbl, only all pin_cfgs in the
> tbl are undef and the corresponding pin_cfgs on the laptop are undef
> too, this match returns true.
> 
> This new match function has lower priority than existing match
> functions, so the existing tbls still work as before after applying this
> patch.
> 
> And for the same vendor and same codec, we could define at most one
> undef tbl for it, for example for the Dell laptops with alc255 codec,
> some of them need to apply DELL1_MIC, the rest need to apply DELL2_MIC,
> here we could only define undef tbl for DELL1_MIC or DELL2_MIC, we could
> not define undef tbls for both of them.
> 
> My plan is to change the existing tbl to undef tbl for MIC_NO_PRESENCE
> fixups gradually.
> 
> Signed-off-by: Hui Wang <hui.wang at canonical.com>

The idea sounds good, but I believe it'd be less confusing if we split
to two tables, one is for positive matching and another for negative
matching.

And the existing fixup function can be reused for the negative
matching, too, something like below.


thanks,

Takashi

--- a/sound/pci/hda/hda_auto_parser.c
+++ b/sound/pci/hda/hda_auto_parser.c
@@ -884,7 +884,8 @@ EXPORT_SYMBOL_GPL(snd_hda_apply_fixup);
 #define IGNORE_SEQ_ASSOC (~(AC_DEFCFG_SEQUENCE | AC_DEFCFG_DEF_ASSOC))
 
 static bool pin_config_match(struct hda_codec *codec,
-			     const struct hda_pintbl *pins)
+			     const struct hda_pintbl *pins,
+			     bool match_all_pins)
 {
 	const struct hda_pincfg *pin;
 	int i;
@@ -908,7 +909,8 @@ static bool pin_config_match(struct hda_codec *codec,
 					return false;
 			}
 		}
-		if (!found && (cfg & 0xf0000000) != 0x40000000)
+		if (match_all_pins &&
+		    !found && (cfg & 0xf0000000) != 0x40000000)
 			return false;
 	}
 
@@ -920,10 +922,12 @@ static bool pin_config_match(struct hda_codec *codec,
  * @codec: the HDA codec
  * @pin_quirk: zero-terminated pin quirk list
  * @fixlist: the fixup list
+ * @match_all_pins: all valid pins must match with the table entries
  */
 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 hda_fixup *fixlist,
+			    bool match_all_pins)
 {
 	const struct snd_hda_pin_quirk *pq;
 
@@ -935,7 +939,7 @@ void snd_hda_pick_pin_fixup(struct hda_codec *codec,
 			continue;
 		if (codec->core.vendor_id != pq->codec)
 			continue;
-		if (pin_config_match(codec, pq->pins)) {
+		if (pin_config_match(codec, pq->pins, match_all_pins)) {
 			codec->fixup_id = pq->value;
 #ifdef CONFIG_SND_DEBUG_VERBOSE
 			codec->fixup_name = pq->name;
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -361,7 +361,8 @@ void snd_hda_pick_fixup(struct hda_codec *codec,
 			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);
+			    const struct hda_fixup *fixlist,
+			    bool match_all_pins);
 
 /* helper macros to retrieve pin default-config values */
 #define get_defcfg_connect(cfg) \
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -7950,7 +7950,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_pick_pin_fixup(codec, alc269_pin_fixup_tbl, alc269_fixups, true);
 	snd_hda_pick_fixup(codec, NULL,	alc269_fixup_vendor_tbl,
 			   alc269_fixups);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
@@ -8935,7 +8935,7 @@ static int patch_alc662(struct hda_codec *codec)
 
 	snd_hda_pick_fixup(codec, alc662_fixup_models,
 		       alc662_fixup_tbl, alc662_fixups);
-	snd_hda_pick_pin_fixup(codec, alc662_pin_fixup_tbl, alc662_fixups);
+	snd_hda_pick_pin_fixup(codec, alc662_pin_fixup_tbl, alc662_fixups, true);
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
 
 	alc_auto_parse_customize_define(codec);