Could it be a GSOC project? 1. Make an application to dump the codec structure and initial pin configuration. 2. In Coreboot repository, add a public code to load initial pin configuration as a CBFS modules.
Is it too small? I kinda don't have much time to do this. Zheng > -----Original Message----- > From: [email protected] [mailto:[email protected]] > On Behalf Of Bao, Zheng > Sent: Tuesday, March 09, 2010 11:02 AM > To: [email protected] > Subject: Re: [coreboot] [PATCH] sb600: new way to load initial verb > > > > > -----Original Message----- > > From: Carl-Daniel Hailfinger > [mailto:[email protected]] > > Sent: Monday, March 08, 2010 10:15 PM > > To: Bao, Zheng > > Cc: Stefan Reinauer; [email protected] > > Subject: Re: [coreboot] [PATCH] sb600: new way to load initial verb > > > > On 08.03.2010 09:20, Bao, Zheng wrote: > > > The original way to load initial verb is quite inflexible. > > > The format of the command is: > > > CAd 31:28 - codec address > > > NID 27:20 - Nid > > > Verb 19:0 - Verb data > > > Each Nid has 4 byte data to write. It has to write one at a time. > > > 0x01471c10, //1st byte 10 > > > 0x01471d40, //2nd byte 40 > > > 0x01471e01, //3rd byte 01 > > > 0x01471f01, //4th byte 01 > > > > > > The new code in sb600_hda.c and the code structure are quite > > > preliminary. When it is almost done and can satisfy everybody, We > can > > > define the pin configuration code at mainboard. > > > > > > > It would be cool if all chipsets could use the new verb > implementation. > > > > > > > #define CODEC_PIN_CONFIG_FILE "codec/alc_882.h" > > > or > > > config CODEC_PIN_CONFIG_FILE > > > string > > > default "codec/alc_882.h" > > > depends on BOARD_AMD_DBM690T > > > > > > > Or we define that file as separate uncompressed CBFS file. > > > > > > > Now I am wondering if it is legal provide the intial pin > configuration > > > code in coreboot. If it is legal, how can we get it? The 882 code is > > > got from CIM. > > > > > > > Sorry, what is CIM? > > In theory, the pin configuration is mainboard specific. If that is > true, > > the pin configuration is similar to IRQ routing: There is usually only > > one correct and working solution. So it is possible that the mainboard > > vendor and the codec vendor own parts of it. Not sure. We could tell > > users to dump verbs from their mainboards (is that possible?) and add > > the resulting file to CBFS. > > > It is mainboard specific. Each codec also has its own code. The > mainboard code is based on the codec code, I think. Dumping verbs is > possible. It needs a lot of work from the ground up. There are some > similar tools like http://helllabs.org/codecgraph/. Integrating them to > coreboot/util needs permission, doesn't it? > > > > > > Signed-off-by: Zheng Bao <[email protected]> > > > > > > Index: src/codec/alc_882.h > > > =================================================================== > > > --- src/codec/alc_882.h (revision 0) > > > +++ src/codec/alc_882.h (revision 0) > > > @@ -0,0 +1,12 @@ > > > > > > > struct _pin_config_codec_entry pin_config_file[] = { > > > > > + {0x14, 0x01014010}, > > > + {0x15, 0x01011012}, > > > + {0x16, 0x01016011}, > > > + {0x17, 0x01012014}, > > > + {0x18, 0x01A19030}, > > > + {0x19, 0x411111F0}, > > > + {0x1a, 0x01813080}, > > > + {0x1b, 0x411111F0}, > > > + {0x1C, 0x411111F0}, > > > + {0x1d, 0x411111F0}, > > > + {0x1e, 0x01441150}, > > > + {0x1f, 0x01C46160}, > > > > > > > {-1, -1} > > }; > > > > > > > Index: src/southbridge/amd/sb600/sb600_hda.c > > > =================================================================== > > > --- src/southbridge/amd/sb600/sb600_hda.c (revision 5195) > > > +++ src/southbridge/amd/sb600/sb600_hda.c (working copy) > > > @@ -90,68 +90,19 @@ > > > 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, > > > +typedef struct _pin_config_codec_entry { > > > > > > > Can you please kill the typedef and use "struct > _pin_config_codec_entry" > > instead? > > > > > > > + u8 nid; > > > + u32 pin_config; > > > +} pin_config_codec_entry; > > > + > > > +static pin_config_codec_entry pin_config_file[] = { > > > +#ifdef CODEC_PIN_CONFIG_FILE > > > + #include CODEC_PIN_CONFIG_FILE > > > > > > > Including C source makes code difficult to read. Can you move the > > complete struct to alc_882.h? > > > > Hm. Could we simply store the whole verb stuff uncompressed inside > CBFS? > > > > I think the purpose of the CBFS is to integrate some binary files which > are built at other place. The verb data we got is in text mode. We need > to build it into binary and store it into CBFS. Is it wasting? Why don't > we do it as fam10 patch code? > > > > > > +#endif > > > + {-1, -1} > > > }; > > > -static u32 find_verb(u32 viddid, u32 ** verb) > > > + > > > +static void find_verb(u32 viddid, pin_config_codec_entry ** verb) > > > { > > > device_t azalia_dev = dev_find_slot(0, PCI_DEVFN(0x14, 2)); > > > struct southbridge_amd_sb600_config *cfg = > > > @@ -159,12 +110,13 @@ > > > 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); > > > + > > > + *verb = NULL; > > > if (!cfg) > > > - return 0; > > > + return; > > > if (viddid != cfg->hda_viddid) > > > - return 0; > > > - *verb = (u32 *) cim_verb_data; > > > - return sizeof(cim_verb_data) / sizeof(u32); > > > + return; > > > + *verb = pin_config_file; > > > } > > > > > > /** > > > @@ -214,9 +166,8 @@ > > > > > > static void codec_init(u8 * base, int addr) > > > { > > > - u32 dword; > > > - u32 *verb; > > > - u32 verb_size; > > > + u32 dword, dword1, pin_config; > > > + pin_config_codec_entry *verb=NULL; > > > int i; > > > > > > /* 1 */ > > > @@ -233,23 +184,32 @@ > > > > > > /* 2 */ > > > printk_debug("codec viddid: %08x\n", dword); > > > - verb_size = find_verb(dword, &verb); > > > + find_verb(dword, &verb); > > > > > > - if (!verb_size) { > > > + if (verb == NULL) { > > > 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; > > > + for (verb; verb->nid != 0xFF; verb++) { > > > + dword = addr << 28 | verb->nid << 20 | 7 << 16; > > > + pin_config = verb->pin_config; > > > > > > - write32(base + 0x60, verb[i]); > > > + for (i = 4; i > 0; i--, pin_config >>= 8) { > > > + if (wait_for_ready(base) == -1) > > > + return; > > > > > > - if (wait_for_valid(base) == -1) > > > - return; > > > + if (verb->nid != 1) > > > + dword1 = dword | ((0x20 - i) & 0xFF) << > 8; > > > + else > > > + dword1 = dword | ((0x24 - i) & 0xFF) << > 8; > > > + dword1 |= (pin_config & 0xFF); > > > + write32(base + 0x60, dword1); > > > + > > > + if (wait_for_valid(base) == -1) > > > + return; > > > + } > > > } > > > printk_debug("verb loaded!\n"); > > > } > > > > > > > Looks good. > > > > Regards, > > Carl-Daniel > > > > -- > > "I do consider assignment statements and pointer variables to be among > > computer science's most valuable treasures." > > -- Donald E. Knuth > > > > > > -- > coreboot mailing list: [email protected] > http://www.coreboot.org/mailman/listinfo/coreboot -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
