[coreboot] [PATCH] sb600: new way to load initial verb
Bao, Zheng
Zheng.Bao at amd.com
Fri Mar 12 03:23:17 CET 2010
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: coreboot-bounces at coreboot.org
[mailto:coreboot-bounces at coreboot.org]
> On Behalf Of Bao, Zheng
> Sent: Tuesday, March 09, 2010 11:02 AM
> To: coreboot at coreboot.org
> Subject: Re: [coreboot] [PATCH] sb600: new way to load initial verb
>
>
>
> > -----Original Message-----
> > From: Carl-Daniel Hailfinger
> [mailto:c-d.hailfinger.devel.2006 at gmx.net]
> > Sent: Monday, March 08, 2010 10:15 PM
> > To: Bao, Zheng
> > Cc: Stefan Reinauer; coreboot at coreboot.org
> > 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 <zheng.bao at amd.com>
> > >
> > > 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: coreboot at coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
More information about the coreboot
mailing list