Enable Full Screen Splash Images

[bebon901]

This will allow users to enter a cmdline value which will enable the kernel to read a PPM file, and then rendering it fullscreen at the early boot stage.
I am aware it needs a lot of tidying up, but put here for others to reference!

[timg236]

I think it would be preferable (more efficient and smaller code) to use a binary format rather than parsing PPM. TGA is very simple and supports palettes.

[6by9]

There may already be part of this implemented for uefi in drivers/video/fbdev/efifb.c for allowing a vendor specific bitmap to be passed in such that it can be merged into the boot screens. That's how you can get say "Asus" as the background whilst Ubuntu is starting.
I can't remember the full runes, but the BMP file gets exposed as /sys/firmware/acpi/bgrt

Loading the file using the request_firmware calls to load from /lib/firmware (which can be in the initramfs) is far cleaner than messing with filp_open calls for yourself.

[pelwell]

I'm not going to give this a full review until you've tidied it up some - it's too tiring otherwise.

A few kernel coding rules (see https://www.kernel.org/doc/html/v6.12/process/coding-style.html for more):

1. Use TABs for indentation, with tab stops set to 8. Spaces can be used to precise alignment, but only after any TABs.
2. You have the preferred brace style and no trailing whitespace, so that's good.
3. Your local variable declarations are also pretty good, but for full marks put them in "reverse Christmas tree" format, longest to shortest, making sure there is a blank line before any statements.
4. Make all local-scope functions static, and ideally with a module-specific prefix.
5. Prefer pr_info (or dev_info) over printk(KERN_INFO .

scripts/checkpatch.pl -g <from-commit>..<to-commit> is your fussy friend.

[bebon901]

There may already be part of this implemented for uefi in drivers/video/fbdev/efifb.c for allowing a vendor specific bitmap to be passed in such that it can be merged into the boot screens. That's how you can get say "Asus" as the background whilst Ubuntu is starting. I can't remember the full runes, but the BMP file gets exposed as /sys/firmware/acpi/bgrt

Loading the file using the request_firmware calls to load from /lib/firmware (which can be in the initramfs) is far cleaner than messing with filp_open calls for yourself.

I've had a look around the efifb.c file, and it looks like it's ultimately expecting the data from the ACPI Boot Graphics Resource Table - which following a conversation with Tom, looks like a path we don't want to go down.
Will have a look into the request_firmware option though!

[6by9]

I've had a look around the efifb.c file, and it looks like it's ultimately expecting the data from the ACPI Boot Graphics Resource Table - which following a conversation with Tom, looks like a path we don't want to go down. Will have a look into the request_firmware option though!

I was more thinking if we have the BMP parser etc already in the kernel, it'd be nicer for that to be abstracted out and reused.
I will say that I haven't looked at the detail for whether it actually achieves the same goal as you have here.

[bebon901]

Have now tidied up, and is ready for review @pelwell

Also, it complains about creating a new typedef - what would you like to do about this? - I know a previous comment suggested creating typedef long ImagePalette[224][3]

[pelwell]

This is looking much better. You could replace the typedef with a structure whose only member is the two-dimensional array. And on that subject, is there any reason not to make it an array of u8/uint8_ts? Using longs is wasteful and confusing, in the sense that it makes the reader wonder where you use values large enough to justify their use.

[bebon901]

Thanks! I've updated it to include your suggestions.

[hex0id]

when are you guys merging this in?