Re: [Patch V2 0/9] Add DisplayUpdateProgressLib for capsules

["Zeng, Star" <star.zeng@intel.com>]

Mike,

Thanks for the clarification.

Reviewed-by: Star Zeng <star.zeng@intel.com>


Thanks,
Star
-----Original Message-----
From: Kinney, Michael D 
Sent: Saturday, April 21, 2018 7:12 AM
To: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>; Dong, Eric <eric.dong@intel.com>; Wei, David <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>; Steele, Kelly <kelly.steele@intel.com>
Subject: RE: [Patch V2 0/9] Add DisplayUpdateProgressLib for capsules

Star,

I did consider a PCD based method and I even prototyped it.  It does work.  However, it uses the Dynamic PCD memory database as a shared resource between all the FMP modules and depends on the FMP modules to set the PCDs when their
SetImage() service is called.  Since not all FMP modules may need to use these settings, the PCDs may contain values from other FMP modules that were processing capsules in the same boot.  This requires the caller of SetImage() to always reset the PCDs each time SetImage() is called for any FMP.

The Protocol per FMP device provides the information for all FMP consumers on the same handle as the FMP Protocol so the rules for usage are clear from a consumer perspective.

Mike


> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, April 19, 2018 9:51 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Kinney, Michael D 
> <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Dong, Eric 
> <eric.dong@intel.com>; Wei, David <david.wei@intel.com>; Guo, Mang 
> <mang.guo@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Zeng, 
> Star <star.zeng@intel.com>
> Subject: RE: [Patch V2 0/9] Add DisplayUpdateProgressLib for capsules
> 
> I also had another comment to the protocol at
> https://lists.01.org/pipermail/edk2-devel/2018-
> April/023720.html before.
> 
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Friday, April 20, 2018 12:23 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; 
> edk2-devel@lists.01.org
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Zeng, Star 
> <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Wei, David 
> <david.wei@intel.com>; Guo, Mang <mang.guo@intel.com>; Steele, Kelly 
> <kelly.steele@intel.com>
> Subject: RE: [Patch V2 0/9] Add DisplayUpdateProgressLib for capsules
> 
> Thanks Mike.
> 
> I still think we should add a Version flag for 
> EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL for future extension, as 
> we discussed in V1.
> 
> Other update seems good to me. Reviewed-by:
> Jiewen.yao@Intel.com
> 
> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Thursday, April 12, 2018 8:48 AM
> > To: edk2-devel@lists.01.org
> > Cc: Sean Brogan <sean.brogan@microsoft.com>; Zeng,
> Star
> > <star.zeng@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Wei, David
> <david.wei@intel.com>; Guo, Mang
> > <mang.guo@intel.com>; Steele, Kelly
> <kelly.steele@intel.com>; Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Subject: [Patch V2 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/MsCapsul
> eSupport/MsCap
> > s
> uleUpdatePkg/Include/Library/DisplayUpdateProgressLib.h
> >
> https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsul
> eSupport/MsCap
> > su
> leUpdatePkg/Library/DisplayUpdateProgressGraphicsLib
> >
> https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsul
> eSupport/MsCap
> > su leUpdatePkg/Library/DisplayUpdateProgressTextLib
> >
> > Updates for V2
> > ==============
> > * Change DisplayUpdateProgressGraphicsLib to 
> > DisplayUpdateProgressLibGraphics
> > * Change DisplayUpdateProgressTextLib to
> DisplayUpdateProgressLibText
> > * Clarify that color in Firmware Management Progress
> Protocol is the
> > foreground color
> > * Add missing parameters to
> PerformFlashWriteWithProgress() function header.
> > * Update PerformFlashWriteWithProgress() function
> header describing
> > the use of
> >   the start and end percentage values.
> > * Update QuarkPlatformPkg
> PerformFlashWriteWithProgress() to call
> > Progress() for
> >   the end precentage.
> > * Update Vlv2Tbl2DevicePkg
> PerformFlashWriteWithProgress() to call Progress()
> >   for the end precentage.
> >
> > 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 <sean.brogan@microsoft.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: David Wei <david.wei@intel.com>
> > Cc: Mang Guo <mang.guo@intel.com>
> > Cc: Kelly Steele <kelly.steele@intel.com>
> >
> > Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> > 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  |
> 51 +++
> >  .../DisplayUpdateProgressLibGraphics.c             |
> 475
> > +++++++++++++++++++++
> >  .../DisplayUpdateProgressLibGraphics.inf           |
> 60 +++
> >  .../DisplayUpdateProgressLibGraphics.uni           |
> 18 +
> >  .../DisplayUpdateProgressLibText.c                 |
> 174 ++++++++
> >  .../DisplayUpdateProgressLibText.inf               |
> 53 +++
> >  .../DisplayUpdateProgressLibText.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                    |
> 78 +++-
> >  QuarkPlatformPkg/Quark.dsc                         |
> 1 +
> >  .../Include/Library/PlatformFlashAccessLib.h       |
> 49 ++-
> >  .../PlatformFlashAccessLibNull.c                   |
> 70 ++-
> >  .../SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c |
> 90 +++-
> >  .../PlatformFlashAccessLib.c                       |
> 102 +++--
> >  .../PlatformFlashAccessLib.inf                     |
> 3 +-
> >  Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc            |
> 1 +
> >  Vlv2TbltDevicePkg/PlatformPkgIA32.dsc              |
> 1 +
> >  Vlv2TbltDevicePkg/PlatformPkgX64.dsc               |
> 1 +
> >  25 files changed, 1387 insertions(+), 104 deletions(-
> )  create mode
> > 100644
> MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h
> >  create mode 100644
> >
> MdeModulePkg/Include/Protocol/FirmwareManagementProgress
> .h
> >  create mode 100644
> >
> MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/Di
> splayUpdatePro
> > g
> > ressLibGraphics.c
> >  create mode 100644
> >
> MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/Di
> splayUpdatePro
> > g
> > ressLibGraphics.inf
> >  create mode 100644
> >
> MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/Di
> splayUpdatePro
> > g
> > ressLibGraphics.uni
> >  create mode 100644
> >
> MdeModulePkg/Library/DisplayUpdateProgressLibText/Displa
> yUpdateProgres
> > sLi
> > bText.c
> >  create mode 100644
> >
> MdeModulePkg/Library/DisplayUpdateProgressLibText/Displa
> yUpdateProgres
> > sLi
> > bText.inf
> >  create mode 100644
> >
> MdeModulePkg/Library/DisplayUpdateProgressLibText/Displa
> yUpdateProgres
> > sLi
> > bText.uni
> >
> > --
> > 2.14.2.windows.3