Change in coreboot[master]: {drivers,soc}/intel: Move chipset specific logo handling to SoC
Wim Vervoorn has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel: Move chipset specific logo handling to SoC ......................................................................
{drivers,soc}/intel: Move chipset specific logo handling to SoC
FSP logo handling used FspsConfig.LogoPtr and FspsConfig.LogoSize which are chipset specific.
Create soc_load_logo() which will pass the logo pointer and size. This function will call fsp_load_logo which will load the logo.
BUG=NA TEST= Build and verified logo is displayed on Facebook Monolith
Change-Id: I30c7bdc0532ff8823e06f4136f210b542385d5ce Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/include/fsp/api.h M src/drivers/intel/fsp2_0/logo.c M src/drivers/intel/fsp2_0/silicon_init.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/chip.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/fsp_params.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/chip.c 10 files changed, 58 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/37792/1
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 824fd0b..ddf9ad7 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -156,10 +156,14 @@ is limited till EFI_PEI_MP_SERVICE_PPI and this option might be useful to add further PPI if required.
+config HAVE_FSP_LOGO_SUPPORT + bool + default n + config FSP2_0_DISPLAY_LOGO bool "Enable logo" default n - depends on HAVE_FSP_GOP + depends on HAVE_FSP_LOGO_SUPPORT help Uses the FSP to display the boot logo. This method supports a BMP file only. The uncompressed size can be up to 1 MB. The logo can be compressed diff --git a/src/drivers/intel/fsp2_0/include/fsp/api.h b/src/drivers/intel/fsp2_0/include/fsp/api.h index fb42c76..60adb98 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/api.h +++ b/src/drivers/intel/fsp2_0/include/fsp/api.h @@ -70,13 +70,15 @@ uint8_t fsp_memory_soc_version(void);
/* Load logo to be displayed by FSP */ -void load_logo(FSPS_UPD *supd); +const struct cbmem_entry *fsp_load_logo(UINT32 *logo_ptr, UINT32 *logo_size);
/* Callback after processing FSP notify */ void platform_fsp_notify_status(enum fsp_notify_phase phase);
/* Initialize memory margin analysis settings. */ void setup_mma(FSP_M_CONFIG *memory_cfg); +/* Update the SOC specific logo param and load the logo. */ +const struct cbmem_entry *soc_load_logo(FSPS_UPD *supd); /* Update the SOC specific memory config param for mma. */ void soc_update_memory_params_for_mma(FSP_M_CONFIG *memory_cfg, struct mma_config_param *mma_cfg); diff --git a/src/drivers/intel/fsp2_0/logo.c b/src/drivers/intel/fsp2_0/logo.c index feeec3b..ba2b5dc 100644 --- a/src/drivers/intel/fsp2_0/logo.c +++ b/src/drivers/intel/fsp2_0/logo.c @@ -11,17 +11,24 @@ * GNU General Public License for more details. */
-#include <soc/ramstage.h> -#include <console/console.h> +#include <cbfs.h> +#include <cbmem.h> #include <fsp/api.h> -#include <include/cbfs.h>
-void load_logo(FSPS_UPD *supd) +const struct cbmem_entry *fsp_load_logo(UINT32 *logo_ptr, UINT32 *logo_size) { - FSP_S_CONFIG *params = &supd->FspsConfig; + const struct cbmem_entry *logo_entry = NULL; + void *logo_buffer;
- params->LogoSize = cbfs_boot_load_file("logo.bmp", (void *)params->LogoPtr, - params->LogoSize, CBFS_TYPE_RAW); - if (!params->LogoSize) - params->LogoPtr = 0; + logo_entry = cbmem_entry_add(CBMEM_ID_FSP_LOGO, 1 * MiB); + if (logo_entry) { + logo_buffer = cbmem_entry_start(logo_entry); + if (logo_buffer) { + *logo_size = cbfs_boot_load_file("logo.bmp", (void *)logo_buffer, + 1 * MiB, CBFS_TYPE_RAW); + if (logo_size) + *logo_ptr = (UINT32)logo_buffer; + } + } + return (logo_entry); } diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index ebdbdbf..33d15af 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -34,7 +34,7 @@ fsp_silicon_init_fn silicon_init; uint32_t status; uint8_t postcode; - const struct cbmem_entry *logo_entry; + const struct cbmem_entry *logo_entry = NULL;
supd = (FSPS_UPD *) (hdr->cfg_region_offset + hdr->image_base);
@@ -57,14 +57,9 @@ /* Give SoC/mainboard a chance to populate entries */ platform_fsp_silicon_init_params_cb(upd);
-#if (CONFIG(HAVE_FSP_GOP)) - if (CONFIG(FSP2_0_DISPLAY_LOGO)) { - upd->FspsConfig.LogoSize = 1 * MiB; - logo_entry = cbmem_entry_add(CBMEM_ID_FSP_LOGO, upd->FspsConfig.LogoSize); - upd->FspsConfig.LogoPtr = (UINT32)cbmem_entry_start(logo_entry); - load_logo(upd); - } -#endif + /* Populate logo related entries */ + if (CONFIG(FSP2_0_DISPLAY_LOGO)) + logo_entry = soc_load_logo(upd);
/* Call SiliconInit */ silicon_init = (void *) (hdr->image_base + @@ -77,7 +72,7 @@ timestamp_add_now(TS_FSP_SILICON_INIT_END); post_code(POST_FSP_SILICON_EXIT);
- if (CONFIG(FSP2_0_DISPLAY_LOGO)) + if (logo_entry) cbmem_entry_remove(logo_entry);
fsp_debug_after_silicon_init(status); @@ -160,3 +155,9 @@ fsps_load(s3wake); do_silicon_init(&fsps_hdr); } + +/* Load bmp and set FSP parameters, fsp_load_logo can be used */ +__weak const struct cbmem_entry *soc_load_logo(FSPS_UPD *supd) +{ + return NULL; +} diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index a39765f..2f4ebb0 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -102,6 +102,7 @@ select HAVE_CF9_RESET_PREPARE select INTEL_GMA_ADD_VBT if RUN_FSP_GOP select HAVE_FSP_GOP + select HAVE_FSP_LOGO_SUPPORT select NO_UART_ON_SUPERIO select INTEL_GMA_ACPI select INTEL_GMA_SWSMISCI diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c index 6c195bb..c0192ae 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -879,4 +879,10 @@ printk(BIOS_DEBUG, "WEAK: %s/%s called\n", __FILE__, __func__); }
+/* Handle FSP logo params */ +const struct cbmem_entry *soc_load_logo(FSPS_UPD *supd) +{ + return(fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize)); +} + BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_ENTRY, spi_flash_init_cb, NULL); diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 8820508..26bae06 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -66,6 +66,7 @@ select FSP_M_XIP select GENERIC_GPIO_LIB select HAVE_FSP_GOP + select HAVE_FSP_LOGO_SUPPORT select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER select IDT_IN_EVERY_STAGE diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c index dfc7e22..63c5917 100644 --- a/src/soc/intel/cannonlake/fsp_params.c +++ b/src/soc/intel/cannonlake/fsp_params.c @@ -13,6 +13,7 @@ * GNU General Public License for more details. */
+#include <cbmem.h> #include <console/console.h> #include <device/device.h> #include <device/pci.h> @@ -483,3 +484,9 @@ *size = ARRAY_SIZE(serial_io_dev); return serial_io_dev; } + +/* Handle FSP logo params */ +const struct cbmem_entry *soc_load_logo(FSPS_UPD *supd) +{ + return(fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize)); +} diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 5fc2a2d..d90fb6b 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -33,6 +33,7 @@ select FSP_M_XIP select GENERIC_GPIO_LIB select HAVE_FSP_GOP + select HAVE_FSP_LOGO_SUPPORT select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER select INTEL_CAR_NEM_ENHANCED diff --git a/src/soc/intel/skylake/chip.c b/src/soc/intel/skylake/chip.c index de11a9e..896e7f5 100644 --- a/src/soc/intel/skylake/chip.c +++ b/src/soc/intel/skylake/chip.c @@ -15,6 +15,7 @@
#include <bootmode.h> #include <bootstate.h> +#include <cbmem.h> #include <fsp/api.h> #include <arch/acpi.h> #include <console/console.h> @@ -416,3 +417,9 @@ { printk(BIOS_DEBUG, "WEAK: %s/%s called\n", __FILE__, __func__); } + +/* Handle FSP logo params */ +const struct cbmem_entry *soc_load_logo(FSPS_UPD *supd) +{ + return(fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize)); +}
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel: Move chipset specific logo handling to SoC ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/apollolake/ch... File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/apollolake/ch... PS1, Line 885: return(fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize)); return is not a function, parentheses are not required
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/cannonlake/fs... PS1, Line 491: return(fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize)); return is not a function, parentheses are not required
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/skylake/chip.... File src/soc/intel/skylake/chip.c:
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/skylake/chip.... PS1, Line 424: return(fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize)); return is not a function, parentheses are not required
Hello Patrick Rudolph, Frans Hendriks,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37792
to look at the new patch set (#2).
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
{drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC
FSP logo handling used FspsConfig.LogoPtr and FspsConfig.LogoSize which are chipset specific.
Create soc_load_logo() which will pass the logo pointer and size. This function will call fsp_load_logo which will load the logo.
BUG=NA TEST= Build and verified logo is displayed on Facebook Monolith
Change-Id: I30c7bdc0532ff8823e06f4136f210b542385d5ce Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/include/fsp/api.h M src/drivers/intel/fsp2_0/logo.c M src/drivers/intel/fsp2_0/silicon_init.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/chip.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/fsp_params.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/chip.c 10 files changed, 58 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/37792/2
Hello Patrick Rudolph, Frans Hendriks, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37792
to look at the new patch set (#3).
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
{drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC
FSP logo handling used FspsConfig.LogoPtr and FspsConfig.LogoSize which are chipset specific.
Create soc_load_logo() which will pass the logo pointer and size. This function will call fsp_load_logo which will load the logo.
BUG=NA TEST= Build and verified logo is displayed on Facebook Monolith
Change-Id: I30c7bdc0532ff8823e06f4136f210b542385d5ce Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/include/fsp/api.h M src/drivers/intel/fsp2_0/logo.c M src/drivers/intel/fsp2_0/silicon_init.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/chip.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/fsp_params.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/chip.c 10 files changed, 58 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/37792/3
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/apollolake/ch... File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/apollolake/ch... PS1, Line 885: return(fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize));
return is not a function, parentheses are not required
Done
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/cannonlake/fs... PS1, Line 491: return(fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize));
return is not a function, parentheses are not required
Done
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/skylake/chip.... File src/soc/intel/skylake/chip.c:
https://review.coreboot.org/c/coreboot/+/37792/1/src/soc/intel/skylake/chip.... PS1, Line 424: return(fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize));
return is not a function, parentheses are not required
Done
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37792/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37792/4//COMMIT_MSG@13 PS4, Line 13: load the logo pass logo parameters to FSP?
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/Kc... PS4, Line 159: space
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/lo... File src/drivers/intel/fsp2_0/logo.c:
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/lo... PS4, Line 29: if (logo_size) : *logo_ptr = (UINT32)logo_buffer; else remove cbmem entry and set logo_entry=NULL?
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Frans Hendriks, Rizwan Qureshi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37792
to look at the new patch set (#5).
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
{drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC
FSP logo handling used FspsConfig.LogoPtr and FspsConfig.LogoSize which are chipset specific.
Create soc_load_logo() which will pass the logo pointer and size. This function will call fsp_load_logo which will load the logo.
BUG=NA TEST= Build and verified logo is displayed on Facebook Monolith
Change-Id: I30c7bdc0532ff8823e06f4136f210b542385d5ce Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/include/fsp/api.h M src/drivers/intel/fsp2_0/logo.c M src/drivers/intel/fsp2_0/silicon_init.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/chip.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/fsp_params.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/chip.c 10 files changed, 58 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/37792/5
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37792/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37792/4//COMMIT_MSG@13 PS4, Line 13: load the logo
pass logo parameters to FSP?
The soc_load_logo actually passes the pointer and length to the FSP. The fsp_load_logo will load the logo into cbmem and return the pointer and the length. So I think the text is correct.
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/Kc... PS4, Line 159:
space
Done
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/lo... File src/drivers/intel/fsp2_0/logo.c:
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/lo... PS4, Line 29: if (logo_size) : *logo_ptr = (UINT32)logo_buffer;
else remove cbmem entry and set logo_entry=NULL?
No. The logo_entry points to the allocated cbmem structure. As this is allocated it should not be set to NULL without removing the area first. Removing of this area is done by the calling routine anyhow.
The returned logo_pointer and logo_size are the pointer passed back to the fsp. If the logo is not available both are set to 0.
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37792/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37792/4//COMMIT_MSG@13 PS4, Line 13: load the logo
The soc_load_logo actually passes the pointer and length to the FSP. […]
Done
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/lo... File src/drivers/intel/fsp2_0/logo.c:
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/lo... PS4, Line 29: if (logo_size) : *logo_ptr = (UINT32)logo_buffer;
No. The logo_entry points to the allocated cbmem structure. […]
Ok, this is what I said for a invalid logo instance we can remove the cbmem entry here itself and set the logo_entry to NULL and not reserve it further. remove routine would find the logo_entry null and would not invoke the cbfs remove. thoughts?
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/lo... File src/drivers/intel/fsp2_0/logo.c:
https://review.coreboot.org/c/coreboot/+/37792/4/src/drivers/intel/fsp2_0/lo... PS4, Line 29: if (logo_size) : *logo_ptr = (UINT32)logo_buffer;
Ok, this is what I said for a invalid logo instance we can remove the cbmem entry here itself and se […]
It is of course possible to do this but the benefit is marginal as the cbmem entry will be removed directly after the fsp call.
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 6: Code-Review+2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 6: Code-Review+1
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 6: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
{drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC
FSP logo handling used FspsConfig.LogoPtr and FspsConfig.LogoSize which are chipset specific.
Create soc_load_logo() which will pass the logo pointer and size. This function will call fsp_load_logo which will load the logo.
BUG=NA TEST= Build and verified logo is displayed on Facebook Monolith
Change-Id: I30c7bdc0532ff8823e06f4136f210b542385d5ce Signed-off-by: Wim Vervoorn wvervoorn@eltan.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37792 Reviewed-by: Frans Hendriks fhendriks@eltan.com Reviewed-by: Aamir Bohra aamir.bohra@intel.com Reviewed-by: Subrata Banik subrata.banik@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/include/fsp/api.h M src/drivers/intel/fsp2_0/logo.c M src/drivers/intel/fsp2_0/silicon_init.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/chip.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/fsp_params.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/chip.c 10 files changed, 58 insertions(+), 21 deletions(-)
Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Aamir Bohra: Looks good to me, but someone else must approve Frans Hendriks: Looks good to me, approved
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 824fd0b..7ce7838 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -156,10 +156,14 @@ is limited till EFI_PEI_MP_SERVICE_PPI and this option might be useful to add further PPI if required.
+config HAVE_FSP_LOGO_SUPPORT + bool + default n + config FSP2_0_DISPLAY_LOGO bool "Enable logo" default n - depends on HAVE_FSP_GOP + depends on HAVE_FSP_LOGO_SUPPORT help Uses the FSP to display the boot logo. This method supports a BMP file only. The uncompressed size can be up to 1 MB. The logo can be compressed diff --git a/src/drivers/intel/fsp2_0/include/fsp/api.h b/src/drivers/intel/fsp2_0/include/fsp/api.h index fb42c76..60adb98 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/api.h +++ b/src/drivers/intel/fsp2_0/include/fsp/api.h @@ -70,13 +70,15 @@ uint8_t fsp_memory_soc_version(void);
/* Load logo to be displayed by FSP */ -void load_logo(FSPS_UPD *supd); +const struct cbmem_entry *fsp_load_logo(UINT32 *logo_ptr, UINT32 *logo_size);
/* Callback after processing FSP notify */ void platform_fsp_notify_status(enum fsp_notify_phase phase);
/* Initialize memory margin analysis settings. */ void setup_mma(FSP_M_CONFIG *memory_cfg); +/* Update the SOC specific logo param and load the logo. */ +const struct cbmem_entry *soc_load_logo(FSPS_UPD *supd); /* Update the SOC specific memory config param for mma. */ void soc_update_memory_params_for_mma(FSP_M_CONFIG *memory_cfg, struct mma_config_param *mma_cfg); diff --git a/src/drivers/intel/fsp2_0/logo.c b/src/drivers/intel/fsp2_0/logo.c index feeec3b..ba2b5dc 100644 --- a/src/drivers/intel/fsp2_0/logo.c +++ b/src/drivers/intel/fsp2_0/logo.c @@ -11,17 +11,24 @@ * GNU General Public License for more details. */
-#include <soc/ramstage.h> -#include <console/console.h> +#include <cbfs.h> +#include <cbmem.h> #include <fsp/api.h> -#include <include/cbfs.h>
-void load_logo(FSPS_UPD *supd) +const struct cbmem_entry *fsp_load_logo(UINT32 *logo_ptr, UINT32 *logo_size) { - FSP_S_CONFIG *params = &supd->FspsConfig; + const struct cbmem_entry *logo_entry = NULL; + void *logo_buffer;
- params->LogoSize = cbfs_boot_load_file("logo.bmp", (void *)params->LogoPtr, - params->LogoSize, CBFS_TYPE_RAW); - if (!params->LogoSize) - params->LogoPtr = 0; + logo_entry = cbmem_entry_add(CBMEM_ID_FSP_LOGO, 1 * MiB); + if (logo_entry) { + logo_buffer = cbmem_entry_start(logo_entry); + if (logo_buffer) { + *logo_size = cbfs_boot_load_file("logo.bmp", (void *)logo_buffer, + 1 * MiB, CBFS_TYPE_RAW); + if (logo_size) + *logo_ptr = (UINT32)logo_buffer; + } + } + return (logo_entry); } diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index ebdbdbf..33d15af 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -34,7 +34,7 @@ fsp_silicon_init_fn silicon_init; uint32_t status; uint8_t postcode; - const struct cbmem_entry *logo_entry; + const struct cbmem_entry *logo_entry = NULL;
supd = (FSPS_UPD *) (hdr->cfg_region_offset + hdr->image_base);
@@ -57,14 +57,9 @@ /* Give SoC/mainboard a chance to populate entries */ platform_fsp_silicon_init_params_cb(upd);
-#if (CONFIG(HAVE_FSP_GOP)) - if (CONFIG(FSP2_0_DISPLAY_LOGO)) { - upd->FspsConfig.LogoSize = 1 * MiB; - logo_entry = cbmem_entry_add(CBMEM_ID_FSP_LOGO, upd->FspsConfig.LogoSize); - upd->FspsConfig.LogoPtr = (UINT32)cbmem_entry_start(logo_entry); - load_logo(upd); - } -#endif + /* Populate logo related entries */ + if (CONFIG(FSP2_0_DISPLAY_LOGO)) + logo_entry = soc_load_logo(upd);
/* Call SiliconInit */ silicon_init = (void *) (hdr->image_base + @@ -77,7 +72,7 @@ timestamp_add_now(TS_FSP_SILICON_INIT_END); post_code(POST_FSP_SILICON_EXIT);
- if (CONFIG(FSP2_0_DISPLAY_LOGO)) + if (logo_entry) cbmem_entry_remove(logo_entry);
fsp_debug_after_silicon_init(status); @@ -160,3 +155,9 @@ fsps_load(s3wake); do_silicon_init(&fsps_hdr); } + +/* Load bmp and set FSP parameters, fsp_load_logo can be used */ +__weak const struct cbmem_entry *soc_load_logo(FSPS_UPD *supd) +{ + return NULL; +} diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index a39765f..2f4ebb0 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -102,6 +102,7 @@ select HAVE_CF9_RESET_PREPARE select INTEL_GMA_ADD_VBT if RUN_FSP_GOP select HAVE_FSP_GOP + select HAVE_FSP_LOGO_SUPPORT select NO_UART_ON_SUPERIO select INTEL_GMA_ACPI select INTEL_GMA_SWSMISCI diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c index 6c195bb..907149a 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -879,4 +879,10 @@ printk(BIOS_DEBUG, "WEAK: %s/%s called\n", __FILE__, __func__); }
+/* Handle FSP logo params */ +const struct cbmem_entry *soc_load_logo(FSPS_UPD *supd) +{ + return fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize); +} + BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_ENTRY, spi_flash_init_cb, NULL); diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 8820508..26bae06 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -66,6 +66,7 @@ select FSP_M_XIP select GENERIC_GPIO_LIB select HAVE_FSP_GOP + select HAVE_FSP_LOGO_SUPPORT select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER select IDT_IN_EVERY_STAGE diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c index dfc7e22..dc4a2a8 100644 --- a/src/soc/intel/cannonlake/fsp_params.c +++ b/src/soc/intel/cannonlake/fsp_params.c @@ -13,6 +13,7 @@ * GNU General Public License for more details. */
+#include <cbmem.h> #include <console/console.h> #include <device/device.h> #include <device/pci.h> @@ -483,3 +484,9 @@ *size = ARRAY_SIZE(serial_io_dev); return serial_io_dev; } + +/* Handle FSP logo params */ +const struct cbmem_entry *soc_load_logo(FSPS_UPD *supd) +{ + return fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize); +} diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 5fc2a2d..d90fb6b 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -33,6 +33,7 @@ select FSP_M_XIP select GENERIC_GPIO_LIB select HAVE_FSP_GOP + select HAVE_FSP_LOGO_SUPPORT select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER select INTEL_CAR_NEM_ENHANCED diff --git a/src/soc/intel/skylake/chip.c b/src/soc/intel/skylake/chip.c index de11a9e..8e86156 100644 --- a/src/soc/intel/skylake/chip.c +++ b/src/soc/intel/skylake/chip.c @@ -15,6 +15,7 @@
#include <bootmode.h> #include <bootstate.h> +#include <cbmem.h> #include <fsp/api.h> #include <arch/acpi.h> #include <console/console.h> @@ -416,3 +417,9 @@ { printk(BIOS_DEBUG, "WEAK: %s/%s called\n", __FILE__, __func__); } + +/* Handle FSP logo params */ +const struct cbmem_entry *soc_load_logo(FSPS_UPD *supd) +{ + return fsp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize); +}
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37792/7/src/drivers/intel/fsp2_0/lo... File src/drivers/intel/fsp2_0/logo.c:
https://review.coreboot.org/c/coreboot/+/37792/7/src/drivers/intel/fsp2_0/lo... PS7, Line 34: } This looks much like the function in fsp1_1/, aren't they identical? I also don't see anything FSP specific here (beside the Windows-API style types), so I guess it belongs into lib/.
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37792 )
Change subject: {drivers,soc}/intel/fsp2_0: Move chipset specific logo handling to SoC ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37792/7/src/drivers/intel/fsp2_0/lo... File src/drivers/intel/fsp2_0/logo.c:
https://review.coreboot.org/c/coreboot/+/37792/7/src/drivers/intel/fsp2_0/lo... PS7, Line 34: }
This looks much like the function in fsp1_1/, aren't they identical? […]
You are right that the code is exactly the same and the only difference is in the includes. Still I don't think it is a good idea to move this to lib as this functionality is only relevant for the fsp implementations. It is not generic code. Also I don't expect the FSP 1.1 code to be around forever and once that goes away the fsp 2 code is the only location.
-
Aamir Bohra (Code Review) -
build bot (Jenkins) (Code Review)
-
Frans Hendriks (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Subrata Banik (Code Review)
-
Wim Vervoorn (Code Review)