Re: [PATCH 13/19] ArmPlatformPkg: HdLcd Remove redundant Bpp

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

On Tue, Sep 26, 2017 at 09:15:23PM +0100, evan.lloyd@arm.com wrote:
> From: EvanLloyd <evan.lloyd@arm.com>
> 
> Because of copy/paste effects, HdLcdArmVExpress.c contained a
> table entry "LCD_BPP Bpp;" specifying the Bits per Pixel for each mode.
> However, all modes are LCD_BITS_PER_PIXEL_24.
> 
> This change removes the table entry and related use of the field.

Since it looks to be modifying some of the same lines, I would prefer
to see this patch before the previous one. Or am I misreading that?

/
    Leif

> 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/HdLcdArmVExpress.c | 42 ++++++--------------
>  1 file changed, 13 insertions(+), 29 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> index dc2c5fb89923304c46d137ec8eaefc9418548d06..2401cdb30cb7252964ce1f363aa26d99933c09be 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
> @@ -30,7 +30,6 @@
>  
>  typedef struct {
>    UINT32                     Mode;
> -  LCD_BPP                    Bpp;
>    UINT32                     OscFreq;
>  
>    // These are used by HDLCD
> @@ -42,37 +41,37 @@ typedef struct {
>  **/
>  STATIC CONST DISPLAY_MODE mDisplayModes[] = {
>    { // Mode 0 : VGA : 640 x 480 x 24 bpp
> -    VGA, LCD_BITS_PER_PIXEL_24,
> +    VGA,
>      VGA_OSC_FREQUENCY,
>      {VGA_H_RES_PIXELS, VGA_H_SYNC, VGA_H_BACK_PORCH, VGA_H_FRONT_PORCH},
>      {VGA_V_RES_PIXELS, VGA_V_SYNC, VGA_V_BACK_PORCH, VGA_V_FRONT_PORCH}
>    },
>    { // Mode 1 : SVGA : 800 x 600 x 24 bpp
> -    SVGA, LCD_BITS_PER_PIXEL_24,
> +    SVGA,
>      SVGA_OSC_FREQUENCY,
>      {SVGA_H_RES_PIXELS, SVGA_H_SYNC, SVGA_H_BACK_PORCH, SVGA_H_FRONT_PORCH},
>      {SVGA_V_RES_PIXELS, SVGA_V_SYNC, SVGA_V_BACK_PORCH, SVGA_V_FRONT_PORCH}
>    },
>    { // Mode 2 : XGA : 1024 x 768 x 24 bpp
> -    XGA, LCD_BITS_PER_PIXEL_24,
> +    XGA,
>      XGA_OSC_FREQUENCY,
>      {XGA_H_RES_PIXELS, XGA_H_SYNC, XGA_H_BACK_PORCH, XGA_H_FRONT_PORCH},
>      {XGA_V_RES_PIXELS, XGA_V_SYNC, XGA_V_BACK_PORCH, XGA_V_FRONT_PORCH}
>    },
>    { // Mode 3 : SXGA : 1280 x 1024 x 24 bpp
> -    SXGA, LCD_BITS_PER_PIXEL_24,
> +    SXGA,
>      (SXGA_OSC_FREQUENCY/2),
>      {SXGA_H_RES_PIXELS, SXGA_H_SYNC, SXGA_H_BACK_PORCH, SXGA_H_FRONT_PORCH},
>      {SXGA_V_RES_PIXELS, SXGA_V_SYNC, SXGA_V_BACK_PORCH, SXGA_V_FRONT_PORCH}
>    },
>    { // Mode 4 : UXGA : 1600 x 1200 x 24 bpp
> -    UXGA, LCD_BITS_PER_PIXEL_24,
> +    UXGA,
>      (UXGA_OSC_FREQUENCY/2),
>      {UXGA_H_RES_PIXELS, UXGA_H_SYNC, UXGA_H_BACK_PORCH, UXGA_H_FRONT_PORCH},
>      {UXGA_V_RES_PIXELS, UXGA_V_SYNC, UXGA_V_BACK_PORCH, UXGA_V_FRONT_PORCH}
>    },
>    { // Mode 5 : HD : 1920 x 1080 x 24 bpp
> -    HD, LCD_BITS_PER_PIXEL_24,
> +    HD,
>      (HD_OSC_FREQUENCY/2),
>      {HD_H_RES_PIXELS, HD_H_SYNC, HD_H_BACK_PORCH, HD_H_FRONT_PORCH},
>      {HD_V_RES_PIXELS, HD_V_SYNC, HD_V_BACK_PORCH, HD_V_FRONT_PORCH}
> @@ -283,27 +282,12 @@ LcdPlatformQueryMode (
>    Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution;
>    Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution;
>  
> -  switch (mDisplayModes[ModeNumber].Bpp) {
> -    case LCD_BITS_PER_PIXEL_24:
> -      Info->PixelFormat                   = PixelRedGreenBlueReserved8BitPerColor;
> -      Info->PixelInformation.RedMask      = LCD_24BPP_RED_MASK;
> -      Info->PixelInformation.GreenMask    = LCD_24BPP_GREEN_MASK;
> -      Info->PixelInformation.BlueMask     = LCD_24BPP_BLUE_MASK;
> -      Info->PixelInformation.ReservedMask = LCD_24BPP_RESERVED_MASK;
> -      break;
> -
> -    case LCD_BITS_PER_PIXEL_16_555:
> -    case LCD_BITS_PER_PIXEL_16_565:
> -    case LCD_BITS_PER_PIXEL_12_444:
> -    case LCD_BITS_PER_PIXEL_8:
> -    case LCD_BITS_PER_PIXEL_4:
> -    case LCD_BITS_PER_PIXEL_2:
> -    case LCD_BITS_PER_PIXEL_1:
> -    default:
> -      // These are not supported
> -      ASSERT (FALSE);
> -      break;
> -  }
> +  /* Bits per Pixel is always LCD_BITS_PER_PIXEL_24 */
> +  Info->PixelFormat                   = PixelRedGreenBlueReserved8BitPerColor;
> +  Info->PixelInformation.RedMask      = LCD_24BPP_RED_MASK;
> +  Info->PixelInformation.GreenMask    = LCD_24BPP_GREEN_MASK;
> +  Info->PixelInformation.BlueMask     = LCD_24BPP_BLUE_MASK;
> +  Info->PixelInformation.ReservedMask = LCD_24BPP_RESERVED_MASK;
>  
>    return EFI_SUCCESS;
>  }
> @@ -365,7 +349,7 @@ LcdPlatformGetBpp (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  *Bpp = mDisplayModes[ModeNumber].Bpp;
> +  *Bpp = LCD_BITS_PER_PIXEL_24;
>  
>    return EFI_SUCCESS;
>  }
> -- 
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
>