[M] Change in coreboot[master]: cpu/intel/speedstep: Use acpigen_write_processor_device()

[Nico Huber (Code Review)]

Attention is currently required from: Arthur Heymans, Kyösti Mälkki.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74441 )

Change subject: cpu/intel/speedstep: Use acpigen_write_processor_device()
......................................................................

Patch Set 4:

(1 comment)

File src/southbridge/intel/i82801gx/fadt.c:

https://review.coreboot.org/c/coreboot/+/74441/comment/c6d34d4b_dd8c6436 
PS4, Line 29: 	fadt->p_lvl3_lat = chip->c3_latency;
> > I wouldn't say the current implementation is right. But if we want to drop
> > Processor(), shouldn't we first have an alternative ready?
>
> Despite the removal of [WIP] I have no rush to get this in. Future OSPM still has to parse deprecated Processor(), right? If not, then this patch is better than nothing.

I guess that finding an OS in the future that still has support for (now)
15 years old machines and a brand new OSPM might be difficult. Also, general
purpose OS's would likely keep Processor() support as long as they want to
support such old machines because most of them won't get firmware updates.
IOW sometimes, not updating firmware can be more compatible. Because nobody
expects it.

But I'm not concerned about switching to _CST etc. That's been in OS's for
a long time already.
> > In any case, we shouldn't leave dead code around. AFAICT, this was the only
> > consumer of `c3_latency`.
>
> We also have _CST entries without C3, and _CST entries with C3 where latency does not match with the devicetree... Elsewhere I also popped the question, if it is valid to evaluate BSP MSR's to fill in AP _CST entries.

Probably not unconditionally. There can be boards with broken, low-power
c-states. So there should be an opt-out. And there's also a tradeoff
question, on mobile devices we might want the lowest power states, but
probably not on desktops.