coreboot-gerrit August 2023

coreboot-gerrit@coreboot.org

1 participants
6717 discussions

[XS] Change in coreboot[master]: soc/intel/alderlake/meminit.c: Guard CsPiStartHighinEct properly
by 9elements QA (Code Review) 03 Aug '23

03 Aug '23

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76915?usp=email ) Change subject: soc/intel/alderlake/meminit.c: Guard CsPiStartHighinEct properly ...................................................................... Patch Set 3: Automatic boot test returned (PASS/FAIL/TOTAL): 6 / 9 / 15 FAIL: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES_ and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/176210 FAIL: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/176208 FAIL: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/176207 FAIL: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/176206 FAIL: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload TianoCore : https://lava.9esec.io/r/176205 FAIL: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload SeaBIOS : https://lava.9esec.io/r/176204 FAIL: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload TianoCore : https://lava.9esec.io/r/176203 FAIL: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload SeaBIOS : https://lava.9esec.io/r/176202 FAIL: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload TianoCore : https://lava.9esec.io/r/176201 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload SeaBIOS : https://lava.9esec.io/r/176200 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload SeaBIOS : https://lava.9esec.io/r/176198 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/176197 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/176196 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/176195 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/176194 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/76915?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95bd2485b98be4ab3a39eaaebb9efb34db93bbe8 Gerrit-Change-Number: 76915 Gerrit-PatchSet: 3 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-Comment-Date: Thu, 03 Aug 2023 12:43:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: include/efi: Include BMP and BLT header macro definitions
by Subrata Banik (Code Review) 03 Aug '23

03 Aug '23

Attention is currently required from: Kapil Porwal, Nick Vaccaro, Paul Menzel, Paz Zcharya. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76920?usp=email ) Change subject: include/efi: Include BMP and BLT header macro definitions ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/76920/comment/5f3381b1_b3b8b751 : PS1, Line 14: > One space? Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/76920?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4e282d135007d288aadb5996a662524f76428874 Gerrit-Change-Number: 76920 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Paz Zcharya <pazz(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paz Zcharya <pazz(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Thu, 03 Aug 2023 12:43:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: include/efi: Include BMP and BLT header macro definitions
by Paul Menzel (Code Review) 03 Aug '23

03 Aug '23

Attention is currently required from: Kapil Porwal, Nick Vaccaro, Paz Zcharya, Subrata Banik. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76920?usp=email ) Change subject: include/efi: Include BMP and BLT header macro definitions ...................................................................... Patch Set 1: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/76920/comment/c1c8461e_b7f0b4ea : PS1, Line 14: One space? https://review.coreboot.org/c/coreboot/+/76920/comment/b528526b_fe0eb8a7 : PS1, Line 17: Note: BLT in UEFI BMP implementation stands for `Bit-block transfer`. : It is a method of copying graphis data (specifically images and fonts) : from one location to another (framebuffer), where the data is stored : in blocks of bits. Thank you for adding this. -- To view, visit https://review.coreboot.org/c/coreboot/+/76920?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4e282d135007d288aadb5996a662524f76428874 Gerrit-Change-Number: 76920 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Paz Zcharya <pazz(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paz Zcharya <pazz(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Thu, 03 Aug 2023 12:39:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: drivers/intel/fsp2_0: Add API to convert BMP images to GOP BLT buffer
by Subrata Banik (Code Review) 03 Aug '23

03 Aug '23

Attention is currently required from: Andrey Petrov, Kapil Porwal, Nick Vaccaro. Hello Andrey Petrov, Kapil Porwal, Nick Vaccaro, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/76921?usp=email to look at the new patch set (#3). Change subject: drivers/intel/fsp2_0: Add API to convert BMP images to GOP BLT buffer ...................................................................... drivers/intel/fsp2_0: Add API to convert BMP images to GOP BLT buffer This patch adds an API to convert BMP images into GOP BLT buffers for Intel FSP-S. This is required to display the OEM splash screen at pre-boot phase. Previously, Intel FSP-S had provision to consume the *.BMP file as is. However, starting with the Alder Lake platform, Intel FSP has dropped this conversion logic and expects the bootloader to pass the BLT buffer directly. This patch implements the conversion logic in coreboot. It also adds a new config (BMP_LOGO_TO_GOP_BLT) to ensure backward compatibility with older generation FSP. BUG=b:284799726 TEST=Able to build and boot google/rex Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I992b45d65374f09498ff0cab497f7091e1e7a350 --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/Makefile.inc A src/drivers/intel/fsp2_0/fsp_gop_blt.c A src/drivers/intel/fsp2_0/include/fsp/fsp_gop_blt.h 4 files changed, 295 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/76921/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/76921?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I992b45d65374f09498ff0cab497f7091e1e7a350 Gerrit-Change-Number: 76921 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[master]: drivers/intel/fsp2_0: Add API to convert BMP images to GOP BLT buffer
by Subrata Banik (Code Review) 03 Aug '23

03 Aug '23

Attention is currently required from: Andrey Petrov, Kapil Porwal, Nick Vaccaro. Hello Andrey Petrov, Kapil Porwal, Nick Vaccaro, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/76921?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: drivers/intel/fsp2_0: Add API to convert BMP images to GOP BLT buffer ...................................................................... drivers/intel/fsp2_0: Add API to convert BMP images to GOP BLT buffer This patch adds an API to convert BMP images into GOP BLT buffers for Intel FSP-S. This is required to display the OEM splash screen at pre-boot phase. Previously, Intel FSP-S had provision to consume the *.BMP file as is. However, starting with the Alder Lake platform, Intel FSP has dropped this conversion logic and expects the bootloader to pass the BLT buffer directly. This patch implements the conversion logic in coreboot. It also adds a new config (BMP_LOGO_TO_GOP_BLT) to ensure backward compatibility with older generation FSP. BUG=b:284799726 TEST=Able to build and boot google/rex Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I992b45d65374f09498ff0cab497f7091e1e7a350 --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/Makefile.inc A src/drivers/intel/fsp2_0/fsp_gop_blt.c A src/drivers/intel/fsp2_0/include/fsp/fsp_gop_blt.h 4 files changed, 295 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/76921/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/76921?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I992b45d65374f09498ff0cab497f7091e1e7a350 Gerrit-Change-Number: 76921 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[master]: soc/amd/common/data_fabric_helper: add comment about cfg_inst_acc_en
by Fred Reitberger (Code Review) 03 Aug '23

03 Aug '23

Attention is currently required from: Felix Held, Jason Glenesk, Matt DeVillier, Raul Rangel. Fred Reitberger has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76885?usp=email ) Change subject: soc/amd/common/data_fabric_helper: add comment about cfg_inst_acc_en ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/76885?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9aff01750c2c1e3506141b3ed293a980a64f8fac Gerrit-Change-Number: 76885 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 03 Aug 2023 11:44:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/common/tcss: Configure USB-C ports with attached devices
by Kapil Porwal (Code Review) 03 Aug '23

03 Aug '23

Attention is currently required from: Eric Lai, John Su, John Zhao, Matt DeVillier, Subrata Banik. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72909?usp=email ) Change subject: soc/intel/common/tcss: Configure USB-C ports with attached devices ...................................................................... Patch Set 4: (2 comments) File src/soc/intel/common/block/tcss/tcss.c: https://review.coreboot.org/c/coreboot/+/72909/comment/8eedc8a7_614b721e : PS4, Line 357: if ((ret < 0) || !mux_info.usb) could you please change it as below ` if ((ret < 0) || !mux_info.usb || (mux_info.dp && mux_info.hpd_lvl))` https://review.coreboot.org/c/coreboot/+/72909/comment/4b46b617_d354e226 : PS4, Line 361: ret = send_pmc_connect_request(i, &mux_info, port_info); > I'm afraid that we might end up sending the PMC connect cmd twice if DP=1 and USB=1 for the same por […] Correct. The current code will try to send the PMC CONN command for a DP+USB device as well but since we have already sent the PMC CONN command for all DP devices in `tcss_configure_dp_mode`, the second CONN command will be rejected by PMC. You can see below error messages in the logs - ``` [DEBUG] port C1 CONN req: usage 0 usb3 3 usb2 1 ufp 0 ori_hsl 1 ori_sbu 1 dbg_acc 0 [DEBUG] Raw Buffer output 0 00006130 [DEBUG] Raw Buffer output 1 00000000 [ERROR] pmc_send_ipc_cmd status: fatal [ERROR] Port 1 connect request failed ``` You can filter DP devices in this function to avoid the error. -- To view, visit https://review.coreboot.org/c/coreboot/+/72909?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69522dbcc8cae6bbf41659ae653107d0e031c812 Gerrit-Change-Number: 72909 Gerrit-PatchSet: 4 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Gerrit-Reviewer: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Reviewer: John Zhao <john.zhao(a)intel.corp-partner.google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: CoolStar <coolstarorganization(a)gmail.com> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-CC: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-CC: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: John Zhao <john.zhao(a)intel.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Thu, 03 Aug 2023 11:41:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/meteorlake: Add provision to show pre-boot splash screen
by Subrata Banik (Code Review) 03 Aug '23

