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