Attention is currently required from: Jason Glenesk, Raul Rangel, Matt DeVillier, Fred Reitberger, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73231 )
Change subject: soc/amd/*/acpi: don't write unused p_lvl[2,3]_lat FADT fields ......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/cezanne/acpi.c:
https://review.coreboot.org/c/coreboot/+/73231/comment/72cbd6dd_9913ea6d PS1, Line 79: ACPI_FADT_C2_NOT_SUPPORTED
the fadt struct is zero'd and prepopulated before we populate it further in here. […]
So you think that having the latency set to 0 here is the correct option?
I'd say that we should add a comment saying that it should be overwritten by the CST and that if it isn't, it's not supported. Any other value the we put in here (other than *possibly* saying that it's supported, and that we're putting in the longest possible latency because we don't know) doesn't make sense either.
As far as what is being done on the intel platforms, I don't think that's the best thing to go by either unless there have been changes that you can look back to with reasons for what is being done currently.
So if we want to change this value, make an argument for why it should be 0 or some other value. To me, it seems like leaving the way it currently is, and adding a comment saying that it's overridden by the CST is probably the best way to go.