Hung-Te Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
soc/mediatek/mt8183: Compress calibration blob with LZ4
The DRAM calibration blob can be compressed using pre-RAM algorithm (currently LZ4), which will save ~12ms in boot time.
On Kodama, boot time difference: Before: 1,082,711 After: 1,070,309
BUG=b:139099592 TEST=build and boot, cbfstool coreboot.rom print -v (see dram compressed) BRANCH=kukui
Change-Id: Ic3bd49d67ee6f80a0e4d8f6945744642611edf64 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/soc/mediatek/mt8183/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/36054/1
diff --git a/src/soc/mediatek/mt8183/Makefile.inc b/src/soc/mediatek/mt8183/Makefile.inc index 70fd080..e3d3db0 100644 --- a/src/soc/mediatek/mt8183/Makefile.inc +++ b/src/soc/mediatek/mt8183/Makefile.inc @@ -84,7 +84,7 @@ DRAM_CBFS := $(CONFIG_CBFS_PREFIX)/dram $(DRAM_CBFS)-file := $(MT8183_BLOB_DIR)/dram.elf $(DRAM_CBFS)-type := stage -$(DRAM_CBFS)-compression := none +$(DRAM_CBFS)-compression := $(CBFS_PRERAM_COMPRESS_FLAG) ifneq ($(wildcard $($(DRAM_CBFS)-file)),) cbfs-files-y += $(DRAM_CBFS) endif
Hello Julius Werner, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36054
to look at the new patch set (#2).
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
soc/mediatek/mt8183: Compress calibration blob with LZ4
The DRAM calibration blob can be compressed using pre-RAM algorithm (currently LZ4), which will save ~12ms in boot time.
On Kodama, boot time difference: Before: 1,082,711 After: 1,070,309
BUG=b:139099592,b:117953502 TEST=build and boot, cbfstool coreboot.rom print -v (see dram compressed) BRANCH=kukui
Change-Id: Ic3bd49d67ee6f80a0e4d8f6945744642611edf64 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/soc/mediatek/mt8183/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/36054/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
Patch Set 2: Code-Review+1
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
Patch Set 2: Code-Review+1
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... File src/soc/mediatek/mt8183/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... PS2, Line 87: CBFS_PRERAM_COMPRESS_FLAG nit: could also use CBFS_COMPRESS_FLAG? RAM isn't up yet when you load this, but the important part is whether the LZMA code is compiled into the stage, which is true for romstage.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... File src/soc/mediatek/mt8183/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... PS2, Line 87: CBFS_PRERAM_COMPRESS_FLAG
nit: could also use CBFS_COMPRESS_FLAG? RAM isn't up yet when you load this, but the important part […]
probably not that easy.
in src/lib/cbfs.c: CBFS_COMPRESS_LZMA is totally ignored (return 0) in ROM STAGE.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... File src/soc/mediatek/mt8183/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... PS2, Line 87: CBFS_PRERAM_COMPRESS_FLAG
probably not that easy. […]
No it's not? Only if you have a POSTCAR_STAGE or if COMPRESS_RAMSTAGE is disabled, neither of which should be true for your case.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... File src/soc/mediatek/mt8183/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... PS2, Line 87: CBFS_PRERAM_COMPRESS_FLAG
No it's not? Only if you have a POSTCAR_STAGE or if COMPRESS_RAMSTAGE is disabled, neither of which […]
Hmmm. When I try that last time it just failed... lzma uses a different API in reading (mmap, deflate then unmap), that's probably why. LZ4 does need double space (which we're running out of SRAM).
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... File src/soc/mediatek/mt8183/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... PS2, Line 87: CBFS_PRERAM_COMPRESS_FLAG
Hmmm. When I try that last time it just failed... […]
correction. lzma needs to fetch whole buffer first (in ARM we don't have mmap so it's read into reserved space for CBFS cache) while lz4 can be decoded using same prepared buffer.
I think we want to stick with PRERAM_COMPRESS at the moment.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... File src/soc/mediatek/mt8183/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... PS2, Line 87: CBFS_PRERAM_COMPRESS_FLAG
Hmmm. When I try that last time it just failed... […]
Okay, fair point (I assume you mean LZ4 *doesn't* need double space).
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... File src/soc/mediatek/mt8183/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36054/2/src/soc/mediatek/mt8183/Mak... PS2, Line 87: CBFS_PRERAM_COMPRESS_FLAG
(I assume you mean LZ4 *doesn't* need double space).
yes, hit enter unexpectedly when I was trying to add 'not'.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36054 )
Change subject: soc/mediatek/mt8183: Compress calibration blob with LZ4 ......................................................................
soc/mediatek/mt8183: Compress calibration blob with LZ4
The DRAM calibration blob can be compressed using pre-RAM algorithm (currently LZ4), which will save ~12ms in boot time.
On Kodama, boot time difference: Before: 1,082,711 After: 1,070,309
BUG=b:139099592,b:117953502 TEST=build and boot, cbfstool coreboot.rom print -v (see dram compressed) BRANCH=kukui
Change-Id: Ic3bd49d67ee6f80a0e4d8f6945744642611edf64 Signed-off-by: Hung-Te Lin hungte@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/36054 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Yu-Ping Wu yupingso@google.com Reviewed-by: Julius Werner jwerner@chromium.org --- M src/soc/mediatek/mt8183/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Julius Werner: Looks good to me, approved Yu-Ping Wu: Looks good to me, but someone else must approve
diff --git a/src/soc/mediatek/mt8183/Makefile.inc b/src/soc/mediatek/mt8183/Makefile.inc index 70fd080..e3d3db0 100644 --- a/src/soc/mediatek/mt8183/Makefile.inc +++ b/src/soc/mediatek/mt8183/Makefile.inc @@ -84,7 +84,7 @@ DRAM_CBFS := $(CONFIG_CBFS_PREFIX)/dram $(DRAM_CBFS)-file := $(MT8183_BLOB_DIR)/dram.elf $(DRAM_CBFS)-type := stage -$(DRAM_CBFS)-compression := none +$(DRAM_CBFS)-compression := $(CBFS_PRERAM_COMPRESS_FLAG) ifneq ($(wildcard $($(DRAM_CBFS)-file)),) cbfs-files-y += $(DRAM_CBFS) endif
-
Hung-Te Lin (Code Review) -
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Yu-Ping Wu (Code Review)