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

Attached patch adds multiple frame buffer resolution support to 
the PV xenfb frame buffer driver.

Code is essentially the same as I sent in the previous RFC except
the frame buffer size is now 800x600 if the backend does not
support feature-resize,  same memory footprint.  

xenkbd driver now checks feature-resize so that it can scale the 
absolute mouse position properly.

Corresponding backend IOEMU patch is required for functionality but 
this patch is not dependent on it.  Change is compatible with older
backend implementations.

Please apply to tip of linux-2.6.18-xen

Signed-      off-      by: Pat Campbell <plc@novell.com>








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

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@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> On Wed, Jan 2, 2008 at 10:59 AM, in message
<20080102175935.GE1237@redhat.com>,
"Daniel P. Berrange" <berrange@redhat.com> 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@lists.xensource.com
http://lists.xensource.com/xen-devel