Attention is currently required from: Nico Huber, 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)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51575/comment/a5df74ef_fa6ed061 PS1, Line 7: drivers/intel/fsp2_0: move selection of HAVE_DISPLAY_MTRRS to FSP driver
I guess we could simply guard the debug option with some ARCH_X86. I never […]
the code works on the fsp-based amd platforms; haven't checked if the mtrr printing code looks at the additional bits that get set when X86_AMD_FIXED_MTRRS is selected, but it wouldn't be really wrong not to look at those when just printing the mtrrs
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/51575/comment/5f269b38_df12c49f PS2, Line 6: select HAVE_DISPLAY_MTRRS
no, a depends on isn't the right thing to do here, since the fsp driver itself provides some funct […]
ah, you're right, the fsp driver doesn't provide that functionality. what i probably intended to write was that the fsp driver has a lot of calls to display_mtrrs that only do something when DISPLAY_MTRRS is selected which is guarded by HAVE_DISPLAY_MTRRS, so we should select HAVE_DISPLAY_MTRRS at least for all platforms hat use the fsp driver. did a quick git grep display_mtrrs and it's also called in postcar, so there's also a call to that for non-fsp platforms. i don't think that we have any x86 platform in tree that doesn't support mtrrs, so we can probably remove the HAVE_DISPLAY_MTRRS option completely
-
Felix Held (Code Review)