radeonfb broken

radeonfb broken

[Felix von Leitner]

I am using the radeonfb on a Radeon 9600 Pro with a TFT display, and
radeonfb very recently started to give me a picture at all, so I am
quite happy about that.

However, running mplayer does not work at all on radeonfb.  mplayer
inquires about the color depth, I am using 32 bit color depth for this,
but radeonfb says it's DirectColor instead of TrueColor, so mplayer
tries to initialize the palette and fails.

Also, using fbset to set the mode to 1600x1200 fails.  The mode is
changed, but the text console resolution stays the same.  Worse, a
"setfont" changes back to 1024x768.

Also, I cannot view images on console with fbi or fbv.

Felix

Re: radeonfb broken

[Timothy Miller]

Felix von Leitner wrote:
> I am using the radeonfb on a Radeon 9600 Pro with a TFT display, and
> radeonfb very recently started to give me a picture at all, so I am
> quite happy about that.
> 
> However, running mplayer does not work at all on radeonfb.  mplayer
> inquires about the color depth, I am using 32 bit color depth for this,
> but radeonfb says it's DirectColor instead of TrueColor, so mplayer
> tries to initialize the palette and fails.
> 
> Also, using fbset to set the mode to 1600x1200 fails.  The mode is
> changed, but the text console resolution stays the same.  Worse, a
> "setfont" changes back to 1024x768.
> 
> Also, I cannot view images on console with fbi or fbv.
> 
> Felix


What annoys me most about the Radeon driver is the off-by-one error in 
the bmove routine.  Whenever text is copied to the right or down, it 
gets positioned incorrectly.  I posted the fix, but no one paid attention.

Re: radeonfb broken

[Benjamin Herrenschmidt]

> 
> What annoys me most about the Radeon driver is the off-by-one error in 
> the bmove routine.  Whenever text is copied to the right or down, it 
> gets positioned incorrectly.  I posted the fix, but no one paid attention.

Mayb it was just "missed" in the flow of hundreds of mails that go
through this list. Can you re-sent it to me, and also precise which
kernel version it applies to ?

Ben.

Re: radeonfb broken

[Timothy Miller]

Benjamin Herrenschmidt wrote:
>>What annoys me most about the Radeon driver is the off-by-one error in 
>>the bmove routine.  Whenever text is copied to the right or down, it 
>>gets positioned incorrectly.  I posted the fix, but no one paid attention.
> 
> 
> Mayb it was just "missed" in the flow of hundreds of mails that go
> through this list. Can you re-sent it to me, and also precise which
> kernel version it applies to ?
>

Oh, I failed to mention this bit.

I've seen it in 2.4.22-gentoo-r7 and 2.4.25-gentoo.

The bug is NOT present in Red Hat's 2.4.18-27.7.x

Re: radeonfb broken

[Benjamin Herrenschmidt]

On Sat, 2004-04-17 at 01:58, Timothy Miller wrote:
> Benjamin Herrenschmidt wrote:
> >>What annoys me most about the Radeon driver is the off-by-one error in 
> >>the bmove routine.  Whenever text is copied to the right or down, it 
> >>gets positioned incorrectly.  I posted the fix, but no one paid attention.
> > 
> > 
> > Mayb it was just "missed" in the flow of hundreds of mails that go
> > through this list. Can you re-sent it to me, and also precise which
> > kernel version it applies to ?
> >
> 
> Oh, I failed to mention this bit.
> 
> I've seen it in 2.4.22-gentoo-r7 and 2.4.25-gentoo.
> 
> The bug is NOT present in Red Hat's 2.4.18-27.7.x

Can you send me the fix you posted back then ?

Ben.

Re: radeonfb broken

[Timothy Miller]

Benjamin Herrenschmidt wrote:
> On Sat, 2004-04-17 at 01:58, Timothy Miller wrote:
> 
>>Benjamin Herrenschmidt wrote:
>>
>>>>What annoys me most about the Radeon driver is the off-by-one error in 
>>>>the bmove routine.  Whenever text is copied to the right or down, it 
>>>>gets positioned incorrectly.  I posted the fix, but no one paid attention.
>>>
>>>
>>>Mayb it was just "missed" in the flow of hundreds of mails that go
>>>through this list. Can you re-sent it to me, and also precise which
>>>kernel version it applies to ?
>>>
>>
>>Oh, I failed to mention this bit.
>>
>>I've seen it in 2.4.22-gentoo-r7 and 2.4.25-gentoo.
>>
>>The bug is NOT present in Red Hat's 2.4.18-27.7.x
> 
> 
> Can you send me the fix you posted back then ?


