[M] Change in coreboot[main]: soc/intel/alderlake: Add a function to force disable memory channels

[Kapil Porwal (Code Review)]

Attention is currently required from: Angel Pons, Christian Walter, Dinesh Gehlot, Eric Lai, Jamie Chen, Jayvik Desai, Jeremy Soller, Lean Sheng Tan, Michał Kopeć, Michał Żygowski, Nick Vaccaro, Piotr Król, Sean Rhodes, Subrata Banik, Tim Crawford.

Kapil Porwal has posted comments on this change by Kapil Porwal. ( https://review.coreboot.org/c/coreboot/+/85529?usp=email )

Change subject: soc/intel/alderlake: Add a function to force disable memory channels
......................................................................

Patch Set 2:

(5 comments)

File src/soc/intel/alderlake/meminit.c:

https://review.coreboot.org/c/coreboot/+/85529/comment/405f6b22_e91d85c4?usp... :
PS1, Line 235:
> can you please write a help text for easy explanation of mem_init_override_channel_mask and ch_disab […]

Acknowledged

https://review.coreboot.org/c/coreboot/+/85529/comment/c7c2dd62_46fe2e05?usp... :
PS1, Line 236: static void mem_init_override_channel_mask(FSP_M_CONFIG *mem_cfg,
             : 		uint32_t ch_disable_mask)
>

Acknowledged

https://review.coreboot.org/c/coreboot/+/85529/comment/4e6c32c5_3f41db65?usp... :
PS1, Line 252: 	if (ch_disable_mask == 0) {
> > Do we have any invalid combination here? Like we must have Mc0Ch0 at least? Or you cann't disable  […]

Agreed. Added a check for Mc0Ch0.

@[e-mail] could you please let us know if there are any other constraints?

https://review.coreboot.org/c/coreboot/+/85529/comment/673d828e_987a0dfd?usp... :
PS1, Line 264: void memcfg_init(FSPM_UPD *memupd, const struct mb_cfg *mb_cfg,
             : 		 const struct mem_spd *spd_info, bool half_populated,
             : 		 uint32_t ch_disable_mask)
> How about: […]

There are multiple ways to implement the solution -
1. patchset#1 - This way is consistent with how we pass data from mainboard code to soc APIs but it involves changes to all the ADL based mainbaords/variants.
2. Override API guarded with Kconfig option (suggested by Subrata) - It keeps the SoC layer API intact and involve changes to the concerned variant only.
3. Create multiple prototypes of memcfg_init API (suggested by Angel) - It changes the SoC layer API along with changes to the concerned mainboard & variant.
4. Export the new function and directly call it from mainboard/variant code (suggested by Angel) - It Adds one more API along with changes to the concerned mainboard & variant.

I like #2 & #4, but #2 keeps the changes to minimal.

Please share your thoughts.

https://review.coreboot.org/c/coreboot/+/85529/comment/cd228f7d_0374a57e?usp... :
PS1, Line 315: 	mem_init_override_channel_mask(mem_cfg, ch_disable_mask);
> Could this be called at the very end of the function? If so, you could export this function (make it non-static) and call it from mainboards.

Yes, it is doable. Please refer the comment above.