2024-04-09 02:49:44

by Baojun Xu

[permalink] [raw]
Subject: [PATCH v2 1/3] ALSA: hda/tas2781: Modification for add tas2781 driver for SPI

Integrate tas2781 configs for HP Laptops. Every tas2781 in the laptop
will work as a single speaker on SPI bus. The code support realtek as
the primary codec.

Signed-off-by: Baojun Xu <[email protected]>

---
- Change depends on SPI to SPI_MASTER in sound/pci/hda/Kconfig
- Add select SND_HDA_SCODEC_COMPONENT in sound/pci/hda/Kconfig
- Change comp_generic_fixup() 5th parameter from "-%s:00" to
"-%s:00-tas2781-hda.%d" in sound/pci/hda/patch_realtek.c
- Change chain_id from ALC285_FIXUP_THINKPAD_HEADSET_JACK to
ALC285_FIXUP_HP_GPIO_LED in sound/pci/hda/patch_realtek.c
- Remove project name "Gemtree"
---
drivers/acpi/scan.c | 1 +
drivers/platform/x86/serial-multi-instantiate.c | 10 ++++++++++
sound/pci/hda/Kconfig | 15 +++++++++++++++
sound/pci/hda/Makefile | 2 ++
sound/pci/hda/patch_realtek.c | 15 +++++++++++++++
5 files changed, 43 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 7c157bf92695..8166343f26d0 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1766,6 +1766,7 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
{"INT33FE", },
{"INT3515", },
/* Non-conforming _HID for Cirrus Logic already released */
+ {"TXNW2781", },
{"CLSA0100", },
{"CLSA0101", },
/*
diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
index 97b9c6392230..f54636b576d1 100644
--- a/drivers/platform/x86/serial-multi-instantiate.c
+++ b/drivers/platform/x86/serial-multi-instantiate.c
@@ -368,6 +368,15 @@ static const struct smi_node cs35l57_hda = {
.bus_type = SMI_AUTO_DETECT,
};

+static const struct smi_node tas2781_hda = {
+ .instances = {
+ { "tas2781-hda", IRQ_RESOURCE_AUTO, 0 },
+ { "tas2781-hda", IRQ_RESOURCE_AUTO, 0 },
+ {}
+ },
+ .bus_type = SMI_AUTO_DETECT,
+};
+
/*
* Note new device-ids must also be added to ignore_serial_bus_ids in
* drivers/acpi/scan.c: acpi_device_enumeration_by_parent().
@@ -383,6 +392,7 @@ static const struct acpi_device_id smi_acpi_ids[] = {
/* Non-conforming _HID for Cirrus Logic already released */
{ "CLSA0100", (unsigned long)&cs35l41_hda },
{ "CLSA0101", (unsigned long)&cs35l41_hda },
+ { "TXNW2781", (unsigned long)&tas2781_hda },
{ }
};
MODULE_DEVICE_TABLE(acpi, smi_acpi_ids);
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index f806636242ee..f25ad9ea51d9 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -202,6 +202,21 @@ config SND_HDA_SCODEC_TAS2781_I2C
comment "Set to Y if you want auto-loading the side codec driver"
depends on SND_HDA=y && SND_HDA_SCODEC_TAS2781_I2C=m

+config SND_HDA_SCODEC_TAS2781_SPI
+ tristate "Build TAS2781 HD-audio side codec support for SPI Bus"
+ depends on SPI_MASTER
+ depends on ACPI
+ depends on EFI
+ depends on SND_SOC
+ select SND_HDA_SCODEC_COMPONENT
+ select CRC32_SARWATE
+ help
+ Say Y or M here to include TAS2781 SPI HD-audio side codec support
+ in snd-hda-intel driver, such as ALC287.
+
+comment "Set to Y if you want auto-loading the side codec driver"
+ depends on SND_HDA=y && SND_HDA_SCODEC_TAS2781_SPI=m
+
config SND_HDA_CODEC_REALTEK
tristate "Build Realtek HD-audio codec support"
select SND_HDA_GENERIC
diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile
index 13e04e1f65de..9785cddb4845 100644
--- a/sound/pci/hda/Makefile
+++ b/sound/pci/hda/Makefile
@@ -39,6 +39,7 @@ snd-hda-scodec-cs35l56-spi-objs := cs35l56_hda_spi.o
snd-hda-cs-dsp-ctls-objs := hda_cs_dsp_ctl.o
snd-hda-scodec-component-objs := hda_component.o
snd-hda-scodec-tas2781-i2c-objs := tas2781_hda_i2c.o
+snd-hda-scodec-tas2781-spi-objs := tas2781_hda_spi.o tas2781_spi_fwlib.o

# common driver
obj-$(CONFIG_SND_HDA) := snd-hda-codec.o
@@ -70,6 +71,7 @@ obj-$(CONFIG_SND_HDA_SCODEC_CS35L56_SPI) += snd-hda-scodec-cs35l56-spi.o
obj-$(CONFIG_SND_HDA_CS_DSP_CONTROLS) += snd-hda-cs-dsp-ctls.o
obj-$(CONFIG_SND_HDA_SCODEC_COMPONENT) += snd-hda-scodec-component.o
obj-$(CONFIG_SND_HDA_SCODEC_TAS2781_I2C) += snd-hda-scodec-tas2781-i2c.o
+obj-$(CONFIG_SND_HDA_SCODEC_TAS2781_SPI) += snd-hda-scodec-tas2781-spi.o

