diff mbox series

[v3,2/2] ALSA: hda/tas2781: Add tas2781 HDA driver

Message ID 20230818085558.1431-2-shenghao-ding@ti.com (mailing list archive)
State Superseded
Headers show
Series [v3,1/2] ALSA: hda/tas2781: Add tas2781 HDA driver | expand

Commit Message

Shenghao Ding Aug. 18, 2023, 8:55 a.m. UTC
Integrate tas2781 configs for Lenovo Laptops. All of the tas2781s in the
laptop will be aggregated as one audio device. The code support realtek
as the primary codec. Rename "struct cs35l41_dev_name" to
"struct scodec_dev_name" for all other side codecs instead of the certain
one.

Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>

---
Changes in v3:
 - remove workaround code for 0x17aa38be, laptop vendor will fix it in acpi.
 - rename comp_match_tas2781_dev_name to avoid indentation
 - simplify the check of vendor id with Lenovo
 - ThinkPad is one of Lenovo's brands, I was suggested me to use
   ALC269_FIXUP_THINKPAD_ACPI.
 - Add comments on ACARD_SINGLE_RANGE_EXT_TLV
 - Add the range check for tas_priv->tasdevice[] in tas2781_acpi_get_i2c_res.
 - remove acpi_subsystem_id
 - Issue in Laptop 0x17aa38be ACPI talbe caused codec->bus->pci->subsystem_device
   is not equal to (codec->core.subsystem_id & 0xffff) in snd_hda_pick_fixup.
   The former is 0x3802 and the latter is 0x38be leads to getting the wrong
   fixup_id and enter into the wrong entry. Although, this issue has been raised
   to the laptop manufacturer, but the ACPI table is locked, cannot be changed
   any more. Correct the wrong entry in the code.
 - Rename "struct cs35l41_dev_name" to "struct scodec_dev_name" for all
   other side codecs instead of one certain codec.
 - Ignore the checkpatch complaints in alc269_fixup_tbl
 - Drop the hunk which is irrelevant with my code in
   alc_fixup_headset_mode_alc255_no_hp_mic
 - Add tiwai@suse.de into Cc list
 - remove useless index
 - combine ALC287_FIXUP_TAS2781_I2C_2 and ALC287_FIXUP_TAS2781_I2C_4 together as
   ALC287_FIXUP_TAS2781_I2C, The code view all the tas2781s in the laptop as one instance.
 - delete the white space at the end of the line in alc_fixup_headset_mode_alc255_no_hp_mic
---
 sound/pci/hda/patch_realtek.c | 88 +++++++++++++++++++++++++++++++++--
 1 file changed, 85 insertions(+), 3 deletions(-)

Comments

Pierre-Louis Bossart Aug. 18, 2023, 4 p.m. UTC | #1
The first patch in this series has the same commit title as the second
one, can this be updated with a more meaningful description of the two
patches?


> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 44fccfb93cff..ba1b02ed184a 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -6721,7 +6721,7 @@ static void comp_generic_playback_hook(struct hda_pcm_stream *hinfo, struct hda_
>  	}
>  }
>  
> -struct cs35l41_dev_name {
> +struct scodec_dev_name {

you are changing things in patch_realtek.c that are completely unrelated
to the tas2781, usually the recommendation is that the changes have to
be part of a different patch so that the real addition of tas2781 parts
are easier to review.

>  	const char *bus;
>  	const char *hid;
>  	int index;
> @@ -6730,7 +6730,7 @@ struct cs35l41_dev_name {
>  /* match the device name in a slightly relaxed manner */
>  static int comp_match_cs35l41_dev_name(struct device *dev, void *data)
>  {
> -	struct cs35l41_dev_name *p = data;
> +	struct scodec_dev_name *p = data;
>  	const char *d = dev_name(dev);
>  	int n = strlen(p->bus);
>  	char tmp[32];
> @@ -6746,12 +6746,32 @@ static int comp_match_cs35l41_dev_name(struct device *dev, void *data)
>  	return !strcmp(d + n, tmp);
>  }
>  
> +static int comp_match_tas2781_dev_name(struct device *dev,
> +	void *data)
> +{
> +	struct scodec_dev_name *p = data;
> +	const char *d = dev_name(dev);
> +	int n = strlen(p->bus);
> +	char tmp[32];
> +
> +	/* check the bus name */
> +	if (strncmp(d, p->bus, n))
> +		return 0;
> +	/* skip the bus number */
> +	if (isdigit(d[n]))
> +		n++;
> +	/* the rest must be exact matching */
> +	snprintf(tmp, sizeof(tmp), "-%s:00", p->hid);

ACPI can sometimes add :01 suffixes, this looks like the re-invention of
an ACPI helper?

Adding Andy for the ACPI review.

> +
> +	return !strcmp(d + n, tmp);
> +}
> +
>  static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char *bus,
>  				  const char *hid, int count)
>  {
>  	struct device *dev = hda_codec_dev(cdc);
>  	struct alc_spec *spec = cdc->spec;
> -	struct cs35l41_dev_name *rec;
> +	struct scodec_dev_name *rec;
>  	int ret, i;
>  
>  	switch (action) {
> @@ -6779,6 +6799,41 @@ static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char
>  	}
>  }
>  
> +static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
> +	const char *bus, const char *hid)
> +{
> +	struct device *dev = hda_codec_dev(cdc);
> +	struct alc_spec *spec = cdc->spec;
> +	struct scodec_dev_name *rec;
> +	int ret;
> +
> +	switch (action) {
> +	case HDA_FIXUP_ACT_PRE_PROBE:
> +		rec = devm_kmalloc(dev, sizeof(*rec), GFP_KERNEL);
> +		if (!rec)
> +			return;
> +		rec->bus = bus;
> +		rec->hid = hid;
> +		rec->index = 0;
> +		spec->comps[0].codec = cdc;
> +		component_match_add(dev, &spec->match,
> +			comp_match_tas2781_dev_name, rec);
> +		ret = component_master_add_with_match(dev, &comp_master_ops,
> +			spec->match);
> +		if (ret)
> +			codec_err(cdc,
> +				"Fail to register component aggregator %d\n",
> +				ret);
> +		else
> +			spec->gen.pcm_playback_hook =
> +				comp_generic_playback_hook;
> +		break;
> +	case HDA_FIXUP_ACT_FREE:

This is the first use of FIXUP_ACT_FREE in this function, is this
required/intentional?

> +		component_master_del(dev, &comp_master_ops);

Also is there a need to test that the PRE_PROBE actually worked?

> +		break;
> +	}
> +}
> +
>  static void cs35l41_fixup_i2c_two(struct hda_codec *cdc, const struct hda_fixup *fix, int action)
>  {
>  	cs35l41_generic_fixup(cdc, action, "i2c", "CSC3551", 2);
> @@ -6806,6 +6861,12 @@ static void alc287_fixup_legion_16ithg6_speakers(struct hda_codec *cdc, const st
>  	cs35l41_generic_fixup(cdc, action, "i2c", "CLSA0101", 2);
>  }
>  
> +static void tas2781_fixup_i2c(struct hda_codec *cdc,
> +	const struct hda_fixup *fix, int action)
> +{
> +	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");

TI ACPI ID is TXNW

https://uefi.org/ACPI_ID_List?search=TEXAS

There's also a PNP ID PXN

https://uefi.org/PNP_ID_List?search=TEXAS

"TIAS" looks like an invented identifier. It's not uncommon but should
be recorded with a comment if I am not mistaken.


> +}
> +
>  /* for alc295_fixup_hp_top_speakers */
>  #include "hp_x360_helper.c"
>  
> @@ -7231,6 +7292,7 @@ enum {
>  	ALC295_FIXUP_DELL_INSPIRON_TOP_SPEAKERS,
>  	ALC236_FIXUP_DELL_DUAL_CODECS,
>  	ALC287_FIXUP_CS35L41_I2C_2_THINKPAD_ACPI,
> +	ALC287_FIXUP_TAS2781_I2C,
>  };
>  
>  /* A special fixup for Lenovo C940 and Yoga Duet 7;
> @@ -9309,6 +9371,12 @@ static const struct hda_fixup alc269_fixups[] = {
>  		.chained = true,
>  		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
>  	},
> +	[ALC287_FIXUP_TAS2781_I2C] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = tas2781_fixup_i2c,
> +		.chained = true,
> +		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,

If this is part of the THINKPAD chain, should this fixup name also refer
to THINKPAD, as e.g. ALC287_FIXUP_CS35L41_I2C_2_THINKPAD_ACPI
Andy Shevchenko Aug. 18, 2023, 5:01 p.m. UTC | #2
On Fri, Aug 18, 2023 at 11:00:34AM -0500, Pierre-Louis Bossart wrote:

...

> > +static int comp_match_tas2781_dev_name(struct device *dev,
> > +	void *data)
> > +{
> > +	struct scodec_dev_name *p = data;
> > +	const char *d = dev_name(dev);
> > +	int n = strlen(p->bus);
> > +	char tmp[32];
> > +
> > +	/* check the bus name */
> > +	if (strncmp(d, p->bus, n))
> > +		return 0;

> > +	/* skip the bus number */
> > +	if (isdigit(d[n]))
> > +		n++;

Why do you think it can't be two or more digits?

> > +	/* the rest must be exact matching */
> > +	snprintf(tmp, sizeof(tmp), "-%s:00", p->hid);
> 
> ACPI can sometimes add :01 suffixes, this looks like the re-invention of
> an ACPI helper?
> 
> Adding Andy for the ACPI review.
> 
> > +	return !strcmp(d + n, tmp);
> > +}

Yes, this looks like reinventing a wheel.
Just compare dev_name() against what is in p->....

...

> > +static void tas2781_fixup_i2c(struct hda_codec *cdc,
> > +	const struct hda_fixup *fix, int action)
> > +{
> > +	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
> 
> TI ACPI ID is TXNW
> 
> https://uefi.org/ACPI_ID_List?search=TEXAS
> 
> There's also a PNP ID PXN
> 
> https://uefi.org/PNP_ID_List?search=TEXAS
> 
> "TIAS" looks like an invented identifier. It's not uncommon but should
> be recorded with a comment if I am not mistaken.
> 
> > +}

Thank you, but actually it's a strong NAK to this even with the comment.
We have to teach people to follow the specification (may be even hard way).

So where did you get the ill-formed ACPI ID?
Is Texas Instrument aware of this?
Can we have a confirmation letter from TI for this ID, please?
Takashi Iwai Aug. 20, 2023, 9:16 a.m. UTC | #3
On Fri, 18 Aug 2023 19:01:16 +0200,
Andy Shevchenko wrote:
> 
> On Fri, Aug 18, 2023 at 11:00:34AM -0500, Pierre-Louis Bossart wrote:
> 
> ...
> 
> > > +static int comp_match_tas2781_dev_name(struct device *dev,
> > > +	void *data)
> > > +{
> > > +	struct scodec_dev_name *p = data;
> > > +	const char *d = dev_name(dev);
> > > +	int n = strlen(p->bus);
> > > +	char tmp[32];
> > > +
> > > +	/* check the bus name */
> > > +	if (strncmp(d, p->bus, n))
> > > +		return 0;
> 
> > > +	/* skip the bus number */
> > > +	if (isdigit(d[n]))
> > > +		n++;
> 
> Why do you think it can't be two or more digits?
> 
> > > +	/* the rest must be exact matching */
> > > +	snprintf(tmp, sizeof(tmp), "-%s:00", p->hid);
> > 
> > ACPI can sometimes add :01 suffixes, this looks like the re-invention of
> > an ACPI helper?
> > 
> > Adding Andy for the ACPI review.
> > 
> > > +	return !strcmp(d + n, tmp);
> > > +}
> 
> Yes, this looks like reinventing a wheel.
> Just compare dev_name() against what is in p->....

Note that comp_match_tas7281_dev_name() is a copy of
comp_patch_cs35l41_dev_name() and it was implemented in a hackish way
to be applicable to both I2C and SPI device names that have slightly
different naming rules.

> ...
> 
> > > +static void tas2781_fixup_i2c(struct hda_codec *cdc,
> > > +	const struct hda_fixup *fix, int action)
> > > +{
> > > +	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
> > 
> > TI ACPI ID is TXNW
> > 
> > https://uefi.org/ACPI_ID_List?search=TEXAS
> > 
> > There's also a PNP ID PXN
> > 
> > https://uefi.org/PNP_ID_List?search=TEXAS
> > 
> > "TIAS" looks like an invented identifier. It's not uncommon but should
> > be recorded with a comment if I am not mistaken.
> > 
> > > +}
> 
> Thank you, but actually it's a strong NAK to this even with the comment.
> We have to teach people to follow the specification (may be even hard way).
> 
> So where did you get the ill-formed ACPI ID?
> Is Texas Instrument aware of this?
> Can we have a confirmation letter from TI for this ID, please?

This is used already for products that have been long in the market,
so it's way too late to correct it, I'm afraid.

What we can do is to get the confirmation from TI, complain it, and
some verbose comment in the code, indeed.


thanks,

Takashi
Andy Shevchenko Aug. 21, 2023, 9:06 a.m. UTC | #4
On Sun, Aug 20, 2023 at 11:16:08AM +0200, Takashi Iwai wrote:
> On Fri, 18 Aug 2023 19:01:16 +0200,
> Andy Shevchenko wrote:
> > On Fri, Aug 18, 2023 at 11:00:34AM -0500, Pierre-Louis Bossart wrote:

...

> > > > +static void tas2781_fixup_i2c(struct hda_codec *cdc,
> > > > +	const struct hda_fixup *fix, int action)
> > > > +{
> > > > +	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
> > > 
> > > TI ACPI ID is TXNW
> > > 
> > > https://uefi.org/ACPI_ID_List?search=TEXAS
> > > 
> > > There's also a PNP ID PXN
> > > 
> > > https://uefi.org/PNP_ID_List?search=TEXAS
> > > 
> > > "TIAS" looks like an invented identifier. It's not uncommon but should
> > > be recorded with a comment if I am not mistaken.
> > > 
> > > > +}
> > 
> > Thank you, but actually it's a strong NAK to this even with the comment.
> > We have to teach people to follow the specification (may be even hard way).
> > 
> > So where did you get the ill-formed ACPI ID?
> > Is Texas Instrument aware of this?
> > Can we have a confirmation letter from TI for this ID, please?
> 
> This is used already for products that have been long in the market,
> so it's way too late to correct it, I'm afraid.
> 
> What we can do is to get the confirmation from TI, complain it, and
> some verbose comment in the code, indeed.

Oh, no! Who made that ID, I really want to point that at their faces.
Look at the Coreboot (successful) case, they created something, but
in time asked and then actually fixed the ill-formed ID (that was for
one of RTC chips).

For this, please make sure that commit message has that summary, explaining that
- states that ID is ill-formed
- states that there are products with it (DSDT excerpt is a must)
- lists (a few?) products where that ID is used
- ideally explains who invented that and Cc them to the patch, so they will
  know they made a big mistake
Takashi Iwai Aug. 21, 2023, 9:14 a.m. UTC | #5
On Mon, 21 Aug 2023 11:06:20 +0200,
Andy Shevchenko wrote:
> 
> On Sun, Aug 20, 2023 at 11:16:08AM +0200, Takashi Iwai wrote:
> > On Fri, 18 Aug 2023 19:01:16 +0200,
> > Andy Shevchenko wrote:
> > > On Fri, Aug 18, 2023 at 11:00:34AM -0500, Pierre-Louis Bossart wrote:
> 
> ...
> 
> > > > > +static void tas2781_fixup_i2c(struct hda_codec *cdc,
> > > > > +	const struct hda_fixup *fix, int action)
> > > > > +{
> > > > > +	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
> > > > 
> > > > TI ACPI ID is TXNW
> > > > 
> > > > https://uefi.org/ACPI_ID_List?search=TEXAS
> > > > 
> > > > There's also a PNP ID PXN
> > > > 
> > > > https://uefi.org/PNP_ID_List?search=TEXAS
> > > > 
> > > > "TIAS" looks like an invented identifier. It's not uncommon but should
> > > > be recorded with a comment if I am not mistaken.
> > > > 
> > > > > +}
> > > 
> > > Thank you, but actually it's a strong NAK to this even with the comment.
> > > We have to teach people to follow the specification (may be even hard way).
> > > 
> > > So where did you get the ill-formed ACPI ID?
> > > Is Texas Instrument aware of this?
> > > Can we have a confirmation letter from TI for this ID, please?
> > 
> > This is used already for products that have been long in the market,
> > so it's way too late to correct it, I'm afraid.
> > 
> > What we can do is to get the confirmation from TI, complain it, and
> > some verbose comment in the code, indeed.
> 
> Oh, no! Who made that ID, I really want to point that at their faces.
> Look at the Coreboot (successful) case, they created something, but
> in time asked and then actually fixed the ill-formed ID (that was for
> one of RTC chips).
> 
> For this, please make sure that commit message has that summary, explaining that
> - states that ID is ill-formed
> - states that there are products with it (DSDT excerpt is a must)
> - lists (a few?) products where that ID is used
> - ideally explains who invented that and Cc them to the patch, so they will
>   know they made a big mistake

Sure, we should complain further and ask them that such a problem
won't happen again.  I'm 100% for it.

But the fact is that lots of machines have been already shipped with
this ID since long time ago, and 99.99% of them have been running on
Windows.  Hence I expect that the chance to get a corrected ID is very
very low, and waiting for the support on Linux until the correction of
ID actually happens makes little sense; that's my point.


Takashi
Andy Shevchenko Aug. 21, 2023, 9:26 a.m. UTC | #6
On Mon, Aug 21, 2023 at 11:14:59AM +0200, Takashi Iwai wrote:
> On Mon, 21 Aug 2023 11:06:20 +0200,
> Andy Shevchenko wrote:
> > On Sun, Aug 20, 2023 at 11:16:08AM +0200, Takashi Iwai wrote:
> > > On Fri, 18 Aug 2023 19:01:16 +0200,
> > > Andy Shevchenko wrote:
> > > > On Fri, Aug 18, 2023 at 11:00:34AM -0500, Pierre-Louis Bossart wrote:

...

> > > > > > +	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
> > > > > 
> > > > > TI ACPI ID is TXNW
> > > > > 
> > > > > https://uefi.org/ACPI_ID_List?search=TEXAS
> > > > > 
> > > > > There's also a PNP ID PXN
> > > > > 
> > > > > https://uefi.org/PNP_ID_List?search=TEXAS
> > > > > 
> > > > > "TIAS" looks like an invented identifier. It's not uncommon but should
> > > > > be recorded with a comment if I am not mistaken.
> > > > > 
> > > > > > +}
> > > > 
> > > > Thank you, but actually it's a strong NAK to this even with the comment.
> > > > We have to teach people to follow the specification (may be even hard way).
> > > > 
> > > > So where did you get the ill-formed ACPI ID?
> > > > Is Texas Instrument aware of this?
> > > > Can we have a confirmation letter from TI for this ID, please?
> > > 
> > > This is used already for products that have been long in the market,
> > > so it's way too late to correct it, I'm afraid.
> > > 
> > > What we can do is to get the confirmation from TI, complain it, and
> > > some verbose comment in the code, indeed.
> > 
> > Oh, no! Who made that ID, I really want to point that at their faces.
> > Look at the Coreboot (successful) case, they created something, but
> > in time asked and then actually fixed the ill-formed ID (that was for
> > one of RTC chips).
> > 
> > For this, please make sure that commit message has that summary, explaining that
> > - states that ID is ill-formed
> > - states that there are products with it (DSDT excerpt is a must)
> > - lists (a few?) products where that ID is used
> > - ideally explains who invented that and Cc them to the patch, so they will
> >   know they made a big mistake
> 
> Sure, we should complain further and ask them that such a problem
> won't happen again.  I'm 100% for it.
> 
> But the fact is that lots of machines have been already shipped with
> this ID since long time ago, and 99.99% of them have been running on
> Windows.  Hence I expect that the chance to get a corrected ID is very
> very low, and waiting for the support on Linux until the correction of
> ID actually happens makes little sense; that's my point.

