> From: Heinrich Schuchardt <[email protected]> > Date: Fri, 17 Sep 2021 13:29:06 +0200 > > On 9/17/21 11:23 AM, Mark Kettenis wrote: > >> From: Heinrich Schuchardt <[email protected]> > >> 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 <[email protected]> > >>> --- > >>> 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 */ > >>> > >> > >> >
