Re: [PATCH v3 11/16] ArmPlatformPkg: PCD to swap red/blue format for HDLCD

[Evan Lloyd <Evan.Lloyd@arm.com>]

Reviewed-by: Evan Lloyd <evan.lloyd@arm.com>

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Girish
> Pathak
> Sent: 20 March 2018 16:12
> To: edk2-devel@lists.01.org
> Cc: nd <nd@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-
> Fitt@arm.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org
> Subject: [edk2] [PATCH v3 11/16] ArmPlatformPkg: PCD to swap red/blue
> format for HDLCD
> 
> From: Girish Pathak <girish.pathak at arm.com>
> 
> This change adds a new PCD PcdArmHdlcdSwapBlueRedSelect to swap
> values for HDLCD RED_SELECT and BLUE_SELECT registers on platforms
> where blue and red hardware lines are swapped.
> 
> If set to TRUE in the platform dsc, HDLCD library will swap the values while
> setting RED_SELECT and BLUE_SELECT registers. The default value of the
> PCD is FALSE.
> 
> NOTE: The motive for this is that a discrepancy in the Red/Blue lines exists
> between some VersatileExpress platforms.  Rather than have divergent code,
> this build switch allows a simple, pragmatic solution.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak <girish.pathak@arm.com>
> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
> ---
> 
> Notes:
>     v3:
>     - Please don't nest CPP and C conditionals like this.
>       It is difficult to follow, and results in poor build
>       time coverage (the non-taken branch at the CPP level
>       is never seen by the compiler)                       [Ard]
> 
>       Done                                                 [Girish]
> 
>  ArmPlatformPkg/ArmPlatformPkg.dec      |  3 +++
>  ArmPlatformPkg/Library/HdLcd/HdLcd.c   | 11 ++++++++++-
>  ArmPlatformPkg/Library/HdLcd/HdLcd.inf |  4 +++-
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec
> b/ArmPlatformPkg/ArmPlatformPkg.dec
> index
> 378bee9cbc9e4bd50c37b38156016424e24cba73..5231ea822f05c2f281a6
> 190d6eae0fc7d0bc0cb3 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -104,6 +104,9 @@ [PcdsFixedAtBuild.common]
>    # Default is set to UEFI console font format
> PixelBlueGreenRedReserved8BitPerColor
> 
> gArmPlatformTokenSpaceGuid.PcdGopPixelFormat|0x00000001|UINT32|0
> x00000040
> 
> +  ## If set, this will swap settings for HDLCD RED_SELECT and
> + BLUE_SELECT registers
> +
> +
> gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect|FALSE|BO
> OLEAN|
> + 0x00000045
> +
>  [PcdsFixedAtBuild.common,PcdsDynamic.common]
>    ## PL031 RealTimeClock
> 
> gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x00000024
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> index
> 96f2bf437fbabd2509f860c67c5442def5b5f03d..5396dde3ba6cd147a8333
> 241a9bc71ab05d7fee3 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> @@ -73,6 +73,8 @@ LcdSetMode (
>    SCAN_TIMINGS      *Horizontal;
>    SCAN_TIMINGS      *Vertical;
> 
> +  EFI_GRAPHICS_PIXEL_FORMAT  PixelFormat;
> +
>    EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  ModeInfo;
> 
>    // Set the video mode timings and other relevant information @@ -96,7
> +98,14 @@ LcdSetMode (
>      return Status;
>    }
> 
> -  if (ModeInfo.PixelFormat == PixelBlueGreenRedReserved8BitPerColor) {
> +  // By default PcdArmHdLcdSwapBlueRedSelect is set to false  //
> + However on the Juno platform HW lines for BLUE and RED are swapped  //
> + Therefore PcdArmHdLcdSwapBlueRedSelect is set to TRUE for the Juno
> + platform  PixelFormat = FixedPcdGetBool
> (PcdArmHdLcdSwapBlueRedSelect)
> +                ? PixelRedGreenBlueReserved8BitPerColor
> +                : PixelBlueGreenRedReserved8BitPerColor;
> +
> +  if (ModeInfo.PixelFormat == PixelFormat) {
>      MmioWrite32 (HDLCD_REG_RED_SELECT,  (8 << 8) | 16);
>      MmioWrite32 (HDLCD_REG_BLUE_SELECT, (8 << 8) | 0);
>    } else {
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> index
> 67aad05d210b95b2d23b8e52e4392685efcf3795..7f2ba7bf1c602f4c214ea
> caa6425bf9ec7e6da15 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> @@ -2,7 +2,7 @@
>  #
>  #  Component description file for HDLCD module  # -#  Copyright (c) 2011-
> 2012, ARM Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2011-2018, ARM Ltd. All rights reserved.<BR>
>  #
>  #  This program and the accompanying materials  #  are licensed and made
> available under the terms and conditions of the BSD License @@ -40,3
> +40,5 @@ [LibraryClasses]
> 
>  [FixedPcd]
>    gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase
> +  gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect
> +
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel