Re: [PATCH v3 03/16] ArmPlatformPkg: Tidy Lcd code: Coding standard

[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 03/16] ArmPlatformPkg: Tidy Lcd code: Coding
> standard
> 
> From: Girish Pathak <girish.pathak at 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.
> 
> 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:
>     - Minor coding style changes                              [Ard]
> 
>       Done                                                    [Girish]
> 
>     - Changing one style to the other is just pointless churn [Ard]
> 
>       Fixing the include guards: is a small improvement.
>       (Ideally patchcheck should reject these.)
>       Reducing lines to 80 columns: makes Leif (at least)
>       happy, and aligns with formatter behaviour. Correcting
>       Doxygen format comments: prevents Doxygen generating
>       gibberish. Spaces before '(': Maintains consistency,
>       and aligns  with desired formatter behaviour.           [Evan]
> 
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c
> | 187 +++++++++++---------
> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h |
> 10 +-
>  ArmPlatformPkg/Include/Library/LcdPlatformLib.h                    |  14 +-
>  ArmPlatformPkg/Library/HdLcd/HdLcd.c                               |  88 +++++----
>  ArmPlatformPkg/Library/HdLcd/HdLcd.h                               |  21 ++-
>  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c                         |  64 ++++---
>  6 files changed, 208 insertions(+), 176 deletions(-)
> 
> diff --git
> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> index
> b721061fc1df5695092e8c71da97ae0b9af46b3f..905eb26ee01b5037dfbaf3
> c054a62593837c8b5f 100644
> ---
> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> +++
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> - Copyright (c) 2011-2014, 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
>   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>
> @@ -22,12 +22,10 @@
> 
>  #include "LcdGraphicsOutputDxe.h"
> 
> -
> /**********************************************************
> ************
> - *
> - *  This file implements the Graphics Output protocol on
> ArmVersatileExpress
> - *  using the Lcd controller
> - *
> -
> **********************************************************
> ************/
> +/** This file implements the Graphics Output protocol on
> +ArmVersatileExpress
> +  using the Lcd controller
> +
> +**/
> 
>  //
>  // Global variables
> @@ -64,7 +62,10 @@ 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)),
> +          (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8)
> +        },
>        },
>        // Hardware Device Path for Lcd
>        EFI_CALLER_ID_GUID // Use the driver's GUID @@ -73,10 +74,13 @@
> LCD_INSTANCE mLcdTemplate = {
>      {
>        END_DEVICE_PATH_TYPE,
>        END_ENTIRE_DEVICE_PATH_SUBTYPE,
> -      { sizeof(EFI_DEVICE_PATH_PROTOCOL), 0 }
> +      {
> +        sizeof (EFI_DEVICE_PATH_PROTOCOL),
> +        0
> +      }
>      }
>    },
> -  (EFI_EVENT) NULL // ExitBootServicesEvent
> +  (EFI_EVENT)NULL // ExitBootServicesEvent
>  };
> 
>  EFI_STATUS
> @@ -86,7 +90,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 +117,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 +143,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 +162,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.
> +  // 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,
> -            &Instance->ExitBootServicesEvent
> -            );
> +                  EVT_SIGNAL_EXIT_BOOT_SERVICES,
> +                  TPL_NOTIFY,
> +                  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;
>    }
> 
> @@ -198,48 +207,46 @@ LcdGraphicsOutputDxeInitialize (
>    goto EXIT;
> 
>  EXIT_ERROR_UNINSTALL_PROTOCOL:
> -  /* The following function could return an error message,
> -   * however, to get here something must have gone wrong already,
> -   * so preserve the original error, i.e. don't change
> -   * the Status variable, even it fails to uninstall the protocol.
> -   */
> +  // The following function could return an error message,  // however,
> + to get here something must have gone wrong already,  // so preserve
> + the original error, i.e. don't change  // 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;
> -
>  }
> 
> -/***************************************
> - * This function should be called
> - * on Event: ExitBootServices
> - * to free up memory, stop the driver
> - * and uninstall the protocols
> - ***************************************/
> +/** This function should be called
> +  on Event: ExitBootServices
> +  to free up memory, stop the driver
> +  and uninstall the protocols
> +**/
>  VOID
>  LcdGraphicsExitBootServicesEvent (
>    IN EFI_EVENT  Event,
>    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 ();
>    }
>  }
> 
> -/***************************************
> - * GraphicsOutput Protocol function, mapping to
> - * EFI_GRAPHICS_OUTPUT_PROTOCOL.QueryMode
> - ***************************************/
> +/** GraphicsOutput Protocol function, mapping to
> +  EFI_GRAPHICS_OUTPUT_PROTOCOL.QueryMode
> +**/
>  EFI_STATUS
>  EFIAPI
>  LcdGraphicsQueryMode (
> @@ -252,19 +259,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,21 +285,20 @@ 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:
>    return Status;
>  }
> 
> -/***************************************
> - * GraphicsOutput Protocol function, mapping to
> - * EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode
> - ***************************************/
> +/** GraphicsOutput Protocol function, mapping to
> +  EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode
> +**/
>  EFI_STATUS
>  EFIAPI
>  LcdGraphicsSetMode (
> @@ -305,47 +314,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);
> +                                 * Instance->ModeInfo.PixelsPerScanLine
> +                                 * 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 +371,8 @@ LcdGraphicsSetMode (
>        0,
>        This->Mode->Info->HorizontalResolution,
>        This->Mode->Info->VerticalResolution,
> -      0);
> +      0
> +      );
> 
>  EXIT:
>    return Status;
> @@ -372,7 +383,7 @@ GetBytesPerPixel (
>    IN  LCD_BPP       Bpp
>    )
>  {
> -  switch(Bpp) {
> +  switch (Bpp) {
>    case LCD_BITS_PER_PIXEL_24:
>      return 4;
> 
> diff --git
> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> h
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> h
> index
> b66efd34561f655b74a5ecfad8a97281cdd5929d..16d92f45c47948630d62
> 62a63fd1af4e076706d1 100644
> ---
> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> h
> +++
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> h
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2011, 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
>    which accompanies this distribution.  The full text of the license may be
> found at @@ -11,9 +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>
> 
> @@ -25,7 +24,6 @@
> 
>  #include <Protocol/DevicePath.h>
> 
> -
>  //
>  // Device structures
>  //
> @@ -106,4 +104,4 @@ InitializeDisplay (
>    IN LCD_INSTANCE* Instance
>  );
> 
> -#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..3d13e417972c67cc51ae4
> 410efd548053511e5d1 100644
> --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> @@ -1,6 +1,6 @@
>  /** @file
> 
> - Copyright (c) 2011, 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
>   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>
> 
> @@ -158,8 +158,9 @@
>  #define LCD_12BPP_444_BLUE_MASK         0x0000000F
>  #define LCD_12BPP_444_RESERVED_MASK     0x0000F000
> 
> -
> -// The enumeration indexes maps the PL111 LcdBpp values used in the LCD
> Control Register
> +/** The enumeration indexes maps the PL111 LcdBpp values used in the
> +LCD Control
> +  Register
> +**/
>  typedef enum {
>    LCD_BITS_PER_PIXEL_1 = 0,
>    LCD_BITS_PER_PIXEL_2,
> @@ -171,7 +172,6 @@ typedef enum {
>    LCD_BITS_PER_PIXEL_12_444
>  } LCD_BPP;
> 
> -
>  EFI_STATUS
>  LcdPlatformInitializeDisplay (
>    IN EFI_HANDLE   Handle
> @@ -218,4 +218,4 @@ LcdPlatformGetBpp (
>    OUT LCD_BPP*                              Bpp
>    );
> 
> -#endif
> +#endif /* LCD_PLATFORM_LIB_H_ */
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> index
> 24efb68f23e3393a96fc760732d978b6346a2807..2cd1be9d255dd5734176
> d752d60f06f11a047fcb 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> @@ -1,6 +1,6 @@
> -/** @file  Lcd.c
> +/** @file
> 
> -  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 @@ -21,12 +21,9 @@
> 
>  #include "HdLcd.h"
> 
> -
> /**********************************************************
> ************
> - *
> - *  This file contains all the bits of the Lcd that are
> - *  platform independent.
> - *
> -
> **********************************************************
> ************/
> +/** This file contains all the bits of the Lcd that are
> +  platform independent.
> +**/
> 
>  STATIC
>  UINTN
> @@ -34,7 +31,7 @@ GetBytesPerPixel (
>    IN  LCD_BPP       Bpp
>    )
>  {
> -  switch(Bpp) {
> +  switch (Bpp) {
>    case LCD_BITS_PER_PIXEL_24:
>      return 4;
> 
> @@ -60,21 +57,27 @@ 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_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;
>  }
> @@ -96,46 +99,53 @@ LcdSetMode (
>    UINT32            BytesPerPixel;
>    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;
>    }
> 
> -  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/Library/HdLcd/HdLcd.h
> b/ArmPlatformPkg/Library/HdLcd/HdLcd.h
> index
> 6df97a9dfee60e9fda615cf3bea1b6a164a42333..cd2c0366c7b563d7fb313f
> 82abeef7eb1aa3ef72 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.h
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.h
> @@ -1,6 +1,6 @@
> -/** @file  HDLcd.h
> +/** @file
> 
> - 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 @@ -12,13 +12,10 @@
> 
>   **/
> 
> -#ifndef _HDLCD_H_
> -#define _HDLCD_H_
> +#ifndef HDLCD_H_
> +#define HDLCD_H_
> 
> -//
>  // HDLCD Controller Register Offsets
> -//
> -
>  #define HDLCD_REG_VERSION                 ((UINTN)PcdGet32
> (PcdArmHdLcdBase) + 0x000)
>  #define HDLCD_REG_INT_RAWSTAT             ((UINTN)PcdGet32
> (PcdArmHdLcdBase) + 0x010)
>  #define HDLCD_REG_INT_CLEAR               ((UINTN)PcdGet32
> (PcdArmHdLcdBase) + 0x014)
> @@ -44,10 +41,7 @@
>  #define HDLCD_REG_GREEN_SELECT            ((UINTN)PcdGet32
> (PcdArmHdLcdBase) + 0x248)
>  #define HDLCD_REG_BLUE_SELECT             ((UINTN)PcdGet32
> (PcdArmHdLcdBase) + 0x24C)
> 
> -
> -//
>  // HDLCD Values of registers
> -//
> 
>  // HDLCD Interrupt mask, clear and status register
>  #define HDLCD_DMA_END                     BIT0    /* DMA has finished reading a
> frame */
> @@ -79,6 +73,11 @@
>  #define HDLCD_DATA_LOW                    0
>  #define HDLCD_PXCLK_LOW                   0
> 
> +// Default polarities
> +#define HDLCD_DEFAULT_POLARITIES   (HDLCD_PXCLK_LOW |
> HDLCD_DATA_HIGH |       \
> +                                    HDLCD_DATEN_HIGH | HDLCD_HSYNC_LOW |      \
> +                                    HDLCD_VSYNC_HIGH)
> +
>  // Pixel Format
>  #define HDLCD_LITTLE_ENDIAN              (0 << 31)
>  #define HDLCD_BIG_ENDIAN                 (1 << 31)
> @@ -86,4 +85,4 @@
>  // Number of bytes per pixel
>  #define HDLCD_4BYTES_PER_PIXEL           ((4 - 1) << 3)
> 
> -#endif /* _HDLCD_H_ */
> +#endif /* HDLCD_H_ */
> diff --git a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> index
> 9b4a02045ab7ca170e0d4362ee0e2bcf1d275bdb..3f3f7019f1d252995eab6
> d8c3d49db8be8e09e40 100644
> --- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> +++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> @@ -1,6 +1,6 @@
> -/** @file  PL111Lcd.c
> +/** @file
> 
> -  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 @@ -19,13 +19,10 @@
> 
>  #include "PL111Lcd.h"
> 
> -
> /**********************************************************
> ************
> - *
> - *  This file contains all the bits of the PL111 that are
> - *  platform independent.
> - *
> -
> **********************************************************
> ************/
> +/** This file contains all the bits of the PL111 that are
> +  platform independent.
> 
> +**/
>  EFI_STATUS
>  LcdIdentify (
>    VOID
> @@ -54,11 +51,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 +78,54 @@ 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;
> +  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)'
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel