On Tue, Sep 26, 2017 at 09:15:29PM +0100, evan.ll...@arm.com wrote:
> From: Girish Pathak <girish.pat...@arm.com>
> 
> This change adds support for the ARM Mali DP500/DP500/DP650 display
> processors using the GOP protocol. It has been tested on FVP base
> models + DP550 support.
> 
> This change does not modify functionality provided by PL111 or
> HDLCD. The driver should be suitable for those platforms
> that implement ARM Mali DP500/DP550/DP650 replacing PL111/HDLCD.

Do we have anyone able to test this on hardware?
The models are sometimes more forgiving...

> Only "Layer Graphics" of the ARM Mali DP is configured for rendering
> the RGB/BGR format frame buffer to satisfy the UEFI GOP requirements
> Other layers e.g. video layers are not configured.
> 
> NOTE: This change implements the Mali DP on models. Versions for actual
> hardware are liable to require extra handling for clock input changes,
> etc.

Right. That's a no on my question above.
OK - but please drop this note. Or at least the "NOTE: This change
implements the Mali DP on models." bit.

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Girish Pathak <girish.pat...@arm.com>
> Signed-off-by: Evan Lloyd <evan.ll...@arm.com>
> ---
>  ArmPlatformPkg/ArmPlatformPkg.dec                                           
> |   4 +
>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec                            
> |   4 +-
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf         
> |  45 +++
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf 
> |   3 +
>  ArmPlatformPkg/Drivers/ArmMaliDp/ArmMaliDp.inf                              
> |  44 +++
>  ArmPlatformPkg/Include/Drivers/ArmMaliDp.h                                  
> | 249 ++++++++++++
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c           
> | 377 ++++++++++++++++++
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c          
> |  10 +-
>  ArmPlatformPkg/Drivers/ArmMaliDp/ArmMaliDp.c                                
> | 412 ++++++++++++++++++++
>  9 files changed, 1146 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec 
> b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 
> 0174f63e77f5b8430e106289366feb9a6577fb99..3be236413cb8e12ce32e6349732d2767fe0f3024
>  100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -111,6 +111,10 @@ [PcdsFixedAtBuild.common]
>    gArmPlatformTokenSpaceGuid.PcdPL111LcdBase|0x0|UINT32|0x00000026
>    gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase|0x0|UINT32|0x00000027
>  
> +  ## ARM Mali Display Processor DP500/DP550/DP650
> +  gArmPlatformTokenSpaceGuid.PcdArmMaliDpBase|0x0|UINT64|0x00000050
> +  
> gArmPlatformTokenSpaceGuid.PcdArmMaliDpMemoryRegionLength|0x0|UINT32|0x00000051
> +
>    ## If set, frame buffer memory will be reserved and mapped in the system 
> RAM
>    
> gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize|0x0|UINT32|0x00000033
>    
> gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase|0x0|UINT64|0x00000034
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec 
> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec
> index 
> 3814513c2241c03a8c45fda92d571c65368f64b4..1a55b0434ab4bc53d631be2bffd313684bcea9c4
>  100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec
> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec
> @@ -1,7 +1,7 @@
>  #/** @file
>  # Arm Versatile Express package.
>  #
> -#  Copyright (c) 2012-2015, ARM Limited. All rights reserved.
> +#  Copyright (c) 2012-2017, ARM Limited. All rights reserved.
>  #
>  #  This program and the accompanying materials are licensed and made 
> available
>  #  under the terms and conditions of the BSD License which accompanies this
> @@ -47,6 +47,8 @@ [PcdsFixedAtBuild.common]
>    gArmVExpressTokenSpaceGuid.PcdPL111LcdVideoModeOscId|1|UINT32|0x00000002
>    gArmVExpressTokenSpaceGuid.PcdHdLcdVideoModeOscId|0|UINT32|0x00000003
>  
> +  gArmVExpressTokenSpaceGuid.PcdArmMaliDpMaxMode|0x0|UINT32|0x00000005
> +
>    #
>    # Device path of block device on which Fastboot will flash partitions
>    #
> diff --git 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf 
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..b85205a8ac6bfbd8befdd8634a7df89e213175bc
> --- /dev/null
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
> @@ -0,0 +1,45 @@
> +#/** @file ArmMaliDpLib.inf
> +#
> +#  Component description file for ArmMaliDpLib module
> +#
> +#  Copyright (c) 2017, ARM Limited. All rights reserved.
> +#
> +#  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
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005

Bump to something more recent?

> +  BASE_NAME                      = ArmMaliDpLib
> +  FILE_GUID                      = 36C47FED-2F3F-49C7-89CE-31B13F0431D8
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = LcdPlatformLib
> +
> +[Sources.common]
> +  ArmMaliDpLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec

Please sort alphabetically.

> +
> +[LibraryClasses]
> +  BaseLib
> +  DxeServicesTableLib
> +
> +[FixedPcd.common]
> +  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase
> +  gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize
> +  gArmPlatformTokenSpaceGuid.PcdGopPixelFormat
> +
> +  # MaxMode must be one number higher than the actual max mode,
> +  # i.e. for actual maximum mode 2, set the value to 3.
> +  # See Section 11.9 of the UEFI Specification 2.6 Errata A (Jan 2017)
> +  gArmVExpressTokenSpaceGuid.PcdArmMaliDpMaxMode
> +
> diff --git 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf 
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
> index 
> 60789e9b8ff1b936db04953a765fb164b0e85a40..6ad7d94c8c76404c8a84345b5f992884ab5aca36
>  100644
> --- 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
> +++ 
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
> @@ -60,5 +60,8 @@ [FixedPcd]
>    gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferBase
>    gArmPlatformTokenSpaceGuid.PcdArmLcdDdrFrameBufferSize
>  
> +  gArmPlatformTokenSpaceGuid.PcdArmMaliDpBase
> +  gArmPlatformTokenSpaceGuid.PcdArmMaliDpMemoryRegionLength
> +
>  [Ppis]
>    gArmMpCoreInfoPpiGuid
> diff --git a/ArmPlatformPkg/Drivers/ArmMaliDp/ArmMaliDp.inf 
> b/ArmPlatformPkg/Drivers/ArmMaliDp/ArmMaliDp.inf
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..69155a123ef71ecf5361ab29e0ae71454c7ec6cd
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/ArmMaliDp/ArmMaliDp.inf
> @@ -0,0 +1,44 @@
> +#/** @file ArmMaliDp.inf
> +#
> +#  Component description file for ArmMaliDp module
> +#
> +#  Copyright (c) 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
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005

Bump?

> +  BASE_NAME                      = ArmMaliDp
> +  FILE_GUID                      = E724AAF7-19E2-40A3-BAE1-D82A7C8B7A76
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = LcdHwLib
> +
> +[Sources.common]
> +  ArmMaliDp.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec

Sort alphabetically?

> +
> +[LibraryClasses]
> +  UefiLib
> +  BaseLib
> +  DebugLib
> +  IoLib
> +  BaseMemoryLib
> +  LcdPlatformLib

