Re: [PATCH] Add support for passing EFI framebuffer into FreeBSD 10.1 and later

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 28.10.2014 21:32, Kris Moore wrote:
> Got another patch for review today.
> 
> This patch adds support for FreeBSD's new VT console driver when booting
> in EFI mode on FreeBSD 10.1 and later.
> 
> Please let me know any comments, or things that need addressed to get
> this merged in.
> 
> Thanks!
> 
> -- Kris Moore PC-BSD Software iXsystems
> 
> 
> patch-grub-fb
> 
> 
> From a9c20c1640cdea0ba902095cb18760f5ebe98875 Mon Sep 17 00:00:00 2001
> From: Kris Moore <address@hidden>
> Date: Tue, 28 Oct 2014 15:59:59 -0400
> Subject: [PATCH 1/2] Add support for passing EFI framebuffer information to
>  FreeBSD kernel on FreeBSD 10.1 and later.
> 
> ---
>  grub-core/loader/i386/bsd.c          | 75 
> ++++++++++++++++++++++++++++++++++++
>  grub-core/video/efi_gop.c            | 14 +++++++
>  include/grub/i386/bsd.h              |  1 +
>  include/grub/i386/freebsd_bootinfo.h | 33 ++++++++++++++++
>  include/grub/video.h                 | 18 +++++++++
>  5 files changed, 141 insertions(+)
>  create mode 100644 include/grub/i386/freebsd_bootinfo.h
> 
> diff --git grub-core/loader/i386/bsd.c grub-core/loader/i386/bsd.c
> index 8f691e0..c685552 100644
> --- grub-core/loader/i386/bsd.c
> +++ grub-core/loader/i386/bsd.c
> @@ -48,6 +48,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  #ifdef GRUB_MACHINE_EFI
>  #include <grub/efi/efi.h>
>  #define NETBSD_DEFAULT_VIDEO_MODE "800x600"
> +#define FREEBSD_DEFAULT_VIDEO_MODE "800x600"
>  #else
>  #define NETBSD_DEFAULT_VIDEO_MODE "text"
>  #include <grub/i386/pc/vbe.h>
> @@ -584,6 +585,63 @@ freebsd_get_zfs (void)
>    grub_free (uuid);
>  }
>  
> +#ifdef GRUB_MACHINE_EFI
> +
> +static grub_err_t
> +grub_freebsd_setup_video (void)
> +{
> +  struct grub_video_mode_info mode_info;
> +  void *framebuffer;
> +  const char *modevar;
> +  struct grub_freebsd_btinfo_framebuf efifb;
> +  grub_err_t err;
> +  grub_video_driver_id_t driv_id;
> +
> +  modevar = grub_env_get ("gfxpayload");
> +
> +  /* Now all graphical modes are acceptable.
> +     May change in future if we have modes without framebuffer.  */
> +  if (modevar && *modevar != 0)
> +    {
> +      char *tmp;
> +      tmp = grub_xasprintf ("%s;" FREEBSD_DEFAULT_VIDEO_MODE, modevar);
> +      if (! tmp)
> +     return grub_errno;
> +      err = grub_video_set_mode (tmp, 0, 0);
> +      grub_free (tmp);
> +    }
> +  else
> +    err = grub_video_set_mode (FREEBSD_DEFAULT_VIDEO_MODE, 0, 0);
> +
> +  if (err)
> +    return err;
> +
> +  driv_id = grub_video_get_driver_id ();
> +  if (driv_id == GRUB_VIDEO_DRIVER_NONE)
> +    return GRUB_ERR_NONE;
> +
> +  err = grub_video_get_info_and_fini (&mode_info, &framebuffer);
> +
> +  if (err)
> +    return err;
> +
> +  efifb.fb_width = mode_info.width;
> +  efifb.fb_height = mode_info.height;
> +  efifb.fb_stride = mode_info.stride;
> +
> +  efifb.fb_addr = (grub_addr_t) framebuffer;
> +  efifb.fb_size = ALIGN_UP (mode_info.pitch * efifb.fb_height, 65536);
> +
> +  efifb.fb_mask_red = mode_info.red_mask;
> +  efifb.fb_mask_green = mode_info.green_mask;
> +  efifb.fb_mask_blue = mode_info.blue_mask;
> +  efifb.fb_mask_reserved = mode_info.reserved_mask;
> +
> +  err = grub_bsd_add_meta ( FREEBSD_MODINFO_METADATA | 
> FREEBSD_BTINFO_FRAMEBUF, &efifb, sizeof (efifb));
> +  return err;
> +}
> +#endif
> +
>  static grub_err_t
>  grub_freebsd_boot (void)
>  {
> @@ -687,6 +745,10 @@ grub_freebsd_boot (void)
>               *(grub_uint32_t *) p_tag = bootflags;
>             break;
>  
> +         case FREEBSD_MODINFO_METADATA | FREEBSD_BTINFO_FRAMEBUF:
> +           grub_memcpy (p_tag, tag->data, tag->len);
> +           break;
> +
>           case FREEBSD_MODINFO_METADATA | FREEBSD_MODINFOMD_ENVP:
>             if (is_64bit)
>               *(grub_uint64_t *) p_tag = bi.environment;
> @@ -716,7 +778,10 @@ grub_freebsd_boot (void)
>  
>    bi.kern_end = kern_end;
>  
> +#ifndef GRUB_MACHINE_EFI
> +  /* Don't set text mode on EFI boot */
>    grub_video_set_mode ("text", 0, 0);
> +#endif
>  
>    if (is_64bit)
>      {
> @@ -1560,6 +1625,16 @@ grub_cmd_freebsd (grub_extcmd_context_t ctxt, int 
> argc, char *argv[])
>                                  FREEBSD_MODINFOMD_KERNEND, &data, len);
>         if (err)
>           return err;
> +
> +#ifdef GRUB_MACHINE_EFI
> +       err = grub_freebsd_setup_video ();
> +       if (err)
> +       {
> +         grub_print_error ();
> +         grub_puts_ (N_("Booting in EFI blind mode"));
> +         grub_errno = GRUB_ERR_NONE;
> +       }
> +#endif
>       }
>        grub_bsd_get_device (&freebsd_biosdev, &unit, &slice, &part);
>        freebsd_zfsguid = 0;
> diff --git grub-core/video/efi_gop.c grub-core/video/efi_gop.c
> index 7f9d1c2..cfe0119 100644
> --- grub-core/video/efi_gop.c
> +++ grub-core/video/efi_gop.c
> @@ -180,6 +180,8 @@ grub_video_gop_fill_real_mode_info (unsigned mode,
>    out->mode_type = GRUB_VIDEO_MODE_TYPE_RGB;
>    out->bpp = grub_video_gop_get_bpp (in);
>    out->bytes_per_pixel = out->bpp >> 3;
> +  out->stride = in->pixels_per_scanline;
> +  out->pixel_format = in->pixel_format;
Please, don't add redundant fields to structs. All this info can be
inferred from available fields in FreeBSD code. As a bonus it will work
with other gfx drivers.
Another point: is this driver usable on non-EFI? It's of particular
interest on coreboot where changing video mode currently requires reflash.

signature.asc
Description: OpenPGP digital signature