From 31fc5b06a6be62b30739d33eeabe6c2727679bb1 Mon Sep 17 00:00:00 2001 From: Nicholas Sudsgaard Date: Thu, 7 Aug 2025 08:31:24 +0900 Subject: [PATCH] device: Introduce reworked azalia verb table The current implementation of the HDA verb table has been shown to have some problems. The primary issue is that it requires the programmer to keep track of the amount of verbs that are going to be loaded. While this may sound simple, in practice there have been numerous cases where this "count field" has been forgotten to be updated or miscounted. In the case where the "count field" is incorrect, coreboot will start looking for codecs in some random memory location, essentially making loading further codecs impossible. Another issue is the "count field" treats 4 32-bit values as a single group, therefore the amount of verbs in the table must be a multiple of 4. This makes intuitive sense when only using the AZALIA_PIN_CFG() or AZALIA_SUBVENDOR() macros. However, once the verb table requires "controls" that use < 4 verbs (e.g. "Coefficient Index"), we need to add padding values to ensure the alignment is correct. This adds unnecessary verbs to the table which can further lead to unnecessary processing. Therefore, in this change we proprose a solution by separating the codec entries in the verb table into structures, which allows us to separate the verbs into an array and automatically calculate the "count field" using the ARRAY_SIZE() macro. It also makes iteration and access to member fields easier. We also now count the verbs and not 4 32-bit groups, eliminating the aforementioned alignment issue. Additionally, this change also changes the way coreboot searches for entries in the verb table. Before, we searched the table for only a matching vendor ID, but now we search for a matching vendor ID and codec address pair. This allows a mainboard to be able to correctly load multiple audio codecs that use the same chips. To make reviewing this large rework easier, we temporarily keep both implementations (legacy and reworked) and allow boards to choose which implementation to use by selecting a Kconfig. Newer boards are discouraged from using the legacy implementation, as it is not selected by default. This allows us to slowly change the codebase instead of changing everything at once. TEST= 1. Timeless build with AZALIA_USE_LEGACY_VERB_TABLE=y produces identical binaries (with INCLUDE_CONFIG_FILE=n) 2. HP ProBook 450 G3 using reworked verb table was able to load all verbs successfully. Change-Id: Ib16237de89956405afa3be5b4e3f64a4d62e6a48 Signed-off-by: Nicholas Sudsgaard Reviewed-on: https://review.coreboot.org/c/coreboot/+/88656 Reviewed-by: Elyes Haouas Reviewed-by: Angel Pons Reviewed-by: Maximilian Brune Tested-by: build bot (Jenkins) --- src/device/azalia_device.c | 60 ++++++++++++++++++++++++++++++ src/include/device/azalia_device.h | 28 ++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/src/device/azalia_device.c b/src/device/azalia_device.c index f894895d6c..82c6fdca41 100644 --- a/src/device/azalia_device.c +++ b/src/device/azalia_device.c @@ -233,6 +233,7 @@ __weak void mainboard_azalia_program_runtime_verbs(u8 *base, u32 viddid) { } +#if CONFIG(AZALIA_USE_LEGACY_VERB_TABLE) void azalia_codec_init(u8 *base, int addr, const u32 *verb_table, u32 verb_table_bytes) { if (azalia_write_verb(base, AZALIA_VERB_GET_VENDOR_ID(addr)) < 0) @@ -270,6 +271,65 @@ void azalia_codecs_init(u8 *base, u16 codec_mask) azalia_program_verb_table(base, pc_beep_verbs, pc_beep_verbs_size); } +#else +static struct azalia_codec *find_codec(struct azalia_codec *codecs, u32 vid, u8 addr) +{ + for (struct azalia_codec *codec = codecs; codec->vendor_id; codec++) { + if (codec->vendor_id == vid && codec->address == addr) + return codec; + } + return NULL; +} + +void azalia_codec_init(u8 *base, struct azalia_codec *codec) +{ + /* Give a generic name if nothing was provided */ + const char *name = codec->name ? codec->name : "codec"; + + printk(BIOS_DEBUG, "azalia_audio: initializing %s...\n", name); + printk(BIOS_DEBUG, "azalia_audio: - vendor ID: 0x%08x\n", codec->vendor_id); + printk(BIOS_DEBUG, "azalia_audio: - subsystem ID: 0x%08x\n", codec->subsystem_id); + printk(BIOS_DEBUG, "azalia_audio: - address: %u\n", codec->address); + + if (azalia_program_verb_table(base, codec->verbs, codec->verb_count) < 0) { + printk(BIOS_WARNING, "azalia_audio: failed to load verbs\n"); + return; + } + + mainboard_azalia_program_runtime_verbs(base, codec->vendor_id); + + printk(BIOS_DEBUG, "azalia_audio: done\n"); +} + +void azalia_custom_codecs_init(u8 *base, struct azalia_codec *codecs, u16 codec_mask) +{ + for (u8 i = 0; i < AZALIA_MAX_CODECS; i++) { + if (!(codec_mask & BIT(i))) + continue; + + if (azalia_write_verb(base, AZALIA_VERB_GET_VENDOR_ID(i)) < 0) + continue; + u32 vid = read32(base + HDA_IR_REG); + + struct azalia_codec *codec = find_codec(codecs, vid, i); + if (codec == NULL) { + printk(BIOS_WARNING, + "azalia_audio: cannot find verbs for codec with vendor ID: 0x%08x (address %u)\n", + vid, i); + continue; + } + + azalia_codec_init(base, codec); + } + + azalia_program_verb_table(base, pc_beep_verbs, pc_beep_verbs_size); +} + +void azalia_codecs_init(u8 *base, u16 codec_mask) +{ + azalia_custom_codecs_init(base, mainboard_azalia_codecs, codec_mask); +} +#endif void azalia_audio_init(struct device *dev) { diff --git a/src/include/device/azalia_device.h b/src/include/device/azalia_device.h index 21d00b0dae..66126d2e8c 100644 --- a/src/include/device/azalia_device.h +++ b/src/include/device/azalia_device.h @@ -20,11 +20,30 @@ #define AZALIA_MAX_CODECS 15 +/* + * The HDA specification refers to the vendor/device ID (similar to PCI IDs) + * collectively as "vendor ID". While this could be slightly confusing, we will + * call it "vendor ID" as the specification and most implementations do. + */ +struct azalia_codec { + const char *name; + u32 vendor_id; + u32 subsystem_id; + u8 address; + const u32 *verbs; + size_t verb_count; +}; + enum cb_err azalia_enter_reset(u8 *base); enum cb_err azalia_exit_reset(u8 *base); u32 azalia_find_verb(const u32 *verb_table, u32 verb_table_bytes, u32 viddid, const u32 **verb); int azalia_program_verb_table(u8 *base, const u32 *verbs, u32 verb_size); +#if CONFIG(AZALIA_USE_LEGACY_VERB_TABLE) void azalia_codec_init(u8 *base, int addr, const u32 *verb_table, u32 verb_table_bytes); +#else +void azalia_codec_init(u8 *base, struct azalia_codec *codec); +void azalia_custom_codecs_init(u8 *base, struct azalia_codec *codecs, u16 codec_mask); +#endif void azalia_codecs_init(u8 *base, u16 codec_mask); void azalia_audio_init(struct device *dev); extern struct device_operations default_azalia_audio_ops; @@ -32,8 +51,12 @@ extern struct device_operations default_azalia_audio_ops; /* Optional hook to program codec settings that are only known at runtime */ void mainboard_azalia_program_runtime_verbs(u8 *base, u32 viddid); +#if CONFIG(AZALIA_USE_LEGACY_VERB_TABLE) extern const u32 cim_verb_data[]; extern const u32 cim_verb_data_size; +#else +extern struct azalia_codec mainboard_azalia_codecs[]; +#endif extern const u32 pc_beep_verbs[]; extern const u32 pc_beep_verbs_size; @@ -161,9 +184,14 @@ enum azalia_pin_misc { (((association) << 4) & 0x000000f0) | \ (((sequence) << 0) & 0x0000000f)) +#if CONFIG(AZALIA_USE_LEGACY_VERB_TABLE) #define AZALIA_ARRAY_SIZES const u32 pc_beep_verbs_size = \ ARRAY_SIZE(pc_beep_verbs); \ const u32 cim_verb_data_size = sizeof(cim_verb_data) +#else +#define AZALIA_ARRAY_SIZES const u32 pc_beep_verbs_size = \ + ARRAY_SIZE(pc_beep_verbs) +#endif #define AZALIA_VERB_12B(codec, pin, verb, val) \ ((codec) << 28 | (pin) << 20 | (verb) << 8 | (val))