[coreboot] [ patch] v3 Geode graphics init.
Marc Jones
Marc.Jones at AMD.com
Fri Feb 22 00:42:07 CET 2008
Carl-Daniel Hailfinger wrote:
> On 21.02.2008 23:06, Marc Jones wrote:
>> Marc Jones wrote:
>>
>>> Carl-Daniel Hailfinger wrote:
>>>> On 21.02.2008 02:54, Marc Jones wrote:
>>>>
>>>>> Carl-Daniel Hailfinger wrote:
>>>>>
>>>>>> On 21.02.2008 01:09, Marc Jones wrote:
>>>>>>
>>>>>>> ram_resource(dev, idx++, 1024,
>>>>>>> - (get_systop() - (1 * 1024 * 1024)) / 1024);
>>>>>>> + (get_systop(nb_pci) - (1 * 1024 * 1024)) / 1024);
>>>>>>> }
>>>>>>>
>>>>> 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)
>>>
>>>
>>>
>> There is a bug here. get_systop returns KB. It is already in KB so it
>> doesn't need the /1024 and the 1MB doesn't need * 1024.
>> The correct line is :
>> ram_resource(dev, idx++, 1024,
>> (get_systop(nb_pci) - 1024);
>>
>>
>> I will check v2.
>>
>
> Thanks.
>
>
>>>>> +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 */
>>>>>
>
> If get_systop really returns kBytes, the comment above (/* 4K */) does
> not match the code which adds 4*1024 K = 4 M.
Argh! This is all messed up. Something happened from v2 to v3.
get_systop should return bytes. Not KB. I'll get this sorted out.
Marc
--
Marc Jones
Senior Firmware Engineer
(970) 226-9684 Office
mailto:Marc.Jones at amd.com
http://www.amd.com/embeddedprocessors
More information about the coreboot
mailing list