Re: [edk2] [PATCH v2 09/13] ArmPlatformPkg: PCD to swap red/blue format for HDLCD

[Ard Biesheuvel]

On 22 December 2017 at 18:34,   wrote:
> From: Girish Pathak 
>
> 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 
> Signed-off-by: Evan Lloyd 
> ---
>  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|0x0001|UINT32|0x0040
>
> +  ## If set, this will swap settings for HDLCD RED_SELECT and BLUE_SELECT 
> registers
> +  
> gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect|FALSE|BOOLEAN|0x0045
> +
>  [PcdsFixedAtBuild.common,PcdsDynamic.common]
>## PL031 RealTimeClock
>gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x0024
> 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

[edk2] [PATCH v2 09/13] ArmPlatformPkg: PCD to swap red/blue format for HDLCD

[evan . lloyd]

From: Girish Pathak 

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 
Signed-off-by: Evan Lloyd 
---
 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|0x0001|UINT32|0x0040
 
+  ## If set, this will swap settings for HDLCD RED_SELECT and BLUE_SELECT 
registers
+  
gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect|FALSE|BOOLEAN|0x0045
+
 [PcdsFixedAtBuild.common,PcdsDynamic.common]
   ## PL031 RealTimeClock
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x0024
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
 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