diff mbox series

[edk2,edk2-platforms,3/3] Silicon/SynQuacerPlatformFlashAccessLib: replace progress indication

Message ID 20180607113203.27606-4-ard.biesheuvel@linaro.org
State New
Headers show
Series DeveloperBox capsule update changes for progress reporting | expand

Commit Message

Ard Biesheuvel June 7, 2018, 11:32 a.m. UTC 
Replace the home cooked progress indication with calls into the new
(*Progress)() argument to PerformFlashWriteWithProgress(), which
allows the flash access routine to report progress via a platform
provided callback.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c                        |  1 +
 Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c   | 44 ++++++--------------
 Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf |  1 -
 3 files changed, 14 insertions(+), 32 deletions(-)

-- 
2.17.0

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

Comments

Leif Lindholm June 12, 2018, 10:52 p.m. UTC | #1 
On Thu, Jun 07, 2018 at 01:32:03PM +0200, Ard Biesheuvel wrote:
> Replace the home cooked progress indication with calls into the new

> (*Progress)() argument to PerformFlashWriteWithProgress(), which

> allows the flash access routine to report progress via a platform

> provided callback.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


I think 1-2 depends on changes that aren't currently happening based
on discussions. If I'm wrong, set me straight.

However, does this one not make sense on its own?
If so, it has
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


> ---

>  Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c                        |  1 +

>  Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c   | 44 ++++++--------------

>  Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf |  1 -

>  3 files changed, 14 insertions(+), 32 deletions(-)

> 

> diff --git a/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c b/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c

> index 38f1830b5c2e..c5e46dcd4ddf 100644

> --- a/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c

> +++ b/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c

> @@ -72,6 +72,7 @@ PerformFlashWriteWithProgress (

>  {

>    EFI_STATUS                      Status;

>    AMD_ISCP_DXE_PROTOCOL           *IscpDxeProtocol;

> +  UINTN                           Remaining;

>  

>    if (FlashAddressType != FlashAddressTypeRelativeAddress) {

>      DEBUG ((DEBUG_ERROR, "%a: only FlashAddressTypeRelativeAddress supported\n",

> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c

> index 48d385993b38..ebb6ce189aa5 100644

> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c

> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c

> @@ -17,7 +17,6 @@

>  #include <PiDxe.h>

>  

>  #include <Library/BaseMemoryLib.h>

> -#include <Library/BootLogoLib.h>

>  #include <Library/DebugLib.h>

>  #include <Library/DxeServicesTableLib.h>

>  #include <Library/MemoryAllocationLib.h>

> @@ -195,17 +194,7 @@ PerformFlashWriteWithProgress (

>    EFI_LBA                             Lba;

>    EFI_PHYSICAL_ADDRESS                FvbBaseAddress;

>    UINTN                               NumBytes;

> -  EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION Black;

> -  EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION White;

> -  UINTN                               Resolution;

> -  UINTN                               CurrentProgress;

> -  BOOLEAN                             HaveBootGraphics;

> -

> -  Black.Raw = 0x00000000;

> -  White.Raw = 0x00FFFFFF;

> -

> -  Status = BootLogoEnableLogo ();

> -  HaveBootGraphics = !EFI_ERROR (Status);

> +  UINTN                               Remaining;

>  

>    if (FlashAddressType != FlashAddressTypeAbsoluteAddress) {

>      DEBUG ((DEBUG_ERROR, "%a: only FlashAddressTypeAbsoluteAddress supported\n",

> @@ -274,14 +263,8 @@ PerformFlashWriteWithProgress (

>      return Status;

>    }

>  

> -  if (HaveBootGraphics) {

> -    Resolution = (BlockSize * 100) / Length + 1;

> -    CurrentProgress = 0;

> -

> -    Status = BootLogoUpdateProgress (White.Pixel, Black.Pixel,

> -               L"Updating firmware - please wait", Black.Pixel, 100, 0);

> -  } else {

> -    Print (L"Updating firmware - please wait ");

> +  if (Progress == NULL) {

> +    Print (L"\n\nUpdating firmware - please wait ");

>    }

>  

>    //

> @@ -298,7 +281,8 @@ PerformFlashWriteWithProgress (

>      return Status;

>    }

>  

> -  while (Length > 0) {

> +  Remaining = Length;

> +  while (Remaining > 0) {

>      //

>      // Write the new data

>      //

> @@ -315,20 +299,18 @@ PerformFlashWriteWithProgress (

>        }

>      }

>  

> -    if (HaveBootGraphics) {

> -      Status = BootLogoUpdateProgress (White.Pixel, Black.Pixel,

> -                L"Updating firmware - please wait", White.Pixel,

> -                CurrentProgress + Resolution, CurrentProgress);

> -      CurrentProgress += Resolution;

> +    Buffer += BlockSize;

> +    Remaining -= BlockSize;

> +    Lba++;

> +

> +    if (Progress != NULL) {

> +      Progress (EndPercentage -

> +                (Remaining * (EndPercentage - StartPercentage)) / Length);

>      } else {

>        Print (L".");

>      }

> -

> -    Buffer += BlockSize;

> -    Length -= BlockSize;

> -    Lba++;

>    }

> -  if (!HaveBootGraphics) {

> +  if (Progress == NULL) {

>      Print (L"\n");

>    }

>  

> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf

> index 4dfa11372a38..6ca34ada1d03 100644

> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf

> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf

> @@ -33,7 +33,6 @@ [Protocols]

>  

>  [LibraryClasses]

>    BaseMemoryLib

> -  BootLogoLib

>    DebugLib

>    DxeServicesTableLib

>    UefiBootServicesTableLib

> -- 

> 2.17.0

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel June 13, 2018, 7:16 a.m. UTC | #2 
On 13 June 2018 at 00:52, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Jun 07, 2018 at 01:32:03PM +0200, Ard Biesheuvel wrote:

>> Replace the home cooked progress indication with calls into the new

>> (*Progress)() argument to PerformFlashWriteWithProgress(), which

>> allows the flash access routine to report progress via a platform

>> provided callback.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.1

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>

> I think 1-2 depends on changes that aren't currently happening based

> on discussions. If I'm wrong, set me straight.

>

> However, does this one not make sense on its own?


Not really. The progress reporting only works for PersistAcrossReset
capsules to begin with. That is essentially what all the fuss is
about.

I don't necessarily agree with that, but given that Windows and
Linux/x86 only use that type of capsule, it made sense for our stuff
to get aligned with that.

> If so, it has

> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>


Thanks.

>> ---

>>  Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c                        |  1 +

>>  Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c   | 44 ++++++--------------

>>  Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf |  1 -

>>  3 files changed, 14 insertions(+), 32 deletions(-)

>>

>> diff --git a/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c b/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c

>> index 38f1830b5c2e..c5e46dcd4ddf 100644

>> --- a/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c

>> +++ b/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c

>> @@ -72,6 +72,7 @@ PerformFlashWriteWithProgress (

>>  {

>>    EFI_STATUS                      Status;

>>    AMD_ISCP_DXE_PROTOCOL           *IscpDxeProtocol;

>> +  UINTN                           Remaining;

>>

>>    if (FlashAddressType != FlashAddressTypeRelativeAddress) {

>>      DEBUG ((DEBUG_ERROR, "%a: only FlashAddressTypeRelativeAddress supported\n",

>> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c

>> index 48d385993b38..ebb6ce189aa5 100644

>> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c

>> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c

>> @@ -17,7 +17,6 @@

>>  #include <PiDxe.h>

>>

>>  #include <Library/BaseMemoryLib.h>

>> -#include <Library/BootLogoLib.h>

>>  #include <Library/DebugLib.h>

>>  #include <Library/DxeServicesTableLib.h>

>>  #include <Library/MemoryAllocationLib.h>

>> @@ -195,17 +194,7 @@ PerformFlashWriteWithProgress (

>>    EFI_LBA                             Lba;

>>    EFI_PHYSICAL_ADDRESS                FvbBaseAddress;

>>    UINTN                               NumBytes;

>> -  EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION Black;

>> -  EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION White;

>> -  UINTN                               Resolution;

>> -  UINTN                               CurrentProgress;

>> -  BOOLEAN                             HaveBootGraphics;

>> -

>> -  Black.Raw = 0x00000000;

>> -  White.Raw = 0x00FFFFFF;

>> -

>> -  Status = BootLogoEnableLogo ();

>> -  HaveBootGraphics = !EFI_ERROR (Status);

>> +  UINTN                               Remaining;

>>

>>    if (FlashAddressType != FlashAddressTypeAbsoluteAddress) {

>>      DEBUG ((DEBUG_ERROR, "%a: only FlashAddressTypeAbsoluteAddress supported\n",

>> @@ -274,14 +263,8 @@ PerformFlashWriteWithProgress (

>>      return Status;

>>    }

>>

>> -  if (HaveBootGraphics) {

>> -    Resolution = (BlockSize * 100) / Length + 1;

>> -    CurrentProgress = 0;

>> -

>> -    Status = BootLogoUpdateProgress (White.Pixel, Black.Pixel,

>> -               L"Updating firmware - please wait", Black.Pixel, 100, 0);

>> -  } else {

>> -    Print (L"Updating firmware - please wait ");

>> +  if (Progress == NULL) {

>> +    Print (L"\n\nUpdating firmware - please wait ");

>>    }

>>

>>    //

>> @@ -298,7 +281,8 @@ PerformFlashWriteWithProgress (

>>      return Status;

>>    }

>>

>> -  while (Length > 0) {

>> +  Remaining = Length;

>> +  while (Remaining > 0) {

>>      //

>>      // Write the new data

>>      //

>> @@ -315,20 +299,18 @@ PerformFlashWriteWithProgress (

>>        }

>>      }

>>

>> -    if (HaveBootGraphics) {

>> -      Status = BootLogoUpdateProgress (White.Pixel, Black.Pixel,

>> -                L"Updating firmware - please wait", White.Pixel,

>> -                CurrentProgress + Resolution, CurrentProgress);

>> -      CurrentProgress += Resolution;

>> +    Buffer += BlockSize;

>> +    Remaining -= BlockSize;

>> +    Lba++;

>> +

>> +    if (Progress != NULL) {

>> +      Progress (EndPercentage -

>> +                (Remaining * (EndPercentage - StartPercentage)) / Length);

>>      } else {

>>        Print (L".");

>>      }

>> -

>> -    Buffer += BlockSize;

>> -    Length -= BlockSize;

>> -    Lba++;

>>    }

>> -  if (!HaveBootGraphics) {

>> +  if (Progress == NULL) {

>>      Print (L"\n");

>>    }

>>

>> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf

>> index 4dfa11372a38..6ca34ada1d03 100644

>> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf

>> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf

>> @@ -33,7 +33,6 @@ [Protocols]

>>

>>  [LibraryClasses]

>>    BaseMemoryLib

>> -  BootLogoLib

>>    DebugLib

>>    DxeServicesTableLib

>>    UefiBootServicesTableLib

>> --

>> 2.17.0

>>

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

Patch

diff --git a/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c b/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c
index 38f1830b5c2e..c5e46dcd4ddf 100644
--- a/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c
+++ b/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c
@@ -72,6 +72,7 @@  PerformFlashWriteWithProgress (
 {
   EFI_STATUS                      Status;
   AMD_ISCP_DXE_PROTOCOL           *IscpDxeProtocol;
+  UINTN                           Remaining;
 
   if (FlashAddressType != FlashAddressTypeRelativeAddress) {
     DEBUG ((DEBUG_ERROR, "%a: only FlashAddressTypeRelativeAddress supported\n",
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c
index 48d385993b38..ebb6ce189aa5 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c
@@ -17,7 +17,6 @@ 
 #include <PiDxe.h>
 
 #include <Library/BaseMemoryLib.h>
-#include <Library/BootLogoLib.h>
 #include <Library/DebugLib.h>
 #include <Library/DxeServicesTableLib.h>
 #include <Library/MemoryAllocationLib.h>
@@ -195,17 +194,7 @@  PerformFlashWriteWithProgress (
   EFI_LBA                             Lba;
   EFI_PHYSICAL_ADDRESS                FvbBaseAddress;
   UINTN                               NumBytes;
-  EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION Black;
-  EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION White;
-  UINTN                               Resolution;
-  UINTN                               CurrentProgress;
-  BOOLEAN                             HaveBootGraphics;
-
-  Black.Raw = 0x00000000;
-  White.Raw = 0x00FFFFFF;
-
-  Status = BootLogoEnableLogo ();
-  HaveBootGraphics = !EFI_ERROR (Status);
+  UINTN                               Remaining;
 
   if (FlashAddressType != FlashAddressTypeAbsoluteAddress) {
     DEBUG ((DEBUG_ERROR, "%a: only FlashAddressTypeAbsoluteAddress supported\n",
@@ -274,14 +263,8 @@  PerformFlashWriteWithProgress (
     return Status;
   }
 
-  if (HaveBootGraphics) {
-    Resolution = (BlockSize * 100) / Length + 1;
-    CurrentProgress = 0;
-
-    Status = BootLogoUpdateProgress (White.Pixel, Black.Pixel,
-               L"Updating firmware - please wait", Black.Pixel, 100, 0);
-  } else {
-    Print (L"Updating firmware - please wait ");
+  if (Progress == NULL) {
+    Print (L"\n\nUpdating firmware - please wait ");
   }
 
   //
@@ -298,7 +281,8 @@  PerformFlashWriteWithProgress (
     return Status;
   }
 
-  while (Length > 0) {
+  Remaining = Length;
+  while (Remaining > 0) {
     //
     // Write the new data
     //
@@ -315,20 +299,18 @@  PerformFlashWriteWithProgress (
       }
     }
 
-    if (HaveBootGraphics) {
-      Status = BootLogoUpdateProgress (White.Pixel, Black.Pixel,
-                L"Updating firmware - please wait", White.Pixel,
-                CurrentProgress + Resolution, CurrentProgress);
-      CurrentProgress += Resolution;
+    Buffer += BlockSize;
+    Remaining -= BlockSize;
+    Lba++;
+
+    if (Progress != NULL) {
+      Progress (EndPercentage -
+                (Remaining * (EndPercentage - StartPercentage)) / Length);
     } else {
       Print (L".");
     }
-
-    Buffer += BlockSize;
-    Length -= BlockSize;
-    Lba++;
   }
-  if (!HaveBootGraphics) {
+  if (Progress == NULL) {
     Print (L"\n");
   }
 
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf
index 4dfa11372a38..6ca34ada1d03 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf
@@ -33,7 +33,6 @@  [Protocols]
 
 [LibraryClasses]
   BaseMemoryLib
-  BootLogoLib
   DebugLib
   DxeServicesTableLib
   UefiBootServicesTableLib