Re: [coreboot] [PATCH] x86: add coreboot framebuffer support

[H. Peter Anvin]

On 09/04/2014 03:25 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann
>
> arch/x86/Kconfig           |  12 +++
>  arch/x86/kernel/Makefile   |   1 +
>  arch/x86/kernel/coreboot.c | 232 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 245 insertions(+)
>  create mode 100644 arch/x86/kernel/coreboot.c
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 778178f..3aeb038 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2372,6 +2372,18 @@ config X86_SYSFB
>  If unsure, say Y.
>
> +config COREBOOT
> bool "coreboot"
> depends on X86_SYSFB
> depends on FB_SIMPLE
> help
>  Add coreboot framebuffer support to the linux kernel.	 Say Y here
>  if you want the linux kernel find and use the firmware framebuffer
>  initialized by coreboot.  Useful when using the linux kernel as
>  coreboot payload.
>  If unsure, or if you don't know what coreboot is, say N.
>
> endmenu

This should NOT be named CONFIG_COREBOOT.  CONFIG_FB_COREBOOT perhaps.
> +struct cb_header {
> u32     signature;
> u32     header_bytes;
> u32     header_checksum;
> u32     table_bytes;
> u32     table_checksum;
> u32     table_entries;
>
> +};
>
> +#define CB_TAG_MAINBOARD        0x0003
> +#define CB_TAG_VERSION          0x0004
> +#define CB_TAG_FORWARD          0x0011
> +#define CB_TAG_FRAMEBUFFER      0x0012
>
> +struct cb_mainboard {
> u8      vendor_idx;
> u8      part_idx;
> char    strings[0];
>
> +};
>
> +struct cb_framebuffer {
> u64     physical_address;
> u32     x_resolution;
> u32     y_resolution;
> u32     bytes_per_line;
> u8      bits_per_pixel;
> u8      red_mask_pos;
> u8      red_mask_size;
> u8      green_mask_pos;
> u8      green_mask_size;
> u8      blue_mask_pos;
> u8      blue_mask_size;
> u8      reserved_mask_pos;
> u8      reserved_mask_size;
>
> +};
>
> +struct cb_entry {
> u32     tag;
> u32     size;
> union {
> char    string[0];
> u64     forward;
> struct cb_mainboard   mb;
> struct cb_framebuffer fb;
> } u;
>
> +};
>
> +#define CB_SIGNATURE 0x4f49424C  /* "LBIO" */

This stuff belongs in a header file.

I'm not a fan of Coreboot having invented its own nonstandard hacks, but
I guess it is pretty much unavoidable.

-hpa