Sorry.  I forgot all about this.  If the description I sent isn't good 
enough, I'll have to make a proper patch.  Is that what you want?  I 
doubt I gave my original email, but its essential content is in my more 
recent posting anyhow.

Re: radeonfb broken

[Benjamin Herrenschmidt]

> Sorry.  I forgot all about this.  If the description I sent isn't good 
> enough, I'll have to make a proper patch.  Is that what you want?  I 
> doubt I gave my original email, but its essential content is in my more 
> recent posting anyhow.

Yes, please.
Ben.

Re: radeonfb broken

[Timothy Miller]

Benjamin Herrenschmidt wrote:
>>What annoys me most about the Radeon driver is the off-by-one error in 
>>the bmove routine.  Whenever text is copied to the right or down, it 
>>gets positioned incorrectly.  I posted the fix, but no one paid attention.
> 
> 
> Mayb it was just "missed" in the flow of hundreds of mails that go
> through this list. Can you re-sent it to me, and also precise which
> kernel version it applies to ?
> 
> Ben.


BTW, now that we're on the topic of Radeon, could someone tell me how to 
tell the kernel the default resolution to use when initializing the 
console?

When using a CRT, it defaults to 640x480.  When I use my Planar PQ191 
19" 1280x1024 monitor, it defaults to 1024x768.  I want it to default to 
1280x1024.  There's a tool, fbset or something like that, which I can 
use AFTER bootup, but trying to put that into init causes all sorts of 
conflicts.  I need to be able to tell the kernel, either at compile time 
or on the boot command line.


Moving off topic, my cries for help from the XFree86 people also seem to 
have gotten lost in the flow of hundreds of usenet messages.  Try as I 
might, I cannot seem to get XFree86 to talk to my monitor at anything 
other than 60Hz.  Even though LCD monitors have a persistent image, 
increasing the frame rate CAN reduce motion blur slightly.


Thanks.

Re: radeonfb broken

[Randy.Dunlap]

On Fri, 16 Apr 2004 12:06:22 -0400 Timothy Miller wrote:

| 
| BTW, now that we're on the topic of Radeon, could someone tell me how to 
| tell the kernel the default resolution to use when initializing the 
| console?
| 
| When using a CRT, it defaults to 640x480.  When I use my Planar PQ191 
| 19" 1280x1024 monitor, it defaults to 1024x768.  I want it to default to 
| 1280x1024.  There's a tool, fbset or something like that, which I can 
| use AFTER bootup, but trying to put that into init causes all sorts of 
| conflicts.  I need to be able to tell the kernel, either at compile time 
| or on the boot command line.

See <Documentation/fb/modedb.txt> for a little help: <quote>

Valid mode specifiers (mode_option argument):

    <xres>x<yres>[-<bpp>][@<refresh>]
    <name>[-<bpp>][@<refresh>]

with <xres>, <yres>, <bpp> and <refresh> decimal numbers and <name> a string.
Things between square brackets are optional.
</quote>


I haven't done this, but I've seen emails using syntax like so:

(from /etc/lilo.conf:)

append="video=radeonfb:1024x768-32@100"

or:
append="video=radeonfb radeonfb.mode_option=1280x1024-32@60"

or:
append="video=radeonfb:1024x768-32@100"


Looking at the driver source code, I'd guess that the middle
one above is closest to correct.

ISTM that Documentation/kernel-parameters.txt needs a paragraph
about syntax for specifying parameters for built-in modules vs.
loadable modules....


| Moving off topic, my cries for help from the XFree86 people also seem to 
| have gotten lost in the flow of hundreds of usenet messages.  Try as I 
| might, I cannot seem to get XFree86 to talk to my monitor at anything 
| other than 60Hz.  Even though LCD monitors have a persistent image, 
| increasing the frame rate CAN reduce motion blur slightly.


--
~Randy
"We have met the enemy and he is us."  -- Pogo (by Walt Kelly)
(Again.  Sometimes I think ln -s /usr/src/linux/.config .signature)

Re: radeonfb broken

[Timothy Miller]

Andrew Morton asked me to repost the patch for the Radeon FB off-by-one 
bug.  I'll see about making a proper patch when I get home, but if you 
want to fix it quicker, I'll just tell you what to change.


In the druvers/video/radeonfb.c, there is a function called 
fbcon_radeon_bmove.  In there, you'll see this code:

	if (srcy < dsty) {
                 srcy += height;
                 dsty += height;
         } else
                 dp_cntl |= DST_Y_TOP_TO_BOTTOM;

         if (srcx < dstx) {
                 srcx += width;
                 dstx += width;
         } else
                 dp_cntl |= DST_X_LEFT_TO_RIGHT;