Sort alphabetically?

> +
> +[FixedPcd]
> +  gArmPlatformTokenSpaceGuid.PcdArmMaliDpBase
> +
> diff --git a/ArmPlatformPkg/Include/Drivers/ArmMaliDp.h 
> b/ArmPlatformPkg/Include/Drivers/ArmMaliDp.h
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..4955176800ea59f352399d9c6b590195d4ccfa41
> --- /dev/null
> +++ b/ArmPlatformPkg/Include/Drivers/ArmMaliDp.h
> @@ -0,0 +1,249 @@
> +/** @file  ArmMaliDp.h
> +
> +  File defines registers and other flags of ARM Mali DP display controller
> +
> +  Copyright (c) 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
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +#ifndef ARMMALIDP_H_
> +#define ARMMALIDP_H_
> +
> +/**********************************************************************
> + *
> + *  This header file contains all the bits of the ARM Mali DP that are
> + *  platform independent.
> + *
> + **********************************************************************/
> +#define DP_BASE                            (FixedPcdGet64 (PcdArmMaliDpBase))
> +
> +// MALI DP Ids
> +#define MALIDP_500                         0x500
> +#define MALIDP_550                         0x550
> +#define MALIDP_650                         0x650
> +
> +// DP500 Peripheral Ids
> +#define DP500_ID_PART_0                    0x00
> +#define DP500_ID_DES_0                     0xB
> +#define DP500_ID_PART_1                    0x5
> +
> +#define DP500_ID_REVISION                  0x1
> +#define DP500_ID_JEDEC                     0x1
> +#define DP500_ID_DES_1                     0x3
> +
> +#define DP500_PERIPHERAL_ID0_VAL           (DP500_ID_PART_0)
> +#define DP500_PERIPHERAL_ID1_VAL           ((DP500_ID_DES_0 << 4)      \
> +                                            | DP500_ID_PART_1)
> +#define DP500_PERIPHERAL_ID2_VAL           ((DP500_ID_REVISION << 4)   \
> +                                            | (DP500_ID_JEDEC << 3)    \
> +                                            | (DP500_ID_DES_1))
> +
> +// DP550 Peripheral Ids
> +#define DP550_ID_PART_0                    0x50
> +#define DP550_ID_DES_0                     0xB
> +#define DP550_ID_PART_1                    0x5
> +
> +#define DP550_ID_REVISION                  0x0
> +#define DP550_ID_JEDEC                     0x1
> +#define DP550_ID_DES_1                     0x3
> +
> +#define DP550_PERIPHERAL_ID0_VAL           (DP550_ID_PART_0)
> +#define DP550_PERIPHERAL_ID1_VAL           ((DP550_ID_DES_0 << 4)      \
> +                                               | DP550_ID_PART_1)
> +#define DP550_PERIPHERAL_ID2_VAL           ((DP550_ID_REVISION << 4)   \
> +                                               | (DP550_ID_JEDEC << 3) \
> +                                               | (DP550_ID_DES_1))
> +
> +// DP650 Peripheral Ids
> +#define DP650_ID_PART_0                    0x50
> +#define DP650_ID_DES_0                     0xB
> +#define DP650_ID_PART_1                    0x6
> +
> +#define DP650_ID_REVISION                  0x0
> +#define DP650_ID_JEDEC                     0x1
> +#define DP650_ID_DES_1                     0x3
> +
> +#define DP650_PERIPHERAL_ID0_VAL           (DP650_ID_PART_0)
> +#define DP650_PERIPHERAL_ID1_VAL           ((DP650_ID_DES_0 << 4)      \
> +                                            | DP650_ID_PART_1)
> +#define DP650_PERIPHERAL_ID2_VAL           ((DP650_ID_REVISION << 4)   \
> +                                            | (DP650_ID_JEDEC << 3)    \
> +                                            | (DP650_ID_DES_1))
> +
> +// Display Engine (DE) control register offsets for DP550/DP650
> +#define DP_DE_STATUS                       0x00000
> +#define DP_DE_IRQ_SET                      0x00004
> +#define DP_DE_IRQ_MASK                     0x00008
> +#define DP_DE_IRQ_CLEAR                    0x0000C
> +#define DP_DE_CONTROL                      0x00010
> +#define DP_DE_PROG_LINE                    0x00014
> +#define DP_DE_AXI_CONTROL                  0x00018
> +#define DP_DE_AXI_QOS                      0x0001C
> +#define DP_DE_DISPLAY_FUNCTION             0x00020
> +
> +#define DP_DE_H_INTERVALS                  0x00030
> +#define DP_DE_V_INTERVALS                  0x00034
> +#define DP_DE_SYNC_CONTROL                 0x00038
> +#define DP_DE_HV_ACTIVESIZE                0x0003C
> +#define DP_DE_DISPLAY_SIDEBAND             0x00040
> +#define DP_DE_BACKGROUND_COLOR             0x00044
> +#define DP_DE_DISPLAY_SPLIT                0x00048
> +#define DP_DE_OUTPUT_DEPTH                 0x0004C
> +
> +// Display Engine (DE) control register offsets for DP500
> +#define DP_DE_DP500_CORE_ID                0x00018
> +#define DP_DE_DP500_CONTROL                0x0000C
> +#define DP_DE_DP500_PROG_LINE              0x00010
> +#define DP_DE_DP500_H_INTERVALS            0x00028
> +#define DP_DE_DP500_V_INTERVALS            0x0002C
> +#define DP_DE_DP500_SYNC_CONTROL           0x00030
> +#define DP_DE_DP500_HV_ACTIVESIZE          0x00034
> +#define DP_DE_DP500_BG_COLOR_RG            0x0003C
> +#define DP_DE_DP500_BG_COLOR_B             0x00040
> +
> +/* Display Engine (DE) graphics layer (LG) register offsets
> + * NOTE: For DP500 it will be LG2.
> + */
> +#define DE_LG_OFFSET                       0x00300
> +#define DP_DE_LG_FORMAT                    (DE_LG_OFFSET)
> +#define DP_DE_LG_CONTROL                   (DE_LG_OFFSET + 0x04)
> +#define DP_DE_LG_COMPOSE                   (DE_LG_OFFSET + 0x08)
> +#define DP_DE_LG_IN_SIZE                   (DE_LG_OFFSET + 0x0C)
> +#define DP_DE_LG_CMP_SIZE                  (DE_LG_OFFSET + 0x10)
> +#define DP_DE_LG_OFFSET                    (DE_LG_OFFSET + 0x14)
> +#define DP_DE_LG_H_STRIDE                  (DE_LG_OFFSET + 0x18)
> +#define DP_DE_LG_PTR_LOW                   (DE_LG_OFFSET + 0x1C)
> +#define DP_DE_LG_PTR_HIGH                  (DE_LG_OFFSET + 0x20)
> +#define DP_DE_LG_CHROMA_KEY                (DE_LG_OFFSET + 0x2C)
> +#define DP_DE_LG_AD_CONTROL                (DE_LG_OFFSET + 0x30)
> +#define DP_DE_LG_MMU_CONTROL               (DE_LG_OFFSET + 0x48)
> +
> +// Display core (DC) control register offsets.
> +#define DP_DC_OFFSET                       0x0C000
> +#define DP_DC_STATUS                       (DP_DC_OFFSET + 0x00)
> +#define DP_DC_IRQ_SET                      (DP_DC_OFFSET + 0x04)
> +#define DP_DC_IRQ_MASK                     (DP_DC_OFFSET + 0x08)
> +#define DP_DC_IRQ_CLEAR                    (DP_DC_OFFSET + 0x0C)
> +#define DP_DC_CONTROL                      (DP_DC_OFFSET + 0x10)
> +#define DP_DC_CONFIG_VALID                 (DP_DC_OFFSET + 0x14)
> +#define DP_DC_CORE_ID                      (DP_DC_OFFSET + 0x18)
> +
> +// DP500 has a global configuration register.
> +#define DP_DP500_CONFIG_VALID              (0xF00)
> +
> +// Display core ID register offsets.
> +#define DP_DC_ID_OFFSET                    0x0FF00
> +#define DP_DC_ID_PERIPHERAL_ID4            (DP_DC_ID_OFFSET + 0xD0)
> +#define DP_DC_CONFIGURATION_ID             (DP_DC_ID_OFFSET + 0xD4)
> +#define DP_DC_PERIPHERAL_ID0               (DP_DC_ID_OFFSET + 0xE0)
> +#define DP_DC_PERIPHERAL_ID1               (DP_DC_ID_OFFSET + 0xE4)
> +#define DP_DC_PERIPHERAL_ID2               (DP_DC_ID_OFFSET + 0xE8)
> +#define DP_DC_COMPONENT_ID0                (DP_DC_ID_OFFSET + 0xF0)
> +#define DP_DC_COMPONENT_ID1                (DP_DC_ID_OFFSET + 0xF4)
> +#define DP_DC_COMPONENT_ID2                (DP_DC_ID_OFFSET + 0xF8)
> +#define DP_DC_COMPONENT_ID3                (DP_DC_ID_OFFSET + 0xFC)
> +
> +#define DP_DP500_ID_OFFSET                 0x0F00
> +#define DP_DP500_ID_PERIPHERAL_ID4         (DP_DP500_ID_OFFSET + 0xD0)
> +#define DP_DP500_CONFIGURATION_ID          (DP_DP500_ID_OFFSET + 0xD4)
> +#define DP_DP500_PERIPHERAL_ID0            (DP_DP500_ID_OFFSET + 0xE0)
> +#define DP_DP500_PERIPHERAL_ID1            (DP_DP500_ID_OFFSET + 0xE4)
> +#define DP_DP500_PERIPHERAL_ID2            (DP_DP500_ID_OFFSET + 0xE8)
> +#define DP_DP500_COMPONENT_ID0             (DP_DP500_ID_OFFSET + 0xF0)
> +#define DP_DP500_COMPONENT_ID1             (DP_DP500_ID_OFFSET + 0xF4)
> +#define DP_DP500_COMPONENT_ID2             (DP_DP500_ID_OFFSET + 0xF8)
> +#define DP_DP500_COMPONENT_ID3             (DP_DP500_ID_OFFSET + 0xFC)
> +
> +// Display status configuration mode activation flag
> +#define DP_DC_STATUS_CM_ACTIVE_FLAG        (0x1U << 16)
> +
> +// Display core control configuration mode
> +#define DP_DC_CONTROL_SRST_ACTIVE          (0x1U << 18)
> +#define DP_DC_CONTROL_CRST_ACTIVE          (0x1U << 17)
> +#define DP_DC_CONTROL_CM_ACTIVE            (0x1U << 16)
> +
> +#define DP_DE_DP500_CONTROL_SOFTRESET_REQ  (0x1U << 16)
> +#define DP_DE_DP500_CONTROL_CONFIG_REQ     (0x1U << 17)
> +
> +// Display core configuration valid register
> +#define DP_DC_CONFIG_VALID_CVAL            (0x1U)
> +
> +/* DC_CORE_ID
> + * Display core version register PRODUCT_ID
> + */
> +#define DP_DC_CORE_ID_SHIFT                16
> +#define DP_DE_DP500_CORE_ID_SHIFT          DP_DC_CORE_ID_SHIFT
> +
> +// Timing settings
> +#define DP_DE_HBACKPORCH_SHIFT             16
> +#define DP_DE_VBACKPORCH_SHIFT             16
> +#define DP_DE_VSP_SHIFT                    28
> +#define DP_DE_VSYNCWIDTH_SHIFT             16
> +#define DP_DE_HSP_SHIFT                    13
> +#define DP_DE_V_ACTIVE_SHIFT               16
> +
> +// BACKGROUND_COLOR
> +#define DP_DE_BG_R_PIXEL_SHIFT             16
> +#define DP_DE_BG_G_PIXEL_SHIFT             8
> +
> +//Graphics layer LG_FORMAT Pixel Format
> +#define DP_PIXEL_FORMAT_ARGB_8888          0x8
> +#define DP_PIXEL_FORMAT_ABGR_8888          0x9
> +#define DP_PIXEL_FORMAT_RGBA_8888          0xA
> +#define DP_PIXEL_FORMAT_BGRA_8888          0xB
> +#define DP_PIXEL_FORMAT_XRGB_8888          0x10
> +#define DP_PIXEL_FORMAT_XBGR_8888          0x11
> +#define DP_PIXEL_FORMAT_RGBX_8888          0x12
> +#define DP_PIXEL_FORMAT_BGRX_8888          0x13
> +#define DP_PIXEL_FORMAT_RGB_888            0x18
> +#define DP_PIXEL_FORMAT_BGR_888            0x19
> +
> +// DP500 format code are different than DP550/DP650
> +#define DP_PIXEL_FORMAT_DP500_ARGB_8888    0x2
> +#define DP_PIXEL_FORMAT_DP500_ABGR_8888    0x3
> +#define DP_PIXEL_FORMAT_DP500_XRGB_8888    0x4
> +#define DP_PIXEL_FORMAT_DP500_XBGR_8888    0x5
> +
> +// Graphics layer LG_PTR_LOW and LG_PTR_HIGH
> +#define DP_DE_LG_PTR_LOW_MASK              0xFFFFFFFFU
> +#define DP_DE_LG_PTR_HIGH_SHIFT            32
> +
> +// Graphics layer LG_CONTROL register characteristics
> +#define DP_DE_LG_L_ALPHA_SHIFT             16
> +#define DP_DE_LG_CHK_SHIFT                 15
> +#define DP_DE_LG_PMUL_SHIFT                14
> +#define DP_DE_LG_COM_SHIFT                 12
> +#define DP_DE_LG_VFP_SHIFT                 11
> +#define DP_DE_LG_HFP_SHIFT                 10
> +#define DP_DE_LG_ROTATION_SHIFT            8
> +
> +#define DP_DE_LG_LAYER_BLEND_NO_BG         0x0U
> +#define DP_DE_LG_PIXEL_BLEND_NO_BG         0x1U
> +#define DP_DE_LG_LAYER_BLEND_BG            0x2U
> +#define DP_DE_LG_PIXEL_BLEND_BG            0x3U
> +#define DP_DE_LG_ENABLE                    0x1U
> +
> +// Graphics layer LG_IN_SIZE register characteristics
> +#define DP_DE_LG_V_IN_SIZE_SHIFT           16
> +
> +// Graphics layer LG_CMP_SIZE register characteristics
> +#define DP_DE_LG_V_CMP_SIZE_SHIFT          16
> +#define DP_DE_LG_V_OFFSET_SHIFT            16
> +
> +// Helper display timing macro functions.
> +#define H_INTERVALS(Hfp, Hbp)        ((Hbp << DP_DE_HBACKPORCH_SHIFT) | Hfp)
> +#define V_INTERVALS(Vfp, Vbp)        ((Vbp << DP_DE_VBACKPORCH_SHIFT) | Vfp)
> +#define SYNC_WIDTH(Hsw, Vsw)         ((Vsw << DP_DE_VSYNCWIDTH_SHIFT) | Hsw)
> +#define HV_ACTIVE(Hor, Ver)          ((Ver << DP_DE_V_ACTIVE_SHIFT)   | Hor)
> +
> +// Helper layer graphics macros.
> +#define FRAME_IN_SIZE(Hor, Ver)      ((Ver << DP_DE_LG_V_IN_SIZE_SHIFT) | 
> Hor)
> +#define FRAME_CMP_SIZE(Hor, Ver)     ((Ver << DP_DE_LG_V_CMP_SIZE_SHIFT) | 
> Hor)
> +
> +#endif /* ARMMALIDP_H_ */
> diff --git 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c 
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..81924cc41f5898605038a1bc20af096fbb85d81c
> --- /dev/null
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
> @@ -0,0 +1,377 @@
> +/** @file ArmMaliDpLib.c
> +*
> +*  The file contains ARM Mali DP platform specific implementation.
> +*
> +*  Copyright (c) 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
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  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/PcdLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/LcdPlatformLib.h>

Please sort alphabetically (at least within the Library/ ones).

> +#include <ArmPlatform.h>
> +
> +/** Check an address is within 40 bits.
> +  *
> +  * The ARM Mali DP frame buffer address size can not be wider than 40 bits
> +**/
> +#define  DP_VALID_BASE_ADDR(Address)  ((Address >> 40) == 0)
> +
> +typedef struct {
> +  UINT32                      Mode;
> +  UINT32                      OscFreq;
> +  SCAN_TIMINGS                Horizontal;
> +  SCAN_TIMINGS                Vertical;
> +} DISPLAY_MODE;
> +
> +/** The display modes implemented by this driver.
> +  *
> +  * On Models, the OSC frequencies (listed for each mode below) are not used.
> +  * However these frequencies are useful on hardware plaforms where related
> +  * clock (or PLL) settings are based on these pixel clocks.
> +  *
> +  * Since the clock settings are defined externally, the driver must
> +  * communicate pixel clock frequencies to relevant modules
> +  * responsible for setting clocks. e.g. SCP.
> +**/
> +STATIC CONST DISPLAY_MODE mDisplayModes[] = {
> +  {
> +    // Mode 0 : VGA : 640 x 480 x 24 bpp.
> +    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 : WVGA : 800 x 480 x 24 bpp.
> +    WVGA,
> +    WVGA_OSC_FREQUENCY,
> +    {WVGA_H_RES_PIXELS, WVGA_H_SYNC, WVGA_H_BACK_PORCH, WVGA_H_FRONT_PORCH},
> +    {WVGA_V_RES_PIXELS, WVGA_V_SYNC, WVGA_V_BACK_PORCH, WVGA_V_FRONT_PORCH}
> +  },
> +  {
> +    // Mode 2 : SVGA : 800 x 600 x 24 bpp.
> +    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 3 : QHD : 960 x 540 x 24 bpp.
> +    QHD,
> +    QHD_OSC_FREQUENCY,
> +    {QHD_H_RES_PIXELS, QHD_H_SYNC, QHD_H_BACK_PORCH, QHD_H_FRONT_PORCH},
> +    {QHD_V_RES_PIXELS, QHD_V_SYNC, QHD_V_BACK_PORCH, QHD_V_FRONT_PORCH}
> +  },
> +  {
> +    // Mode 4 : WSVGA : 1024 x 600 x 24 bpp.
> +    WSVGA,
> +    WSVGA_OSC_FREQUENCY,
> +    {WSVGA_H_RES_PIXELS, WSVGA_H_SYNC, WSVGA_H_BACK_PORCH, 
> WSVGA_H_FRONT_PORCH},
> +    {WSVGA_V_RES_PIXELS, WSVGA_V_SYNC, WSVGA_V_BACK_PORCH, 
> WSVGA_V_FRONT_PORCH}
> +  },
> +  {
> +    // Mode 5 : XGA : 1024 x 768 x 24 bpp.
> +    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 6 : HD : 1280 x 720 x 24 bpp.
> +    HD720,
> +    HD720_OSC_FREQUENCY,
> +    {HD720_H_RES_PIXELS, HD720_H_SYNC, HD720_H_BACK_PORCH, 
> HD720_H_FRONT_PORCH},
> +    {HD720_V_RES_PIXELS, HD720_V_SYNC, HD720_V_BACK_PORCH, 
> HD720_V_FRONT_PORCH}
> +  },
> +  {
> +    // Mode 7 : WXGA : 1280 x 800 x 24 bpp.
> +    WXGA,
> +    WXGA_OSC_FREQUENCY,
> +    {WXGA_H_RES_PIXELS, WXGA_H_SYNC, WXGA_H_BACK_PORCH, WXGA_H_FRONT_PORCH},
> +    {WXGA_V_RES_PIXELS, WXGA_V_SYNC, WXGA_V_BACK_PORCH, WXGA_V_FRONT_PORCH}
> +  },
> +  { // Mode 8 : SXGA : 1280 x 1024 x 24 bpp.
> +    SXGA,
> +    SXGA_OSC_FREQUENCY,
> +    {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 9 : WSXGA+ : 1680 x 1050 x 24 bpp.
> +    WSXGA,
> +    WSXGA_OSC_FREQUENCY,
> +    {WSXGA_H_RES_PIXELS, WSXGA_H_SYNC, WSXGA_H_BACK_PORCH, 
> WSXGA_H_FRONT_PORCH},
> +    {WSXGA_V_RES_PIXELS,WSXGA_V_SYNC, WSXGA_V_BACK_PORCH, 
> WSXGA_V_FRONT_PORCH}
> +  },
> +  {
> +    // Mode 10 : HD : 1920 x 1080 x 24 bpp.
> +    HD,
> +    HD_OSC_FREQUENCY,
> +    {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}
> +  }
> +};
> +
> +/** If PcdArmMaliDpMaxMode is 0, platform supports full range of modes
> +  * else platform supports modes from 0 to PcdArmMaliDpMaxMode - 1
> +**/
> +STATIC CONST UINT32 mMaxMode = ((FixedPcdGet32 (PcdArmMaliDpMaxMode) != 0)
> +                                   ? FixedPcdGet32 (PcdArmMaliDpMaxMode)
> +                                   : sizeof (mDisplayModes) / sizeof 
> (DISPLAY_MODE));
> +
> +
> +/** Platform related initialization function.
> +  *
> +  * @retval EFI_SUCCESS            Platform initialization success.
> +  * @retval EFI_UNSUPPORTED        PcdGopPixelFormat must be
> +  *                                PixelRedGreenBlueReserved8BitPerColor OR
> +  *                                PixelBlueGreenRedReserved8BitPerColor
> +  *                                any other format is not supported.
> +**/
> +EFI_STATUS
> +LcdPlatformInitializeDisplay (
> +  IN CONST EFI_HANDLE   Handle
> +  )
> +{
> +  EFI_GRAPHICS_PIXEL_FORMAT PixelFormat;
> +
> +  (VOID)Handle;
> +
> +  // PixelBitMask and PixelBltOnly pixel formats are not supported
> +  PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
> +  if (PixelFormat != PixelRedGreenBlueReserved8BitPerColor
> +    && PixelFormat != PixelBlueGreenRedReserved8BitPerColor) {
> +
> +    ASSERT (PixelFormat == PixelRedGreenBlueReserved8BitPerColor
> +      ||  PixelFormat == PixelBlueGreenRedReserved8BitPerColor);
> +   return EFI_UNSUPPORTED;
> +  }

This is the same sequence I asked for the existing drivers if it could
be a common function, isn't it?

Could it? :)

> +
> +  return EFI_SUCCESS;
> +}
> +
> +/** 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 as a base 
> buffer
> +  * address for any layer of the ARM Mali DP.
> +  *
> +  * @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_INVALID_PARAMETER   VramBaseAddress or VramSize are NULL.
> +  * @retval EFI_UNSUPPORTED         Allocated address wider than 40 bits
> +  * @retval !EFI_SUCCESS            Other errors.
> +**/
> +EFI_STATUS
> +LcdPlatformGetVram (
> +  OUT EFI_PHYSICAL_ADDRESS * CONST VramBaseAddress,
> +  OUT UINTN                * CONST VramSize
> +  )
> +{
> +  EFI_STATUS      Status;
> +
> +  // Check VramBaseAddress and VramSize are not NULL.
> +  if (VramBaseAddress == NULL || VramSize == NULL) {
> +    ASSERT (VramBaseAddress != NULL);
> +    ASSERT (VramSize != NULL);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Set the VRAM size.
> +  *VramSize = (UINTN)FixedPcdGet32 (PcdArmLcdDdrFrameBufferSize);
> +
> +  // Check if memory is already reserved for the frame buffer.
> +#if (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase) != 0)

This conditional adds a lot of complexity (and length) to this
function. Can we instead break this out into a conditionally-selected
helper function?

So, here we get:

  Status = GetVramBuffer (VramBaseAddress, VramSize);
  (or even return GetVramBuffer();)

Then somewhere above, we get

STATIC
EFI_STATUS
GetVramBuffer (
  OUT EFI_PHYSICAL_ADDRESS * CONST VramBaseAddress,
  OUT UINTN                * CONST VramSize
  )
#if (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase) != 0)
#if (!DP_VALID_BASE_ADDR (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase)))
#error ARM Mali DP frame buffer base address cannot be wider than
#endif
{
  // VRAM already allocated - return it directly
  *VramBaseAddress =
    (EFI_PHYSICAL_ADDRESS)FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase);

  return EFI_SUCCESS;
}  
#else
{
  EFI_STATUS Status;

  // Attempt to allocate the VRAM from the DRAM.
  Status = gBS->AllocatePages (
                  AllocateAnyPages,
                  EfiBootServicesData,
                  EFI_SIZE_TO_PAGES (*VramSize),
                  VramBaseAddress
                  );

  if (EFI_ERROR (Status)) {
    DEBUG ((DEBUG_ERROR, "ArmMaliDpLib: Failed to allocate frame
    buffer.\n"));
    ASSERT_EFI_ERROR (Status);
    return Status;
  }

  // ARM Mali DP frame buffer base address can not be wider than 40
  // bits.
  if (!DP_VALID_BASE_ADDR (*VramBaseAddress)) {
    gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES
    gBS->(*VramSize));
    ASSERT (DP_VALID_BASE_ADDR (*VramBaseAddress));
    return EFI_UNSUPPORTED;
  }

  /* Mark the VRAM as write-combining. The VRAM is inside the DRAM,
  which is cacheable.
   * For ARM/AArch64 EFI_MEMORY_WC memory is actually uncached.
   */
  Status = gDS->SetMemorySpaceAttributes (
                  *VramBaseAddress,
                  *VramSize,
                  EFI_MEMORY_WC
                  );

  if (EFI_ERROR (Status)) {
    ASSERT_EFI_ERROR (Status);
    gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES
    gBS->(*VramSize));
  }
}
#endif

?

> +
> +#if (!DP_VALID_BASE_ADDR (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase)))
> +#error ARM Mali DP frame buffer base address cannot be wider than 40 bits.
> +#else
> +
> +  *VramBaseAddress =
> +    (EFI_PHYSICAL_ADDRESS)FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase);
> +
> +  Status = EFI_SUCCESS;
> +#endif
> +
> +#else
> +  // If not already reserved, attempt to allocate the VRAM from the DRAM.
> +  Status = gBS->AllocatePages (
> +                  AllocateAnyPages,
> +                  EfiBootServicesData,
> +                  EFI_SIZE_TO_PAGES (*VramSize),
> +                  VramBaseAddress
> +                  );
> +
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "ArmMaliDpLib: Failed to allocate frame 
> buffer.\n"));
> +    ASSERT_EFI_ERROR (Status);
> +    return Status;
> +  }
> +
> +  // ARM Mali DP frame buffer base address can not be wider than 40 bits.
> +  if (!DP_VALID_BASE_ADDR (*VramBaseAddress)) {
> +    gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
> +    ASSERT (DP_VALID_BASE_ADDR (*VramBaseAddress));
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  /* Mark the VRAM as write-combining. The VRAM is inside the DRAM, which is 
> cacheable.
> +   * For ARM/AArch64 EFI_MEMORY_WC memory is actually uncached.
> +   */
> +  Status = gDS->SetMemorySpaceAttributes (
> +                  *VramBaseAddress,
> +                  *VramSize,
> +                  EFI_MEMORY_WC
> +                  );
> +
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
> +  }
> +
> +#endif
> +  return Status;
> +}
> +
> +/** Return total number of modes supported.
> +  *
> +  * Note: Valid mode numbers are 0 to MaxMode - 1
> +  * See Section 12.9 of the UEFI Specification 2.7
> +  *
> +  * @retval UINT32             Mode Number.
> +**/
> +UINT32
> +LcdPlatformGetMaxMode (VOID)
> +{
> +  return  mMaxMode;
> +}
> +
> +/** Set the requested display mode.
> +  *
> +  * @param IN ModeNumber             Mode Number.
> +  *
> +  * @retval EFI_SUCCESS              Set mode success.
> +  * @retval EFI_INVALID_PARAMETER    Requested mode not found.
> +**/
> +EFI_STATUS
> +LcdPlatformSetMode (
> +  IN CONST UINT32 ModeNumber
> +  )
> +{
> +
> +  if (ModeNumber >= mMaxMode) {
> +    ASSERT (ModeNumber < mMaxMode);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  /* On models, platform specific clock/mux settings are not required
> +   * Display controller specific settings for Mali DP are done in LcdSetMode.
> +   */
> +  return EFI_SUCCESS;
> +}
> +
> +/** 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             Requested mode found.
> +  * @retval EFI_INVALID_PARAMETER   Info is NULL.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +**/
> +EFI_STATUS
> +LcdPlatformQueryMode (
> +  IN  CONST UINT32                                 ModeNumber,
> +  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info
> +  )
> +{
> +  if (ModeNumber >= mMaxMode || Info == NULL) {
> +    ASSERT (ModeNumber < mMaxMode);
> +    ASSERT (Info != NULL);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Info->Version = 0;
> +  Info->HorizontalResolution = 
> mDisplayModes[ModeNumber].Horizontal.Resolution;
> +  Info->VerticalResolution = mDisplayModes[ModeNumber].Vertical.Resolution;
> +  Info->PixelsPerScanLine = mDisplayModes[ModeNumber].Horizontal.Resolution;
> +
> +  Info->PixelFormat = FixedPcdGet32 (PcdGopPixelFormat);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/** Returns the display timing information for the requested mode number.
> +  *
> +  * @param IN  ModeNumber           Mode Number.
> +  * @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             Requested mode found.
> +  * @retval EFI_INVALID_PARAMETER   Requested mode not found.
> +  * @retval EFI_INVALID_PARAMETER   One of the OUT parameters is NULL.
> +**/
> +EFI_STATUS
> +LcdPlatformGetTimings (
> +  IN  UINT32         ModeNumber,
> +  OUT CONST SCAN_TIMINGS ** Horizontal,
> +  OUT CONST SCAN_TIMINGS ** Vertical
> +  )
> +{
> +  if (ModeNumber >= mMaxMode || Horizontal == NULL || Vertical == NULL) {
> +    ASSERT (ModeNumber < mMaxMode);
> +    // One of the pointers is NULL
> +    ASSERT (Horizontal != NULL);
> +    ASSERT (Vertical != NULL);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *Horizontal = &mDisplayModes[ModeNumber].Horizontal;
> +  *Vertical = &mDisplayModes[ModeNumber].Vertical;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/** Return bytes 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.
> +  * @retval EFI_INVALID_PARAMETER   Bpp is NULL.
> +**/
> +EFI_STATUS
> +LcdPlatformGetBpp (
> +  IN  CONST UINT32    ModeNumber,
> +  OUT LCD_BPP * CONST Bpp
> +  )
> +{
> +  if (ModeNumber >= mMaxMode || Bpp == NULL) {
> +    // Check valid ModeNumber and Bpp.
> +    ASSERT (ModeNumber < mMaxMode);
> +    ASSERT (Bpp != NULL);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *Bpp = LCD_BITS_PER_PIXEL_24;
> +
> +  return EFI_SUCCESS;
> +}
> diff --git 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c 
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> index 
> 5cd529750a3d2d3b0d381b58d875d378afaba2c2..87a7a26132e39b933ecd2a95e38b2ab869cd4079
>  100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> @@ -21,9 +21,10 @@
>  #include <ArmPlatform.h>
>  
>  #define FRAME_BUFFER_DESCRIPTOR ((FixedPcdGet64 
> (PcdArmLcdDdrFrameBufferBase) != 0) ? 1 : 0)
> +#define DP_BASE_DESCRIPTOR      ((FixedPcdGet64 (PcdArmMaliDpBase) != 0) ? 1 
> : 0)
>  
>  // Number of Virtual Memory Map Descriptors
> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 + FRAME_BUFFER_DESCRIPTOR)
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 + FRAME_BUFFER_DESCRIPTOR + 
> DP_BASE_DESCRIPTOR)
>  
>  // DDR attributes
>  #define DDR_ATTRIBUTES_CACHED   ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> @@ -159,6 +160,13 @@ ArmPlatformGetVirtualMemoryMap (
>    VirtualMemoryTable[Index].Attributes = 
> ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
>  #endif
>  
> +#if (FixedPcdGet64 (PcdArmMaliDpBase) != 0)
> +  // DP500/DP550/DP650 peripheral memory region
> +  VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64 
> (PcdArmMaliDpBase);
> +  VirtualMemoryTable[Index].VirtualBase = FixedPcdGet64 (PcdArmMaliDpBase);
> +  VirtualMemoryTable[Index].Length = FixedPcdGet32 
> (PcdArmMaliDpMemoryRegionLength);
> +  VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +#endif
>    // Map sparse memory region if present
>    if (HasSparseMemory) {
>      VirtualMemoryTable[++Index].PhysicalBase = SparseMemoryBase;
> diff --git a/ArmPlatformPkg/Drivers/ArmMaliDp/ArmMaliDp.c 
> b/ArmPlatformPkg/Drivers/ArmMaliDp/ArmMaliDp.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..ff7236371d27915d9f1e34162dce0954dde63a77
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/ArmMaliDp/ArmMaliDp.c
> @@ -0,0 +1,412 @@
> +/** @file  ArmMaliDp.c
> +
> +  ARM Mali DP 500/550/650 display controller driver
> +
> +  Copyright (c) 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
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  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 <Library/IoLib.h>
> +#include <Library/DebugLib.h>

Swap above two lines?

> +#include <Library/LcdPlatformLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Drivers/ArmMaliDp.h>
> +#include <Drivers/LcdHw.h>
> +
> +// CORE_ID of the MALI DP
> +STATIC UINT32 gDpDeviceId;
> +
> +/** Disable the graphics layer
> +  *
> +  * This is done by clearing the EN bit
> +  * of the LG_CONTROL register.
> +**/
> +STATIC
> +VOID
> +LayerGraphicsDisable (VOID)
> +{
> +  MmioAnd32 (DP_BASE + DP_DE_LG_CONTROL, ~DP_DE_LG_ENABLE);
> +}
> +
> +/** Enable the graphics layer
> +  *
> +  * This is done by setting the EN bit
> +  * of the LG_CONTROL register.
> +**/
> +STATIC
> +VOID
> +LayerGraphicsEnable (VOID)
> +{
> +  MmioOr32 (DP_BASE + DP_DE_LG_CONTROL, DP_DE_LG_ENABLE);
> +}
> +
> +/** Set the frame address of the graphics layer.
> +  *
> +  * @param  FrameBaseAddress        Address of the data buffer
> +  *                                 to be used as a frame buffer.
> +**/
> +STATIC
> +VOID
> +LayerGraphicsSetFrame (
> +  IN CONST EFI_PHYSICAL_ADDRESS FrameBaseAddress
> +  )
> +{
> +  // Disable the graphics layer.
> +  LayerGraphicsDisable ();
> +
> +  /* Set up memory address of the data buffer for graphics layer.
> +   * write lower bits of the address.
> +   */
> +  MmioWrite32 (
> +    DP_BASE + DP_DE_LG_PTR_LOW,
> +    DP_DE_LG_PTR_LOW_MASK & FrameBaseAddress
> +    );
> +
> +  // Write higher bits of the address.
> +  MmioWrite32 (
> +    DP_BASE + DP_DE_LG_PTR_HIGH,
> +    (UINT32)(FrameBaseAddress >> DP_DE_LG_PTR_HIGH_SHIFT)
> +    );
> +
> +  // Enable the graphics layer.
> +  LayerGraphicsEnable ();
> +}
> +
> +/** Configures various graphics layer characteristics.
> +  *
> +  * @param  HRes                 Horizontal resolution of the graphics layer.
> +  * @param  VRes                 Vertical resolution of the graphics layer.
> +**/
> +STATIC
> +VOID
> +LayerGraphicsConfig (
> +  IN CONST EFI_GRAPHICS_PIXEL_FORMAT UefiGfxPixelFormat,
> +  IN CONST UINT32 HRes,
> +  IN CONST UINT32 VRes
> +  )
> +{
> +  UINT32 PixelFormat;
> +
> +  // Disable the graphics layer before configuring any settings.
> +  LayerGraphicsDisable ();
> +
> +  // Setup graphics layer size.
> +  MmioWrite32 (DP_BASE + DP_DE_LG_IN_SIZE, FRAME_IN_SIZE (HRes, VRes));
> +
> +  // Setup graphics layer composition size.
> +  MmioWrite32 (DP_BASE + DP_DE_LG_CMP_SIZE, FRAME_CMP_SIZE (HRes, VRes));
> +
> +  // Setup memory stride (total visible pixels on a line * 4 ).
> +  MmioWrite32 (DP_BASE + DP_DE_LG_H_STRIDE, (HRes * sizeof (UINT32)));
> +
> +  /* Set the format.
> +   *
> +   * In PixelBlueGreenRedReserved8BitPerColor format, byte 0 represents blue,
> +   * byte 1 represents green, and byte 2 represents red, and byte 3 is 
> reserved
> +   * which is equivalent to XRGB format of the DP500/DP550/DP650. Whereas
> +   * PixelRedGreenBlueReserved8BitPerColor is equivalent to XBGR of the
> +   * DP500/DP550/DP650.
> +   */
> +  if (UefiGfxPixelFormat == PixelBlueGreenRedReserved8BitPerColor) {
> +    PixelFormat = (gDpDeviceId == MALIDP_500) ? 
> DP_PIXEL_FORMAT_DP500_XRGB_8888
> +                     : DP_PIXEL_FORMAT_XRGB_8888;
> +  } else {
> +    PixelFormat = (gDpDeviceId == MALIDP_500) ? 
> DP_PIXEL_FORMAT_DP500_XBGR_8888
> +                     : DP_PIXEL_FORMAT_XBGR_8888;
> +  }
> +
> +  MmioWrite32 (DP_BASE + DP_DE_LG_FORMAT, PixelFormat);
> +
> +  // Enable graphics layer.
> +  LayerGraphicsEnable ();
> +}
> +
> +/** Configure timing information of the display.
> +  * @param IN Horizontal           Pointer to horizontal timing parameters.
> +  *                                (Resolution, Sync, Back porch, Front 
> porch)
> +  * @param IN Vertical             Pointer to vertical timing parameters.
> +  *                                (Resolution, Sync, Back porch, Front 
> porch)
> +**/
> +STATIC
> +VOID
> +SetDisplayEngineTiming (
> +  IN CONST SCAN_TIMINGS * CONST Horizontal,
> +  IN CONST SCAN_TIMINGS * CONST Vertical
> +  )
> +{
> +  UINTN RegHIntervals;
> +  UINTN RegVIntervals;
> +  UINTN RegSyncControl;
> +  UINTN RegHVActiveSize;
> +
> +  if (gDpDeviceId == MALIDP_500) {
> +    // MALI DP500 timing registers.
> +    RegHIntervals = DP_BASE + DP_DE_DP500_H_INTERVALS;
> +    RegVIntervals = DP_BASE + DP_DE_DP500_V_INTERVALS;
> +    RegSyncControl = DP_BASE + DP_DE_DP500_SYNC_CONTROL;
> +    RegHVActiveSize = DP_BASE + DP_DE_DP500_HV_ACTIVESIZE;
> +  } else {
> +    // MALI DP550/DP650 timing registers.
> +    RegHIntervals = DP_BASE + DP_DE_H_INTERVALS;
> +    RegVIntervals = DP_BASE + DP_DE_V_INTERVALS;
> +    RegSyncControl = DP_BASE + DP_DE_SYNC_CONTROL;
> +    RegHVActiveSize = DP_BASE + DP_DE_HV_ACTIVESIZE;
> +  }
> +
> +  // Horizontal back porch and front porch.
> +  MmioWrite32 (
> +    RegHIntervals,
> +    H_INTERVALS (Horizontal->FrontPorch, Horizontal->BackPorch)
> +    );
> +
> +  // Vertical back porch and front porch.
> +  MmioWrite32 (
> +    RegVIntervals,
> +    V_INTERVALS (Vertical->FrontPorch, Vertical->BackPorch)
> +    );
> +
> +  // Sync control, Horizontal and Vertical sync.
> +  MmioWrite32 (
> +    RegSyncControl,
> +    SYNC_WIDTH (Horizontal->Sync, Vertical->Sync)
> +    );
> +
> +  // Set up Horizontal and Vertical area size.
> +  MmioWrite32 (
> +    RegHVActiveSize,
> +    HV_ACTIVE (Horizontal->Resolution, Vertical->Resolution)
> +    );
> +}
> +
> +/** Return CORE_ID of the ARM Mali DP.
> +  *
> +  * @retval 0                      No Mali DP found.
> +  * @retval 0x500                  Mali DP core id for DP500.
> +  * @retval 0x550                  Mali DP core id for DP550.
> +  * @retval 0x650                  Mali DP core id for DP650.
> +**/
> +STATIC
> +UINT32
> +ArmMaliDpGetCoreId (
> +  )
> +{
> +  UINT32 DpCoreId;
> +
> +  /* First check for DP500 as register offset for DP550/DP650 CORE_ID
> +   * is beyond 3K/4K register space of the DP500.
> +   */
> +  DpCoreId = MmioRead32 (DP_BASE + DP_DE_DP500_CORE_ID);
> +  DpCoreId >>= DP_DE_DP500_CORE_ID_SHIFT;
> +
> +  if (DpCoreId == MALIDP_500) {
> +    return DpCoreId;
> +  }
> +
> +  // Check for DP550 or DP650.
> +  DpCoreId = MmioRead32 (DP_BASE + DP_DC_CORE_ID);
> +  DpCoreId >>= DP_DC_CORE_ID_SHIFT;
> +
> +  if (DpCoreId == MALIDP_550
> +    || DpCoreId == MALIDP_650) {
> +    return DpCoreId;
> +  }
> +
> +  return 0;
> +}
> +
> +/** Check for presence of MALI.
> +  *
> +  * This function returns success if the platform implements
> +  * DP500/DP550/DP650 ARM Mali display processor.
> +  *
> +  * @retval EFI_SUCCESS            Platform implements DP500/DP550/DP650.
> +  * @retval EFI_NOT_FOUND          DP500/DP550/DP650 display processor not
> +  *                                found on the platform.
> +**/
> +EFI_STATUS
> +LcdIdentify (VOID)
> +{
> +  UINT32 DpCoreId;
> +
> +  DEBUG ((DEBUG_WARN,
> +    "Probing ARM Mali DP500/DP550/DP650 at base address 0x%p\n",
> +    DP_BASE
> +    ));
> +
> +#if (DP_BASE == 0)
> +#error "ARM Mali DP peripheral base address is invalid\n"

No quotes and \n on a #error.
Also, would it not make sense to place this one before its first use
in this function?

> +#endif
> +
> +  DpCoreId = ArmMaliDpGetCoreId ();
> +
> +  if (DpCoreId != 0) {
> +     DEBUG ((DEBUG_WARN, "Found ARM Mali DP %x\n", DpCoreId));
> +     return EFI_SUCCESS;
> +  }
> +
> +  DEBUG ((DEBUG_WARN, "ARM Mali DP not found...\n"));
> +
> +  return EFI_NOT_FOUND;
> +}
> +
> +/** Initialize platform display.
> +  *
> +  * @param  FrameBaseAddress       Address of the frame buffer.
> +  *
> +  * @retval EFI_SUCCESS            Display initialization success.
> +  * @retval !(EFI_SUCCESS)         Display initialization failure.
> +**/
> +EFI_STATUS
> +LcdInitialize (
> +  IN CONST EFI_PHYSICAL_ADDRESS FrameBaseAddress
> +  )
> +{
> +  DEBUG ((DEBUG_WARN, "Frame buffer base address = %p\n", FrameBaseAddress));
> +
> +  gDpDeviceId = ArmMaliDpGetCoreId ();
> +  if (gDpDeviceId == 0) {
> +    DEBUG ((DEBUG_ERROR, "ARM Mali DP initialization fail,"
> +       "no ARM Mali DP present\n"));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  // We are using graphics layer of the Mali DP as a main frame buffer.
> +  LayerGraphicsSetFrame (FrameBaseAddress);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/** Set ARM Mali DP in cofiguration mode.
> +  *
> +  * The ARM Mali DP must be in the configuration mode for
> +  * configuration of the H_INTERVALS, V_INTERVALS, SYNC_CONTROL
> +  * and HV_ACTIVESIZE.
> +**/
> +STATIC
> +VOID
> +SetConfigurationMode (VOID)
> +{
> + // Request configuration Mode.
> + if (gDpDeviceId == MALIDP_500) {
> +   MmioOr32 (DP_BASE + DP_DE_DP500_CONTROL, DP_DE_DP500_CONTROL_CONFIG_REQ);
> + } else {
> +   MmioOr32 (DP_BASE + DP_DC_CONTROL, DP_DC_CONTROL_CM_ACTIVE);
> + }
> +}
> +
> +/** Set ARM Mali DP in normal mode.
> +  *
> +  * Normal mode is the main operating mode of the display processor
> +  * in which display layer data is fetched from frame buffer and
> +  * displayed.
> +**/
> +STATIC
> +VOID
> +SetNormalMode (VOID)
> +{
> + // Disable configuration Mode.
> + if (gDpDeviceId == MALIDP_500) {
> +   MmioAnd32 (DP_BASE + DP_DE_DP500_CONTROL, 
> ~DP_DE_DP500_CONTROL_CONFIG_REQ);
> + } else {
> +   MmioAnd32 (DP_BASE + DP_DC_CONTROL, ~DP_DC_CONTROL_CM_ACTIVE );

There is a stray space character before close parentheses copied into
the else branch in this and the next function.

> + }
> +}
> +
> +/** Set the global configuration valid flag.
> +  *
> +  * Any new configuration parameters written to the display engine are not
> +  * activated until the global configuration valid flag is set in the
> +  * CONFIG_VALID register.
> +**/
> +STATIC
> +VOID
> +SetConfigValid (VOID)
> +{
> +  if (gDpDeviceId == MALIDP_500) {
> +    MmioOr32 (DP_BASE + DP_DP500_CONFIG_VALID, DP_DC_CONFIG_VALID);
> +  } else {
> +    MmioOr32 (DP_BASE + DP_DC_CONFIG_VALID, DP_DC_CONFIG_VALID );

Here too.

/
    Leif

> +  }
> +}
> +
> +/** 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.
> +  * @retval EFI_DEVICE_ERROR       If mode does not support more
> +  *                                than 24 bytes per pixel format.
> +**/
> +EFI_STATUS
> +LcdSetMode (
> +  IN CONST UINT32  ModeNumber
> +  )
> +{
> +  EFI_STATUS          Status;
> +  CONST SCAN_TIMINGS  *Horizontal;
> +  CONST SCAN_TIMINGS  *Vertical;
> +
> +  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  ModeInfo;
> +
> +  // Get the display mode timings and other relevant information.
> +  Status = LcdPlatformGetTimings (
> +             ModeNumber,
> +             &Horizontal,
> +             &Vertical
> +             );
> +
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    return Status;
> +  }
> +  ASSERT (Horizontal != NULL);
> +  ASSERT (Vertical != NULL);
> +
> +  // Get the pixel format information.
> +  Status = LcdPlatformQueryMode (ModeNumber, &ModeInfo);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    return Status;
> +  }
> +
> +  // Request configuration mode.
> +  SetConfigurationMode ();
> +
> +  // Configure the graphics layer.
> +  LayerGraphicsConfig (
> +    ModeInfo.PixelFormat,
> +    Horizontal->Resolution,
> +    Vertical->Resolution
> +    );
> +
> +  // Set the display engine timings.
> +  SetDisplayEngineTiming (Horizontal, Vertical);
> +
> +  // After configuration, set Mali DP in normal mode.
> +  SetNormalMode ();
> +
> +  /* Any parameters written to the display engine are not activated until
> +   * CONFIG_VALID is set.
> +   */
> +  SetConfigValid ();
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  * This function de-initializes the display.
> +**/
> +VOID
> +LcdShutdown (VOID)
> +{
> +  // Disable graphics layer.
> +  LayerGraphicsDisable ();
> +}
> -- 
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to