Re: [PATCH] x86: add coreboot framebuffer support

["H. Peter Anvin" <hpa@zytor.com>]

On 09/04/2014 03:25 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  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