Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35880 )
Change subject: kbl boards / fsp2.0: remove redundant CdClock setting ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Naresh, do you remember why you explicitly set this for the KBLRVP?
I'm not sure what FSP version I was using when working on this. But I guess the HW/Display needed this frequency.
For reference : FSP master:(Kaby Lake FSP 3.6.0 ) https://github.com/IntelFsp/FSP/blob/master/KabylakeFspBinPkg/Include/FspsUp...
FSP older version (Kaby Lake FSP 3.1.0) https://github.com/IntelFsp/FSP/blob/16d143d63854938fceec60b762ae1b25aa48dbf...
It's not even clear to me why this UPD exists. Is it a work- around for the GOP because it can't decide the frequency based on the display mode it sets?
If these boards need the explicit `CdClock = 3` for proper FSP/GOP operation, I would leave the code as is. Maybe with a comment, why that is necessary.
OTOH, if we'd move the `CdClock = 3` to soc/ code, we could just guard it with `if (CONFIG(USE_FSP_GOP))`. Maybe even add an `else CdClock = 0;` if we'd want to test that. It should work with any reasonable gfx driver and would save some power if we don't initialize the display early.
sounds resonable; I will update this CB
\o/ I just found some similiar approch in icelake... we should use that :D