[RFC] sound: cs35l41: Add support for Legion 7 16ACHg6 laptop
tanureal at opensource.cirrus.com
tanureal at opensource.cirrus.com
Thu Oct 14 14:54:54 CEST 2021
On 10/13/21 5:15 PM, Takashi Iwai <tiwai at suse.de> wrote:
> On Tue, 12 Oct 2021 22:22:07 +0200,
> Takashi Iwai wrote:
> >
> > On Fri, 08 Oct 2021 13:19:02 +0200,
> > Lucas Tanure wrote:
> >>
> >> Hi,
> >>
> >> I would like to get some guidance about this solution to
> >> support the 16ACHg6 laptop.
> >>
> >> Hardware:
> >> - The 16ACHg6 laptop has two CS35L41 amplifiers, connected
> >> to Realtek ALC287 by an I2S bus and by and direct I2C to the CPU.
> >> - The ALC287 codec is connected to the CPU by an HDA bus.
> >> - The CS35L41 has a DSP which will require firmware to be loaded.
> >>
> >> Architecture:
> >> - To load the firmware for CS35L41, this solution will require
> >> the wm_adsp library, which requires regmap, header definitions and
> >> register tables.
> >> - To minimize the duplication of the code, the HDA functions will
> >> be placed inside the ASoC CS35L41 driver.
> >> - Finally, HDA patch_realtek will access exposed functions from
> >> ASoC CS35L41 driver to initialize the amplifiers, start and stop
> >> streams and load firmware.
> >
> > Through a very quick glance, a potential problem is that this design
> > would make the HD-audio codec driver dependent on those other ASoC
> > ones. As the Realtek HD-audio codec driver is used by quite many
> > other people, we'd like to reduce such dependency mess.
> >
> > Maybe a dynamic binding with component framework can be used?
> >
> > Alternatively, we may build up a stuff on top of ASoC like what SOF
> > driver did. It'll be another lot of work, though.
>
> Or, yet another (and easier) alternative would be to create a new
> codec driver that is specific to vendor+subsystem pair. We'll need to
> extend the hda_device_id and its matching mechanism, and the realtek
> codec driver needs to exclude the matching with the given SSID
> explicitly.
>
> A patch below is an example.
>
>
> Takashi
>
> ---
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index ae2e75d15b21..5558f2ba2fcf 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -248,6 +248,7 @@ struct serio_device_id {
>
> struct hda_device_id {
> __u32 vendor_id;
> + __u32 subsystem_id;
> __u32 rev_id;
> __u8 api_version;
> const char *name;
> diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
> index 0e45963bb767..3e316a798361 100644
> --- a/include/sound/hda_codec.h
> +++ b/include/sound/hda_codec.h
> @@ -79,10 +79,12 @@ typedef int (*hda_codec_patch_t)(struct hda_codec *);
> #define HDA_CODEC_ID_GENERIC_HDMI 0x00000101
> #define HDA_CODEC_ID_GENERIC 0x00000201
>
> -#define HDA_CODEC_REV_ENTRY(_vid, _rev, _name, _patch) \
> - { .vendor_id = (_vid), .rev_id = (_rev), .name = (_name), \
> - .api_version = HDA_DEV_LEGACY, \
> +#define HDA_CODEC_FULL_ENTRY(_vid, _subsystem, _rev, _name, _patch) \
> + { .vendor_id = (_vid), .subsystem_id = (_subsystem), .rev_id = (_rev), \
> + .name = (_name), .api_version = HDA_DEV_LEGACY, \
> .driver_data = (unsigned long)(_patch) }
> +#define HDA_CODEC_REV_ENTRY(_vid, _rev, _name, _patch) \
> + HDA_CODEC_FULL_ENTRY(_vid, 0, _rev, _name, _patch)
> #define HDA_CODEC_ENTRY(_vid, _name, _patch) \
> HDA_CODEC_REV_ENTRY(_vid, 0, _name, _patch)
>
> diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
> index cc3625617a0e..641b4f9bb2be 100644
> --- a/scripts/mod/devicetable-offsets.c
> +++ b/scripts/mod/devicetable-offsets.c
> @@ -211,6 +211,7 @@ int main(void)
>
> DEVID(hda_device_id);
> DEVID_FIELD(hda_device_id, vendor_id);
> + DEVID_FIELD(hda_device_id, subsystem_id);
> DEVID_FIELD(hda_device_id, rev_id);
> DEVID_FIELD(hda_device_id, api_version);
>
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 49aba862073e..d8faf0065c95 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -1255,15 +1255,17 @@ static int do_ulpi_entry(const char *filename, void *symval,
> return 1;
> }
>
> -/* Looks like: hdaudio:vNrNaN */
> +/* Looks like: hdaudio:vNsNrNaN */
> static int do_hda_entry(const char *filename, void *symval, char *alias)
> {
> DEF_FIELD(symval, hda_device_id, vendor_id);
> + DEF_FIELD(symval, hda_device_id, subsystem_id);
> DEF_FIELD(symval, hda_device_id, rev_id);
> DEF_FIELD(symval, hda_device_id, api_version);
>
> strcpy(alias, "hdaudio:");
> ADD(alias, "v", vendor_id != 0, vendor_id);
> + ADD(alias, "s", subsystem_id != 0, subsystem_id);
> ADD(alias, "r", rev_id != 0, rev_id);
> ADD(alias, "a", api_version != 0, api_version);
>
> diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
> index 3e9e9ac804f6..662abd40ca6a 100644
> --- a/sound/hda/hdac_device.c
> +++ b/sound/hda/hdac_device.c
> @@ -206,8 +206,9 @@ EXPORT_SYMBOL_GPL(snd_hdac_device_set_chip_name);
> */
> int snd_hdac_codec_modalias(struct hdac_device *codec, char *buf, size_t size)
> {
> - return scnprintf(buf, size, "hdaudio:v%08Xr%08Xa%02X\n",
> - codec->vendor_id, codec->revision_id, codec->type);
> + return scnprintf(buf, size, "hdaudio:v%08Xs%08Xr%08Xa%02X\n",
> + codec->vendor_id, codec->subsystem_id,
> + codec->revision_id, codec->type);
> }
> EXPORT_SYMBOL_GPL(snd_hdac_codec_modalias);
>
> diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile
> index b8fa682ce66a..9f559773bf99 100644
> --- a/sound/pci/hda/Makefile
> +++ b/sound/pci/hda/Makefile
> @@ -15,6 +15,7 @@ CFLAGS_hda_intel.o := -I$(src)
>
> snd-hda-codec-generic-objs := hda_generic.o
> snd-hda-codec-realtek-objs := patch_realtek.o
> +snd-hda-codec-test-objs := patch_test.o
> snd-hda-codec-cmedia-objs := patch_cmedia.o
> snd-hda-codec-analog-objs := patch_analog.o
> snd-hda-codec-idt-objs := patch_sigmatel.o
> @@ -32,7 +33,7 @@ obj-$(CONFIG_SND_HDA) := snd-hda-codec.o
>
> # codec drivers
> obj-$(CONFIG_SND_HDA_GENERIC) += snd-hda-codec-generic.o
> -obj-$(CONFIG_SND_HDA_CODEC_REALTEK) += snd-hda-codec-realtek.o
> +obj-$(CONFIG_SND_HDA_CODEC_REALTEK) += snd-hda-codec-realtek.o snd-hda-codec-test.o
> obj-$(CONFIG_SND_HDA_CODEC_CMEDIA) += snd-hda-codec-cmedia.o
> obj-$(CONFIG_SND_HDA_CODEC_ANALOG) += snd-hda-codec-analog.o
> obj-$(CONFIG_SND_HDA_CODEC_SIGMATEL) += snd-hda-codec-idt.o
> diff --git a/sound/pci/hda/hda_bind.c b/sound/pci/hda/hda_bind.c
> index 1c8bffc3eec6..367f220ec91e 100644
> --- a/sound/pci/hda/hda_bind.c
> +++ b/sound/pci/hda/hda_bind.c
> @@ -200,7 +200,7 @@ static inline bool codec_probed(struct hda_codec *codec)
> static void request_codec_module(struct hda_codec *codec)
> {
> #ifdef MODULE
> - char modalias[32];
> + char modalias[64];
> const char *mod = NULL;
>
> switch (codec->probe_id) {
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 22d27b12c4e7..993b49554457 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -16,6 +16,7 @@
> #include <linux/pci.h>
> #include <linux/dmi.h>
> #include <linux/module.h>
> +#include <linux/export.h>
> #include <linux/input.h>
> #include <linux/leds.h>
> #include <sound/core.h>
> @@ -9510,7 +9511,7 @@ static void alc269_fill_coef(struct hda_codec *codec)
>
> /*
> */
> -static int patch_alc269(struct hda_codec *codec)
> +int snd_hda_codec_realtek_alc269_probe(struct hda_codec *codec)
> {
> struct alc_spec *spec;
> int err;
> @@ -9667,6 +9668,9 @@ static int patch_alc269(struct hda_codec *codec)
>
> alc_pre_init(codec);
>
> + if (codec->fixup_id != HDA_FIXUP_ID_NOT_SET)
> + goto skip_pick_fixup;
> +
> snd_hda_pick_fixup(codec, alc269_fixup_models,
> alc269_fixup_tbl, alc269_fixups);
> /* FIXME: both TX300 and ROG Strix G17 have the same SSID, and
> @@ -9683,6 +9687,8 @@ static int patch_alc269(struct hda_codec *codec)
> snd_hda_pick_pin_fixup(codec, alc269_fallback_pin_fixup_tbl, alc269_fixups, false);
> snd_hda_pick_fixup(codec, NULL, alc269_fixup_vendor_tbl,
> alc269_fixups);
> +
> + skip_pick_fixup:
> snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_PRE_PROBE);
>
> alc_auto_parse_customize_define(codec);
> @@ -9709,6 +9715,18 @@ static int patch_alc269(struct hda_codec *codec)
> alc_free(codec);
> return err;
> }
> +EXPORT_SYMBOL(snd_hda_codec_realtek_alc269_probe);
> +
> +static int patch_alc269(struct hda_codec *codec)
> +{
> + if (codec->core.vendor_id == 0x10ec0298 &&
> + codec->core.subsystem_id == 0x102806e5) {
> + pr_info("XXX realtek codec driver: skipping\n");
> + return -ENODEV;
> + }
> +
> + return snd_hda_codec_realtek_alc269_probe(codec);
> +}
>
> /*
> * ALC861
> diff --git a/sound/pci/hda/patch_test.c b/sound/pci/hda/patch_test.c
> new file mode 100644
> index 000000000000..9070cc075af0
> --- /dev/null
> +++ b/sound/pci/hda/patch_test.c
> @@ -0,0 +1,31 @@
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <sound/core.h>
> +#include <sound/hda_codec.h>
> +
> +int snd_hda_codec_realtek_alc269_probe(struct hda_codec *codec);
> +
> +// static const struct hda_fixup test_fixup = { ... };
> +
> +static int test_probe(struct hda_codec *codec)
> +{
> + pr_info("XXX forked driver\n");
> + // codec->fixup_id = 0;
> + // codec->fixup_list = &test_fixup;
> + return snd_hda_codec_realtek_alc269_probe(codec);
> +}
> +
> +static const struct hda_device_id snd_hda_id_test[] = {
> + HDA_CODEC_FULL_ENTRY(0x10ec0298, 0x102806e5, 0, test_probe),
> + {} /* terminator */
> +};
> +MODULE_DEVICE_TABLE(hdaudio, snd_hda_id_test);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Test HD-audio codec");
> +
> +static struct hda_codec_driver test_driver = {
> + .id = snd_hda_id_test,
> +};
> +
> +module_hda_codec_driver(test_driver);
>
>
We need the Realtek for the I2S of cs35l41, the audio goes from CPU to Realtek then cs35l41. So we don't want to skip realtek initializations and functions.
I am finishing a second version that I will send soon.
Thanks
Lucas
More information about the Alsa-devel
mailing list