| Submitter | Bao, Zheng |
|---|---|
| Date | 2010-03-05 09:29:49 |
| Message ID | <DD1CC71B621B004FA76856E5129D6B17038260ED@sbjgexmb1.amd.com> |
| Download | mbox | patch |
| Permalink | /patch/1009/ |
| State | Rejected |
| Headers | show |
Comments
On 3/5/10 10:29 AM, Bao, Zheng wrote: > Don't load verb in BIOS stage. It is not BIOS's responsibility. > Who is responsible for that, then? On i945 I definitely don't get sound on most systems if I don't load the verb in coreboot. I also don't think the Linux drivers do it. > And we can not have verb for every single codec. > Have a look at the i82801gx southbridge; I think it was based on the sb600 code, but I changed it so I can have a per mainboard verb table. In mainboard_enable() in mainboard.c, call verb_setup() which does: #include "hda_verb.h" static void verb_setup(void) { cim_verb_data = mainboard_cim_verb_data; cim_verb_data_size = sizeof(mainboard_cim_verb_data); } and hda_verb.c looks like this: static u32 mainboard_cim_verb_data[] = { /* coreboot specific header */ 0x10ec0262, // Codec Vendor ID / Device ID 0x10714700, // Subsystem ID 0x0000000d, // Number of jacks /* HDA Codec Subsystem ID Verb Table: 0x10ec0000 */ 0x00172000, 0x00172100, 0x001722EC, 0x00172310, /* Pin Widget Verb Table */ /* Pin Complex (NID 0x12) */ 0x01271CF0, 0x01271D11, 0x01271E11, ... }; extern u32 * cim_verb_data; extern u32 cim_verb_data_size;
Stefan Reinauer wrote: > On 3/5/10 10:29 AM, Bao, Zheng wrote: >> Don't load verb in BIOS stage. It is not BIOS's responsibility. >> > Who is responsible for that, then? Those verbs are motherboard specific as they describe the actual external connections so they should not be in southbridge code. They cannot be inferred by the driver so they absolutely should be done by coreboot. So they should be moved to motherboard specific code not dropped entirely. Andrew
On 05.03.2010 11:13, Stefan Reinauer wrote: > On 3/5/10 10:29 AM, Bao, Zheng wrote: > >> Don't load verb in BIOS stage. It is not BIOS's responsibility. >> >> > Who is responsible for that, then? > > On i945 I definitely don't get sound on most systems if I don't load the > verb in coreboot. I also don't think the Linux drivers do it. > >> And we can not have verb for every single codec. >> >> > Have a look at the i82801gx southbridge; I think it was based on the > sb600 code, but I changed it so I can have a per mainboard verb table. > I think we should try to mirror the behaviour of the vendor BIOS. The Linux kernel source has some interesting info about HD Audio: http://www.mjmwired.net/kernel/Documentation/sound/alsa/HD-Audio.txt Regards, Carl-Daniel
Patch
Index: src/southbridge/amd/sb600/sb600_hda.c =================================================================== --- src/southbridge/amd/sb600/sb600_hda.c (revision 5184) +++ src/southbridge/amd/sb600/sb600_hda.c (working copy) @@ -90,88 +90,10 @@ return 0; } -static u32 cim_verb_data[] = { - 0x01471c10, - 0x01471d40, - 0x01471e01, - 0x01471f01, -/* 1 */ - 0x01571c12, - 0x01571d10, - 0x01571e01, - 0x01571f01, -/* 2 */ - 0x01671c11, - 0x01671d60, - 0x01671e01, - 0x01671f01, -/* 3 */ - 0x01771c14, - 0x01771d20, - 0x01771e01, - 0x01771f01, -/* 4 */ - 0x01871c30, - 0x01871d90, - 0x01871ea1, - 0x01871f01, -/* 5 */ - 0x01971cf0, - 0x01971d11, - 0x01971e11, - 0x01971f41, -/* 6 */ - 0x01a71c80, - 0x01a71d30, - 0x01a71e81, - 0x01a71f01, -/* 7 */ - 0x01b71cf0, - 0x01b71d11, - 0x01b71e11, - 0x01b71f41, -/* 8 */ - 0x01c71cf0, - 0x01c71d11, - 0x01c71e11, - 0x01c71f41, -/* 9 */ - 0x01d71cf0, - 0x01d71d11, - 0x01d71e11, - 0x01d71f41, -/* 10 */ - 0x01e71c50, - 0x01e71d11, - 0x01e71e44, - 0x01e71f01, -/* 11 */ - 0x01f71c60, - 0x01f71d61, - 0x01f71ec4, - 0x01f71f01, -}; -static u32 find_verb(u32 viddid, u32 ** verb) -{ - device_t azalia_dev = dev_find_slot(0, PCI_DEVFN(0x14, 2)); - struct southbridge_amd_sb600_config *cfg = - (struct southbridge_amd_sb600_config *)azalia_dev->chip_info; - printk_debug("Dev=%s\n", dev_path(azalia_dev)); - printk_debug("Default viddid=%x\n", cfg->hda_viddid); - printk_debug("Reading viddid=%x\n", viddid); - if (!cfg) - return 0; - if (viddid != cfg->hda_viddid) - return 0; - *verb = (u32 *) cim_verb_data; - return sizeof(cim_verb_data) / sizeof(u32); -} - /** * Wait 50usec for for the codec to indicate it is ready * no response would imply that the codec is non-operative */ - static int wait_for_ready(u8 *base) {
Don't load verb in BIOS stage. It is not BIOS's responsibility. And we can not have verb for every single codec. Signed-off-by: Zheng Bao <zheng.bao@amd.com> /* Use a 50 usec timeout - the Linux kernel uses the @@ -194,7 +116,6 @@ * the previous command. No response would imply that the code * is non-operative */ - static int wait_for_valid(u8 *base) { /* Use a 50 usec timeout - the Linux kernel uses the @@ -215,9 +136,6 @@ static void codec_init(u8 * base, int addr) { u32 dword; - u32 *verb; - u32 verb_size; - int i; /* 1 */ if (wait_for_ready(base) == -1) @@ -232,26 +150,7 @@ dword = read32(base + 0x64); /* 2 */ - printk_debug("codec viddid: %08x\n", dword); - verb_size = find_verb(dword, &verb); - - if (!verb_size) { - printk_debug("No verb!\n"); - return; - } - - printk_debug("verb_size: %d\n", verb_size); - /* 3 */ - for (i = 0; i < verb_size; i++) { - if (wait_for_ready(base) == -1) - return; - - write32(base + 0x60, verb[i]); - - if (wait_for_valid(base) == -1) - return; - } - printk_debug("verb loaded!\n"); + printk_debug("%x(th) codec viddid: %08x\n", addr, dword); } static void codecs_init(u8 * base, u32 codec_mask)