Re: [TRIVIAL PATCH] FBDEV: Small impact patch for fbdev

On Wed, 2002-12-11 at 10:59, James Simmons wrote:

quoted

Here's a diff to correct several small things that escaped through the
cracks.

Ug. Now that a wider test base is being done and more drivers actually
compile we are going to see more cracks.

quoted

1.  The YNOMOVE scrollmode for non-accelerated drivers is just very slow
because of a lot of block moves (leads to slow and jerky scrolling in
vesafb with ypanning enabled).  Depending on var->accel_flags, set the
scrollmode to either YREDRAW or YNOMOVE. For drivers with hardware
acceleration, set var->accel_flags to nonzero for max speed.

I think the scrollmode is better determined on a case-to-case basis, but
the above should be a good enough differentiation.

Thanks. I have had several emails complementing the speed improvements.
Another speed boost will be a plus.

Yes, probably not much at low pixel depths, but at high pixel depths and
resolutions, 2.5 should be much better than 2.4 (even 2-3x faster in
some cases).  And for drivers with hardware acceleration, whatever the
color depth, it will be much better because they were written
specifically for them. 

Personally, the performance gain is just a minor side-effect. The
greatest benefit in all this is the separation of the fbdev-fbcon
structure into components, where each component has a defined use and
function, and very much independent of each other.  This really makes it
easier to understand the code, port or write drivers too it, write
smaller and more efficient code, and even debug.  And you basically did
all this, so great job :-)  

 > > 2.  fb_pan_display() always returns an error.  User apps will
complain.

Fixed. Actually I used the following code.

int fb_pan_display(struct fb_var_screeninfo *var, struct fb_info *info)
{
        int xoffset = var->xoffset;
        int yoffset = var->yoffset;
        int err;

        if (xoffset < 0 || yoffset < 0 || info->fbops->fb_pan_display ||
            xoffset + info->var.xres > info->var.xres_virtual ||
            yoffset + info->var.yres > info->var.yres_virtual)
                return -EINVAL;
        if ((err = info->fbops->fb_pan_display(var, info)))
                return err;
        info->var.xoffset = var->xoffset;
        info->var.yoffset = var->yoffset;
        if (var->vmode & FB_VMODE_YWRAP)

instead. The reason is I didn't like the idea of xoffset and yoffset being
changed even if the hardware panning function failed. Comments?

Yes, I think thats better.

Tony