From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::244; helo=mail-wm0-x244.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7899921CEB13F for ; Wed, 25 Oct 2017 07:23:21 -0700 (PDT) Received: by mail-wm0-x244.google.com with SMTP id q124so2292543wmb.0 for ; Wed, 25 Oct 2017 07:27:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=yshDrVCGKf3hCtb4afkFhBbzoL3v9S3/m1xt4s3JJLE=; b=YTr1tYXVsnx57/iasuFKOzmjZ4ZguZ2McXc2CXqda5O0Qn+dCi0NsDo8D4VafTfl2x ErPysf0sz/YF+QYrxS4j3O1G6HAOge7t857ENyNviGxC/jkjbalxDcRcUjNwLQN4914n 4npqC1ku2AbNiHlGMpUeUD2PbbYpd5E2frrW8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=yshDrVCGKf3hCtb4afkFhBbzoL3v9S3/m1xt4s3JJLE=; b=Ma99DPgPOxLQYUEYSudnq+nIwUNilRJHgXGyIjwSQv0QNpwiWYmpx0v0x/IpPMT3D9 eR658Y+wYToYMN/NfqG0yKan90Klj4G26fvYrOuAerrhqppXB0Zk7JmgGn9ycDLSikII cslQIbBJkYhwmXwbWiQw4qCbo7NYGiK8HIQfrqHyW4ChVi4UCmAAEyMQXv2U5Eze1Htx lNSo4JBG1jFw/KKcDbr84FpJxFRC2e7LWRfNN8nWK2er2WUvFhS/NyYSd9Zgl9F9h2YH VwKbm6nEym5IHweAzZ1VGNhnf0r44C90McjkTxxYrgtP0tIU7hsv+w6pP+WslU5BX3bX Xdyw== X-Gm-Message-State: AMCzsaUFdUb/FghSorFvZv306MNboikoQkYiltm8VgOu1tfWYq3E8IYn QnI9CPMKPgmliHsWbMrlGkHOHw== X-Google-Smtp-Source: ABhQp+R70apLvMg2b2fuCKFgDNcZS1A8Y8up6/ETMY8vdvjAfeFyoKwTb/4L8oyHjfsSMi8fvkApyw== X-Received: by 10.28.236.203 with SMTP id h72mr2119313wmi.147.1508941624344; Wed, 25 Oct 2017 07:27:04 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id i16sm3462220wrf.19.2017.10.25.07.27.02 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 25 Oct 2017 07:27:03 -0700 (PDT) Date: Wed, 25 Oct 2017 15:27:01 +0100 From: Leif Lindholm To: evan.lloyd@arm.com Cc: edk2-devel@lists.01.org Message-ID: <20171025142701.eyebi4qycxnisvue@bivouac.eciton.net> References: <20170926201529.11644-1-evan.lloyd@arm.com> <20170926201529.11644-15-evan.lloyd@arm.com> MIME-Version: 1.0 In-Reply-To: <20170926201529.11644-15-evan.lloyd@arm.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH 14/19] ArmPlatformPkg: Add PCD to select pixel format X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Oct 2017 14:23:22 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Sep 26, 2017 at 09:15:24PM +0100, evan.lloyd@arm.com wrote: > From: Girish Pathak > > Current HDLCD and PL111 platform libraries do not support display modes > with PixelBlueGreenRedReserved8BitPerColor format. i.e. because of > historical confusion, they do not support the UEFI default! > > In LcdPlatformLib for PL111, LcdPlatformQueryMode returns the pixel > format as PixelRedGreenBlueReserved8BitPerColor which is wrong, because > that does not match the display controller's pixel format which is set > to BGR in PL111Lcd GOP driver. > > Also it is not possible to configure pixel format as RGB/BGR for the > display modes for a platform at build time. > > This change adds PcdGopPixelFormat to configure pixel format as > PixelRedGreenBlueReserved8BitPerColor or > PixelBlueGreenRedReserved8BitPerColor or > PixelBitMask. > With this change, pixel format can be selected in the platform specific > .dsc file for all supported display modes. > > Support for PixelBitMask is not implemented in PL111 or HDLCD > GOP driver, hence HDLCD and PL111 platform libraries will return error > EFI_UNSUPPORTED if PcdGopPixelFormat is set to PixelBitMask. > Indeed, it is not clear what selecting PixelBitMask might mean, but > the option is allowed as it might suit a custom platform. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Girish Pathak > Signed-off-by: Evan Lloyd > --- > ArmPlatformPkg/ArmPlatformPkg.dec | 9 ++++- > ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf | 1 + > ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf | 1 + > ArmPlatformPkg/Include/Library/LcdPlatformLib.h | 6 +++- > ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c | 23 ++++++++---- > ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 37 +++++++++----------- > ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c | 32 +++++++++++------ > ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c | 16 ++++++++- > 8 files changed, 83 insertions(+), 42 deletions(-) > > diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec > index b8a6b131632dc6788b73a034a41b9e3176dffafa..548d2b211753275337c6973e05227cee8451c185 100644 > --- a/ArmPlatformPkg/ArmPlatformPkg.dec > +++ b/ArmPlatformPkg/ArmPlatformPkg.dec > @@ -1,6 +1,6 @@ > #/** @file > # > -# Copyright (c) 2011-2016, ARM Limited. All rights reserved. > +# Copyright (c) 2011-2017, ARM Limited. All rights reserved. > # Copyright (c) 2015, Intel Corporation. All rights reserved. > # > # This program and the accompanying materials > @@ -126,6 +126,13 @@ [PcdsFixedAtBuild.common] > gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L""|VOID*|0x0000001B > gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L""|VOID*|0x0000001C > > + # Graphics Output Pixel format > + # 0 : PixelRedGreenBlueReserved8BitPerColor > + # 1 : PixelBlueGreenRedReserved8BitPerColor > + # 2 : PixelBitMask > + # Default is set to UEFI console font format PixelBlueGreenRedReserved8BitPerColor > + gArmPlatformTokenSpaceGuid.PcdGopPixelFormat|0x00000001|UINT32|0x00000040 > + > [PcdsFixedAtBuild.common,PcdsDynamic.common] > ## PL031 RealTimeClock > gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x00000024 > diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf > index bba851c9bd6089cee748b45f40599b24c1293785..37756481596c7e978ed9ed0a932eeb2aa0a3b657 100644 > --- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf > +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf > @@ -42,3 +42,4 @@ [Protocols] > [FixedPcd] > gArmVExpressTokenSpaceGuid.PcdPL111LcdMaxMode > gArmVExpressTokenSpaceGuid.PcdHdLcdVideoModeOscId > + gArmPlatformTokenSpaceGuid.PcdGopPixelFormat > diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf > index 1a044baf4698aa6bfa5cd6038f01e84f7a633ea9..6f1cb3b55ff814d007718b5597f821dd20100477 100644 > --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf > +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf > @@ -42,3 +42,4 @@ [Protocols] > [FixedPcd] > gArmVExpressTokenSpaceGuid.PcdPL111LcdMaxMode > gArmVExpressTokenSpaceGuid.PcdPL111LcdVideoModeOscId > + gArmPlatformTokenSpaceGuid.PcdGopPixelFormat > diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h > index f2f345b18fd15f2cde159fd42d3208a28f598a1f..d357c22c46b62966859793372c447883e12e1e80 100644 > --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h > +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h > @@ -139,7 +139,6 @@ > #define LCD_12BPP_444_BLUE_MASK 0x0000000F > #define LCD_12BPP_444_RESERVED_MASK 0x0000F000 > > - Spurious whitespace change adding an extra hunk to the patch, please drop. > // The enumeration indexes maps the PL111 LcdBpp values used in the LCD Control Register > typedef enum { > LCD_BITS_PER_PIXEL_1 = 0, > @@ -165,6 +164,10 @@ typedef struct { > * @param IN Handle Handle to the LCD device instance. > * > * @retval EFI_SUCCESS Platform initialization success. > + * @retval EFI_UNSUPPORTED PcdGopPixelFormat must be > + * PixelRedGreenBlueReserved8BitPerColor OR > + * PixelBlueGreenRedReserved8BitPerColor > + * any other format is not supported. > * @retval !(EFI_SUCCESS) Other errors. > **/ > EFI_STATUS > @@ -216,6 +219,7 @@ LcdPlatformSetMode ( > * (on success). > * > * @retval EFI_SUCCESS Success if the requested mode is found. > + * @retval EFI_INVALID_PARAMETER Info is NULL. > * @retval EFI_INVALID_PARAMETER Requested mode not found. > **/ > EFI_STATUS > diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c > index 2401cdb30cb7252964ce1f363aa26d99933c09be..07730ed0d7a84b3fb64047bd1013fbc97301db53 100644 > --- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c > +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c > @@ -15,7 +15,6 @@ > #include > > #include > -#include > #include > #include > #include > @@ -91,6 +90,10 @@ EFI_EDID_ACTIVE_PROTOCOL mEdidActive = { > /** HDLCD Platform specific initialization function. > * > * @retval EFI_SUCCESS Plaform library initialization success. > + * @retval EFI_UNSUPPORTED PcdGopPixelFormat must be > + * PixelRedGreenBlueReserved8BitPerColor OR > + * PixelBlueGreenRedReserved8BitPerColor > + * any other format is not supported. > * @retval !(EFI_SUCCESS) Other errors. > **/ > EFI_STATUS > @@ -99,6 +102,7 @@ LcdPlatformInitializeDisplay ( > ) > { > EFI_STATUS Status; > + EFI_GRAPHICS_PIXEL_FORMAT PixelFormat; > > /* Set the FPGA multiplexer to select the video output from the > * motherboard or the daughterboard */ > @@ -120,6 +124,16 @@ LcdPlatformInitializeDisplay ( > NULL > ); > > + // PixelBitMask and PixelBltOnly pixel formats are not supported > + PixelFormat = FixedPcdGet32 (PcdGopPixelFormat); > + if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor > + && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) { > + > + ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor > + || PixelFormat == PixelBlueGreenRedReserved8BitPerColor); > + return EFI_UNSUPPORTED; > + } > + This block looks identical to a similar block in the next file. Could this be a shared helper function? > return Status; > } > > @@ -282,12 +296,7 @@ LcdPlatformQueryMode ( > Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution; > Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution; > > - /* Bits per Pixel is always LCD_BITS_PER_PIXEL_24 */ > - Info->PixelFormat = PixelRedGreenBlueReserved8BitPerColor; > - Info->PixelInformation.RedMask = LCD_24BPP_RED_MASK; > - Info->PixelInformation.GreenMask = LCD_24BPP_GREEN_MASK; > - Info->PixelInformation.BlueMask = LCD_24BPP_BLUE_MASK; > - Info->PixelInformation.ReservedMask = LCD_24BPP_RESERVED_MASK; > + Info->PixelFormat = FixedPcdGet32 (PcdGopPixelFormat); > > return EFI_SUCCESS; > } > diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c > index 753c5b615361f83625cdd4f0506909721da014b6..31d5e037e42a51f1c08e5bf2fa22eeb7be23e45f 100644 > --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c > +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c > @@ -149,6 +149,10 @@ EFI_EDID_ACTIVE_PROTOCOL mEdidActive = { > /** PL111 Platform specific initialization function. > * > * @retval EFI_SUCCESS Plaform library initialization success. > + * @retval EFI_UNSUPPORTED PcdGopPixelFormat must be > + * PixelRedGreenBlueReserved8BitPerColor OR > + * PixelBlueGreenRedReserved8BitPerColor > + * any other format is not supported > * @retval !(EFI_SUCCESS) Other errors. > **/ > EFI_STATUS > @@ -157,6 +161,7 @@ LcdPlatformInitializeDisplay ( > ) > { > EFI_STATUS Status; > + EFI_GRAPHICS_PIXEL_FORMAT PixelFormat; > > // Set the FPGA multiplexer to select the video output from the motherboard or the daughterboard > Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, PL111_CLCD_SITE); > @@ -172,6 +177,16 @@ LcdPlatformInitializeDisplay ( > ); > } > > + // PixelBitMask and PixelBltOnly pixel formats are not supported > + PixelFormat = FixedPcdGet32 (PcdGopPixelFormat); > + if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor > + && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) { > + > + ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor > + || PixelFormat == PixelBlueGreenRedReserved8BitPerColor); > + return EFI_UNSUPPORTED; > + } > + (This one.) > return Status; > } > > @@ -366,27 +381,7 @@ LcdPlatformQueryMode ( > Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution; > Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution; > > - switch (mDisplayModes[ModeNumber].Bpp) { > - case LCD_BITS_PER_PIXEL_24: > - Info->PixelFormat = PixelRedGreenBlueReserved8BitPerColor; > - Info->PixelInformation.RedMask = LCD_24BPP_RED_MASK; > - Info->PixelInformation.GreenMask = LCD_24BPP_GREEN_MASK; > - Info->PixelInformation.BlueMask = LCD_24BPP_BLUE_MASK; > - Info->PixelInformation.ReservedMask = LCD_24BPP_RESERVED_MASK; > - break; > - > - case LCD_BITS_PER_PIXEL_16_555: > - case LCD_BITS_PER_PIXEL_16_565: > - case LCD_BITS_PER_PIXEL_12_444: > - case LCD_BITS_PER_PIXEL_8: > - case LCD_BITS_PER_PIXEL_4: > - case LCD_BITS_PER_PIXEL_2: > - case LCD_BITS_PER_PIXEL_1: > - default: > - // These are not supported > - ASSERT (FALSE); > - break; > - } > + Info->PixelFormat = FixedPcdGet32 (PcdGopPixelFormat); > > return EFI_SUCCESS; > } > diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c > index 03153c06d314cb497c91889386ca6075c0c9f718..3ea7feca439e7ae9a610b6b3139ddbfad3ac6f9c 100644 > --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c > +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c > @@ -22,6 +22,8 @@ > > #include "LcdGraphicsOutputDxe.h" > > +#define BYTES_PER_PIXEL 4 > + > /********************************************************************** > * > * This file contains all the bits of the Lcd that are > @@ -66,10 +68,6 @@ LcdInitialize ( > HDLCD_LITTLE_ENDIAN | HDLCD_4BYTES_PER_PIXEL > ); > > - MmioWrite32 (HDLCD_REG_RED_SELECT, (0 << 16 | 8 << 8 | 0)); > - MmioWrite32 (HDLCD_REG_GREEN_SELECT, (0 << 16 | 8 << 8 | 8)); > - MmioWrite32 (HDLCD_REG_BLUE_SELECT, (0 << 16 | 8 << 8 | 16)); > - > return EFI_SUCCESS; > } > > @@ -77,7 +75,8 @@ LcdInitialize ( > * > * @param ModeNumber Display mode number. > * @retval EFI_SUCCESS Display set mode success. > - * @retval EFI_DEVICE_ERROR If mode not found/supported. > + * @retval EFI_DEVICE_ERROR Mode not found/supported. > + * @retval !EFI_SUCCESS Other errors. > **/ > EFI_STATUS > LcdSetMode ( > @@ -87,9 +86,8 @@ LcdSetMode ( > EFI_STATUS Status; > CONST SCAN_TIMINGS *Horizontal; > CONST SCAN_TIMINGS *Vertical; > - UINT32 BytesPerPixel; > - LCD_BPP LcdBpp; > > + EFI_GRAPHICS_OUTPUT_MODE_INFORMATION ModeInfo; > > // Set the video mode timings and other relevant information > Status = LcdPlatformGetTimings ( > @@ -104,13 +102,22 @@ LcdSetMode ( > ASSERT (Horizontal != NULL); > ASSERT (Vertical != NULL); > > - Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp); > + // Get the pixel format information. > + Status = LcdPlatformQueryMode (ModeNumber, &ModeInfo); > if (EFI_ERROR (Status)) { > ASSERT_EFI_ERROR (Status); > return Status; > } > > - BytesPerPixel = GetBytesPerPixel (LcdBpp); > + if (ModeInfo.PixelFormat == PixelBlueGreenRedReserved8BitPerColor) { > + MmioWrite32 (HDLCD_REG_RED_SELECT, (8 << 8 | 16)); > + MmioWrite32 (HDLCD_REG_BLUE_SELECT, (8 << 8 | 0)); > + } else { > + MmioWrite32 (HDLCD_REG_BLUE_SELECT, (8 << 8 | 16)); > + MmioWrite32 (HDLCD_REG_RED_SELECT, (8 << 8 | 0)); > + } > + > + MmioWrite32 (HDLCD_REG_GREEN_SELECT, (8 << 8 | 8)); Could we have some parentheses above, to clarify (intended) precedence? > > // Disable the controller > MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_DISABLE); > @@ -118,10 +125,13 @@ LcdSetMode ( > // Update the frame buffer information with the new settings > MmioWrite32 ( > HDLCD_REG_FB_LINE_LENGTH, > - Horizontal->Resolution * BytesPerPixel > + Horizontal->Resolution * BYTES_PER_PIXEL > ); > > - MmioWrite32 (HDLCD_REG_FB_LINE_PITCH, Horizontal->Resolution * BytesPerPixel); > + MmioWrite32 ( > + HDLCD_REG_FB_LINE_PITCH, > + Horizontal->Resolution * BYTES_PER_PIXEL > + ); > > MmioWrite32 (HDLCD_REG_FB_LINE_COUNT, Vertical->Resolution - 1); > > diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c > index 6de60491e9fd0c5bca71e743aac2862ff85f6e7e..4bad2367982e16d5d23c4eab2e6d91bf7db1c031 100644 > --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c > +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c > @@ -80,6 +80,7 @@ LcdInitialize ( > * @param ModeNumber Display mode number. > * @retval EFI_SUCCESS Display set mode success. > * @retval EFI_DEVICE_ERROR If mode not found/supported. > + @retval !EFI_SUCCESS Other errors. > **/ > EFI_STATUS > LcdSetMode ( > @@ -92,6 +93,8 @@ LcdSetMode ( > UINT32 LcdControl; > LCD_BPP LcdBpp; > > + EFI_GRAPHICS_OUTPUT_MODE_INFORMATION ModeInfo; > + > // Set the video mode timings and other relevant information > Status = LcdPlatformGetTimings ( > ModeNumber, > @@ -111,6 +114,13 @@ LcdSetMode ( > return Status; > } > > + // Get the pixel format information. > + Status = LcdPlatformQueryMode (ModeNumber, &ModeInfo); > + if (EFI_ERROR (Status)) { > + ASSERT_EFI_ERROR (Status); > + return Status; > + } > + > // Disable the CLCD_LcdEn bit > MmioAnd32 (PL111_REG_LCD_CONTROL, ~PL111_CTRL_LCD_EN); > > @@ -144,7 +154,11 @@ LcdSetMode ( > > // PL111_REG_LCD_CONTROL > LcdControl = PL111_CTRL_LCD_EN | PL111_CTRL_LCD_BPP (LcdBpp) > - | PL111_CTRL_LCD_TFT | PL111_CTRL_BGR; > + | PL111_CTRL_LCD_TFT; This is not a function call, so I do not believe the continuation line should be indented. / Leif > + > + if (ModeInfo.PixelFormat == PixelBlueGreenRedReserved8BitPerColor) { > + LcdControl |= PL111_CTRL_BGR; > + } > > MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl); > > -- > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") >