[PATCH] efi: Correct handling of frame buffer

[Heinrich Schuchardt]

Am 27. August 2023 16:56:51 MESZ schrieb Alper Nebi Yasak <alpernebiyasak at gmail.com>:
>On 2023-08-25 22:28 +03:00, Simon Glass wrote:
>> The efi_gop driver uses private fields from the video uclass to obtain a
>> pointer to the frame buffer. Use the platform data instead.
>> 
>> Check the VIDEO_COPY setting to determine which frame buffer to use. Once
>> the next stage is running (and making use of U-Boot's EFI boot services)
>> U-Boot does not handle copying from priv->fb to the hardware framebuffer,
>
>So far this copying seems to be done case-by-case like calling
>vidconsole_sync_copy() any time vidconsole does something, but the video
>damage tracking series moves that to video_sync(), so this change is
>incompatible with that (it will keep overwriting copy_fb with fb).
>
>> so we must allow EFI to write directly to the hardware framebuffer.
>
>If you want a fix independent of that series, I think the proper
>approach here is having EFI draw to fb as it already does, then copying
>from that to copy_fb at the end of gop_blt_int().

An EFI app can write directly to the framebuffer and mix that with API calls.

blt functions causing a full copy would be much slower than what you get when eliminating the copy fb.

Best regards

Heinrich


>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>> 
>>  lib/efi_loader/efi_gop.c | 12 +++++++-----
>>  1 file changed, 7 insertions(+), 5 deletions(-)
>> 
>> diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
>> index 778b693f983..a09db31eb46 100644
>> --- a/lib/efi_loader/efi_gop.c
>> +++ b/lib/efi_loader/efi_gop.c
>> @@ -10,6 +10,7 @@
>>  #include <efi_loader.h>
>>  #include <log.h>
>>  #include <malloc.h>
>> +#include <mapmem.h>
>>  #include <video.h>
>>  #include <asm/global_data.h>
>>  
>> @@ -467,10 +468,10 @@ efi_status_t efi_gop_register(void)
>>  	struct efi_gop_obj *gopobj;
>>  	u32 bpix, format, col, row;
>>  	u64 fb_base, fb_size;
>> -	void *fb;
>>  	efi_status_t ret;
>>  	struct udevice *vdev;
>>  	struct video_priv *priv;
>> +	struct video_uc_plat *plat;
>>  
>>  	/* We only support a single video output device for now */
>>  	if (uclass_first_device_err(UCLASS_VIDEO, &vdev)) {
>> @@ -483,9 +484,10 @@ efi_status_t efi_gop_register(void)
>>  	format = priv->format;
>>  	col = video_get_xsize(vdev);
>>  	row = video_get_ysize(vdev);
>> -	fb_base = (uintptr_t)priv->fb;
>> -	fb_size = priv->fb_size;
>> -	fb = priv->fb;
>> +
>> +	plat = dev_get_uclass_plat(vdev);
>> +	fb_base = IS_ENABLED(CONFIG_VIDEO_COPY) ? plat->copy_base : plat->base;
>> +	fb_size = plat->size;
>>  
>>  	switch (bpix) {
>>  	case VIDEO_BPP16:
>> @@ -547,7 +549,7 @@ efi_status_t efi_gop_register(void)
>>  	}
>>  	gopobj->info.pixels_per_scanline = col;
>>  	gopobj->bpix = bpix;
>> -	gopobj->fb = fb;
>> +	gopobj->fb = map_sysmem(fb_base, fb_size);
>>  
>>  	return EFI_SUCCESS;
>>  }