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

["Pat Campbell" <plc@xxxxxxxxxx>]

>>> On Wed, Jan 2, 2008 at 10:59 AM, in message 
>>> <20080102175935.GE1237@xxxxxxxxxx>,
"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote: 
> 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:

I will change check_var() to use this validation so we can support any 
resolution that the memory can hold with the constraint that the width
cannot exceed the scan line length.

if ( var->bits_per_pixerl == xenfb_info->page->depth && 
        var->xres <= XENFB_SCAN_LINE_LEN && 
         xenfb_mem_len >= (var->xres * var->yres * (xenfb_info->page->depth / 
8))) {
   return 0;
}

We need to check xres against XENFB_SCAN_LINE_LEN as X initializes it's
frame buffer line length once in FBDevScreenInit().   Size changes via xrandr 
do not change the X servers line length value. 
 
> 
>>       * 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.

Agreed.  

For reasons stated above will add:
 #define XENFB_SCAN_LINE_LEN 1280  

> 
> Dan.




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