Problem with current fb_get_color_depth function

Problem with current fb_get_color_depth function

[Joshua Kwan]

[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]

Hi Antonino, lists, etc.,

I noticed that after 2.6.9-rc1 or so my framebuffer logo stopped
appearing. Well, I dove into the fb layer code, noticed this change from
you:

int fb_get_color_depth(struct fb_info *info)
{
    struct fb_var_screeninfo *var = &info->var;

    if (var->green.length == var->blue.length &&
        var->green.length == var->red.length &&
        !var->green.offset && !var->blue.offset &&
        !var->red.offset)
        return var->green.length;
    else
        return (var->green.length + var->red.length +
            var->blue.length);
}

That was interesting, because here's what radeonfb does:

        switch (var_to_depth(&v)) {
[...]
                case 16:
                        nom = 2;
                        den = 1;
                        v.red.offset = 11;
                        v.green.offset = 5;
                        v.blue.offset = 0;
                        v.red.length = 5;
                        v.green.length = 6;
                        v.blue.length = 5;
                        v.transp.offset = v.transp.length = 0;
                        break;
[...]

So somehow that first conditional was firing, although I'm not sure how,
because a printk showed that the depth returned was 6. (fb_get_color_depth
should have jumped to the return (R + G + B) part right after it noticed
blue length was not equal to green length. Creepy.)

Well, whatever. The 224-color logo was ignored. This is what I did to
fix it:

--- a/drivers/video/fbmem.c	2004/09/23 01:19:45	1.102
+++ b/drivers/video/fbmem.c	2004/10/10 22:47:14
@@ -65,10 +65,8 @@
 {
 	struct fb_var_screeninfo *var = &info->var;
 
-	if (var->green.length == var->blue.length &&
-	    var->green.length == var->red.length &&
-	    !var->green.offset && !var->blue.offset &&
-	    !var->red.offset)
+	/* If grayscale, all values will be equal */
+	if (var->grayscale)
 		return var->green.length;
 	else
 		return (var->green.length + var->red.length +

because on advice from Andrew Suffield and Matthew Garrett the first
conditional was a roundabout way to check for grayscale displays, where
you can't actually have 24 bits (8 + 8 + 8) of gray. So it is suitable
to just return one of the values arbitrarily. But I noticed there was
also a grayscale variable, so I substituted that for the conditional
and my logo reappeared again :)

So is radeonfb or fb_get_color_depth at fault here? Or was the logo
never appropriate for my display in the first place? (Unlikely, the
CLUT224 linux logo always displayed perfectly for me until now.)

Thanks.

-- 
Joshua Kwan

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 881 bytes --]

Re: Problem with current fb_get_color_depth function

[Antonino A. Daplas]

On Monday 11 October 2004 06:59, Joshua Kwan wrote:
> because on advice from Andrew Suffield and Matthew Garrett the first
> conditional was a roundabout way to check for grayscale displays, where
> you can't actually have 24 bits (8 + 8 + 8) of gray. So it is suitable
> to just return one of the values arbitrarily. But I noticed there was
> also a grayscale variable, so I substituted that for the conditional
> and my logo reappeared again :)
>
> So is radeonfb or fb_get_color_depth at fault here? Or was the logo
> never appropriate for my display in the first place? (Unlikely, the
> CLUT224 linux logo always displayed perfectly for me until now.)
>

This is because radeonfb chooses directcolor for its visual.  With RGB565,
the depth == 16, true.  However, unlike truecolor where color values are
hardwired to linear values, directcolor looks at the hardware CLUT.

So, with directcolor RGB656, these are the CLUT lengths:

Red and Blue = 32 (2 ^ 5)
Green        = 64 (2 ^ 6)

(And you can see this in radeonfb_base:radeonfb_setcolreg())

		if (rinfo->bpp == 16) {
			pindex = regno * 8;

			if (rinfo->depth == 16 && regno > 63)
				return 1;
			if (rinfo->depth == 15 && regno > 31)
				return 1;

However, linux_logo_224 requires a CLUT with these lengths:

Red = Green = Blue = 224;

So, linux_logo_224 cannot be drawn when visual is directcolor at RGB555 or
RGB565 because the logo clut requirements exceeds the hardware clut
capability. You need to use a logo image with a lower depth such as the
16-color logo, linux_logo_16.

In short, fb_get_color_depth() and radeonfb both do the correct thing, but
you need a logo with a lower color depth. Or choose RGB888 or higher, or
set the visual to truecolor.

Tony

PS: Note that this behavior is the same as 2.4 behavior (224-color logo is
only chosen if directcolor and bpp >= 24).  It might have worked before, and
this is probably due to the directcolor clut being ramped to truecolor
values. However, the correct solution is to use a 16-color logo.




 

Re: Problem with current fb_get_color_depth function

[Joshua Kwan]

[-- Attachment #1: Type: text/plain, Size: 1545 bytes --]

[ long overdue follow-up ]

On Mon, Oct 11, 2004 at 08:33:10AM +0800, Antonino A. Daplas wrote:
> So, linux_logo_224 cannot be drawn when visual is directcolor at RGB555 or
> RGB565 because the logo clut requirements exceeds the hardware clut
> capability. You need to use a logo image with a lower depth such as the
> 16-color logo, linux_logo_16.

This is weird, because removing that conditional from fb_get_color_depth
allows a 224-color logo to show correctly on my Radeon framebuffer, in
full color.

Otherwise, it is dithered to kingdom come and mostly appears all orange
and black.

You may be right conceptually, but the fact of the matter is that this
is a regression because 224-color logos work perfectly with the old
fb_get_color_depth. So what is the real problem?

> In short, fb_get_color_depth() and radeonfb both do the correct thing, but
> you need a logo with a lower color depth. Or choose RGB888 or higher, or
> set the visual to truecolor.

What does that last sentence mean? -- I have no idea...

> PS: Note that this behavior is the same as 2.4 behavior (224-color logo is
> only chosen if directcolor and bpp >= 24).  It might have worked before, and
> this is probably due to the directcolor clut being ramped to truecolor
> values. However, the correct solution is to use a 16-color logo.

Oh, I see, I didn't read this before.

Well, 16-color logos being used when 224-color ones work perfectly
enough 99% of the time is pretty bad aesthetically, if you ask me.

-- 
Joshua Kwan

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 881 bytes --]

Re: [Linux-fbdev-devel] Re: Problem with current fb_get_color_depth function

[Antonino A. Daplas]

On Tuesday 02 November 2004 13:55, Joshua Kwan wrote:
> [ long overdue follow-up ]
>
> On Mon, Oct 11, 2004 at 08:33:10AM +0800, Antonino A. Daplas wrote:
> > So, linux_logo_224 cannot be drawn when visual is directcolor at RGB555
> > or RGB565 because the logo clut requirements exceeds the hardware clut
> > capability. You need to use a logo image with a lower depth such as the
> > 16-color logo, linux_logo_16.
>
> This is weird, because removing that conditional from fb_get_color_depth
> allows a 224-color logo to show correctly on my Radeon framebuffer, in
> full color.
>
> Otherwise, it is dithered to kingdom come and mostly appears all orange
> and black.
>
> You may be right conceptually, but the fact of the matter is that this

It is correct.  You cannot force 224, 224, 224 colors on hardware that
accepts only 32, 64, 32.

> is a regression because 224-color logos work perfectly with the old
> fb_get_color_depth. So what is the real problem?

Other drivers do not show the correct logo colors if using 224-color logo
in Directcolor RGB565.  And you cannot consider this a regression since
2.4 behaves in the same way.

>
> > In short, fb_get_color_depth() and radeonfb both do the correct thing,
> > but you need a logo with a lower color depth. Or choose RGB888 or higher,
> > or set the visual to truecolor.
>
> What does that last sentence mean? -- I have no idea...

Meaning, if you want 224-color logos, boot at 8-bit pseudocolor or 24-32 bit
Directcolor.  If the hardware supports it, but in truecolor at >=8 bpp.

>
> > PS: Note that this behavior is the same as 2.4 behavior (224-color logo
> > is only chosen if directcolor and bpp >= 24).  It might have worked
> > before, and this is probably due to the directcolor clut being ramped to
> > truecolor values. However, the correct solution is to use a 16-color
> > logo.
>
> Oh, I see, I didn't read this before.

Yep, this is a code snippet from the 2.4 logo code:

        if (p->visual == FB_VISUAL_DIRECTCOLOR) {
	    ...	
	    if (depth >= 24 && (depth % 8) == 0) {
		/* have at least 8 bits per color */
		src = logo;
		bdepth = depth/8;
		...
	    else if (depth >= 12 && depth <= 23) {
	        /* have 4..7 bits per color, using 16 color image */
		unsigned int pix;
		src = linux_logo16;
		...
>
> Well, 16-color logos being used when 224-color ones work perfectly
> enough 99% of the time is pretty bad aesthetically, if you ask me.

It might work in the radeon, but it doesn't with others.

It's possible to convert the 224 logo to use a 32-entry color map.  If
really wanted, this can be done.

Tony

Re: [Linux-fbdev-devel] Re: Problem with current fb_get_color_depth function

[Geert Uytterhoeven]

On Mon, 1 Nov 2004, Joshua Kwan wrote:
> [ long overdue follow-up ]
> 
> On Mon, Oct 11, 2004 at 08:33:10AM +0800, Antonino A. Daplas wrote:
> > So, linux_logo_224 cannot be drawn when visual is directcolor at RGB555 or
> > RGB565 because the logo clut requirements exceeds the hardware clut
> > capability. You need to use a logo image with a lower depth such as the
> > 16-color logo, linux_logo_16.
> 
> This is weird, because removing that conditional from fb_get_color_depth
> allows a 224-color logo to show correctly on my Radeon framebuffer, in
> full color.
> 
> Otherwise, it is dithered to kingdom come and mostly appears all orange
> and black.
> 
> You may be right conceptually, but the fact of the matter is that this
> is a regression because 224-color logos work perfectly with the old
> fb_get_color_depth. So what is the real problem?
> 
> > In short, fb_get_color_depth() and radeonfb both do the correct thing, but
> > you need a logo with a lower color depth. Or choose RGB888 or higher, or
> > set the visual to truecolor.
> 
> What does that last sentence mean? -- I have no idea...

In directcolor mode, the pixel color is:

    (clut.red[r], clut.green[g], clut.blue[b])

with r, g, and b being the values in the pixel (i.e. they are in the range
0..31 for RGB555, and g in the range 0..63 for RGB565).

In truecolor mode, the pixel color is:

    (r, g, b)

> > PS: Note that this behavior is the same as 2.4 behavior (224-color logo is
> > only chosen if directcolor and bpp >= 24).  It might have worked before, and
> > this is probably due to the directcolor clut being ramped to truecolor
> > values. However, the correct solution is to use a 16-color logo.
> 
> Oh, I see, I didn't read this before.
> 
> Well, 16-color logos being used when 224-color ones work perfectly
> enough 99% of the time is pretty bad aesthetically, if you ask me.

It's very strange that it did work before, since there's no way to set up the
32-entry CLUT (of which BTW the first 16 colors are the standard console
colors) for any random 224-color image in RGB555 mode.

In RGB888 mode (24-bit color) it's possible though, since the CLUT has 256
entries.

Can you show us a screenshot (e.g. using fbgrab) of the old behavior?

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

Re: Re: Problem with current fb_get_color_depth function

[Antonino A. Daplas]

On Tuesday 02 November 2004 13:55, Joshua Kwan wrote:
> [ long overdue follow-up ]
>
> On Mon, Oct 11, 2004 at 08:33:10AM +0800, Antonino A. Daplas wrote:
> > So, linux_logo_224 cannot be drawn when visual is directcolor at RGB555
> > or RGB565 because the logo clut requirements exceeds the hardware clut
> > capability. You need to use a logo image with a lower depth such as the
> > 16-color logo, linux_logo_16.
>
> This is weird, because removing that conditional from fb_get_color_depth
> allows a 224-color logo to show correctly on my Radeon framebuffer, in
> full color.
>
> Otherwise, it is dithered to kingdom come and mostly appears all orange
> and black.
>
> You may be right conceptually, but the fact of the matter is that this
> is a regression because 224-color logos work perfectly with the old
> fb_get_color_depth. So what is the real problem?
>

After giving it a lot of thought, perhaps you are not booting at 16 bpp, but
at 8bpp pseudocolor.  However, radeonfb's default var use only a red, green,
and blue length of 6.  Try this patch and let me know if it helps.

Tony

diff -Nru a/drivers/video/aty/radeon_monitor.c b/drivers/video/aty/radeon_monitor.c
--- a/drivers/video/aty/radeon_monitor.c	2004-10-27 14:58:07 +08:00
+++ b/drivers/video/aty/radeon_monitor.c	2004-10-28 06:04:32 +08:00
@@ -12,9 +12,9 @@
 	.xres_virtual	= 640,
 	.yres_virtual	= 480,
 	.bits_per_pixel = 8,
-	.red		= { 0, 6, 0 },
-	.green		= { 0, 6, 0 },
-	.blue		= { 0, 6, 0 },
+	.red		= { 0, 8, 0 },
+	.green		= { 0, 8, 0 },
+	.blue		= { 0, 8, 0 },
 	.activate	= FB_ACTIVATE_NOW,
 	.height		= -1,
 	.width		= -1,




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [Linux-fbdev-devel] Re: Problem with current fb_get_color_depth function

[Joshua Kwan]

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

On Wed, Nov 03, 2004 at 06:10:12AM +0800, Antonino A. Daplas wrote:
> After giving it a lot of thought, perhaps you are not booting at 16 bpp, but
> at 8bpp pseudocolor.  However, radeonfb's default var use only a red, green,
> and blue length of 6.  Try this patch and let me know if it helps.

The patch rejects against 2.6.10-rc1, but re-adapted, it works. Here is the
updated patch for radeon_monitor.c.

Thanks a lot for your help!

and in case this is final,
Signed-off-by: Joshua Kwan <joshk@triplehelix.org>

-- 
Joshua Kwan

--- a/drivers/video/aty/radeon_monitor.c	2004-11-10 23:38:18.000000000 -0800
+++ b/drivers/video/aty/radeon_monitor.c	2004-11-10 23:38:21.000000000 -0800
@@ -8,7 +8,7 @@
 
 static struct fb_var_screeninfo radeonfb_default_var = {
         640, 480, 640, 480, 0, 0, 8, 0,
-        {0, 6, 0}, {0, 6, 0}, {0, 6, 0}, {0, 0, 0},
+        {0, 8, 0}, {0, 8, 0}, {0, 8, 0}, {0, 0, 0},
         0, 0, -1, -1, 0, 39721, 40, 24, 32, 11, 96, 2,
         0, FB_VMODE_NONINTERLACED
 };

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 881 bytes --]