[coreboot] [PATCH 1/2] Add i915 VGA test Driver.

Kevin O'Connor kevin at koconnor.net
Sat Mar 2 04:02:12 CET 2013


On Fri, Mar 01, 2013 at 03:21:32PM +0100, Denis 'GNUtoo' Carikli wrote:
[...]
> --- /dev/null
> +++ b/vgasrc/i915_reg.h
> @@ -0,0 +1,3901 @@
> +/* Copyright 2003 Tungsten Graphics, Inc., Cedar Park, Texas.
> + * All Rights Reserved.
[...]
> +#endif /* _I915_REG_H_ */

That's one impressively large header file.

> index 0000000..ee0f99f
> --- /dev/null
> +++ b/vgasrc/i915vga.c
> @@ -0,0 +1,126 @@
[...]
> +u32 VISIBLE32FLAT
> +i915_readl_32_notrace(u32 addr)
> +{
> +    return readl((void*)addr);
> +}

I think you may be confused on the different modes that the code runs
in.  The vga bios isn't compiled at all in 32 bit mode - so it does
not make sense to mark a function as VISIBLE32FLAT.  When running in
16bit mode (which the vga bios always runs in) then any pointer access
to a non-stack variable must be wrapped in a macro.  So, you don't
want readl(ptr), what you want is GET_FARVAR(0, ptr) - this will
arrange for the proper segment register to be setup and to perform the
variable access relative to that segment register.  Note that you can
access the full 32bit memory space during the setup phase of the vga
bios as the code runs in "big real mode".  Just make sure to use the
proper macros when accessing pointers (to non-stack variables).

[...]
> +int i915vga_setup(void)
> +{
> +    u32 bar;
> +    dprintf(1, "%s: VGA init...\n", __func__);
> +
> +    int ret = stdvga_setup();

Are you sure you need this?  This writes to the legacy vga ports.

[...]
> --- a/vgasrc/vgahw.h
> +++ b/vgasrc/vgahw.h
> @@ -8,6 +8,7 @@
>  #include "bochsvga.h" // bochsvga_set_mode
>  #include "stdvga.h" // stdvga_set_mode
>  #include "geodevga.h" // geodevga_setup
> +#include "i915vga.h" // i915vga_setup
>  
>  static inline struct vgamode_s *vgahw_find_mode(int mode) {
>      if (CONFIG_VGA_CIRRUS)
> @@ -41,6 +42,8 @@ static inline int vgahw_setup(void) {
>          return bochsvga_setup();
>      if (CONFIG_VGA_GEODEGX2 || CONFIG_VGA_GEODELX)
>          return geodevga_setup();
> +    if (CONFIG_VGA_I915)
> +        return i915vga_setup();
>      return stdvga_setup();

FYI, you will assuredly need to override the other functions (like
vgahw_set_mode) as well.

-Kevin



More information about the coreboot mailing list