Re: [PATCH 7/8] drm/qxl: Use drm_fb_helper deferred_io support

On Wed, Apr 20, 2016 at 05:25:28PM +0200, Noralf Trønnes wrote:

Use the fbdev deferred io support in drm_fb_helper. The (struct fb_ops *)->fb_{fillrect,copyarea,imageblit} functions will now be deferred in the same way that mmap damage is, instead of being flushed directly. This patch has only been compile tested.

Signed-off-by: Noralf Trønnes noralf@tronnes.org

drivers/gpu/drm/qxl/qxl_display.c | 9 +- drivers/gpu/drm/qxl/qxl_drv.h | 7 +- drivers/gpu/drm/qxl/qxl_fb.c | 220 ++++++++++---------------------------- drivers/gpu/drm/qxl/qxl_kms.c | 4 - 4 files changed, 62 insertions(+), 178 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 030409a..9a03524 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -465,7 +465,7 @@ static const struct drm_crtc_funcs qxl_crtc_funcs = { .page_flip = qxl_crtc_page_flip, };

-static void qxl_user_framebuffer_destroy(struct drm_framebuffer *fb) +void qxl_user_framebuffer_destroy(struct drm_framebuffer *fb) { struct qxl_framebuffer *qxl_fb = to_qxl_framebuffer(fb);

@@ -527,12 +527,13 @@ int qxl_framebuffer_init(struct drm_device *dev, struct qxl_framebuffer *qfb, const struct drm_mode_fb_cmd2 *mode_cmd,

•      struct drm_gem_object *obj)
  

•      struct drm_gem_object *obj,
  

•      const struct drm_framebuffer_funcs *funcs)
  

There should be no need at all to have a separate fb funcs table for the fbdev fb. Both /should/ be able to use the exact same (already existing) ->dirty() callback. We need this only in CMA because CMA is a midlayer used by multiple drivers.

With that change you should be able to condense this patch down to pretty much just removing lines. Which is Good (tm).

Cheers, Daniel

