On Sun, Mar 29, 2009 at 09:26:02PM +0200, Geert Uytterhoeven wrote:

On Sun, Mar 29, 2009 at 21:14, Michal Januszewski [off-list ref] wrote:

quoted

On Mon, Mar 2, 2009 at 19:02, Krzysztof Helt [off-list ref] wrote:
A slight omission in the fbdev API I suppose since the LUT entries are

quoted

quoted

nearly always 3*8bits wide. VGA being the exception.

If offsets of all RGB components are the same the length field says
how lone the pallete index is. It does not say anything how long
the LUT entries are. This is the same misunderstanding as done
inside the driver.

Where is this meaning of the length field defined?  While I agree
that interpreting it as the length of the palette seems to make
more sense (considering the current FB API), there are several
places in the fbdev code contradicting this interpretation
(comments in skeletonfb.c, vfb.c, code in vga16fb.c, ..).

Originally it was the width of the color component in the DAC, i.e. 6
for VGA with 18 bit color palettes.
Later is was redefined to be the width of the bitfield in the pixel
data, for consistency with
other visuals (pseudocolor is directcolor where the R, G, B, and A
bitfields overlap),
and to handle hardware where the number of palette entries is not 1 << bpp
(e.g. 64 colors in 8 bpp packed pixels).

That's why vfb.c has an (obsolete) comment:

         * Pseudocolor:
         *    uses offset = 0 && length = RAMDAC register width.
         *    var->{color}.offset is 0
         *    var->{color}.length contains widht of DAC

while the (newer) skeletonfb mentions both:

     * Pseudocolor:
     *    var->{color}.offset is 0
     *    var->{color}.length contains width of DAC or the number of unique
     *                        colors available (color depth)

vga16fb.c is also an old driver.

quoted

IMO, if we're going to fix uvesafb, we should also fix all the
other drivers and documentation along with it.

Yep.

The comment explaining fb_bitfield in linux/fb.h seems to need some
fixing too ie. remove the big-endian comment as I think everyone just
assumes the fb_bitfield values are in the device's native endianness.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

------------------------------------------------------------------------------