On 22 December 2017 at 18:34, <evan.lloyd@arm.com> wrote:
> 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>
> ---
> ArmPlatformPkg/ArmPlatformPkg.dec | 3 +++
> ArmPlatformPkg/Library/HdLcd/HdLcd.inf | 2 ++
> ArmPlatformPkg/Library/HdLcd/HdLcd.c | 4 ++++
> 3 files changed, 9 insertions(+)
>
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
> index e412414e0ba6506c7158e69bac04469e45601736..9a61e4a511024c80e787500de5038779363f0d95 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|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/Library/HdLcd/HdLcd.inf b/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> index 67aad05d210b95b2d23b8e52e4392685efcf3795..0f440d1ef730159a8be3780667700979b607a1e2 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.inf
> @@ -40,3 +40,5 @@ [LibraryClasses]
>
> [FixedPcd]
> gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase
> + gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect
> +
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> index 72cd5fa33b2553195638c595e72843a56b2e267c..4f802977e8b55ed83364d3ec8fa46082e128c76b 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> @@ -98,7 +98,11 @@ LcdSetMode (
> return Status;
> }
>
> +#if (!FixedPcdGetBool (PcdArmHdLcdSwapBlueRedSelect))
> if (ModeInfo.PixelFormat == PixelBlueGreenRedReserved8BitPerColor) {
> +#else
> + if (ModeInfo.PixelFormat == PixelRedGreenBlueReserved8BitPerColor) {
> +#endif
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)
I suggest you use a variable for the RHS of the inner if, or find some
other way to get rid of the #if/#else/#endif
Thanks,
Ard.
> MmioWrite32 (HDLCD_REG_RED_SELECT, (8 << 8) | 16);
> MmioWrite32 (HDLCD_REG_BLUE_SELECT, (8 << 8) | 0);
> } else {
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel