Attention is currently required from: Nigel Tao.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84340?usp=email
to look at the new patch set (#2).
Change subject: lib/jpeg: enable dst-pixel-format allowlist
......................................................................
lib/jpeg: enable dst-pixel-format allowlist
Wuffs can decode image files to a variety of pixel formats (BGRA vs
RGBA, premultiplied vs non-premultiplied alpha, 8-bit vs 16-bit, etc).
If coreboot always decodes to BGRA and never to RGBA then we can cut out
the unused code paths at compile time, via configuration macros.
Before fallback/ramstage 84180 LZMA (174340 decompressed)
After fallback/ramstage 83513 LZMA (173056 decompressed) = 99.21%
It's not a large saving, but every little bit helps.
Change-Id: Ie29592f74f245cb890d18b68060640e9bab192b2
---
M src/lib/jpeg.c
1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/84340/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84340?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie29592f74f245cb890d18b68060640e9bab192b2
Gerrit-Change-Number: 84340
Gerrit-PatchSet: 2
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nigel Tao <nigeltao(a)golang.org>
Nigel Tao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84340?usp=email )
Change subject: lib/jpeg: enable dst-pixel-format allowlist
......................................................................
lib/jpeg: enable dst-pixel-format allowlist
Wuffs can decode image files to a variety of pixel formats (BGRA vs
RGBA, premultiplied vs non-premultiplied alpha, 8-bit vs 16-bit, etc).
If Coreboot always decodes to BGRA and never to RGBA then we can cut out
the unused code paths at compile time, via configuration macros.
Before fallback/ramstage 84180 LZMA (174340 decompressed)
After fallback/ramstage 83513 LZMA (173056 decompressed) = 99.21%
It's not a large saving, but every little bit helps.
Change-Id: Ie29592f74f245cb890d18b68060640e9bab192b2
---
M src/lib/jpeg.c
1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/84340/1
diff --git a/src/lib/jpeg.c b/src/lib/jpeg.c
index 617ab0b..2de58b6 100644
--- a/src/lib/jpeg.c
+++ b/src/lib/jpeg.c
@@ -9,6 +9,10 @@
#include "jpeg.h"
#define WUFFS_CONFIG__AVOID_CPU_ARCH
+#define WUFFS_CONFIG__DST_PIXEL_FORMAT__ENABLE_ALLOWLIST
+#define WUFFS_CONFIG__DST_PIXEL_FORMAT__ALLOW_BGR_565
+#define WUFFS_CONFIG__DST_PIXEL_FORMAT__ALLOW_BGR
+#define WUFFS_CONFIG__DST_PIXEL_FORMAT__ALLOW_BGRA_NONPREMUL
#define WUFFS_CONFIG__MODULES
#define WUFFS_CONFIG__MODULE__BASE
#define WUFFS_CONFIG__MODULE__JPEG
@@ -62,6 +66,9 @@
return JPEG_DECODE_FAILED;
}
+ /* The WUFFS_BASE__PIXEL_FORMAT__FOOBAR values used here match
+ * "#define WUFFS_CONFIG__DST_PIXEL_FORMAT__ALLOW_FOOBAR" above.
+ */
uint32_t pixfmt;
switch (depth) {
case 16:
--
To view, visit https://review.coreboot.org/c/coreboot/+/84340?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie29592f74f245cb890d18b68060640e9bab192b2
Gerrit-Change-Number: 84340
Gerrit-PatchSet: 1
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Attention is currently required from: Alicja Michalska, Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Patrick Rudolph, Shuo Liu, Tim Chu.
Angel Pons has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/84304?usp=email )
Change subject: soc/intel/xeon_sp: Use MemoryMapDataHob to add high RAM resources
......................................................................
Patch Set 1:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84304/comment/c6748716_c1217795?us… :
PS1, Line 9: window
Use plural: window*s*
https://review.coreboot.org/c/coreboot/+/84304/comment/479f6a24_b376ee87?us… :
PS1, Line 10: no
typo: on
https://review.coreboot.org/c/coreboot/+/84304/comment/0544927c_8aec8212?us… :
PS1, Line 13: mechanism for GNR and previous generation SoCs.
Has this change been tested? If so, on which platforms and how? I would recommend testing on previous generation platforms to make sure there are no regressions.
File src/soc/intel/xeon_sp/uncore.c:
https://review.coreboot.org/c/coreboot/+/84304/comment/4a5c3862_fef76db5?us… :
PS1, Line 288: const struct SystemMemoryMapHob *mm = get_system_memory_map();
> `code indent should use tabs where possible`
Please fix.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84304?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie5fbc5735704d95c7ad50740ff0e35737afdbd80
Gerrit-Change-Number: 84304
Gerrit-PatchSet: 1
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 11:25:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Cliff Huang, Kapil Porwal, Pranava Y N, Saurabh Mishra.
Subrata Banik has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84297?usp=email )
Change subject: soc/intel/ptl: Add GPE1 defines
......................................................................
Patch Set 4:
(2 comments)
File src/soc/intel/pantherlake/include/soc/gpe.h:
https://review.coreboot.org/c/coreboot/+/84297/comment/0887b89f_90237e3d?us… :
PS4, Line 30: 146
unable to follow why the index is 145 or 0x91 ?
ideally this should be a bit mask as applicable to GPE1_xxx register
File src/soc/intel/pantherlake/include/soc/pm.h:
https://review.coreboot.org/c/coreboot/+/84297/comment/ab4e9862_62eb0bec?us… :
PS4, Line 285: #if CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1)
: uint32_t gpe1_sts[3];
: uint32_t gpe1_en[3];
: #endif
please maintain the order. GPE1 should be ahead in this structure
--
To view, visit https://review.coreboot.org/c/coreboot/+/84297?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iebf6f6d02b37cc9702e4ee07c1ec0017b6628836
Gerrit-Change-Number: 84297
Gerrit-PatchSet: 4
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 11:24:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Alicja Michalska, Arthur Heymans, Chen, Gang C, Jincheng Li, Shuo Liu.
Angel Pons has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/84305?usp=email )
Change subject: util/cbfstool: Add Intel platform boot policy support
......................................................................
Patch Set 1:
(4 comments)
File src/southbridge/intel/common/firmware/Kconfig:
PS1:
`southbridge/intel/common/firmware/Kconfig` is about the IFD (Intel Flash Descriptor) and its regions. For FIT stuff, I believe the correct location for these settings would be somewhere in `cpu/intel/fit` instead.
https://review.coreboot.org/c/coreboot/+/84305/comment/83a3a817_36f4006a?us… :
PS1, Line 27: config HAVE_PBP_BIN
: bool "Add Intel platform boot policy file"
: default n
Not all FIT-capable platforms support PBP. So this option needs to be guarded by another option that is selected by platforms or mainboards that support PBP:
```suggestion
config PLATFORM_SUPPORTS_PBP_IN_FIT
def_bool n
config HAVE_PBP_BIN
bool "Add Intel platform boot policy file"
depends on PLATFORM_SUPPORTS_PBP_IN_FIT
default n
```
https://review.coreboot.org/c/coreboot/+/84305/comment/94c43c08_9687059b?us… :
PS1, Line 31: The platform boot policy file
Looks like this sentence ends halfway through, is this intentional?
https://review.coreboot.org/c/coreboot/+/84305/comment/e39655d9_f647e7c8?us… :
PS1, Line 35: default "3rdparty/blobs/mainboard/\$(MAINBOARDDIR)/pbp.bin"
Can PBP blobs be published to the blobs repo? If not, I would suggest setting the default path to somewhere in `site-local`:
```suggestion
default "site-local/mainboard/\$(MAINBOARDDIR)/pbp.bin"
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/84305?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I0f9fc61341430b1a35a44d50b108dcfaf31cd11c
Gerrit-Change-Number: 84305
Gerrit-PatchSet: 1
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 11:22:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Arthur Heymans has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/84328?usp=email )
Change subject: soc/intel/xeon_sp/gnr: Enable IRQ routing
......................................................................
Patch Set 1:
(1 comment)
File src/soc/intel/xeon_sp/gnr/chip.c:
https://review.coreboot.org/c/coreboot/+/84328/comment/38aa70db_db820bd3?us… :
PS1, Line 64: pch_pirq_init
that seems to be done as part of espi/lpc init. Would that be possible instead of using a bootstate hook?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84328?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I095c7a302894437c90d854ce4e30467357eee2ba
Gerrit-Change-Number: 84328
Gerrit-PatchSet: 1
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 10:57:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Arthur Heymans has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/84327?usp=email )
Change subject: soc/intel/xeon_sp/gnr: Enable VMX by FSP
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84327?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I0c03f535b6f93761419657127e791c02e8ee4988
Gerrit-Change-Number: 84327
Gerrit-PatchSet: 1
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 10:55:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes