Re: [Linux-fbdev-devel] [PATCH 1/1 2.6.13-rc4] framebuffer: new driver for cyberblade/i1 core

[Geert Uytterhoeven <geert@linux-m68k.org>]

On Sun, 31 Jul 2005, Antonino A. Daplas wrote:
> Knut Petersen wrote:
> > +    //
> > +    // interlaced modes are broken, fail if one is requested
> > +    //
> > +    if (var->vmode & FB_VMODE_INTERLACED)
> > +        return -EINVAL;
> 
> You are too strict :-)  You can always clear FB_VMODE_INTERLACED and
> return success.  This is acceptable behavior.

I would not try to modify the passed FB_VMODE_* flags.

> > +    //
> > +    // fail if requested resolution is higher than physical
> > +    // flatpanel resolution
> > +    //
> > +    if (flatpanel && nativex && var->xres > nativex)
> > +        return -EINVAL;
> > +
> 
> This is acceptable behavior, but you can always return
> with var->xres = nativex.
> 
> > +    //
> > +    // xres != xres_virtual is broken, fail if such an
> > +    // unusual mode is requested
> > +    //
> > +    if (var->xres != var->xres_virtual)
> > +        return -EINVAL;
> 
> Same here, var->xres = var->xres_virtual.

Only when rounding _up_. Rounding down is not allowed.

> > +    // calc max yres_virtual that would fit in memory
> > +    // and max yres_virtual that could be used for scrolling
> > +    // and use minimum of the results as maxvyres
> > +    //
> > +    // adjust vyres_virtual to maxvyres if necessary
> > +    // fail if requested yres is bigger than maxvyres
> > +    //
> > +    s = (0x1fffff / (var->xres * bpp/8)) + var->yres;
> > +    t = info->fix.smem_len / (var->xres * bpp/8);
> > +    maxvyres = t < s ? t : s;
> > +    if (maxvyres < var->yres_virtual)
> > +        var->yres_virtual=maxvyres;
> > +    if (maxvyres < var->yres)
> > +        return -EINVAL;
> > +
> > +    switch (bpp) {
> > +        case 8:
> > +            var->red.offset = 0;
> > +            var->green.offset = 0;
> > +            var->blue.offset = 0;
> > +            var->red.length = 6;
> > +            var->green.length = 6;
> > +            var->blue.length = 6;
> > +            break;
> > +        case 16:
> > +            var->red.offset = 11;
> > +            var->green.offset = 5;
> > +            var->blue.offset = 0;
> > +            var->red.length = 5;
> > +            var->green.length = 6;
> > +            var->blue.length = 5;
> > +            break;
> > +        case 32:
> > +            var->red.offset = 16;
> > +            var->green.offset = 8;
> > +            var->blue.offset = 0;
> > +            var->red.length = 8;
> > +            var->green.length = 8;
> > +            var->blue.length = 8;
> > +            break;
> > +        default:
> > +            return -EINVAL;
> 	Again, you can always round off bits_per_pixel to
> ones that the driver can support.

Only when rounding _up_.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/