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 10:30:08 +0200 Message-ID: <08e59509-d790-b33d-99f4-20410726279f@redhat.com> References: <20180617153235.16219-1-hdegoede@redhat.com> <20180617153235.16219-3-hdegoede@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 SGksCgpPbiAxOC0wNi0xOCAwOTozNiwgQXJkIEJpZXNoZXV2ZWwgd3JvdGU6Cj4gSGFsbG8gSGFu cywKPiAKPiBPbiAxNyBKdW5lIDIwMTggYXQgMTc6MzIsIEhhbnMgZGUgR29lZGUgPGhkZWdvZWRl QHJlZGhhdC5jb20+IHdyb3RlOgo+PiBPbiBzeXN0ZW1zIHdoZXJlIGZiY29uIGlzIGNvbmZpZ3Vy ZWQgZm9yIGRlZmVycmVkIGNvbnNvbGUgdGFrZW92ZXIsIHRoZQo+PiBpbnRlbmQgaXMgZm9yIHRo ZSBmcmFtZWJ1ZmZlciB0byBzaG93IHRoZSBib290IGdyYXBoaWNzIChlLmcgYSB2ZW5kb3IKPj4g bG9nbykgdW50aWwgc29tZSBtZXNzYWdlIChlLmcuIGFuIGVycm9yKSBpcyBwcmludGVkIG9yIGEg Z3JhcGhpY2FsCj4+IHNlc3Npb24gdGFrZXMgb3Zlci4KPj4KPj4gU29tZSBmaXJtd2FyZSBob3dl dmVyIHJlbGllcyBvbiB0aGUgT1MgdG8gc2hvdyB0aGUgYm9vdCBncmFwaGljcwo+PiAoaW5kaWNh dGVkIGJ5IGJncnRfdGFiLnN0YXR1cyBiZWluZyAwKSBhbmQgdGhlIGJvb3QgZ3JhcGhpY3MgbWF5 IGhhdmUKPj4gYmVlbiBkZXN0cm95ZWQgYnkgZS5nLiB0aGUgZ3J1YiBib290IG1lbnUuCj4+Cj4+ IFRoaXMgcGF0Y2ggYWRkcyBzdXBwb3J0IHRvIGVmaWZiIHRvIHNob3cgdGhlIGJvb3QgZ3JhcGhp Y3MgYW5kCj4+IGF1dG9tYXRpY2FsbHkgZW5hYmxlcyB0aGlzIHdoZW4gZmJjb24gaXMgY29uZmln dXJlZCBmb3IgZGVmZXJyZWQKPj4gY29uc29sZSB0YWtlb3Zlci4KPj4KPj4gU2lnbmVkLW9mZi1i eTogSGFucyBkZSBHb2VkZSA8aGRlZ29lZGVAcmVkaGF0LmNvbT4KPiAKPiBJIGhhdmUgdGVzdGVk IHRoaXMgY29kZSBvbiBBUk0gUUVNVS9tYWNoLXZpcnQsIGFuZCB3aXRoIGEgbGl0dGxlIHR3ZWFr Cj4gKHdoaWNoIEkgd2lsbCBwb3N0IHNlcGFyYXRlbHkpLCB0aGUgY29kZSB3b3JrcyBhcyBleHBl Y3RlZCwgaS5lLiwgaXQKPiByZWRyYXdzIHRoZSBib290IGxvZ28gYmFzZWQgb24gdGhlIGNvbnRl bnRzIG9mIHRoZSBCR1JUIHRhYmxlLgoKVGhhdCBpcyBncmVhdC4KCj4gSG93ZXZlciwgd2hhdCBp dCBkb2Vzbid0IGRvIGlzIGNsZWFyIHRoZSBzY3JlZW4sIHdoaWNoIG1lYW5zIHRoZSBsb2dvCj4g aXMgZHJhd24gb24gdG9wIG9mIHdoYXRldmVyIHRoZSBib290IGVudmlyb25tZW50IGxlZnQgYmVo aW5kLCBhbmQgSQo+IGVuZCB1cCB3aXRoIHNvbWV0aGluZyBsaWtlIHRoaXMuCj4gCj4gaHR0cDov L3Blb3BsZS5saW5hcm8ub3JnL35hcmQuYmllc2hldXZlbC9tYWNoLXZpcnQtYmdydC1sb2dvLXJl ZHJhd24ucG5nCgpIbW0sIGxlc3MgZ3JlYXQuIEknbSBub3Qgc3VyZSBob3cgdG8gZGVhbCB3aXRo IHRoaXMsIG9uIHg4NiBpdCBpcyBtb3JlCm9yIGxlc3MgZ3VhcmFudGVlZCB0aGF0IHRoZSBzY3Jl ZW4gaXMgYWxyZWFkeSBjbGVhcmVkIHdoZW4gd2UgbG9hZCBhbmQKY2xlYXJpbmcgYSA0ayBzY3Jl ZW4gbWVhbnMgd3JpdGluZyBhYm91dCAzMk1CLCB3aGljaCBJIGd1ZXNzIHdpdGggbW9kZXJuClJB TSBzcGVlZHMgaXMgbm90IHRoYXQgYmFkIGFjdHVhbGx5LgoKSSBzZWUgdGhhdCB5b3UgZ290IHRo aXMgcGljdHVyZSBieSBtYW51YWwgYm9vdGluZyBmcm9tIHRoZSBFRkkgc2hlbGwsCmluIHdoYXQg c3RhdGUgZG9lcyB0aGUgZmlybXdhcmUgLyBib290bG9hZGVyIG5vcm1hbGx5IGxlYXZlIHRoZQpm cmFtZWJ1ZmZlcj8gIEknbSBhc2tpbmcgYmVjYXVzZSBpZiBub3JtYWxseSBpdCBpcyBlaXRoZXIg Y2xlYXJlZAp0byBibGFjaywgb3IgYWxyZWFkeSBzaG93aW5nIHRoZSBsb2dvIEkgd29uZGVyIGlm IHdlIHNob3VsZCB0YWtlIHRoZQooc21hbGwpIHBlbmFsdHkgb2YgY2xlYXJpbmcgPwoKR2l2ZW4g dGhhdCB3ZSBhcmUgdGFsa2luZyBhYm91dCBvbmx5IDMyIE1CIEkgY291bGQgZG8gYSB2MiB3aGlj aCBqdXN0Cm1lbXNldHMgdGhlIHJlc3Qgb2YgdGhlIHNjcmVlbiB0byAwLgoKU28gd2UgZ2V0OgoK CWZvciAoeT0gMDsgeSA8IGhlaWdodDsgeSsrKSB7CgkJaWYgKGxpbmVfcGFydF9vZl9iZ3J0KSB7 CgkJCW1lbXNldChsZWZ0LW9mLWJncnQpOwoJCQlkcmF3X2JncnRfbGluZSh5KTsKCQkJbWVtc2V0 KHJpZ2h0LW9mLWJncnQpOwoJCX0gZWxzZSB7CgkJCW1lbXNldChsaW5lKTsKCQl9Cgl9CgpOb3Rl IEkndmUgZGVsaWJlcmF0ZWx5IGRvbmUgdGhlIGlmIG9uIGEgcGVyIGxpbmUKYmFzZSB0byBrZWVw IHRoZSBhY3R1YWwgYmxpdCBwYXJ0IG9mIHRoZSBsb29wCmVmZmljaWVudCBhbmQgd2l0aG91dCBh bnkgZXh0cmEgY29uZGl0aW9uYWxzIGluCnRoZXJlLiBJIGFsc28gZG9uJ3Qgc2ltcGx5IGZpcnN0 IG1lbXNldCB0aGUgZW50aXJlCmZiIHRvIDAgdG8gYXZvaWQgYSBmbGFzaCAvIHRlYXJpbmcgaWYg dGhlIGJncnQKaW1hZ2UgaXMgYWxyZWFkeSBpbiBwbGFjZSAod2hpY2ggaGFwcGVucyBvZnRlbiBv bgp4ODYpLgoKSW1wbGVtZW50aW5nIHRoaXMgaXMgZWFzeSBhbmQgYXMgc2FpZCB0aGUgZXh0cmEg ZXhlY3V0aW9uIHRpbWUgc2hvdWxkCmJlIHF1aXRlIHNtYWxsLCBzdGlsbCBJIHdvbmRlciB3aGF0 IG90aGVycyB0aGluayBhYm91dCB0aGlzPwoKSSdtIGxlYW5pbmcgdG93YXJkcyBkb2luZyB0aGUg Y2xlYXJpbmcgLyBtZW1zZXRzIHNpbmNlIEkndmUgc2Vlbgpzb21lIGZpcm13YXJlcyBsZWF2ZSBz b21lIGFydGlmYWN0cyBmcm9tIG5vdCBjb21wbGV0ZWx5IGNsZWFyaW5nCnRoaW5ncyBsaWtlIGEg IlByZXNzIEYyIHRvIGVudGVyIHNldHVwIiBtZXNzYWdlLCBtaXNzaW5nIGEgZmV3CnBpeGVscyBh bmQgbGVhdmluZyB0aG9zZSBvbiBzY3JlZW4uCgpSZWdhcmRzLAoKSGFucwpfX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0 CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3Rv cC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Mon, 18 Jun 2018 08:30:08 +0000 Subject: Re: [PATCH 2/2] efifb: Copy the ACPI BGRT boot graphics to the framebuffer Message-Id: <08e59509-d790-b33d-99f4-20410726279f@redhat.com> List-Id: References: <20180617153235.16219-1-hdegoede@redhat.com> <20180617153235.16219-3-hdegoede@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 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? 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. Regards, Hans