Re: [PATCH 02/19] ArmPlatformPkg: Tidy LcdGraphicsOutputDxe code: Added comments

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

Given that all changes to the first file _remove_ comments, it may be
better with a subject line saying "updating comments".

On Tue, Sep 26, 2017 at 09:15:12PM +0100, evan.lloyd@arm.com wrote:
> From: Girish Pathak <girish.pathak@arm.com>
> 
> There is no functional modification in this change
> As preparation for a Change (Rejig of LcdGraphicsOutPutDxe), some
> comments are modified and a few new comments are added.
> This is to prevent mixing formatting changes with functional
> changes.
> 
> 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/Include/Library/LcdPlatformLib.h                                    | 80 +++++++++++++++-----
>  ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c       | 63 ++++++++++++++-
>  ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 68 ++++++++++++++++-
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c                                | 20 +++++
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c                 |  4 +-
>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c                             | 22 +++++-
>  6 files changed, 231 insertions(+), 26 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> index 72ebcd02ddb321ee0dad51c87ac8ee876d9ca21c..48bdd8a51411137df040aa797fcff272785f7a35 100644
> --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> @@ -18,9 +18,7 @@

Is this where one of the stray copyright updates from 1/19 should go?

>  
>  #define LCD_VRAM_SIZE                     SIZE_8MB
>  
> -//
>  // Modes definitions
> -//
>  #define VGA                               0
>  #define SVGA                              1
>  #define XGA                               2
> @@ -29,9 +27,7 @@
>  #define UXGA                              5
>  #define HD                                6
>  
> -//
>  // VGA Mode: 640 x 480
> -//
>  #define VGA_H_RES_PIXELS                  640
>  #define VGA_V_RES_PIXELS                  480
>  #define VGA_OSC_FREQUENCY                 23750000  /* 0x016A6570 */
> @@ -44,9 +40,7 @@
>  #define VGA_V_FRONT_PORCH                 (  3 - 1)
>  #define VGA_V_BACK_PORCH                  ( 13 - 1)
>  
> -//
>  // SVGA Mode: 800 x 600
> -//
>  #define SVGA_H_RES_PIXELS                 800
>  #define SVGA_V_RES_PIXELS                 600
>  #define SVGA_OSC_FREQUENCY                38250000  /* 0x0247A610 */
> @@ -59,9 +53,7 @@
>  #define SVGA_V_FRONT_PORCH                (  3 - 1)
>  #define SVGA_V_BACK_PORCH                 ( 17 - 1)
>  
> -//
>  // XGA Mode: 1024 x 768
> -//
>  #define XGA_H_RES_PIXELS                  1024
>  #define XGA_V_RES_PIXELS                  768
>  #define XGA_OSC_FREQUENCY                 63500000  /* 0x03C8EEE0 */
> @@ -74,9 +66,7 @@
>  #define XGA_V_FRONT_PORCH                 (  3 - 1)
>  #define XGA_V_BACK_PORCH                  ( 23 - 1)
>  
> -//
>  // SXGA Mode: 1280 x 1024
> -//
>  #define SXGA_H_RES_PIXELS                 1280
>  #define SXGA_V_RES_PIXELS                 1024
>  #define SXGA_OSC_FREQUENCY                109000000  /* 0x067F3540 */
> @@ -89,9 +79,7 @@
>  #define SXGA_V_FRONT_PORCH                (  3 - 1)
>  #define SXGA_V_BACK_PORCH                 ( 29 - 1)
>  
> -//
>  // WSXGA+ Mode: 1680 x 1050
> -//
>  #define WSXGA_H_RES_PIXELS                1680
>  #define WSXGA_V_RES_PIXELS                1050
>  #define WSXGA_OSC_FREQUENCY               147000000  /* 0x08C30AC0 */
> @@ -104,9 +92,7 @@
>  #define WSXGA_V_FRONT_PORCH               (  4 - 1)
>  #define WSXGA_V_BACK_PORCH                ( 41 - 1)
>  
> -//
>  // UXGA Mode: 1600 x 1200
> -//
>  #define UXGA_H_RES_PIXELS                 1600
>  #define UXGA_V_RES_PIXELS                 1200
>  #define UXGA_OSC_FREQUENCY                161000000  /* 0x0998AA40 */
> @@ -119,9 +105,7 @@
>  #define UXGA_V_FRONT_PORCH                (  3 - 1)
>  #define UXGA_V_BACK_PORCH                 ( 38 - 1)
>  
> -//
>  // HD Mode: 1920 x 1080
> -//
>  #define HD_H_RES_PIXELS                   1920
>  #define HD_V_RES_PIXELS                   1080
>  #define HD_OSC_FREQUENCY                  165000000  /* 0x09D5B340 */
> @@ -134,10 +118,7 @@
>  #define HD_V_FRONT_PORCH                  (  3 - 1)
>  #define HD_V_BACK_PORCH                   ( 32 - 1)
>  
> -//
>  // Colour Masks
> -//
> -
>  #define LCD_24BPP_RED_MASK              0x00FF0000
>  #define LCD_24BPP_GREEN_MASK            0x0000FF00
>  #define LCD_24BPP_BLUE_MASK             0x000000FF
> @@ -171,34 +152,85 @@ typedef enum {
>    LCD_BITS_PER_PIXEL_12_444
>  } LCD_BPP;
>  
> -
> +/** Platform related initialization function.
> +  *
> +  * @param IN Handle               Handle to the LCD device instance.
> +  *
> +  * @retval EFI_SUCCESS            Platform initialization success.
> +  * @retval !(EFI_SUCCESS)         Other errors.
> +**/

So ... 6.8 lists
/**
  text
**/
as the 

The format
/**
 * text
**/
is mentioned as "also legal because doxygen ignores the leading *".

The format
/**
  *
**/
is never mentioned, although I guess "also legal" because * ignored.

However, a quick skim in MdePkg suggests the former is the generally
used variant. Can you please update to that format throughout (drop
the leading '*' on lines not starting or ending the comment block)?

No other comments (other than having these prototype documentations
are a great improvement).

/
    Leif

>  EFI_STATUS
>  LcdPlatformInitializeDisplay (
>    IN EFI_HANDLE   Handle
>    );
>  
> +/** Reserve VRAM memory in DRAM for the frame buffer
> +  * (unless it is reserved already).
> +  *
> +  * The allocated address can be used to set the frame buffer.
> +  * @param OUT VramBaseAddress      A pointer to the frame buffer address.
> +  * @param OUT VramSize             A pointer to the size of the frame
> +  *                                 buffer in bytes
> +  *
> +  * @retval EFI_SUCCESS             Frame buffer memory allocation success.
> +  * @retval !(EFI_SUCCESS)          Other errors.
> +**/
>  EFI_STATUS
>  LcdPlatformGetVram (
>    OUT EFI_PHYSICAL_ADDRESS*                 VramBaseAddress,
>    OUT UINTN*                                VramSize
>    );
>  
> +/** Return total number of modes.
> +  *
> +  * @retval UINT32             Mode Number.
> +**/
>  UINT32
>  LcdPlatformGetMaxMode (
>    VOID
>    );
>  
> +/** Set the requested display mode.
> +  *
> +  * @param IN ModeNumber             Mode Number.
> +  * @retval   EFI_SUCCESS            Set mode success.
> +  * @retval   EFI_INVALID_PARAMTER   Requested mode not found.
> +**/
>  EFI_STATUS
>  LcdPlatformSetMode (
>    IN UINT32                                 ModeNumber
>    );
>  
> +/** Return information for the requested mode number.
> +  *
> +  * @param IN ModeNumber            Mode Number.
> +  * @param OUT Info                 Pointer for returned mode information
> +  *                                 (on success).
> +  *
> +  * @retval EFI_SUCCESS             Success if the requested mode is found.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +**/
>  EFI_STATUS
>  LcdPlatformQueryMode (
>    IN  UINT32                                ModeNumber,
>    OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *Info
>    );
>  
> +/** Returns the display timing information for the requested mode number.
> +  *
> +  * @param IN  ModeNumber           Mode Number.
> +  * @param OUT HRes                 Pointer to horizontal resolution.
> +  * @param OUT HSync                Pointer to horizontal sync width.
> +  * @param OUT HBackPorch           Pointer to horizontal back porch.
> +  * @param OUT HFrontPorch          Pointer to horizontal front porch.
> +  * @param OUT VRes                 Pointer to vertical resolution.
> +  * @param OUT VSync                Pointer to vertical sync width.
> +  * @param OUT VBackPorch           Pointer to vertical back porch.
> +  * @param OUT VFrontPorch          Pointer to vertical front porch.
> +
> +  * @retval EFI_SUCCESS             Success if the requested mode is found.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +**/
>  EFI_STATUS
>  LcdPlatformGetTimings (
>    IN  UINT32                              ModeNumber,
> @@ -212,6 +244,14 @@ LcdPlatformGetTimings (
>    OUT UINT32*                             VFrontPorch
>    );
>  
> +/** Return bits per pixel information for a mode number.
> +  *
> +  * @param IN  ModeNumber           Mode Number.
> +  * @param OUT Bpp                  Pointer to value Bytes Per Pixel.
> +  *
> +  * @retval EFI_SUCCESS             The requested mode is found.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +**/
>  EFI_STATUS
>  LcdPlatformGetBpp (
>    IN  UINT32                                ModeNumber,
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> index 2041de5f63c72de6f0ce4047420c282507a1d04a..cfe3259d3c737de240350e8c3eab867b80c40948 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> @@ -44,7 +44,8 @@ typedef struct {
>    UINT32                     VFrontPorch;
>  } LCD_RESOLUTION;
>  
> -
> +/** The display modes supported by the platform.
> +**/
>  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,
> @@ -94,6 +95,11 @@ EFI_EDID_ACTIVE_PROTOCOL      mEdidActive = {
>    NULL
>  };
>  
> +/** HDLCD Platform specific initialization function.
> +  *
> +  * @retval EFI_SUCCESS            Plaform library initialization success.
> +  * @retval !(EFI_SUCCESS)         Other errors.
> +**/
>  EFI_STATUS
>  LcdPlatformInitializeDisplay (
>    IN EFI_HANDLE   Handle
> @@ -124,6 +130,18 @@ LcdPlatformInitializeDisplay (
>    return Status;
>  }
>  
> +/** Reserve VRAM memory in DRAM for the frame buffer
> +  * (unless it is reserved already).
> +  *
> +  * The allocated address can be used to set the frame buffer.
> +  *
> +  * @param OUT VramBaseAddress      A pointer to the frame buffer address.
> +  * @param OUT VramSize             A pointer to the size of the frame
> +  *                                 buffer in bytes
> +  *
> +  * @retval EFI_SUCCESS             Frame buffer memory allocation success.
> +  * @retval !(EFI_SUCCESS)          Other errors.
> +**/
>  EFI_STATUS
>  LcdPlatformGetVram (
>    OUT EFI_PHYSICAL_ADDRESS*  VramBaseAddress,
> @@ -170,6 +188,13 @@ LcdPlatformGetVram (
>    return EFI_SUCCESS;
>  }
>  
> +/** Return total number of modes supported.
> +  *
> +  * Note: Valid mode numbers are 0 to MaxMode - 1
> +  * See Section 11.9 of the UEFI Specification 2.6 Errata A (Jan 2017)
> +  *
> +  * @retval UINT32             Mode Number.
> +**/
>  UINT32
>  LcdPlatformGetMaxMode(VOID)
>  {
> @@ -178,6 +203,10 @@ LcdPlatformGetMaxMode(VOID)
>    return (sizeof (mResolutions) / sizeof (LCD_RESOLUTION));
>  }
>  
> +/** Set the requested display mode.
> +  *
> +  * @param IN ModeNumber             Mode Number.
> +**/
>  EFI_STATUS
>  LcdPlatformSetMode (
>    IN UINT32                         ModeNumber
> @@ -227,6 +256,15 @@ LcdPlatformSetMode (
>    return Status;
>  }
>  
> +/** Return information for the requested mode number.
> +  *
> +  * @param IN ModeNumber            Mode Number.
> +  * @param OUT Info                 Pointer for returned mode information
> +  *                                 (on success).
> +  *
> +  * @retval EFI_SUCCESS             Success if the requested mode is found.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +**/
>  EFI_STATUS
>  LcdPlatformQueryMode (
>    IN  UINT32                                ModeNumber,
> @@ -267,6 +305,21 @@ LcdPlatformQueryMode (
>    return EFI_SUCCESS;
>  }
>  
> +/** Returns the display timing information for the requested mode number.
> +  *
> +  * @param IN  ModeNumber           Mode Number.
> +  * @param OUT HRes                 Pointer to horizontal resolution.
> +  * @param OUT HSync                Pointer to horizontal sync width.
> +  * @param OUT HBackPorch           Pointer to horizontal back porch.
> +  * @param OUT HFrontPorch          Pointer to horizontal front porch.
> +  * @param OUT VRes                 Pointer to vertical resolution.
> +  * @param OUT VSync                Pointer to vertical sync width.
> +  * @param OUT VBackPorch           Pointer to vertical back porch.
> +  * @param OUT VFrontPorch          Pointer to vertical front porch.
> +  *
> +  * @retval EFI_SUCCESS             Success if the requested mode is found.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +**/
>  EFI_STATUS
>  LcdPlatformGetTimings (
>    IN  UINT32                              ModeNumber,
> @@ -296,6 +349,14 @@ LcdPlatformGetTimings (
>    return EFI_SUCCESS;
>  }
>  
> +/** Return bits per pixel for a mode number.
> +  *
> +  * @param IN  ModeNumber           Mode Number.
> +  * @param OUT Bpp                  Pointer to value Bits Per Pixel.
> +  *
> +  * @retval EFI_SUCCESS             The requested mode is found.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +**/
>  EFI_STATUS
>  LcdPlatformGetBpp (
>    IN  UINT32                              ModeNumber,
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> index 8d046816454f642bced00e29c4e02093b74afd24..84880e5fd1dfe6f824b27e53926f9bb32ff6cdf7 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
> @@ -41,7 +41,8 @@ typedef struct {
>    UINT32                     VFrontPorch;
>  } LCD_RESOLUTION;
>  
> -
> +/** The display modes supported by the platform.
> +**/
>  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,
> @@ -151,7 +152,11 @@ EFI_EDID_ACTIVE_PROTOCOL      mEdidActive = {
>    NULL
>  };
>  
> -
> +/** PL111 Platform specific initialization function.
> +  *
> +  * @retval EFI_SUCCESS            Plaform library initialization success.
> +  * @retval !(EFI_SUCCESS)         Other errors.
> +**/
>  EFI_STATUS
>  LcdPlatformInitializeDisplay (
>    IN EFI_HANDLE   Handle
> @@ -176,6 +181,18 @@ LcdPlatformInitializeDisplay (
>    return Status;
>  }
>  
> +/** Reserve VRAM memory in DRAM for the frame buffer
> +  * (unless it is reserved already).
> +  *
> +  * The allocated address can be used to set the frame buffer.
> +  *
> +  * @param OUT VramBaseAddress      A pointer to the frame buffer address.
> +  * @param OUT VramSize             A pointer to the size of the frame
> +  *                                 buffer in bytes
> +  *
> +  * @retval EFI_SUCCESS             Frame buffer memory allocation success.
> +  * @retval !(EFI_SUCCESS)          Other errors.
> +**/
>  EFI_STATUS
>  LcdPlatformGetVram (
>    OUT EFI_PHYSICAL_ADDRESS*  VramBaseAddress,
> @@ -232,6 +249,13 @@ LcdPlatformGetVram (
>    return Status;
>  }
>  
> +/** Return total number of modes supported.
> +  *
> +  * Note: Valid mode numbers are 0 to MaxMode - 1
> +  * See Section 11.9 of the UEFI Specification 2.6 Errata A (Jan 2017)
> +  *
> +  * @retval UINT32             Mode Number.
> +**/
>  UINT32
>  LcdPlatformGetMaxMode(VOID)
>  {
> @@ -249,6 +273,14 @@ LcdPlatformGetMaxMode(VOID)
>    return (PcdGet32 (PcdPL111LcdMaxMode));
>  }
>  
> +/** Set the requested display mode.
> +  *
> +  * @param IN ModeNumber            Mode Number.
> +  *
> +  * @retval  EFI_INVALID_PARAMETER  Requested mode not found.
> +  * @retval  EFI_UNSUPPORTED        PLL111 configuration not supported.
> +  * @retval  !(EFI_SUCCESS)         Other errors.
> +**/
>  EFI_STATUS
>  LcdPlatformSetMode (
>    IN UINT32                         ModeNumber
> @@ -320,6 +352,15 @@ LcdPlatformSetMode (
>    return Status;
>  }
>  
> +/** Return information for the requested mode number.
> +  *
> +  * @param IN ModeNumber            Mode Number.
> +  * @param OUT Info                 Pointer for returned mode information
> +  *                                 (on success).
> +  *
> +  * @retval EFI_SUCCESS             Success if the requested mode is found.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +**/
>  EFI_STATUS
>  LcdPlatformQueryMode (
>    IN  UINT32                                ModeNumber,
> @@ -360,6 +401,21 @@ LcdPlatformQueryMode (
>    return EFI_SUCCESS;
>  }
>  
> +/** Returns the display timing information for the requested mode number.
> +  *
> +  * @param IN  ModeNumber           Mode Number.
> +  * @param OUT HRes                 Pointer to horizontal resolution.
> +  * @param OUT HSync                Pointer to horizontal sync width.
> +  * @param OUT HBackPorch           Pointer to horizontal back porch.
> +  * @param OUT HFrontPorch          Pointer to horizontal front porch.
> +  * @param OUT VRes                 Pointer to vertical resolution.
> +  * @param OUT VSync                Pointer to vertical sync width.
> +  * @param OUT VBackPorch           Pointer to vertical back porch.
> +  * @param OUT VFrontPorch          Pointer to vertical front porch.
> +  *
> +  * @retval EFI_SUCCESS             Success if the requested mode is found.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +**/
>  EFI_STATUS
>  LcdPlatformGetTimings (
>    IN  UINT32                              ModeNumber,
> @@ -389,6 +445,14 @@ LcdPlatformGetTimings (
>    return EFI_SUCCESS;
>  }
>  
> +/** Return bits per pixel for a mode number.
> +  *
> +  * @param IN  ModeNumber           Mode Number.
> +  * @param OUT Bpp                  Pointer to value Bits Per Pixel.
> +  *
> +  * @retval EFI_SUCCESS             The requested mode is found.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +**/
>  EFI_STATUS
>  LcdPlatformGetBpp (
>    IN  UINT32                              ModeNumber,
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> index eb0b6fb3fbbc1cb605469433f6c6dcb85bac668c..744dd3d556b5071defc6bcad5a9a30881bcb4b6f 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
> @@ -29,6 +29,12 @@
>   *
>   **********************************************************************/
>  
> +/** Initialize display.
> +  *
> +  * @param  VramBaseAddress        Address of the frame buffer.
> +  *
> +  * @retval EFI_SUCCESS            Display initialization success.
> +**/
>  EFI_STATUS
>  LcdInitialize (
>    IN EFI_PHYSICAL_ADDRESS   VramBaseAddress
> @@ -67,6 +73,12 @@ LcdInitialize (
>    return EFI_SUCCESS;
>  }
>  
> +/** Set requested mode of the display.
> +  *
> +  * @param  ModeNumber             Display mode number.
> +  * @retval EFI_SUCCESS            Display set mode success.
> +  * @retval EFI_DEVICE_ERROR       If mode not found/supported.
> +**/
>  EFI_STATUS
>  LcdSetMode (
>    IN UINT32  ModeNumber
> @@ -136,6 +148,8 @@ LcdSetMode (
>    return EFI_SUCCESS;
>  }
>  
> +/** De-initializes the display.
> +**/
>  VOID
>  LcdShutdown (
>    VOID
> @@ -145,6 +159,12 @@ LcdShutdown (
>    MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_DISABLE);
>  }
>  
> +/** Check for presence of HDLCD.
> +  *
> +  * @retval EFI_SUCCESS            Platform implements HDLCD.
> +  * @retval EFI_NOT_FOUND          HDLCD display controller not
> +  *                                found.
> +**/
>  EFI_STATUS
>  LcdIdentify (
>    VOID
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c
> index 2dd8f39873f77b1c211bff407cabe90c1795b121..c40c8e0fa6f4b5f7798aeb3c8bf3f261f14cb67b 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c
> @@ -357,8 +357,8 @@ LcdGraphicsSetMode (
>      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);
> diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> index 0b0c4204fbc44bc9e90dce3d7b410ce167d9f40c..f8a3c1f8266c0a11f111c3747688defc0d49877c 100644
> --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
> @@ -26,6 +26,12 @@
>   *
>   **********************************************************************/
>  
> +/** Check for presence of PL111.
> +  *
> +  * @retval EFI_SUCCESS            Platform implements PL111.
> +  * @retval EFI_NOT_FOUND          PL111 display controller not
> +  *                                found.
> +**/
>  EFI_STATUS
>  LcdIdentify (
>    VOID
> @@ -48,6 +54,12 @@ LcdIdentify (
>    return EFI_NOT_FOUND;
>  }
>  
> +/** Initialize display.
> +  *
> +  * @param  VramBaseAddress        Address of the frame buffer.
> +  *
> +  * @retval EFI_SUCCESS            Display initialization success.
> +**/
>  EFI_STATUS
>  LcdInitialize (
>    IN EFI_PHYSICAL_ADDRESS   VramBaseAddress
> @@ -63,6 +75,12 @@ LcdInitialize (
>    return EFI_SUCCESS;
>  }
>  
> +/** Set requested mode of the display.
> +  *
> +  * @param  ModeNumber             Display mode number.
> +  * @retval EFI_SUCCESS            Display set mode success.
> +  * @retval EFI_DEVICE_ERROR       If mode not found/supported.
> +**/
>  EFI_STATUS
>  LcdSetMode (
>    IN UINT32  ModeNumber
> @@ -123,7 +141,7 @@ LcdSetMode (
>  
>    // PL111_REG_LCD_CONTROL
>    LcdControl = PL111_CTRL_LCD_EN | PL111_CTRL_LCD_BPP (LcdBpp)
> -                 | PL111_CTRL_LCD_TFT | PL111_CTRL_BGR;
> +               | PL111_CTRL_LCD_TFT | PL111_CTRL_BGR;
>  
>    MmioWrite32 (PL111_REG_LCD_CONTROL, LcdControl);
>  
> @@ -134,6 +152,8 @@ LcdSetMode (
>    return EFI_SUCCESS;
>  }
>  
> +/** De-initializes the display.
> +*/
>  VOID
>  LcdShutdown (
>    VOID
> -- 
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel