[PATCH 00/11] video: efi: Improve the EFI-app video console
This does not work on some 64-bit machines since only a 32-bit address for the framebuffer is supported in the VESA structure.
This series corrects this and makes a few other minor, video-related improvements.
Simon Glass (11): efi: video: Move payload code into a function efi: video: Return mode info for app also efi: Support a 64-bit frame buffer address x86: Add a few more items to bdinfo efi: Use a fixed value for the timer clock efi: Support copy framebuffer video: Allow a copy framebuffer with pre-allocated fb video: Remove duplicate cursor-positioning function video: Clear the vidconsole rather than the video efi: Add dhrystone, dcache and scroll lines to app video: Add a note about the broken implementation
arch/x86/dts/efi-x86_app.dts | 1 + arch/x86/lib/bdinfo.c | 4 + arch/x86/lib/fsp/fsp_graphics.c | 2 +- cmd/cls.c | 20 +++-- configs/efi-x86_app64_defconfig | 3 + drivers/pci/pci_rom.c | 10 ++- drivers/timer/tsc_timer.c | 4 + drivers/video/coreboot.c | 2 +- drivers/video/efi.c | 125 +++++++++++++++++++++--------- drivers/video/vidconsole-uclass.c | 48 +++++------- drivers/video/video-uclass.c | 32 ++++++-- include/vesa.h | 16 +++- include/video.h | 2 + include/video_console.h | 9 +++ 14 files changed, 193 insertions(+), 85 deletions(-)
Put this into a function, as we have done for the app implementation. Comment both functions. FOr now the app still does not access it correctly.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/video/efi.c | 83 +++++++++++++++++++++++++++++++-------------- 1 file changed, 57 insertions(+), 26 deletions(-)
diff --git a/drivers/video/efi.c b/drivers/video/efi.c index b11e42c0ebf..fc37a68b376 100644 --- a/drivers/video/efi.c +++ b/drivers/video/efi.c @@ -50,6 +50,15 @@ static void efi_find_pixel_bits(u32 mask, u8 *pos, u8 *size) *size = len; }
+/** + * get_mode_info() - Ask EFI for the mode information + * + * Gets info from the graphics-output protocol + * + * @vesa: Place to put the mode information + * Returns: 0 if OK, -ENOSYS if boot services are not available, -ENOTSUPP if + * the protocol is not supported by EFI + */ static int get_mode_info(struct vesa_mode_info *vesa) { efi_guid_t efi_gop_guid = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID; @@ -72,32 +81,54 @@ static int get_mode_info(struct vesa_mode_info *vesa) return 0; }
+/** + * get_mode_from_entry() - Obtain fb info from the EFIET_GOP_MODE payload entry + * + * This gets the mode information provided by the stub to the payload and puts + * it into a vesa structure. It also returns the mode information. + * + * @vesa: Place to put the mode information + * @infop: Returns a pointer to the mode info + * Returns: 0 if OK, -ve on error + */ +static int get_mode_from_entry(struct vesa_mode_info *vesa, + struct efi_gop_mode_info **infop) +{ + struct efi_gop_mode *mode; + int size; + int ret; + + ret = efi_info_get(EFIET_GOP_MODE, (void **)&mode, &size); + if (ret) { + printf("EFI graphics output entry not found\n"); + return ret; + } + vesa->phys_base_ptr = mode->fb_base; + vesa->x_resolution = mode->info->width; + vesa->y_resolution = mode->info->height; + *infop = mode->info; + + return 0; +} + static int save_vesa_mode(struct vesa_mode_info *vesa) { - struct efi_entry_gopmode *mode; const struct efi_framebuffer *fbinfo; - int size; + struct efi_gop_mode_info *info; int ret;
- if (IS_ENABLED(CONFIG_EFI_APP)) { + if (IS_ENABLED(CONFIG_EFI_APP)) ret = get_mode_info(vesa); - if (ret) { - printf("EFI graphics output protocol not found\n"); - return -ENXIO; - } - } else { - ret = efi_info_get(EFIET_GOP_MODE, (void **)&mode, &size); - if (ret == -ENOENT) { - printf("EFI graphics output protocol mode not found\n"); - return -ENXIO; - } - vesa->phys_base_ptr = mode->fb_base; - vesa->x_resolution = mode->info->width; - vesa->y_resolution = mode->info->height; + else + ret = get_mode_from_entry(vesa, &info); + if (ret) { + printf("EFI graphics output protocol not found (err=%dE)\n", + ret); + return ret; }
- if (mode->info->pixel_format < EFI_GOT_BITMASK) { - fbinfo = &efi_framebuffer_format_map[mode->info->pixel_format]; + if (info->pixel_format < EFI_GOT_BITMASK) { + fbinfo = &efi_framebuffer_format_map[info->pixel_format]; vesa->red_mask_size = fbinfo->red.size; vesa->red_mask_pos = fbinfo->red.pos; vesa->green_mask_size = fbinfo->green.size; @@ -108,29 +139,29 @@ static int save_vesa_mode(struct vesa_mode_info *vesa) vesa->reserved_mask_pos = fbinfo->rsvd.pos;
vesa->bits_per_pixel = 32; - vesa->bytes_per_scanline = mode->info->pixels_per_scanline * 4; - } else if (mode->info->pixel_format == EFI_GOT_BITMASK) { - efi_find_pixel_bits(mode->info->pixel_bitmask[0], + vesa->bytes_per_scanline = info->pixels_per_scanline * 4; + } else if (info->pixel_format == EFI_GOT_BITMASK) { + efi_find_pixel_bits(info->pixel_bitmask[0], &vesa->red_mask_pos, &vesa->red_mask_size); - efi_find_pixel_bits(mode->info->pixel_bitmask[1], + efi_find_pixel_bits(info->pixel_bitmask[1], &vesa->green_mask_pos, &vesa->green_mask_size); - efi_find_pixel_bits(mode->info->pixel_bitmask[2], + efi_find_pixel_bits(info->pixel_bitmask[2], &vesa->blue_mask_pos, &vesa->blue_mask_size); - efi_find_pixel_bits(mode->info->pixel_bitmask[3], + efi_find_pixel_bits(info->pixel_bitmask[3], &vesa->reserved_mask_pos, &vesa->reserved_mask_size); vesa->bits_per_pixel = vesa->red_mask_size + vesa->green_mask_size + vesa->blue_mask_size + vesa->reserved_mask_size; - vesa->bytes_per_scanline = (mode->info->pixels_per_scanline * + vesa->bytes_per_scanline = (info->pixels_per_scanline * vesa->bits_per_pixel) / 8; } else { debug("efi set unknown framebuffer format: %d\n", - mode->info->pixel_format); + info->pixel_format); return -EINVAL; }
The mode info is currently not initialised for the app. Fix this by returning it from the function.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/video/efi.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/video/efi.c b/drivers/video/efi.c index fc37a68b376..0479f207032 100644 --- a/drivers/video/efi.c +++ b/drivers/video/efi.c @@ -56,10 +56,12 @@ static void efi_find_pixel_bits(u32 mask, u8 *pos, u8 *size) * Gets info from the graphics-output protocol * * @vesa: Place to put the mode information + * @infop: Returns a pointer to the mode info * Returns: 0 if OK, -ENOSYS if boot services are not available, -ENOTSUPP if * the protocol is not supported by EFI */ -static int get_mode_info(struct vesa_mode_info *vesa) +static int get_mode_info(struct vesa_mode_info *vesa, + struct efi_gop_mode_info **infop) { efi_guid_t efi_gop_guid = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID; struct efi_boot_services *boot = efi_get_boot(); @@ -77,6 +79,7 @@ static int get_mode_info(struct vesa_mode_info *vesa) vesa->phys_base_ptr = mode->fb_base; vesa->x_resolution = mode->info->width; vesa->y_resolution = mode->info->height; + *infop = mode->info;
return 0; } @@ -118,7 +121,7 @@ static int save_vesa_mode(struct vesa_mode_info *vesa) int ret;
if (IS_ENABLED(CONFIG_EFI_APP)) - ret = get_mode_info(vesa); + ret = get_mode_info(vesa, &info); else ret = get_mode_from_entry(vesa, &info); if (ret) {
The current vesa structure only provides a 32-bit value for the frame buffer. Many modern machines use an address outside the range.
It is still useful to have this common struct, but add a separate frame-buffer address as well.
Add a comment for vesa_setup_video_priv() while we are here.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/lib/fsp/fsp_graphics.c | 2 +- drivers/pci/pci_rom.c | 10 ++++++---- drivers/video/coreboot.c | 2 +- drivers/video/efi.c | 34 +++++++++++++++++++++------------ include/vesa.h | 16 +++++++++++++++- 5 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/arch/x86/lib/fsp/fsp_graphics.c b/arch/x86/lib/fsp/fsp_graphics.c index b07c666caf7..2bcc49f6051 100644 --- a/arch/x86/lib/fsp/fsp_graphics.c +++ b/arch/x86/lib/fsp/fsp_graphics.c @@ -106,7 +106,7 @@ static int fsp_video_probe(struct udevice *dev) vesa->phys_base_ptr = dm_pci_read_bar32(dev, 2); gd->fb_base = vesa->phys_base_ptr;
- ret = vesa_setup_video_priv(vesa, uc_priv, plat); + ret = vesa_setup_video_priv(vesa, vesa->phys_base_ptr, uc_priv, plat); if (ret) goto err;
diff --git a/drivers/pci/pci_rom.c b/drivers/pci/pci_rom.c index 4eb4f967446..16a5491d20e 100644 --- a/drivers/pci/pci_rom.c +++ b/drivers/pci/pci_rom.c @@ -325,7 +325,7 @@ err: return ret; }
-int vesa_setup_video_priv(struct vesa_mode_info *vesa, +int vesa_setup_video_priv(struct vesa_mode_info *vesa, u64 fb, struct video_priv *uc_priv, struct video_uc_plat *plat) { @@ -348,9 +348,9 @@ int vesa_setup_video_priv(struct vesa_mode_info *vesa,
/* Use double buffering if enabled */ if (IS_ENABLED(CONFIG_VIDEO_COPY) && plat->base) - plat->copy_base = vesa->phys_base_ptr; + plat->copy_base = fb; else - plat->base = vesa->phys_base_ptr; + plat->base = fb; log_debug("base = %lx, copy_base = %lx\n", plat->base, plat->copy_base); plat->size = vesa->bytes_per_scanline * vesa->y_resolution;
@@ -377,7 +377,9 @@ int vesa_setup_video(struct udevice *dev, int (*int15_handler)(void)) return ret; }
- ret = vesa_setup_video_priv(&mode_info.vesa, uc_priv, plat); + ret = vesa_setup_video_priv(&mode_info.vesa, + mode_info.vesa.phys_base_ptr, uc_priv, + plat); if (ret) { if (ret == -ENFILE) { /* diff --git a/drivers/video/coreboot.c b/drivers/video/coreboot.c index d2d87c75c89..c586475e41e 100644 --- a/drivers/video/coreboot.c +++ b/drivers/video/coreboot.c @@ -57,7 +57,7 @@ static int coreboot_video_probe(struct udevice *dev) goto err; }
- ret = vesa_setup_video_priv(vesa, uc_priv, plat); + ret = vesa_setup_video_priv(vesa, vesa->phys_base_ptr, uc_priv, plat); if (ret) { ret = log_msg_ret("setup", ret); goto err; diff --git a/drivers/video/efi.c b/drivers/video/efi.c index 0479f207032..169637c2882 100644 --- a/drivers/video/efi.c +++ b/drivers/video/efi.c @@ -5,6 +5,8 @@ * EFI framebuffer driver based on GOP */
+#define LOG_CATEGORY LOGC_EFI + #include <common.h> #include <dm.h> #include <efi_api.h> @@ -56,11 +58,12 @@ static void efi_find_pixel_bits(u32 mask, u8 *pos, u8 *size) * Gets info from the graphics-output protocol * * @vesa: Place to put the mode information + * @fbp: Returns the address of the frame buffer * @infop: Returns a pointer to the mode info * Returns: 0 if OK, -ENOSYS if boot services are not available, -ENOTSUPP if * the protocol is not supported by EFI */ -static int get_mode_info(struct vesa_mode_info *vesa, +static int get_mode_info(struct vesa_mode_info *vesa, u64 *fbp, struct efi_gop_mode_info **infop) { efi_guid_t efi_gop_guid = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID; @@ -74,9 +77,13 @@ static int get_mode_info(struct vesa_mode_info *vesa, ret = boot->locate_protocol(&efi_gop_guid, NULL, (void **)&gop); if (ret) return log_msg_ret("prot", -ENOTSUPP); - mode = gop->mode; + log_debug("maxmode %u, mode %u, info %p, size %lx, fb %lx, fb_size %lx\n", + mode->max_mode, mode->mode, mode->info, mode->info_size, + (ulong)mode->fb_base, (ulong)mode->fb_size); + vesa->phys_base_ptr = mode->fb_base; + *fbp = mode->fb_base; vesa->x_resolution = mode->info->width; vesa->y_resolution = mode->info->height; *infop = mode->info; @@ -91,10 +98,11 @@ static int get_mode_info(struct vesa_mode_info *vesa, * it into a vesa structure. It also returns the mode information. * * @vesa: Place to put the mode information + * @fbp: Returns the address of the frame buffer * @infop: Returns a pointer to the mode info * Returns: 0 if OK, -ve on error */ -static int get_mode_from_entry(struct vesa_mode_info *vesa, +static int get_mode_from_entry(struct vesa_mode_info *vesa, u64 *fbp, struct efi_gop_mode_info **infop) { struct efi_gop_mode *mode; @@ -107,6 +115,7 @@ static int get_mode_from_entry(struct vesa_mode_info *vesa, return ret; } vesa->phys_base_ptr = mode->fb_base; + *fbp = mode->fb_base; vesa->x_resolution = mode->info->width; vesa->y_resolution = mode->info->height; *infop = mode->info; @@ -114,16 +123,17 @@ static int get_mode_from_entry(struct vesa_mode_info *vesa, return 0; }
-static int save_vesa_mode(struct vesa_mode_info *vesa) +static int save_vesa_mode(struct vesa_mode_info *vesa, u64 *fbp) { const struct efi_framebuffer *fbinfo; struct efi_gop_mode_info *info; int ret;
+ printf("start\n"); if (IS_ENABLED(CONFIG_EFI_APP)) - ret = get_mode_info(vesa, &info); + ret = get_mode_info(vesa, fbp, &info); else - ret = get_mode_from_entry(vesa, &info); + ret = get_mode_from_entry(vesa, fbp, &info); if (ret) { printf("EFI graphics output protocol not found (err=%dE)\n", ret); @@ -163,8 +173,7 @@ static int save_vesa_mode(struct vesa_mode_info *vesa) vesa->bytes_per_scanline = (info->pixels_per_scanline * vesa->bits_per_pixel) / 8; } else { - debug("efi set unknown framebuffer format: %d\n", - info->pixel_format); + log_err("Unknown framebuffer format: %d\n", info->pixel_format); return -EINVAL; }
@@ -176,19 +185,20 @@ static int efi_video_probe(struct udevice *dev) struct video_uc_plat *plat = dev_get_uclass_plat(dev); struct video_priv *uc_priv = dev_get_uclass_priv(dev); struct vesa_mode_info *vesa = &mode_info.vesa; + u64 fb; int ret;
/* Initialize vesa_mode_info structure */ - ret = save_vesa_mode(vesa); + ret = save_vesa_mode(vesa, &fb); if (ret) goto err;
- ret = vesa_setup_video_priv(vesa, uc_priv, plat); + ret = vesa_setup_video_priv(vesa, fb, uc_priv, plat); if (ret) goto err;
- printf("Video: %dx%dx%d\n", uc_priv->xsize, uc_priv->ysize, - vesa->bits_per_pixel); + printf("Video: %dx%dx%d @ %lx\n", uc_priv->xsize, uc_priv->ysize, + vesa->bits_per_pixel, (ulong)fb);
return 0;
diff --git a/include/vesa.h b/include/vesa.h index a42c1796863..98112208114 100644 --- a/include/vesa.h +++ b/include/vesa.h @@ -108,7 +108,21 @@ extern struct vesa_state mode_info;
struct video_priv; struct video_uc_plat; -int vesa_setup_video_priv(struct vesa_mode_info *vesa, + +/** + * vesa_setup_video_priv() - Set up a video device using VESA information + * + * The vesa struct is used by various x86 drivers, so this is a common function + * to use it to set up the video. + * + * @vesa: Vesa information to use (vesa->phys_base_ptr is ignored) + * @fb: Frame buffer address (since vesa->phys_base_ptr is only 32 bits) + * @uc_priv: Video device's uclass-private information + * @plat: Video devices's uclass-private platform data + * @return 0 if OK, -ENXIO if the x resolution is 0, -EEPROTONOSUPPORT if the + * pixel format is not supported + */ +int vesa_setup_video_priv(struct vesa_mode_info *vesa, u64 fb, struct video_priv *uc_priv, struct video_uc_plat *plat); int vesa_setup_video(struct udevice *dev, int (*int15_handler)(void));
Add the timer and vendor/model information.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/lib/bdinfo.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/x86/lib/bdinfo.c b/arch/x86/lib/bdinfo.c index 0cb79b01bd3..896de37dce4 100644 --- a/arch/x86/lib/bdinfo.c +++ b/arch/x86/lib/bdinfo.c @@ -16,6 +16,10 @@ DECLARE_GLOBAL_DATA_PTR; void arch_print_bdinfo(void) { bdinfo_print_num_l("prev table", gd->arch.table); + bdinfo_print_num_l("clock_rate", gd->arch.clock_rate); + bdinfo_print_num_l("tsc_base", gd->arch.tsc_base); + bdinfo_print_num_l("vendor", gd->arch.x86_vendor); + bdinfo_print_num_l("model", gd->arch.x86_model);
if (IS_ENABLED(CONFIG_EFI_STUB)) efi_show_bdinfo();
It is not yet clear how to read the timer via EFI. The current value seems much too high on a Framework laptop I tried. Adjust it to a lower hard-coded value for now.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/timer/tsc_timer.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c index 50ab4e86dca..44c6cc363e0 100644 --- a/drivers/timer/tsc_timer.c +++ b/drivers/timer/tsc_timer.c @@ -404,6 +404,10 @@ static void tsc_timer_ensure_setup(bool early) if (!gd->arch.clock_rate) { unsigned long fast_calibrate;
+ if (IS_ENABLED(CONFIG_EFI_APP)) { + fast_calibrate = 2750; + goto done; + } fast_calibrate = native_calibrate_tsc(); if (fast_calibrate) goto done;
Add support for this to EFI in case it becomes useful. At present it just slows things down. Enable CONFIG_VIDEO_COPY to turn it on.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/dts/efi-x86_app.dts | 1 + drivers/video/efi.c | 11 +++++++++++ 2 files changed, 12 insertions(+)
diff --git a/arch/x86/dts/efi-x86_app.dts b/arch/x86/dts/efi-x86_app.dts index a5316e2a1a7..7afa3d72d57 100644 --- a/arch/x86/dts/efi-x86_app.dts +++ b/arch/x86/dts/efi-x86_app.dts @@ -27,6 +27,7 @@ }; efi-fb { compatible = "efi-fb"; + u-boot,dm-pre-reloc; };
}; diff --git a/drivers/video/efi.c b/drivers/video/efi.c index 169637c2882..9135a8e8187 100644 --- a/drivers/video/efi.c +++ b/drivers/video/efi.c @@ -207,6 +207,16 @@ err: return ret; }
+static int efi_video_bind(struct udevice *dev) +{ + struct video_uc_plat *plat = dev_get_uclass_plat(dev); + + /* Use a 16MB frame buffer in case VIDEO_COPY is enabled */ + plat->copy_size = 16 << 20; + + return 0; +} + static const struct udevice_id efi_video_ids[] = { { .compatible = "efi-fb" }, { } @@ -216,5 +226,6 @@ U_BOOT_DRIVER(efi_video) = { .name = "efi_video", .id = UCLASS_VIDEO, .of_match = efi_video_ids, + .bind = efi_video_bind, .probe = efi_video_probe, };
On 2/5/23 20:46, Simon Glass wrote:
Add support for this to EFI in case it becomes useful. At present it just slows things down. Enable CONFIG_VIDEO_COPY to turn it on.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/dts/efi-x86_app.dts | 1 + drivers/video/efi.c | 11 +++++++++++ 2 files changed, 12 insertions(+)
diff --git a/arch/x86/dts/efi-x86_app.dts b/arch/x86/dts/efi-x86_app.dts index a5316e2a1a7..7afa3d72d57 100644 --- a/arch/x86/dts/efi-x86_app.dts +++ b/arch/x86/dts/efi-x86_app.dts @@ -27,6 +27,7 @@ }; efi-fb { compatible = "efi-fb";
u-boot,dm-pre-reloc;};
};
diff --git a/drivers/video/efi.c b/drivers/video/efi.c index 169637c2882..9135a8e8187 100644 --- a/drivers/video/efi.c +++ b/drivers/video/efi.c @@ -207,6 +207,16 @@ err: return ret; }
+static int efi_video_bind(struct udevice *dev) +{
- struct video_uc_plat *plat = dev_get_uclass_plat(dev);
- /* Use a 16MB frame buffer in case VIDEO_COPY is enabled */
- plat->copy_size = 16 << 20;
This does not work with today's display sizes:
3840 * 2160 * 4 = 33177600 7680 * 4320 * 4 = 132710400
You have to determine the buffer size from the mode information.
Best regards
Heinrich
- return 0;
+}
- static const struct udevice_id efi_video_ids[] = { { .compatible = "efi-fb" }, { }
@@ -216,5 +226,6 @@ U_BOOT_DRIVER(efi_video) = { .name = "efi_video", .id = UCLASS_VIDEO, .of_match = efi_video_ids,
- .bind = efi_video_bind, .probe = efi_video_probe, };
Hi Heinrich,
On Fri, 10 Feb 2023 at 05:11, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 2/5/23 20:46, Simon Glass wrote:
Add support for this to EFI in case it becomes useful. At present it just slows things down. Enable CONFIG_VIDEO_COPY to turn it on.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/dts/efi-x86_app.dts | 1 + drivers/video/efi.c | 11 +++++++++++ 2 files changed, 12 insertions(+)
diff --git a/arch/x86/dts/efi-x86_app.dts b/arch/x86/dts/efi-x86_app.dts index a5316e2a1a7..7afa3d72d57 100644 --- a/arch/x86/dts/efi-x86_app.dts +++ b/arch/x86/dts/efi-x86_app.dts @@ -27,6 +27,7 @@ }; efi-fb { compatible = "efi-fb";
u-boot,dm-pre-reloc; };};
diff --git a/drivers/video/efi.c b/drivers/video/efi.c index 169637c2882..9135a8e8187 100644 --- a/drivers/video/efi.c +++ b/drivers/video/efi.c @@ -207,6 +207,16 @@ err: return ret; }
+static int efi_video_bind(struct udevice *dev) +{
struct video_uc_plat *plat = dev_get_uclass_plat(dev);/* Use a 16MB frame buffer in case VIDEO_COPY is enabled */plat->copy_size = 16 << 20;This does not work with today's display sizes:
3840 * 2160 * 4 = 33177600 7680 * 4320 * 4 = 132710400
You have to determine the buffer size from the mode information.
I am trying to do this when the device is bound, i.e. before we know the mode information. How about I just increase it?
BTW I notice that the EFI display is very slow on x86, e.g. with scrolling. Are the mtrrs supposed to be set up?
Regards, Simon
Am 14. Februar 2023 20:48:58 MEZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Fri, 10 Feb 2023 at 05:11, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 2/5/23 20:46, Simon Glass wrote:
Add support for this to EFI in case it becomes useful. At present it just slows things down. Enable CONFIG_VIDEO_COPY to turn it on.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/dts/efi-x86_app.dts | 1 + drivers/video/efi.c | 11 +++++++++++ 2 files changed, 12 insertions(+)
diff --git a/arch/x86/dts/efi-x86_app.dts b/arch/x86/dts/efi-x86_app.dts index a5316e2a1a7..7afa3d72d57 100644 --- a/arch/x86/dts/efi-x86_app.dts +++ b/arch/x86/dts/efi-x86_app.dts @@ -27,6 +27,7 @@ }; efi-fb { compatible = "efi-fb";
u-boot,dm-pre-reloc; };};
diff --git a/drivers/video/efi.c b/drivers/video/efi.c index 169637c2882..9135a8e8187 100644 --- a/drivers/video/efi.c +++ b/drivers/video/efi.c @@ -207,6 +207,16 @@ err: return ret; }
+static int efi_video_bind(struct udevice *dev) +{
struct video_uc_plat *plat = dev_get_uclass_plat(dev);/* Use a 16MB frame buffer in case VIDEO_COPY is enabled */plat->copy_size = 16 << 20;This does not work with today's display sizes:
3840 * 2160 * 4 = 33177600 7680 * 4320 * 4 = 132710400
You have to determine the buffer size from the mode information.
I am trying to do this when the device is bound, i.e. before we know the mode information. How about I just increase it?
Why not access the graphics output protocol here?
You have to look for a graphics output protocol before binding a video device anyway.
Best regards
Heinrich
BTW I notice that the EFI display is very slow on x86, e.g. with scrolling. Are the mtrrs supposed to be set up?
Regards, Simon
At present it is not possible for the video driver to use a pre-allocated frame buffer (such as is done with EFI) with the copy framebuffer. This can be useful to speed up the display.
Adjust the implementation so that copy_size can be set to the required size, with this being allocated if the normal framebuffer size is 0.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/video/video-uclass.c | 32 ++++++++++++++++++++++++-------- include/video.h | 2 ++ 2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index 0b773af2ff3..954ebbce23a 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -78,24 +78,40 @@ void video_set_flush_dcache(struct udevice *dev, bool flush) priv->flush_dcache = flush; }
+static ulong alloc_fb_(ulong align, ulong size, ulong *addrp) +{ + ulong base; + + align = align ? align : 1 << 20; + base = *addrp - size; + base &= ~(align - 1); + size = *addrp - base; + *addrp = base; + + return size; +} + static ulong alloc_fb(struct udevice *dev, ulong *addrp) { struct video_uc_plat *plat = dev_get_uclass_plat(dev); - ulong base, align, size; + ulong size; + + if (!plat->size) { + if (IS_ENABLED(CONFIG_VIDEO_COPY) && plat->copy_size) { + size = alloc_fb_(plat->align, plat->copy_size, addrp); + plat->copy_base = *addrp; + return size; + }
- if (!plat->size) return 0; + }
/* Allow drivers to allocate the frame buffer themselves */ if (plat->base) return 0;
- align = plat->align ? plat->align : 1 << 20; - base = *addrp - plat->size; - base &= ~(align - 1); - plat->base = base; - size = *addrp - base; - *addrp = base; + size = alloc_fb_(plat->align, plat->size, addrp); + plat->base = *addrp;
return size; } diff --git a/include/video.h b/include/video.h index 3f67a93bc93..4d99e5dc27f 100644 --- a/include/video.h +++ b/include/video.h @@ -24,6 +24,7 @@ struct udevice; * @base: Base address of frame buffer, 0 if not yet known * @copy_base: Base address of a hardware copy of the frame buffer. See * CONFIG_VIDEO_COPY. + * @copy_size: Size of copy framebuffer, used if @size is 0 * @hide_logo: Hide the logo (used for testing) */ struct video_uc_plat { @@ -31,6 +32,7 @@ struct video_uc_plat { uint size; ulong base; ulong copy_base; + ulong copy_size; bool hide_logo; };
There are two functions for positioning the cursor on the console. Remove one of them.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/video/vidconsole-uclass.c | 44 +++++++------------------------ 1 file changed, 10 insertions(+), 34 deletions(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index 72a13d30527..9f8b8ebe8ac 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -126,26 +126,14 @@ void vidconsole_set_cursor_pos(struct udevice *dev, int x, int y) priv->ycur = y; }
-/** - * set_cursor_position() - set cursor position - * - * @priv: private data of the video console - * @row: new row - * @col: new column - */ -static void set_cursor_position(struct vidconsole_priv *priv, int row, int col) +void vidconsole_position_cursor(struct udevice *dev, uint col, uint row) { - /* - * Ensure we stay in the bounds of the screen. - */ - if (row >= priv->rows) - row = priv->rows - 1; - if (col >= priv->cols) - col = priv->cols - 1; - - priv->ycur = row * priv->y_charsize; - priv->xcur_frac = priv->xstart_frac + - VID_TO_POS(col * priv->x_charsize); + struct vidconsole_priv *priv = dev_get_uclass_priv(dev); + short x, y; + + x = min_t(short, col, priv->cols - 1) * priv->x_charsize; + y = min_t(short, row, priv->rows - 1) * priv->y_charsize; + vidconsole_set_cursor_pos(dev, x, y); }
/** @@ -192,7 +180,7 @@ static void vidconsole_escape_char(struct udevice *dev, char ch) int row = priv->row_saved; int col = priv->col_saved;
- set_cursor_position(priv, row, col); + vidconsole_position_cursor(dev, col, row); priv->escape = 0; return; } @@ -254,7 +242,7 @@ static void vidconsole_escape_char(struct udevice *dev, char ch) if (row < 0) row = 0; /* Right and bottom overflows are handled in the callee. */ - set_cursor_position(priv, row, col); + vidconsole_position_cursor(dev, col, row); break; } case 'H': @@ -278,7 +266,7 @@ static void vidconsole_escape_char(struct udevice *dev, char ch) if (col) --col;
- set_cursor_position(priv, row, col); + vidconsole_position_cursor(dev, col, row);
break; } @@ -644,15 +632,3 @@ int vidconsole_memmove(struct udevice *dev, void *dst, const void *src, return vidconsole_sync_copy(dev, dst, dst + size); } #endif - -void vidconsole_position_cursor(struct udevice *dev, unsigned col, unsigned row) -{ - struct vidconsole_priv *priv = dev_get_uclass_priv(dev); - struct udevice *vid_dev = dev->parent; - struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev); - short x, y; - - x = min_t(short, col * priv->x_charsize, vid_priv->xsize - 1); - y = min_t(short, row * priv->y_charsize, vid_priv->ysize - 1); - vidconsole_set_cursor_pos(dev, x, y); -}
It is better to clear the console device rather than the video device, since the console has the text display. We also need to reset the cursor position with the console, but not with the video device.
Add a new function to handle this and update the 'cls' command to use it.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/cls.c | 12 ++++++++---- drivers/video/vidconsole-uclass.c | 12 ++++++++++++ include/video_console.h | 9 +++++++++ 3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/cmd/cls.c b/cmd/cls.c index 40a32eeab63..073ba5a6c86 100644 --- a/cmd/cls.c +++ b/cmd/cls.c @@ -8,7 +8,7 @@ #include <common.h> #include <command.h> #include <dm.h> -#include <video.h> +#include <video_console.h>
#define CSI "\x1b["
@@ -19,12 +19,16 @@ static int do_video_clear(struct cmd_tbl *cmdtp, int flag, int argc,
/* Send clear screen and home */ printf(CSI "2J" CSI "1;1H"); - if (IS_ENABLED(CONFIG_VIDEO) && !IS_ENABLED(CONFIG_VIDEO_ANSI)) { - if (uclass_first_device_err(UCLASS_VIDEO, &dev)) + if (IS_ENABLED(CONFIG_VIDEO_ANSI)) + return 0; + + if (IS_ENABLED(CONFIG_VIDEO)) { + if (uclass_first_device_err(UCLASS_VIDEO_CONSOLE, &dev)) return CMD_RET_FAILURE; - if (video_clear(dev)) + if (vidconsole_clear_and_reset(dev)) return CMD_RET_FAILURE; } + return CMD_RET_SUCCESS; }
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index 9f8b8ebe8ac..1bb36bd66c7 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -632,3 +632,15 @@ int vidconsole_memmove(struct udevice *dev, void *dst, const void *src, return vidconsole_sync_copy(dev, dst, dst + size); } #endif + +int vidconsole_clear_and_reset(struct udevice *dev) +{ + int ret; + + ret = video_clear(dev_get_parent(dev)); + if (ret) + return ret; + vidconsole_position_cursor(dev, 0, 0); + + return 0; +} diff --git a/include/video_console.h b/include/video_console.h index 9d2c0f210e4..aebf1d5eb99 100644 --- a/include/video_console.h +++ b/include/video_console.h @@ -276,6 +276,15 @@ int vidconsole_put_string(struct udevice *dev, const char *str); void vidconsole_position_cursor(struct udevice *dev, unsigned col, unsigned row);
+/** + * vidconsole_clear_and_reset() - Clear the console and reset the cursor + * + * The cursor is placed at the start of the console + * + * @dev: vidconsole device to adjust + */ +int vidconsole_clear_and_reset(struct udevice *dev); + /** * vidconsole_set_cursor_pos() - set cursor position *
Add these options to provide some performance measurement, see cache status and slightly speed up the appallingly slow console.
Signed-off-by: Simon Glass sjg@chromium.org ---
configs/efi-x86_app64_defconfig | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/configs/efi-x86_app64_defconfig b/configs/efi-x86_app64_defconfig index 605d49ff8cb..dae48840493 100644 --- a/configs/efi-x86_app64_defconfig +++ b/configs/efi-x86_app64_defconfig @@ -22,6 +22,7 @@ CONFIG_SYS_PBSIZE=532 # CONFIG_CMD_BOOTM is not set CONFIG_CMD_PART=y # CONFIG_CMD_NET is not set +CONFIG_CMD_CACHE=y CONFIG_CMD_TIME=y CONFIG_CMD_EXT2=y CONFIG_CMD_EXT4=y @@ -39,7 +40,9 @@ CONFIG_BOOTFILE="bzImage" CONFIG_USE_ROOTPATH=y CONFIG_REGMAP=y CONFIG_SYSCON=y +CONFIG_CONSOLE_SCROLL_LINES=5 # CONFIG_REGEX is not set +CONFIG_CMD_DHRYSTONE=y # CONFIG_GZIP is not set CONFIG_EFI=y CONFIG_EFI_APP_64BIT=y
The cls command is broken. Previous discussion about this was at [1] and [2]. For now, add a note to the source code.
[1] https://patchwork.ozlabs.org/project/uboot/patch/ 20221022092058.106052-1-heinrich.schuchardt@canonical.com/ [2] https://patchwork.ozlabs.org/project/uboot/patch/ 20230106145243.411626-12-sjg@chromium.org/
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/cls.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/cmd/cls.c b/cmd/cls.c index 073ba5a6c86..1125a3f81bb 100644 --- a/cmd/cls.c +++ b/cmd/cls.c @@ -17,7 +17,13 @@ static int do_video_clear(struct cmd_tbl *cmdtp, int flag, int argc, { __maybe_unused struct udevice *dev;
- /* Send clear screen and home */ + /* + * Send clear screen and home + * + * FIXME(Heinrich Schuchardt xypron.glpk@gmx.de): This should go + * through an API and only be written to serial terminals, not video + * displays + */ printf(CSI "2J" CSI "1;1H"); if (IS_ENABLED(CONFIG_VIDEO_ANSI)) return 0;
Am 5. Februar 2023 20:46:27 MEZ schrieb Simon Glass sjg@chromium.org:
The cls command is broken. Previous discussion about this was at [1] and [2]. For now, add a note to the source code.
[1] https://patchwork.ozlabs.org/project/uboot/patch/ 20221022092058.106052-1-heinrich.schuchardt@canonical.com/ [2] https://patchwork.ozlabs.org/project/uboot/patch/ 20230106145243.411626-12-sjg@chromium.org/
Signed-off-by: Simon Glass sjg@chromium.org
cmd/cls.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/cmd/cls.c b/cmd/cls.c index 073ba5a6c86..1125a3f81bb 100644 --- a/cmd/cls.c +++ b/cmd/cls.c @@ -17,7 +17,13 @@ static int do_video_clear(struct cmd_tbl *cmdtp, int flag, int argc, { __maybe_unused struct udevice *dev;
- /* Send clear screen and home */
- /*
* Send clear screen and home** FIXME(Heinrich Schuchardt <xypron.glpk@gmx.de>): This should go* through an API and only be written to serial terminals, not video* displays*/
All our console drivers understand this escape sequence as we use it in the EFI subsystem. Writing to video is just fine.
Best regards
Heinrich
if (IS_ENABLED(CONFIG_VIDEO_ANSI)) return 0;
Hi Heinrich,
On Sun, 5 Feb 2023 at 14:25, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 5. Februar 2023 20:46:27 MEZ schrieb Simon Glass sjg@chromium.org:
The cls command is broken. Previous discussion about this was at [1] and [2]. For now, add a note to the source code.
[1] https://patchwork.ozlabs.org/project/uboot/patch/ 20221022092058.106052-1-heinrich.schuchardt@canonical.com/ [2] https://patchwork.ozlabs.org/project/uboot/patch/ 20230106145243.411626-12-sjg@chromium.org/
Signed-off-by: Simon Glass sjg@chromium.org
cmd/cls.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/cmd/cls.c b/cmd/cls.c index 073ba5a6c86..1125a3f81bb 100644 --- a/cmd/cls.c +++ b/cmd/cls.c @@ -17,7 +17,13 @@ static int do_video_clear(struct cmd_tbl *cmdtp, int flag, int argc, { __maybe_unused struct udevice *dev;
/* Send clear screen and home */
/** Send clear screen and home** FIXME(Heinrich Schuchardt <xypron.glpk@gmx.de>): This should go* through an API and only be written to serial terminals, not video* displays*/ printf(CSI "2J" CSI "1;1H");All our console drivers understand this escape sequence as we use it in the EFI subsystem. Writing to video is just fine.
With CONFIG_VIDEO_ANSI that might be true, but for boards that want to save the code, it is not.
In any case, my opinion has not changed on this. It needs an API, not ad-hoc printf() all over the place :-)
Regards, Simon
Am 5. Februar 2023 20:46:16 MEZ schrieb Simon Glass sjg@chromium.org:
This does not work on some 64-bit machines since only a 32-bit address for the framebuffer is supported in the VESA structure.
"This" requires some preceding sentence. Please, describe what problem you want to solve.
Best regards
Heinrich
This series corrects this and makes a few other minor, video-related improvements.
Simon Glass (11): efi: video: Move payload code into a function efi: video: Return mode info for app also efi: Support a 64-bit frame buffer address x86: Add a few more items to bdinfo efi: Use a fixed value for the timer clock efi: Support copy framebuffer video: Allow a copy framebuffer with pre-allocated fb video: Remove duplicate cursor-positioning function video: Clear the vidconsole rather than the video efi: Add dhrystone, dcache and scroll lines to app video: Add a note about the broken implementation
arch/x86/dts/efi-x86_app.dts | 1 + arch/x86/lib/bdinfo.c | 4 + arch/x86/lib/fsp/fsp_graphics.c | 2 +- cmd/cls.c | 20 +++-- configs/efi-x86_app64_defconfig | 3 + drivers/pci/pci_rom.c | 10 ++- drivers/timer/tsc_timer.c | 4 + drivers/video/coreboot.c | 2 +- drivers/video/efi.c | 125 +++++++++++++++++++++--------- drivers/video/vidconsole-uclass.c | 48 +++++------- drivers/video/video-uclass.c | 32 ++++++-- include/vesa.h | 16 +++- include/video.h | 2 + include/video_console.h | 9 +++ 14 files changed, 193 insertions(+), 85 deletions(-)
Hi Heinrich,
On Sun, 5 Feb 2023 at 14:15, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 5. Februar 2023 20:46:16 MEZ schrieb Simon Glass sjg@chromium.org:
This does not work on some 64-bit machines since only a 32-bit address for the framebuffer is supported in the VESA structure.
"This" requires some preceding sentence. Please, describe what problem you want to solve.
This refers to the subject: video: efi: Improve the EFI-app video console
If that is the only comment, I can resend the series with that change?
Regards, Simon
Best regards
Heinrich
This series corrects this and makes a few other minor, video-related improvements.
Simon Glass (11): efi: video: Move payload code into a function efi: video: Return mode info for app also efi: Support a 64-bit frame buffer address x86: Add a few more items to bdinfo efi: Use a fixed value for the timer clock efi: Support copy framebuffer video: Allow a copy framebuffer with pre-allocated fb video: Remove duplicate cursor-positioning function video: Clear the vidconsole rather than the video efi: Add dhrystone, dcache and scroll lines to app video: Add a note about the broken implementation
arch/x86/dts/efi-x86_app.dts | 1 + arch/x86/lib/bdinfo.c | 4 + arch/x86/lib/fsp/fsp_graphics.c | 2 +- cmd/cls.c | 20 +++-- configs/efi-x86_app64_defconfig | 3 + drivers/pci/pci_rom.c | 10 ++- drivers/timer/tsc_timer.c | 4 + drivers/video/coreboot.c | 2 +- drivers/video/efi.c | 125 +++++++++++++++++++++--------- drivers/video/vidconsole-uclass.c | 48 +++++------- drivers/video/video-uclass.c | 32 ++++++-- include/vesa.h | 16 +++- include/video.h | 2 + include/video_console.h | 9 +++ 14 files changed, 193 insertions(+), 85 deletions(-)
participants (2)
-
Heinrich Schuchardt -
Simon Glass