Re: [coreboot] Enabling SpeedStep for all Denverton SoC variants

[Julien Viard de Galbert]

> Le 29 mai 2018 à 16:48, Sumo  a écrit :
>
> Hi,
>
> Today denverton_core_init() is enabling the Intel SpeedStep only if turbo mode is available. As a result, the SpeedStep is not enable for the C3558 variant (and others).  Any clues of why it was implemented this way?
> I think it is safe to remove the "if (get_turbo_state() == TURBO_ENABLED)" test and always enable the SpeedStep technology.

You are right, I also think it’s safe. (The variant I have access to have Turbo so I can’t test it).
I also have no clue, but it was like this since the first denverton commit… 
So please test and submit a patch ;)

Best Regads
Julien
> Thanks,
> Sumo

Enabling SpeedStep for all Denverton SoC variants

[Sumo]

Thanks Julien! I´ll submit a patch to Gerrit soon.

BTW, have you noticed that Linux is always showing an additional CPU in
the /sys/devices/system/cpu/ folder?
Here, for a quad-core CPU I have the following:
pinelake:~ # ll /sys/devices/system/cpu/cpu*
total 0
drwxr-xr-x 9 root root    0 May 29 22:46 cpu0
drwxr-xr-x 4 root root    0 May 29 22:46 cpu1
drwxr-xr-x 9 root root    0 May 29 22:46 cpu2
drwxr-xr-x 9 root root    0 May 29 22:46 cpu3
drwxr-xr-x 9 root root    0 May 29 22:46 cpu4
pinelake:~ # cat /sys/devices/system/node/node0/cpumap
1d
pinelake:~ # cat /sys/devices/system/node/node0/cpulist
0,2-4

And during the Linux boot I see the following error:
[   10.116007] smpboot: do_boot_cpu failed(-1) to wakeup CPU#1

Looks like the issue in my case is caused by a wrong LAPIC value for the
BSP CPU in the devicetree.cb... The coreboot log shows:
IOAPIC: Bootstrap Processor Local APIC = 0x04
But in the devicetree.cb we have:
device cpu_cluster 0 on
device lapic 0 on end
end

If I change the lapic value from 0 to 4 then Linux is not complaining
about do_boot_cpu
failed... and sysfs shows the correct amount of CPU´s:
pinelake:~ # ll /sys/devices/system/cpu/cpu*
total 0
drwxr-xr-x 9 root root    0 May 29 22:46 cpu0
drwxr-xr-x 4 root root    0 May 29 22:46 cpu1
drwxr-xr-x 9 root root    0 May 29 22:46 cpu2
drwxr-xr-x 9 root root    0 May 29 22:46 cpu3
linux-80my:~ # cat /sys/devices/system/node/node0/cpumap
f
linux-80my:~ # cat /sys/devices/system/node/node0/cpulist
0-3

Please let me know if this happens to you and if the same fix can be
applied!

Thanks,
Sumo

2018-06-04 9:46 GMT-03:00 Julien Viard de Galbert <
[e-mail]>:
> Le 29 mai 2018 à 16:48, Sumo  a écrit :
>
> Hi,
>
> Today denverton_core_init() is enabling the Intel SpeedStep only if turbo
> mode is available. As a result, the SpeedStep is not enable for the C3558
> variant (and others).  Any clues of why it was implemented this way?
>
> I think it is safe to remove the "if (get_turbo_state() == TURBO_ENABLED)"
> test and always enable the SpeedStep technology.
>
> You are right, I also think it’s safe. (The variant I have access to have
> Turbo so I can’t test it).
> I also have no clue, but it was like this since the first denverton
> commit…
> So please test and submit a patch ;)
>
> Best Regads
> Julien
>
> Thanks,
> Sumo