[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