The documentation about the meaning of the color component bitfield lengths
in pseudocolor modes is inconsistent. Fix it, so that it indicates the
correct interpretation everywhere, i.e. that the 1 << length is the number
of palette entries.
Signed-off-by: Michal Januszewski <[email protected]>
Cc: Krzysztof Helt <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
---
This patch will be followed by a number of patches which attempt to
introduce the correct interpretation of the length field in all fbdev
drivers. A separate patch will be sent for each driver so that
in case I made a mistake in a specific driver, the other ones will
not be affected.
diff --git a/drivers/video/skeletonfb.c b/drivers/video/skeletonfb.c
index df53365..3810dae 100644
--- a/drivers/video/skeletonfb.c
+++ b/drivers/video/skeletonfb.c
@@ -309,8 +309,8 @@ static int xxxfb_setcolreg(unsigned regno, unsigned red, unsigned green,
*
* Pseudocolor:
* var->{color}.offset is 0
- * var->{color}.length contains width of DAC or the number of unique
- * colors available (color depth)
+ * var->{color}.length is set so that 1 << length is the number of
+ * available palette entries
* pseudo_palette is not used
* RAMDAC[X] is programmed to (red, green, blue)
* color depth = var->{color}.length
diff --git a/drivers/video/vfb.c b/drivers/video/vfb.c
index 93fe08d..e4eeb91 100644
--- a/drivers/video/vfb.c
+++ b/drivers/video/vfb.c
@@ -318,13 +318,14 @@ static int vfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
* {hardwarespecific} contains width of RAMDAC
* cmap[X] is programmed to (X << red.offset) | (X << green.offset) | (X << blue.offset)
* RAMDAC[X] is programmed to (red, green, blue)
- *
+ *
* Pseudocolor:
- * uses offset = 0 && length = RAMDAC register width.
* var->{color}.offset is 0
- * var->{color}.length contains widht of DAC
+ * var->{color}.length is set so that 1 << length is the number of
+ * available palette entries
* cmap is not used
* RAMDAC[X] is programmed to (red, green, blue)
+ *
* Truecolor:
* does not use DAC. Usually 3 are present.
* var->{color}.offset contains start of bitfield
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 31527e1..ab924b1 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -172,8 +172,11 @@ struct fb_fix_screeninfo {
/* Interpretation of offset for color fields: All offsets are from the right,
* inside a "pixel" value, which is exactly 'bits_per_pixel' wide (means: you
* can use the offset as right argument to <<). A pixel afterwards is a bit
- * stream and is written to video memory as that unmodified. This implies
- * big-endian byte order if bits_per_pixel is greater than 8.
+ * stream and is written to video memory as that unmodified.
+ *
+ * For pseudocolor, offset is always 0 and length, which should be the same
+ * for all color components, indicates the number of available palette entries
+ * (i.e. # of entries = 1 << length).
*/
struct fb_bitfield {
__u32 offset; /* beginning of bitfield */
On Tue, Mar 31, 2009 at 00:00, Michal Januszewski <[email protected]> wrote:
> The documentation about the meaning of the color component bitfield lengths
> in pseudocolor modes is inconsistent. Fix it, so that it indicates the
> correct interpretation everywhere, i.e. that the 1 << length is the number
> of palette entries.
>
> Signed-off-by: Michal Januszewski <[email protected]>
> Cc: Krzysztof Helt <[email protected]>
> Cc: Ville Syrjälä <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
Acked-by: Geert Uytterhoeven <[email protected]>
Except for this:
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -172,8 +172,11 @@ struct fb_fix_screeninfo {
> /* Interpretation of offset for color fields: All offsets are from the right,
> * inside a "pixel" value, which is exactly 'bits_per_pixel' wide (means: you
> * can use the offset as right argument to <<). A pixel afterwards is a bit
> - * stream and is written to video memory as that unmodified. This implies
> - * big-endian byte order if bits_per_pixel is greater than 8.
> + * stream and is written to video memory as that unmodified.
> + *
> + * For pseudocolor, offset is always 0 and length, which should be the same
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is not correct. Offset can be non-zero, e.g. for a 8 bpp frame buffer with
64 palette entries, where the palette index is stored in the upper 6 bits of the
8-bit pixel value, offset would be 2, and length would be 6.
Not that I've seen that (so far)...
> + * for all color components, indicates the number of available palette entries
> + * (i.e. # of entries = 1 << length).
> */
> struct fb_bitfield {
> __u32 offset; /* beginning of bitfield */
>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
The documentation about the meaning of the color component bitfield lengths
in pseudocolor modes is inconsistent. Fix it, so that it indicates the
correct interpretation everywhere, i.e. that 1 << length is the number
of palette entries.
Signed-off-by: Michal Januszewski <[email protected]>
Cc: Krzysztof Helt <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
---
This version of the patch incorporates Geert's comments about the
meaning of the offset field for pseudocolor modes.
diff --git a/drivers/video/skeletonfb.c b/drivers/video/skeletonfb.c
index df53365..63cf3bf 100644
--- a/drivers/video/skeletonfb.c
+++ b/drivers/video/skeletonfb.c
@@ -308,9 +308,11 @@ static int xxxfb_setcolreg(unsigned regno, unsigned red, unsigned green,
* color depth = SUM(var->{color}.length)
*
* Pseudocolor:
- * var->{color}.offset is 0
- * var->{color}.length contains width of DAC or the number of unique
- * colors available (color depth)
+ * var->{color}.offset is 0 unless the palette index takes less than
+ * bits_per_pixel bits and is stored in the upper
+ * bits of the pixel value
+ * var->{color}.length is set so that 1 << length is the number of
+ * available palette entries
* pseudo_palette is not used
* RAMDAC[X] is programmed to (red, green, blue)
* color depth = var->{color}.length
diff --git a/drivers/video/vfb.c b/drivers/video/vfb.c
index 93fe08d..7d58c8a 100644
--- a/drivers/video/vfb.c
+++ b/drivers/video/vfb.c
@@ -318,13 +318,16 @@ static int vfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
* {hardwarespecific} contains width of RAMDAC
* cmap[X] is programmed to (X << red.offset) | (X << green.offset) | (X << blue.offset)
* RAMDAC[X] is programmed to (red, green, blue)
- *
+ *
* Pseudocolor:
- * uses offset = 0 && length = RAMDAC register width.
- * var->{color}.offset is 0
- * var->{color}.length contains widht of DAC
+ * var->{color}.offset is 0 unless the palette index takes less than
+ * bits_per_pixel bits and is stored in the upper
+ * bits of the pixel value
+ * var->{color}.length is set so that 1 << length is the number of available
+ * palette entries
* cmap is not used
* RAMDAC[X] is programmed to (red, green, blue)
+ *
* Truecolor:
* does not use DAC. Usually 3 are present.
* var->{color}.offset contains start of bitfield
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 31527e1..0d52630 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -172,8 +172,12 @@ struct fb_fix_screeninfo {
/* Interpretation of offset for color fields: All offsets are from the right,
* inside a "pixel" value, which is exactly 'bits_per_pixel' wide (means: you
* can use the offset as right argument to <<). A pixel afterwards is a bit
- * stream and is written to video memory as that unmodified. This implies
- * big-endian byte order if bits_per_pixel is greater than 8.
+ * stream and is written to video memory as that unmodified.
+ *
+ * For pseudocolor: offset and length should be the same for all color
+ * components. Offset specifies the position of the least significant bit
+ * of the pallette index in a pixel value. Length indicates the number
+ * of available palette entries (i.e. # of entries = 1 << length).
*/
struct fb_bitfield {
__u32 offset; /* beginning of bitfield */
On Wed, 1 Apr 2009 00:28:35 +0200
Michal Januszewski <[email protected]> wrote:
> The documentation about the meaning of the color component bitfield lengths
> in pseudocolor modes is inconsistent. Fix it, so that it indicates the
> correct interpretation everywhere, i.e. that 1 << length is the number
> of palette entries.
>
> Signed-off-by: Michal Januszewski <[email protected]>
> Cc: Krzysztof Helt <[email protected]>
> Cc: Ville Syrj?l? <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> ---
> This version of the patch incorporates Geert's comments about the
> meaning of the offset field for pseudocolor modes.
>
Acked-by: Krzysztof Helt <[email protected]>
----------------------------------------------------------------------
Twoj znajomy na stronie glownej portalu? Wkrec go! ;)
Sprawdz >>> http://link.interia.pl/f20f2