[SeaBIOS] [PATCH 5/6] vgabios: generate EDID of preferred resolution

[Kevin O'Connor]

On Sat, Sep 01, 2012 at 03:09:21PM +0900, Hiroshi Miura wrote:
> 
>     Add interface to EDID part.
>     Now bios initialize code can register preferred mode
>     from information of host OS.

I don't really understanding what this patch does.  I think it needs
some cleanup or further clarification.

[...]
> +int vesa_set_prefered_mode(u16 x, u16 y) {
> +
> +    SET_VGA(prefered_mode.x, x);
> +    SET_VGA(prefered_mode.y, y);
> +    return 0;
> +}

There's no reason to build a "struct prefered_mode_info" and define an
init function - one can just delcare two global variables - for
example: u16 PreferedXres VAR16, PreferedYres VAR16;

> +int vesa_get_prefered_mode(u16 seg, void *data) {
> +   struct prefered_mode_info *info = data;
> +   int a, b;
> +
> +    a = GET_GLOBAL(prefered_mode.x);
> +    b = GET_GLOBAL(prefered_mode.y);
> +    SET_FARVAR(seg, info->x, a);
> +    SET_FARVAR(seg, info->y, b);
> +    return 0;
> +}

Dead code.

> +static void get_prefered_resolution(struct edid_detailed_timing *dtd, u16 *x, u16 *y) {
> +    u16 maxx, maxy;
> +
> +    maxx = GET_GLOBAL(prefered_mode.x);
> +    maxy = GET_GLOBAL(prefered_mode.y);
> +    if (maxx == 0 && maxy == 0) {/* unknown, no-preference */
> +        dtd = &vbe_edid_dtd_1920x1080;
> +        maxx = 1920; maxy = 1080;
> +    } else if (maxx >= 1920 && (maxy >= 1080 || maxy == 0)){
> +        dtd = &vbe_edid_dtd_1920x1080;
> +        maxx = 1920; maxy = 1080;
> +    } else if (maxx >= 1600 && (maxy >= 1200 || maxy == 0)){
> +        dtd = &vbe_edid_dtd_1600x1200;
> +        maxx = 1600; maxy = 1200;
> +    } else {
> +        dtd = &vbe_edid_dtd_1280x1024;
> +        maxx = 1280; maxy = 1024;
> +    }
> +    *x = maxx; *y = maxy;
> +}

It's preferable to do this type of scanning at init time and store the
results somewhere in memory.

[...]
>      SET_FARVAR(seg, info->standard_timing[0], VBE_EDID_STD_640x480_85Hz);
>      SET_FARVAR(seg, info->standard_timing[1], VBE_EDID_STD_800x600_85Hz);
> -    SET_FARVAR(seg, info->standard_timing[2], VBE_EDID_STD_1024x768_85Hz);
> -    SET_FARVAR(seg, info->standard_timing[3], VBE_EDID_STD_1280x720_70Hz);
> -    SET_FARVAR(seg, info->standard_timing[4], VBE_EDID_STD_1280x960_60Hz);
> -    SET_FARVAR(seg, info->standard_timing[5], VBE_EDID_STD_1440x900_60Hz);
> -    SET_FARVAR(seg, info->standard_timing[6], VBE_EDID_STD_1600x1200_60Hz);
> -    SET_FARVAR(seg, info->standard_timing[7], VBE_EDID_STD_1680x1050_60Hz);
> -    /* detailed timing blocks */
> -    memcpy_far(seg, &(info->desc[0].dtd), get_global_seg(), &vbe_edid_dtd_1152x864,
> -               sizeof (vbe_edid_dtd_1152x864));
> -    memcpy_far(seg, &(info->desc[1].dtd), get_global_seg(), &vbe_edid_dtd_1280x1024,
> -               sizeof (vbe_edid_dtd_1280x1024));
> +    j=2;
> +    if (max_x >= 1152 && max_x < 1024) {
> +        SET_FARVAR(seg, info->standard_timing[2], VBE_EDID_STD_1152x864_70Hz);
> +        j=3;
> +    } else if (max_x >= 1024 && max_x < 1280) {
> +        SET_FARVAR(seg, info->standard_timing[2], VBE_EDID_STD_1152x864_70Hz);
> +        SET_FARVAR(seg, info->standard_timing[3], VBE_EDID_STD_1024x768_85Hz);
> +        j=4;
> +    } else if (max_x >= 1280 && max_x < 1600) {
> +        SET_FARVAR(seg, info->standard_timing[2], VBE_EDID_STD_1024x768_85Hz);
> +        SET_FARVAR(seg, info->standard_timing[3], VBE_EDID_STD_1280x1024_60Hz);
> +        j=4;
> +    } else if (max_x >= 1600 && max_x < 1920) {
> +        SET_FARVAR(seg, info->standard_timing[3], VBE_EDID_STD_1440x900_60Hz);
> +        SET_FARVAR(seg, info->standard_timing[4], VBE_EDID_STD_1600x900_60Hz);
> +        j=5;

Looks odd - nothing sets info->standard_timing[2].

-Kevin