Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38461 )
Change subject: soc/intel/tigerlake: Update FSP params for Jasper Lake ......................................................................
Patch Set 24:
(9 comments)
https://review.coreboot.org/c/coreboot/+/38461/24//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38461/24//COMMIT_MSG@10 PS24, Line 10: - graphics
I would capitalize each element, as well as the acronyms: […]
Done
https://review.coreboot.org/c/coreboot/+/38461/24//COMMIT_MSG@16 PS24, Line 16: etc.
nit: I would omit the "etc"
Done
https://review.coreboot.org/c/coreboot/+/38461/24//COMMIT_MSG@18 PS24, Line 18: These are the initial settings for JSL
super-nit: Period at the end.
Done
https://review.coreboot.org/c/coreboot/+/38461/24//COMMIT_MSG@24 PS24, Line 24: jasper lake
Or, if this Jasper Lake board is google/dedede, maybe use: […]
Done
https://review.coreboot.org/c/coreboot/+/38461/24/src/soc/intel/tigerlake/ch... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/38461/24/src/soc/intel/tigerlake/ch... PS24, Line 208: enum {
Is this is a bitfield? If so, I would use shifts. […]
Agree. Done.
https://review.coreboot.org/c/coreboot/+/38461/24/src/soc/intel/tigerlake/fs... File src/soc/intel/tigerlake/fsp_params_jsl.c:
https://review.coreboot.org/c/coreboot/+/38461/24/src/soc/intel/tigerlake/fs... PS24, Line 114: 1
Shouldn't this mirror the value above?
want to explicitly set it for S3 flow.
https://review.coreboot.org/c/coreboot/+/38461/24/src/soc/intel/tigerlake/ro... File src/soc/intel/tigerlake/romstage/fsp_params_jsl.c:
https://review.coreboot.org/c/coreboot/+/38461/24/src/soc/intel/tigerlake/ro... PS24, Line 32: disable
disable*d*
Done
https://review.coreboot.org/c/coreboot/+/38461/24/src/soc/intel/tigerlake/ro... PS24, Line 46: pci
PCI
Done
https://review.coreboot.org/c/coreboot/+/38461/24/src/soc/intel/tigerlake/ro... PS24, Line 85: Vt-D
VT-d
Done