Re: [PATCH 04/05] video: deferred io sys helpers - metronome

On Mon, Dec 22, 2008 at 12:53 AM, Magnus Damm [off-list ref] wrote:

Okay, overall seems okay. But I'm not confident about this specific
change above. The meaning of dx, etc is not clear to me. Okay, I think
I found a problem with above. Here's my explanation, please correct me
if I'm wrong in my reading of the code you've posted. The typical use
case is just a single mmap client (eg: Xfbdev) and nothing else. No
fbcon. The only code path I see that sets defio->dx is in the
write/blit/* paths via the defio_touch in patch 1/5 and none in the
mmap path so with this implementation, all mmap clients would
repeatedly only hit the above full update scenario and never use the
page based update mechanism.

+       } else {
+               /* walk the written page list and swizzle the data */
+               list_for_each_entry(cur, &fbdefio->pagelist, lru) {
+                       cksum = metronomefb_dpy_update_page(par,
+                                               (cur->index << PAGE_SHIFT));
+                       par->metromem_img_csum -= par->csum_table[cur->index];
+                       par->csum_table[cur->index] = cksum;
+                       par->metromem_img_csum += cksum;
+               }
       }

       metronome_display_cmd(par);
 }

-static void metronomefb_fillrect(struct fb_info *info,
-                                  const struct fb_fillrect *rect)
-{
-       struct metronomefb_par *par = info->par;
-
-       sys_fillrect(info, rect);
-       metronomefb_dpy_update(par);
-}
-
-static void metronomefb_copyarea(struct fb_info *info,
-                                  const struct fb_copyarea *area)
-{
-       struct metronomefb_par *par = info->par;
-
-       sys_copyarea(info, area);
-       metronomefb_dpy_update(par);
-}
-
-static void metronomefb_imageblit(struct fb_info *info,
-                               const struct fb_image *image)
-{
-       struct metronomefb_par *par = info->par;
-
-       sys_imageblit(info, image);
-       metronomefb_dpy_update(par);
-}
-
-/*
- * this is the slow path from userspace. they can seek and write to
- * the fb. it is based on fb_sys_write
- */
-static ssize_t metronomefb_write(struct fb_info *info, const char __user *buf,
-                               size_t count, loff_t *ppos)
-{
-       struct metronomefb_par *par = info->par;
-       unsigned long p = *ppos;
-       void *dst;
-       int err = 0;
-       unsigned long total_size;
-
-       if (info->state != FBINFO_STATE_RUNNING)
-               return -EPERM;
-
-       total_size = info->fix.smem_len;
-
-       if (p > total_size)
-               return -EFBIG;
-
-       if (count > total_size) {
-               err = -EFBIG;
-               count = total_size;
-       }
-
-       if (count + p > total_size) {
-               if (!err)
-                       err = -ENOSPC;
-
-               count = total_size - p;
-       }
-
-       dst = (void __force *)(info->screen_base + p);
-
-       if (copy_from_user(dst, buf, count))
-               err = -EFAULT;
-
-       if  (!err)
-               *ppos += count;
-
-       metronomefb_dpy_update(par);
-
-       return (err) ? err : count;
-}
-
 static struct fb_ops metronomefb_ops = {
       .owner          = THIS_MODULE,
-       .fb_write       = metronomefb_write,
-       .fb_fillrect    = metronomefb_fillrect,
-       .fb_copyarea    = metronomefb_copyarea,
-       .fb_imageblit   = metronomefb_imageblit,
+       .fb_read        = fb_deferred_io_read,
+       .fb_write       = fb_deferred_io_write,
+       .fb_fillrect    = fb_deferred_io_fillrect,
+       .fb_copyarea    = fb_deferred_io_copyarea,
+       .fb_imageblit   = fb_deferred_io_imageblit,
 };

Other than the above problem, looks okay and its nice to get rid of
all that stuff above. :-)

Thanks,
jaya