[edk2] [PATCH v2 07/13] ArmPlatformPkg: Redefine LcdPlatformGetTimings function

evan.lloyd@arm.com posted 13 patches 7 years ago
[edk2] [PATCH v2 07/13] ArmPlatformPkg: Redefine LcdPlatformGetTimings function
Posted by evan.lloyd@arm.com 7 years ago
From: Girish Pathak <girish.pathak at arm.com>

The LcdPlatformGetTimings interface function takes similar sets of
multiple parameters for horizontal and vertical timings which can be
aggregated in a common data type. This change defines a structure
SCAN_TIMINGS for this which can be used to describe both horizontal and
vertical scan timings, and accordingly redefines the
LcdPlatformGetTiming interface, greatly reducing the amount of data
passed about.

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 | 31 ++++++-----
 ArmPlatformPkg/Library/HdLcd/HdLcd.c            | 56 +++++++++-----------
 ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c      | 49 +++++++++--------
 3 files changed, 68 insertions(+), 68 deletions(-)

diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
index 2a70307031fc21c8fb0d655d358ca9a102a95920..0943f28a19133e7bc558f9d529bb8ac8f66ba3fd 100644
--- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
+++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
@@ -153,6 +153,14 @@ typedef enum {
   LCD_BITS_PER_PIXEL_12_444
 } LCD_BPP;
 
+// Display timing settings.
+typedef struct {
+  UINT32                      Resolution;
+  UINT32                      Sync;
+  UINT32                      BackPorch;
+  UINT32                      FrontPorch;
+} SCAN_TIMINGS;
+
 /** Platform related initialization function.
 
   @param[in] Handle              Handle to the LCD device instance.
@@ -228,14 +236,11 @@ LcdPlatformQueryMode (
 
   @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.
+  @param[out] Horizontal          Pointer to horizontal timing parameters.
+                                  (Resolution, Sync, Back porch, Front porch)
+  @param[out] Vertical            Pointer to vertical timing parameters.
+                                  (Resolution, Sync, Back porch, Front porch)
+
 
   @retval EFI_SUCCESS             Display timing information for the requested
                                   mode returned successfully.
@@ -244,14 +249,8 @@ LcdPlatformQueryMode (
 EFI_STATUS
 LcdPlatformGetTimings (
   IN  UINT32                              ModeNumber,
-  OUT UINT32*                             HRes,
-  OUT UINT32*                             HSync,
-  OUT UINT32*                             HBackPorch,
-  OUT UINT32*                             HFrontPorch,
-  OUT UINT32*                             VRes,
-  OUT UINT32*                             VSync,
-  OUT UINT32*                             VBackPorch,
-  OUT UINT32*                             VFrontPorch
+  OUT CONST SCAN_TIMINGS                  **Horizontal,
+  OUT CONST SCAN_TIMINGS                  **Vertical
   );
 
 /** Return bits per pixel information for a mode number.
diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
index b2779af041fae58d712270002cc7d6d277360311..1fc04c2d14d8185370454be459a23bdec41f6602 100644
--- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
+++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
@@ -98,35 +98,26 @@ LcdSetMode (
   IN CONST UINT32  ModeNumber
   )
 {
-  EFI_STATUS        Status;
-  UINT32            HRes;
-  UINT32            HSync;
-  UINT32            HBackPorch;
-  UINT32            HFrontPorch;
-  UINT32            VRes;
-  UINT32            VSync;
-  UINT32            VBackPorch;
-  UINT32            VFrontPorch;
-  UINT32            BytesPerPixel;
-  LCD_BPP           LcdBpp;
+  EFI_STATUS          Status;
+  CONST SCAN_TIMINGS *Horizontal;
+  CONST SCAN_TIMINGS *Vertical;
+  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
+             &Horizontal,
+             &Vertical
              );
   if (EFI_ERROR (Status)) {
     ASSERT (FALSE);
     return Status;
   }
 
+  ASSERT (Horizontal != NULL);
+  ASSERT (Vertical != NULL);
+
   Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
   if (EFI_ERROR (Status)) {
     ASSERT (FALSE);
@@ -139,21 +130,26 @@ LcdSetMode (
   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,
+    Horizontal->Resolution * BytesPerPixel
+    );
+
+  MmioWrite32 (HDLCD_REG_FB_LINE_PITCH, Horizontal->Resolution * BytesPerPixel);
+
+  MmioWrite32 (HDLCD_REG_FB_LINE_COUNT, Vertical->Resolution - 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,        Vertical->Sync);
+  MmioWrite32 (HDLCD_REG_V_BACK_PORCH,  Vertical->BackPorch);
+  MmioWrite32 (HDLCD_REG_V_DATA,        Vertical->Resolution - 1);
+  MmioWrite32 (HDLCD_REG_V_FRONT_PORCH, Vertical->FrontPorch);
 
   // 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,        Horizontal->Sync);
+  MmioWrite32 (HDLCD_REG_H_BACK_PORCH,  Horizontal->BackPorch);
+  MmioWrite32 (HDLCD_REG_H_DATA,        Horizontal->Resolution - 1);
+  MmioWrite32 (HDLCD_REG_H_FRONT_PORCH, Horizontal->FrontPorch);
 
   // Enable the controller
   MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_ENABLE);
diff --git a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
index b236cbfcfffadd6fa7610dcae5c5bc748aac0f69..d1aba083fdc2ee4a7c25294955c4413465dca1e8 100644
--- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
+++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
@@ -81,35 +81,26 @@ LcdSetMode (
   IN CONST UINT32  ModeNumber
   )
 {
-  EFI_STATUS        Status;
-  UINT32            HRes;
-  UINT32            HSync;
-  UINT32            HBackPorch;
-  UINT32            HFrontPorch;
-  UINT32            VRes;
-  UINT32            VSync;
-  UINT32            VBackPorch;
-  UINT32            VFrontPorch;
-  UINT32            LcdControl;
-  LCD_BPP           LcdBpp;
+  EFI_STATUS          Status;
+  CONST SCAN_TIMINGS *Horizontal;
+  CONST SCAN_TIMINGS *Vertical;
+  UINT32              LcdControl;
+  LCD_BPP             LcdBpp;
 
   // Set the video mode timings and other relevant information
   Status = LcdPlatformGetTimings (
              ModeNumber,
-             &HRes,
-             &HSync,
-             &HBackPorch,
-             &HFrontPorch,
-             &VRes,
-             &VSync,
-             &VBackPorch,
-             &VFrontPorch
+             &Horizontal,
+             &Vertical
              );
   if (EFI_ERROR (Status)) {
     ASSERT (FALSE);
     return Status;
   }
 
+  ASSERT (Horizontal != NULL);
+  ASSERT (Vertical != NULL);
+
   Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
   if (EFI_ERROR (Status)) {
     ASSERT (FALSE);
@@ -122,15 +113,29 @@ LcdSetMode (
   // Set Timings
   MmioWrite32 (
     PL111_REG_LCD_TIMING_0,
-    HOR_AXIS_PANEL (HBackPorch, HFrontPorch, HSync, HRes)
+    HOR_AXIS_PANEL (
+      Horizontal->BackPorch,
+      Horizontal->FrontPorch,
+      Horizontal->Sync,
+      Horizontal->Resolution
+      )
     );
 
   MmioWrite32 (
     PL111_REG_LCD_TIMING_1,
-    VER_AXIS_PANEL (VBackPorch, VFrontPorch, VSync, VRes)
+    VER_AXIS_PANEL (
+      Vertical->BackPorch,
+      Vertical->FrontPorch,
+      Vertical->Sync,
+      Vertical->Resolution
+      )
+    );
+
+  MmioWrite32 (
+    PL111_REG_LCD_TIMING_2,
+    CLK_SIG_POLARITY (Horizontal->Resolution)
     );
 
-  MmioWrite32 (PL111_REG_LCD_TIMING_2, CLK_SIG_POLARITY (HRes));
   MmioWrite32 (PL111_REG_LCD_TIMING_3, 0);
 
   // PL111_REG_LCD_CONTROL
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 07/13] ArmPlatformPkg: Redefine LcdPlatformGetTimings function
Posted by Ard Biesheuvel 7 years ago
On 22 December 2017 at 18:34,  <evan.lloyd@arm.com> wrote:
> From: Girish Pathak <girish.pathak at arm.com>
>
> The LcdPlatformGetTimings interface function takes similar sets of
> multiple parameters for horizontal and vertical timings which can be
> aggregated in a common data type. This change defines a structure
> SCAN_TIMINGS for this which can be used to describe both horizontal and
> vertical scan timings, and accordingly redefines the
> LcdPlatformGetTiming interface, greatly reducing the amount of data
> passed about.
>
> 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 | 31 ++++++-----
>  ArmPlatformPkg/Library/HdLcd/HdLcd.c            | 56 +++++++++-----------
>  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c      | 49 +++++++++--------
>  3 files changed, 68 insertions(+), 68 deletions(-)
>
> diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> index 2a70307031fc21c8fb0d655d358ca9a102a95920..0943f28a19133e7bc558f9d529bb8ac8f66ba3fd 100644
> --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> @@ -153,6 +153,14 @@ typedef enum {
>    LCD_BITS_PER_PIXEL_12_444
>  } LCD_BPP;
>
> +// Display timing settings.
> +typedef struct {
> +  UINT32                      Resolution;
> +  UINT32                      Sync;
> +  UINT32                      BackPorch;
> +  UINT32                      FrontPorch;
> +} SCAN_TIMINGS;
> +
>  /** Platform related initialization function.
>
>    @param[in] Handle              Handle to the LCD device instance.
> @@ -228,14 +236,11 @@ LcdPlatformQueryMode (
>
>    @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.
> +  @param[out] Horizontal          Pointer to horizontal timing parameters.
> +                                  (Resolution, Sync, Back porch, Front porch)
> +  @param[out] Vertical            Pointer to vertical timing parameters.
> +                                  (Resolution, Sync, Back porch, Front porch)
> +
>
>    @retval EFI_SUCCESS             Display timing information for the requested
>                                    mode returned successfully.
> @@ -244,14 +249,8 @@ LcdPlatformQueryMode (
>  EFI_STATUS
>  LcdPlatformGetTimings (
>    IN  UINT32                              ModeNumber,
> -  OUT UINT32*                             HRes,
> -  OUT UINT32*                             HSync,
> -  OUT UINT32*                             HBackPorch,
> -  OUT UINT32*                             HFrontPorch,
> -  OUT UINT32*                             VRes,
> -  OUT UINT32*                             VSync,
> -  OUT UINT32*                             VBackPorch,
> -  OUT UINT32*                             VFrontPorch
> +  OUT CONST SCAN_TIMINGS                  **Horizontal,
> +  OUT CONST SCAN_TIMINGS                  **Vertical
>    );
>
>  /** Return bits per pixel information for a mode number.
> diff --git a/ArmPlatformPkg/Library/HdLcd/HdLcd.c b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> index b2779af041fae58d712270002cc7d6d277360311..1fc04c2d14d8185370454be459a23bdec41f6602 100644
> --- a/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> +++ b/ArmPlatformPkg/Library/HdLcd/HdLcd.c
> @@ -98,35 +98,26 @@ LcdSetMode (
>    IN CONST UINT32  ModeNumber
>    )
>  {
> -  EFI_STATUS        Status;
> -  UINT32            HRes;
> -  UINT32            HSync;
> -  UINT32            HBackPorch;
> -  UINT32            HFrontPorch;
> -  UINT32            VRes;
> -  UINT32            VSync;
> -  UINT32            VBackPorch;
> -  UINT32            VFrontPorch;
> -  UINT32            BytesPerPixel;
> -  LCD_BPP           LcdBpp;
> +  EFI_STATUS          Status;
> +  CONST SCAN_TIMINGS *Horizontal;
> +  CONST SCAN_TIMINGS *Vertical;

Please align the * with the 'S' in Status.

With that fixed,
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> +  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
> +             &Horizontal,
> +             &Vertical
>               );
>    if (EFI_ERROR (Status)) {
>      ASSERT (FALSE);
>      return Status;
>    }
>
> +  ASSERT (Horizontal != NULL);
> +  ASSERT (Vertical != NULL);
> +
>    Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
>    if (EFI_ERROR (Status)) {
>      ASSERT (FALSE);
> @@ -139,21 +130,26 @@ LcdSetMode (
>    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,
> +    Horizontal->Resolution * BytesPerPixel
> +    );
> +
> +  MmioWrite32 (HDLCD_REG_FB_LINE_PITCH, Horizontal->Resolution * BytesPerPixel);
> +
> +  MmioWrite32 (HDLCD_REG_FB_LINE_COUNT, Vertical->Resolution - 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,        Vertical->Sync);
> +  MmioWrite32 (HDLCD_REG_V_BACK_PORCH,  Vertical->BackPorch);
> +  MmioWrite32 (HDLCD_REG_V_DATA,        Vertical->Resolution - 1);
> +  MmioWrite32 (HDLCD_REG_V_FRONT_PORCH, Vertical->FrontPorch);
>
>    // 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,        Horizontal->Sync);
> +  MmioWrite32 (HDLCD_REG_H_BACK_PORCH,  Horizontal->BackPorch);
> +  MmioWrite32 (HDLCD_REG_H_DATA,        Horizontal->Resolution - 1);
> +  MmioWrite32 (HDLCD_REG_H_FRONT_PORCH, Horizontal->FrontPorch);
>
>    // Enable the controller
>    MmioWrite32 (HDLCD_REG_COMMAND, HDLCD_ENABLE);
> diff --git a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> index b236cbfcfffadd6fa7610dcae5c5bc748aac0f69..d1aba083fdc2ee4a7c25294955c4413465dca1e8 100644
> --- a/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> +++ b/ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c
> @@ -81,35 +81,26 @@ LcdSetMode (
>    IN CONST UINT32  ModeNumber
>    )
>  {
> -  EFI_STATUS        Status;
> -  UINT32            HRes;
> -  UINT32            HSync;
> -  UINT32            HBackPorch;
> -  UINT32            HFrontPorch;
> -  UINT32            VRes;
> -  UINT32            VSync;
> -  UINT32            VBackPorch;
> -  UINT32            VFrontPorch;
> -  UINT32            LcdControl;
> -  LCD_BPP           LcdBpp;
> +  EFI_STATUS          Status;
> +  CONST SCAN_TIMINGS *Horizontal;
> +  CONST SCAN_TIMINGS *Vertical;
> +  UINT32              LcdControl;
> +  LCD_BPP             LcdBpp;
>
>    // Set the video mode timings and other relevant information
>    Status = LcdPlatformGetTimings (
>               ModeNumber,
> -             &HRes,
> -             &HSync,
> -             &HBackPorch,
> -             &HFrontPorch,
> -             &VRes,
> -             &VSync,
> -             &VBackPorch,
> -             &VFrontPorch
> +             &Horizontal,
> +             &Vertical
>               );
>    if (EFI_ERROR (Status)) {
>      ASSERT (FALSE);
>      return Status;
>    }
>
> +  ASSERT (Horizontal != NULL);
> +  ASSERT (Vertical != NULL);
> +
>    Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
>    if (EFI_ERROR (Status)) {
>      ASSERT (FALSE);
> @@ -122,15 +113,29 @@ LcdSetMode (
>    // Set Timings
>    MmioWrite32 (
>      PL111_REG_LCD_TIMING_0,
> -    HOR_AXIS_PANEL (HBackPorch, HFrontPorch, HSync, HRes)
> +    HOR_AXIS_PANEL (
> +      Horizontal->BackPorch,
> +      Horizontal->FrontPorch,
> +      Horizontal->Sync,
> +      Horizontal->Resolution
> +      )
>      );
>
>    MmioWrite32 (
>      PL111_REG_LCD_TIMING_1,
> -    VER_AXIS_PANEL (VBackPorch, VFrontPorch, VSync, VRes)
> +    VER_AXIS_PANEL (
> +      Vertical->BackPorch,
> +      Vertical->FrontPorch,
> +      Vertical->Sync,
> +      Vertical->Resolution
> +      )
> +    );
> +
> +  MmioWrite32 (
> +    PL111_REG_LCD_TIMING_2,
> +    CLK_SIG_POLARITY (Horizontal->Resolution)
>      );
>
> -  MmioWrite32 (PL111_REG_LCD_TIMING_2, CLK_SIG_POLARITY (HRes));
>    MmioWrite32 (PL111_REG_LCD_TIMING_3, 0);
>
>    // PL111_REG_LCD_CONTROL
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel