* [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 @ 2019-09-25 15:50 Pete Batard 2019-09-26 19:30 ` [edk2-devel] " Laszlo Ersek 2019-09-30 1:24 ` Ni, Ray 0 siblings, 2 replies; 6+ messages in thread From: Pete Batard @ 2019-09-25 15:50 UTC (permalink / raw) To: devel; +Cc: zhichao.gao, ray.ni The existing loop is set to call PlatformBootManagerWaitCallback every second except the last one. We believe this is a mistake as it prevents the called code from performing timeout expiration tasks such as, for instance, ensuring that the last segment of a progress bar is displayed before continuing (which is a current issue for the RPi3 platform). Signed-off-by: Pete Batard <pete@akeo.ie> --- MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 1 + 1 file changed, 1 insertion(+) diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c index f3d5e5ac0615..7968a58f3454 100644 --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c @@ -341,6 +341,7 @@ BdsWait ( TimeoutRemain--; } } + PlatformBootManagerWaitCallback (0); DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n")); } -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 2019-09-25 15:50 [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 Pete Batard @ 2019-09-26 19:30 ` Laszlo Ersek 2019-09-29 7:39 ` Liming Gao 2019-09-30 1:24 ` Ni, Ray 1 sibling, 1 reply; 6+ messages in thread From: Laszlo Ersek @ 2019-09-26 19:30 UTC (permalink / raw) To: devel, pete; +Cc: zhichao.gao, ray.ni On 09/25/19 17:50, Pete Batard wrote: > The existing loop is set to call PlatformBootManagerWaitCallback every > second except the last one. We believe this is a mistake as it prevents > the called code from performing timeout expiration tasks such as, for > instance, ensuring that the last segment of a progress bar is displayed > before continuing (which is a current issue for the RPi3 platform). > > Signed-off-by: Pete Batard <pete@akeo.ie> > --- > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > index f3d5e5ac0615..7968a58f3454 100644 > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > @@ -341,6 +341,7 @@ BdsWait ( > TimeoutRemain--; > } > } > + PlatformBootManagerWaitCallback (0); > DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n")); > } > > I can confirm the problem statement, from a quick test with OVMF. Regarding the PlatformBootManagerWaitCallback (0) call, it promises to do the right thing, considering the following library instances: - ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c - OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c In both cases, the function goes, VOID EFIAPI PlatformBootManagerWaitCallback ( UINT16 TimeoutRemain ) { EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION Black; EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION White; UINT16 Timeout; Timeout = PcdGet16 (PcdPlatformBootTimeOut); Black.Raw = 0x00000000; White.Raw = 0x00FFFFFF; BootLogoUpdateProgress ( White.Pixel, Black.Pixel, L"Start boot option", White.Pixel, (Timeout - TimeoutRemain) * 100 / Timeout, 0 ); } If we substitute 0 for TimeoutRemain in the BootLogoUpdateProgress() call, we get (Timeout * 100 / Timeout), i.e. 100%. I haven't tested the patch, and I can't review it quickly for real, but it looks plausible to me. Thanks Laszlo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 2019-09-26 19:30 ` [edk2-devel] " Laszlo Ersek @ 2019-09-29 7:39 ` Liming Gao 0 siblings, 0 replies; 6+ messages in thread From: Liming Gao @ 2019-09-29 7:39 UTC (permalink / raw) To: devel@edk2.groups.io, lersek@redhat.com, pete@akeo.ie Cc: Gao, Zhichao, Ni, Ray This change is good to me. I also verify it in Emulator platform. Reviewed-by: Liming Gao <liming.gao@intel.com> Thanks Liming >-----Original Message----- >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >Laszlo Ersek >Sent: Friday, September 27, 2019 3:30 AM >To: devel@edk2.groups.io; pete@akeo.ie >Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com> >Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/BdsDxe: Also call >PlatformBootManagerWaitCallback on 0 > >On 09/25/19 17:50, Pete Batard wrote: >> The existing loop is set to call PlatformBootManagerWaitCallback every >> second except the last one. We believe this is a mistake as it prevents >> the called code from performing timeout expiration tasks such as, for >> instance, ensuring that the last segment of a progress bar is displayed >> before continuing (which is a current issue for the RPi3 platform). >> >> Signed-off-by: Pete Batard <pete@akeo.ie> >> --- >> MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c >b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c >> index f3d5e5ac0615..7968a58f3454 100644 >> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c >> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c >> @@ -341,6 +341,7 @@ BdsWait ( >> TimeoutRemain--; >> } >> } >> + PlatformBootManagerWaitCallback (0); >> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n")); >> } >> >> > >I can confirm the problem statement, from a quick test with OVMF. > >Regarding the PlatformBootManagerWaitCallback (0) call, it promises to >do the right thing, considering the following library instances: > >- ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c >- OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > >In both cases, the function goes, > >VOID >EFIAPI >PlatformBootManagerWaitCallback ( > UINT16 TimeoutRemain > ) >{ > EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION Black; > EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION White; > UINT16 Timeout; > > Timeout = PcdGet16 (PcdPlatformBootTimeOut); > > Black.Raw = 0x00000000; > White.Raw = 0x00FFFFFF; > > BootLogoUpdateProgress ( > White.Pixel, > Black.Pixel, > L"Start boot option", > White.Pixel, > (Timeout - TimeoutRemain) * 100 / Timeout, > 0 > ); >} > >If we substitute 0 for TimeoutRemain in the BootLogoUpdateProgress() >call, we get (Timeout * 100 / Timeout), i.e. 100%. > >I haven't tested the patch, and I can't review it quickly for real, but >it looks plausible to me. > >Thanks >Laszlo > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 2019-09-25 15:50 [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 Pete Batard 2019-09-26 19:30 ` [edk2-devel] " Laszlo Ersek @ 2019-09-30 1:24 ` Ni, Ray 2019-09-30 23:12 ` Laszlo Ersek 1 sibling, 1 reply; 6+ messages in thread From: Ni, Ray @ 2019-09-30 1:24 UTC (permalink / raw) To: devel@edk2.groups.io, pete@akeo.ie; +Cc: Gao, Zhichao Existing close source platform BDS only updates UI in this function. Reviewed-by: Ray Ni <ray.ni@intel.com> > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pete Batard > Sent: Wednesday, September 25, 2019 11:50 PM > To: devel@edk2.groups.io > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com> > Subject: [edk2-devel] [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 > > The existing loop is set to call PlatformBootManagerWaitCallback every > second except the last one. We believe this is a mistake as it prevents > the called code from performing timeout expiration tasks such as, for > instance, ensuring that the last segment of a progress bar is displayed > before continuing (which is a current issue for the RPi3 platform). > > Signed-off-by: Pete Batard <pete@akeo.ie> > --- > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > index f3d5e5ac0615..7968a58f3454 100644 > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > @@ -341,6 +341,7 @@ BdsWait ( > TimeoutRemain--; > } > } > + PlatformBootManagerWaitCallback (0); > DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n")); > } > > -- > 2.21.0.windows.1 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 2019-09-30 1:24 ` Ni, Ray @ 2019-09-30 23:12 ` Laszlo Ersek 2019-10-08 14:11 ` Liming Gao 0 siblings, 1 reply; 6+ messages in thread From: Laszlo Ersek @ 2019-09-30 23:12 UTC (permalink / raw) To: devel, ray.ni, pete@akeo.ie; +Cc: Gao, Zhichao On 09/30/19 03:24, Ni, Ray wrote: > Existing close source platform BDS only updates UI in this function. > Reviewed-by: Ray Ni <ray.ni@intel.com> Thanks, Ray! Can you please push this patch for Pete? Thanks Laszlo > >> -----Original Message----- >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pete Batard >> Sent: Wednesday, September 25, 2019 11:50 PM >> To: devel@edk2.groups.io >> Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com> >> Subject: [edk2-devel] [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 >> >> The existing loop is set to call PlatformBootManagerWaitCallback every >> second except the last one. We believe this is a mistake as it prevents >> the called code from performing timeout expiration tasks such as, for >> instance, ensuring that the last segment of a progress bar is displayed >> before continuing (which is a current issue for the RPi3 platform). >> >> Signed-off-by: Pete Batard <pete@akeo.ie> >> --- >> MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c >> index f3d5e5ac0615..7968a58f3454 100644 >> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c >> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c >> @@ -341,6 +341,7 @@ BdsWait ( >> TimeoutRemain--; >> } >> } >> + PlatformBootManagerWaitCallback (0); >> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n")); >> } >> >> -- >> 2.21.0.windows.1 >> >> >> > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 2019-09-30 23:12 ` Laszlo Ersek @ 2019-10-08 14:11 ` Liming Gao 0 siblings, 0 replies; 6+ messages in thread From: Liming Gao @ 2019-10-08 14:11 UTC (permalink / raw) To: devel@edk2.groups.io, lersek@redhat.com, Ni, Ray, pete@akeo.ie Cc: Gao, Zhichao Push @2de1f611be06ded3a59726a4052a9039be7d459b > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek > Sent: Tuesday, October 1, 2019 7:12 AM > To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; pete@akeo.ie > Cc: Gao, Zhichao <zhichao.gao@intel.com> > Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 > > On 09/30/19 03:24, Ni, Ray wrote: > > Existing close source platform BDS only updates UI in this function. > > Reviewed-by: Ray Ni <ray.ni@intel.com> > > Thanks, Ray! Can you please push this patch for Pete? > > Thanks > Laszlo > > > > >> -----Original Message----- > >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pete Batard > >> Sent: Wednesday, September 25, 2019 11:50 PM > >> To: devel@edk2.groups.io > >> Cc: Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com> > >> Subject: [edk2-devel] [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 > >> > >> The existing loop is set to call PlatformBootManagerWaitCallback every > >> second except the last one. We believe this is a mistake as it prevents > >> the called code from performing timeout expiration tasks such as, for > >> instance, ensuring that the last segment of a progress bar is displayed > >> before continuing (which is a current issue for the RPi3 platform). > >> > >> Signed-off-by: Pete Batard <pete@akeo.ie> > >> --- > >> MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > >> index f3d5e5ac0615..7968a58f3454 100644 > >> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > >> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > >> @@ -341,6 +341,7 @@ BdsWait ( > >> TimeoutRemain--; > >> } > >> } > >> + PlatformBootManagerWaitCallback (0); > >> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n")); > >> } > >> > >> -- > >> 2.21.0.windows.1 > >> > >> > >> > > > > > > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-08 14:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-09-25 15:50 [PATCH 1/1] MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0 Pete Batard 2019-09-26 19:30 ` [edk2-devel] " Laszlo Ersek 2019-09-29 7:39 ` Liming Gao 2019-09-30 1:24 ` Ni, Ray 2019-09-30 23:12 ` Laszlo Ersek 2019-10-08 14:11 ` Liming Gao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox