From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 15 Apr 2019 08:56:31 -0700 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C44AB3087944; Mon, 15 Apr 2019 15:56:27 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-136.rdu2.redhat.com [10.10.121.136]) by smtp.corp.redhat.com (Postfix) with ESMTP id 52BB819C71; Mon, 15 Apr 2019 15:56:24 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 0/2] MdeModulePkg: Make the screen seamless To: "Gao, Zhichao" , "devel@edk2.groups.io" Cc: "Wang, Jian J" , "Wu, Hao A" , "Ni, Ray" , "Zeng, Star" , "Gao, Liming" , Sean Brogan , Michael Turner , Bret Barkelew References: <20190412031436.19088-1-zhichao.gao@intel.com> <85e3937d-65d6-11f1-f786-4c169eed628b@redhat.com> <3CE959C139B4C44DBEA1810E3AA6F9000B7C1CCF@SHSMSX101.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: Date: Mon, 15 Apr 2019 17:56:23 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <3CE959C139B4C44DBEA1810E3AA6F9000B7C1CCF@SHSMSX101.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.45]); Mon, 15 Apr 2019 15:56:27 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Zhichao, On 04/13/19 09:52, Gao, Zhichao wrote: > > >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Friday, April 12, 2019 4:06 PM >> To: devel@edk2.groups.io; Gao, Zhichao >> Cc: Wang, Jian J ; Wu, Hao A ; >> Ni, Ray ; Zeng, Star ; Gao, Liming >> ; Sean Brogan ; >> Michael Turner ; Bret Barkelew >> >> Subject: Re: [edk2-devel] [PATCH 0/2] MdeModulePkg: Make the screen >> seamless >> >> On 04/12/19 05:14, Gao, Zhichao wrote: >>> For now most platforms support display function at PEI phase. >>> But the conspliter and graphics console driver would clear the screen >>> at BDS connect console phase. Maybe some platforms would show logo in >>> the next or maybe not. For consumers, it looks like the screen >>> flashed. >>> So change the behavior of graphics console devices while connect >>> console devices to maintain seamless screen from PEI. >>> >>> Test has done on MinPlatform Kabylake-RVP3 which support PEI display. >>> >>> Cc: Jian J Wang >>> Cc: Hao Wu >>> Cc: Ray Ni >>> Cc: Star Zeng >>> Cc: Liming Gao >>> Cc: Sean Brogan >>> Cc: Michael Turner >>> Cc: Bret Barkelew >>> >>> Aaron Antone (2): >>> MdeModulePkg/ConSplitterDxe: Optimize the >> ConSplitterTextOutSetMode >>> MdeModulePkg/GraphicsConsoleDxe: Do not clean the screen >>> >>> .../Console/ConSplitterDxe/ConSplitter.c | 34 +++++++++----- >>> .../Console/ConSplitterDxe/ConSplitter.h | 4 +- >>> .../GraphicsConsoleDxe/GraphicsConsole.c | 45 +++++++++---------- >>> 3 files changed, 48 insertions(+), 35 deletions(-) >>> >> >> EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode() is specified to clear the >> screen to black. Is this series compatible with that? > > No. We only consider the console section. You answered my question "is this compatible with...?" with "no", so I first thought you meant, "no, this series is incompatible with the EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode() specification". But reading the rest of your reply, I think your answer is, "EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode() is not affected by this series". Can you confirm that? Because, I think I'd agree with that. This series modifies two drivers, and from those, only ConSplitterDxe *produces* a GOP. Therefore my original question is relevant only for patch#1 (which is the one modifying ConSplitterDxe). And in patch #1, indeed it looks like we don't touch the GOP. (I guess this is also what you mean by "console section" -- that is, the text console functionality.) The rest of the discussion refers to patch #2 / GraphicsConsoleDxe: > There are two pcds to control the graphics output mode > PcdVideoHorizontalResolution and PcdVideoVerticalResolution. Usually > we set them as zero to make the mode to be the max mode the graphics > supported and the graphics output protocol would initialize the mode > to be the max mode in general. If so the SetMode would not be runt. > But that is done in the graphics output driver and the driver is > usually a binary file. So we can't desire that the graphics driver > would set the max mode, that is the graphics output driver's vendor > decided. > In the other condition, these two pcds would set a value and then > graphics output driver would focus to set the mode and clear the > screen. That is controlled by the consumer. By default the two pcds is > initialized as 800 and 600. Because this resolution may be the most > normal resolution and the screen would always be cleared. > > In my opinion, the behavior of graphics output section in this driver > is fine and should not be changed. And also, it is hard for us to > control it because the driver is usually not open source. > The upon results are based on kabylake Rvp3 platform. Maybe I missed > something. Any incorrect, please feel free to point out. The above is all fine, thank you for the answer. However, now having looked at patch#2 in a bit more detail, I have to ask my question again, although a bit differently: In patch #2, we modify the GraphicsConsoleConOutSetMode() function, to omit clearing the display under some conditions. The GraphicsConsoleConOutSetMode() function implements EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.SetMode(). According to the UEFI spec, The SetMode() function sets the output device(s) to the requested mode. On success the device is in the geometry for the requested mode, and the device has been cleared to the current background color with the cursor at (0,0). So, if patch #2 implements a EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.SetMode() member function that does not clear the screen on every call, does it conform to the spec? Now, if the "do not clear the screen" exception is not observable / triggerable by any UEFI driver or UEFI application that is written against the UEFI spec -- in other words, if the exception only applies to platform / DXE modules --, then I guess the exception could be fine. But I'd like to hear others' comments too, and if this behavior is conformant, then this nuance should be mentioned in the commit message and/or in a comment, in patch #2. ... Finally, what about controller disconnect/reconnect in the UEFI shell (or in other UEFI applications)? I assume that the (This->Mode->Mode == -1) exception would apply again, even though at that time we wouldn't be starting up just after the PEI phase. Thanks, Laszlo