Those adds need to be reduced by one.  The code should look like this:


	if (srcy < dsty) {
                 srcy += height - 1;
                 dsty += height - 1;
         } else
                 dp_cntl |= DST_Y_TOP_TO_BOTTOM;

         if (srcx < dstx) {
                 srcx += width - 1;
                 dstx += width - 1;
         } else
                 dp_cntl |= DST_X_LEFT_TO_RIGHT;



This bug is in the mainline kernel, and since I have direct experience 
programming for the Radeon, I knew how to fix it, but I also noticed 
that the Red Hat kernel "2.4.18-27.7.x" has the proper fix in it.


Whenever I download a new 2.4 kernel for gentoo, I have to manually make 
that fix.  I'm also disappointed that acceleration for Radeon has 
disappeared completely from 2.6.

Re: radeonfb broken

[James Simmons]

Thanks for the fix.


On Fri, 16 Apr 2004, Timothy Miller wrote:

> 
> Andrew Morton asked me to repost the patch for the Radeon FB off-by-one 
> bug.  I'll see about making a proper patch when I get home, but if you 
> want to fix it quicker, I'll just tell you what to change.
> 
> 
> In the druvers/video/radeonfb.c, there is a function called 
> fbcon_radeon_bmove.  In there, you'll see this code:
> 
> 	if (srcy < dsty) {
>                  srcy += height;
>                  dsty += height;
>          } else
>                  dp_cntl |= DST_Y_TOP_TO_BOTTOM;
> 
>          if (srcx < dstx) {
>                  srcx += width;
>                  dstx += width;
>          } else
>                  dp_cntl |= DST_X_LEFT_TO_RIGHT;
> 
> 
> Those adds need to be reduced by one.  The code should look like this:
> 
> 
> 	if (srcy < dsty) {
>                  srcy += height - 1;
>                  dsty += height - 1;
>          } else
>                  dp_cntl |= DST_Y_TOP_TO_BOTTOM;
> 
>          if (srcx < dstx) {
>                  srcx += width - 1;
>                  dstx += width - 1;
>          } else
>                  dp_cntl |= DST_X_LEFT_TO_RIGHT;
> 
> 
> 
> This bug is in the mainline kernel, and since I have direct experience 
> programming for the Radeon, I knew how to fix it, but I also noticed 
> that the Red Hat kernel "2.4.18-27.7.x" has the proper fix in it.
> 
> 
> Whenever I download a new 2.4 kernel for gentoo, I have to manually make 
> that fix.  I'm also disappointed that acceleration for Radeon has 
> disappeared completely from 2.6.
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: radeonfb broken

[Timothy Miller]

James Simmons wrote:
> Thanks for the fix.
> 

My pleasure!

Re: radeonfb broken

[David Eger]

My apologies; the bug is mine.  It's a simple issue of taking care
of overlapping regions in calls to copyarea().  I sent the fix to the
linux-fbdev mailing list earlier today.   The patch is reprinted below;
Hopefully Linus will take it for 2.6.6.

-dte

--- drivers/video/aty/radeon_accel.c.orig	2004-04-19 01:26:52.000000000 +0200
+++ drivers/video/aty/radeon_accel.c	2004-04-19 01:49:14.000000000 +0200
@@ -53,6 +53,18 @@
 static void radeonfb_prim_copyarea(struct radeonfb_info *rinfo, 
 				   const struct fb_copyarea *area)
 {
+	int xdir, ydir;
+	u32 sx, sy, dx, dy, w, h;
+
+	w = area->width; h = area->height;
+	dx = area->dx; dy = area->dy;
+	sx = area->sx; sy = area->sy;
+	xdir = sx - dx;
+	ydir = sy - dy;
+
+	if ( xdir < 0 ) { sx += w-1; dx += w-1; }
+	if ( ydir < 0 ) { sy += h-1; dy += h-1; }
+
 	radeon_fifo_wait(3);
 	OUTREG(DP_GUI_MASTER_CNTL,
 		rinfo->dp_gui_master_cntl /* i.e. GMC_DST_32BPP */
@@ -60,12 +72,13 @@
 		| ROP3_S 
 		| DP_SRC_RECT );
 	OUTREG(DP_WRITE_MSK, 0xffffffff);
-	OUTREG(DP_CNTL, (DST_X_LEFT_TO_RIGHT | DST_Y_TOP_TO_BOTTOM));
+	OUTREG(DP_CNTL, (xdir>=0 ? DST_X_LEFT_TO_RIGHT : 0)
+			| (ydir>=0 ? DST_Y_TOP_TO_BOTTOM : 0));
 
 	radeon_fifo_wait(3);
-	OUTREG(SRC_Y_X, (area->sy << 16) | area->sx);
-	OUTREG(DST_Y_X, (area->dy << 16) | area->dx);
-	OUTREG(DST_HEIGHT_WIDTH, (area->height << 16) | area->width);
+	OUTREG(SRC_Y_X, (sy << 16) | sx);
+	OUTREG(DST_Y_X, (dy << 16) | dx);
+	OUTREG(DST_HEIGHT_WIDTH, (h << 16) | w);
 }
 
 

Re: radeonfb broken

[Benjamin Herrenschmidt]

> However, running mplayer does not work at all on radeonfb.  mplayer
> inquires about the color depth, I am using 32 bit color depth for this,
> but radeonfb says it's DirectColor instead of TrueColor, so mplayer
> tries to initialize the palette and fails.

It is DirectColor and setting the palette should work. If it doesn't
then we indeed have a bug, but so far, I didn't have any problems
with MacOnLinux which does fbdev and sets the palette properly not
with XFree which does also set the palette. In DirectColor, the
palette is actually a gamma table.

> Also, using fbset to set the mode to 1600x1200 fails.  The mode is
> changed, but the text console resolution stays the same.  Worse, a
> "setfont" changes back to 1024x768.

Yes, that's not a radeonfb problem. The current 2.6 fbcon driver is
completely broken in this regard imho. It tries to adapt the display
mode on stty and setfont, which usually doesn't work properly since it
can't calculate supported modes properly (well... radeonfb sort of can
but it's the only one who can and there are still issues). On the
other hand, it doesn't adapt properly to changes done via fbset.

I have done some workarounds for this but they triggered other bugs
in the fbcon driver at that time, some of them causing memory
corruption, so I didn't commit my changes upstream.

A bunch of this should have been fixed by now, I need to re-sync with
James Simmons and see if I can make that to work now.

> Also, I cannot view images on console with fbi or fbv.
> 
> Felix
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>

Re: radeonfb broken

[David Eger]

On Mon, Apr 19, 2004 at 10:31:27AM -0400, Timothy Miller wrote:
>Ah, so you are the one who wrote the Radeon driver?  Thanks for the fix.

Originally it was Ani Joshi's driver.  Now BenH maintains it.
I just wrote the acceleration code for radeon for 2.6 ;-)

>One question I have is how to turn on acceleration in the 2.6 kernel. 

Funny you should ask. We're discussing over on the fbdev list
( http://linux-fbdev.sourceforge.net/ ) why it's so slow and how to fix it.
It turns out that the fbcon layer has some heuristics to decide how
best to accelerate text rendering (i.e., redraw, call driver accel routines,
or do a screen-to-screen bitblt).  These heuristics are just plain *bad*.
At the moment, I just comment out the logic and hard-wire the choice
to "use the driver accel routines, dumbass" as in the excerpt below.

We ought to fix this logic so the video cards indicate which method
is best.  The current fields in fb_fix_screeninfo don't really
convey the message well, as you can see in the logic below..


Excerpt from linux/drivers/video/console/fbcon.c:

#define divides(a, b)   ((!(a) || (b)%(a)) ? 0 : 1)

/* ... */

static __inline__ void updatescrollmode(struct display *p, struct 
vc_data *vc)
{
        struct fb_info *info = registered_fb[(int) con2fb_map[vc->vc_num]];

        int m;
        /*if (p->scrollmode & __SCROLL_YFIXED)
                return;
        if (divides(info->fix.ywrapstep, vc->vc_font.height) &&
            divides(vc->vc_font.height, info->var.yres_virtual))
                m = __SCROLL_YWRAP;
        else if (divides(info->fix.ypanstep, vc->vc_font.height) &&
                 info->var.yres_virtual >= info->var.yres + 
vc->vc_font.height)
                m = __SCROLL_YPAN;
        else if (p->scrollmode & __SCROLL_YNOMOVE)
                m = __SCROLL_YREDRAW;
        else*/
                m = __SCROLL_YMOVE;
        p->scrollmode = (p->scrollmode & ~__SCROLL_YMASK) | m;
}

- end Excerpt

So the new driver will work once we patch it to tell fbcon not to be
stupid ;-)

sorry, i'm not sure about setting initial resolution with kernel arguments..

-dte



----- End forwarded message -----