From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ard Biesheuvel Subject: Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer Date: Mon, 18 Jun 2018 10:43:38 +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" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <08e59509-d790-b33d-99f4-20410726279f@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Hans de Goede Cc: "open list:EFIFB FRAMEBUFFER DRIVER" , linux-efi , dri-devel , Bartlomiej Zolnierkiewicz List-Id: linux-efi@vger.kernel.org T24gMTggSnVuZSAyMDE4IGF0IDEwOjMwLCBIYW5zIGRlIEdvZWRlIDxoZGVnb2VkZUByZWRoYXQu Y29tPiB3cm90ZToKPiBIaSwKPgo+IE9uIDE4LTA2LTE4IDA5OjM2LCBBcmQgQmllc2hldXZlbCB3 cm90ZToKPj4KPj4gSGFsbG8gSGFucywKPj4KPj4gT24gMTcgSnVuZSAyMDE4IGF0IDE3OjMyLCBI YW5zIGRlIEdvZWRlIDxoZGVnb2VkZUByZWRoYXQuY29tPiB3cm90ZToKPj4+Cj4+PiBPbiBzeXN0 ZW1zIHdoZXJlIGZiY29uIGlzIGNvbmZpZ3VyZWQgZm9yIGRlZmVycmVkIGNvbnNvbGUgdGFrZW92 ZXIsIHRoZQo+Pj4gaW50ZW5kIGlzIGZvciB0aGUgZnJhbWVidWZmZXIgdG8gc2hvdyB0aGUgYm9v dCBncmFwaGljcyAoZS5nIGEgdmVuZG9yCj4+PiBsb2dvKSB1bnRpbCBzb21lIG1lc3NhZ2UgKGUu Zy4gYW4gZXJyb3IpIGlzIHByaW50ZWQgb3IgYSBncmFwaGljYWwKPj4+IHNlc3Npb24gdGFrZXMg b3Zlci4KPj4+Cj4+PiBTb21lIGZpcm13YXJlIGhvd2V2ZXIgcmVsaWVzIG9uIHRoZSBPUyB0byBz aG93IHRoZSBib290IGdyYXBoaWNzCj4+PiAoaW5kaWNhdGVkIGJ5IGJncnRfdGFiLnN0YXR1cyBi ZWluZyAwKSBhbmQgdGhlIGJvb3QgZ3JhcGhpY3MgbWF5IGhhdmUKPj4+IGJlZW4gZGVzdHJveWVk IGJ5IGUuZy4gdGhlIGdydWIgYm9vdCBtZW51Lgo+Pj4KPj4+IFRoaXMgcGF0Y2ggYWRkcyBzdXBw b3J0IHRvIGVmaWZiIHRvIHNob3cgdGhlIGJvb3QgZ3JhcGhpY3MgYW5kCj4+PiBhdXRvbWF0aWNh bGx5IGVuYWJsZXMgdGhpcyB3aGVuIGZiY29uIGlzIGNvbmZpZ3VyZWQgZm9yIGRlZmVycmVkCj4+ PiBjb25zb2xlIHRha2VvdmVyLgo+Pj4KPj4+IFNpZ25lZC1vZmYtYnk6IEhhbnMgZGUgR29lZGUg PGhkZWdvZWRlQHJlZGhhdC5jb20+Cj4+Cj4+Cj4+IEkgaGF2ZSB0ZXN0ZWQgdGhpcyBjb2RlIG9u IEFSTSBRRU1VL21hY2gtdmlydCwgYW5kIHdpdGggYSBsaXR0bGUgdHdlYWsKPj4gKHdoaWNoIEkg d2lsbCBwb3N0IHNlcGFyYXRlbHkpLCB0aGUgY29kZSB3b3JrcyBhcyBleHBlY3RlZCwgaS5lLiwg aXQKPj4gcmVkcmF3cyB0aGUgYm9vdCBsb2dvIGJhc2VkIG9uIHRoZSBjb250ZW50cyBvZiB0aGUg QkdSVCB0YWJsZS4KPgo+Cj4gVGhhdCBpcyBncmVhdC4KPgo+PiBIb3dldmVyLCB3aGF0IGl0IGRv ZXNuJ3QgZG8gaXMgY2xlYXIgdGhlIHNjcmVlbiwgd2hpY2ggbWVhbnMgdGhlIGxvZ28KPj4gaXMg ZHJhd24gb24gdG9wIG9mIHdoYXRldmVyIHRoZSBib290IGVudmlyb25tZW50IGxlZnQgYmVoaW5k LCBhbmQgSQo+PiBlbmQgdXAgd2l0aCBzb21ldGhpbmcgbGlrZSB0aGlzLgo+Pgo+PiBodHRwOi8v cGVvcGxlLmxpbmFyby5vcmcvfmFyZC5iaWVzaGV1dmVsL21hY2gtdmlydC1iZ3J0LWxvZ28tcmVk cmF3bi5wbmcKPgo+Cj4gSG1tLCBsZXNzIGdyZWF0LiBJJ20gbm90IHN1cmUgaG93IHRvIGRlYWwg d2l0aCB0aGlzLCBvbiB4ODYgaXQgaXMgbW9yZQo+IG9yIGxlc3MgZ3VhcmFudGVlZCB0aGF0IHRo ZSBzY3JlZW4gaXMgYWxyZWFkeSBjbGVhcmVkIHdoZW4gd2UgbG9hZCBhbmQKPiBjbGVhcmluZyBh IDRrIHNjcmVlbiBtZWFucyB3cml0aW5nIGFib3V0IDMyTUIsIHdoaWNoIEkgZ3Vlc3Mgd2l0aCBt b2Rlcm4KPiBSQU0gc3BlZWRzIGlzIG5vdCB0aGF0IGJhZCBhY3R1YWxseS4KPgo+IEkgc2VlIHRo YXQgeW91IGdvdCB0aGlzIHBpY3R1cmUgYnkgbWFudWFsIGJvb3RpbmcgZnJvbSB0aGUgRUZJIHNo ZWxsLAo+IGluIHdoYXQgc3RhdGUgZG9lcyB0aGUgZmlybXdhcmUgLyBib290bG9hZGVyIG5vcm1h bGx5IGxlYXZlIHRoZQo+IGZyYW1lYnVmZmVyPwoKVUVGSSBkb2VzIG5vdCB1c3VhbGx5IGNsZWFy IHRoZSBzY3JlZW4gd2hlbiBpdCBib290cyB0aGUgZGVmYXVsdAplbnRyeSwgc28gaXQgaXMgZW50 aXJlbHkgZGVwZW5kZW50IG9uIHRoZSBib290IGxvYWRlciB0aGF0IHJ1bnMgbmV4dC4KSSBndWVz cyBHUlVCIHVzdWFsbHkgY2xlYXJzIHRoZSBzY3JlZW4gdW5jb25kaXRpb25hbGx5PwoKSW4gYW55 IGNhc2UsIEkgdGhpbmsgaXQgaXMgcmVhc29uYWJsZSB0byBjbGVhciB0aGUgc2NyZWVuIGlmIHlv dQpyZWRyYXcgdGhlIGxvZ28sIGJ1dCBJIGRvIHdvbmRlciBpZiBpdCBpcyBzYWZlIHRvIGFzc3Vt ZSB0aGF0IHRoZQpiYWNrZ3JvdW5kIGNvbG9yIGlzIGJsYWNrLgoKSW5zdGVhZCwgd2UgbWF5IGRl Y2lkZSB0byBjbGVhciB0aGUgc2NyZWVuIGJlZm9yZSBFeGl0Qm9vdFNlcnZpY2VzKCkKW3VzaW5n IEVGSV9TSU1QTEVfVEVYVF9PVVRQVVRfUFJPVE9DT0wuQ2xlYXJTY3JlZW4oKV0uCgo+ICBJJ20g YXNraW5nIGJlY2F1c2UgaWYgbm9ybWFsbHkgaXQgaXMgZWl0aGVyIGNsZWFyZWQKPiB0byBibGFj aywgb3IgYWxyZWFkeSBzaG93aW5nIHRoZSBsb2dvIEkgd29uZGVyIGlmIHdlIHNob3VsZCB0YWtl IHRoZQo+IChzbWFsbCkgcGVuYWx0eSBvZiBjbGVhcmluZyA/Cj4KPiBHaXZlbiB0aGF0IHdlIGFy ZSB0YWxraW5nIGFib3V0IG9ubHkgMzIgTUIgSSBjb3VsZCBkbyBhIHYyIHdoaWNoIGp1c3QKPiBt ZW1zZXRzIHRoZSByZXN0IG9mIHRoZSBzY3JlZW4gdG8gMC4KPgo+IFNvIHdlIGdldDoKPgo+ICAg ICAgICAgZm9yICh5PSAwOyB5IDwgaGVpZ2h0OyB5KyspIHsKPiAgICAgICAgICAgICAgICAgaWYg KGxpbmVfcGFydF9vZl9iZ3J0KSB7Cj4gICAgICAgICAgICAgICAgICAgICAgICAgbWVtc2V0KGxl ZnQtb2YtYmdydCk7Cj4gICAgICAgICAgICAgICAgICAgICAgICAgZHJhd19iZ3J0X2xpbmUoeSk7 Cj4gICAgICAgICAgICAgICAgICAgICAgICAgbWVtc2V0KHJpZ2h0LW9mLWJncnQpOwo+ICAgICAg ICAgICAgICAgICB9IGVsc2Ugewo+ICAgICAgICAgICAgICAgICAgICAgICAgIG1lbXNldChsaW5l KTsKPiAgICAgICAgICAgICAgICAgfQo+ICAgICAgICAgfQo+Cj4gTm90ZSBJJ3ZlIGRlbGliZXJh dGVseSBkb25lIHRoZSBpZiBvbiBhIHBlciBsaW5lCj4gYmFzZSB0byBrZWVwIHRoZSBhY3R1YWwg YmxpdCBwYXJ0IG9mIHRoZSBsb29wCj4gZWZmaWNpZW50IGFuZCB3aXRob3V0IGFueSBleHRyYSBj b25kaXRpb25hbHMgaW4KPiB0aGVyZS4gSSBhbHNvIGRvbid0IHNpbXBseSBmaXJzdCBtZW1zZXQg dGhlIGVudGlyZQo+IGZiIHRvIDAgdG8gYXZvaWQgYSBmbGFzaCAvIHRlYXJpbmcgaWYgdGhlIGJn cnQKPiBpbWFnZSBpcyBhbHJlYWR5IGluIHBsYWNlICh3aGljaCBoYXBwZW5zIG9mdGVuIG9uCj4g eDg2KS4KPgo+IEltcGxlbWVudGluZyB0aGlzIGlzIGVhc3kgYW5kIGFzIHNhaWQgdGhlIGV4dHJh IGV4ZWN1dGlvbiB0aW1lIHNob3VsZAo+IGJlIHF1aXRlIHNtYWxsLCBzdGlsbCBJIHdvbmRlciB3 aGF0IG90aGVycyB0aGluayBhYm91dCB0aGlzPwo+Cj4gSSdtIGxlYW5pbmcgdG93YXJkcyBkb2lu ZyB0aGUgY2xlYXJpbmcgLyBtZW1zZXRzIHNpbmNlIEkndmUgc2Vlbgo+IHNvbWUgZmlybXdhcmVz IGxlYXZlIHNvbWUgYXJ0aWZhY3RzIGZyb20gbm90IGNvbXBsZXRlbHkgY2xlYXJpbmcKPiB0aGlu Z3MgbGlrZSBhICJQcmVzcyBGMiB0byBlbnRlciBzZXR1cCIgbWVzc2FnZSwgbWlzc2luZyBhIGZl dwo+IHBpeGVscyBhbmQgbGVhdmluZyB0aG9zZSBvbiBzY3JlZW4uCj4KCkkgdGhpbmsgdGhlIG92 ZXJoZWFkIG9mIGRvaW5nIHRoZSBjbGVhcmluZyBpcyBub3QgZ29pbmcgdG8gYmUgdGhlCmJvdHRs ZW5lY2suIEJ1dCByZWRyYXdpbmcgYSBsb2dvIG9uIGJsYWNrIGJhY2tncm91bmQgdGhhdCB3YXMg ZGVzaWduZWQKdG8gYmUgZGlzcGxheWVkIG92ZXIgYW5vdGhlciBjb2xvciBpcyBnb2luZyB0byBs b29rIHJlYWxseSB1Z2x5IC4uLgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVz a3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9k cmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ard Biesheuvel Date: Mon, 18 Jun 2018 08:43:38 +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: <08e59509-d790-b33d-99f4-20410726279f@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Hans de Goede Cc: "open list:EFIFB FRAMEBUFFER DRIVER" , linux-efi , dri-devel , Bartlomiej Zolnierkiewicz 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? 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()]. > 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 ...