Change in coreboot[master]: {driver,soc}/intel: Move chipset specific logo handling to soc
Wim Vervoorn has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37791 ) Change subject: {driver,soc}/intel: Move chipset specific logo handling to soc ...................................................................... {driver,soc}/intel: Move chipset specific logo handling to soc FSP logo handling used PcdLogoPtr and PcdLogoSize which are elements of the chipset specific FSP structures. 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 FBG1701 Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Signed-off-by: Wim Vervoorn <wvervoorn@eltan.com> --- M src/drivers/intel/fsp1_1/include/fsp/ramstage.h M src/drivers/intel/fsp1_1/logo.c M src/drivers/intel/fsp1_1/ramstage.c M src/soc/intel/braswell/chip.c 4 files changed, 34 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37791/1 diff --git a/src/drivers/intel/fsp1_1/include/fsp/ramstage.h b/src/drivers/intel/fsp1_1/include/fsp/ramstage.h index a5eac0e..e50edd8 100644 --- a/src/drivers/intel/fsp1_1/include/fsp/ramstage.h +++ b/src/drivers/intel/fsp1_1/include/fsp/ramstage.h @@ -26,6 +26,7 @@ /* Perform Intel silicon init. */ void intel_silicon_init(void); void fsp_run_silicon_init(FSP_INFO_HEADER *fsp_info_header, int is_s3_wakeup); +const struct cbmem_entry *fsp_load_logo(UINT32 *logo_ptr, UINT32 *logo_size); /* Called after the silicon init code has run. */ void soc_after_silicon_init(void); /* Initialize UPD data before SiliconInit call. */ @@ -33,7 +34,7 @@ void mainboard_silicon_init_params(SILICON_INIT_UPD *params); void soc_display_silicon_init_params(const SILICON_INIT_UPD *old, SILICON_INIT_UPD *new); -void load_logo(SILICON_INIT_UPD *params); +const struct cbmem_entry *soc_load_logo(SILICON_INIT_UPD *params); void load_vbt(uint8_t s3_resume, SILICON_INIT_UPD *params); #endif /* _INTEL_COMMON_RAMSTAGE_H_ */ diff --git a/src/drivers/intel/fsp1_1/logo.c b/src/drivers/intel/fsp1_1/logo.c index 03b2715..23aad01 100644 --- a/src/drivers/intel/fsp1_1/logo.c +++ b/src/drivers/intel/fsp1_1/logo.c @@ -11,15 +11,24 @@ * GNU General Public License for more details. */ +#include <cbfs.h> +#include <cbmem.h> #include <soc/ramstage.h> -#include <console/console.h> -#include <fsp/ramstage.h> -#include <include/cbfs.h> -void load_logo(SILICON_INIT_UPD *params) +const struct cbmem_entry *fsp_load_logo(UINT32 *logo_ptr, UINT32 *logo_size) { - params->PcdLogoSize = cbfs_boot_load_file("logo.bmp", (void *)params->PcdLogoPtr, - params->PcdLogoSize, CBFS_TYPE_RAW); - if (!params->PcdLogoSize) - params->PcdLogoPtr = 0; + const struct cbmem_entry *logo_entry = NULL; + void *logo_buffer; + + 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/fsp1_1/ramstage.c b/src/drivers/intel/fsp1_1/ramstage.c index 9ecdfd6..40e79cc 100644 --- a/src/drivers/intel/fsp1_1/ramstage.c +++ b/src/drivers/intel/fsp1_1/ramstage.c @@ -69,7 +69,7 @@ EFI_STATUS status; UPD_DATA_REGION *upd_ptr; VPD_DATA_REGION *vpd_ptr; - const struct cbmem_entry *logo_entry; + const struct cbmem_entry *logo_entry = NULL; /* Display the FSP header */ if (fsp_info_header == NULL) { @@ -96,13 +96,8 @@ load_vbt(is_s3_wakeup, &silicon_init_params); mainboard_silicon_init_params(&silicon_init_params); - if (CONFIG(FSP1_1_DISPLAY_LOGO) && !is_s3_wakeup) { - silicon_init_params.PcdLogoSize = 1 * MiB; - logo_entry = cbmem_entry_add(CBMEM_ID_FSP_LOGO, - silicon_init_params.PcdLogoSize); - silicon_init_params.PcdLogoPtr = (UINT32)cbmem_entry_start(logo_entry); - load_logo(&silicon_init_params); - } + if (CONFIG(FSP1_1_DISPLAY_LOGO) && !is_s3_wakeup) + logo_entry = soc_load_logo(&silicon_init_params); /* Display the UPD data */ if (CONFIG(DISPLAY_UPD_DATA)) @@ -122,7 +117,7 @@ printk(BIOS_DEBUG, "FspSiliconInit returned 0x%08x\n", status); /* The logo_entry can be freed up now as it is not required any longer */ - if (CONFIG(FSP1_1_DISPLAY_LOGO) && !is_s3_wakeup) + if (logo_entry && !is_s3_wakeup) cbmem_entry_remove(logo_entry); /* Mark graphics init done after SiliconInit if VBT was provided */ @@ -214,3 +209,9 @@ __weak void soc_silicon_init_params(SILICON_INIT_UPD *params) { } + +/* Load bmp and set FSP parameters, fsp_load_logo can be used */ +__weak const struct cbmem_entry *soc_load_logo(SILICON_INIT_UPD *params) +{ + return NULL; +} diff --git a/src/soc/intel/braswell/chip.c b/src/soc/intel/braswell/chip.c index d179cea..026b281 100644 --- a/src/soc/intel/braswell/chip.c +++ b/src/soc/intel/braswell/chip.c @@ -181,6 +181,11 @@ board_silicon_USB2_override(params); } +const struct cbmem_entry *soc_load_logo(SILICON_INIT_UPD *params) +{ + return fsp_load_logo(¶ms->PcdLogoPtr, ¶ms->PcdLogoSize); +} + void soc_display_silicon_init_params(const SILICON_INIT_UPD *old, SILICON_INIT_UPD *new) { -- To view, visit https://review.coreboot.org/c/coreboot/+/37791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Gerrit-Change-Number: 37791 Gerrit-PatchSet: 1 Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-MessageType: newchange
Hello Patrick Rudolph, Huang Jin, Frans Hendriks, Lee Leahy, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37791 to look at the new patch set (#2). 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 PcdLogoPtr and PcdLogoSize which are elements of the chipset specific FSP structures. 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 FBG1701 Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Signed-off-by: Wim Vervoorn <wvervoorn@eltan.com> --- M src/drivers/intel/fsp1_1/include/fsp/ramstage.h M src/drivers/intel/fsp1_1/logo.c M src/drivers/intel/fsp1_1/ramstage.c M src/soc/intel/braswell/chip.c 4 files changed, 34 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37791/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/37791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Gerrit-Change-Number: 37791 Gerrit-PatchSet: 2 Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Huang Jin <huang.jin@intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
Hello Patrick Rudolph, Huang Jin, Frans Hendriks, Lee Leahy, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37791 to look at the new patch set (#3). Change subject: {drivers,soc}/intel/fsp1_1: Move chipset specific logo handling to SoC ...................................................................... {drivers,soc}/intel/fsp1_1: Move chipset specific logo handling to SoC FSP logo handling used PcdLogoPtr and PcdLogoSize which are elements of the chipset specific FSP structures. 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 FBG1701 Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Signed-off-by: Wim Vervoorn <wvervoorn@eltan.com> --- M src/drivers/intel/fsp1_1/include/fsp/ramstage.h M src/drivers/intel/fsp1_1/logo.c M src/drivers/intel/fsp1_1/ramstage.c M src/soc/intel/braswell/chip.c 4 files changed, 34 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37791/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/37791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Gerrit-Change-Number: 37791 Gerrit-PatchSet: 3 Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Huang Jin <huang.jin@intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37791 ) Change subject: {drivers,soc}/intel/fsp1_1: Move chipset specific logo handling to SoC ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/37791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Gerrit-Change-Number: 37791 Gerrit-PatchSet: 3 Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra@intel.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Huang Jin <huang.jin@intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 19 Dec 2019 08:34:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37791 ) Change subject: {drivers,soc}/intel/fsp1_1: Move chipset specific logo handling to SoC ...................................................................... {drivers,soc}/intel/fsp1_1: Move chipset specific logo handling to SoC FSP logo handling used PcdLogoPtr and PcdLogoSize which are elements of the chipset specific FSP structures. 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 FBG1701 Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Signed-off-by: Wim Vervoorn <wvervoorn@eltan.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37791 Reviewed-by: Frans Hendriks <fhendriks@eltan.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org> --- M src/drivers/intel/fsp1_1/include/fsp/ramstage.h M src/drivers/intel/fsp1_1/logo.c M src/drivers/intel/fsp1_1/ramstage.c M src/soc/intel/braswell/chip.c 4 files changed, 34 insertions(+), 18 deletions(-) Approvals: build bot (Jenkins): Verified Frans Hendriks: Looks good to me, approved diff --git a/src/drivers/intel/fsp1_1/include/fsp/ramstage.h b/src/drivers/intel/fsp1_1/include/fsp/ramstage.h index a5eac0e..e50edd8 100644 --- a/src/drivers/intel/fsp1_1/include/fsp/ramstage.h +++ b/src/drivers/intel/fsp1_1/include/fsp/ramstage.h @@ -26,6 +26,7 @@ /* Perform Intel silicon init. */ void intel_silicon_init(void); void fsp_run_silicon_init(FSP_INFO_HEADER *fsp_info_header, int is_s3_wakeup); +const struct cbmem_entry *fsp_load_logo(UINT32 *logo_ptr, UINT32 *logo_size); /* Called after the silicon init code has run. */ void soc_after_silicon_init(void); /* Initialize UPD data before SiliconInit call. */ @@ -33,7 +34,7 @@ void mainboard_silicon_init_params(SILICON_INIT_UPD *params); void soc_display_silicon_init_params(const SILICON_INIT_UPD *old, SILICON_INIT_UPD *new); -void load_logo(SILICON_INIT_UPD *params); +const struct cbmem_entry *soc_load_logo(SILICON_INIT_UPD *params); void load_vbt(uint8_t s3_resume, SILICON_INIT_UPD *params); #endif /* _INTEL_COMMON_RAMSTAGE_H_ */ diff --git a/src/drivers/intel/fsp1_1/logo.c b/src/drivers/intel/fsp1_1/logo.c index 03b2715..23aad01 100644 --- a/src/drivers/intel/fsp1_1/logo.c +++ b/src/drivers/intel/fsp1_1/logo.c @@ -11,15 +11,24 @@ * GNU General Public License for more details. */ +#include <cbfs.h> +#include <cbmem.h> #include <soc/ramstage.h> -#include <console/console.h> -#include <fsp/ramstage.h> -#include <include/cbfs.h> -void load_logo(SILICON_INIT_UPD *params) +const struct cbmem_entry *fsp_load_logo(UINT32 *logo_ptr, UINT32 *logo_size) { - params->PcdLogoSize = cbfs_boot_load_file("logo.bmp", (void *)params->PcdLogoPtr, - params->PcdLogoSize, CBFS_TYPE_RAW); - if (!params->PcdLogoSize) - params->PcdLogoPtr = 0; + const struct cbmem_entry *logo_entry = NULL; + void *logo_buffer; + + 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/fsp1_1/ramstage.c b/src/drivers/intel/fsp1_1/ramstage.c index 9ecdfd6..40e79cc 100644 --- a/src/drivers/intel/fsp1_1/ramstage.c +++ b/src/drivers/intel/fsp1_1/ramstage.c @@ -69,7 +69,7 @@ EFI_STATUS status; UPD_DATA_REGION *upd_ptr; VPD_DATA_REGION *vpd_ptr; - const struct cbmem_entry *logo_entry; + const struct cbmem_entry *logo_entry = NULL; /* Display the FSP header */ if (fsp_info_header == NULL) { @@ -96,13 +96,8 @@ load_vbt(is_s3_wakeup, &silicon_init_params); mainboard_silicon_init_params(&silicon_init_params); - if (CONFIG(FSP1_1_DISPLAY_LOGO) && !is_s3_wakeup) { - silicon_init_params.PcdLogoSize = 1 * MiB; - logo_entry = cbmem_entry_add(CBMEM_ID_FSP_LOGO, - silicon_init_params.PcdLogoSize); - silicon_init_params.PcdLogoPtr = (UINT32)cbmem_entry_start(logo_entry); - load_logo(&silicon_init_params); - } + if (CONFIG(FSP1_1_DISPLAY_LOGO) && !is_s3_wakeup) + logo_entry = soc_load_logo(&silicon_init_params); /* Display the UPD data */ if (CONFIG(DISPLAY_UPD_DATA)) @@ -122,7 +117,7 @@ printk(BIOS_DEBUG, "FspSiliconInit returned 0x%08x\n", status); /* The logo_entry can be freed up now as it is not required any longer */ - if (CONFIG(FSP1_1_DISPLAY_LOGO) && !is_s3_wakeup) + if (logo_entry && !is_s3_wakeup) cbmem_entry_remove(logo_entry); /* Mark graphics init done after SiliconInit if VBT was provided */ @@ -214,3 +209,9 @@ __weak void soc_silicon_init_params(SILICON_INIT_UPD *params) { } + +/* Load bmp and set FSP parameters, fsp_load_logo can be used */ +__weak const struct cbmem_entry *soc_load_logo(SILICON_INIT_UPD *params) +{ + return NULL; +} diff --git a/src/soc/intel/braswell/chip.c b/src/soc/intel/braswell/chip.c index d179cea..026b281 100644 --- a/src/soc/intel/braswell/chip.c +++ b/src/soc/intel/braswell/chip.c @@ -181,6 +181,11 @@ board_silicon_USB2_override(params); } +const struct cbmem_entry *soc_load_logo(SILICON_INIT_UPD *params) +{ + return fsp_load_logo(¶ms->PcdLogoPtr, ¶ms->PcdLogoSize); +} + void soc_display_silicon_init_params(const SILICON_INIT_UPD *old, SILICON_INIT_UPD *new) { -- To view, visit https://review.coreboot.org/c/coreboot/+/37791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Gerrit-Change-Number: 37791 Gerrit-PatchSet: 4 Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra@intel.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Huang Jin <huang.jin@intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: merged
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37791 ) Change subject: {drivers,soc}/intel/fsp1_1: Move chipset specific logo handling to SoC ...................................................................... Patch Set 4: (2 comments) https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/lo... File src/drivers/intel/fsp1_1/logo.c: https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/lo... PS4, Line 29: if (logo_size) Should this be `*logo_size`? A null-pointer check after dereference seems odd. https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/ra... File src/drivers/intel/fsp1_1/ramstage.c: https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/ra... PS4, Line 217: } I doubt this will ever be used. -- To view, visit https://review.coreboot.org/c/coreboot/+/37791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Gerrit-Change-Number: 37791 Gerrit-PatchSet: 4 Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra@intel.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Huang Jin <huang.jin@intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 31 Dec 2019 15:23:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37791 ) Change subject: {drivers,soc}/intel/fsp1_1: Move chipset specific logo handling to SoC ...................................................................... Patch Set 4: (2 comments) https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/lo... File src/drivers/intel/fsp1_1/logo.c: https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/lo... PS4, Line 29: if (logo_size)
Should this be `*logo_size`? A null-pointer check after dereference seems odd. Thanks for locating this issue. TI will create a patch to correct this issue.
https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/ra... File src/drivers/intel/fsp1_1/ramstage.c: https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/ra... PS4, Line 217: }
I doubt this will ever be used. It is used in the Quark X1000 case. This doesn't provide the option to pass in a logo. Other than that you are right. I don't expect any new FSP1.1 based chipsets and the Braswell is covered.
-- To view, visit https://review.coreboot.org/c/coreboot/+/37791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Gerrit-Change-Number: 37791 Gerrit-PatchSet: 4 Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra@intel.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Huang Jin <huang.jin@intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 02 Jan 2020 11:07:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37791 ) Change subject: {drivers,soc}/intel/fsp1_1: Move chipset specific logo handling to SoC ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/ra... File src/drivers/intel/fsp1_1/ramstage.c: https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/ra... PS4, Line 217: }
It is used in the Quark X1000 case. This doesn't provide the option to pass in a logo. […] Quark is using FSP2.0 only now. I'll just let Jenkins test what happens if I remove this :)
-- To view, visit https://review.coreboot.org/c/coreboot/+/37791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Gerrit-Change-Number: 37791 Gerrit-PatchSet: 4 Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra@intel.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Huang Jin <huang.jin@intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 02 Jan 2020 14:35:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Wim Vervoorn <wvervoorn@eltan.com> Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment
Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37791 ) Change subject: {drivers,soc}/intel/fsp1_1: Move chipset specific logo handling to SoC ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/ra... File src/drivers/intel/fsp1_1/ramstage.c: https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/ra... PS4, Line 217: }
Quark is using FSP2.0 only now. […] In this case 124 to 126 and 129 should be removed as well.
-- To view, visit https://review.coreboot.org/c/coreboot/+/37791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Gerrit-Change-Number: 37791 Gerrit-PatchSet: 4 Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra@intel.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Huang Jin <huang.jin@intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 02 Jan 2020 14:45:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Wim Vervoorn <wvervoorn@eltan.com> Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37791 ) Change subject: {drivers,soc}/intel/fsp1_1: Move chipset specific logo handling to SoC ...................................................................... Patch Set 4: (1 comment) https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/ra... File src/drivers/intel/fsp1_1/ramstage.c: https://review.coreboot.org/c/coreboot/+/37791/4/src/drivers/intel/fsp1_1/ra... PS4, Line 217: }
In this case 124 to 126 and 129 should be removed as well. Right... and I just realized that we can theoretically drop all the weak functions (if we assume that there won't be any new platforms with FSP1.1).
-- To view, visit https://review.coreboot.org/c/coreboot/+/37791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86943e64ca1ddd05e7e88fc6b882cfd33b98272e Gerrit-Change-Number: 37791 Gerrit-PatchSet: 4 Gerrit-Owner: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra@intel.com> Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com> Gerrit-Reviewer: Huang Jin <huang.jin@intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy@intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Wim Vervoorn <wvervoorn@eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 02 Jan 2020 16:11:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Wim Vervoorn <wvervoorn@eltan.com> Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment
participants (4)
-
Frans Hendriks (Code Review) -
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Wim Vervoorn (Code Review)