From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Subject: Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer Date: Mon, 18 Jun 2018 11:13:11 +0200 Message-ID: References: <20180617153235.16219-1-hdegoede@redhat.com> <20180617153235.16219-3-hdegoede@redhat.com> <08e59509-d790-b33d-99f4-20410726279f@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Ard Biesheuvel Cc: "open list:EFIFB FRAMEBUFFER DRIVER" , linux-efi , dri-devel , Bartlomiej Zolnierkiewicz List-Id: linux-efi@vger.kernel.org SGksCgpPbiAxOC0wNi0xOCAxMDo0MywgQXJkIEJpZXNoZXV2ZWwgd3JvdGU6Cj4gT24gMTggSnVu ZSAyMDE4IGF0IDEwOjMwLCBIYW5zIGRlIEdvZWRlIDxoZGVnb2VkZUByZWRoYXQuY29tPiB3cm90 ZToKPj4gSGksCj4+Cj4+IE9uIDE4LTA2LTE4IDA5OjM2LCBBcmQgQmllc2hldXZlbCB3cm90ZToK Pj4+Cj4+PiBIYWxsbyBIYW5zLAo+Pj4KPj4+IE9uIDE3IEp1bmUgMjAxOCBhdCAxNzozMiwgSGFu cyBkZSBHb2VkZSA8aGRlZ29lZGVAcmVkaGF0LmNvbT4gd3JvdGU6Cj4+Pj4KPj4+PiBPbiBzeXN0 ZW1zIHdoZXJlIGZiY29uIGlzIGNvbmZpZ3VyZWQgZm9yIGRlZmVycmVkIGNvbnNvbGUgdGFrZW92 ZXIsIHRoZQo+Pj4+IGludGVuZCBpcyBmb3IgdGhlIGZyYW1lYnVmZmVyIHRvIHNob3cgdGhlIGJv b3QgZ3JhcGhpY3MgKGUuZyBhIHZlbmRvcgo+Pj4+IGxvZ28pIHVudGlsIHNvbWUgbWVzc2FnZSAo ZS5nLiBhbiBlcnJvcikgaXMgcHJpbnRlZCBvciBhIGdyYXBoaWNhbAo+Pj4+IHNlc3Npb24gdGFr ZXMgb3Zlci4KPj4+Pgo+Pj4+IFNvbWUgZmlybXdhcmUgaG93ZXZlciByZWxpZXMgb24gdGhlIE9T IHRvIHNob3cgdGhlIGJvb3QgZ3JhcGhpY3MKPj4+PiAoaW5kaWNhdGVkIGJ5IGJncnRfdGFiLnN0 YXR1cyBiZWluZyAwKSBhbmQgdGhlIGJvb3QgZ3JhcGhpY3MgbWF5IGhhdmUKPj4+PiBiZWVuIGRl c3Ryb3llZCBieSBlLmcuIHRoZSBncnViIGJvb3QgbWVudS4KPj4+Pgo+Pj4+IFRoaXMgcGF0Y2gg YWRkcyBzdXBwb3J0IHRvIGVmaWZiIHRvIHNob3cgdGhlIGJvb3QgZ3JhcGhpY3MgYW5kCj4+Pj4g YXV0b21hdGljYWxseSBlbmFibGVzIHRoaXMgd2hlbiBmYmNvbiBpcyBjb25maWd1cmVkIGZvciBk ZWZlcnJlZAo+Pj4+IGNvbnNvbGUgdGFrZW92ZXIuCj4+Pj4KPj4+PiBTaWduZWQtb2ZmLWJ5OiBI YW5zIGRlIEdvZWRlIDxoZGVnb2VkZUByZWRoYXQuY29tPgo+Pj4KPj4+Cj4+PiBJIGhhdmUgdGVz dGVkIHRoaXMgY29kZSBvbiBBUk0gUUVNVS9tYWNoLXZpcnQsIGFuZCB3aXRoIGEgbGl0dGxlIHR3 ZWFrCj4+PiAod2hpY2ggSSB3aWxsIHBvc3Qgc2VwYXJhdGVseSksIHRoZSBjb2RlIHdvcmtzIGFz IGV4cGVjdGVkLCBpLmUuLCBpdAo+Pj4gcmVkcmF3cyB0aGUgYm9vdCBsb2dvIGJhc2VkIG9uIHRo ZSBjb250ZW50cyBvZiB0aGUgQkdSVCB0YWJsZS4KPj4KPj4KPj4gVGhhdCBpcyBncmVhdC4KPj4K Pj4+IEhvd2V2ZXIsIHdoYXQgaXQgZG9lc24ndCBkbyBpcyBjbGVhciB0aGUgc2NyZWVuLCB3aGlj aCBtZWFucyB0aGUgbG9nbwo+Pj4gaXMgZHJhd24gb24gdG9wIG9mIHdoYXRldmVyIHRoZSBib290 IGVudmlyb25tZW50IGxlZnQgYmVoaW5kLCBhbmQgSQo+Pj4gZW5kIHVwIHdpdGggc29tZXRoaW5n IGxpa2UgdGhpcy4KPj4+Cj4+PiBodHRwOi8vcGVvcGxlLmxpbmFyby5vcmcvfmFyZC5iaWVzaGV1 dmVsL21hY2gtdmlydC1iZ3J0LWxvZ28tcmVkcmF3bi5wbmcKPj4KPj4KPj4gSG1tLCBsZXNzIGdy ZWF0LiBJJ20gbm90IHN1cmUgaG93IHRvIGRlYWwgd2l0aCB0aGlzLCBvbiB4ODYgaXQgaXMgbW9y ZQo+PiBvciBsZXNzIGd1YXJhbnRlZWQgdGhhdCB0aGUgc2NyZWVuIGlzIGFscmVhZHkgY2xlYXJl ZCB3aGVuIHdlIGxvYWQgYW5kCj4+IGNsZWFyaW5nIGEgNGsgc2NyZWVuIG1lYW5zIHdyaXRpbmcg YWJvdXQgMzJNQiwgd2hpY2ggSSBndWVzcyB3aXRoIG1vZGVybgo+PiBSQU0gc3BlZWRzIGlzIG5v dCB0aGF0IGJhZCBhY3R1YWxseS4KPj4KPj4gSSBzZWUgdGhhdCB5b3UgZ290IHRoaXMgcGljdHVy ZSBieSBtYW51YWwgYm9vdGluZyBmcm9tIHRoZSBFRkkgc2hlbGwsCj4+IGluIHdoYXQgc3RhdGUg ZG9lcyB0aGUgZmlybXdhcmUgLyBib290bG9hZGVyIG5vcm1hbGx5IGxlYXZlIHRoZQo+PiBmcmFt ZWJ1ZmZlcj8KPiAKPiBVRUZJIGRvZXMgbm90IHVzdWFsbHkgY2xlYXIgdGhlIHNjcmVlbiB3aGVu IGl0IGJvb3RzIHRoZSBkZWZhdWx0Cj4gZW50cnksIHNvIGl0IGlzIGVudGlyZWx5IGRlcGVuZGVu dCBvbiB0aGUgYm9vdCBsb2FkZXIgdGhhdCBydW5zIG5leHQuCj4gSSBndWVzcyBHUlVCIHVzdWFs bHkgY2xlYXJzIHRoZSBzY3JlZW4gdW5jb25kaXRpb25hbGx5PwoKSXQgZGVwZW5kcywgR1JVQiBl aXRoZXIgbGVhdmVzIGl0IGNvbXBsZXRlbHkgYWxvbmUgKGxlYXZpbmcgdGhlCkJHUlQgZ3JhcGhp Y3Mgd2hpY2ggdGhlIGZpcm13YXJlIGhvcGVmdWxseSBoYXMgcHV0IHVwIGFscmVhZHkKaW4gcGxh Y2UpIG9yIGlmIGl0IGFjdHVhbGx5IGRyYXdzIGFueXRoaW5nLCB0aGVuIGl0IGNsZWFycyBpaXQK YmVmb3JlIHN0YXJ0aW5nIHRoZSBrZXJuZWwuCgo+IEluIGFueSBjYXNlLCBJIHRoaW5rIGl0IGlz IHJlYXNvbmFibGUgdG8gY2xlYXIgdGhlIHNjcmVlbiBpZiB5b3UKPiByZWRyYXcgdGhlIGxvZ28s IGJ1dCBJIGRvIHdvbmRlciBpZiBpdCBpcyBzYWZlIHRvIGFzc3VtZSB0aGF0IHRoZQo+IGJhY2tn cm91bmQgY29sb3IgaXMgYmxhY2suCj4gCj4gSW5zdGVhZCwgd2UgbWF5IGRlY2lkZSB0byBjbGVh ciB0aGUgc2NyZWVuIGJlZm9yZSBFeGl0Qm9vdFNlcnZpY2VzKCkKPiBbdXNpbmcgRUZJX1NJTVBM RV9URVhUX09VVFBVVF9QUk9UT0NPTC5DbGVhclNjcmVlbigpXS4KCk9uIG1vc3QgeDg2IG1hY2hp bmVzIChidXQgbm90IGFsbCwgR1JSKSB0aGUgZmlybXdhcmUgaXRzZWxmIHdpbGwKZHJhdyB0aGUg bG9nbyBhbHJlYWR5LiBJIGFjdHVhbGx5IGhhdmUgZ3J1YiBwYXRjaGVzIHBlbmRpbmcgdG8gbWFr ZQppdCBub3QgZG8gYW55IHRleHQtb3V0cHV0IHJlbGF0ZWQgY2FsbHMgYXQgYWxsIHVubGVzcyBp dCBhY3R1YWxseQpoYXMgYSBtZXNzYWdlIC8gbWVudSBpdCB3YW50cyB0byBkaXNwbGF5LgoKQ2Fs bGluZyBFRklfU0lNUExFX1RFWFRfT1VUUFVUX1BST1RPQ09MLkNsZWFyU2NyZWVuKCkgd2lsbCBj YXVzZQphIGJvb3R1cCBzZXF1ZW5jZSBsaWtlIHRoaXM6CgpmaXJtd2FyZSBkcmF3cyBsb2dvCmNs ZWFyc2NyZWVuCnJlZHJhdyBsb2dvCgpXaGljaCB3aWxsIGNhdXNlIGFuIHVnbHkgZmxhc2ggdG8g YmxhY2suIFdoZXJlIGFzIHRoZSBwdXJwb3NlCmlzIHRvIGhhdmUgYSBzbW9vdGggYm9vdCB3aXRo IHRoZSBsb2dvIGJlaW5nIG9uIHNjcmVlbiBhbGwKdGhlIHRpbWUgd2l0aG91dCBhbnkgZmxpY2tl cnMgLyBmbGFzaGVzLgoKSSd2ZSBuZXZlciBzZWVuIGEgbWFjaGluZSB3aGVyZSB0aGUgYmFja2dy b3VuZCBpcyBub3QgYmxhY2ssCnRoZSBiYWNrZ3JvdW5kIG5vdCBiZWluZyBibGFjayB3b3VsZCBh bHNvIGJyZWFrIHRoZSBzcGlubmluZwpkb3RzIHdoaWNoIG1pY3Jvc29mdHMgZHJhd3Mgb24gdG9w IG9mIHRoZSBiYWNrZ3JvdW5kIHdoaWxlCmJvb3RpbmcsIHNvIGEgbWVtc2V0IHRvIDAgc2VlbXMg c2Vuc2libGUgdG8gbWUuCgo+PiAgIEknbSBhc2tpbmcgYmVjYXVzZSBpZiBub3JtYWxseSBpdCBp cyBlaXRoZXIgY2xlYXJlZAo+PiB0byBibGFjaywgb3IgYWxyZWFkeSBzaG93aW5nIHRoZSBsb2dv IEkgd29uZGVyIGlmIHdlIHNob3VsZCB0YWtlIHRoZQo+PiAoc21hbGwpIHBlbmFsdHkgb2YgY2xl YXJpbmcgPwo+Pgo+PiBHaXZlbiB0aGF0IHdlIGFyZSB0YWxraW5nIGFib3V0IG9ubHkgMzIgTUIg SSBjb3VsZCBkbyBhIHYyIHdoaWNoIGp1c3QKPj4gbWVtc2V0cyB0aGUgcmVzdCBvZiB0aGUgc2Ny ZWVuIHRvIDAuCj4+Cj4+IFNvIHdlIGdldDoKPj4KPj4gICAgICAgICAgZm9yICh5PSAwOyB5IDwg aGVpZ2h0OyB5KyspIHsKPj4gICAgICAgICAgICAgICAgICBpZiAobGluZV9wYXJ0X29mX2JncnQp IHsKPj4gICAgICAgICAgICAgICAgICAgICAgICAgIG1lbXNldChsZWZ0LW9mLWJncnQpOwo+PiAg ICAgICAgICAgICAgICAgICAgICAgICAgZHJhd19iZ3J0X2xpbmUoeSk7Cj4+ICAgICAgICAgICAg ICAgICAgICAgICAgICBtZW1zZXQocmlnaHQtb2YtYmdydCk7Cj4+ICAgICAgICAgICAgICAgICAg fSBlbHNlIHsKPj4gICAgICAgICAgICAgICAgICAgICAgICAgIG1lbXNldChsaW5lKTsKPj4gICAg ICAgICAgICAgICAgICB9Cj4+ICAgICAgICAgIH0KPj4KPj4gTm90ZSBJJ3ZlIGRlbGliZXJhdGVs eSBkb25lIHRoZSBpZiBvbiBhIHBlciBsaW5lCj4+IGJhc2UgdG8ga2VlcCB0aGUgYWN0dWFsIGJs aXQgcGFydCBvZiB0aGUgbG9vcAo+PiBlZmZpY2llbnQgYW5kIHdpdGhvdXQgYW55IGV4dHJhIGNv bmRpdGlvbmFscyBpbgo+PiB0aGVyZS4gSSBhbHNvIGRvbid0IHNpbXBseSBmaXJzdCBtZW1zZXQg dGhlIGVudGlyZQo+PiBmYiB0byAwIHRvIGF2b2lkIGEgZmxhc2ggLyB0ZWFyaW5nIGlmIHRoZSBi Z3J0Cj4+IGltYWdlIGlzIGFscmVhZHkgaW4gcGxhY2UgKHdoaWNoIGhhcHBlbnMgb2Z0ZW4gb24K Pj4geDg2KS4KPj4KPj4gSW1wbGVtZW50aW5nIHRoaXMgaXMgZWFzeSBhbmQgYXMgc2FpZCB0aGUg ZXh0cmEgZXhlY3V0aW9uIHRpbWUgc2hvdWxkCj4+IGJlIHF1aXRlIHNtYWxsLCBzdGlsbCBJIHdv bmRlciB3aGF0IG90aGVycyB0aGluayBhYm91dCB0aGlzPwo+Pgo+PiBJJ20gbGVhbmluZyB0b3dh cmRzIGRvaW5nIHRoZSBjbGVhcmluZyAvIG1lbXNldHMgc2luY2UgSSd2ZSBzZWVuCj4+IHNvbWUg ZmlybXdhcmVzIGxlYXZlIHNvbWUgYXJ0aWZhY3RzIGZyb20gbm90IGNvbXBsZXRlbHkgY2xlYXJp bmcKPj4gdGhpbmdzIGxpa2UgYSAiUHJlc3MgRjIgdG8gZW50ZXIgc2V0dXAiIG1lc3NhZ2UsIG1p c3NpbmcgYSBmZXcKPj4gcGl4ZWxzIGFuZCBsZWF2aW5nIHRob3NlIG9uIHNjcmVlbi4KPj4KPiAK PiBJIHRoaW5rIHRoZSBvdmVyaGVhZCBvZiBkb2luZyB0aGUgY2xlYXJpbmcgaXMgbm90IGdvaW5n IHRvIGJlIHRoZQo+IGJvdHRsZW5lY2suIEJ1dCByZWRyYXdpbmcgYSBsb2dvIG9uIGJsYWNrIGJh Y2tncm91bmQgdGhhdCB3YXMgZGVzaWduZWQKPiB0byBiZSBkaXNwbGF5ZWQgb3ZlciBhbm90aGVy IGNvbG9yIGlzIGdvaW5nIHRvIGxvb2sgcmVhbGx5IHVnbHkgLi4uCgpEbyB5b3Uga25vdyBvZiBh bnkgZXhhbXBsZXMgb2YgdGhpcyA/CgpUaGVyZSBzZWVtcyB0byBiZSBubyBrbm93biB3YXkgdG8g Z2V0IHRoZSBiYWNrZ3JvdW5kIGNvbG9yLCBzbyBibGFjayAvCmFsbCAwIHNlZW1zIHRoZSBiZSB0 aGUgYmVzdCBiZXQuCgpJIHdvdWxkIGV4cGVjdCBhbnkgbm9uIGJsYWNrIGJhY2tncm91bmQgbG9n b3MgdG8gc2ltcGx5IGJlIHNjcmVlbgpmaWxsaW5nLgoKUmVnYXJkcywKCkhhbnMKX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcg bGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRl c2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Mon, 18 Jun 2018 09:13:11 +0000 Subject: Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer Message-Id: List-Id: References: <20180617153235.16219-1-hdegoede@redhat.com> <20180617153235.16219-3-hdegoede@redhat.com> <08e59509-d790-b33d-99f4-20410726279f@redhat.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Ard Biesheuvel Cc: "open list:EFIFB FRAMEBUFFER DRIVER" , linux-efi , dri-devel , Bartlomiej Zolnierkiewicz Hi, On 18-06-18 10:43, Ard Biesheuvel wrote: > On 18 June 2018 at 10:30, Hans de Goede wrote: >> Hi, >> >> On 18-06-18 09:36, Ard Biesheuvel wrote: >>> >>> Hallo Hans, >>> >>> On 17 June 2018 at 17:32, Hans de Goede wrote: >>>> >>>> On systems where fbcon is configured for deferred console takeover, the >>>> intend is for the framebuffer to show the boot graphics (e.g a vendor >>>> logo) until some message (e.g. an error) is printed or a graphical >>>> session takes over. >>>> >>>> Some firmware however relies on the OS to show the boot graphics >>>> (indicated by bgrt_tab.status being 0) and the boot graphics may have >>>> been destroyed by e.g. the grub boot menu. >>>> >>>> This patch adds support to efifb to show the boot graphics and >>>> automatically enables this when fbcon is configured for deferred >>>> console takeover. >>>> >>>> Signed-off-by: Hans de Goede >>> >>> >>> I have tested this code on ARM QEMU/mach-virt, and with a little tweak >>> (which I will post separately), the code works as expected, i.e., it >>> redraws the boot logo based on the contents of the BGRT table. >> >> >> That is great. >> >>> However, what it doesn't do is clear the screen, which means the logo >>> is drawn on top of whatever the boot environment left behind, and I >>> end up with something like this. >>> >>> http://people.linaro.org/~ard.biesheuvel/mach-virt-bgrt-logo-redrawn.png >> >> >> Hmm, less great. I'm not sure how to deal with this, on x86 it is more >> or less guaranteed that the screen is already cleared when we load and >> clearing a 4k screen means writing about 32MB, which I guess with modern >> RAM speeds is not that bad actually. >> >> I see that you got this picture by manual booting from the EFI shell, >> in what state does the firmware / bootloader normally leave the >> framebuffer? > > UEFI does not usually clear the screen when it boots the default > entry, so it is entirely dependent on the boot loader that runs next. > I guess GRUB usually clears the screen unconditionally? It depends, GRUB either leaves it completely alone (leaving the BGRT graphics which the firmware hopefully has put up already in place) or if it actually draws anything, then it clears iit before starting the kernel. > In any case, I think it is reasonable to clear the screen if you > redraw the logo, but I do wonder if it is safe to assume that the > background color is black. > > Instead, we may decide to clear the screen before ExitBootServices() > [using EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen()]. On most x86 machines (but not all, GRR) the firmware itself will draw the logo already. I actually have grub patches pending to make it not do any text-output related calls at all unless it actually has a message / menu it wants to display. Calling EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.ClearScreen() will cause a bootup sequence like this: firmware draws logo clearscreen redraw logo Which will cause an ugly flash to black. Where as the purpose is to have a smooth boot with the logo being on screen all the time without any flickers / flashes. I've never seen a machine where the background is not black, the background not being black would also break the spinning dots which microsofts draws on top of the background while booting, so a memset to 0 seems sensible to me. >> I'm asking because if normally it is either cleared >> to black, or already showing the logo I wonder if we should take the >> (small) penalty of clearing ? >> >> Given that we are talking about only 32 MB I could do a v2 which just >> memsets the rest of the screen to 0. >> >> So we get: >> >> for (y= 0; y < height; y++) { >> if (line_part_of_bgrt) { >> memset(left-of-bgrt); >> draw_bgrt_line(y); >> memset(right-of-bgrt); >> } else { >> memset(line); >> } >> } >> >> Note I've deliberately done the if on a per line >> base to keep the actual blit part of the loop >> efficient and without any extra conditionals in >> there. I also don't simply first memset the entire >> fb to 0 to avoid a flash / tearing if the bgrt >> image is already in place (which happens often on >> x86). >> >> Implementing this is easy and as said the extra execution time should >> be quite small, still I wonder what others think about this? >> >> I'm leaning towards doing the clearing / memsets since I've seen >> some firmwares leave some artifacts from not completely clearing >> things like a "Press F2 to enter setup" message, missing a few >> pixels and leaving those on screen. >> > > I think the overhead of doing the clearing is not going to be the > bottleneck. But redrawing a logo on black background that was designed > to be displayed over another color is going to look really ugly ... Do you know of any examples of this ? There seems to be no known way to get the background color, so black / all 0 seems the be the best bet. I would expect any non black background logos to simply be screen filling. Regards, Hans