Attention is currently required from: Raul Rangel, Martin Roth, Furquan Shaikh, Marshall Dawson, Angel Pons, Andrey Petrov, Patrick Rudolph. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51575 )
Change subject: drivers/intel/fsp2_0/Kconfig: select HAVE_DISPLAY_MTRRS ......................................................................
Patch Set 2:
(2 comments)
Patchset:
PS2:
Felix, is this something to update and merge or abandon?
this patch improves things for socs using the fsp 2.x. didn't have the time to investigate if this can be enabled for all x86, but for that case i'd need to check if all x86 socs/cpus have code to actually print the mtrrs, since announcing to have functionality to display the mtrrs, but not actually having that, would be bad too.
i'd say that this can be merged as-is right now, since it fixes this for the fsp 2.x platforms
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/51575/comment/32c87606_c7fd55c9 PS2, Line 6: select HAVE_DISPLAY_MTRRS
Wouldn't a `depends on` clause be more appropriate? It's the AMD SoCs that should be selecting `HAVE […]
no, a depends on isn't the right thing to do here, since the fsp driver itself provides some functionality to print the mtrrs, so we can unhide the option to print the mtrrs with selecting this option in the fsp driver's Kconfig. if some component provides certain functionality that has a kconfig option to show that the is implemented, that component needs to select that option
-
Felix Held (Code Review)