Change in coreboot[master]: soc/intel/jsl: Add CdClock config

[Angel Pons (Code Review)]

Attention is currently required from: Jamie Chen, Henry Sun, Tim Wawrzynczak, Paul Menzel, Ren Kuo, Simon Yang, Kane Chen, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60009 )

Change subject: soc/intel/jsl: Add CdClock config
......................................................................

Patch Set 16:

(4 comments)

File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/60009/comment/53e99b47_e5db9faf 
PS16, Line 208: # Core Display Clock Frequency selection
              : 	register "CdClock" = "0xff"
With the changes I've suggested in my other comments, this is no longer needed.

File src/soc/intel/jasperlake/chip.h:

https://review.coreboot.org/c/coreboot/+/60009/comment/ef71ade3_933d8627 
PS16, Line 411: 0xff: 648 MHz
Looks like 0xff is the default setting (a value of 7 also specifies 648 MHz). However, the devicetree values default to 0 if unspecified, so 172.8 MHz CdClock would be used as default instead. One way to avoid this is to make the devicetree values +1 and handle a value of 0 as a special case. See comments in fsp_params.c on how to implement this.

https://review.coreboot.org/c/coreboot/+/60009/comment/b2296532_8c182529 
PS16, Line 408: /*
              : 	 * Core Display Clock Frequency selection
              : 	 * 0: 172.8 MHz, 1: 180 MHz, 2: 192 MHz, 3: 307 MHz, 4: 312 MHz, 5: 552 MHz,
              : 	 * 6: 556.8 MHz, 7: 648 MHz, 8: 652.8 MHz, 0xff: 648 MHz
              : 	 */
              : 	uint8_t CdClock;
Why not use an enum?

/* Core Display Clock Frequency selection, FSP values + 1 */
 enum {
    CD_CLOCK_172_8_MHZ = 1,
    CD_CLOCK_180_MHZ   = 2,
    CD_CLOCK_192_MHZ   = 3,
    CD_CLOCK_307_MHZ   = 4,
    CD_CLOCK_312_MHZ   = 5,
    CD_CLOCK_552_MHZ   = 6,
    CD_CLOCK_556_8_MHZ = 7,
    CD_CLOCK_648_MHZ   = 8,
    CD_CLOCK_652_8_MHZ = 9,
 } cd_clock;

Note that I've added 1 to the values to allow having a non-zero default value, see my other comments. Also note that I've shortened the comment, as the enum values' names already specify the corresponding frequency.

File src/soc/intel/jasperlake/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/60009/comment/024c30ef_95a71312 
PS16, Line 212: params->CdClock = config->CdClock;
To handle non-zero default values:

params->CdClock = config->cd_clock ? config->cd_clock - 1 : 0xff;

Note that I've renamed the devicetree setting to `cd_clock`, as it no longer has a 1:1 mapping to the `CdClock` FSP UPD.