# this must be the last entry after codec drivers;
# otherwise the codec patches won't be hooked before the PCI probe
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index cdcb28aa9d7b..4945309f45ae 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6913,6 +6913,13 @@ static void tas2781_fixup_i2c(struct hda_codec *cdc,
comp_generic_fixup(cdc, action, "i2c", "TIAS2781", "-%s:00", 1);
}

+static void tas2781_fixup_spi(struct hda_codec *cdc,
+ const struct hda_fixup *fix, int action)
+{
+ comp_generic_fixup(cdc, action, "spi", "TXNW2781",
+ "-%s:00-tas2781-hda.%d", 2);
+}
+
static void yoga7_14arb7_fixup_i2c(struct hda_codec *cdc,
const struct hda_fixup *fix, int action)
{
@@ -7451,6 +7458,7 @@ enum {
ALC236_FIXUP_DELL_DUAL_CODECS,
ALC287_FIXUP_CS35L41_I2C_2_THINKPAD_ACPI,
ALC287_FIXUP_TAS2781_I2C,
+ ALC245_FIXUP_TAS2781_SPI_2,
ALC287_FIXUP_YOGA7_14ARB7_I2C,
ALC245_FIXUP_HP_MUTE_LED_COEFBIT,
ALC245_FIXUP_HP_X360_MUTE_LEDS,
@@ -9614,6 +9622,12 @@ static const struct hda_fixup alc269_fixups[] = {
.chained = true,
.chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK,
},
+ [ALC245_FIXUP_TAS2781_SPI_2] = {
+ .type = HDA_FIXUP_FUNC,
+ .v.func = tas2781_fixup_spi,
+ .chained = true,
+ .chain_id = ALC285_FIXUP_HP_GPIO_LED,
+ },
[ALC287_FIXUP_YOGA7_14ARB7_I2C] = {
.type = HDA_FIXUP_FUNC,
.v.func = yoga7_14arb7_fixup_i2c,
@@ -10038,6 +10052,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0x103c, 0x8b8d, "HP", ALC236_FIXUP_HP_GPIO_LED),
SND_PCI_QUIRK(0x103c, 0x8b8f, "HP", ALC245_FIXUP_CS35L41_SPI_4_HP_GPIO_LED),
SND_PCI_QUIRK(0x103c, 0x8b92, "HP", ALC245_FIXUP_CS35L41_SPI_2_HP_GPIO_LED),
+ SND_PCI_QUIRK(0x103c, 0x8b93, "HP", ALC245_FIXUP_TAS2781_SPI_2),
SND_PCI_QUIRK(0x103c, 0x8b96, "HP", ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF),
SND_PCI_QUIRK(0x103c, 0x8b97, "HP", ALC236_FIXUP_HP_MUTE_LED_MICMUTE_VREF),
SND_PCI_QUIRK(0x103c, 0x8bdd, "HP Envy 17", ALC287_FIXUP_CS35L41_I2C_2),
--
2.40.1



2024-04-09 13:04:27

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] ALSA: hda/tas2781: Modification for add tas2781 driver for SPI

On Tue, Apr 09, 2024 at 10:48:13AM +0800, Baojun Xu wrote:
> Integrate tas2781 configs for HP Laptops. Every tas2781 in the laptop
> will work as a single speaker on SPI bus. The code support realtek as

Realtek

> the primary codec.

..

> {"INT33FE", },
> {"INT3515", },
> /* Non-conforming _HID for Cirrus Logic already released */
> + {"TXNW2781", },

This seems wrong. The ID here is correct from ACPI specification perspective.
Can you elaborate why you think it's "non-conforming _HID"?

> {"CLSA0100", },
> {"CLSA0101", },

..

> /* Non-conforming _HID for Cirrus Logic already released */
> { "CLSA0100", (unsigned long)&cs35l41_hda },
> { "CLSA0101", (unsigned long)&cs35l41_hda },
> + { "TXNW2781", (unsigned long)&tas2781_hda },

Ditto.

> { }

..

> @@ -39,6 +39,7 @@ snd-hda-scodec-cs35l56-spi-objs := cs35l56_hda_spi.o
> snd-hda-cs-dsp-ctls-objs := hda_cs_dsp_ctl.o
> snd-hda-scodec-component-objs := hda_component.o
> snd-hda-scodec-tas2781-i2c-objs := tas2781_hda_i2c.o
> +snd-hda-scodec-tas2781-spi-objs := tas2781_hda_spi.o tas2781_spi_fwlib.o

Actually these 'objs' has to be 'y', can you fix it in the prerequisite patch?

Also wondering why fwlib is only a requirement for SPI. How does I?C work?

--
With Best Regards,
Andy Shevchenko



2024-04-16 08:25:34

by Baojun Xu

[permalink] [raw]
Subject: Re: [EXTERNAL] Re: [PATCH v2 1/3] ALSA: hda/tas2781: Modification for add tas2781 driver for SPI

Hi Andy,

Thansk for your suggestions, answer in line.
> From: Andy Shevchenko <[email protected]>
> Sent: 09 April 2024 21:02
> To: Xu, Baojun
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Lu, Kevin; Ding, Shenghao; Navada Kanyana, Mukund; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Subject: [EXTERNAL] Re: [PATCH v2 1/3] ALSA: hda/tas2781: Modification for add tas2781 driver for SPI
>
> On Tue, Apr 09, 2024 at 10: 48: 13AM +0800, Baojun Xu wrote: > Integrate tas2781 configs for HP Laptops. Every tas2781 in the laptop > will work as a single speaker on SPI bus. The code support realtek as Realtek > the primary codec. 
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source of this email and know the content is safe. If you wish to report this message to IT Security, please forward the message as an attachment to [email protected]
>
> ZjQcmQRYFpfptBannerEnd
>
> On Tue, Apr 09, 2024 at 10:48:13AM +0800, Baojun Xu wrote:
> > Integrate tas2781 configs for HP Laptops. Every tas2781 in the laptop
> > will work as a single speaker on SPI bus. The code support realtek as
>
> Realtek
>
> > the primary codec.
>
> ...
>
> > {"INT33FE", },
> > {"INT3515", },
> > /* Non-conforming _HID for Cirrus Logic already released */
> > + {"TXNW2781", },
>
> This seems wrong. The ID here is correct from ACPI specification perspective.
> Can you elaborate why you think it's "non-conforming _HID"?

Just put into middle of array, should no effective to functions.
Ok, I will change it's position in next patch.
>
> > {"CLSA0100", },
> > {"CLSA0101", },
> >
> > ...
>
> > /* Non-conforming _HID for Cirrus Logic already released */
> > { "CLSA0100", (unsigned long)&cs35l41_hda },
> > { "CLSA0101", (unsigned long)&cs35l41_hda },
> > + { "TXNW2781", (unsigned long)&tas2781_hda },
>
> Ditto.
>
> > { }
>
> ...
>
> > @@ -39,6 +39,7 @@ snd-hda-scodec-cs35l56-spi-objs := cs35l56_hda_spi.o
> > snd-hda-cs-dsp-ctls-objs := hda_cs_dsp_ctl.o
> > snd-hda-scodec-component-objs := hda_component.o
> > snd-hda-scodec-tas2781-i2c-objs := tas2781_hda_i2c.o
> > +snd-hda-scodec-tas2781-spi-objs := tas2781_hda_spi.o tas2781_spi_fwlib.o
>
> Actually these 'objs' has to be 'y', can you fix it in the prerequisite patch?
>

Do you mean set CONFIG_SND_HDA_SCODEC_TAS2781_SPI=y in .config?
It's m now.

> Also wondering why fwlib is only a requirement for SPI. How does I²C work?

Because in I2C mode, one probed device driver will support all devices,
firmware binary is only one file, include all of devices.
But in SPI mode, multi driver probed, so we use single firmware binary for
every spi device.

>
> --
> With Best Regards,
> Andy Shevchenko
>
>
>

Best Regards
Jim




2024-04-16 14:02:33

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [EXTERNAL] Re: [PATCH v2 1/3] ALSA: hda/tas2781: Modification for add tas2781 driver for SPI

On Tue, Apr 16, 2024 at 07:45:21AM +0000, Xu, Baojun wrote:
> > From: Andy Shevchenko <[email protected]>
> > Sent: 09 April 2024 21:02
> > On Tue, Apr 09, 2024 at 10: 48: 13AM +0800, Baojun Xu wrote:
> > On Tue, Apr 09, 2024 at 10:48:13AM +0800, Baojun Xu wrote:

..

> > > @@ -39,6 +39,7 @@ snd-hda-scodec-cs35l56-spi-objs := cs35l56_hda_spi.o
> > > snd-hda-cs-dsp-ctls-objs := hda_cs_dsp_ctl.o
> > > snd-hda-scodec-component-objs := hda_component.o
> > > snd-hda-scodec-tas2781-i2c-objs := tas2781_hda_i2c.o
> > > +snd-hda-scodec-tas2781-spi-objs := tas2781_hda_spi.o tas2781_spi_fwlib.o
> >
> > Actually these 'objs' has to be 'y', can you fix it in the prerequisite patch?
>
> Do you mean set CONFIG_SND_HDA_SCODEC_TAS2781_SPI=y in .config?

No. I mean the Kconfig syntax in use. -objs is for user space tools. Kernel
code should use -y in this case.

> It's m now.
>
> > Also wondering why fwlib is only a requirement for SPI. How does I²C work?
>
> Because in I2C mode, one probed device driver will support all devices,
> firmware binary is only one file, include all of devices.
> But in SPI mode, multi driver probed, so we use single firmware binary for
> every spi device.

But does I²C version still need the firmware? Can't the FW handling be factored
out to a single module for both?

--
With Best Regards,
Andy Shevchenko