From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.20; helo=mga02.intel.com; envelope-from=michael.d.kinney@intel.com; receiver=edk2-devel@lists.01.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id AD767225264B1 for ; Wed, 4 Apr 2018 17:37:49 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Apr 2018 17:37:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,409,1517904000"; d="scan'208";a="29629449" Received: from orsmsx104.amr.corp.intel.com ([10.22.225.131]) by fmsmga007.fm.intel.com with ESMTP; 04 Apr 2018 17:37:47 -0700 Received: from orsmsx158.amr.corp.intel.com (10.22.240.20) by ORSMSX104.amr.corp.intel.com (10.22.225.131) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 4 Apr 2018 17:37:47 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.67]) by ORSMSX158.amr.corp.intel.com ([169.254.10.54]) with mapi id 14.03.0319.002; Wed, 4 Apr 2018 17:37:47 -0700 From: "Kinney, Michael D" To: "Yao, Jiewen" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: Sean Brogan , "Zeng, Star" , "Dong, Eric" , "Wei, David" , "Guo, Mang" , "Steele, Kelly" , Bret Barkelew Thread-Topic: [Patch 0/9] Add DisplayUpdateProgressLib for capsules Thread-Index: AQHTzHFsT0DbZlOZn068OQx5Rr1mN6PxUUkQ Date: Thu, 5 Apr 2018 00:37:46 +0000 Message-ID: References: <20180404202554.9568-1-michael.d.kinney@intel.com> <74D8A39837DF1E4DA445A8C0B3885C503AB376CC@shsmsx102.ccr.corp.intel.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503AB376CC@shsmsx102.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.22.254.139] MIME-Version: 1.0 Subject: Re: [Patch 0/9] Add DisplayUpdateProgressLib for capsules X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Apr 2018 00:37:49 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jiewen, Responses below. Mike > -----Original Message----- > From: Yao, Jiewen > Sent: Wednesday, April 4, 2018 5:03 PM > To: Kinney, Michael D ; > edk2-devel@lists.01.org > Cc: Sean Brogan ; Zeng, Star > ; Dong, Eric > ; Wei, David > ; Guo, Mang ; > Steele, Kelly > Subject: RE: [Patch 0/9] Add DisplayUpdateProgressLib > for capsules >=20 > Thanks Mike. > It is a good feature to add progress support. >=20 > Some thought below: > 1) for EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL > Do you think if we need add full support for > WatchdogTimer services? > Such as WatchdogCode, WatchdogData? >=20 > Or should we add a version field for the protocol for > future extension? >=20 A version field is a good idea. Let's keep it as simple as possible right now and only add new features based on real use cases. > 2) For EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL and > DisplayUpdateProgress(), we only add > EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION *Color. >=20 > Do the color means the foreground color or background > color? > I found you treat as background in TextLib. But it > seems complicated. > Can we add both to avoid calculation? >=20 It is foreground color. The text lib should use grey as background like the graphics lib. > That might be another reason to add version field for > EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL. >=20 > 3) According to lib naming conversion, we should use > DisplayUpdateProgressLibGraphic, and > DisplayUpdateProgressLibText. :-) >=20 Yes. We should make that change. > 4) For PerformFlashWriteWithProgress(), I found the > caller (system firmware update) handles Progress > (StartPercentage) and Progress (EndPercentage). >=20 > And the callee (Quark flash access lib) handles > Progress (StartPercentage) but not Progress > (EndPercentage). >=20 > That is a little weird to me. Can we add more detail > description in the PerformFlashWriteWithProgress() > function header to clarify who should handle > StartPercentage and EndPercentage ? The Progress() API passed into functions is OPTIONAL. Which means it may never get called by the next layer down. As a result, if the top level always wants the progress bar to be displayed correctly, it must always call for start before and end after. The Quark and Vlv2 versions of the lib should do the start and the end. That is a bug that I can fix. >=20 >=20 > Thank you > Yao Jiewen >=20 > > -----Original Message----- > > From: Kinney, Michael D > > Sent: Thursday, April 5, 2018 4:26 AM > > To: edk2-devel@lists.01.org > > Cc: Sean Brogan ; Zeng, > Star > > ; Dong, Eric > ; Yao, Jiewen > > ; Wei, David > ; Guo, Mang > > ; Steele, Kelly > > > Subject: [Patch 0/9] Add DisplayUpdateProgressLib for > capsules > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D801 > > > > Based on content from: > > > > > https://github.com/Microsoft/MS_UEFI/blob/share/MsCapsu > leSupport/MsCaps > > > uleUpdatePkg/Include/Library/DisplayUpdateProgressLib.h > > > https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsu > leSupport/MsCapsu > > leUpdatePkg/Library/DisplayUpdateProgressGraphicsLib > > > https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsu > leSupport/MsCapsu > > leUpdatePkg/Library/DisplayUpdateProgressTextLib > > > > Add DisplayUpdateProgressLib class along > implementations for both graphical > > (Graphics Output Protocol based) and text (Simple > Text Output Protocol based) > > consoles. Also add the EDK II Firmware Management > Progress Protocol that is > > an > > optional protocol that provides the progress bar > color and a watchdog timeout > > value thaty can be used when a firmware image is > updated in a firmware device. > > > > * Add progress support to DxeCapsuleLibFmp > > * Add progress support to SystemFirmwareUpdateDxe > > * Add progress support to PlatformFlashAccessLib > class and instances. > > * Reduce Print() calls during a firmware update. > > > > Cc: Sean Brogan > > Cc: Star Zeng > > Cc: Eric Dong > > Cc: Jiewen Yao > > Cc: David Wei > > Cc: Mang Guo > > Cc: Kelly Steele > > > > Signed-off-by: Michael D Kinney > > > Contributed-under: TianoCore Contribution Agreement > 1.1 > > > > Kinney, Michael D (3): > > QuarkPlatformPkg: Add DisplayUpdateProgressLib > mapping > > MdeModulePkg/DxeCapsuleLibFmp: Add progress bar > support > > SignedCapsulePkg/SystemFirmwareUpdateDxe: Use > progress API > > > > Michael D Kinney (6): > > MdeModulePkg: Add DisplayUpdateProgressLib class > > MdeModulePkg: Add DisplayUpdateProgressLib > instances > > Vlv2Tbl2DevicePkg: Add DisplayUpdateProgressLib > mapping > > SignedCapsulePkg/PlatformFlashAccessLib: Add > progress API > > Vlv2TbltDevicePkg/PlatformFlashAccessLib: Add > progress API > > QuarkPlatformPkg/PlatformFlashAccessLib: Add > progress API > > > > .../Include/Library/DisplayUpdateProgressLib.h | > 65 +++ > > .../Include/Protocol/FirmwareManagementProgress.h | > 50 +++ > > .../DisplayUpdateProgressGraphicsLib.c | > 475 > > +++++++++++++++++++++ > > .../DisplayUpdateProgressGraphicsLib.inf | > 60 +++ > > .../DisplayUpdateProgressGraphicsLib.uni | > 18 + > > .../DisplayUpdateProgressTextLib.c | > 142 ++++++ > > .../DisplayUpdateProgressTextLib.inf | > 53 +++ > > .../DisplayUpdateProgressTextLib.uni | > 18 + > > .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | > 47 +- > > .../Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf | > 8 +- > > .../DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | > 84 +++- > > .../DxeCapsuleLibFmp/DxeCapsuleProcessLibNull.c | > 21 +- > > .../DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf | > 7 +- > > MdeModulePkg/MdeModulePkg.dec | > 11 + > > MdeModulePkg/MdeModulePkg.dsc | > 3 + > > .../PlatformFlashAccessLibDxe.c | > 59 ++- > > QuarkPlatformPkg/Quark.dsc | > 1 + > > .../Include/Library/PlatformFlashAccessLib.h | > 33 +- > > .../PlatformFlashAccessLibNull.c | > 54 ++- > > .../SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c | > 92 ++-- > > .../PlatformFlashAccessLib.c | > 84 ++-- > > .../PlatformFlashAccessLib.inf | > 3 +- > > Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | > 1 + > > Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | > 1 + > > Vlv2TbltDevicePkg/PlatformPkgX64.dsc | > 1 + > > 25 files changed, 1285 insertions(+), 106 > deletions(-) > > create mode 100644 > > > MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h > > create mode 100644 > > > MdeModulePkg/Include/Protocol/FirmwareManagementProgres > s.h > > create mode 100644 > > > MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/D > isplayUpdateProg > > ressGraphicsLib.c > > create mode 100644 > > > MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/D > isplayUpdateProg > > ressGraphicsLib.inf > > create mode 100644 > > > MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/D > isplayUpdateProg > > ressGraphicsLib.uni > > create mode 100644 > > > MdeModulePkg/Library/DisplayUpdateProgressTextLib/Displ > ayUpdateProgressT > > extLib.c > > create mode 100644 > > > MdeModulePkg/Library/DisplayUpdateProgressTextLib/Displ > ayUpdateProgressT > > extLib.inf > > create mode 100644 > > > MdeModulePkg/Library/DisplayUpdateProgressTextLib/Displ > ayUpdateProgressT > > extLib.uni > > > > -- > > 2.14.2.windows.3