[PATCH v2 06/13] efi: Support copy framebuffer

[Simon Glass]

Hi Heinrich,

On Thu, 23 Feb 2023 at 04:58, Heinrich Schuchardt <xypron.glpk at 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 <sjg at 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