Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81890?usp=email )
Change subject: mb/asus/p8z77-m: Light DRAM_LED during early boot
......................................................................
mb/asus/p8z77-m: Light DRAM_LED during early boot
Turn on DRAM_LED on the mainboard in early bootblock, and turn it off
in ramstage. Primarily an indication if boot fails during raminit,
modeled after vendor firmware.
This LED is controlled by GPIO07 on the super I/O.
Boot tested on hardware.
Change-Id: I549b51375d1ef056d5fc01871bfe62d60b8a01cb
Signed-off-by: Keith Hui <buurin(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/81890
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
---
M src/mainboard/asus/p8x7x-series/variants/p8z77-m/early_init.c
M src/mainboard/asus/p8x7x-series/variants/p8z77-m/overridetree.cb
2 files changed, 16 insertions(+), 1 deletion(-)
Approvals:
Felix Singer: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/src/mainboard/asus/p8x7x-series/variants/p8z77-m/early_init.c b/src/mainboard/asus/p8x7x-series/variants/p8z77-m/early_init.c
index 6897658..d13a97b 100644
--- a/src/mainboard/asus/p8x7x-series/variants/p8z77-m/early_init.c
+++ b/src/mainboard/asus/p8x7x-series/variants/p8z77-m/early_init.c
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
#include <bootblock_common.h>
+#include <device/pnp_ops.h>
#include <northbridge/intel/sandybridge/raminit.h>
#include <northbridge/intel/sandybridge/pei_data.h>
#include <southbridge/intel/bd82x6x/pch.h>
@@ -11,11 +12,23 @@
#include <option.h>
#define SERIAL_DEV PNP_DEV(0x2e, NCT6779D_SP1)
+#define GPIO0_DEV PNP_DEV(0x2e, NCT6779D_WDT1_GPIO01_V)
void bootblock_mainboard_early_init(void)
{
nuvoton_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
+ nuvoton_pnp_enter_conf_state(GPIO0_DEV);
+
+ /* Turn on DRAM_LED. If raminit dies, this would remain on and we know
+ * we have a problem. We turn it off in ramstage. */
+ pnp_set_logical_device(GPIO0_DEV);
+ pnp_write_config(GPIO0_DEV, 0x30, 0x02);
+ pnp_write_config(GPIO0_DEV, 0xe0, 0x7f);
+ pnp_write_config(GPIO0_DEV, 0xe1, 0x00);
+
+ nuvoton_pnp_exit_conf_state(GPIO0_DEV);
+
/*
* TODO: Put PCIe root port 7 (00:1c.6) into subtractive decode and have it accept I/O
* cycles. This should allow a POST card in the PCI slot, connected via an ASM1083
diff --git a/src/mainboard/asus/p8x7x-series/variants/p8z77-m/overridetree.cb b/src/mainboard/asus/p8x7x-series/variants/p8z77-m/overridetree.cb
index 6232c9b..41351b5 100644
--- a/src/mainboard/asus/p8x7x-series/variants/p8z77-m/overridetree.cb
+++ b/src/mainboard/asus/p8x7x-series/variants/p8z77-m/overridetree.cb
@@ -59,6 +59,9 @@
end
device pnp 2e.6 off end # CIR
device pnp 2e.8 off end # WDT1
+ device pnp 2e.108 on # GPIO 0
+ drq 0xe1 = 0x80 # GP07 high turns DRAM_LED off
+ end
device pnp 2e.a on # ACPI
drq 0xe4 = 0x10 # Enable 3VSBSW#, needed for S3 suspend
drq 0xe7 = 0x11 # HWM reset by LRESET#, 0.5s S3 delay for compatibility
@@ -78,7 +81,6 @@
end
device pnp 2e.9 off end # GPIO 8
device pnp 2e.308 on end # GPIO by I/O
- device pnp 2e.108 on end # GPIO 0
device pnp 2e.109 on end # GPIO 1
device pnp 2e.209 on # GPIO 2
drq 0xe0 = 0xbf # GP26 output
--
To view, visit https://review.coreboot.org/c/coreboot/+/81890?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I549b51375d1ef056d5fc01871bfe62d60b8a01cb
Gerrit-Change-Number: 81890
Gerrit-PatchSet: 3
Gerrit-Owner: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Attention is currently required from: Martin L Roth, Nico Huber, Nigel Tao, Paul Menzel.
Jonathon Hall has posted comments on this change by Nigel Tao. ( https://review.coreboot.org/c/coreboot/+/83895?usp=email )
Change subject: lib/jpeg: avoid calling malloc and free
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
Patchset:
PS2:
Thank you Nigel, this is much better than increasing the heap size. I will test it with the Heads bootsplash as soon as I can spare a few minutes. I'll abandon patch 83476 if we get consensus on this approach.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83895?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: Ie4c52520cbce498539517c4898ff765365a6beba
Gerrit-Change-Number: 83895
Gerrit-PatchSet: 2
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Comment-Date: Tue, 13 Aug 2024 12:41:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Martin L Roth, Nico Huber, Paul Menzel.
Nigel Tao has posted comments on this change by Nigel Tao. ( https://review.coreboot.org/c/coreboot/+/83895?usp=email )
Change subject: lib/jpeg: avoid calling malloc and free
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Adding reviewers and CC based on the people mentioned in:
https://review.coreboot.org/c/coreboot/+/83476https://review.coreboot.org/c/coreboot/+/78271
This (and the related https://review.coreboot.org/c/coreboot/+/83894) is my first coreboot gerrit contribution and I'm also very new to coreboot. Please let me know if I'm doing something wrong. I've surely added too many reviewers but I don't know who the primary reviewer should be.
After following the coreboot tutorial, I've tested this patch on qemu with the config below (and an 800x600 bootsplash.jpg file):
```
CONFIG_BOOTSPLASH=y
CONFIG_BOOTSPLASH_IMAGE=y
CONFIG_GENERIC_LINEAR_FRAMEBUFFER=y
CONFIG_PAYLOAD_ELF=y
CONFIG_PAYLOAD_FILE="payloads/coreinfo/build/coreinfo.elf"
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/83895?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: Ie4c52520cbce498539517c4898ff765365a6beba
Gerrit-Change-Number: 83895
Gerrit-PatchSet: 2
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-CC: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 13 Aug 2024 12:38:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Felix Singer, Jonathon Hall, Martin L Roth, Nico Huber, Paul Menzel.
Nigel Tao has posted comments on this change by Jonathon Hall. ( https://review.coreboot.org/c/coreboot/+/83476?usp=email )
Change subject: bootsplash: Increase heap from 1 MB to 4 MB when bootsplash is enabled
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> > But if overestimating memory requirements (by a few MB) is problematic for coreboot, I can do some […]
My counter-proposal is now
https://review.coreboot.org/c/coreboot/+/83895
--
To view, visit https://review.coreboot.org/c/coreboot/+/83476?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: Ia4348d39effbc16c1b42ab01bcf1e4ec5d652fa9
Gerrit-Change-Number: 83476
Gerrit-PatchSet: 2
Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Comment-Date: Tue, 13 Aug 2024 12:31:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Nigel Tao <nigeltao(a)golang.org>
Comment-In-Reply-To: Jonathon Hall <jonathon.hall(a)puri.sm>
Nigel Tao has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/83895?usp=email )
Change subject: lib/jpeg: avoid calling malloc and free
......................................................................
lib/jpeg: avoid calling malloc and free
Since commit 1d029b40c9de ("lib/jpeg: Replace decoder with Wuffs'
implementation"), a relatively large heap allocation is needed to decode
many JPEGs for use as work area. The prior decoder did not need this,
but also had many limitations in the JPEGs it could decode, was not as
memory-safe and quickly crashed under fuzzing.
This commit keeps using Wuffs' JPEG decoder, but it no longer requires
any heap allocation (and thus configuring the heap size depending on how
big a bootsplash image you want to support).
Change-Id: Ie4c52520cbce498539517c4898ff765365a6beba
Signed-off-by: Nigel Tao <nigeltao(a)golang.org>
---
M src/lib/jpeg.c
1 file changed, 21 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/83895/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83895?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: Ie4c52520cbce498539517c4898ff765365a6beba
Gerrit-Change-Number: 83895
Gerrit-PatchSet: 2
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Nigel Tao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83895?usp=email )
Change subject: lib/jpeg: avoid calling malloc and free
......................................................................
lib/jpeg: avoid calling malloc and free
Since commit 1d029b40c9de ("lib/jpeg: Replace decoder with Wuffs'
implementation"), a relatively large heap allocation is needed to decode
many JPEGs for use as work area. The prior decoder did not need this,
but also had many limitations in the JPEGs it could decode, was not as
memory-safe and quickly crashed under fuzzing.
This commit keeps using Wuffs' JPEG decoder, but it no longer requires
any heap allocation (and thus configuring the heap size depending on how
big a bootsplash image you want to support).
Change-Id: Ie4c52520cbce498539517c4898ff765365a6beba
Signed-off-by: Nigel Tao <nigeltao(a)golang.org>
---
M src/lib/jpeg.c
1 file changed, 20 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/83895/1
diff --git a/src/lib/jpeg.c b/src/lib/jpeg.c
index 242cf0c..4fa850b 100644
--- a/src/lib/jpeg.c
+++ b/src/lib/jpeg.c
@@ -85,6 +85,24 @@
return JPEG_DECODE_FAILED;
}
+ /* Opting in to lower quality means that we can pass an empty slice as the
+ * "work buffer" argument to wuffs_jpeg__decoder__decode_frame below.
+ *
+ * Decoding progressive (not sequential) JPEGs would still require dynamic
+ * memory allocation (and the amount of work buffer required depends on the
+ * image dimensions), but we choose to just reject progressive JPEGs. It is
+ * simpler than sometimes calling malloc (which can fail, especially for
+ * large allocations) and free.
+ *
+ * More commentary about these quirks is at
+ * https://github.com/google/wuffs/blob/beaf45650085a16780b5f708b72daaeb1aa865…
+ */
+ wuffs_jpeg__decoder__set_quirk(
+ &dec, WUFFS_BASE__QUIRK_QUALITY,
+ WUFFS_BASE__QUIRK_QUALITY__VALUE__LOWER_QUALITY);
+ wuffs_jpeg__decoder__set_quirk(
+ &dec, WUFFS_JPEG__QUIRK_REJECT_PROGRESSIVE_JPEGS, 1);
+
wuffs_base__image_config imgcfg;
wuffs_base__io_buffer src = wuffs_base__ptr_u8__reader(filedata, filesize, true);
status = wuffs_jpeg__decoder__decode_image_config(&dec, &imgcfg, &src);
@@ -104,19 +122,9 @@
return JPEG_DECODE_FAILED;
}
- uint64_t workbuf_len_min_incl = wuffs_jpeg__decoder__workbuf_len(&dec).min_incl;
- uint8_t *workbuf_array = malloc(workbuf_len_min_incl);
- if ((workbuf_array == NULL) && workbuf_len_min_incl) {
- return JPEG_DECODE_FAILED;
- }
-
- wuffs_base__slice_u8 workbuf =
- wuffs_base__make_slice_u8(workbuf_array, workbuf_len_min_incl);
status = wuffs_jpeg__decoder__decode_frame(&dec, &pixbuf, &src,
- WUFFS_BASE__PIXEL_BLEND__SRC, workbuf, NULL);
-
- free(workbuf_array);
-
+ WUFFS_BASE__PIXEL_BLEND__SRC,
+ wuffs_base__empty_slice_u8(), NULL);
if (status.repr) {
return JPEG_DECODE_FAILED;
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/83895?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: Ie4c52520cbce498539517c4898ff765365a6beba
Gerrit-Change-Number: 83895
Gerrit-PatchSet: 1
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Nigel Tao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83894?usp=email )
Change subject: vc/wuffs: upgrade to Wuffs 0.4.0-alpha.8
......................................................................
vc/wuffs: upgrade to Wuffs 0.4.0-alpha.8
We were previously at Wuffs 0.4.0-alpha.2. The C file was copied from
https://github.com/google/wuffs-mirror-release-c and its hash matches
https://github.com/google/wuffs-mirror-release-c/blob/90e4d81a6a8b7b601e8e5…
$ sha256sum src/vendorcode/wuffs/wuffs-v0.4.c
6c22caff4af929112601379a73f72461bc4719a5215366bcc90d599cbc442bb6 src/vendorcode/wuffs/wuffs-v0.4.c
Change-Id: Ie90d989384e0db2b23d7d1b3d9a57920ac8a95a2
Signed-off-by: Nigel Tao <nigeltao(a)golang.org>
---
M src/vendorcode/wuffs/wuffs-v0.4.c
1 file changed, 22,492 insertions(+), 5,930 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/83894/1
--
To view, visit https://review.coreboot.org/c/coreboot/+/83894?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: Ie90d989384e0db2b23d7d1b3d9a57920ac8a95a2
Gerrit-Change-Number: 83894
Gerrit-PatchSet: 1
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Attention is currently required from: Nicholas Chin, Pablo, Paul Menzel.
Hello Nicholas Chin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83817?usp=email
to look at the new patch set (#7).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: Documentation: getting-started: faq: Remove line break in URL breaking link
......................................................................
Documentation: getting-started: faq: Remove line break in URL breaking link
Change-Id: I3f950af4201486cd90e5fa61a4657ab7ae643825
Signed-off-by: Pablo Iranzo Gómez <Pablo.Iranzo(a)gmail.com>
---
M Documentation/getting_started/faq.md
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/83817/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/83817?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: I3f950af4201486cd90e5fa61a4657ab7ae643825
Gerrit-Change-Number: 83817
Gerrit-PatchSet: 7
Gerrit-Owner: Pablo <Pablo(a)Iranzo.io>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Pablo <Pablo(a)Iranzo.io>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>