Re: [PATCH 15/19] ArmPlatformPkg: PCD to swap red/blue format for HDLCD

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Sep 26, 2017 at 09:15:25PM +0100, evan.lloyd@arm.com wrote:
> From: Girish Pathak <girish.pathak@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 driver 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.

Notes are generally a better fit for under the first --- line.
I have no other comments on this patch, so it's not worth updating it
for, just to keep in mind for future.



> 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>
> ---
>  ArmPlatformPkg/ArmPlatformPkg.dec                                      | 3 +++
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf | 1 +
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c                    | 4 ++++
>  3 files changed, 8 insertions(+)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 548d2b211753275337c6973e05227cee8451c185..77eb789ad8fe4ddcbf25abefad2e7b7d3d5e1722 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -133,6 +133,9 @@ [PcdsFixedAtBuild.common]
>    # Default is set to UEFI console font format PixelBlueGreenRedReserved8BitPerColor
>    gArmPlatformTokenSpaceGuid.PcdGopPixelFormat|0x00000001|UINT32|0x00000040
>  
> +  ## If set, this will swap settings for HDLCD RED_SELECT and BLUE_SELECT registers
> +  gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect|FALSE|BOOLEAN|0x00000045
> +
>  [PcdsFixedAtBuild.common,PcdsDynamic.common]
>    ## PL031 RealTimeClock
>    gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x00000024
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf
> index 26e580a594fc328187407ac4c1787f180fbf4b17..b050add98e3f9cf62ac335883193b9d0928c3dc9 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf
> @@ -55,6 +55,7 @@ [Protocols]
>  
>  [FixedPcd]
>    gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase
> +  gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect
>  
>  [FeaturePcd]
>    gArmPlatformTokenSpaceGuid.PcdGopDisableOnExitBootServices
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> index 3ea7feca439e7ae9a610b6b3139ddbfad3ac6f9c..8f83d6ecba290994dbced1c11dfdb6d705323c03 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> @@ -109,7 +109,11 @@ LcdSetMode (
>      return Status;
>    }
>  
> +#if (!FixedPcdGetBool (PcdArmHdLcdSwapBlueRedSelect))
>    if (ModeInfo.PixelFormat == PixelBlueGreenRedReserved8BitPerColor) {
> +#else
> +  if (ModeInfo.PixelFormat == PixelRedGreenBlueReserved8BitPerColor) {
> +#endif
>      MmioWrite32 (HDLCD_REG_RED_SELECT,  (8 << 8 | 16));
>      MmioWrite32 (HDLCD_REG_BLUE_SELECT, (8 << 8 | 0));
>    } else {
> -- 
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>