Thanks Mike. It is a good feature to add progress support. 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?
Or should we add a version field for the protocol for future extension? 2) For EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL and DisplayUpdateProgress(), we only add EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION *Color. 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? That might be another reason to add version field for EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL. 3) According to lib naming conversion, we should use DisplayUpdateProgressLibGraphic, and DisplayUpdateProgressLibText. :-) 4) For PerformFlashWriteWithProgress(), I found the caller (system firmware update) handles Progress (StartPercentage) and Progress (EndPercentage). And the callee (Quark flash access lib) handles Progress (StartPercentage) but not Progress (EndPercentage). 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 ? Thank you Yao Jiewen > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, April 5, 2018 4:26 AM > To: [email protected] > Cc: Sean Brogan <[email protected]>; Zeng, Star > <[email protected]>; Dong, Eric <[email protected]>; Yao, Jiewen > <[email protected]>; Wei, David <[email protected]>; Guo, Mang > <[email protected]>; Steele, Kelly <[email protected]> > Subject: [Patch 0/9] Add DisplayUpdateProgressLib for capsules > > https://bugzilla.tianocore.org/show_bug.cgi?id=801 > > Based on content from: > > https://github.com/Microsoft/MS_UEFI/blob/share/MsCapsuleSupport/MsCaps > uleUpdatePkg/Include/Library/DisplayUpdateProgressLib.h > https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport/MsCapsu > leUpdatePkg/Library/DisplayUpdateProgressGraphicsLib > https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport/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 <[email protected]> > Cc: Star Zeng <[email protected]> > Cc: Eric Dong <[email protected]> > Cc: Jiewen Yao <[email protected]> > Cc: David Wei <[email protected]> > Cc: Mang Guo <[email protected]> > Cc: Kelly Steele <[email protected]> > > Signed-off-by: Michael D Kinney <[email protected]> > 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/FirmwareManagementProgress.h > create mode 100644 > MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/DisplayUpdateProg > ressGraphicsLib.c > create mode 100644 > MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/DisplayUpdateProg > ressGraphicsLib.inf > create mode 100644 > MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/DisplayUpdateProg > ressGraphicsLib.uni > create mode 100644 > MdeModulePkg/Library/DisplayUpdateProgressTextLib/DisplayUpdateProgressT > extLib.c > create mode 100644 > MdeModulePkg/Library/DisplayUpdateProgressTextLib/DisplayUpdateProgressT > extLib.inf > create mode 100644 > MdeModulePkg/Library/DisplayUpdateProgressTextLib/DisplayUpdateProgressT > extLib.uni > > -- > 2.14.2.windows.3 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
