Hi Heinrich,
On Thu, 23 Feb 2023 at 04:58, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
>
> On 2/22/23 20:12, Simon Glass wrote:
> > Add support for this to EFI in case it becomes useful. At present it just
> > slows things down. You can enable CONFIG_VIDEO_COPY to turn it on.
> >
> > Signed-off-by: Simon Glass <s...@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Obtain copy framebuffer size from EFI instead of using a fixed value
> > - Dropping debugging printf()
> > - use new bootph-xxx tag
> >
> > arch/x86/dts/efi-x86_app.dts | 1 +
> > drivers/video/efi.c | 25 ++++++++++++++++++++++++-
> > 2 files changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/dts/efi-x86_app.dts b/arch/x86/dts/efi-x86_app.dts
> > index 6d843a9820b..59e2e402d5e 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";
> > + bootph-some-ram;
> > };
> >
> > };
> > diff --git a/drivers/video/efi.c b/drivers/video/efi.c
> > index 169637c2882..a8e9282ab87 100644
> > --- a/drivers/video/efi.c
> > +++ b/drivers/video/efi.c
> > @@ -129,7 +129,6 @@ static int save_vesa_mode(struct vesa_mode_info *vesa,
> > u64 *fbp)
> > struct efi_gop_mode_info *info;
> > int ret;
> >
> > - printf("start\n");
> > if (IS_ENABLED(CONFIG_EFI_APP))
> > ret = get_mode_info(vesa, fbp, &info);
> > else
> > @@ -207,6 +206,29 @@ err:
> > return ret;
> > }
> >
> > +static int efi_video_bind(struct udevice *dev)
> > +{
> > + if (IS_ENABLED(CONFIG_VIDEO_COPY)) {
> > + struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> > + struct vesa_mode_info vesa;
>
> struct vesa_mode_info vesa = {0};
>
> see below
>
> > + int ret;
> > + u64 fb;
> > +
> > + /*
> > + * Initialise vesa_mode_info structure so we can figure out
> > the
> > + * required framebuffer size. If something goes wrong, just do
> > + * without a copy framebuffer
> > + */
> > + ret = save_vesa_mode(&vesa, &fb);
> > + if (!ret) {
> > + plat->copy_size = vesa.bytes_per_scanline *
> > + vesa.y_resolution;
>
> If there is no EFI_GRAPHICS_OUTPUT_PROTOCOL, ret = -ENXIO but vesa is
> *not* initialized. You don't want to use random bytes from the stack to
> calculate copy_size and afterwards write into a non-existent framebuffer.
But if ret is -ENXIO then the copy size is not set, right?
>
> If you initialize vesa to zero, you can test if copy_size == 0 and in
> this case return 1 to indicate that binding failed.
If it fails we just don't have a copy buffer. Everything still works OK.
Regards,
Simon
