From: Heinrich Schuchardt xypron.glpk@gmx.de Date: Fri, 17 Sep 2021 13:29:06 +0200
On 9/17/21 11:23 AM, Mark Kettenis wrote:
From: Heinrich Schuchardt xypron.glpk@gmx.de Date: Fri, 17 Sep 2021 04:56:31 +0200
Hi Heinrich,
On 9/16/21 3:01 PM, Mark Kettenis wrote:
Provide correct framebuffer information for 30bpp modes.
This is not enough to get a correct GOP implementation for the 30bpp mode.
Have a look at efi_gop_pixel efi_vid16_to_blt_col() and efi_blt_col_to_vid16() and where they are used.
Ah right. This does indeed not support EFI_GRAPHICS_OUTPUT_PROTOCOL.Blt() correctly. I shouldn't be too hard to translate between XRGB2101010 and XRGB8888. Any ideas how I could test that code?
setenv efi_selftest block image transfer bootefi selftest
should show an animated submarine with yellow portholes similar to
https://gist.github.com/xypron/7e1f73408465da71e3ef946250e76776#file-sub-png
Best regards
Cool thanks. Looks like my implementation works! I'll wait a bit for feedback on the other diffs in this series before sending a v2 with that.
Cheers,
Mark
Signed-off-by: Mark Kettenis kettenis@openbsd.org
lib/efi_loader/efi_gop.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c index 1206b2d7a2..42bf49b184 100644 --- a/lib/efi_loader/efi_gop.c +++ b/lib/efi_loader/efi_gop.c @@ -227,6 +227,7 @@ static efi_uintn_t gop_get_bpp(struct efi_gop *this)
switch (gopobj->bpix) {#ifdef CONFIG_DM_VIDEO
- case VIDEO_BPP30: case VIDEO_BPP32: #else case LCD_COLOR32:
@@ -468,6 +469,7 @@ efi_status_t efi_gop_register(void) switch (bpix) { #ifdef CONFIG_DM_VIDEO case VIDEO_BPP16:
- case VIDEO_BPP30: case VIDEO_BPP32: #else case LCD_COLOR32:
@@ -518,6 +520,14 @@ efi_status_t efi_gop_register(void) #endif { gopobj->info.pixel_format = EFI_GOT_BGRA8; +#ifdef CONFIG_DM_VIDEO
This symbol is not 30bpp specific. Is there a Kconfig variable that we can use to hide 30bpp support where it is not needed?
I quite deliberately didn't add a separate config option for 30bpp as there is very little code that is added to the already existing 32bpp code. In a sense 30bpp is just a different submode of the 32bpp support with just a different color map, so I thought it made sense to group it under CONFIG_VIDEO_32BPP. I did initially consider adding a separate config option for it, but it quickly turns into an #ifdef maze that makes it hard to understand the code.
The CONFIG_DM_VIDEO check is just there to make sure the 30bpp is only offered in the DM model. Based on recent discussions I expect the !CONFIG_DM_VIDEO case to disappear in the near feature so adding 30bpp support for the non-DM case doesn't make sense.
The EFI GOP code currently doesn't check the CONFIG_VIDEO_BPP16 and CONFIG_VIDEO_BPP32 defines to see which of these modes are enabled within U-Boot, so we don't "hide" 16bpp support on platforms that just support 32bpp either.
Which modes does the M1 support?
We're not sure. It does support at least 8-bit (32bpp) and 10-bit (30bpp) color depth modes. But the firmware tends to come up with 10-bit color depth whenever it can and I'm not planning to add support for changing the framebuffer mode on the M1, since the interface to do that is "interesting" [1].
[1] See Alyssa Rozenzweig's talk at XDC 2021, https://www.youtube.com/watch?v=uTZISTjqy9Q
- } else if (bpix == VIDEO_BPP30) {
gopobj->info.pixel_format = EFI_GOT_BITMASK;gopobj->info.pixel_bitmask[0] = 0x3ff00000; /* red */gopobj->info.pixel_bitmask[1] = 0x000ffc00; /* green */gopobj->info.pixel_bitmask[2] = 0x000003ff; /* blue */gopobj->info.pixel_bitmask[3] = 0xc0000000; /* reserved */+#endif } else { gopobj->info.pixel_format = EFI_GOT_BITMASK; gopobj->info.pixel_bitmask[0] = 0xf800; /* red */