Re: [U-Boot] [PATCH 07/11] dm: video: Add driver for coreboot framebuffer device

On 9 October 2016 at 05:14, Bin Meng bmeng.cn@gmail.com wrote:

This adds a DM driver for coreboot framebuffer device.

Signed-off-by: Bin Meng bmeng.cn@gmail.com

drivers/video/Kconfig | 9 ++++++ drivers/video/coreboot.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 drivers/video/coreboot.c

Reviewed-by: Simon Glass sjg@chromium.org

Nice!

Nits below

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index fd26690..554a03f 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -101,6 +101,15 @@ config VIDCONSOLE_AS_LCD to update the environment, the breakage may be confusing for users. This option will be removed around the end of 2016.

+config VIDEO_COREBOOT

•   bool "Enable coreboot framebuffer driver support"
  

•   depends on X86 && SYS_COREBOOT
  

•   help
  

•     Turn on this option to enable a framebuffer driver when U-Boot is
  

•     loaded by coreboot where graphics device is configured by coreboot
  

where the graphics device

•     already. This can in principle be used with any platform that
  

•     coreboot supports.
  

config VIDEO_VESA bool "Enable VESA video driver support" default n diff --git a/drivers/video/coreboot.c b/drivers/video/coreboot.c new file mode 100644 index 0000000..3a94aa1 --- /dev/null +++ b/drivers/video/coreboot.c @@ -0,0 +1,79 @@ +/*

• • Copyright (C) 2016, Bin Meng bmeng.cn@gmail.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <dm.h> +#include <vbe.h> +#include <video.h> +#include <asm/arch/sysinfo.h>

+static int save_vesa_mode(struct cb_framebuffer *fb,

•                     struct vesa_mode_info *vesa)
  

+{

•   /*
  

•    * If there is no framebuffer structure, bail out and keep
  

•    * running on the serial console.
  

•    */
  

•   if (!fb)
  

•           return -ENXIO;
  

•   vesa->x_resolution = fb->x_resolution;
  

•   vesa->y_resolution = fb->y_resolution;
  

•   vesa->bits_per_pixel = fb->bits_per_pixel;
  

•   vesa->bytes_per_scanline = fb->bytes_per_line;
  

•   vesa->phys_base_ptr = fb->physical_address;
  

•   vesa->red_mask_size = fb->red_mask_size;
  

•   vesa->red_mask_pos = fb->red_mask_pos;
  

•   vesa->green_mask_size = fb->green_mask_size;
  

•   vesa->green_mask_pos = fb->green_mask_pos;
  

•   vesa->blue_mask_size = fb->blue_mask_size;
  

•   vesa->blue_mask_pos = fb->blue_mask_pos;
  

•   vesa->reserved_mask_size = fb->reserved_mask_size;
  

•   vesa->reserved_mask_pos = fb->reserved_mask_pos;
  

•   return 0;
  

+}

+static int coreboot_video_probe(struct udevice *dev) +{

•   struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
  

•   struct video_priv *uc_priv = dev_get_uclass_priv(dev);
  

•   struct cb_framebuffer *fb = lib_sysinfo.framebuffer;
  

•   struct vesa_mode_info *vesa = &mode_info.vesa;
  

•   int ret;
  

•   printf("Video: ");
  

•   /* Initialize vesa_mode_info structure */
  

•   ret = save_vesa_mode(fb, vesa);
  

•   if (ret)
  

•           goto err;
  

•   ret = vbe_setup_video_priv(vesa, uc_priv, plat);
  

•   if (ret)
  

•           goto err;
  

•   printf("%dx%dx%d\n", uc_priv->xsize, uc_priv->ysize,
  

•          vesa->bits_per_pixel);
  

Again at some point (later) it would be good to avoid printing info in drivers.

•   return 0;
  

+err:

•   printf("No video mode configured in coreboot!\n");
  

•   return ret;
  

+}

+static const struct udevice_id coreboot_video_ids[] = {

•   { .compatible = "coreboot-fb" },
  

•   { }
  

+};

+U_BOOT_DRIVER(coreboot_video) = {

•   .name   = "coreboot_video",
  

•   .id     = UCLASS_VIDEO,
  

•   .of_match = coreboot_video_ids,
  

•   .probe  = coreboot_video_probe,
  

+};

2.9.2

Regards, Simon