| Message ID | 20230222191257.1307427-4-sjg@chromium.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | video: efi: Improve the EFI-app video console | expand |
On 2/22/23 20:12, Simon Glass wrote: > 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> > --- > > (no changes since v1) > > 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 47b6e6e5bcf..f0dfe631490 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 %s/@return/Return:/g cf. https://docs.kernel.org/doc-guide/kernel-doc.html#return-values Best regards Heinrich > + * 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));
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 47b6e6e5bcf..f0dfe631490 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));
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> --- (no changes since v1) 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(-)