From patchwork Tue Nov 4 19:41:50 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laszlo Ersek X-Patchwork-Id: 40166 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-lb0-f200.google.com (mail-lb0-f200.google.com [209.85.217.200]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id D4BE621894 for ; Tue, 4 Nov 2014 19:47:23 +0000 (UTC) Received: by mail-lb0-f200.google.com with SMTP id f15sf8092269lbj.3 for ; Tue, 04 Nov 2014 11:47:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:date:from:user-agent :mime-version:to:references:in-reply-to:subject:precedence:reply-to :list-id:list-unsubscribe:list-archive:list-post:list-help :list-subscribe:errors-to:x-original-sender :x-original-authentication-results:mailing-list:content-type :content-transfer-encoding; bh=uWf9gD1klJVtmKc74phqOahYQlThTA6pq6uEhpxz+80=; b=j1KamTg0fdr2pfZGm5fLgXK1S4xamZliI1dY/AmhUeA6XePPv0VNUxLkJb+LEvN8Tp 4ZPITWngZOa1KNYgLm7j2TaPZFCd3qK6Q+so9b5l46ILXy+ezF00+4nvPP5qG/I9Vu/n Bg9HCmYORuXKGppaU8ucb5x9yHZ1hb9BiqE0Bu0a8TxpnUMtLZm2WevYv5k7f84NNwuq vNsybq9ElW0Nnjk+sQOmWrN8nIHhUtXcogdMk6Da+c4EP14gdpFvzaaO23fDhKylUidL Bv6FbldPsa614hzG1hhgjsy2z0zFmKkVU4Y0xc07EMPq5TCC5i6N1/tqES9TNgzCdCvP Hbgw== X-Gm-Message-State: ALoCoQnRQoY9M+AJjolkWunv7Ybf3aKf64CgZ416/yhp05u3LMyK5MhuSok4axeplqrMYsSONLe2 X-Received: by 10.112.138.202 with SMTP id qs10mr8982081lbb.5.1415130442203; Tue, 04 Nov 2014 11:47:22 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.27.97 with SMTP id s1ls933867lag.81.gmail; Tue, 04 Nov 2014 11:47:22 -0800 (PST) X-Received: by 10.112.199.227 with SMTP id jn3mr36701918lbc.86.1415130442027; Tue, 04 Nov 2014 11:47:22 -0800 (PST) Received: from mail-lb0-f182.google.com (mail-lb0-f182.google.com. [209.85.217.182]) by mx.google.com with ESMTPS id q1si2470393laq.20.2014.11.04.11.47.22 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 04 Nov 2014 11:47:22 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.182 as permitted sender) client-ip=209.85.217.182; Received: by mail-lb0-f182.google.com with SMTP id f15so13162520lbj.13 for ; Tue, 04 Nov 2014 11:47:21 -0800 (PST) X-Received: by 10.112.189.10 with SMTP id ge10mr61395173lbc.23.1415130441813; Tue, 04 Nov 2014 11:47:21 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.184.201 with SMTP id ew9csp179959lbc; Tue, 4 Nov 2014 11:47:21 -0800 (PST) X-Received: by 10.50.79.193 with SMTP id l1mr26284133igx.30.1415130440337; Tue, 04 Nov 2014 11:47:20 -0800 (PST) Received: from lists.sourceforge.net (lists.sourceforge.net. [216.34.181.88]) by mx.google.com with ESMTPS id t4si2557429icw.27.2014.11.04.11.47.19 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 04 Nov 2014 11:47:20 -0800 (PST) Received-SPF: pass (google.com: domain of edk2-devel-bounces@lists.sourceforge.net designates 216.34.181.88 as permitted sender) client-ip=216.34.181.88; Received: from localhost ([127.0.0.1] helo=sfs-ml-3.v29.ch3.sourceforge.com) by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1Xlk4J-00058D-Gk; Tue, 04 Nov 2014 19:47:07 +0000 Received: from sog-mx-2.v43.ch3.sourceforge.com ([172.29.43.192] helo=mx.sourceforge.net) by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1Xlk4H-000580-Lk for edk2-devel@lists.sourceforge.net; Tue, 04 Nov 2014 19:47:05 +0000 Received-SPF: pass (sog-mx-2.v43.ch3.sourceforge.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=lersek@redhat.com; helo=mx1.redhat.com; Received: from mx1.redhat.com ([209.132.183.28]) by sog-mx-2.v43.ch3.sourceforge.com with esmtps (TLSv1:AES256-SHA:256) (Exim 4.76) id 1Xlk4F-0002W7-Gf for edk2-devel@lists.sourceforge.net; Tue, 04 Nov 2014 19:47:05 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sA4JfsRY017098 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 4 Nov 2014 14:41:55 -0500 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-81.ams2.redhat.com [10.36.116.81]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sA4Jfptv018091; Tue, 4 Nov 2014 14:41:52 -0500 Message-ID: <54592BFE.80407@redhat.com> Date: Tue, 04 Nov 2014 20:41:50 +0100 From: Laszlo Ersek User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: edk2-devel@lists.sourceforge.net, "Jordan Justen (Intel address)" References: <1414064030-11029-1-git-send-email-lersek@redhat.com> <1414064030-11029-3-git-send-email-lersek@redhat.com> In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Spam-Score: -2.1 (--) X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for sender-domain -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 SPF_PASS SPF: sender matches SPF record -0.6 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain X-Headers-End: 1Xlk4F-0002W7-Gf Subject: Re: [edk2] [PATCH v2 2/9] IntelFrameworkModulePkg: BdsDxe: poll keyboard at front page with zero timeout X-BeenThere: edk2-devel@lists.sourceforge.net X-Mailman-Version: 2.1.9 Precedence: list Reply-To: edk2-devel@lists.sourceforge.net List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , Errors-To: edk2-devel-bounces@lists.sourceforge.net X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: lersek@redhat.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.182 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 Hi Eric On 10/28/14 08:01, Dong, Eric wrote: > Laszlo, > > I refine the code in ShowProgress(). when TimeoutDefault is zero, BDS > also read key. I have verified it works. Please check this new code. > > Thanks, > Eric > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Thursday, October 23, 2014 7:34 PM > To: edk2-devel@lists.sourceforge.net > Subject: [edk2] [PATCH v2 2/9] IntelFrameworkModulePkg: BdsDxe: poll keyboard at front page with zero timeout > > When PlatformBdsEnterFrontPage() is called with a zero TimeoutDefault value, PlatformBdsEnterFrontPage() --> ShowProgress() returns EFI_TIMEOUT immediately, without looking at the keyboard at all. > > This is slightly incorrect, because a keypress might already be pending at that time. Change ShowProgress() such that it polls the keyboard once even if TimeoutDefault equals zero, using a 100 nanosecond event timeout. > > (The UEFI specification explicitly allows a zero TriggerTime argument in > gBS->SetTimer() -- meaning "next timer tick" for TimerRelative --, but > passing zero to gBS->SetTimer() would require a reorganization of the > WaitForSingleEvent() utility function. So let's just use a 100ns timeout > here: it is indistinguishable from "next timer tick" for the purposes of polling the keyboard for an already pending keypress.) > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek > --- > > Notes: > v2: > - new in v2 > > IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c > index 219f691..c093416 100644 > --- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c > +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c > @@ -931,7 +931,15 @@ ShowProgress ( > EFI_GRAPHICS_OUTPUT_BLT_PIXEL Color; > > if (TimeoutDefault == 0) { > - return EFI_TIMEOUT; > + // > + // this amounts to a poll -- 1 * 100ns timeout > + // > + Status = WaitForSingleEvent (gST->ConIn->WaitForKey, 1); > + > + if (Status == EFI_TIMEOUT) { > + return EFI_TIMEOUT; > + } > + return HandleKeyPress (); > } > > DEBUG ((EFI_D_INFO, "\n\nStart showing progress bar... Press any key to stop it! ...Zzz....\n")); > -- > 1.8.3.1 > > > > ------------------------------------------------------------------------------ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > FrontPage.c.patch > > > Index: Universal/BdsDxe/FrontPage.c > =================================================================== > --- Universal/BdsDxe/FrontPage.c (revision 16227) > +++ Universal/BdsDxe/FrontPage.c (working copy) > @@ -891,64 +891,62 @@ > EFI_GRAPHICS_OUTPUT_BLT_PIXEL Background; > EFI_GRAPHICS_OUTPUT_BLT_PIXEL Color; > > - if (TimeoutDefault == 0) { > - return EFI_TIMEOUT; > - } > + if (TimeoutDefault != 0) { > + DEBUG ((EFI_D_INFO, "\n\nStart showing progress bar... Press any key to stop it! ...Zzz....\n")); > > - DEBUG ((EFI_D_INFO, "\n\nStart showing progress bar... Press any key to stop it! ...Zzz....\n")); > + SetMem (&Foreground, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0xff); > + SetMem (&Background, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0x0); > + SetMem (&Color, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0xff); > + > + TmpStr = GetStringById (STRING_TOKEN (STR_START_BOOT_OPTION)); > > - SetMem (&Foreground, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0xff); > - SetMem (&Background, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0x0); > - SetMem (&Color, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0xff); > - > - TmpStr = GetStringById (STRING_TOKEN (STR_START_BOOT_OPTION)); > - > - if (!FeaturePcdGet(PcdBootlogoOnlyEnable)) { > - // > - // Clear the progress status bar first > - // > - if (TmpStr != NULL) { > - PlatformBdsShowProgress (Foreground, Background, TmpStr, Color, 0, 0); > - } > - } > - > - > - TimeoutRemain = TimeoutDefault; > - while (TimeoutRemain != 0) { > - DEBUG ((EFI_D_INFO, "Showing progress bar...Remaining %d second!\n", TimeoutRemain)); > - > - Status = WaitForSingleEvent (gST->ConIn->WaitForKey, ONE_SECOND); > - if (Status != EFI_TIMEOUT) { > - break; > - } > - TimeoutRemain--; > - > if (!FeaturePcdGet(PcdBootlogoOnlyEnable)) { > // > - // Show progress > + // Clear the progress status bar first > // > if (TmpStr != NULL) { > - PlatformBdsShowProgress ( > - Foreground, > - Background, > - TmpStr, > - Color, > - ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault), > - 0 > - ); > + PlatformBdsShowProgress (Foreground, Background, TmpStr, Color, 0, 0); > } > } > - } > - > - if (TmpStr != NULL) { > - gBS->FreePool (TmpStr); > - } > + > > - // > - // Timeout expired > - // > - if (TimeoutRemain == 0) { > - return EFI_TIMEOUT; > + TimeoutRemain = TimeoutDefault; > + while (TimeoutRemain != 0) { > + DEBUG ((EFI_D_INFO, "Showing progress bar...Remaining %d second!\n", TimeoutRemain)); > + > + Status = WaitForSingleEvent (gST->ConIn->WaitForKey, ONE_SECOND); > + if (Status != EFI_TIMEOUT) { > + break; > + } > + TimeoutRemain--; > + > + if (!FeaturePcdGet(PcdBootlogoOnlyEnable)) { > + // > + // Show progress > + // > + if (TmpStr != NULL) { > + PlatformBdsShowProgress ( > + Foreground, > + Background, > + TmpStr, > + Color, > + ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault), > + 0 > + ); > + } > + } > + } > + > + if (TmpStr != NULL) { > + gBS->FreePool (TmpStr); > + } > + > + // > + // Timeout expired > + // > + if (TimeoutRemain == 0) { > + return EFI_TIMEOUT; > + } > } > > // Sorry about the late response. So, first of all, it's easier to see what your patch does if one generates the diff without regard to whitespace (git diff -b). Then it becomes: ------------ SetMem (&Foreground, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0xff); @@ -950,6 +947,7 @@ ShowProgress ( if (TimeoutRemain == 0) { return EFI_TIMEOUT; } + } // // User pressed some key ------------ This is easier to verify. Basically, if TimeoutDefault equals 0, then we jump immediately to gST->ConIn->ReadKeyStroke(), without waiting for the gST->ConIn->WaitForKey event. This way ReadKeyStroke() can return EFI_SUCCESS (same as before, if a keypress has been pending by the time we reach this code), or ReadKeyStroke() can return EFI_NOT_READY (also immediately). This changes the return code for this case from EFI_TIMEOUT to EFI_NOT_READY. However, the only caller, PlatformBdsEnterFrontPage(), handles this status code identically to EFI_TIMEOUT, so it's fine. Reviewed-by: Laszlo Ersek Tested-by: Laszlo Ersek Jordan, if you drop the first two patches form this series, then the rest (patches v2 3/9 to 9/9) apply just as fine, and work as intended (on top of Eric's patch). If you're OK with the series in that form (as in, R-b), then I'll await Eric's commit, and then push patches 3 to 9 on top. Thanks! Laszlo ------------------------------------------------------------------------------ diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c index f69b17c..a0c6381 100644 --- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c @@ -891,10 +891,7 @@ ShowProgress ( EFI_GRAPHICS_OUTPUT_BLT_PIXEL Background; EFI_GRAPHICS_OUTPUT_BLT_PIXEL Color; - if (TimeoutDefault == 0) { - return EFI_TIMEOUT; - } - + if (TimeoutDefault != 0) { DEBUG ((EFI_D_INFO, "\n\nStart showing progress bar... Press any key to stop it! ...Zzz....\n"));