Re: PATCH: bugfix für RadeonFB (against 2.4.22-ac4, bug in 2.6.0-test9, too)

[Ronald Lembcke <es186@fen-net.de>]

On Wed, Nov 19, 2003 at 10:50:22AM +0100, Geert Uytterhoeven wrote:
> Your change is not correct: bpp is the _physical_ bits per pixel, i.e. it's 16
> for both color depth 15 (5/5/5 mode) and color depth 16 (5/6/5).
> 
> To differentiate between 5/5/5 and 5/6/5, you have to look at green.length, and
> apply standard fbdev fit-our-round-up[*] rules.
I don't see where this rule applies here. I searched on google, but
did't find anything about rounding of color depth.

> Note that some hardware in addition does RGBA4444, too.
Yes, but that nothing to do with these patches.

Sorry, I don't see, where my patches for intelfb and radonfb (as said
before, I'm not sure about the imsttfb patch anyway) make the driver
less correct. Where are they wrong?

Nothing about the use of bits_per_pixel is changed.
The only change is whether 555 or 565 is default.

If those patches are wrong, than matroxfb and rivafb are buggy, too:

matrox/matroxfb_crtc2.c:
	if (var->bits_per_pixel == 16) {
		if (var->green.length == 5) {
			var->red.offset = 10;
			var->red.length = 5;
			var->green.offset = 5;
			var->green.length = 5;
			var->blue.offset = 0;
			var->blue.length = 5;
			var->transp.offset = 15;
			var->transp.length = 1;
			*mode = 15;
		} else {
			var->red.offset = 11;
			var->red.length = 5;
			var->green.offset = 5;
			var->green.length = 6;
			var->blue.offset = 0;
			var->blue.length = 5;
			var->transp.offset = 0;
			var->transp.length = 0;
		}



riva/fbdev.c
        switch (v.bits_per_pixel) {
[...]
	case 9 ... 15:
		v.green.length = 5;
		/* fall through */
	case 16:
		v.bits_per_pixel = 16;
		nom = 2;
		den = 1;
		if (v.green.length == 5) {
			/* 0rrrrrgg gggbbbbb */
			v.red.offset = 10;
			v.green.offset = 5;
			v.blue.offset = 0;
			v.red.length = 5;
			v.green.length = 5;
			v.blue.length = 5;
		} else {
			/* rrrrrggg gggbbbbb */
			v.red.offset = 11;
			v.green.offset = 5;
			v.blue.offset = 0;
			v.red.length = 5;
			v.green.length = 6;
			v.blue.length = 5;
		}
		break;


They do the same thing as intelfb and radonfb with the patches applied.
So both must be wrong or right.

Roni