[coreboot] [ patch] v3 Geode graphics init.
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Fri Feb 22 00:11:16 CET 2008
On 21.02.2008 17:29, Marc Jones wrote:
> Carl-Daniel Hailfinger wrote:
>> On 21.02.2008 02:54, Marc Jones wrote:
>>> Carl-Daniel Hailfinger wrote:
>>>> As the comment say. 0-640K is claimed prior to the 1MB to systop.
>>>
>>> Sorry, I still don't understand although I'm really trying hard.
>>> Maybe my comment was misplaced and thus misunderstood.
>>> The first ram_resource claims 0 - 640k.
>>> The second ram_resource claims 1M - (systop-1M). Where's that
>>> hardcoded missing 1 MB at the top of RAM going?
>>>
>>>
> systop-1M is the size.
> static void ram_resource(device_t dev, unsigned long index,
> unsigned long basek, unsigned long sizek)
OK, thanks.
>> One more complaint (probably a genuine bug):
>>
>>> +u32 get_systop(struct northbridge_amd_geodelx_pci_config *nb_pci)
>>> +{
>>> + struct gliutable *gl = NULL;
>>> + u32 systop;
>>> + struct msr msr;
>>> + int i;
>>> +
>>> + for (i = 0; gliu0table[i].desc_name != GL_END; i++) {
>>> + if (gliu0table[i].desc_type == R_SYSMEM) {
>>> + gl = &gliu0table[i];
>>> + break;
>>> + }
>>> + }
>>> + if (gl) {
>>> + msr = rdmsr(gl->desc_name);
>>> + systop = ((msr.hi & 0xFF) << 24) | ((msr.lo & 0xFFF00000)
>>> >> 8);
>>> + systop += 4 * 1024; /* 4K */
>>> + } else {
>>> + systop =
>>
>> systop is the address of top of usable RAM in Bytes.
>>
>>> + ((sizeram() - nb_pci->geode_video_mb) * 1024) -
>>> SMM_SIZE - 1024;
>>
>> sizeram() and geode_video_mb are in MBytes, yet they only get
>> multiplied by 1024.
>> A factor of 1024 is missing here.
>
> Real bug. Fortunately we never take that path. I will fix it in this
> patch for v3 and submit a patch for v2.
Thanks for fixing.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list