Re: [coreboot] patch to display ASRock E350M1 freq correctly

[Marc Jones]

Keeping this on the list.

On Wed, Oct 5, 2011 at 7:03 PM, mopz0506 mopz0506  wrote:
> I'm very surprised too.
>
> I'm not familiar with coreboot code base, but seems all SB800-based
>  system are running hardcoded codepath and ignores a lot of Agesa code.
> There are so many indirection and everytime you call the same
> function it may do different thing, depends on "The service desired",
> and the callback.
>
> Don't have serial port/cable so what I can do is guessing.
>
> so below is what happened, I guess.
>
> in agesawrapper_amdinitlate (), AmdLateParams has been filled
> with 0 (And AmdLateParams.StdHeader.HeapBasePtr).
>
> so we must set "AmdLateParams.PlatformConfig.UserOptionPstate"
> to "TRUE", tell Agesa that we want a PState table.
>
> then call AmdInitLate(&AmdLateParams) and let Agesa to build
>  SSDT for us.
>
> AmdInitLate() call GenerateSsdt () to do the real work.
>
> GenerateSsdt () call HeapLocateBuffer() to get the location of
> PState data, which Agesa prepared for us before.
>
> unlucky, HeapLocateBuffer() requires (NULL != StdHeader->HeapBasePtr),
> here StdHeader is &AmdLateParams.StdHead.
>
> since AmdLateParams has been initialized to 0, aka. the stdheader.HeapBasePtr
>  is NULL, the HeapLocateBuffer() failed and return AGESA_ERROR.
>
> so, all those pointer, *DmiTable, *AcpiPstate, *AcpiSrat, *AcpiSlit,
> *AcpiWheaMce, *AcpiWheaCmc, *AcpiAlib, always being NULL.
>
> since AcpiPstate is NULL, write_acpi_tables() always skip Agesa and
> fallback to hardcoded data and ACPI setting.
>
> we must set the AmdLateParams.StdHeader to a valid HeapBasePtr to fetch
> Agesa's PState data.
>
> but seems HeapManager is for Agesa internal only. Tried HeapGetBaseAddress()
> the computer frozen. I know nothing about programming on such raw hardware
> and don't know why.
>
> I checked agesawrapper.c from other mainboards and found nothing.
>
> how could I get a valid HeapBasePtr to call HeapLocateBuffer() and
> HeapAllocateBuffer()?
>
> Any hints?
>
> Thanks,
> mopz0506
>
> On Wed, Oct 5, 2011 at 12:24 AM, Marc Jones  wrote:
> > On Tue, Oct 4, 2011 at 1:38 AM, Rudolf Marek  wrote:
> > > Hi all,
> > > > I'm not native speaker and sorry for my English.
> > >
> > > Yep me too ;) But your doing OK we can understand what you wrote.
> > > > Finally I think maybe I can fix it myself and start
> > > > digging ACPI and source code.
> > >
> > > I'm quite surprised that this is hardcoded. We used to have a functions
> > > which place it there runtime depending on CPU model. I don't know
> > > why it is hardcoded here. I think a proper fix would be to provide that
> > > runtime depending on CPU. Maybe AGESA has some infrastructure for that. Did
> > > not check so far.
> > >
> > > Thanks
> > > Rudolf
> >
> > This should be generated by the AMDInitLate call, which needs some
> > work in coreboot acpi/agesa wrappers. Feel free to check out the
> > tables that are generated and look at how they should be integrated.
> >
> > Marc