[edk2] [Patch 0/9] Add DisplayUpdateProgressLib for capsules

Michael D Kinney posted 9 patches 6 years, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
.../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/DisplayUpdateProgressGraphicsLib.c
create mode 100644 MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/DisplayUpdateProgressGraphicsLib.inf
create mode 100644 MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/DisplayUpdateProgressGraphicsLib.uni
create mode 100644 MdeModulePkg/Library/DisplayUpdateProgressTextLib/DisplayUpdateProgressTextLib.c
create mode 100644 MdeModulePkg/Library/DisplayUpdateProgressTextLib/DisplayUpdateProgressTextLib.inf
create mode 100644 MdeModulePkg/Library/DisplayUpdateProgressTextLib/DisplayUpdateProgressTextLib.uni
[edk2] [Patch 0/9] Add DisplayUpdateProgressLib for capsules
Posted by Michael D Kinney 6 years, 7 months ago
https://bugzilla.tianocore.org/show_bug.cgi?id=801

Based on content from:

https://github.com/Microsoft/MS_UEFI/blob/share/MsCapsuleSupport/MsCapsuleUpdatePkg/Include/Library/DisplayUpdateProgressLib.h
https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport/MsCapsuleUpdatePkg/Library/DisplayUpdateProgressGraphicsLib
https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport/MsCapsuleUpdatePkg/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 <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  |  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/DisplayUpdateProgressGraphicsLib.c
 create mode 100644 MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/DisplayUpdateProgressGraphicsLib.inf
 create mode 100644 MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/DisplayUpdateProgressGraphicsLib.uni
 create mode 100644 MdeModulePkg/Library/DisplayUpdateProgressTextLib/DisplayUpdateProgressTextLib.c
 create mode 100644 MdeModulePkg/Library/DisplayUpdateProgressTextLib/DisplayUpdateProgressTextLib.inf
 create mode 100644 MdeModulePkg/Library/DisplayUpdateProgressTextLib/DisplayUpdateProgressTextLib.uni

-- 
2.14.2.windows.3

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 0/9] Add DisplayUpdateProgressLib for capsules
Posted by Kinney, Michael D 6 years, 7 months ago
This patch series is also available for review on the 
following branch:

https://github.com/mdkinney/edk2/tree/Bug_801_DisplayUpdateProgressLib

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Michael D Kinney
> Sent: Wednesday, April 4, 2018 1:26 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Wei, David <david.wei@intel.com>
> Subject: [edk2] [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/MsCapsu
> leSupport/MsCapsuleUpdatePkg/Include/Library/DisplayUpd
> ateProgressLib.h
> https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsu
> leSupport/MsCapsuleUpdatePkg/Library/DisplayUpdateProgr
> essGraphicsLib
> https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsu
> leSupport/MsCapsuleUpdatePkg/Library/DisplayUpdateProgr
> essTextLib
> 
> 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  |
> 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
> isplayUpdateProgressGraphicsLib.c
>  create mode 100644
> MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/D
> isplayUpdateProgressGraphicsLib.inf
>  create mode 100644
> MdeModulePkg/Library/DisplayUpdateProgressGraphicsLib/D
> isplayUpdateProgressGraphicsLib.uni
>  create mode 100644
> MdeModulePkg/Library/DisplayUpdateProgressTextLib/Displ
> ayUpdateProgressTextLib.c
>  create mode 100644
> MdeModulePkg/Library/DisplayUpdateProgressTextLib/Displ
> ayUpdateProgressTextLib.inf
>  create mode 100644
> MdeModulePkg/Library/DisplayUpdateProgressTextLib/Displ
> ayUpdateProgressTextLib.uni
> 
> --
> 2.14.2.windows.3
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 0/9] Add DisplayUpdateProgressLib for capsules
Posted by Yao, Jiewen 6 years, 7 months ago
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: 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>
> 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 <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  |  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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 0/9] Add DisplayUpdateProgressLib for capsules
Posted by Kinney, Michael D 6 years, 6 months ago
Jiewen,

Responses below.

Mike

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Wednesday, April 4, 2018 5:03 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 0/9] Add DisplayUpdateProgressLib
> for capsules
> 
> 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?
> 

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.
> 
> 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?
> 

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.
> 
> 3) According to lib naming conversion, we should use
> DisplayUpdateProgressLibGraphic, and
> DisplayUpdateProgressLibText. :-)
> 

Yes.  We should make that change.

> 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 ?

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.

> 
> 
> Thank you
> Yao Jiewen
> 
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Thursday, April 5, 2018 4:26 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>
> > 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/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 <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  |
> 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

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