Re: [PATCH 01/19] ArmPlatformPkg: Tidy LcdGraphicsOutputDxe code: Coding standard

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

On Tue, Sep 26, 2017 at 09:15:11PM +0100, evan.lloyd@arm.com wrote:
> From: Girish Pathak <girish.pathak@arm.com>
> 
> There is no functional modification in this change
> As preparation for further work, the formatting is corrected to meet
> the EDKII coding standard.
> Of specific note, some invalid include guards were fixed.

Subjects says to be cleaning LcdGraphicsOutputDxe, but that is only
one of the modules modified.

> 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/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf       |   9 +-
>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf |   6 +-
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf                  |   4 +-
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf               |   4 +-
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h                      |  11 +-
>  ArmPlatformPkg/Include/Library/LcdPlatformLib.h                                         |  10 +-
>  ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c            | 103 +++++++-----
>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c      | 173 ++++++++++++--------
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c                                     |  83 ++++++----
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c                      | 130 ++++++++-------
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c                                  |  54 ++++--
>  11 files changed, 350 insertions(+), 237 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
> index dff17e86fd3e563b38318f696a94f2c75276b31f..4733bb8e662d64eca0976af21b2abb7036b4424b 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
> @@ -1,8 +1,8 @@
> -#/** @file
> +#/** @file HdLcdArmVExpress.inf
>  #
> -#  Component description file for HdLcdArmLib module
> +#  Component description file for HdLcdArmVExpress module
>  #
> -#  Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2011-2017, 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
> @@ -23,8 +23,7 @@ [Defines]
>    LIBRARY_CLASS                  = LcdPlatformLib
>  
>  [Sources.common]
> -
> -HdLcdArmVExpress.c
> +  HdLcdArmVExpress.c
>  
>  [Packages]
>    MdePkg/MdePkg.dec
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
> index 658558ab15230d07f1c04d29a8e2bf8d14f1d6a2..3fde707c33dbcbd8adbbf18bbba718b823194abc 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
> @@ -1,8 +1,8 @@
> -#/** @file
> +#/** @file PL111LcdArmVExpressLib.inf
>  #
> -#  Component description file for ArmVeGraphicsDxe module
> +#  Component description file for PL111LcdArmVExpressLib module
>  #
> -#  Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2011-2017, 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
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf
> index 462d1fa402d7a1939b2876b74a5b76e8edf2e1f6..26e580a594fc328187407ac4c1787f180fbf4b17 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf
> @@ -1,8 +1,8 @@
> -#/** @file
> +#/** @file HdLcdGraphicsOutputDxe.inf
>  #
>  #  Component description file for HDLCD module
>  #
> -#  Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2011-2017, 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
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf
> index 003cc2ffa912b39d6f1342c92445eefce94b1f8b..ad0348500326c4567f0e1b235c84b694e61306bf 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf
> @@ -1,8 +1,8 @@
> -#/** @file
> +#/** @file PL111LcdGraphicsOutputDxe.inf
>  #
>  #  Component description file for PL111LcdGraphicsOutputDxe module
>  #
> -#  Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2011-2017, 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
>  #  which accompanies this distribution.  The full text of the license may be found at

I guess there has been some sort of rebase accident with the above 2
files - they update the copyright only.

> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h
> index 8856b79901b632071ae4c4081b69e5188daa23ed..85e918de66624d61c6d0e05c5a67c516cd7619aa 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h
> @@ -1,6 +1,6 @@
> -/** @file
> +/** @file LcdGraphicsOutputDxe.h
>  
> -  Copyright (c) 2011, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2011-2017, 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
>    which accompanies this distribution.  The full text of the license may be found at
> @@ -11,8 +11,8 @@
>  
>  **/
>  
> -#ifndef __ARM_VE_GRAPHICS_DXE_H__
> -#define __ARM_VE_GRAPHICS_DXE_H__
> +#ifndef LCD_GRAPHICS_OUTPUT_DXE_H_
> +#define LCD_GRAPHICS_OUTPUT_DXE_H_
>  
>  
>  #include <Base.h>
> @@ -124,5 +124,4 @@ VOID
>  LcdShutdown (
>    VOID
>  );
> -
> -#endif /* __ARM_VE_GRAPHICS_DXE_H__ */
> +#endif /* LCD_GRAPHICS_OUTPUT_DXE_H_ */
> diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> index b9bdf471e2d65dba7a0fcb0f7ecc352bd576b46b..72ebcd02ddb321ee0dad51c87ac8ee876d9ca21c 100644
> --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> @@ -1,6 +1,6 @@
> -/** @file
> +/** @file LcdPlatformLib.h
>  
> - Copyright (c) 2011, ARM Ltd. All rights reserved.<BR>
> + Copyright (c) 2011-2017, 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
>   which accompanies this distribution.  The full text of the license may be found at
> @@ -11,8 +11,8 @@
>  
>   **/
>  
> -#ifndef __LCDPLATFORMLIB_H
> -#define __LCDPLATFORMLIB_H
> +#ifndef LCD_PLATFORM_LIB_H_
> +#define LCD_PLATFORM_LIB_H_
>  
>  #include <Protocol/GraphicsOutput.h>
>  
> @@ -218,4 +218,4 @@ LcdPlatformGetBpp (
>    OUT LCD_BPP*                              Bpp
>    );
>  
> -#endif
> +#endif // LCD_PLATFORM_LIB_H_
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> index b1106ee19b98cebac01820924514eac79b97d0d5..2041de5f63c72de6f0ce4047420c282507a1d04a 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> @@ -1,6 +1,6 @@
> -/**
> +/** @file HdLcdArmVExpress.c
>  
> -  Copyright (c) 2012, ARM Ltd. All rights reserved.
> +  Copyright (c) 2012-2017, ARM Ltd. All rights reserved.
>  
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD License
> @@ -47,32 +47,38 @@ typedef struct {
>  
>  LCD_RESOLUTION mResolutions[] = {
>    { // Mode 0 : VGA : 640 x 480 x 24 bpp
> -    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, VGA_OSC_FREQUENCY,
> +    VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +    VGA_OSC_FREQUENCY,
>      VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
>      VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
>    },
>    { // Mode 1 : SVGA : 800 x 600 x 24 bpp
> -    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, SVGA_OSC_FREQUENCY,
> +    SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +    SVGA_OSC_FREQUENCY,
>      SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
>      SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
>    },
>    { // Mode 2 : XGA : 1024 x 768 x 24 bpp
> -    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, XGA_OSC_FREQUENCY,
> +    XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +    XGA_OSC_FREQUENCY,
>      XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
>      XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
>    },
>    { // Mode 3 : SXGA : 1280 x 1024 x 24 bpp
> -    SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (SXGA_OSC_FREQUENCY/2),
> +    SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +    (SXGA_OSC_FREQUENCY/2),
>      SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH,
>      SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH
>    },
>    { // Mode 4 : UXGA : 1600 x 1200 x 24 bpp
> -    UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (UXGA_OSC_FREQUENCY/2),
> +    UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +    (UXGA_OSC_FREQUENCY/2),
>      UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH,
>      UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH
>    },
>    { // Mode 5 : HD : 1920 x 1080 x 24 bpp
> -    HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (HD_OSC_FREQUENCY/2),
> +    HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +    (HD_OSC_FREQUENCY/2),
>      HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH,
>      HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH
>    }
> @@ -95,19 +101,25 @@ LcdPlatformInitializeDisplay (
>  {
>    EFI_STATUS  Status;
>  
> -  // Set the FPGA multiplexer to select the video output from the motherboard or the daughterboard
> -  Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, ARM_VE_DAUGHTERBOARD_1_SITE);
> -  if (EFI_ERROR(Status)) {
> +  /* Set the FPGA multiplexer to select the video output from the
> +   * motherboard or the daughterboard */
> +  Status = ArmPlatformSysConfigSet (
> +             SYS_CFG_MUXFPGA,
> +             ARM_VE_DAUGHTERBOARD_1_SITE
> +             );
> +  if (EFI_ERROR (Status)) {
>      return Status;
>    }
>  
>    // Install the EDID Protocols
>    Status = gBS->InstallMultipleProtocolInterfaces (
> -    &Handle,
> -    &gEfiEdidDiscoveredProtocolGuid,  &mEdidDiscovered,
> -    &gEfiEdidActiveProtocolGuid,      &mEdidActive,
> -    NULL
> -  );
> +                  &Handle,
> +                  &gEfiEdidDiscoveredProtocolGuid,
> +                  &mEdidDiscovered,
> +                  &gEfiEdidActiveProtocolGuid,
> +                  &mEdidActive,
> +                  NULL
> +                  );
>  
>    return Status;
>  }
> @@ -132,16 +144,25 @@ LcdPlatformGetVram (
>    } else {
>      AllocationType = AllocateAddress;
>    }
> -  Status = gBS->AllocatePages (AllocationType, EfiBootServicesData, EFI_SIZE_TO_PAGES(((UINTN)LCD_VRAM_SIZE)), VramBaseAddress);
> -  if (EFI_ERROR(Status)) {
> +  Status = gBS->AllocatePages (
> +                  AllocationType,
> +                  EfiBootServicesData,
> +                  EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
> +                  VramBaseAddress
> +                  );
> +  if (EFI_ERROR (Status)) {
>      return Status;
>    }
>  
> -  // Mark the VRAM as write-combining. The VRAM is inside the DRAM, which is cacheable.
> -  Status = gDS->SetMemorySpaceAttributes (*VramBaseAddress, *VramSize,
> -                  EFI_MEMORY_WC);
> -  ASSERT_EFI_ERROR(Status);
> -  if (EFI_ERROR(Status)) {
> +  /* Mark the VRAM as write-combining.
> +   * The VRAM is inside the DRAM, which is cacheable. */
> +  Status = gDS->SetMemorySpaceAttributes (
> +                  *VramBaseAddress,
> +                  *VramSize,
> +                  EFI_MEMORY_WC
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +  if (EFI_ERROR (Status)) {
>      gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
>      return Status;
>    }
> @@ -150,15 +171,11 @@ LcdPlatformGetVram (
>  }
>  
>  UINT32
> -LcdPlatformGetMaxMode (
> -  VOID
> -  )
> +LcdPlatformGetMaxMode(VOID)
>  {
> -  //
> -  // The following line will report correctly the total number of graphics modes
> -  // that could be supported by the graphics driver:
> -  //
> -  return (sizeof(mResolutions) / sizeof(LCD_RESOLUTION));
> +  /* The following line will report correctly the total number of graphics modes
> +   * that could be supported by the graphics driver: */
> +  return (sizeof (mResolutions) / sizeof (LCD_RESOLUTION));
>  }
>  
>  EFI_STATUS
> @@ -174,25 +191,35 @@ LcdPlatformSetMode (
>  
>    // Set the video mode oscillator
>    do {
> -    Status = ArmPlatformSysConfigSetDevice (SYS_CFG_OSC_SITE1, PcdGet32(PcdHdLcdVideoModeOscId), mResolutions[ModeNumber].OscFreq);
> +    Status = ArmPlatformSysConfigSetDevice (
> +               SYS_CFG_OSC_SITE1,
> +               PcdGet32 (PcdHdLcdVideoModeOscId),
> +               mResolutions[ModeNumber].OscFreq
> +               );
>    } while (Status == EFI_TIMEOUT);
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>      ASSERT_EFI_ERROR (Status);
>      return Status;
>    }
>  
>    // Set the DVI into the new mode
>    do {
> -    Status = ArmPlatformSysConfigSet (SYS_CFG_DVIMODE, mResolutions[ModeNumber].Mode);
> +    Status = ArmPlatformSysConfigSet (
> +               SYS_CFG_DVIMODE,
> +               mResolutions[ModeNumber].Mode
> +               );
>    } while (Status == EFI_TIMEOUT);
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>      ASSERT_EFI_ERROR (Status);
>      return Status;
>    }
>  
>    // Set the multiplexer
> -  Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, ARM_VE_DAUGHTERBOARD_1_SITE);
> -  if (EFI_ERROR(Status)) {
> +  Status = ArmPlatformSysConfigSet (
> +             SYS_CFG_MUXFPGA,
> +             ARM_VE_DAUGHTERBOARD_1_SITE
> +             );
> +  if (EFI_ERROR (Status)) {
>      ASSERT_EFI_ERROR (Status);
>      return Status;
>    }
> @@ -233,7 +260,7 @@ LcdPlatformQueryMode (
>      case LCD_BITS_PER_PIXEL_1:
>      default:
>        // These are not supported
> -      ASSERT(FALSE);
> +      ASSERT (FALSE);
>        break;
>    }
>  
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> index 3f3ceb3d2fa82f614e0c6dac8455c117745cf3a6..8d046816454f642bced00e29c4e02093b74afd24 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> @@ -1,6 +1,6 @@
> -/** @file
> +/** @file PL111LcdArmVExpress.c
>  
> -  Copyright (c) 2011-2015, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2011-2017, 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
>    which accompanies this distribution.  The full text of the license may be found at
> @@ -43,83 +43,99 @@ typedef struct {
>  
>  
>  LCD_RESOLUTION mResolutions[] = {
> -  { // Mode 0 : VGA : 640 x 480 x 24 bpp
> -      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, VGA_OSC_FREQUENCY,
> +  {   // Mode 0 : VGA : 640 x 480 x 24 bpp

Should the changes in this hunk not fix the alignment of the values
below (from 4 to 2) rather than increase indentati on of the comment
(from 2 to 4)?

> +      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +      VGA_OSC_FREQUENCY,
>        VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
>        VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
>    },
> -  { // Mode 1 : SVGA : 800 x 600 x 24 bpp
> -      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, SVGA_OSC_FREQUENCY,
> +  {   // Mode 1 : SVGA : 800 x 600 x 24 bpp
> +      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +      SVGA_OSC_FREQUENCY,
>        SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
>        SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
>    },
> -  { // Mode 2 : XGA : 1024 x 768 x 24 bpp
> -      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, XGA_OSC_FREQUENCY,
> +  {   // Mode 2 : XGA : 1024 x 768 x 24 bpp
> +      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +      XGA_OSC_FREQUENCY,
>        XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
>        XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
>    },
> -  { // Mode 3 : SXGA : 1280 x 1024 x 24 bpp
> -      SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (SXGA_OSC_FREQUENCY/2),
> +  {   // Mode 3 : SXGA : 1280 x 1024 x 24 bpp
> +      SXGA, SXGA_H_RES_PIXELS, SXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +      (SXGA_OSC_FREQUENCY/2),
>        SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH,
>        SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH
>    },
> -  { // Mode 4 : UXGA : 1600 x 1200 x 24 bpp
> -      UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (UXGA_OSC_FREQUENCY/2),
> +  {   // Mode 4 : UXGA : 1600 x 1200 x 24 bpp
> +      UXGA, UXGA_H_RES_PIXELS, UXGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +      (UXGA_OSC_FREQUENCY/2),
>        UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH,
>        UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH
>    },
> -  { // Mode 5 : HD : 1920 x 1080 x 24 bpp
> -      HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24, (HD_OSC_FREQUENCY/2),
> +  {   // Mode 5 : HD : 1920 x 1080 x 24 bpp
> +      HD, HD_H_RES_PIXELS, HD_V_RES_PIXELS, LCD_BITS_PER_PIXEL_24,
> +      (HD_OSC_FREQUENCY/2),
>        HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH,
>        HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH
>    },
> -  { // Mode 6 : VGA : 640 x 480 x 16 bpp (565 Mode)
> -      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565, VGA_OSC_FREQUENCY,
> +  {   // Mode 6 : VGA : 640 x 480 x 16 bpp (565 Mode)
> +      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565,
> +      VGA_OSC_FREQUENCY,
>        VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
>        VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
>    },
> -  { // Mode 7 : SVGA : 800 x 600 x 16 bpp (565 Mode)
> -      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565, SVGA_OSC_FREQUENCY,
> +  {   // Mode 7 : SVGA : 800 x 600 x 16 bpp (565 Mode)
> +      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565,
> +      SVGA_OSC_FREQUENCY,
>        SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
>        SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
>    },
> -  { // Mode 8 : XGA : 1024 x 768 x 16 bpp (565 Mode)
> -      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565, XGA_OSC_FREQUENCY,
> +  {   // Mode 8 : XGA : 1024 x 768 x 16 bpp (565 Mode)
> +      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_565,
> +      XGA_OSC_FREQUENCY,
>        XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
>        XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
>    },
> -  { // Mode 9 : VGA : 640 x 480 x 15 bpp
> -      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555, VGA_OSC_FREQUENCY,
> +  {   // Mode 9 : VGA : 640 x 480 x 15 bpp
> +      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
> +      VGA_OSC_FREQUENCY,
>        VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
>        VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
>    },
> -  { // Mode 10 : SVGA : 800 x 600 x 15 bpp
> -      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555, SVGA_OSC_FREQUENCY,
> +  {   // Mode 10 : SVGA : 800 x 600 x 15 bpp
> +      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
> +      SVGA_OSC_FREQUENCY,
>        SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
>        SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
>    },
> -  { // Mode 11 : XGA : 1024 x 768 x 15 bpp
> -      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555, XGA_OSC_FREQUENCY,
> +  {   // Mode 11 : XGA : 1024 x 768 x 15 bpp
> +      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
> +      XGA_OSC_FREQUENCY,
>        XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
>        XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
>    },
> -  { // Mode 12 : XGA : 1024 x 768 x 15 bpp - All the timing info is derived from Linux Kernel Driver Settings
> -      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555, 63500000,
> +  {   // Mode 12 : XGA : 1024 x 768 x 15 bpp - All the timing info is derived from Linux Kernel Driver Settings
> +      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_16_555,
> +      63500000,
>        XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
>        XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
>    },
> -  { // Mode 13 : VGA : 640 x 480 x 12 bpp (444 Mode)
> -      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444, VGA_OSC_FREQUENCY,
> +  {   // Mode 13 : VGA : 640 x 480 x 12 bpp (444 Mode)
> +      VGA, VGA_H_RES_PIXELS, VGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444,
> +      VGA_OSC_FREQUENCY,
>        VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH,
>        VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH
>    },
> -  { // Mode 14 : SVGA : 800 x 600 x 12 bpp (444 Mode)
> -      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444, SVGA_OSC_FREQUENCY,
> +  {   // Mode 14 : SVGA : 800 x 600 x 12 bpp (444 Mode)
> +      SVGA, SVGA_H_RES_PIXELS, SVGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444,
> +      SVGA_OSC_FREQUENCY,
>        SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH,
>        SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH
>    },
> -  { // Mode 15 : XGA : 1024 x 768 x 12 bpp (444 Mode)
> -      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444, XGA_OSC_FREQUENCY,
> +  {   // Mode 15 : XGA : 1024 x 768 x 12 bpp (444 Mode)
> +      XGA, XGA_H_RES_PIXELS, XGA_V_RES_PIXELS, LCD_BITS_PER_PIXEL_12_444,
> +      XGA_OSC_FREQUENCY,
>        XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH,
>        XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH
>    }
> @@ -145,14 +161,16 @@ LcdPlatformInitializeDisplay (
>  
>    // Set the FPGA multiplexer to select the video output from the motherboard or the daughterboard
>    Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, PL111_CLCD_SITE);
> -  if (!EFI_ERROR(Status)) {
> +  if (!EFI_ERROR (Status)) {
>      // Install the EDID Protocols
> -    Status = gBS->InstallMultipleProtocolInterfaces(
> -      &Handle,
> -      &gEfiEdidDiscoveredProtocolGuid,  &mEdidDiscovered,
> -      &gEfiEdidActiveProtocolGuid,      &mEdidActive,
> -      NULL
> -    );
> +    Status = gBS->InstallMultipleProtocolInterfaces (
> +                    &Handle,
> +                    &gEfiEdidDiscoveredProtocolGuid,
> +                    &mEdidDiscovered,
> +                    &gEfiEdidActiveProtocolGuid,
> +                    &mEdidActive,
> +                    NULL
> +                    );
>    }
>  
>    return Status;
> @@ -169,29 +187,38 @@ LcdPlatformGetVram (
>    Status = EFI_SUCCESS;
>  
>    // Is it on the motherboard or on the daughterboard?
> -  switch(PL111_CLCD_SITE) {
> +  switch (PL111_CLCD_SITE) {
>  
>    case ARM_VE_MOTHERBOARD_SITE:
> -    *VramBaseAddress = (EFI_PHYSICAL_ADDRESS) PL111_CLCD_VRAM_MOTHERBOARD_BASE;
> +    *VramBaseAddress = (EFI_PHYSICAL_ADDRESS)PL111_CLCD_VRAM_MOTHERBOARD_BASE;
>      *VramSize = LCD_VRAM_SIZE;
>      break;
>  
>    case ARM_VE_DAUGHTERBOARD_1_SITE:
> -    *VramBaseAddress = (EFI_PHYSICAL_ADDRESS) LCD_VRAM_CORE_TILE_BASE;
> +    *VramBaseAddress = (EFI_PHYSICAL_ADDRESS)LCD_VRAM_CORE_TILE_BASE;
>      *VramSize = LCD_VRAM_SIZE;
>  
>      // Allocate the VRAM from the DRAM so that nobody else uses it.
> -    Status = gBS->AllocatePages( AllocateAddress, EfiBootServicesData, EFI_SIZE_TO_PAGES(((UINTN)LCD_VRAM_SIZE)), VramBaseAddress);
> -    if (EFI_ERROR(Status)) {
> +    Status = gBS->AllocatePages (
> +                    AllocateAddress,
> +                    EfiBootServicesData,
> +                    EFI_SIZE_TO_PAGES (((UINTN)LCD_VRAM_SIZE)),
> +                    VramBaseAddress
> +                    );
> +    if (EFI_ERROR (Status)) {
>        return Status;
>      }
>  
> -    // Mark the VRAM as write-combining. The VRAM is inside the DRAM, which is cacheable.
> -    Status = gDS->SetMemorySpaceAttributes (*VramBaseAddress, *VramSize,
> -                    EFI_MEMORY_WC);
> -    ASSERT_EFI_ERROR(Status);
> -    if (EFI_ERROR(Status)) {
> -      gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES(*VramSize));
> +    // Mark the VRAM as write-combining.
> +    // The VRAM is inside the DRAM, which is cacheable.
> +    Status = gDS->SetMemorySpaceAttributes (
> +                    *VramBaseAddress,
> +                    *VramSize,
> +                    EFI_MEMORY_WC
> +                    );
> +    ASSERT_EFI_ERROR (Status);
> +    if (EFI_ERROR (Status)) {
> +      gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
>        return Status;
>      }
>      break;
> @@ -206,19 +233,20 @@ LcdPlatformGetVram (
>  }
>  
>  UINT32
> -LcdPlatformGetMaxMode (
> -  VOID
> -  )
> +LcdPlatformGetMaxMode(VOID)
>  {
> -  // The following line will report correctly the total number of graphics modes
> -  // supported by the PL111CLCD.
> -  //return (sizeof(mResolutions) / sizeof(CLCD_RESOLUTION)) - 1;
> +  /* The following line would correctly reports the total number
> +   * of graphics modes supported by the PL111CLCD.
> +   * return (sizeof(mResolutions) / sizeof(CLCD_RESOLUTION)) - 1;
> +   */
>  
> -  // However, on some platforms it is desirable to ignore some graphics modes.
> -  // This could be because the specific implementation of PL111 has certain limitations.
> +  /* However, on some platforms it is desirable to ignore some graphics modes.
> +   * This could be because the specific implementation of PL111 has
> +   * certain limitations.
> +   */
>  
>    // Set the maximum mode allowed
> -  return (PcdGet32(PcdPL111LcdMaxMode));
> +  return (PcdGet32 (PcdPL111LcdMaxMode));
>  }
>  
>  EFI_STATUS
> @@ -238,22 +266,26 @@ LcdPlatformSetMode (
>  
>    LcdSite = PL111_CLCD_SITE;
>  
> -  switch(LcdSite) {
> +  switch (LcdSite) {
>    case ARM_VE_MOTHERBOARD_SITE:
>      Function = SYS_CFG_OSC;
>      OscillatorId = PL111_CLCD_MOTHERBOARD_VIDEO_MODE_OSC_ID;
>      break;
>    case ARM_VE_DAUGHTERBOARD_1_SITE:
>      Function = SYS_CFG_OSC_SITE1;
> -    OscillatorId = (UINT32)PcdGet32(PcdPL111LcdVideoModeOscId);
> +    OscillatorId = (UINT32)PcdGet32 (PcdPL111LcdVideoModeOscId);
>      break;
>    default:
>      return EFI_UNSUPPORTED;
>    }
>  
>    // Set the video mode oscillator
> -  Status = ArmPlatformSysConfigSetDevice (Function, OscillatorId, mResolutions[ModeNumber].OscFreq);
> -  if (EFI_ERROR(Status)) {
> +  Status = ArmPlatformSysConfigSetDevice (
> +             Function,
> +             OscillatorId,
> +             mResolutions[ModeNumber].OscFreq
> +             );
> +  if (EFI_ERROR (Status)) {
>      ASSERT_EFI_ERROR (Status);
>      return Status;
>    }
> @@ -267,8 +299,11 @@ LcdPlatformSetMode (
>      SysId &= ~ARM_FVP_SYS_ID_VARIANT_MASK;
>      if (SysId != ARM_FVP_BASE_BOARD_SYS_ID) {
>        // Set the DVI into the new mode
> -      Status = ArmPlatformSysConfigSet (SYS_CFG_DVIMODE, mResolutions[ModeNumber].Mode);
> -      if (EFI_ERROR(Status)) {
> +      Status = ArmPlatformSysConfigSet (
> +                 SYS_CFG_DVIMODE,
> +                 mResolutions[ModeNumber].Mode
> +                 );
> +      if (EFI_ERROR (Status)) {
>          ASSERT_EFI_ERROR (Status);
>          return Status;
>        }
> @@ -277,7 +312,7 @@ LcdPlatformSetMode (
>  
>    // Set the multiplexer
>    Status = ArmPlatformSysConfigSet (SYS_CFG_MUXFPGA, LcdSite);
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>      ASSERT_EFI_ERROR (Status);
>      return Status;
>    }
> @@ -318,7 +353,7 @@ LcdPlatformQueryMode (
>      case LCD_BITS_PER_PIXEL_1:
>      default:
>        // These are not supported
> -      ASSERT(FALSE);
> +      ASSERT (FALSE);
>        break;
>    }
>  
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> index 2bfe2c0fe2dcd05f4983eea57542cfe3d30bf1ce..eb0b6fb3fbbc1cb605469433f6c6dcb85bac668c 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> @@ -1,6 +1,6 @@
> -/** @file  Lcd.c
> +/** @file  HdLcd.c
>  
> -  Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2011-2017, 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
> @@ -35,21 +35,34 @@ LcdInitialize (
>    )
>  {
>    // Disable the controller
> -  MmioWrite32(HDLCD_REG_COMMAND, HDLCD_DISABLE);
> +  MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_DISABLE);
>  
>    // Disable all interrupts
> -  MmioWrite32(HDLCD_REG_INT_MASK, 0);
> +  MmioWrite32 (HDLCD_REG_INT_MASK, 0);
>  
>    // Define start of the VRAM. This never changes for any graphics mode
> -  MmioWrite32(HDLCD_REG_FB_BASE, (UINT32) VramBaseAddress);
> +  MmioWrite32 (HDLCD_REG_FB_BASE, (UINT32)VramBaseAddress);
>  
>    // Setup various registers that never change
> -  MmioWrite32(HDLCD_REG_BUS_OPTIONS,  (4 << 8) | HDLCD_BURST_8);
> -  MmioWrite32(HDLCD_REG_POLARITIES,   HDLCD_PXCLK_LOW | HDLCD_DATA_HIGH | HDLCD_DATEN_HIGH | HDLCD_HSYNC_LOW | HDLCD_VSYNC_HIGH);
> -  MmioWrite32(HDLCD_REG_PIXEL_FORMAT, 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));
> +  MmioWrite32 (HDLCD_REG_BUS_OPTIONS,  (4 << 8) | HDLCD_BURST_8);
> +
> +  MmioWrite32 (
> +    HDLCD_REG_POLARITIES,
> +    HDLCD_PXCLK_LOW
> +    | HDLCD_DATA_HIGH
> +    | HDLCD_DATEN_HIGH
> +    | HDLCD_HSYNC_LOW
> +    | HDLCD_VSYNC_HIGH
> +    );

Or maybe a #define HDLCD_DEFAULT_POLARITIES?

> +
> +  MmioWrite32 (
> +    HDLCD_REG_PIXEL_FORMAT,
> +    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;
>  }
> @@ -73,44 +86,52 @@ LcdSetMode (
>  
>  
>    // Set the video mode timings and other relevant information
> -  Status = LcdPlatformGetTimings (ModeNumber,
> -                                  &HRes,&HSync,&HBackPorch,&HFrontPorch,
> -                                  &VRes,&VSync,&VBackPorch,&VFrontPorch);
> +  Status = LcdPlatformGetTimings (
> +             ModeNumber,
> +             &HRes,
> +             &HSync,
> +             &HBackPorch,
> +             &HFrontPorch,
> +             &VRes,
> +             &VSync,
> +             &VBackPorch,
> +             &VFrontPorch
> +             );
>    ASSERT_EFI_ERROR (Status);
> -  if (EFI_ERROR( Status )) {
> +  if (EFI_ERROR (Status)) {
>      return EFI_DEVICE_ERROR;
>    }
>  
> -  Status = LcdPlatformGetBpp (ModeNumber,&LcdBpp);
> +  Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
>    ASSERT_EFI_ERROR (Status);
> -  if (EFI_ERROR( Status )) {
> +  if (EFI_ERROR (Status)) {
>      return EFI_DEVICE_ERROR;
>    }
>  
> -  BytesPerPixel = GetBytesPerPixel(LcdBpp);
> +  BytesPerPixel = GetBytesPerPixel (LcdBpp);
>  
>    // Disable the controller
> -  MmioWrite32(HDLCD_REG_COMMAND, HDLCD_DISABLE);
> +  MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_DISABLE);
>  
>    // Update the frame buffer information with the new settings
> -  MmioWrite32(HDLCD_REG_FB_LINE_LENGTH, HRes * BytesPerPixel);
> -  MmioWrite32(HDLCD_REG_FB_LINE_PITCH,  HRes * BytesPerPixel);
> -  MmioWrite32(HDLCD_REG_FB_LINE_COUNT,  VRes - 1);
> +  MmioWrite32 (HDLCD_REG_FB_LINE_LENGTH, HRes * BytesPerPixel);
> +  MmioWrite32 (HDLCD_REG_FB_LINE_PITCH,  HRes * BytesPerPixel);
> +  MmioWrite32 (HDLCD_REG_FB_LINE_COUNT,  VRes - 1);
>  
>    // Set the vertical timing information
> -  MmioWrite32(HDLCD_REG_V_SYNC,         VSync);
> -  MmioWrite32(HDLCD_REG_V_BACK_PORCH,   VBackPorch);
> -  MmioWrite32(HDLCD_REG_V_DATA,         VRes - 1);
> -  MmioWrite32(HDLCD_REG_V_FRONT_PORCH,  VFrontPorch);
> +  MmioWrite32 (HDLCD_REG_V_SYNC,         VSync);
> +  MmioWrite32 (HDLCD_REG_V_BACK_PORCH,   VBackPorch);
> +  MmioWrite32 (HDLCD_REG_V_DATA,         VRes - 1);
> +  MmioWrite32 (HDLCD_REG_V_FRONT_PORCH,  VFrontPorch);
>  
>    // Set the horizontal timing information
> -  MmioWrite32(HDLCD_REG_H_SYNC,         HSync);
> -  MmioWrite32(HDLCD_REG_H_BACK_PORCH,   HBackPorch);
> -  MmioWrite32(HDLCD_REG_H_DATA,         HRes - 1);
> -  MmioWrite32(HDLCD_REG_H_FRONT_PORCH,  HFrontPorch);
> +  MmioWrite32 (HDLCD_REG_H_SYNC,         HSync);
> +  MmioWrite32 (HDLCD_REG_H_BACK_PORCH,   HBackPorch);
> +  MmioWrite32 (HDLCD_REG_H_DATA,         HRes - 1);
> +  MmioWrite32 (HDLCD_REG_H_FRONT_PORCH,  HFrontPorch);
>  
>    // Enable the controller
> -  MmioWrite32(HDLCD_REG_COMMAND, HDLCD_ENABLE);
> +  MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_ENABLE);
>  
>    return EFI_SUCCESS;
>  }
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c
> index b721061fc1df5695092e8c71da97ae0b9af46b3f..2dd8f39873f77b1c211bff407cabe90c1795b121 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c
> @@ -1,6 +1,6 @@
> -/** @file
> +/** @file LcdGraphicsOutputDxe.c
>  
> - Copyright (c) 2011-2014, ARM Ltd. All rights reserved.<BR>
> + Copyright (c) 2011-2017, 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
>   which accompanies this distribution.  The full text of the license may be found at
> @@ -9,7 +9,7 @@
>   THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>   WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  
> - **/
> +**/
>  
>  #include <PiDxe.h>
>  #include <Library/BaseMemoryLib.h>
> @@ -64,7 +64,9 @@ LCD_INSTANCE mLcdTemplate = {
>      {
>        {
>          HARDWARE_DEVICE_PATH, HW_VENDOR_DP,
> -        { (UINT8) (sizeof(VENDOR_DEVICE_PATH)), (UINT8) ((sizeof(VENDOR_DEVICE_PATH)) >> 8) },
> +        { (UINT8)(sizeof (VENDOR_DEVICE_PATH)),

I thought the '{' should always be on its own line?

> +          (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8)
> +        },
>        },
>        // Hardware Device Path for Lcd
>        EFI_CALLER_ID_GUID // Use the driver's GUID
> @@ -73,7 +75,7 @@ LCD_INSTANCE mLcdTemplate = {
>      {
>        END_DEVICE_PATH_TYPE,
>        END_ENTIRE_DEVICE_PATH_SUBTYPE,
> -      { sizeof(EFI_DEVICE_PATH_PROTOCOL), 0 }
> +      { sizeof (EFI_DEVICE_PATH_PROTOCOL), 0 }

And here
  {
    sizeof ...
  }
?
>      }
>    },
>    (EFI_EVENT) NULL // ExitBootServicesEvent
> @@ -86,7 +88,7 @@ LcdInstanceContructor (
>  {
>    LCD_INSTANCE* Instance;
>  
> -  Instance = AllocateCopyPool (sizeof(LCD_INSTANCE), &mLcdTemplate);
> +  Instance = AllocateCopyPool (sizeof (LCD_INSTANCE), &mLcdTemplate);
>    if (Instance == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> @@ -113,23 +115,23 @@ InitializeDisplay (
>    UINTN                  VramSize;
>  
>    Status = LcdPlatformGetVram (&VramBaseAddress, &VramSize);
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>      return Status;
>    }
>  
>    // Setup the LCD
>    Status = LcdInitialize (VramBaseAddress);
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>      goto EXIT_ERROR_LCD_SHUTDOWN;
>    }
>  
>    Status = LcdPlatformInitializeDisplay (Instance->Handle);
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>      goto EXIT_ERROR_LCD_SHUTDOWN;
>    }
>  
>    // Setup all the relevant mode information
> -  Instance->Gop.Mode->SizeOfInfo      = sizeof(EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
> +  Instance->Gop.Mode->SizeOfInfo      = sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
>    Instance->Gop.Mode->FrameBufferBase = VramBaseAddress;
>  
>    // Set the flag before changing the mode, to avoid infinite loops
> @@ -139,7 +141,8 @@ InitializeDisplay (
>    goto EXIT;
>  
>  EXIT_ERROR_LCD_SHUTDOWN:
> -  DEBUG((DEBUG_ERROR, "InitializeDisplay: ERROR - Can not initialise the display. Exit Status=%r\n", Status));
> +  DEBUG ((DEBUG_ERROR, "InitializeDisplay: ERROR - Can not initialise the display. Exit Status=%r\n", Status));
> +
>    LcdShutdown ();
>  
>  EXIT:
> @@ -157,40 +160,44 @@ LcdGraphicsOutputDxeInitialize (
>    LCD_INSTANCE* Instance;
>  
>    Status = LcdIdentify ();
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>      goto EXIT;
>    }
>  
>    Status = LcdInstanceContructor (&Instance);
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>      goto EXIT;
>    }
>  
>    // Install the Graphics Output Protocol and the Device Path
> -  Status = gBS->InstallMultipleProtocolInterfaces(
> -            &Instance->Handle,
> -            &gEfiGraphicsOutputProtocolGuid, &Instance->Gop,
> -            &gEfiDevicePathProtocolGuid,     &Instance->DevicePath,
> -            NULL
> -            );
> +  Status = gBS->InstallMultipleProtocolInterfaces (
> +                  &Instance->Handle,
> +                  &gEfiGraphicsOutputProtocolGuid,
> +                  &Instance->Gop,
> +                  &gEfiDevicePathProtocolGuid,
> +                  &Instance->DevicePath,
> +                  NULL
> +                  );
>  
> -  if (EFI_ERROR(Status)) {
> -    DEBUG((DEBUG_ERROR, "GraphicsOutputDxeInitialize: Can not install the protocol. Exit Status=%r\n", Status));
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "LcdGraphicsOutputDxeInitialize: Can not install the protocol. Exit Status=%r\n", Status));
>      goto EXIT;
>    }
>  
> -  // Register for an ExitBootServicesEvent
> -  // When ExitBootServices starts, this function here will make sure that the graphics driver will shut down properly,
> -  // i.e. it will free up all allocated memory and perform any necessary hardware re-configuration.
> +  /* Register for an ExitBootServicesEvent
> +   * When ExitBootServices starts, this function will make sure that the
> +   * graphics driver shuts down properly, i.e. it will free up all
> +   * allocated memory and perform any necessary hardware re-configuration. */
>    Status = gBS->CreateEvent (
>              EVT_SIGNAL_EXIT_BOOT_SERVICES,
>              TPL_NOTIFY,
> -            LcdGraphicsExitBootServicesEvent, NULL,
> +            LcdGraphicsExitBootServicesEvent,
> +            NULL,
>              &Instance->ExitBootServicesEvent
>              );
>  
> -  if (EFI_ERROR(Status)) {
> -    DEBUG((DEBUG_ERROR, "GraphicsOutputDxeInitialize: Can not install the ExitBootServicesEvent handler. Exit Status=%r\n", Status));
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "LcdGraphicsOutputDxeInitialize: Can not install the ExitBootServicesEvent handler. Exit Status=%r\n", Status));
>      goto EXIT_ERROR_UNINSTALL_PROTOCOL;
>    }
>  
> @@ -204,11 +211,13 @@ EXIT_ERROR_UNINSTALL_PROTOCOL:
>     * the Status variable, even it fails to uninstall the protocol.
>     */
>    gBS->UninstallMultipleProtocolInterfaces (
> -    Instance->Handle,
> -    &gEfiGraphicsOutputProtocolGuid, &Instance->Gop, // Uninstall Graphics Output protocol
> -    &gEfiDevicePathProtocolGuid,     &Instance->DevicePath,     // Uninstall device path
> -    NULL
> -    );
> +         Instance->Handle,
> +         &gEfiGraphicsOutputProtocolGuid,
> +         &Instance->Gop, // Uninstall Graphics Output protocol
> +         &gEfiDevicePathProtocolGuid,
> +         &Instance->DevicePath,     // Uninstall device path
> +         NULL
> +         );
>  
>  EXIT:
>    return Status;
> @@ -227,9 +236,9 @@ LcdGraphicsExitBootServicesEvent (
>    IN VOID       *Context
>    )
>  {
> -  // By default, this PCD is FALSE. But if a platform starts a predefined OS that
> -  // does not use a framebuffer then we might want to disable the display controller
> -  // to avoid to display corrupted information on the screen.
> +  /* By default, this PCD is FALSE. But if a platform starts a predefined OS
> +   * that does not use a framebuffer then we might want to disable the display
> +   * controller to avoid to display corrupted information on the screen. */
>    if (FeaturePcdGet (PcdGopDisableOnExitBootServices)) {
>      // Turn-off the Display controller
>      LcdShutdown ();
> @@ -252,19 +261,22 @@ LcdGraphicsQueryMode (
>    EFI_STATUS Status = EFI_SUCCESS;
>    LCD_INSTANCE *Instance;
>  
> -  Instance = LCD_INSTANCE_FROM_GOP_THIS(This);
> +  Instance = LCD_INSTANCE_FROM_GOP_THIS (This);
>  
>    // Setup the hardware if not already done
> -  if( !mDisplayInitialized ) {
> -    Status = InitializeDisplay(Instance);
> -    if (EFI_ERROR(Status)) {
> +  if (!mDisplayInitialized) {
> +    Status = InitializeDisplay (Instance);
> +    if (EFI_ERROR (Status)) {
>        goto EXIT;
>      }
>    }
>  
>    // Error checking
> -  if ( (This == NULL) || (Info == NULL) || (SizeOfInfo == NULL) || (ModeNumber >= This->Mode->MaxMode) ) {
> -    DEBUG((DEBUG_ERROR, "LcdGraphicsQueryMode: ERROR - For mode number %d : Invalid Parameter.\n", ModeNumber ));
> +  if ( (This == NULL)
> +    || (Info == NULL)
> +    || (SizeOfInfo == NULL)
> +    || (ModeNumber >= This->Mode->MaxMode)) {
> +    DEBUG ((DEBUG_ERROR, "LcdGraphicsQueryMode: ERROR - For mode number %d : Invalid Parameter.\n", ModeNumber));
>      Status = EFI_INVALID_PARAMETER;
>      goto EXIT;
>    }
> @@ -275,11 +287,11 @@ LcdGraphicsQueryMode (
>      goto EXIT;
>    }
>  
> -  *SizeOfInfo = sizeof( EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
> +  *SizeOfInfo = sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
>  
> -  Status = LcdPlatformQueryMode (ModeNumber,*Info);
> -  if (EFI_ERROR(Status)) {
> -    FreePool(*Info);
> +  Status = LcdPlatformQueryMode (ModeNumber, *Info);
> +  if (EFI_ERROR (Status)) {
> +    FreePool (*Info);
>    }
>  
>  EXIT:
> @@ -305,47 +317,48 @@ LcdGraphicsSetMode (
>    Instance = LCD_INSTANCE_FROM_GOP_THIS (This);
>  
>    // Setup the hardware if not already done
> -  if(!mDisplayInitialized) {
> +  if (!mDisplayInitialized) {
>      Status = InitializeDisplay (Instance);
> -    if (EFI_ERROR(Status)) {
> +    if (EFI_ERROR (Status)) {
>        goto EXIT;
>      }
>    }
>  
>    // Check if this mode is supported
> -  if( ModeNumber >= This->Mode->MaxMode ) {
> -    DEBUG((DEBUG_ERROR, "LcdGraphicsSetMode: ERROR - Unsupported mode number %d .\n", ModeNumber ));
> +  if (ModeNumber >= This->Mode->MaxMode) {
> +    DEBUG ((DEBUG_ERROR, "LcdGraphicsSetMode: ERROR - Unsupported mode number %d .\n", ModeNumber));
>      Status = EFI_UNSUPPORTED;
>      goto EXIT;
>    }
>  
>    // Set the oscillator frequency to support the new mode
>    Status = LcdPlatformSetMode (ModeNumber);
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>      Status = EFI_DEVICE_ERROR;
>      goto EXIT;
>    }
>  
>    // Update the UEFI mode information
>    This->Mode->Mode = ModeNumber;
> -  LcdPlatformQueryMode (ModeNumber,&Instance->ModeInfo);
> -  Status = LcdPlatformGetBpp(ModeNumber, &Bpp);
> -  if (EFI_ERROR(Status)) {
> +  LcdPlatformQueryMode (ModeNumber, &Instance->ModeInfo);
> +  Status = LcdPlatformGetBpp (ModeNumber, &Bpp);
> +  if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "LcdGraphicsSetMode: ERROR - Couldn't get bytes per pixel, status: %r\n", Status));
>      goto EXIT;
>    }
>    This->Mode->FrameBufferSize =  Instance->ModeInfo.VerticalResolution
>                                 * Instance->ModeInfo.PixelsPerScanLine
> -                               * GetBytesPerPixel(Bpp);
> +                               * GetBytesPerPixel (Bpp);
>  
>    // Set the hardware to the new mode
>    Status = LcdSetMode (ModeNumber);
> -  if (EFI_ERROR(Status)) {
> +  if (EFI_ERROR (Status)) {
>      Status = EFI_DEVICE_ERROR;
>      goto EXIT;
>    }
>  
> -  // The UEFI spec requires that we now clear the visible portions of the output display to black.
> +  // The UEFI spec requires that we now clear the visible portions of the
> +  // output display to black.
>  
>    // Set the fill colour to black
>    SetMem (&FillColour, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0x0);
> @@ -361,7 +374,8 @@ LcdGraphicsSetMode (
>        0,
>        This->Mode->Info->HorizontalResolution,
>        This->Mode->Info->VerticalResolution,
> -      0);
> +      0
> +      );
>  
>  EXIT:
>    return Status;
> @@ -372,7 +386,7 @@ GetBytesPerPixel (
>    IN  LCD_BPP       Bpp
>    )
>  {
> -  switch(Bpp) {
> +  switch (Bpp) {
>    case LCD_BITS_PER_PIXEL_24:
>      return 4;
>  
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> index b5e113b844d4e4df2e35ccd880c01344b4b8b9d7..0b0c4204fbc44bc9e90dce3d7b410ce167d9f40c 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> @@ -1,6 +1,6 @@
>  /** @file  PL111Lcd.c
>  
> -  Copyright (c) 2011-2012, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2011-2017, 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
> @@ -54,11 +54,11 @@ LcdInitialize (
>    )
>  {
>    // Define start of the VRAM. This never changes for any graphics mode
> -  MmioWrite32(PL111_REG_LCD_UP_BASE, (UINT32) VramBaseAddress);
> -  MmioWrite32(PL111_REG_LCD_LP_BASE, 0); // We are not using a double buffer
> +  MmioWrite32 (PL111_REG_LCD_UP_BASE, (UINT32)VramBaseAddress);
> +  MmioWrite32 (PL111_REG_LCD_LP_BASE, 0); // We are not using a double buffer
>  
>    // Disable all interrupts from the PL111
> -  MmioWrite32(PL111_REG_LCD_IMSC, 0);
> +  MmioWrite32 (PL111_REG_LCD_IMSC, 0);
>  
>    return EFI_SUCCESS;
>  }
> @@ -81,37 +81,55 @@ LcdSetMode (
>    LCD_BPP           LcdBpp;
>  
>    // Set the video mode timings and other relevant information
> -  Status = LcdPlatformGetTimings (ModeNumber,
> -                                  &HRes,&HSync,&HBackPorch,&HFrontPorch,
> -                                  &VRes,&VSync,&VBackPorch,&VFrontPorch);
> +  Status = LcdPlatformGetTimings (
> +             ModeNumber,
> +             &HRes,
> +             &HSync,
> +             &HBackPorch,
> +             &HFrontPorch,
> +             &VRes,
> +             &VSync,
> +             &VBackPorch,
> +             &VFrontPorch
> +             );
>    ASSERT_EFI_ERROR (Status);
> -  if (EFI_ERROR( Status )) {
> +  if (EFI_ERROR (Status)) {
>      return EFI_DEVICE_ERROR;
>    }
>  
> -  Status = LcdPlatformGetBpp (ModeNumber,&LcdBpp);
> +  Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
>    ASSERT_EFI_ERROR (Status);
> -  if (EFI_ERROR( Status )) {
> +  if (EFI_ERROR (Status)) {
>      return EFI_DEVICE_ERROR;
>    }
>  
>    // Disable the CLCD_LcdEn bit
> -  LcdControl = MmioRead32( PL111_REG_LCD_CONTROL);
> -  MmioWrite32(PL111_REG_LCD_CONTROL,  LcdControl & ~1);
> +  LcdControl = MmioRead32 (PL111_REG_LCD_CONTROL);
> +  MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl & ~1);
>  
>    // Set Timings
> -  MmioWrite32 (PL111_REG_LCD_TIMING_0, HOR_AXIS_PANEL(HBackPorch, HFrontPorch, HSync, HRes));
> -  MmioWrite32 (PL111_REG_LCD_TIMING_1, VER_AXIS_PANEL(VBackPorch, VFrontPorch, VSync, VRes));
> -  MmioWrite32 (PL111_REG_LCD_TIMING_2, CLK_SIG_POLARITY(HRes));
> +  MmioWrite32 (
> +    PL111_REG_LCD_TIMING_0,
> +    HOR_AXIS_PANEL (HBackPorch, HFrontPorch, HSync, HRes)
> +    );
> +
> +  MmioWrite32 (
> +    PL111_REG_LCD_TIMING_1,
> +    VER_AXIS_PANEL (VBackPorch, VFrontPorch, VSync, VRes)
> +    );
> +
> +  MmioWrite32 (PL111_REG_LCD_TIMING_2, CLK_SIG_POLARITY (HRes));
>    MmioWrite32 (PL111_REG_LCD_TIMING_3, 0);
>  
>    // PL111_REG_LCD_CONTROL
> -  LcdControl = PL111_CTRL_LCD_EN | PL111_CTRL_LCD_BPP(LcdBpp) | PL111_CTRL_LCD_TFT | PL111_CTRL_BGR;
> -  MmioWrite32(PL111_REG_LCD_CONTROL,  LcdControl);
> +  LcdControl = PL111_CTRL_LCD_EN | PL111_CTRL_LCD_BPP (LcdBpp)
> +                 | PL111_CTRL_LCD_TFT | PL111_CTRL_BGR;

No additional indentation of continuation line.

> +

And this extra blank line makes this operation (PL111_REG_LCD_CONTROL)
look different than the subsequent (Turn on power to the LCD Panel).

/
    Leif

> +  MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl);
>  
>    // Turn on power to the LCD Panel
>    LcdControl |= PL111_CTRL_LCD_PWR;
> -  MmioWrite32(PL111_REG_LCD_CONTROL,  LcdControl);
> +  MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl);
>  
>    return EFI_SUCCESS;
>  }
> -- 
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>