03 Aug '23

Attention is currently required from: Eran Mitrani, Jakub Czapiga, Kapil Porwal, Tarun Tuli. Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/76922?usp=email ) Change subject: soc/intel/meteorlake: Add provision to show pre-boot splash screen ...................................................................... soc/intel/meteorlake: Add provision to show pre-boot splash screen This patch adds the ability to show a pre-boot splash screen on Meteor Lake systems using FSP-S. The patch calls into `fsp_convert_bmp_to_gop_blt()` when the `BMP_LOGO_TO_GOP_BLT` config is enabled. This function converts a BMP file to a BLT buffer, which is then used by FSP-S to render the splash screen. Additionally, increase the heap size (malloc'able size) upto 512KB (when BMP_LOGO_TO_GOP_BLT config is enabled) to accommodate high resolution logo file. BUG=b:284799726 TEST=Able to see pre-boot splash screen while booting google/rex. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I3608bfacc21574e12cde0e2012a16e6388ce54df --- M src/soc/intel/meteorlake/Kconfig M src/soc/intel/meteorlake/fsp_params.c 2 files changed, 11 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/76922/1 diff --git a/src/soc/intel/meteorlake/Kconfig b/src/soc/intel/meteorlake/Kconfig index 2eb1405..90d6645 100644 --- a/src/soc/intel/meteorlake/Kconfig +++ b/src/soc/intel/meteorlake/Kconfig @@ -186,6 +186,7 @@ config HEAP_SIZE hex + default 0x80000 if BMP_LOGO_TO_GOP_BLT default 0x10000 # Intel recommends reserving the PCIe TBT root port resources as below: diff --git a/src/soc/intel/meteorlake/fsp_params.c b/src/soc/intel/meteorlake/fsp_params.c index 30800e6..72d9479 100644 --- a/src/soc/intel/meteorlake/fsp_params.c +++ b/src/soc/intel/meteorlake/fsp_params.c @@ -10,6 +10,7 @@ #include <device/pci.h> #include <fsp/api.h> #include <fsp/fsp_debug_event.h> +#include <fsp/fsp_gop_blt.h> #include <fsp/ppi/mp_service_ppi.h> #include <fsp/util.h> #include <option.h> @@ -859,5 +860,13 @@ /* Handle FSP logo params */ void soc_load_logo(FSPS_UPD *supd) { - bmp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize); + if (CONFIG(BMP_LOGO_TO_GOP_BLT)) + fsp_convert_bmp_to_gop_blt(&supd->FspsConfig.LogoPtr, + &supd->FspsConfig.LogoSize, + &supd->FspsConfig.BltBufferAddress, + &supd->FspsConfig.BltBufferSize, + &supd->FspsConfig.LogoPixelHeight, + &supd->FspsConfig.LogoPixelWidth); + else + bmp_load_logo(&supd->FspsConfig.LogoPtr, &supd->FspsConfig.LogoSize); } -- To view, visit https://review.coreboot.org/c/coreboot/+/76922?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3608bfacc21574e12cde0e2012a16e6388ce54df Gerrit-Change-Number: 76922 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-MessageType: newchange

1 0

[L] Change in coreboot[master]: drivers/intel/fsp2_0: Add API to convert BMP images to GOP BLT buffer
by Subrata Banik (Code Review) 03 Aug '23

03 Aug '23

Attention is currently required from: Andrey Petrov. Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/76921?usp=email ) Change subject: drivers/intel/fsp2_0: Add API to convert BMP images to GOP BLT buffer ...................................................................... drivers/intel/fsp2_0: Add API to convert BMP images to GOP BLT buffer This patch adds an API to convert BMP images into GOP BLT buffers for Intel FSP-S. This is required to display the OEM splash screen at pre-boot phase. Previously, Intel FSP-S had provision to consume the *.BMP file as is. However, starting with the Alder Lake platform, Intel FSP has dropped this conversion logic and expects the bootloader to pass the BLT buffer directly. This patch implements the conversion logic in coreboot. It also adds a new config (BMP_LOGO_TO_GOP_BLT) to ensure backward compatibility with older generation FSP. BUG=b:284799726 TEST=Able to build and boot google/rex Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I992b45d65374f09498ff0cab497f7091e1e7a350 --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/Makefile.inc A src/drivers/intel/fsp2_0/fsp_gop_blt.c A src/drivers/intel/fsp2_0/include/fsp/fsp_gop_blt.h 4 files changed, 296 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/76921/1 diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 378f4fb..15502b8 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -209,6 +209,17 @@ BMP file only. The uncompressed size can be up to 1 MB. The logo can be compressed using LZMA. +config BMP_LOGO_TO_GOP_BLT + bool "Enable logo by passing GOP BLT Buffer to FSP" + default n + select BMP_LOGO + depends on HAVE_FSP_LOGO_SUPPORT + help + Uses the FSP to display the boot logo. This method translates a *.BMP graphics image + to a GOP blt buffer which can be directly consumed by FSP (rather running the EDK2 + specific conversion logic). `FSP2_0_LOGO_FILE_NAME` to keep the logo file inside CBFS + in compressed (using LZMA) format. The uncompressed size can be up to 1 MB. + config FSP2_0_LOGO_FILE_NAME string "Logo file" depends on BMP_LOGO diff --git a/src/drivers/intel/fsp2_0/Makefile.inc b/src/drivers/intel/fsp2_0/Makefile.inc index ae82833..11d85e8 100644 --- a/src/drivers/intel/fsp2_0/Makefile.inc +++ b/src/drivers/intel/fsp2_0/Makefile.inc @@ -33,6 +33,7 @@ ramstage-$(CONFIG_CACHE_MRC_SETTINGS) += save_mrc_data.c ramstage-$(CONFIG_MMA) += mma_core.c ramstage-$(CONFIG_ENABLE_FSP_ERROR_INFO) += fsp_error_info_hob.c +ramstage-$(CONFIG_BMP_LOGO_TO_GOP_BLT) += fsp_gop_blt.c ifneq ($(CONFIG_NO_FSP_TEMP_RAM_EXIT),y) postcar-$(CONFIG_FSP_CAR) += temp_ram_exit.c diff --git a/src/drivers/intel/fsp2_0/fsp_gop_blt.c b/src/drivers/intel/fsp2_0/fsp_gop_blt.c new file mode 100644 index 0000000..e51d537 --- /dev/null +++ b/src/drivers/intel/fsp2_0/fsp_gop_blt.c @@ -0,0 +1,270 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <bootsplash.h> +#include <console/console.h> +#include <fsp/api.h> +#include <fsp/fsp_gop_blt.h> +#include <lib.h> +#include <stdlib.h> + +static bool is_bmp_image_valid(efi_bmp_image_header *header) +{ + if (header == NULL) + return false; + + /* Check if the BMP Header Signature is valid */ + if (header->CharB != 'B' || header->CharM != 'M') + return false; + + /* Check if the BMP Image Header Length is valid */ + if (!header->PixelHeight || !header->PixelWidth) + return false; + + if (header->Size < header->ImageOffset) + return false; + + if (header->ImageOffset < sizeof (efi_bmp_image_header)) + return false; + + return true; +} + +static bool is_bmp_image_compressed(efi_bmp_image_header *header) +{ + if (header == NULL) + return false; + + if (header->CompressionType != 0) + return true; + + return false; +} + +static bool is_bitmap_format_supported(efi_bmp_image_header *header) +{ + if (header == NULL) + return false; + + /* + * Check BITMAP format is supported + * BMP_IMAGE_HEADER = BITMAP_FILE_HEADER + BITMAP_INFO_HEADER + */ + if (header->HeaderSize != sizeof (efi_bmp_image_header) - + OFFSET_OF (efi_bmp_image_header, HeaderSize)) + return false; + + return true; +} + +static bool do_bmp_image_authentication(efi_bmp_image_header *header) +{ + if (header == NULL) + return false; + + if (!is_bmp_image_valid(header)) { + printk(BIOS_ERR, "%s: BMP Image Header is invalid.\n", __func__); + return false; + } + + /* + * BMP image compression is unsupported by FSP implementation, + * hence, exit if the BMP image is compressed. + */ + if (is_bmp_image_compressed(header)) { + printk(BIOS_ERR, "%s: BMP Image Compression is unsupported.\n", __func__); + return false; + } + + if (!is_bitmap_format_supported(header)) { + printk(BIOS_ERR, "%s: BmpHeader Header Size (0x%x) is not as expected.\n", + __func__, header->HeaderSize); + return false; + } + + return true; +} + +static uint32_t calculate_blt_buffer_size(efi_bmp_image_header *header) +{ + uint32_t blt_buffer_size; + + if (header == NULL) + return 0; + + /* Calculate the size required for BLT buffer */ + blt_buffer_size = header->PixelWidth * header->PixelHeight * + sizeof(efi_graphics_output_blt_pixel); + if (!blt_buffer_size) + return 0; + + return blt_buffer_size; +} + +static uint32_t get_color_map_num(efi_bmp_image_header *header) +{ + uint32_t col_map_number = 0; + + if (header == NULL) + return 0; + + switch (header->BitPerPixel) { + case 1: + col_map_number = 2; + break; + case 4: + col_map_number = 16; + break; + case 8: + col_map_number = 256; + break; + default: + break; + } + + /* + * At times BMP file may have padding data between its header section and the + * data section. + */ + if (header->ImageOffset - sizeof (efi_bmp_image_header) < + sizeof (efi_bmp_color_map) * col_map_number) + return 0; + + return col_map_number; +} + +/* Fill BMP image into BLT buffer format */ +static void *fill_blt_buffer(efi_bmp_image_header *header, + uint32_t logo_ptr, uint32_t blt_buffer_size) +{ + efi_graphics_output_blt_pixel *gop_blt_buffer; + efi_graphics_output_blt_pixel *gop_blt_ptr; + efi_graphics_output_blt_pixel *gop_blt; + uint8_t *bmp_image; + uint8_t *bmp_image_header; + efi_bmp_color_map *bmp_color_map; + size_t image_index; + + if (header == NULL) + return NULL; + + gop_blt_ptr = malloc(sizeof(uint8_t) * blt_buffer_size); + if (!gop_blt_ptr) + die("%s: out of memory. Consider increasing the `CONFIG_HEAP_SIZE`\n", + __func__); + + bmp_image = ((UINT8 *)logo_ptr) + header->ImageOffset; + bmp_image_header = bmp_image; + gop_blt_buffer = gop_blt_ptr; + bmp_color_map = (efi_bmp_color_map *)(logo_ptr + sizeof (efi_bmp_image_header)); + + for (size_t height = 0; height < header->PixelHeight; height++) { + gop_blt = &gop_blt_buffer[(header->PixelHeight - height - 1) * + header->PixelWidth]; + for (size_t width = 0; width < header->PixelWidth; width++, bmp_image++, + gop_blt++) { + switch (header->BitPerPixel) { + /* Translate 1-bit (2 colors) BMP to 24-bit color */ + case 1: + for (size_t index = 0; index < 8 && width < header->PixelWidth; index++) { + gop_blt->Red = bmp_color_map[((*bmp_image) >> (7 - index)) & 0x1].Red; + gop_blt->Green = bmp_color_map[((*bmp_image) >> (7 - index)) & 0x1].Green; + gop_blt->Blue = bmp_color_map[((*bmp_image) >> (7 - index)) & 0x1].Blue; + gop_blt++; + width++; + } + gop_blt--; + width--; + break; + + /* Translate 4-bit (16 colors) BMP Palette to 24-bit color */ + case 4: + size_t index = (*bmp_image) >> 4; + gop_blt->Red = bmp_color_map[index].Red; + gop_blt->Green = bmp_color_map[index].Green; + gop_blt->Blue = bmp_color_map[index].Blue; + if (width < (header->PixelWidth - 1)) { + gop_blt++; + width++; + index = (*bmp_image) & 0x0f; + gop_blt->Red = bmp_color_map[index].Red; + gop_blt->Green = bmp_color_map[index].Green; + gop_blt->Blue = bmp_color_map[index].Blue; + } + break; + + /* Translate 8-bit (256 colors) BMP Palette to 24-bit color */ + case 8: + gop_blt->Red = bmp_color_map[*bmp_image].Red; + gop_blt->Green = bmp_color_map[*bmp_image].Green; + gop_blt->Blue = bmp_color_map[*bmp_image].Blue; + break; + + /* For 24-bit BMP */ + case 24: + gop_blt->Blue = *bmp_image++; + gop_blt->Green = *bmp_image++; + gop_blt->Red = *bmp_image; + break; + + /* Conver 32 bit to 24bit bmp - just ignore the final byte of each pixel */ + case 32: + gop_blt->Blue = *bmp_image++; + gop_blt->Green = *bmp_image++; + gop_blt->Red = *bmp_image++; + break; + + /* Other bit format of BMP is not supported. */ + default: + free(gop_blt_ptr); + gop_blt_ptr = NULL; + + printk(BIOS_ERR, "%s, BMP Bit format not supported. 0x%X\n", __func__, + header->BitPerPixel); + return NULL; + break; + } + } + image_index = (uintptr_t)bmp_image - (uintptr_t)bmp_image_header; + /* Each row in BMP Image should be 4-byte align */ + if ((image_index % 4) != 0) + bmp_image = bmp_image + (4 - (image_index % 4)); + } + + return gop_blt_ptr; +} + +/* Convert a *.BMP graphics image to a GOP blt buffer */ +void fsp_convert_bmp_to_gop_blt (uint32_t *logo, uint32_t *logo_size, + uint32_t *blt_ptr, uint32_t *blt_size, + uint32_t *pixel_height, uint32_t *pixel_width) +{ + uint32_t logo_ptr, logo_ptr_size, blt_buffer_size; + efi_bmp_image_header *bmp_header; + + if (!logo || !logo_size || !blt_ptr || !blt_size || !pixel_height || !pixel_width) + return; + + bmp_load_logo(&logo_ptr, &logo_ptr_size); + if (!logo_ptr || logo_ptr_size < sizeof (efi_bmp_image_header)) { + printk(BIOS_ERR, "%s: BMP Image size is too small.\n", __func__); + return; + } + + bmp_header = (efi_bmp_image_header *)logo_ptr; + if (!do_bmp_image_authentication(bmp_header) || (bmp_header->Size != logo_ptr_size)) + return; + + blt_buffer_size = calculate_blt_buffer_size(bmp_header); + if (!blt_buffer_size) + return; + + if (!get_color_map_num(bmp_header)) + return; + + *logo = logo_ptr; + *logo_size = logo_ptr_size; + *blt_size = blt_buffer_size; + *pixel_height = bmp_header->PixelHeight; + *pixel_width = bmp_header->PixelWidth; + *blt_ptr = (uint32_t)fill_blt_buffer(bmp_header, logo_ptr, blt_buffer_size); +} diff --git a/src/drivers/intel/fsp2_0/include/fsp/fsp_gop_blt.h b/src/drivers/intel/fsp2_0/include/fsp/fsp_gop_blt.h new file mode 100644 index 0000000..886859e --- /dev/null +++ b/src/drivers/intel/fsp2_0/include/fsp/fsp_gop_blt.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef FSP_GOP_BLT_H +#define FSP_GOP_BLT_H + +#include <efi/efi_datatype.h> +#include <types.h> + +/* Convert a *.BMP graphics image to a GOP blt buffer */ +void fsp_convert_bmp_to_gop_blt (uint32_t *logo, uint32_t *logo_size, + uint32_t *blt_ptr, uint32_t *blt_size, + uint32_t *pixel_height, uint32_t *pixel_width); + +#endif /* FSP_GOP_BLT_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/76921?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I992b45d65374f09498ff0cab497f7091e1e7a350 Gerrit-Change-Number: 76921 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[master]: include/efi: Include BMP and BLT header macro definitions
by Subrata Banik (Code Review) 03 Aug '23

03 Aug '23

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/76920?usp=email ) Change subject: include/efi: Include BMP and BLT header macro definitions ...................................................................... include/efi: Include BMP and BLT header macro definitions This patch adds BMP image header and BLT header macros in `efi_datatype.h` to implement a converter inside coreboot FSP 2.0 driver that converts any input *.BMP image into the BLT buffer. The output BLT buffer is used by FSP-S to render any pre-boot display. Added `Bmp.h` and `GraphicsOutput.h` files for `UDK base >= 2017`, as these files were added with the UDK version 2017. Note: BLT in UEFI BMP implementation stands for `Bit-block transfer`. It is a method of copying graphis data (specifically images and fonts) from one location to another (framebuffer), where the data is stored in blocks of bits. BUG=b:284799726 TEST=Able to build and boot google/rex. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I4e282d135007d288aadb5996a662524f76428874 --- M src/include/efi/efi_datatype.h 1 file changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/76920/1 diff --git a/src/include/efi/efi_datatype.h b/src/include/efi/efi_datatype.h index 581193a..cbc47c7 100644 --- a/src/include/efi/efi_datatype.h +++ b/src/include/efi/efi_datatype.h @@ -7,9 +7,11 @@ #include <Uefi/UefiBaseType.h> #if CONFIG_UDK_VERSION >= CONFIG_UDK_2017_VERSION +#include <IndustryStandard/Bmp.h> #include <Guid/StatusCodeDataTypeId.h> #include <Pi/PiPeiCis.h> #include <Pi/PiStatusCode.h> +#include <Protocol/GraphicsOutput.h> #include <Protocol/MpService.h> /* Data structure for EFI_PEI_SERVICE. */ @@ -24,6 +26,12 @@ typedef EFI_STATUS_CODE_DATA efi_status_code_data_t; /* Status string data type definition */ typedef EFI_STATUS_CODE_STRING_DATA efi_status_code_string_data; +/* Data structure for EFI_GRAPHICS_OUTPUT_BLT_PIXEL. */ +typedef EFI_GRAPHICS_OUTPUT_BLT_PIXEL efi_graphics_output_blt_pixel; +/* Data structure for BMP_IMAGE_HEADER. */ +typedef BMP_IMAGE_HEADER efi_bmp_image_header; +/* Data structure for BMP_COLOR_MAP; . */ +typedef BMP_COLOR_MAP efi_bmp_color_map; #endif /* Basic Data types */ -- To view, visit https://review.coreboot.org/c/coreboot/+/76920?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4e282d135007d288aadb5996a662524f76428874 Gerrit-Change-Number: 76920 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2023