Add the declaration and include file for the new LcdHwLib library class,
which will allow us to abstract away from platform  variations in the
LCD graphics output driver.

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>
[ardb: add NULL implementation as well and add it to ArmPlatformPkg.dsc]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/ArmPlatformPkg.dec                    |  1 +
 ArmPlatformPkg/ArmPlatformPkg.dsc                    |  1 +
 ArmPlatformPkg/Include/Library/LcdHwLib.h            | 68 ++++++++++++++++++
 ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c   | 75 ++++++++++++++++++++
 ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf | 28 ++++++++
 5 files changed, 173 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 6a7bbc02d011..b33b6e630d85 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -33,6 +33,7 @@ [Includes.common]
 
 [LibraryClasses]
   ArmPlatformLib|Include/Library/ArmPlatformLib.h
+  LcdHwLib|Include/Library/LcdHwLib.h
   LcdPlatformLib|Include/Library/LcdPlatformLib.h
   NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h
   PL011UartLib|Include/Library/PL011UartLib.h
diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc b/ArmPlatformPkg/ArmPlatformPkg.dsc
index c3a8c257cb02..9dd64b472acf 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dsc
+++ b/ArmPlatformPkg/ArmPlatformPkg.dsc
@@ -101,6 +101,7 @@ [Components.common]
 
   ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
   ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
+  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf
   ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.inf
   ArmPlatformPkg/Library/NorFlashPlatformNullLib/NorFlashPlatformNullLib.inf
   ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
diff --git a/ArmPlatformPkg/Include/Library/LcdHwLib.h b/ArmPlatformPkg/Include/Library/LcdHwLib.h
new file mode 100644
index 000000000000..0c0480862aea
--- /dev/null
+++ b/ArmPlatformPkg/Include/Library/LcdHwLib.h
@@ -0,0 +1,68 @@
+/** @file LcdHwLib.h
+
+  This file contains interface functions for LcdHwLib of ArmPlatformPkg
+
+  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 LCD_HW_LIB_H_
+#define LCD_HW_LIB_H_
+
+#include <Uefi/UefiBaseType.h>
+
+/**
+  Check for presence of display
+
+  @retval EFI_SUCCESS            Platform implements display.
+  @retval EFI_NOT_FOUND          Display not found on the platform.
+
+**/
+EFI_STATUS
+LcdIdentify (
+  VOID
+  );
+
+/**
+  Initialize display.
+
+  @param  FrameBaseAddress       Address of the frame buffer.
+  @retval EFI_SUCCESS            Display initialization success.
+  @retval !(EFI_SUCCESS)         Display initialization failure.
+
+**/
+EFI_STATUS
+LcdInitialize (
+  EFI_PHYSICAL_ADDRESS  FrameBaseAddress
+  );
+
+/**
+  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
+  );
+
+/**
+  De-initializes the display.
+**/
+VOID
+LcdShutdown (
+  VOID
+  );
+
+#endif /* LCD_HW_LIB_H_ */
diff --git a/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
new file mode 100644
index 000000000000..2d31b5183c95
--- /dev/null
+++ b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
@@ -0,0 +1,75 @@
+/** @file
+
+  Copyright (c) 2017, Linaro, 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 <Base.h>
+#include <Uefi/UefiBaseType.h>
+#include <Library/DebugLib.h>
+#include <Library/LcdPlatformLib.h>
+
+/**
+  Check for presence of display
+  
+  @retval EFI_SUCCESS            Platform implements display.
+  @retval EFI_NOT_FOUND          Display not found on the platform.
+
+**/
+EFI_STATUS
+LcdIdentify (
+  VOID
+  )
+{
+  return EFI_SUCCESS;
+}
+
+/**
+  Initialize display.
+
+  @param  FrameBaseAddress       Address of the frame buffer.
+  @retval EFI_SUCCESS            Display initialization success.
+  @retval !(EFI_SUCCESS)         Display initialization failure.
+
+**/
+EFI_STATUS
+LcdInitialize (
+  EFI_PHYSICAL_ADDRESS  FrameBaseAddress
+  )
+{
+  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
+  )
+{
+  return EFI_SUCCESS;
+}
+
+/**
+  De-initializes the display.
+**/
+VOID
+LcdShutdown (
+  VOID
+  )
+{
+}
diff --git a/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf
new file mode 100644
index 000000000000..d556bed65548
--- /dev/null
+++ b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf
@@ -0,0 +1,28 @@
+#/** @file
+#
+#  Copyright (c) 2017, Linaro, 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.
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = LcdHwNullLib
+  FILE_GUID                      = bb1fde98-1de2-410e-8850-fdcb8e67ebc0
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = LcdHwLib
+
+[Sources]
+  LcdHwNullLib.c
+
+[Packages]
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  MdePkg/MdePkg.dec
-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On Fri, Dec 08, 2017 at 05:31:24PM +0000, Ard Biesheuvel wrote:
> Add the declaration and include file for the new LcdHwLib library class,
> which will allow us to abstract away from platform  variations in the
> LCD graphics output driver.
> 
> 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>
> [ardb: add NULL implementation as well and add it to ArmPlatformPkg.dsc]
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPlatformPkg/ArmPlatformPkg.dec                    |  1 +
>  ArmPlatformPkg/ArmPlatformPkg.dsc                    |  1 +
>  ArmPlatformPkg/Include/Library/LcdHwLib.h            | 68 ++++++++++++++++++
>  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c   | 75 ++++++++++++++++++++
>  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf | 28 ++++++++
>  5 files changed, 173 insertions(+)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 6a7bbc02d011..b33b6e630d85 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -33,6 +33,7 @@ [Includes.common]
>  
>  [LibraryClasses]
>    ArmPlatformLib|Include/Library/ArmPlatformLib.h
> +  LcdHwLib|Include/Library/LcdHwLib.h
>    LcdPlatformLib|Include/Library/LcdPlatformLib.h
>    NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h
>    PL011UartLib|Include/Library/PL011UartLib.h
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc b/ArmPlatformPkg/ArmPlatformPkg.dsc
> index c3a8c257cb02..9dd64b472acf 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dsc
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc
> @@ -101,6 +101,7 @@ [Components.common]
>  
>    ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
>    ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
> +  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf
>    ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.inf
>    ArmPlatformPkg/Library/NorFlashPlatformNullLib/NorFlashPlatformNullLib.inf
>    ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> diff --git a/ArmPlatformPkg/Include/Library/LcdHwLib.h b/ArmPlatformPkg/Include/Library/LcdHwLib.h
> new file mode 100644
> index 000000000000..0c0480862aea
> --- /dev/null
> +++ b/ArmPlatformPkg/Include/Library/LcdHwLib.h
> @@ -0,0 +1,68 @@
> +/** @file LcdHwLib.h
> +
> +  This file contains interface functions for LcdHwLib of ArmPlatformPkg
> +
> +  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 LCD_HW_LIB_H_
> +#define LCD_HW_LIB_H_
> +
> +#include <Uefi/UefiBaseType.h>
> +
> +/**
> +  Check for presence of display
> +
> +  @retval EFI_SUCCESS            Platform implements display.
> +  @retval EFI_NOT_FOUND          Display not found on the platform.
> +
> +**/
> +EFI_STATUS
> +LcdIdentify (
> +  VOID
> +  );
> +
> +/**
> +  Initialize display.
> +
> +  @param  FrameBaseAddress       Address of the frame buffer.
> +  @retval EFI_SUCCESS            Display initialization success.
> +  @retval !(EFI_SUCCESS)         Display initialization failure.
> +
> +**/
> +EFI_STATUS
> +LcdInitialize (
> +  EFI_PHYSICAL_ADDRESS  FrameBaseAddress
> +  );
> +
> +/**
> +  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
> +  );
> +
> +/**
> +  De-initializes the display.
> +**/
> +VOID
> +LcdShutdown (
> +  VOID
> +  );
> +
> +#endif /* LCD_HW_LIB_H_ */
> diff --git a/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
> new file mode 100644
> index 000000000000..2d31b5183c95
> --- /dev/null
> +++ b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
> @@ -0,0 +1,75 @@
> +/** @file
> +
> +  Copyright (c) 2017, Linaro, 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 <Base.h>
> +#include <Uefi/UefiBaseType.h>
> +#include <Library/DebugLib.h>
> +#include <Library/LcdPlatformLib.h>
> +
> +/**
> +  Check for presence of display
> +  
> +  @retval EFI_SUCCESS            Platform implements display.
> +  @retval EFI_NOT_FOUND          Display not found on the platform.
> +
> +**/
> +EFI_STATUS
> +LcdIdentify (
> +  VOID
> +  )
> +{

Bikeshedding:
Since this Null implementation can never fill any functionality other
than letting build complete without platform-specific glue - would it
not be more expected for it to ASSERT and/or return failure
everywhere?

/
    Leif

> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Initialize display.
> +
> +  @param  FrameBaseAddress       Address of the frame buffer.
> +  @retval EFI_SUCCESS            Display initialization success.
> +  @retval !(EFI_SUCCESS)         Display initialization failure.
> +
> +**/
> +EFI_STATUS
> +LcdInitialize (
> +  EFI_PHYSICAL_ADDRESS  FrameBaseAddress
> +  )
> +{
> +  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
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  De-initializes the display.
> +**/
> +VOID
> +LcdShutdown (
> +  VOID
> +  )
> +{
> +}
> diff --git a/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf
> new file mode 100644
> index 000000000000..d556bed65548
> --- /dev/null
> +++ b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf
> @@ -0,0 +1,28 @@
> +#/** @file
> +#
> +#  Copyright (c) 2017, Linaro, 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.
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = LcdHwNullLib
> +  FILE_GUID                      = bb1fde98-1de2-410e-8850-fdcb8e67ebc0
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = LcdHwLib
> +
> +[Sources]
> +  LcdHwNullLib.c
> +
> +[Packages]
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  MdePkg/MdePkg.dec
> -- 
> 2.11.0
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 11 December 2017 at 17:32, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Fri, Dec 08, 2017 at 05:31:24PM +0000, Ard Biesheuvel wrote:
>> Add the declaration and include file for the new LcdHwLib library class,
>> which will allow us to abstract away from platform  variations in the
>> LCD graphics output driver.
>>
>> 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>
>> [ardb: add NULL implementation as well and add it to ArmPlatformPkg.dsc]
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmPlatformPkg/ArmPlatformPkg.dec                    |  1 +
>>  ArmPlatformPkg/ArmPlatformPkg.dsc                    |  1 +
>>  ArmPlatformPkg/Include/Library/LcdHwLib.h            | 68 ++++++++++++++++++
>>  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c   | 75 ++++++++++++++++++++
>>  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf | 28 ++++++++
>>  5 files changed, 173 insertions(+)
>>
>> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
>> index 6a7bbc02d011..b33b6e630d85 100644
>> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
>> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
>> @@ -33,6 +33,7 @@ [Includes.common]
>>
>>  [LibraryClasses]
>>    ArmPlatformLib|Include/Library/ArmPlatformLib.h
>> +  LcdHwLib|Include/Library/LcdHwLib.h
>>    LcdPlatformLib|Include/Library/LcdPlatformLib.h
>>    NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h
>>    PL011UartLib|Include/Library/PL011UartLib.h
>> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc b/ArmPlatformPkg/ArmPlatformPkg.dsc
>> index c3a8c257cb02..9dd64b472acf 100644
>> --- a/ArmPlatformPkg/ArmPlatformPkg.dsc
>> +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc
>> @@ -101,6 +101,7 @@ [Components.common]
>>
>>    ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
>>    ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
>> +  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf
>>    ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.inf
>>    ArmPlatformPkg/Library/NorFlashPlatformNullLib/NorFlashPlatformNullLib.inf
>>    ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>> diff --git a/ArmPlatformPkg/Include/Library/LcdHwLib.h b/ArmPlatformPkg/Include/Library/LcdHwLib.h
>> new file mode 100644
>> index 000000000000..0c0480862aea
>> --- /dev/null
>> +++ b/ArmPlatformPkg/Include/Library/LcdHwLib.h
>> @@ -0,0 +1,68 @@
>> +/** @file LcdHwLib.h
>> +
>> +  This file contains interface functions for LcdHwLib of ArmPlatformPkg
>> +
>> +  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 LCD_HW_LIB_H_
>> +#define LCD_HW_LIB_H_
>> +
>> +#include <Uefi/UefiBaseType.h>
>> +
>> +/**
>> +  Check for presence of display
>> +
>> +  @retval EFI_SUCCESS            Platform implements display.
>> +  @retval EFI_NOT_FOUND          Display not found on the platform.
>> +
>> +**/
>> +EFI_STATUS
>> +LcdIdentify (
>> +  VOID
>> +  );
>> +
>> +/**
>> +  Initialize display.
>> +
>> +  @param  FrameBaseAddress       Address of the frame buffer.
>> +  @retval EFI_SUCCESS            Display initialization success.
>> +  @retval !(EFI_SUCCESS)         Display initialization failure.
>> +
>> +**/
>> +EFI_STATUS
>> +LcdInitialize (
>> +  EFI_PHYSICAL_ADDRESS  FrameBaseAddress
>> +  );
>> +
>> +/**
>> +  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
>> +  );
>> +
>> +/**
>> +  De-initializes the display.
>> +**/
>> +VOID
>> +LcdShutdown (
>> +  VOID
>> +  );
>> +
>> +#endif /* LCD_HW_LIB_H_ */
>> diff --git a/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
>> new file mode 100644
>> index 000000000000..2d31b5183c95
>> --- /dev/null
>> +++ b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
>> @@ -0,0 +1,75 @@
>> +/** @file
>> +
>> +  Copyright (c) 2017, Linaro, 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 <Base.h>
>> +#include <Uefi/UefiBaseType.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/LcdPlatformLib.h>
>> +
>> +/**
>> +  Check for presence of display
>> +
>> +  @retval EFI_SUCCESS            Platform implements display.
>> +  @retval EFI_NOT_FOUND          Display not found on the platform.
>> +
>> +**/
>> +EFI_STATUS
>> +LcdIdentify (
>> +  VOID
>> +  )
>> +{
>
> Bikeshedding:
> Since this Null implementation can never fill any functionality other
> than letting build complete without platform-specific glue - would it
> not be more expected for it to ASSERT and/or return failure
> everywhere?
>

I think it makes sense to ASSERT on Null library member that take OUT
parameters, because you can never return anything meaningful. In this
case, we can just pretend that the Null LcdHwLib does not require
initialization, is always present and only supports a single mode, and
the driver could theoretically still work. Highly unlikely to be
useful for anything but still.

On ArmVirtQemu, we switched to the Null ArmPlatformLib implementation
because we need to fulfil the dependency to be able to use PrePeiCore,
but we no longer have any code to put in there.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On Mon, Dec 11, 2017 at 05:56:37PM +0000, Ard Biesheuvel wrote:
> On 11 December 2017 at 17:32, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Fri, Dec 08, 2017 at 05:31:24PM +0000, Ard Biesheuvel wrote:
> >> Add the declaration and include file for the new LcdHwLib library class,
> >> which will allow us to abstract away from platform  variations in the
> >> LCD graphics output driver.
> >>
> >> 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>
> >> [ardb: add NULL implementation as well and add it to ArmPlatformPkg.dsc]
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  ArmPlatformPkg/ArmPlatformPkg.dec                    |  1 +
> >>  ArmPlatformPkg/ArmPlatformPkg.dsc                    |  1 +
> >>  ArmPlatformPkg/Include/Library/LcdHwLib.h            | 68 ++++++++++++++++++
> >>  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c   | 75 ++++++++++++++++++++
> >>  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf | 28 ++++++++
> >>  5 files changed, 173 insertions(+)
> >>
> >> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
> >> index 6a7bbc02d011..b33b6e630d85 100644
> >> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> >> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> >> @@ -33,6 +33,7 @@ [Includes.common]
> >>
> >>  [LibraryClasses]
> >>    ArmPlatformLib|Include/Library/ArmPlatformLib.h
> >> +  LcdHwLib|Include/Library/LcdHwLib.h
> >>    LcdPlatformLib|Include/Library/LcdPlatformLib.h
> >>    NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h
> >>    PL011UartLib|Include/Library/PL011UartLib.h
> >> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc b/ArmPlatformPkg/ArmPlatformPkg.dsc
> >> index c3a8c257cb02..9dd64b472acf 100644
> >> --- a/ArmPlatformPkg/ArmPlatformPkg.dsc
> >> +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc
> >> @@ -101,6 +101,7 @@ [Components.common]
> >>
> >>    ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
> >>    ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
> >> +  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf
> >>    ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.inf
> >>    ArmPlatformPkg/Library/NorFlashPlatformNullLib/NorFlashPlatformNullLib.inf
> >>    ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> >> diff --git a/ArmPlatformPkg/Include/Library/LcdHwLib.h b/ArmPlatformPkg/Include/Library/LcdHwLib.h
> >> new file mode 100644
> >> index 000000000000..0c0480862aea
> >> --- /dev/null
> >> +++ b/ArmPlatformPkg/Include/Library/LcdHwLib.h
> >> @@ -0,0 +1,68 @@
> >> +/** @file LcdHwLib.h
> >> +
> >> +  This file contains interface functions for LcdHwLib of ArmPlatformPkg
> >> +
> >> +  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 LCD_HW_LIB_H_
> >> +#define LCD_HW_LIB_H_
> >> +
> >> +#include <Uefi/UefiBaseType.h>
> >> +
> >> +/**
> >> +  Check for presence of display
> >> +
> >> +  @retval EFI_SUCCESS            Platform implements display.
> >> +  @retval EFI_NOT_FOUND          Display not found on the platform.
> >> +
> >> +**/
> >> +EFI_STATUS
> >> +LcdIdentify (
> >> +  VOID
> >> +  );
> >> +
> >> +/**
> >> +  Initialize display.
> >> +
> >> +  @param  FrameBaseAddress       Address of the frame buffer.
> >> +  @retval EFI_SUCCESS            Display initialization success.
> >> +  @retval !(EFI_SUCCESS)         Display initialization failure.
> >> +
> >> +**/
> >> +EFI_STATUS
> >> +LcdInitialize (
> >> +  EFI_PHYSICAL_ADDRESS  FrameBaseAddress
> >> +  );
> >> +
> >> +/**
> >> +  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
> >> +  );
> >> +
> >> +/**
> >> +  De-initializes the display.
> >> +**/
> >> +VOID
> >> +LcdShutdown (
> >> +  VOID
> >> +  );
> >> +
> >> +#endif /* LCD_HW_LIB_H_ */
> >> diff --git a/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
> >> new file mode 100644
> >> index 000000000000..2d31b5183c95
> >> --- /dev/null
> >> +++ b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
> >> @@ -0,0 +1,75 @@
> >> +/** @file
> >> +
> >> +  Copyright (c) 2017, Linaro, 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 <Base.h>
> >> +#include <Uefi/UefiBaseType.h>
> >> +#include <Library/DebugLib.h>
> >> +#include <Library/LcdPlatformLib.h>
> >> +
> >> +/**
> >> +  Check for presence of display
> >> +
> >> +  @retval EFI_SUCCESS            Platform implements display.
> >> +  @retval EFI_NOT_FOUND          Display not found on the platform.
> >> +
> >> +**/
> >> +EFI_STATUS
> >> +LcdIdentify (
> >> +  VOID
> >> +  )
> >> +{
> >
> > Bikeshedding:
> > Since this Null implementation can never fill any functionality other
> > than letting build complete without platform-specific glue - would it
> > not be more expected for it to ASSERT and/or return failure
> > everywhere?
> 
> I think it makes sense to ASSERT on Null library member that take OUT
> parameters, because you can never return anything meaningful. In this
> case, we can just pretend that the Null LcdHwLib does not require
> initialization, is always present and only supports a single mode, and
> the driver could theoretically still work. Highly unlikely to be
> useful for anything but still.

I don't necessarily agree, but I also don't consider this very
important.

I guess my internal model of LibNulls is that if they can be
paractically used as do-nothing variants, they return OK on anything,
whereas if practical use requires further implementation, they ASSERT
and/or return error. This is not necessarily a correct world view.

For the ArmPlatformPkg resolution, you don't care about
runtime behaviour - it will never be executed.

> On ArmVirtQemu, we switched to the Null ArmPlatformLib implementation
> because we need to fulfil the dependency to be able to use PrePeiCore,
> but we no longer have any code to put in there.

Sure - but in this case you would only use this library if you
actually had a display driver to link against.

Anyway:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 12 December 2017 at 17:12, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Mon, Dec 11, 2017 at 05:56:37PM +0000, Ard Biesheuvel wrote:
>> On 11 December 2017 at 17:32, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > On Fri, Dec 08, 2017 at 05:31:24PM +0000, Ard Biesheuvel wrote:
>> >> Add the declaration and include file for the new LcdHwLib library class,
>> >> which will allow us to abstract away from platform  variations in the
>> >> LCD graphics output driver.
>> >>
>> >> 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>
>> >> [ardb: add NULL implementation as well and add it to ArmPlatformPkg.dsc]
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >>  ArmPlatformPkg/ArmPlatformPkg.dec                    |  1 +
>> >>  ArmPlatformPkg/ArmPlatformPkg.dsc                    |  1 +
>> >>  ArmPlatformPkg/Include/Library/LcdHwLib.h            | 68 ++++++++++++++++++
>> >>  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c   | 75 ++++++++++++++++++++
>> >>  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf | 28 ++++++++
>> >>  5 files changed, 173 insertions(+)
>> >>
>> >> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
>> >> index 6a7bbc02d011..b33b6e630d85 100644
>> >> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
>> >> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
>> >> @@ -33,6 +33,7 @@ [Includes.common]
>> >>
>> >>  [LibraryClasses]
>> >>    ArmPlatformLib|Include/Library/ArmPlatformLib.h
>> >> +  LcdHwLib|Include/Library/LcdHwLib.h
>> >>    LcdPlatformLib|Include/Library/LcdPlatformLib.h
>> >>    NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h
>> >>    PL011UartLib|Include/Library/PL011UartLib.h
>> >> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc b/ArmPlatformPkg/ArmPlatformPkg.dsc
>> >> index c3a8c257cb02..9dd64b472acf 100644
>> >> --- a/ArmPlatformPkg/ArmPlatformPkg.dsc
>> >> +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc
>> >> @@ -101,6 +101,7 @@ [Components.common]
>> >>
>> >>    ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
>> >>    ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
>> >> +  ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.inf
>> >>    ArmPlatformPkg/Library/LcdPlatformNullLib/LcdPlatformNullLib.inf
>> >>    ArmPlatformPkg/Library/NorFlashPlatformNullLib/NorFlashPlatformNullLib.inf
>> >>    ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>> >> diff --git a/ArmPlatformPkg/Include/Library/LcdHwLib.h b/ArmPlatformPkg/Include/Library/LcdHwLib.h
>> >> new file mode 100644
>> >> index 000000000000..0c0480862aea
>> >> --- /dev/null
>> >> +++ b/ArmPlatformPkg/Include/Library/LcdHwLib.h
>> >> @@ -0,0 +1,68 @@
>> >> +/** @file LcdHwLib.h
>> >> +
>> >> +  This file contains interface functions for LcdHwLib of ArmPlatformPkg
>> >> +
>> >> +  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 LCD_HW_LIB_H_
>> >> +#define LCD_HW_LIB_H_
>> >> +
>> >> +#include <Uefi/UefiBaseType.h>
>> >> +
>> >> +/**
>> >> +  Check for presence of display
>> >> +
>> >> +  @retval EFI_SUCCESS            Platform implements display.
>> >> +  @retval EFI_NOT_FOUND          Display not found on the platform.
>> >> +
>> >> +**/
>> >> +EFI_STATUS
>> >> +LcdIdentify (
>> >> +  VOID
>> >> +  );
>> >> +
>> >> +/**
>> >> +  Initialize display.
>> >> +
>> >> +  @param  FrameBaseAddress       Address of the frame buffer.
>> >> +  @retval EFI_SUCCESS            Display initialization success.
>> >> +  @retval !(EFI_SUCCESS)         Display initialization failure.
>> >> +
>> >> +**/
>> >> +EFI_STATUS
>> >> +LcdInitialize (
>> >> +  EFI_PHYSICAL_ADDRESS  FrameBaseAddress
>> >> +  );
>> >> +
>> >> +/**
>> >> +  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
>> >> +  );
>> >> +
>> >> +/**
>> >> +  De-initializes the display.
>> >> +**/
>> >> +VOID
>> >> +LcdShutdown (
>> >> +  VOID
>> >> +  );
>> >> +
>> >> +#endif /* LCD_HW_LIB_H_ */
>> >> diff --git a/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
>> >> new file mode 100644
>> >> index 000000000000..2d31b5183c95
>> >> --- /dev/null
>> >> +++ b/ArmPlatformPkg/Library/LcdHwNullLib/LcdHwNullLib.c
>> >> @@ -0,0 +1,75 @@
>> >> +/** @file
>> >> +
>> >> +  Copyright (c) 2017, Linaro, 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 <Base.h>
>> >> +#include <Uefi/UefiBaseType.h>
>> >> +#include <Library/DebugLib.h>
>> >> +#include <Library/LcdPlatformLib.h>
>> >> +
>> >> +/**
>> >> +  Check for presence of display
>> >> +
>> >> +  @retval EFI_SUCCESS            Platform implements display.
>> >> +  @retval EFI_NOT_FOUND          Display not found on the platform.
>> >> +
>> >> +**/
>> >> +EFI_STATUS
>> >> +LcdIdentify (
>> >> +  VOID
>> >> +  )
>> >> +{
>> >
>> > Bikeshedding:
>> > Since this Null implementation can never fill any functionality other
>> > than letting build complete without platform-specific glue - would it
>> > not be more expected for it to ASSERT and/or return failure
>> > everywhere?
>>
>> I think it makes sense to ASSERT on Null library member that take OUT
>> parameters, because you can never return anything meaningful. In this
>> case, we can just pretend that the Null LcdHwLib does not require
>> initialization, is always present and only supports a single mode, and
>> the driver could theoretically still work. Highly unlikely to be
>> useful for anything but still.
>
> I don't necessarily agree, but I also don't consider this very
> important.
>
> I guess my internal model of LibNulls is that if they can be
> paractically used as do-nothing variants, they return OK on anything,
> whereas if practical use requires further implementation, they ASSERT
> and/or return error. This is not necessarily a correct world view.
>
> For the ArmPlatformPkg resolution, you don't care about
> runtime behaviour - it will never be executed.
>

Indeed. Which is essentially why it doesn't matter if we ASSERT () or not.

>> On ArmVirtQemu, we switched to the Null ArmPlatformLib implementation
>> because we need to fulfil the dependency to be able to use PrePeiCore,
>> but we no longer have any code to put in there.
>
> Sure - but in this case you would only use this library if you
> actually had a display driver to link against.
>

Yes.

> Anyway:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel