Attention is currently required from: Ashish Kumar Mishra, Karthik Ramasubramanian, Shelley Chen, Subrata Banik.
Julius Werner has posted comments on this change by Ashish Kumar Mishra. ( https://review.coreboot.org/c/coreboot/+/83420?usp=email )
Change subject: vc/google/chromeos: Add configurable compression for logo file in cbfs ......................................................................
Patch Set 2:
(2 comments)
Patchset:
PS2:
Is there really a platform where the most efficient compression algorithm for ramstage is LZMA but […]
Hmm... okay, so you're saying LZ4 is faster, but it doesn't fit when everything is LZ4, and so you want individual options for every file so that you can at least make as many files LZ4 as possible without overflowing? Okay, I get that, I guess. It's not pretty but I guess it's what we need.
Let's at least try to match the defaults so that everything follows the ramstage compression option by default unless explicitly overridden.
File src/vendorcode/google/chromeos/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/83420/comment/3c273dc4_544786f6?usp... : PS2, Line 29: ifeq ($(CONFIG_BMP_LOGO_COMPRESS_LZMA),y) : cb_logo.bmp-compression := LZMA : else ifeq ($(CONFIG_BMP_LOGO_COMPRESS_LZ4),y) : cb_logo.bmp-compression := LZ4 : endif :
BMP_LOGO_COMPRESS_FLAG := none […]
+1 to what Karthik said
I'm not sure why you need a HAVE_... option in addition to the choice itself? I think you should just need the choice. Just make the defaults match the normal ramstage defaults: ``` choice ... default BMP_LOGO_COMPRESS_LZMA if COMPRESS_RAMSTAGE_LZMA default BMP_LOGO_COMPRESS_LZ4 if COMPRESS_RAMSTAGE_LZ4 default BMP_LOGO_COMPRESS_NONE ```