Re: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot timeout message

[Ryan Harkin]

On 5 May 2016 at 06:09, Ni, Ruiyu <ruiyu...@intel.com> wrote:
>
>
> Regards,
> Ray
>
>>-Original Message-
>>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
>>Egranov
>>Sent: Thursday, May 5, 2016 8:07 AM
>>To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org
>>Cc: Fan, Jeff <jeff@intel.com>
>>Subject: Re: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot 
>>timeout message
>>
>>Hi Ray,
>>
>>Thanks for the review. My answers below.
>>
>>Thanks,
>>Daniil
>>
>>On 05/04/2016 12:07 AM, Ni, Ruiyu wrote:
>>> 2 comments below.
>>>
>>> Regards,
>>> Ray
>>>
>>>> -Original Message-
>>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>>>> Daniil Egranov
>>>> Sent: Wednesday, May 4, 2016 9:34 AM
>>>> To: edk2-devel@lists.01.org
>>>> Cc: Fan, Jeff <jeff@intel.com>
>>>> Subject: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot 
>>>> timeout message
>>>>
>>>> The PlatformBdsShowProgress() supports graphics mode only, which is not
>>>> applicable for RS-232 serial console. Show the progress message as a 
>>>> console
>>>> text message in case PlatformBdsShowProgress() fails.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Daniil Egranov <daniil.egra...@arm.com>
>>>> ---
>>>> IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 9 -
>>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>>> b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>>> index 6958979..d1a5c05 100644
>>>> --- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>>> +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>>> @@ -925,7 +925,7 @@ ShowProgress (
>>>>  // Show progress
>>>>  //
>>>>  if (TmpStr != NULL) {
>>>> -  PlatformBdsShowProgress (
>>>> +  Status = PlatformBdsShowProgress (
>>>>  Foreground,
>>>>  Background,
>>>>  TmpStr,
>>>> @@ -933,12 +933,19 @@ ShowProgress (
>>>>  ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault),
>>>>  0
>>>>  );
>>>> +  if (EFI_ERROR(Status)) {
>>>> +//if graphics mode is not supported (serial console) show 
>>>> text progress message
>>>> +AsciiPrint ("\rPress any key to enter Boot Menu in %d seconds 
>>>> ", TimeoutRemain);

When I was testing this patch, I noticed that if I press "enter", it
immediately continues to boot the configured boot entry.  If I press
any other key (in reality I only tried a few like 'x' and ESC) then it
goes to the Boot Menu.

I didn't see another revision yet in response to Ruiyu's comments, so
I figured I was in time for a quick mod.

I was thinking the message could get very complex, but thought
something like this might be better:

"Press ENTER to boot now or any other key to show the Boot Menu in %d
seconds "

>>>> +  }
>>> 1. Why use AsciiPrint but not Print(L"")?
>>> I agree they are the same but normally we use Print().
>>>
>>
>>I was not sure which one to use. I'll correct it.
>>
> Thanks!
>
>
>>>>  }
>>>>}
>>>>  }
>>>>
>>>>  if (TmpStr != NULL) {
>>>>gBS->FreePool (TmpStr);
>>>> +if (EFI_ERROR(Status)) {
>>>> +  AsciiPrint ("\n");
>>>> +}
>>> 2. What's the purpose of the EOL here?
>>>
>>
>>The AsciiPrint above uses cartridge return without a new line so this
>>EOL preserves last message from being erased by other console outputs.
>>
> I see. Thanks! I agree!
>
>>>>  }
>>>>
>>>>  //
>>>> --
>>>> 2.7.4
>>>>
>>>> ___
>>>> 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
>>>
>>___
>>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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot timeout message

[Ni, Ruiyu]

Regards,
Ray

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
>Egranov
>Sent: Thursday, May 5, 2016 8:07 AM
>To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org
>Cc: Fan, Jeff <jeff@intel.com>
>Subject: Re: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot 
>timeout message
>
>Hi Ray,
>
>Thanks for the review. My answers below.
>
>Thanks,
>Daniil
>
>On 05/04/2016 12:07 AM, Ni, Ruiyu wrote:
>> 2 comments below.
>>
>> Regards,
>> Ray
>>
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>>> Daniil Egranov
>>> Sent: Wednesday, May 4, 2016 9:34 AM
>>> To: edk2-devel@lists.01.org
>>> Cc: Fan, Jeff <jeff@intel.com>
>>> Subject: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot 
>>> timeout message
>>>
>>> The PlatformBdsShowProgress() supports graphics mode only, which is not
>>> applicable for RS-232 serial console. Show the progress message as a console
>>> text message in case PlatformBdsShowProgress() fails.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Daniil Egranov <daniil.egra...@arm.com>
>>> ---
>>> IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 9 -
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>> b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>> index 6958979..d1a5c05 100644
>>> --- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>> +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>>> @@ -925,7 +925,7 @@ ShowProgress (
>>>  // Show progress
>>>  //
>>>  if (TmpStr != NULL) {
>>> -  PlatformBdsShowProgress (
>>> +  Status = PlatformBdsShowProgress (
>>>  Foreground,
>>>  Background,
>>>  TmpStr,
>>> @@ -933,12 +933,19 @@ ShowProgress (
>>>  ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault),
>>>  0
>>>  );
>>> +  if (EFI_ERROR(Status)) {
>>> +//if graphics mode is not supported (serial console) show text 
>>> progress message
>>> +AsciiPrint ("\rPress any key to enter Boot Menu in %d seconds  
>>>", TimeoutRemain);
>>> +  }
>> 1. Why use AsciiPrint but not Print(L"")?
>> I agree they are the same but normally we use Print().
>>
>
>I was not sure which one to use. I'll correct it.
>
Thanks!


>>>  }
>>>}
>>>  }
>>>
>>>  if (TmpStr != NULL) {
>>>gBS->FreePool (TmpStr);
>>> +if (EFI_ERROR(Status)) {
>>> +  AsciiPrint ("\n");
>>> +}
>> 2. What's the purpose of the EOL here?
>>
>
>The AsciiPrint above uses cartridge return without a new line so this
>EOL preserves last message from being erased by other console outputs.
>
I see. Thanks! I agree!

>>>  }
>>>
>>>  //
>>> --
>>> 2.7.4
>>>
>>> ___
>>> 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
>>
>___
>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 v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot timeout message

[Daniil Egranov]

Hi Ray,

Thanks for the review. My answers below.

Thanks,
Daniil

On 05/04/2016 12:07 AM, Ni, Ruiyu wrote:

2 comments below.

Regards,
Ray


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
Egranov
Sent: Wednesday, May 4, 2016 9:34 AM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff <jeff@intel.com>
Subject: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot 
timeout message

The PlatformBdsShowProgress() supports graphics mode only, which is not
applicable for RS-232 serial console. Show the progress message as a console
text message in case PlatformBdsShowProgress() fails.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov <daniil.egra...@arm.com>
---
IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
index 6958979..d1a5c05 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
@@ -925,7 +925,7 @@ ShowProgress (
 // Show progress
 //
 if (TmpStr != NULL) {
-  PlatformBdsShowProgress (
+  Status = PlatformBdsShowProgress (
 Foreground,
 Background,
 TmpStr,
@@ -933,12 +933,19 @@ ShowProgress (
 ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault),
 0
 );
+  if (EFI_ERROR(Status)) {
+//if graphics mode is not supported (serial console) show text 
progress message
+AsciiPrint ("\rPress any key to enter Boot Menu in %d seconds 
", TimeoutRemain);
+  }

1. Why use AsciiPrint but not Print(L"")?
I agree they are the same but normally we use Print().



I was not sure which one to use. I'll correct it.


 }
   }
 }

 if (TmpStr != NULL) {
   gBS->FreePool (TmpStr);
+if (EFI_ERROR(Status)) {
+  AsciiPrint ("\n");
+}

2. What's the purpose of the EOL here?



The AsciiPrint above uses cartridge return without a new line so this 
EOL preserves last message from being erased by other console outputs.



 }

 //
--
2.7.4

___
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


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

Re: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot timeout message

[Ni, Ruiyu]

2 comments below.

Regards,
Ray

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
>Egranov
>Sent: Wednesday, May 4, 2016 9:34 AM
>To: edk2-devel@lists.01.org
>Cc: Fan, Jeff <jeff@intel.com>
>Subject: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot 
>timeout message
>
>The PlatformBdsShowProgress() supports graphics mode only, which is not
>applicable for RS-232 serial console. Show the progress message as a console
>text message in case PlatformBdsShowProgress() fails.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Daniil Egranov <daniil.egra...@arm.com>
>---
> IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>index 6958979..d1a5c05 100644
>--- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>@@ -925,7 +925,7 @@ ShowProgress (
> // Show progress
> //
> if (TmpStr != NULL) {
>-  PlatformBdsShowProgress (
>+  Status = PlatformBdsShowProgress (
> Foreground,
> Background,
> TmpStr,
>@@ -933,12 +933,19 @@ ShowProgress (
> ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault),
> 0
> );
>+  if (EFI_ERROR(Status)) {
>+//if graphics mode is not supported (serial console) show text 
>progress message
>+AsciiPrint ("\rPress any key to enter Boot Menu in %d seconds 
>", TimeoutRemain);
>+  }
1. Why use AsciiPrint but not Print(L"")?
I agree they are the same but normally we use Print().

> }
>   }
> }
>
> if (TmpStr != NULL) {
>   gBS->FreePool (TmpStr);
>+if (EFI_ERROR(Status)) {
>+  AsciiPrint ("\n");
>+}
2. What's the purpose of the EOL here?

> }
>
> //
>--
>2.7.4
>
>___
>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

[edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot timeout message

[Daniil Egranov]

The PlatformBdsShowProgress() supports graphics mode only, which is not
applicable for RS-232 serial console. Show the progress message as a console
text message in case PlatformBdsShowProgress() fails.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 
---
 IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
index 6958979..d1a5c05 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
@@ -925,7 +925,7 @@ ShowProgress (
 // Show progress
 //
 if (TmpStr != NULL) {
-  PlatformBdsShowProgress (
+  Status = PlatformBdsShowProgress (
 Foreground,
 Background,
 TmpStr,
@@ -933,12 +933,19 @@ ShowProgress (
 ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault),
 0
 );
+  if (EFI_ERROR(Status)) {
+//if graphics mode is not supported (serial console) show text 
progress message
+AsciiPrint ("\rPress any key to enter Boot Menu in %d seconds 
", TimeoutRemain);
+  }
 }
   }
 }
 
 if (TmpStr != NULL) {
   gBS->FreePool (TmpStr);
+if (EFI_ERROR(Status)) {
+  AsciiPrint ("\n");
+}
 }
 
 //
-- 
2.7.4

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