Re: Fix 8bpp RGB fields length

["Ville Syrjälä" <syrjala@sci.fi>]

On Sun, Mar 29, 2009 at 09:26:02PM +0200, Geert Uytterhoeven wrote:
> On Sun, Mar 29, 2009 at 21:14, Michal Januszewski <michalj@gmail.com> wrote:
> > On Mon, Mar 2, 2009 at 19:02, Krzysztof Helt <krzysztof.h1@poczta.fm> wrote:
> > A slight omission in the fbdev API I suppose since the LUT entries are
> >>> 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.
> 
> > 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/

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