On Wed, Apr 20, 2016 at 05:25:29PM +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.
> The deferred mmap functionality is kept disabled by default, because of the
> list corruption problems mentioned in commit 677d23b70bf9
> ("drm/udl: disable fb_defio by default").
> This patch has only been compile tested.
>
> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
> ---
> drivers/gpu/drm/udl/udl_drv.h | 2 -
> drivers/gpu/drm/udl/udl_fb.c | 152
> ++++--------------------------------------
> 2 files changed, 12 insertions(+), 142 deletions(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 4a064ef..0b03d34 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -81,8 +81,6 @@ struct udl_framebuffer {
> struct drm_framebuffer base;
> struct udl_gem_object *obj;
> bool active_16; /* active on the 16-bit channel */
> - int x1, y1, x2, y2; /* dirty rect */
> - spinlock_t dirty_lock;
> };
>
> #define to_udl_fb(x) container_of(x, struct udl_framebuffer, base)
> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
> index a52de2f..b44d4a7 100644
> --- a/drivers/gpu/drm/udl/udl_fb.c
> +++ b/drivers/gpu/drm/udl/udl_fb.c
> @@ -77,68 +77,6 @@ static uint16_t rgb16(uint32_t col)
> }
> #endif
>
> -/*
> - * NOTE: fb_defio.c is holding info->fbdefio.mutex
> - * Touching ANY framebuffer memory that triggers a page fault
> - * in fb_defio will cause a deadlock, when it also tries to
> - * grab the same mutex.
> - */
> -static void udlfb_dpy_deferred_io(struct fb_info *info,
> - struct list_head *pagelist)
> -{
> - struct page *cur;
> - struct fb_deferred_io *fbdefio = info->fbdefio;
> - struct udl_fbdev *ufbdev = info->par;
> - struct drm_device *dev = ufbdev->ufb.base.dev;
> - struct udl_device *udl = dev->dev_private;
> - struct urb *urb;
> - char *cmd;
> - cycles_t start_cycles, end_cycles;
> - int bytes_sent = 0;
> - int bytes_identical = 0;
> - int bytes_rendered = 0;
> -
> - if (!fb_defio)
> - return;
> -
> - start_cycles = get_cycles();
> -
> - urb = udl_get_urb(dev);
> - if (!urb)
> - return;
> -
> - cmd = urb->transfer_buffer;
> -
> - /* walk the written page list and render each to device */
> - list_for_each_entry(cur, &fbdefio->pagelist, lru) {
> -
> - if (udl_render_hline(dev, (ufbdev->ufb.base.bits_per_pixel / 8),
> - &urb, (char *) info->fix.smem_start,
> - &cmd, cur->index << PAGE_SHIFT,
> - cur->index << PAGE_SHIFT,
> - PAGE_SIZE, &bytes_identical, &bytes_sent))
> - goto error;
> - bytes_rendered += PAGE_SIZE;
> - }
> -
> - if (cmd > (char *) urb->transfer_buffer) {
> - /* Send partial buffer remaining before exiting */
> - int len = cmd - (char *) urb->transfer_buffer;
> - udl_submit_urb(dev, urb, len);
> - bytes_sent += len;
> - } else
> - udl_urb_completion(urb);
> -
> -error:
> - atomic_add(bytes_sent, &udl->bytes_sent);
> - atomic_add(bytes_identical, &udl->bytes_identical);
> - atomic_add(bytes_rendered, &udl->bytes_rendered);
> - end_cycles = get_cycles();
> - atomic_add(((unsigned int) ((end_cycles - start_cycles)
> - >> 10)), /* Kcycles */
> - &udl->cpu_kcycles_used);
> -}
> -
> int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
> int width, int height)
> {
> @@ -152,9 +90,6 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x,
> int y,
> struct urb *urb;
> int aligned_x;
> int bpp = (fb->base.bits_per_pixel / 8);
> - int x2, y2;
> - bool store_for_later = false;
> - unsigned long flags;
>
> if (!fb->active_16)
> return 0;
> @@ -180,38 +115,6 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x,
> int y,
> (y + height > fb->base.height))
> return -EINVAL;
>
> - /* if we are in atomic just store the info
> - can't test inside spin lock */
> - if (in_atomic())
> - store_for_later = true;
> -
> - x2 = x + width - 1;
> - y2 = y + height - 1;
> -
> - spin_lock_irqsave(&fb->dirty_lock, flags);
> -
> - if (fb->y1 < y)
> - y = fb->y1;
> - if (fb->y2 > y2)
> - y2 = fb->y2;
> - if (fb->x1 < x)
> - x = fb->x1;
> - if (fb->x2 > x2)
> - x2 = fb->x2;
> -
> - if (store_for_later) {
> - fb->x1 = x;
> - fb->x2 = x2;
> - fb->y1 = y;
> - fb->y2 = y2;
> - spin_unlock_irqrestore(&fb->dirty_lock, flags);
> - return 0;
> - }
> -
> - fb->x1 = fb->y1 = INT_MAX;
> - fb->x2 = fb->y2 = 0;
> -
> - spin_unlock_irqrestore(&fb->dirty_lock, flags);
> start_cycles = get_cycles();
>
> urb = udl_get_urb(dev);
> @@ -219,14 +122,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int
> x, int y,
> return 0;
> cmd = urb->transfer_buffer;
>
> - for (i = y; i <= y2 ; i++) {
> + for (i = y; i < height ; i++) {
> const int line_offset = fb->base.pitches[0] * i;
> const int byte_offset = line_offset + (x * bpp);
> const int dev_byte_offset = (fb->base.width * bpp * i) + (x *
> bpp);
> if (udl_render_hline(dev, bpp, &urb,
> (char *) fb->obj->vmapping,
> &cmd, byte_offset, dev_byte_offset,
> - (x2 - x + 1) * bpp,
> + width * bpp,
> &bytes_identical, &bytes_sent))
> goto error;
> }
> @@ -283,36 +186,6 @@ static int udl_fb_mmap(struct fb_info *info, struct
> vm_area_struct *vma)
> return 0;
> }
>
> -static void udl_fb_fillrect(struct fb_info *info, const struct fb_fillrect
> *rect)
> -{
> - struct udl_fbdev *ufbdev = info->par;
> -
> - sys_fillrect(info, rect);
> -
> - udl_handle_damage(&ufbdev->ufb, rect->dx, rect->dy, rect->width,
> - rect->height);
> -}
> -
> -static void udl_fb_copyarea(struct fb_info *info, const struct fb_copyarea
> *region)
> -{
> - struct udl_fbdev *ufbdev = info->par;
> -
> - sys_copyarea(info, region);
> -
> - udl_handle_damage(&ufbdev->ufb, region->dx, region->dy, region->width,
> - region->height);
> -}
> -
> -static void udl_fb_imageblit(struct fb_info *info, const struct fb_image
> *image)
> -{
> - struct udl_fbdev *ufbdev = info->par;
> -
> - sys_imageblit(info, image);
> -
> - udl_handle_damage(&ufbdev->ufb, image->dx, image->dy, image->width,
> - image->height);
> -}
> -
> /*
> * It's common for several clients to have framebuffer open simultaneously.
> * e.g. both fbcon and X. Makes things interesting.
> @@ -330,20 +203,20 @@ static int udl_fb_open(struct fb_info *info, int user)
>
> ufbdev->fb_count++;
>
> - if (fb_defio && (info->fbdefio == NULL)) {
> - /* enable defio at last moment if not disabled by client */
> + if (!info->fbdefio) {
> + /* enable defio at last moment */
>
> struct fb_deferred_io *fbdefio;
>
> fbdefio = kmalloc(sizeof(struct fb_deferred_io), GFP_KERNEL);
> -
> if (fbdefio) {
> fbdefio->delay = DL_DEFIO_WRITE_DELAY;
> - fbdefio->deferred_io = udlfb_dpy_deferred_io;
> + fbdefio->deferred_io = drm_fb_helper_deferred_io;
Why all these changes here? I figured just exchanging the deferred_io
pointer should be all that's really needed in the setup code?
-Daniel
> + info->fbdefio = fbdefio;
> + fb_deferred_io_init(info);
> + if (!fb_defio) /* see commit 677d23b */
> + info->fbops->fb_mmap = udl_fb_mmap;
> }
> -
> - info->fbdefio = fbdefio;
> - fb_deferred_io_init(info);
> }
>
> pr_notice("open /dev/fb%d user=%d fb_info=%p count=%d\n",
> @@ -379,9 +252,9 @@ static struct fb_ops udlfb_ops = {
> .owner = THIS_MODULE,
> .fb_check_var = drm_fb_helper_check_var,
> .fb_set_par = drm_fb_helper_set_par,
> - .fb_fillrect = udl_fb_fillrect,
> - .fb_copyarea = udl_fb_copyarea,
> - .fb_imageblit = udl_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,
> @@ -458,7 +331,6 @@ udl_framebuffer_init(struct drm_device *dev,
> {
> int ret;
>
> - spin_lock_init(&ufb->dirty_lock);
> ufb->obj = obj;
> drm_helper_mode_fill_fb_struct(&ufb->base, mode_cmd);
> ret = drm_framebuffer_init(dev, &ufb->base, &udlfb_funcs);
> --
> 2.2.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
