Re: [Xen-devel][PATCH][LINUX] Dynamic modes support for PV xenfb (1of2)

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

On Sun, Dec 30, 2007 at 11:45:11AM -0700, Pat Campbell wrote:
> +static int
> +xenfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
> +{
> +     struct xenfb_info *xenfb_info;
> +
> +     xenfb_info = info->par;
> +
> +     if (!xenfb_info->feature_resize) {
> +             if (var->xres == XENFB_WIDTH && var->yres == XENFB_HEIGHT) {
> +                     return 0;
> +             }
> +             return -EINVAL;
> +     }
> +     if (var->xres == 1024 && var->yres == 768) {
> +             return 0;
> +     }
> +     if (var->xres == 800 && var->yres == 600) {
> +             return 0;
> +     }
> +     if (var->xres == 640 && var->yres == 480) {
> +             return 0;
> +     }
> +     return -EINVAL;
> +}

This validates only 640x480, 800x600, and 1024x768, but further
down you made a change to enable 1280x1024:

>       * PAGE_SIZE / sizeof(*pd) bytes.  With PAGE_SIZE == 4096 and
>       * sizeof(unsigned long) == 4, that's 4 Megs.  Two directory
>       * pages should be enough for a while.
> +     *
> +     * Increased to 3 to support 1280x1024 resolution on a 64bit system
> +     *  (1280*1024*4)/PAGE_SIZE = 1280 pages required
> +     *  PAGE_SIZE/64bit long = 512 pages per page directory
>       */
> -    unsigned long pd[2];
> +    unsigned long pd[3];


I think if we're going to the trouble of making sizes configurable
we should at the very least support all the common regular aspect and
widescreen modes, so that people can run the guest fullscreen at
native resolution. My curent physical graphics card + monitor for 
example supports the following choices via xrandr

  1680x1050 1280x800 1280x1024 1600x1024 1440x900 1400x1050 1360x768 
  1280x960 1280x768 1280x720 1152x864 1024x768 960x600 960x540 896x672
  840x525 832x624 800x600 800x512 720x450 680x384 640x512 640x480 
  640x400 640x384 640x360 576x432 512x384 416x312 400x300 320x240

I don't know if there is a definitive list of video modes, or if that
is a non-sensical question.  Could we just allow *any* mode that fits
in the amount of RAM allowed by the 'struct xenfb_page pd[3]' ? or
at the very least dramatically increase the set of modes we allow
in the validation tables to something like the list I just included
above.

>  };
>  
>  /*
> @@ -120,6 +138,8 @@ struct xenfb_page
>   * solution is found, but don't leak it to the backend.
>   */
>  #ifdef __KERNEL__
> +#define XENFB_MAX_WIDTH 1024
> +#define XENFB_MAX_HEIGHT 768

I think it'd be beter to get rid of these vars. They don't make any sense
when you have configurable modes. With a non-constrained aspect ratio for
the FB, there's no (w,h) pair that can usefully define limits. The limit
is just whatever fits in your RAM per aspect ratio.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel