Re: [Linux-fbdev-devel] [PATCH 01/05] video: deferred io sys helpers - core V2

[Jaya K. <jay...@gm...>]

On Wed, Dec 24, 2008 at 3:29 AM, Magnus Damm <mag...@gm...> wrote:
> From: Magnus Damm <da...@ig...>
>
> Add shared sys helpers to the deferred io code. Instead of duplicating
> the code in each driver we can keep it in one place. This saves a
> few lines. The new helper functions make use of the deferred io delay.
> While at it, keep track of the dirty area to allow partial screen update.

Hi Magnus,

Ok, while the overall patchset looks good, I still had trouble with
the dx, dy, w, h stuff. It had me a bit confused. The above comment
about keeping track of the dirty area made me realize why I think that
portion can be improved. The reason is that we already have a pagelist
to track touched pages and the dx, dy, w, h stuff duplicates that. I
have an idea of how we can avoid this duplication which I'll sketch
below.

> +ssize_t fb_deferred_io_write(struct fb_info *info, const char __user *buf,
> +                            size_t count, loff_t *ppos)
> +{
> +       int ret = fb_sys_write(info, buf, count, ppos);
> +
> +       if (ret > 0)
> +               fb_deferred_io_touch(info, 0, 0, INT_MAX, INT_MAX);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(fb_deferred_io_write);

As you have noticed, doing it using x, y, w, h results in needing to
do things like INT_MAX above and side effects like dx != -1 in the
client drivers. I think that we can avoid this. Instead, I would
prepare the pagelist directly in write, imageblit, etc since we know
exactly which pages have been touched.

I guess this would be better explained if I posted some sample code
rather than just talking about it so I'll do a quick and dirty
implementation for write only in metronomefb so that we can discuss
this further.

Thanks,
jaya