From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::22b; helo=mail-it0-x22b.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x22b.google.com (mail-it0-x22b.google.com [IPv6:2607:f8b0:4001:c0b::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0D17C2214E329 for ; Mon, 11 Dec 2017 09:52:01 -0800 (PST) Received: by mail-it0-x22b.google.com with SMTP id r6so17799085itr.3 for ; Mon, 11 Dec 2017 09:56:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=sx5FxLY3OaVQmzfnBeWGmGTfOAi9GhYDAV806rXpAVs=; b=KqE6mq2NSF2DMSn3/iJdYvBdjC6081yO7+Nvx6uJ2RJ6e/98zwO269tJe8oFGegIPP QAn7wKWz5O7tIKRfwUD37Ci27nku1sD70bULZm2VsbdnkSD5x5DQtn0Z3hYRNBS5zli5 /2hLdeXkJMdQ2fvUzsMxUZ2syP1Grs32S1Dyo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=sx5FxLY3OaVQmzfnBeWGmGTfOAi9GhYDAV806rXpAVs=; b=hUcvoVV5H56ZEw3oj8uVfaX0WXj3rNNUdnphcziovQYzGQa7+pmVddXIs5qzz8sD87 30Gs972PvXpDaF4Wopha0d6jFlM2gz6Oq5QicUfkwijlqZl1Cgusn5M6A+Opdt/6Aliy 2Nwm/6Oksv8pTh9oJ2By6UHz981zeGGbO4UN9tsDeazXLOObw2VJq/aGwNIizuR9QI7M GxBmKHprf32OUibMCjGQpCCpFlz6PAbsKNmfjuRBYueS1bLLmH9SyAo1zkUfayCc/9qK z1V1hkuUGMu5AjwC8Ohk+Kna/WhlsRGw3c4F++Q+GWUR5/5ySxy4+K38ika+z9Z0cJzo 8wWw== X-Gm-Message-State: AKGB3mLCjCDi9WIMqsm/z/gMNpFVfGelGVmqIsAgQx61CVXXQwWC/uPc 8b7SzaEhOO/fxMUyf9rabuPjH+VvS3s251/RozrmvA== X-Google-Smtp-Source: ACJfBotDxqke15ILpUXQXhuvYjcOMBBG4BgVmWJMGTVJh76n+CQLE8KTdb1kBOPUMsCrcX0go5dq62oUK0gM92pKDV8= X-Received: by 10.107.170.90 with SMTP id t87mr1521979ioe.253.1513014998387; Mon, 11 Dec 2017 09:56:38 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.16 with HTTP; Mon, 11 Dec 2017 09:56:37 -0800 (PST) In-Reply-To: <20171211173242.u5yol2egafnvciwx@bivouac.eciton.net> References: <20171208173128.28485-1-ard.biesheuvel@linaro.org> <20171208173128.28485-2-ard.biesheuvel@linaro.org> <20171211173242.u5yol2egafnvciwx@bivouac.eciton.net> From: Ard Biesheuvel Date: Mon, 11 Dec 2017 17:56:37 +0000 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Girish Pathak , Evan Lloyd Subject: Re: [PATCH v2 1/5] ArmPlatformPkg: introduce LcdHwLib library class X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Dec 2017 17:52:02 -0000 Content-Type: text/plain; charset="UTF-8" On 11 December 2017 at 17:32, Leif Lindholm 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 >> Signed-off-by: Evan Lloyd >> [ardb: add NULL implementation as well and add it to ArmPlatformPkg.dsc] >> Signed-off-by: Ard Biesheuvel >> --- >> 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.
>> + >> + 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 >> + >> +/** >> + 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 >> +#include >> +#include >> +#include >> + >> +/** >> + 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.