From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS, UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 91C81C433F5 for ; Fri, 17 Sep 2021 12:41:43 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C386D61056 for ; Fri, 17 Sep 2021 12:41:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C386D61056 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5E8BB8319C; Fri, 17 Sep 2021 14:41:40 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 6D25D83208; Fri, 17 Sep 2021 14:41:38 +0200 (CEST) Received: from sibelius.xs4all.nl (sibelius.xs4all.nl [83.163.83.176]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id F30D782D18 for ; Fri, 17 Sep 2021 14:41:34 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=mark.kettenis@xs4all.nl Received: from localhost (bloch.sibelius.xs4all.nl [local]) by bloch.sibelius.xs4all.nl (OpenSMTPD) with ESMTPA id 5c007735; Fri, 17 Sep 2021 14:41:28 +0200 (CEST) Date: Fri, 17 Sep 2021 14:41:28 +0200 (CEST) From: Mark Kettenis To: Heinrich Schuchardt Cc: kettenis@openbsd.org, u-boot@lists.denx.de, agust@denx.de, agraf@csgraf.de In-Reply-To: <77fcfc8f-ecb6-d503-66b2-d4015b04072e@gmx.de> (message from Heinrich Schuchardt on Fri, 17 Sep 2021 13:29:06 +0200) Subject: Re: [PATCH 2/3] efi_loader: GOP: Add 30bpp support References: <20210916130117.20894-1-kettenis@openbsd.org> <20210916130117.20894-3-kettenis@openbsd.org> <561465f19c71ab5c@bloch.sibelius.xs4all.nl> <77fcfc8f-ecb6-d503-66b2-d4015b04072e@gmx.de> Message-ID: <561466f9214bbbc9@bloch.sibelius.xs4all.nl> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean > From: Heinrich Schuchardt > Date: Fri, 17 Sep 2021 13:29:06 +0200 > > On 9/17/21 11:23 AM, Mark Kettenis wrote: > >> From: Heinrich Schuchardt > >> 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 > >>> --- > >>> 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 */ > >>> > >> > >> >