Message ID | 20220209130907.252-2-dinhngoc.tu@irit.fr (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | x86: Use Multiboot framebuffer | expand |
On 09.02.2022 14:09, Tu Dinh Ngoc wrote: > --- a/xen/arch/x86/boot/reloc.c > +++ b/xen/arch/x86/boot/reloc.c > @@ -156,6 +156,8 @@ static multiboot_info_t *mbi2_reloc(u32 mbi_in) > multiboot_info_t *mbi_out; > u32 ptr; > unsigned int i, mod_idx = 0; > + u64 fbaddr; > + u8 fbtype; Style: Despite adjacent pre-existing code doing so, new code should not be using u<N> anymore, but use uint<N>_t instead. > @@ -254,6 +256,26 @@ static multiboot_info_t *mbi2_reloc(u32 mbi_in) > ++mod_idx; > break; > > + case MULTIBOOT2_TAG_TYPE_FRAMEBUFFER: > + fbaddr = get_mb2_data(tag, framebuffer, framebuffer_addr); > + fbtype = get_mb2_data(tag, framebuffer, framebuffer_type); > + if (fbaddr == 0 || fbtype != MULTIBOOT2_FRAMEBUFFER_TYPE_RGB) Style: Blanks needed immediately inside the parentheses. > + break; > + mbi_out->flags |= MBI_FB; > + mbi_out->fb.addr = fbaddr; > + mbi_out->fb.pitch = get_mb2_data(tag, framebuffer, framebuffer_pitch); > + mbi_out->fb.width = get_mb2_data(tag, framebuffer, framebuffer_width); > + mbi_out->fb.height = get_mb2_data(tag, framebuffer, framebuffer_height); > + mbi_out->fb.bpp = get_mb2_data(tag, framebuffer, framebuffer_bpp); > + mbi_out->fb.type = fbtype; > + mbi_out->fb.red_pos = get_mb2_data(tag, framebuffer, framebuffer_red_field_position); > + mbi_out->fb.red_size = get_mb2_data(tag, framebuffer, framebuffer_red_mask_size); > + mbi_out->fb.green_pos = get_mb2_data(tag, framebuffer, framebuffer_green_field_position); > + mbi_out->fb.green_size = get_mb2_data(tag, framebuffer, framebuffer_green_mask_size); > + mbi_out->fb.blue_pos = get_mb2_data(tag, framebuffer, framebuffer_blue_field_position); > + mbi_out->fb.blue_size = get_mb2_data(tag, framebuffer, framebuffer_blue_mask_size); > + break; Some of these lines are overly long. Much like you don't have frambuffer_ prefixes on multiboot_info_t field names, you could omit them on multiboot2_tag_framebuffer_t, which would reduce line length some already. You're likely still not going to get around wrapping some of the lines. > --- a/xen/include/xen/multiboot.h > +++ b/xen/include/xen/multiboot.h > @@ -42,6 +42,7 @@ > #define MBI_BIOSCONFIG (_AC(1,u) << 8) > #define MBI_LOADERNAME (_AC(1,u) << 9) > #define MBI_APM (_AC(1,u) << 10) > +#define MBI_FB (_AC(1,u) << 11) From what I can see in grub's multiboot.h bit 11 is VBE_INFO, while bit 12 is FRAMEBUFFER_INFO. > @@ -101,6 +102,22 @@ typedef struct { > > /* Valid if flags sets MBI_APM */ > u32 apm_table; > + > + /* Valid if flags sets MBI_FB */ > + struct { > + u64 addr; > + u32 pitch; > + u32 width; > + u32 height; > + u8 bpp; > + u8 type; > + u8 red_pos; > + u8 red_size; > + u8 green_pos; > + u8 green_size; > + u8 blue_pos; > + u8 blue_size; > + } fb; > } multiboot_info_t; What you add is not MB1-compatible (VBE fields come first). Furthermore the addition means mbi_reloc() will suddenly copy more data, which I don't think can be assumed to be fully compatible. Jan
diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c index 4f4039bb7c..01a53d3ae5 100644 --- a/xen/arch/x86/boot/reloc.c +++ b/xen/arch/x86/boot/reloc.c @@ -156,6 +156,8 @@ static multiboot_info_t *mbi2_reloc(u32 mbi_in) multiboot_info_t *mbi_out; u32 ptr; unsigned int i, mod_idx = 0; + u64 fbaddr; + u8 fbtype; ptr = alloc_mem(sizeof(*mbi_out)); mbi_out = _p(ptr); @@ -254,6 +256,26 @@ static multiboot_info_t *mbi2_reloc(u32 mbi_in) ++mod_idx; break; + case MULTIBOOT2_TAG_TYPE_FRAMEBUFFER: + fbaddr = get_mb2_data(tag, framebuffer, framebuffer_addr); + fbtype = get_mb2_data(tag, framebuffer, framebuffer_type); + if (fbaddr == 0 || fbtype != MULTIBOOT2_FRAMEBUFFER_TYPE_RGB) + break; + mbi_out->flags |= MBI_FB; + mbi_out->fb.addr = fbaddr; + mbi_out->fb.pitch = get_mb2_data(tag, framebuffer, framebuffer_pitch); + mbi_out->fb.width = get_mb2_data(tag, framebuffer, framebuffer_width); + mbi_out->fb.height = get_mb2_data(tag, framebuffer, framebuffer_height); + mbi_out->fb.bpp = get_mb2_data(tag, framebuffer, framebuffer_bpp); + mbi_out->fb.type = fbtype; + mbi_out->fb.red_pos = get_mb2_data(tag, framebuffer, framebuffer_red_field_position); + mbi_out->fb.red_size = get_mb2_data(tag, framebuffer, framebuffer_red_mask_size); + mbi_out->fb.green_pos = get_mb2_data(tag, framebuffer, framebuffer_green_field_position); + mbi_out->fb.green_size = get_mb2_data(tag, framebuffer, framebuffer_green_mask_size); + mbi_out->fb.blue_pos = get_mb2_data(tag, framebuffer, framebuffer_blue_field_position); + mbi_out->fb.blue_size = get_mb2_data(tag, framebuffer, framebuffer_blue_mask_size); + break; + case MULTIBOOT2_TAG_TYPE_END: return mbi_out; diff --git a/xen/include/xen/multiboot.h b/xen/include/xen/multiboot.h index d1b43e1183..2d829b5fa7 100644 --- a/xen/include/xen/multiboot.h +++ b/xen/include/xen/multiboot.h @@ -42,6 +42,7 @@ #define MBI_BIOSCONFIG (_AC(1,u) << 8) #define MBI_LOADERNAME (_AC(1,u) << 9) #define MBI_APM (_AC(1,u) << 10) +#define MBI_FB (_AC(1,u) << 11) #ifndef __ASSEMBLY__ @@ -101,6 +102,22 @@ typedef struct { /* Valid if flags sets MBI_APM */ u32 apm_table; + + /* Valid if flags sets MBI_FB */ + struct { + u64 addr; + u32 pitch; + u32 width; + u32 height; + u8 bpp; + u8 type; + u8 red_pos; + u8 red_size; + u8 green_pos; + u8 green_size; + u8 blue_pos; + u8 blue_size; + } fb; } multiboot_info_t; /* The module structure. */ diff --git a/xen/include/xen/multiboot2.h b/xen/include/xen/multiboot2.h index 5acd225044..a86a080038 100644 --- a/xen/include/xen/multiboot2.h +++ b/xen/include/xen/multiboot2.h @@ -177,6 +177,39 @@ typedef struct { u32 mod_end; char cmdline[]; } multiboot2_tag_module_t; + +typedef struct { + u8 red; + u8 green; + u8 blue; +} multiboot2_framebuffer_color_t; + +typedef struct { + u32 type; + u32 size; + u64 framebuffer_addr; + u32 framebuffer_pitch; + u32 framebuffer_width; + u32 framebuffer_height; + u8 framebuffer_bpp; + u8 framebuffer_type; + u16 reserved; + + union { + struct { + u16 framebuffer_palette_num_colors; + multiboot2_framebuffer_color_t framebuffer_palette[0]; + }; + struct { + u8 framebuffer_red_field_position; + u8 framebuffer_red_mask_size; + u8 framebuffer_green_field_position; + u8 framebuffer_green_mask_size; + u8 framebuffer_blue_field_position; + u8 framebuffer_blue_mask_size; + }; + }; +} multiboot2_tag_framebuffer_t; #endif /* __ASSEMBLY__ */ #endif /* __MULTIBOOT2_H__ */