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=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,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 78116C433EF for ; Fri, 17 Sep 2021 09:23:22 +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 54254610A6 for ; Fri, 17 Sep 2021 09:23:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 54254610A6 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 45D5E831DB; Fri, 17 Sep 2021 11:23:18 +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 97B4982E03; Fri, 17 Sep 2021 11:23:16 +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 B14D582E03 for ; Fri, 17 Sep 2021 11:23:12 +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 2cb20b60; Fri, 17 Sep 2021 11:23:11 +0200 (CEST) Date: Fri, 17 Sep 2021 11:23:11 +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: (message from Heinrich Schuchardt on Fri, 17 Sep 2021 04:56:31 +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> Message-ID: <561465f19c71ab5c@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 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? > > 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 */ > > > >