WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel][PATCH][LINUX] Dynamic modes support for xenfb (2 of 2)

To: Pat Campbell <plc@xxxxxxxxxx>
Subject: Re: [Xen-devel][PATCH][LINUX] Dynamic modes support for xenfb (2 of 2)
From: Markus Armbruster <armbru@xxxxxxxxxx>
Date: Tue, 25 Mar 2008 15:43:28 +0100
Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Tue, 25 Mar 2008 07:43:56 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <47E722A2.9030506@xxxxxxxxxx> (Pat Campbell's message of "Sun\, 23 Mar 2008 21\:40\:18 -0600")
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <47D98787.6000501@xxxxxxxxxx> <87hcf91cmh.fsf@xxxxxxxxxxxxxxxxx> <47DD7082.9090401@xxxxxxxxxx> <874pb5w8xa.fsf@xxxxxxxxxxxxxxxxx> <47E013CE.5090608@xxxxxxxxxx> <87r6e7auam.fsf@xxxxxxxxxxxxxxxxx> <47E722A2.9030506@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.1 (gnu/linux)
Pat Campbell <plc@xxxxxxxxxx> writes:

> Markus Armbruster wrote:
>> Pat Campbell <plc@xxxxxxxxxx> writes:
>>
>>   
>>> Markus Armbruster wrote:
>>>     
>>>> Pat Campbell <plc@xxxxxxxxxx> writes:
>>>>
>>>>   
>>>>       
>>>>> Markus Armbruster wrote:
>>>>>     
>>>>>         
>>>>>> Pat Campbell <plc@xxxxxxxxxx> writes:
>>>>>>
>>>>>>   
>>>>>>       
>>>>>>           
>>>>>>> Attached patch adds dynamic frame buffer resolution support to
>>>>>>> the PV xenfb frame buffer driver.
[...]
> I have attached front and back for your review.  Back has not changed
> except to include opengl changes. Front has synchronization fix.  I
> added a spin lock and struct xenfb_resize into struct xenfb.  struct
> xenfb_resize was added to protect the screen size values from changes
> outside the driver.  IE User application calls to ioctl(fb,
> FBIOPUT_VSCREENINFO, &fb_var); while the driver is processing a previous
> call.

Yup, protection against that is required.

Looks good!  One easily fixed bug and a few remarks inline.

[...]
> diff -r de57c3f218fb drivers/xen/fbfront/xenfb.c
> --- a/drivers/xen/fbfront/xenfb.c     Thu Mar 20 11:35:25 2008 +0000
> +++ b/drivers/xen/fbfront/xenfb.c     Sun Mar 23 19:52:17 2008 -0600
> @@ -62,15 +62,21 @@ struct xenfb_info
>       struct xenfb_page       *page;
>       unsigned long           *mfns;
>       int                     update_wanted; /* XENFB_TYPE_UPDATE wanted */
> +     int                     feature_resize; /* Backend has resize feature */
> +     struct xenfb_resize     resize;
> +     int                     resize_dpy;
> +     spinlock_t              resize_lock;
>  
>       struct xenbus_device    *xbdev;
>  };
>  
>  /*
> - * How the locks work together
> - *
> - * There are two locks: spinlock dirty_lock protecting the dirty
> - * rectangle, and mutex mm_lock protecting mappings.
> + * There are three locks:
> + *    spinlock resize_lock protecting resize_dpy and screen size

Suggest

 *    spinlock resize_lock protecting resize_dpy and resize

because with the new screen size in one place now we can be both both
specific and concise here.

> + *    spinlock dirty_lock protecting the dirty rectangle
> + *    mutex mm_lock protecting mappings.
> + *
> + * How the dirty and mapping locks work together
>   *
>   * The problem is that dirty rectangle and mappings aren't
>   * independent: the dirty rectangle must cover all faulted pages in
[...]
> @@ -150,14 +177,22 @@ static void xenfb_do_update(struct xenfb
>       event.update.width = w;
>       event.update.height = h;
>  
> -     prod = info->page->out_prod;
>       /* caller ensures !xenfb_queue_full() */
> -     mb();                   /* ensure ring space available */
> -     XENFB_OUT_RING_REF(info->page, prod) = event;
> -     wmb();                  /* ensure ring contents visible */
> -     info->page->out_prod = prod + 1;
> -
> -     notify_remote_via_irq(info->irq);
> +     xenfb_send_event(info, &event);
> +}
> +
> +static void xenfb_do_resize(struct xenfb_info *info)
> +{
> +     union xenfb_out_event event;
> +
> +     event.type = XENFB_TYPE_RESIZE;
> +     event.resize.width = info->resize.width;
> +     event.resize.height = info->resize.height;
> +     event.resize.stride = info->resize.stride;
> +     event.resize.depth = info->resize.depth;

Could do event.resize = info->resize, except info->resize.type isn't
set up for that (see below).

> +
> +     /* caller ensures !xenfb_queue_full() */
> +     xenfb_send_event(info, &event);
>  }
>  
>  static int xenfb_queue_full(struct xenfb_info *info)
> @@ -209,11 +244,26 @@ static void xenfb_update_screen(struct x
>       xenfb_do_update(info, x1, y1, x2 - x1, y2 - y1);
>  }
>  
> +static void xenfb_handle_resize_dpy(struct xenfb_info *info)
> +{
> +     int flags;

Type error!  unsigned long flags

> +
> +     spin_lock_irqsave(&info->resize_lock, flags);
> +     if (info->resize_dpy) {
> +             if (!xenfb_queue_full(info)) {
> +                     info->resize_dpy = 0;
> +                     xenfb_do_resize(info);
> +             }
> +     }
> +     spin_unlock_irqrestore(&info->resize_lock, flags);
> +}
> +
>  static int xenfb_thread(void *data)
>  {
>       struct xenfb_info *info = data;
>  
>       while (!kthread_should_stop()) {
> +             xenfb_handle_resize_dpy(info);
>               if (info->dirty) {
>                       info->dirty = 0;
>                       xenfb_update_screen(info);
> @@ -413,6 +463,55 @@ static int xenfb_mmap(struct fb_info *fb
>       return 0;
>  }
>  
> +static int
> +xenfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
> +{
> +     struct xenfb_info *xenfb_info;
> +     int required_mem_len;
> +
> +     xenfb_info = info->par;
> +
> +     if (!xenfb_info->feature_resize) {
> +             if (var->xres == video[KPARAM_WIDTH] &&
> +                     var->yres == video[KPARAM_HEIGHT] &&
> +                     var->bits_per_pixel == xenfb_info->page->depth) {
> +                     return 0;
> +             }
> +             return -EINVAL;
> +     }
> +
> +     /* Can't resize past initial width and height */
> +     if (var->xres > video[KPARAM_WIDTH] || var->yres > video[KPARAM_HEIGHT])
> +             return -EINVAL;
> +
> +     required_mem_len = var->xres * var->yres * (xenfb_info->page->depth / 
> 8);
> +     if (var->bits_per_pixel == xenfb_info->page->depth &&
> +             var->xres <= info->fix.line_length / (XENFB_DEPTH / 8) &&
> +             required_mem_len <= info->fix.smem_len) {
> +             var->xres_virtual = var->xres;
> +             var->yres_virtual = var->yres;
> +             return 0;
> +     }
> +     return -EINVAL;
> +}
> +
> +static int xenfb_set_par(struct fb_info *info)
> +{
> +     struct xenfb_info *xenfb_info;
> +     unsigned long flags;
> +
> +     xenfb_info = info->par;
> +
> +     spin_lock_irqsave(&xenfb_info->resize_lock, flags);
> +     xenfb_info->resize.width = info->var.xres;
> +     xenfb_info->resize.height = info->var.yres;
> +     xenfb_info->resize.stride = info->fix.line_length;
> +     xenfb_info->resize.depth = info->var.bits_per_pixel;
> +     xenfb_info->resize_dpy = 1;

Not a bug, just a little trap for the unwary: xenfb_info->resize.type
remains zero.

> +     spin_unlock_irqrestore(&xenfb_info->resize_lock, flags);
> +     return 0;
> +}
> +
>  static struct fb_ops xenfb_fb_ops = {
>       .owner          = THIS_MODULE,
>       .fb_setcolreg   = xenfb_setcolreg,
[...]

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