Yes, I understand that. But we have to inform them to prevent from
repeating this big mistake in the future.
diff mbox series

Patch

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 44fccfb93cff..ba1b02ed184a 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6721,7 +6721,7 @@  static void comp_generic_playback_hook(struct hda_pcm_stream *hinfo, struct hda_
 	}
 }
 
-struct cs35l41_dev_name {
+struct scodec_dev_name {
 	const char *bus;
 	const char *hid;
 	int index;
@@ -6730,7 +6730,7 @@  struct cs35l41_dev_name {
 /* match the device name in a slightly relaxed manner */
 static int comp_match_cs35l41_dev_name(struct device *dev, void *data)
 {
-	struct cs35l41_dev_name *p = data;
+	struct scodec_dev_name *p = data;
 	const char *d = dev_name(dev);
 	int n = strlen(p->bus);
 	char tmp[32];
@@ -6746,12 +6746,32 @@  static int comp_match_cs35l41_dev_name(struct device *dev, void *data)
 	return !strcmp(d + n, tmp);
 }
 
+static int comp_match_tas2781_dev_name(struct device *dev,
+	void *data)
+{
+	struct scodec_dev_name *p = data;
+	const char *d = dev_name(dev);
+	int n = strlen(p->bus);
+	char tmp[32];
+
+	/* check the bus name */
+	if (strncmp(d, p->bus, n))
+		return 0;
+	/* skip the bus number */
+	if (isdigit(d[n]))
+		n++;
+	/* the rest must be exact matching */
+	snprintf(tmp, sizeof(tmp), "-%s:00", p->hid);
+
+	return !strcmp(d + n, tmp);
+}
+
 static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char *bus,
 				  const char *hid, int count)
 {
 	struct device *dev = hda_codec_dev(cdc);
 	struct alc_spec *spec = cdc->spec;
-	struct cs35l41_dev_name *rec;
+	struct scodec_dev_name *rec;
 	int ret, i;
 
 	switch (action) {
@@ -6779,6 +6799,41 @@  static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char
 	}
 }
 
+static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
+	const char *bus, const char *hid)
+{
+	struct device *dev = hda_codec_dev(cdc);
+	struct alc_spec *spec = cdc->spec;
+	struct scodec_dev_name *rec;
+	int ret;
+
+	switch (action) {
+	case HDA_FIXUP_ACT_PRE_PROBE:
+		rec = devm_kmalloc(dev, sizeof(*rec), GFP_KERNEL);
+		if (!rec)
+			return;
+		rec->bus = bus;
+		rec->hid = hid;
+		rec->index = 0;
+		spec->comps[0].codec = cdc;
+		component_match_add(dev, &spec->match,
+			comp_match_tas2781_dev_name, rec);
+		ret = component_master_add_with_match(dev, &comp_master_ops,
+			spec->match);
+		if (ret)
+			codec_err(cdc,
+				"Fail to register component aggregator %d\n",
+				ret);
+		else
+			spec->gen.pcm_playback_hook =
+				comp_generic_playback_hook;
+		break;
+	case HDA_FIXUP_ACT_FREE:
+		component_master_del(dev, &comp_master_ops);
+		break;
+	}
+}
+
 static void cs35l41_fixup_i2c_two(struct hda_codec *cdc, const struct hda_fixup *fix, int action)
 {
 	cs35l41_generic_fixup(cdc, action, "i2c", "CSC3551", 2);
@@ -6806,6 +6861,12 @@  static void alc287_fixup_legion_16ithg6_speakers(struct hda_codec *cdc, const st
 	cs35l41_generic_fixup(cdc, action, "i2c", "CLSA0101", 2);
 }
 
+static void tas2781_fixup_i2c(struct hda_codec *cdc,
+	const struct hda_fixup *fix, int action)
+{
+	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
+}
+
 /* for alc295_fixup_hp_top_speakers */
 #include "hp_x360_helper.c"
 
@@ -7231,6 +7292,7 @@  enum {
 	ALC295_FIXUP_DELL_INSPIRON_TOP_SPEAKERS,
 	ALC236_FIXUP_DELL_DUAL_CODECS,
 	ALC287_FIXUP_CS35L41_I2C_2_THINKPAD_ACPI,
+	ALC287_FIXUP_TAS2781_I2C,
 };
 
 /* A special fixup for Lenovo C940 and Yoga Duet 7;
@@ -9309,6 +9371,12 @@  static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
 	},
+	[ALC287_FIXUP_TAS2781_I2C] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = tas2781_fixup_i2c,
+		.chained = true,
+		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
+	},
 };
 
 static const struct snd_pci_quirk alc269_fixup_tbl[] = {
@@ -9884,6 +9952,20 @@  static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x17aa, 0x3853, "Lenovo Yoga 7 15ITL5", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
 	SND_PCI_QUIRK(0x17aa, 0x3855, "Legion 7 16ITHG6", ALC287_FIXUP_LEGION_16ITHG6),
 	SND_PCI_QUIRK(0x17aa, 0x3869, "Lenovo Yoga7 14IAL7", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),
+	SND_PCI_QUIRK(0x17aa, 0x387d, "Yoga S780-16 pro Quad AAC", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x387e, "Yoga S780-16 pro Quad YC", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x3881, "YB9 dual powe mode2 YC", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x3884, "Y780 YG DUAL", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x3886, "Y780 VECO DUAL", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x38a7, "Y780P AMD YG dual", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x38a8, "Y780P AMD VECO dual", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x38ba, "Yoga S780-14.5 Air AMD quad YC", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x38bb, "Yoga S780-14.5 Air AMD quad AAC", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x38be, "Yoga S980-14.5 proX YC Dual", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x38bf, "Yoga S980-14.5 proX LX Dual", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x38c3, "Y980 DUAL", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x38cb, "Y790 YG DUAL", ALC287_FIXUP_TAS2781_I2C),
+	SND_PCI_QUIRK(0x17aa, 0x38cd, "Y790 VECO DUAL", ALC287_FIXUP_TAS2781_I2C),
 	SND_PCI_QUIRK(0x17aa, 0x3902, "Lenovo E50-80", ALC269_FIXUP_DMIC_THINKPAD_ACPI),
 	SND_PCI_QUIRK(0x17aa, 0x3977, "IdeaPad S210", ALC283_FIXUP_INT_MIC),
 	SND_PCI_QUIRK(0x17aa, 0x3978, "Lenovo B50-70", ALC269_FIXUP_DMIC_THINKPAD_ACPI),