[coreboot-gerrit] Change in coreboot[master]: drivers/intel/fsp2_0: Add provision to extract FSP Performance Data

[Subrata Banik (Code Review)]

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Paul Menzel, Nick Vaccaro, Eric Lai, Andrey Petrov.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62942 )

Change subject: drivers/intel/fsp2_0: Add provision to extract FSP Performance Data
......................................................................

Patch Set 5:

(2 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/62942/comment/4486d8e4_92326ea5 
PS2, Line 9: This patch enriches coreboot FSP2.0 driver to extract the FSP timestamp
           : from FPDT (Firmware Performance Data Table) and display as part of
           : ramstage boot stage if the SoC user selects the required
           : `GET_FSP_TIMESTAMP` config.
> > Extracting this data sounds great. But instead of printing it to the CBMEM console, could you please add them to the CBMEM timestamp table?
>
> we already have FSP TS data as part of cbmem -t table. what this CL does is to have detailed break down of the FSP timestamp which might be useful to debug or optimise the FSP per component boot time.
> > Is the FSP data theoretically also accessible from the operating sytsem?
>
> Not sure, if I understood your comments, but cbmem -t is able to show the FSP API wise boot time impact. where this CL is more deep dive into those FSP APIs.
>
> 950:calling FspMemoryInit                             1,936,192 (59,275)
>  951:returning from FspMemoryInit                      81,024,105 (79,087,913)
>  971:loading FSP-S                                     82,941,127 (21,318)
>  954:calling FspSiliconInit                            83,166,997 (219,188)
>  955:returning from FspSiliconInit                     102,564,474 (19,397,476)
>  962:calling FspMultiPhaseSiInit                       102,564,627 (152)
>  963:returning from FspMultiPhaseSiInit                108,645,466 (6,080,839)

@Paul, any further opens ?

File src/drivers/intel/fsp2_0/fsp_timestamp.c:

https://review.coreboot.org/c/coreboot/+/62942/comment/8670e100_77f80b18 
PS4, Line 111: 
             : BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_LOAD, BS_ON_EXIT, fsp_get_timestamp, NULL);
             : BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, fsp_get_timestamp, NULL);
> Instead of boot state entries, can we just add call fsp_print_timestamps() from somewhere in the flo […]

Ack