[coreboot] Questions about more AMD related flags

[xdrudis]

On Fri, Nov 05, 2010 at 08:11:28PM +0100, Patrick Georgi wrote:
> SET_FIDVID*:
> These have _very_ weird behaviour, being set to some defaults in the two
> init_cpus.c (and fidvid.c seems to expect to be included after that
> one?), and some other settings somewhere else.
> I tried to untangle that while moving to Kconfig, but didn't quite
> succeed, so is here anyone who knows which defaults (or dependencies)
> are correct for each of the following? So far I have:
>

I've been playing with fidvid.c a little for my single FAM10 cpu (4
cores) slowly at my scarce free time.

I'm quite ignorant and the only result I got has been a consistent
hang in prep_fid_vid after warm reset with my changes, and without 
them it sometimes hanged around there and often later.

So don't quite believe me but 
> 
> For Fam10h:
> +config SET_FIDVID
> +       bool
> +       default y
> +

This enables or disables all cpu and northbridge frecuency and voltage
settings required by BKDG . In order to comply with BKDG it should be
on for all FAM10.  I believe it is set to 1 in all FAM10 boards ?

> +config SET_FIDVID_CORE0_ONLY
> +       bool
> +       default n
> +       depends on SET_FIDVID
> +

I remember I didn't realise for a while that SET_FIDVID_CORE0_ONLY
was off but it was, so I agree it was weird to me, but I thought it 
was just me. There are things in the BKDG that need to be done 
for each core, not just core0, so I decided it 
was rightly off. I'd say FAM10 needs  SET_FIDVID_CORE0_ONLY
to off irrespective of board.

For example, 
fixPsNbVidBeforeWR() in src/cpu/amd/model_10xxx/fidvid.c 
implements steps 1-6 of BKDG 2.4.2.9.1 in case of 
needing nb vid update. The BKDG #31116 rev 3.48 April 2010
says the pstate MSRs need writes for all cores.
The BSP core 0 will always do it, because mainboard/.../romstage.c
calls init_fidvid_bsp(). But the other cores wouldn't 
do it if SET_FIDVID_CORE0_ONLY was on (init_cpus() would call
init_fidvid_ap for core0 of nodes other than BSP but not 
for cores 1,2,3,... of any node)  

Is this what you were asking ?

> +config SET_FIDVID_CORE_RANGE
> +       bool
> +       default n
> +       depends on SET_FIDVID
> +

bool ? I Only saw it used in init_fidvid_bsp as the second 
parameter to for_each_ap, and it does:

if 1 iterates over core0 in nodes >0
if 2 iterates over cores 1 .. max (skips core0 of each node)
else iterates over all cores in all nodes (except bsp core 0)

this iterations wait for the cores to give their results and 
then calculates the common fid (frequency Id for the
northbridge, wich must be the minimum i.e. slowest required for
all processors). I guess one can check for each core or for
one core in each processor (node). BKDG says 

   "F3xD4[NbFid] must be matched between all processors in the coherent
   fabric of a multi-socket sys- tem. The lowest setting from all
   processors in a multi-socket system (determined by using the following
   equa- tions on each processor and selecting the lowest value) is used
   as the common NbFid. "

So I would suppose you don't really need to check all cores in 
a processor. If they share the northbridge how could they
require different frequencies for it ?. But this is only with
respect to the frecuency. Maybe you want to wait for all simply
to make sure you already set the NBVid before you set the NbFid?
I believe for tilapia_fam10 you already have waited 
at wait_all_other_cores_started  before calling init_fidvid_bsp,
so you could have SET_FIDVID_CORE_RANGE to 1 and save a little.
I think it's always 0, though.

> +config SET_FIDVID_DEBUG
> +       bool
> +       default y
> +       depends on SET_FIDVID
> +

For me it's ok at 1 . For FAM10 at least it is only use to enable
traces for fidvid code (frequency and voltage setting).


> +config SET_FIDVID_STORE_AP_APICID_AT_FIRST
> +       bool
> +       default y
> +       depends on SET_FIDVID
> 
> 

model_10xx/fidvid.c 
// if we are tight of CAR stack, disable it
#define SET_FIDVID_STORE_AP_APICID_AT_FIRST 1

I don't fully understand the advantage of enabling it.
Apparently it only enables a table of core apicids 
that is first built and then read to wait for each core
to tell you its nbfid. Why can't you directly iterate
once and wait and calculate the nbfid (which is what
would happen if this was 0) ?.
The other difference is that setting it to 0 would
make it not use SET_FIDVID_CORE_RANGE, and iterate 
always for all cores that have calculated fid, 
(in fact iterate for all cores
if SET_FIDVID_CORE0_ONLY is 0 as it should, or only
cores 0 if SET_FIDVID_CORE0_ONLY is 1 against BKDG). 

I don't know about fam f, it uses a different fidvid.c .