[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [RFC][PATCH v1] xen-fbfront: replace deferred io with buffer queue



Use N-buffering instead of old deferred I/O, which is not suitable for high
frame rates. This includes new event type -- xenfb_in_released,
to track buffers not being used by backend.
Also use grants for fb pages, as they allow backend to map them
to video devices.

Signed-off-by: Sergiy Kibrik <sergiy.kibrik@xxxxxxxxxxxxxxx>
---

Hi,
Here I'd like to present changes to xen-fbfront driver as example of
how it can possibly be modified to support high frame rates.
With this changes plus modified xenfb backend I was able to achieve 60 FPS on
ARM based DRA7xx SoC, but this is rather display limitation, not driver itself.

The idea is to send full resolution shared buffers to backend -- modern UI and
multimedia applications cause heavy screen redraw. For this purpose driver
allocates N-times bigger buffer than actual framebuffer resolution, each chunk
assigned ID (0-N), which is carried within event message to backend.
As soon as buffers displayed & released on host side, backend sends back release
event with ID of released chunk, which can be used as framebuffer again.

For my setup I used modified Xen framebuffer backend [1], DRA7xx SoC and
OMAP Display Subsystem for video output (omap_vout V4L2 driver).

[1] http://www-archive.xenproject.org/files/summit_3/Xenpvfb.pdf

 drivers/video/fbdev/xen-fbfront.c |  344 ++++++++++++++++++++++++++-----------
 include/xen/interface/io/fbif.h   |    9 +-
 2 files changed, 250 insertions(+), 103 deletions(-)

diff --git a/drivers/video/fbdev/xen-fbfront.c 
b/drivers/video/fbdev/xen-fbfront.c
index 09dc447..a3e40ac 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -11,13 +11,7 @@
  *  more details.
  */
 
-/*
- * TODO:
- *
- * Switch to grant tables when they become capable of dealing with the
- * frame buffer.
- */
-
+/*#define DEBUG*/
 #include <linux/console.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -32,20 +26,34 @@
 #include <xen/xen.h>
 #include <xen/events.h>
 #include <xen/page.h>
+#include <xen/grant_table.h>
 #include <xen/interface/io/fbif.h>
 #include <xen/interface/io/protocols.h>
 #include <xen/xenbus.h>
 #include <xen/platform_pci.h>
 
+enum xenfb_buf_state {
+       XENFB_BUF_RELEASED = 0,
+       XENFB_BUF_ACTIVE,
+       XENFB_BUF_NEXT,
+};
+
 struct xenfb_info {
        unsigned char           *fb;
+       int                     fb_size;
        struct fb_info          *fb_info;
        int                     x1, y1, x2, y2; /* dirty rectangle,
                                                   protected by dirty_lock */
-       spinlock_t              dirty_lock;
+
+       spinlock_t              b_lock;
+       enum xenfb_buf_state    *buffers;
+       int                     nr_buffers;
        int                     nr_pages;
+       struct completion       completion;
+
        int                     irq;
        struct xenfb_page       *page;
+       int                     ring_ref;
        unsigned long           *mfns;
        int                     update_wanted; /* XENFB_TYPE_UPDATE wanted */
        int                     feature_resize; /* XENFB_TYPE_RESIZE ok */
@@ -70,6 +78,43 @@ static void xenfb_init_shared_page(struct xenfb_info *, 
struct fb_info *);
 static int xenfb_connect_backend(struct xenbus_device *, struct xenfb_info *);
 static void xenfb_disconnect_backend(struct xenfb_info *);
 
+static void *rvmalloc(unsigned long size)
+{
+       void *mem;
+       unsigned long adr;
+
+       size = PAGE_ALIGN(size);
+       mem = vmalloc_32(size);
+       if (!mem)
+               return NULL;
+
+       memset(mem, 0, size); /* Clear the ram out, no junk to the user */
+       adr = (unsigned long) mem;
+       while (size > 0) {
+               SetPageReserved(vmalloc_to_page((void *)adr));
+               adr += PAGE_SIZE;
+               size -= PAGE_SIZE;
+       }
+
+       return mem;
+}
+
+static void rvfree(void *mem, unsigned long size)
+{
+       unsigned long adr;
+
+       if (!mem)
+               return;
+
+       adr = (unsigned long) mem;
+       while ((long) size > 0) {
+               ClearPageReserved(vmalloc_to_page((void *)adr));
+               adr += PAGE_SIZE;
+               size -= PAGE_SIZE;
+       }
+       vfree(mem);
+}
+
 static void xenfb_send_event(struct xenfb_info *info,
                             union xenfb_out_event *event)
 {
@@ -147,7 +192,7 @@ static void xenfb_refresh(struct xenfb_info *info,
        if (!info->update_wanted)
                return;
 
-       spin_lock_irqsave(&info->dirty_lock, flags);
+       spin_lock_irqsave(&info->b_lock, flags);
 
        /* Combine with dirty rectangle: */
        if (info->y1 < y1)
@@ -165,7 +210,7 @@ static void xenfb_refresh(struct xenfb_info *info,
                info->x2 = x2;
                info->y1 = y1;
                info->y2 = y2;
-               spin_unlock_irqrestore(&info->dirty_lock, flags);
+               spin_unlock_irqrestore(&info->b_lock, flags);
                return;
        }
 
@@ -173,42 +218,12 @@ static void xenfb_refresh(struct xenfb_info *info,
        info->x1 = info->y1 = INT_MAX;
        info->x2 = info->y2 = 0;
 
-       spin_unlock_irqrestore(&info->dirty_lock, flags);
+       spin_unlock_irqrestore(&info->b_lock, flags);
 
        if (x1 <= x2 && y1 <= y2)
                xenfb_do_update(info, x1, y1, x2 - x1 + 1, y2 - y1 + 1);
 }
 
-static void xenfb_deferred_io(struct fb_info *fb_info,
-                             struct list_head *pagelist)
-{
-       struct xenfb_info *info = fb_info->par;
-       struct page *page;
-       unsigned long beg, end;
-       int y1, y2, miny, maxy;
-
-       miny = INT_MAX;
-       maxy = 0;
-       list_for_each_entry(page, pagelist, lru) {
-               beg = page->index << PAGE_SHIFT;
-               end = beg + PAGE_SIZE - 1;
-               y1 = beg / fb_info->fix.line_length;
-               y2 = end / fb_info->fix.line_length;
-               if (y2 >= fb_info->var.yres)
-                       y2 = fb_info->var.yres - 1;
-               if (miny > y1)
-                       miny = y1;
-               if (maxy < y2)
-                       maxy = y2;
-       }
-       xenfb_refresh(info, 0, miny, fb_info->var.xres, maxy - miny + 1);
-}
-
-static struct fb_deferred_io xenfb_defio = {
-       .delay          = HZ / 20,
-       .deferred_io    = xenfb_deferred_io,
-};
-
 static int xenfb_setcolreg(unsigned regno, unsigned red, unsigned green,
                           unsigned blue, unsigned transp,
                           struct fb_info *info)
@@ -275,26 +290,40 @@ static ssize_t xenfb_write(struct fb_info *p, const char 
__user *buf,
        return res;
 }
 
+static int find_released(struct xenfb_info *info)
+{
+       int i;
+       for (i = 0; i < info->nr_buffers; i++)
+               if (info->buffers[i] == XENFB_BUF_RELEASED)
+                       return i;
+       return -1;
+}
+
 static int
 xenfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
        struct xenfb_info *xenfb_info;
        int required_mem_len;
+       int buffer;
+       unsigned long flags;
 
        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;
-               }
+       if (var->xres != video[KPARAM_WIDTH] ||
+           var->yres != video[KPARAM_HEIGHT] ||
+           var->bits_per_pixel != xenfb_info->page->depth)
                return -EINVAL;
-       }
 
-       /* Can't resize past initial width and height */
-       if (var->xres > video[KPARAM_WIDTH] || var->yres > video[KPARAM_HEIGHT])
-               return -EINVAL;
+       if (xenfb_queue_full(xenfb_info))
+               return -EBUSY;
+
+       spin_lock_irqsave(&xenfb_info->b_lock, flags);
+       buffer = var->yoffset / var->yres;
+       if (find_released(xenfb_info) == -1) {
+               spin_unlock_irqrestore(&xenfb_info->b_lock, flags);
+               return -EBUSY;
+       }
+       spin_unlock_irqrestore(&xenfb_info->b_lock, flags);
 
        required_mem_len = var->xres * var->yres * xenfb_info->page->depth / 8;
        if (var->bits_per_pixel == xenfb_info->page->depth &&
@@ -307,25 +336,85 @@ xenfb_check_var(struct fb_var_screeninfo *var, struct 
fb_info *info)
        return -EINVAL;
 }
 
-static int xenfb_set_par(struct fb_info *info)
+static int xenfb_set_par(struct fb_info *fbi)
 {
-       struct xenfb_info *xenfb_info;
+       struct xenfb_info *info = fbi->par;
+       struct xenbus_device *xbdev = info->xbdev;
+       struct fb_var_screeninfo *var = &fbi->var;
+       int buffer = var->yoffset / var->yres;
        unsigned long flags;
 
-       xenfb_info = info->par;
+       BUG_ON(buffer < 0 || buffer >= info->nr_buffers);
+
+       xenfb_do_update(info, var->xoffset, var->yoffset,
+                               var->xres, var->yres);
+       dev_dbg(&xbdev->dev, "sent buf %d\n", buffer);
+
+       spin_lock_irqsave(&info->b_lock, flags);
+       info->buffers[buffer] = XENFB_BUF_NEXT;
+       buffer = find_released(info);
+       if (buffer != -1) {
+               var->yoffset = var->yres * buffer;
+               dev_dbg(&xbdev->dev, "giving out %d\n", buffer);
+               spin_unlock_irqrestore(&info->b_lock, flags);
+               return 0;
+       }
 
-       spin_lock_irqsave(&xenfb_info->resize_lock, flags);
-       xenfb_info->resize.type = XENFB_TYPE_RESIZE;
-       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.offset = 0;
-       xenfb_info->resize_dpy = 1;
-       spin_unlock_irqrestore(&xenfb_info->resize_lock, flags);
+       /* If no buffers available, e.g. in case of double-buffering,
+        * we have to wait for backend to release some. If backend is
+        * already dead, we return last buffer and hope
+        * client knows what to do
+       */
+       INIT_COMPLETION(info->completion);
+       spin_unlock_irqrestore(&info->b_lock, flags);
+
+       if (wait_for_completion_interruptible_timeout(&info->completion,
+                                       msecs_to_jiffies(40)) <= 0) {
+               dev_dbg(&xbdev->dev, "no buffer after timeout\n");
+               return -ETIMEDOUT;
+       }
+
+       spin_lock_irqsave(&info->b_lock, flags);
+       buffer = find_released(info);
+       spin_unlock_irqrestore(&info->b_lock, flags);
+       BUG_ON(buffer == -1); /* wtf is going on? */
+       var->yoffset = var->yres * buffer;
+
+       dev_dbg(&xbdev->dev, "given %d after wait\n", buffer);
        return 0;
 }
 
+static int xenfb_mmap(struct fb_info *fbi,
+                   struct vm_area_struct *vma)
+{
+       struct xenfb_info *info = fbi->par;
+       unsigned long start = vma->vm_start;
+       unsigned long size = vma->vm_end - vma->vm_start;
+       unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
+       unsigned long page, pos;
+
+       if (offset + size > fbi->fix.smem_len)
+               return -EINVAL;
+
+       pos = (unsigned long)info->fb + offset;
+
+       while (size > 0) {
+               page = vmalloc_to_pfn((void *)pos);
+               if (remap_pfn_range(vma, start, page, PAGE_SIZE, PAGE_SHARED))
+                       return -EAGAIN;
+
+               start += PAGE_SIZE;
+               pos += PAGE_SIZE;
+               if (size > PAGE_SIZE)
+                       size -= PAGE_SIZE;
+               else
+                       size = 0;
+       }
+
+       return 0;
+
+}
+
 static struct fb_ops xenfb_fb_ops = {
        .owner          = THIS_MODULE,
        .fb_read        = fb_sys_read,
@@ -336,26 +425,36 @@ static struct fb_ops xenfb_fb_ops = {
        .fb_imageblit   = xenfb_imageblit,
        .fb_check_var   = xenfb_check_var,
        .fb_set_par     = xenfb_set_par,
+       .fb_mmap        = xenfb_mmap,
 };
 
 static irqreturn_t xenfb_event_handler(int rq, void *dev_id)
 {
-       /*
-        * No in events recognized, simply ignore them all.
-        * If you need to recognize some, see xen-kbdfront's
-        * input_handler() for how to do that.
-        */
        struct xenfb_info *info = dev_id;
        struct xenfb_page *page = info->page;
+       uint32_t cons, in_prod;
+       unsigned long flags;
 
-       if (page->in_cons != page->in_prod) {
-               info->page->in_cons = info->page->in_prod;
-               notify_remote_via_irq(info->irq);
+       rmb();
+       in_prod = page->in_prod;
+       dev_dbg(&info->xbdev->dev, "handle %d in events\n", in_prod);
+
+       spin_lock_irqsave(&info->b_lock, flags);
+       for (cons = page->in_cons; cons != in_prod; cons++) {
+               union xenfb_in_event *event = &XENFB_IN_RING_REF(page, cons);
+               if (event->type == XENFB_TYPE_RELEASE) {
+                       int buffer = event->release.buffer;
+                       if (buffer < 0 || buffer >= info->nr_buffers)
+                               continue;
+                       info->buffers[buffer] = XENFB_BUF_RELEASED;
+                       dev_dbg(&info->xbdev->dev, "released %d\n", buffer);
+                       complete(&info->completion);
+               }
        }
+       page->in_cons = cons;
+       spin_unlock_irqrestore(&info->b_lock, flags);
 
-       /* Flush dirty rectangle: */
-       xenfb_refresh(info, INT_MAX, INT_MAX, -INT_MAX, -INT_MAX);
-
+       mb();
        return IRQ_HANDLED;
 }
 
@@ -364,8 +463,6 @@ static int xenfb_probe(struct xenbus_device *dev,
 {
        struct xenfb_info *info;
        struct fb_info *fb_info;
-       int fb_size;
-       int val;
        int ret = 0;
 
        info = kzalloc(sizeof(*info), GFP_KERNEL);
@@ -375,42 +472,50 @@ static int xenfb_probe(struct xenbus_device *dev,
        }
 
        /* Limit kernel param videoram amount to what is in xenstore */
-       if (xenbus_scanf(XBT_NIL, dev->otherend, "videoram", "%d", &val) == 1) {
-               if (val < video[KPARAM_MEM])
-                       video[KPARAM_MEM] = val;
-       }
-
-       /* If requested res does not fit in available memory, use default */
-       fb_size = video[KPARAM_MEM] * 1024 * 1024;
-       if (video[KPARAM_WIDTH] * video[KPARAM_HEIGHT] * XENFB_DEPTH / 8
-           > fb_size) {
+       if (xenbus_scanf(XBT_NIL, dev->otherend,
+                       "num_bufs", "%d", &info->nr_buffers) != 1)
+               info->nr_buffers = 2;
+       if (xenbus_scanf(XBT_NIL, dev->otherend, "size", "%dx%d",
+                       &video[KPARAM_WIDTH], &video[KPARAM_HEIGHT]) != 2) {
                video[KPARAM_WIDTH] = XENFB_WIDTH;
                video[KPARAM_HEIGHT] = XENFB_HEIGHT;
-               fb_size = XENFB_DEFAULT_FB_LEN;
        }
 
        dev_set_drvdata(&dev->dev, info);
        info->xbdev = dev;
        info->irq = -1;
        info->x1 = info->y1 = INT_MAX;
-       spin_lock_init(&info->dirty_lock);
+       spin_lock_init(&info->b_lock);
        spin_lock_init(&info->resize_lock);
+       init_completion(&info->completion);
 
-       info->fb = vzalloc(fb_size);
+       info->fb_size = video[KPARAM_WIDTH] * video[KPARAM_HEIGHT] *
+                       (XENFB_DEPTH / 8) * info->nr_buffers;
+       info->fb = rvmalloc(info->fb_size);
        if (info->fb == NULL)
                goto error_nomem;
 
-       info->nr_pages = (fb_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+       info->nr_pages = (info->fb_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
 
        info->mfns = vmalloc(sizeof(unsigned long) * info->nr_pages);
        if (!info->mfns)
                goto error_nomem;
+       info->buffers = kzalloc(sizeof(info->buffers[0]) * info->nr_buffers,
+                                                               GFP_KERNEL);
+       if (!info->buffers)
+               goto error_nomem;
 
        /* set up shared page */
        info->page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
        if (!info->page)
                goto error_nomem;
 
+       memset(info->page->pd, -1, sizeof(info->page->pd));
+
+       info->ring_ref = xenbus_grant_ring(dev, virt_to_mfn(info->page));
+       if (info->ring_ref < 0)
+               goto error;
+
        /* abusing framebuffer_alloc() to allocate pseudo_palette */
        fb_info = framebuffer_alloc(sizeof(u32) * 256, NULL);
        if (fb_info == NULL)
@@ -438,11 +543,13 @@ static int xenfb_probe(struct xenbus_device *dev,
 
        fb_info->fix.visual = FB_VISUAL_TRUECOLOR;
        fb_info->fix.line_length = fb_info->var.xres * XENFB_DEPTH / 8;
-       fb_info->fix.smem_start = 0;
-       fb_info->fix.smem_len = fb_size;
+       fb_info->fix.smem_start =
+                       (unsigned long)page_to_phys(vmalloc_to_page(info->fb));
+       fb_info->fix.smem_len = info->fb_size;
        strcpy(fb_info->fix.id, "xen");
        fb_info->fix.type = FB_TYPE_PACKED_PIXELS;
        fb_info->fix.accel = FB_ACCEL_NONE;
+       fb_info->fix.ypanstep = fb_info->var.yres;
 
        fb_info->flags = FBINFO_FLAG_DEFAULT | FBINFO_VIRTFB;
 
@@ -453,9 +560,6 @@ static int xenfb_probe(struct xenbus_device *dev,
                goto error;
        }
 
-       fb_info->fbdefio = &xenfb_defio;
-       fb_deferred_io_init(fb_info);
-
        xenfb_init_shared_page(info, fb_info);
 
        ret = xenfb_connect_backend(dev, info);
@@ -521,6 +625,7 @@ static int xenfb_resume(struct xenbus_device *dev)
 static int xenfb_remove(struct xenbus_device *dev)
 {
        struct xenfb_info *info = dev_get_drvdata(&dev->dev);
+       int i;
 
        xenfb_disconnect_backend(info);
        if (info->fb_info) {
@@ -529,9 +634,26 @@ static int xenfb_remove(struct xenbus_device *dev)
                fb_dealloc_cmap(&info->fb_info->cmap);
                framebuffer_release(info->fb_info);
        }
+
+       for (i = 0; i < sizeof(info->page->pd); i++)
+               if (info->page->pd[i] != -1) {
+                       gnttab_end_foreign_access(
+                               info->page->pd[i], 0, 0UL);
+                       info->page->pd[i] = -1;
+               }
+
+       if (info->ring_ref >= 0) {
+               gnttab_end_foreign_access(info->ring_ref, 0, 0UL);
+               info->ring_ref = -1;
+       }
+
        free_page((unsigned long)info->page);
+
+       for (i = 0; i < info->nr_pages; i++)
+               gnttab_end_foreign_access(info->mfns[i], 0, 0UL);
        vfree(info->mfns);
-       vfree(info->fb);
+       rvfree(info->fb, info->fb_size);
+       kfree(info->buffers);
        kfree(info);
 
        return 0;
@@ -545,14 +667,32 @@ static unsigned long vmalloc_to_mfn(void *address)
 static void xenfb_init_shared_page(struct xenfb_info *info,
                                   struct fb_info *fb_info)
 {
-       int i;
+       int i, ref;
+       grant_ref_t gref_head;
        int epd = PAGE_SIZE / sizeof(info->mfns[0]);
 
-       for (i = 0; i < info->nr_pages; i++)
-               info->mfns[i] = vmalloc_to_mfn(info->fb + i * PAGE_SIZE);
+       if (gnttab_alloc_grant_references(info->nr_pages +
+                               DIV_ROUND_UP(info->nr_pages, epd),
+                               &gref_head)) {
+               WARN(1, "failed to allocate %u grants\n", info->nr_pages);
+               return;
+       }
 
-       for (i = 0; i * epd < info->nr_pages; i++)
-               info->page->pd[i] = vmalloc_to_mfn(&info->mfns[i * epd]);
+       for (i = 0; i < info->nr_pages; i++) {
+               ref = gnttab_claim_grant_reference(&gref_head);
+               BUG_ON(ref == -ENOSPC);
+               gnttab_grant_foreign_access_ref(ref, info->xbdev->otherend_id,
+                               vmalloc_to_mfn(info->fb + i * PAGE_SIZE), 0);
+               info->mfns[i] = ref;
+       }
+
+       for (i = 0; i * epd < info->nr_pages; i++) {
+               ref = gnttab_claim_grant_reference(&gref_head);
+               BUG_ON(ref == -ENOSPC);
+               gnttab_grant_foreign_access_ref(ref, info->xbdev->otherend_id,
+                               vmalloc_to_mfn(&info->mfns[i * epd]), 0);
+               info->page->pd[i] = ref;
+       }
 
        info->page->width = fb_info->var.xres;
        info->page->height = fb_info->var.yres;
@@ -585,8 +725,8 @@ static int xenfb_connect_backend(struct xenbus_device *dev,
                xenbus_dev_fatal(dev, ret, "starting transaction");
                goto unbind_irq;
        }
-       ret = xenbus_printf(xbt, dev->nodename, "page-ref", "%lu",
-                           virt_to_mfn(info->page));
+       ret = xenbus_printf(xbt, dev->nodename, "ring-ref", "%u",
+                           info->ring_ref);
        if (ret)
                goto error_xenbus;
        ret = xenbus_printf(xbt, dev->nodename, "event-channel", "%u",
diff --git a/include/xen/interface/io/fbif.h b/include/xen/interface/io/fbif.h
index 974a51e..471d867 100644
--- a/include/xen/interface/io/fbif.h
+++ b/include/xen/interface/io/fbif.h
@@ -77,13 +77,20 @@ union xenfb_out_event {
 
 /*
  * Frontends should ignore unknown in events.
- * No in events currently defined.
  */
 
+#define XENFB_TYPE_RELEASE 4
+struct xenfb_release {
+       uint8_t type;   /* XENFB_TYPE_RELEASE */
+       int32_t buffer; /* buffer # */
+};
+
+
 #define XENFB_IN_EVENT_SIZE 40
 
 union xenfb_in_event {
        uint8_t type;
+       struct xenfb_release release;
        char pad[XENFB_IN_EVENT_SIZE];
 };
 
-- 
1.7.9.5


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.