{ int ret;

qfb->obj = obj;

• ret = drm_framebuffer_init(dev, &qfb->base, &qxl_fb_funcs);

• ret = drm_framebuffer_init(dev, &qfb->base, funcs); if (ret) { qfb->obj = NULL; return ret;

@@ -999,7 +1000,7 @@ qxl_user_framebuffer_create(struct drm_device *dev, if (qxl_fb == NULL) return NULL;

• ret = qxl_framebuffer_init(dev, qxl_fb, mode_cmd, obj);

• ret = qxl_framebuffer_init(dev, qxl_fb, mode_cmd, obj, &qxl_fb_funcs); if (ret) { kfree(qxl_fb); drm_gem_object_unreference_unlocked(obj);

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index 3f3897e..3ad6604 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -324,8 +324,6 @@ struct qxl_device { struct workqueue_struct *gc_queue; struct work_struct gc_work;

• struct work_struct fb_work;
• struct drm_property *hotplug_mode_update_property; int monitors_config_width; int monitors_config_height;

@@ -389,11 +387,13 @@ int qxl_get_handle_for_primary_fb(struct qxl_device *qdev, void qxl_fbdev_set_suspend(struct qxl_device *qdev, int state);

/* qxl_display.c */ +void qxl_user_framebuffer_destroy(struct drm_framebuffer *fb); int qxl_framebuffer_init(struct drm_device *dev, struct qxl_framebuffer *rfb, const struct drm_mode_fb_cmd2 *mode_cmd,

•      struct drm_gem_object *obj);
  

•      struct drm_gem_object *obj,
  

•      const struct drm_framebuffer_funcs *funcs);
  

void qxl_display_read_client_monitors_config(struct qxl_device *qdev); void qxl_send_monitors_config(struct qxl_device *qdev); int qxl_create_monitors_object(struct qxl_device *qdev); @@ -553,7 +553,6 @@ int qxl_irq_init(struct qxl_device *qdev); irqreturn_t qxl_irq_handler(int irq, void *arg);

/* qxl_fb.c */ -int qxl_fb_init(struct qxl_device *qdev); bool qxl_fbdev_qobj_is_fb(struct qxl_device *qdev, struct qxl_bo *qobj);

int qxl_debugfs_add_files(struct qxl_device *qdev, diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c index 06f032d..090dcee 100644 --- a/drivers/gpu/drm/qxl/qxl_fb.c +++ b/drivers/gpu/drm/qxl/qxl_fb.c @@ -30,6 +30,7 @@ #include "drm/drm.h" #include "drm/drm_crtc.h" #include "drm/drm_crtc_helper.h" +#include "drm/drm_rect.h" #include "qxl_drv.h"

#include "qxl_object.h" @@ -46,15 +47,6 @@ struct qxl_fbdev { struct list_head delayed_ops; void *shadow; int size;

• /* dirty memory logging */
• struct {

• spinlock_t lock;
  

• unsigned x1;
  

• unsigned y1;
  

• unsigned x2;
  

• unsigned y2;
  

• } dirty;

};

static void qxl_fb_image_init(struct qxl_fb_image *qxl_fb_image, @@ -82,169 +74,18 @@ static void qxl_fb_image_init(struct qxl_fb_image *qxl_fb_image, } }

-static void qxl_fb_dirty_flush(struct fb_info *info) -{

• struct qxl_fbdev *qfbdev = info->par;
• struct qxl_device *qdev = qfbdev->qdev;
• struct qxl_fb_image qxl_fb_image;
• struct fb_image *image = &qxl_fb_image.fb_image;
• unsigned long flags;
• u32 x1, x2, y1, y2;
• /* TODO: hard coding 32 bpp */
• int stride = qfbdev->qfb.base.pitches[0];
• spin_lock_irqsave(&qfbdev->dirty.lock, flags);
• x1 = qfbdev->dirty.x1;
• x2 = qfbdev->dirty.x2;
• y1 = qfbdev->dirty.y1;
• y2 = qfbdev->dirty.y2;
• qfbdev->dirty.x1 = 0;
• qfbdev->dirty.x2 = 0;
• qfbdev->dirty.y1 = 0;
• qfbdev->dirty.y2 = 0;
• spin_unlock_irqrestore(&qfbdev->dirty.lock, flags);
• /*

• * we are using a shadow draw buffer, at qdev->surface0_shadow
  

• */
  

• qxl_io_log(qdev, "dirty x[%d, %d], y[%d, %d]", x1, x2, y1, y2);
• image->dx = x1;
• image->dy = y1;
• image->width = x2 - x1 + 1;
• image->height = y2 - y1 + 1;
• image->fg_color = 0xffffffff; /* unused, just to avoid uninitialized

• 			 warnings */
  

• image->bg_color = 0;
• image->depth = 32; /* TODO: take from somewhere? */
• image->cmap.start = 0;
• image->cmap.len = 0;
• image->cmap.red = NULL;
• image->cmap.green = NULL;
• image->cmap.blue = NULL;
• image->cmap.transp = NULL;
• image->data = qfbdev->shadow + (x1 * 4) + (stride * y1);
• qxl_fb_image_init(&qxl_fb_image, qdev, info, NULL);
• qxl_draw_opaque_fb(&qxl_fb_image, stride);

-}

-static void qxl_dirty_update(struct qxl_fbdev *qfbdev,

• 	     int x, int y, int width, int height)
  

-{

• struct qxl_device *qdev = qfbdev->qdev;
• unsigned long flags;
• int x2, y2;
• x2 = x + width - 1;
• y2 = y + height - 1;
• spin_lock_irqsave(&qfbdev->dirty.lock, flags);
• if ((qfbdev->dirty.y2 - qfbdev->dirty.y1) &&

•    (qfbdev->dirty.x2 - qfbdev->dirty.x1)) {
  

• if (qfbdev->dirty.y1 < y)
  

• 	y = qfbdev->dirty.y1;
  

• if (qfbdev->dirty.y2 > y2)
  

• 	y2 = qfbdev->dirty.y2;
  

• if (qfbdev->dirty.x1 < x)
  

• 	x = qfbdev->dirty.x1;
  

• if (qfbdev->dirty.x2 > x2)
  

• 	x2 = qfbdev->dirty.x2;
  

• }
• qfbdev->dirty.x1 = x;
• qfbdev->dirty.x2 = x2;
• qfbdev->dirty.y1 = y;
• qfbdev->dirty.y2 = y2;
• spin_unlock_irqrestore(&qfbdev->dirty.lock, flags);
• schedule_work(&qdev->fb_work);

-}

-static void qxl_deferred_io(struct fb_info *info,

• 	    struct list_head *pagelist)
  

-{

• struct qxl_fbdev *qfbdev = info->par;
• unsigned long start, end, min, max;
• struct page *page;
• int y1, y2;
• min = ULONG_MAX;
• max = 0;
• list_for_each_entry(page, pagelist, lru) {

• start = page->index << PAGE_SHIFT;
  

• end = start + PAGE_SIZE - 1;
  

• min = min(min, start);
  

• max = max(max, end);
  

• }
• if (min < max) {

• y1 = min / info->fix.line_length;
  

• y2 = (max / info->fix.line_length) + 1;
  

• qxl_dirty_update(qfbdev, 0, y1, info->var.xres, y2 - y1);
  

• }

-};

static struct fb_deferred_io qxl_defio = { .delay = QXL_DIRTY_DELAY,

• .deferred_io = qxl_deferred_io,

• .deferred_io = drm_fb_helper_deferred_io,

};

-static void qxl_fb_fillrect(struct fb_info *info,

• 	    const struct fb_fillrect *rect)
  

-{

• struct qxl_fbdev *qfbdev = info->par;
• sys_fillrect(info, rect);
• qxl_dirty_update(qfbdev, rect->dx, rect->dy, rect->width,

• 	 rect->height);
  

-}

-static void qxl_fb_copyarea(struct fb_info *info,

• 	    const struct fb_copyarea *area)
  

-{

• struct qxl_fbdev *qfbdev = info->par;
• sys_copyarea(info, area);
• qxl_dirty_update(qfbdev, area->dx, area->dy, area->width,

• 	 area->height);
  

-}

-static void qxl_fb_imageblit(struct fb_info *info,

• 	     const struct fb_image *image)
  

-{

• struct qxl_fbdev *qfbdev = info->par;
• sys_imageblit(info, image);
• qxl_dirty_update(qfbdev, image->dx, image->dy, image->width,

• 	 image->height);
  

-}

-static void qxl_fb_work(struct work_struct *work) -{

• struct qxl_device *qdev = container_of(work, struct qxl_device, fb_work);
• struct qxl_fbdev *qfbdev = qdev->mode_info.qfbdev;
• qxl_fb_dirty_flush(qfbdev->helper.fbdev);

-}

-int qxl_fb_init(struct qxl_device *qdev) -{

• INIT_WORK(&qdev->fb_work, qxl_fb_work);
• return 0;

-}

static struct fb_ops qxlfb_ops = { .owner = THIS_MODULE, .fb_check_var = drm_fb_helper_check_var, .fb_set_par = drm_fb_helper_set_par, /* TODO: copy vmwgfx */

• .fb_fillrect = qxl_fb_fillrect,
• .fb_copyarea = qxl_fb_copyarea,
• .fb_imageblit = qxl_fb_imageblit,

• .fb_fillrect = drm_fb_helper_sys_fillrect,
• .fb_copyarea = drm_fb_helper_sys_copyarea,
• .fb_imageblit = drm_fb_helper_sys_imageblit, .fb_pan_display = drm_fb_helper_pan_display, .fb_blank = drm_fb_helper_blank, .fb_setcmap = drm_fb_helper_setcmap,

@@ -338,6 +179,53 @@ out_unref: return ret; }

+static int qxlfb_framebuffer_dirty(struct drm_framebuffer *fb,

• 		   struct drm_file *file_priv,
  

• 		   unsigned flags, unsigned color,
  

• 		   struct drm_clip_rect *clips,
  

• 		   unsigned num_clips)
  

+{

• struct qxl_device *qdev = fb->dev->dev_private;
• struct fb_info *info = qdev->fbdev_info;
• struct qxl_fbdev *qfbdev = info->par;
• struct qxl_fb_image qxl_fb_image;
• struct fb_image *image = &qxl_fb_image.fb_image;
• /* TODO: hard coding 32 bpp */
• int stride = qfbdev->qfb.base.pitches[0];
• /*

• * we are using a shadow draw buffer, at qdev->surface0_shadow
  

• */
  

• qxl_io_log(qdev, "dirty x[%d, %d], y[%d, %d]", clips->x1, clips->x2,

•    clips->y1, clips->y2);
  

• image->dx = clips->x1;
• image->dy = clips->y1;
• image->width = drm_clip_rect_width(clips);
• image->height = drm_clip_rect_height(clips);
• image->fg_color = 0xffffffff; /* unused, just to avoid uninitialized

• 			 warnings */
  

• image->bg_color = 0;
• image->depth = 32; /* TODO: take from somewhere? */
• image->cmap.start = 0;
• image->cmap.len = 0;
• image->cmap.red = NULL;
• image->cmap.green = NULL;
• image->cmap.blue = NULL;
• image->cmap.transp = NULL;
• image->data = qfbdev->shadow + (clips->x1 * 4) + (stride * clips->y1);
• qxl_fb_image_init(&qxl_fb_image, qdev, info, NULL);
• qxl_draw_opaque_fb(&qxl_fb_image, stride);
• return 0;

+}

+static const struct drm_framebuffer_funcs qxlfb_fb_funcs = {

• .destroy = qxl_user_framebuffer_destroy,
• .dirty = qxlfb_framebuffer_dirty,

+};

static int qxlfb_create(struct qxl_fbdev *qfbdev, struct drm_fb_helper_surface_size *sizes) { @@ -383,7 +271,8 @@ static int qxlfb_create(struct qxl_fbdev *qfbdev,

info->par = qfbdev;

• qxl_framebuffer_init(qdev->ddev, &qfbdev->qfb, &mode_cmd, gobj);

• qxl_framebuffer_init(qdev->ddev, &qfbdev->qfb, &mode_cmd, gobj,

• 	     &qxlfb_fb_funcs);
  
  

  fb = &qfbdev->qfb.base;

@@ -504,7 +393,6 @@ int qxl_fbdev_init(struct qxl_device *qdev) qfbdev->qdev = qdev; qdev->mode_info.qfbdev = qfbdev; spin_lock_init(&qfbdev->delayed_ops_lock);

• spin_lock_init(&qfbdev->dirty.lock); INIT_LIST_HEAD(&qfbdev->delayed_ops);

  drm_fb_helper_prepare(qdev->ddev, &qfbdev->helper,

diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index b2977a1..2319800 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -261,10 +261,6 @@ static int qxl_device_init(struct qxl_device *qdev, qdev->gc_queue = create_singlethread_workqueue("qxl_gc"); INIT_WORK(&qdev->gc_work, qxl_gc_work);

• r = qxl_fb_init(qdev);
• if (r)

• return r;
  

• return 0;

}

-- 2.2.2