On Mon, 2008-10-27 at 19:45 -0400, James Cloos wrote:
> Commit b1ee26bab1 breaks radeonfb on my inspiron 8100 (P3-M with a
> Mobility M7 LW [7500] (1002:4c57 1028:00e6)).
>
> The boot is OK until init(8) starts; after init outputs its version info
> it calls rc(8), which starts by setting the fb font. At that point any
> kernel with b1ee26bab1 locks hard. The cursor stops flashing, magic
> sysrq stops working and the fan starts up after a few seconds. (I can't
> tell whether it is the CPU or the GPU that heats up.)
>
> If it is relevant, I use a 10x20 font, so the font change means the
> console converts from 200x75, 8x16 to 160x60, 10x20.
Annoying... Either I'm doing something wrong (which is always possible)
or we're hitting yet another fancy ATI bug (the M7 generation is known
to be pretty bad in that area).
First, let's see if it's related to the imageblit. Can you re-apply the
reverted patch and apply this little hack on top :
Index: linux-work/drivers/video/aty/radeon_accel.c
===================================================================
--- linux-work.orig/drivers/video/aty/radeon_accel.c 2008-10-28 11:01:49.000000000 +1100
+++ linux-work/drivers/video/aty/radeon_accel.c 2008-10-28 11:01:57.000000000 +1100
@@ -252,6 +252,7 @@ void radeonfb_imageblit(struct fb_info *
if (!image->width || !image->height)
return;
+#if 0
/* We only do 1 bpp color expansion for now */
if (info->flags & FBINFO_HWACCEL_DISABLED || image->depth != 1)
goto fallback;
@@ -275,6 +276,7 @@ void radeonfb_imageblit(struct fb_info *
return;
fallback:
+#endif
radeon_engine_idle(rinfo);
cfb_imageblit(info, image);
And let me know if that makes it not lockup.
Cheers,
Ben.
>>>>> "Benjamin" == Benjamin Herrenschmidt <[email protected]> writes:
Benjamin> Annoying... Either I'm doing something wrong (which is always
Benjamin> possible) or we're hitting yet another fancy ATI bug (the M7
Benjamin> generation is known to be pretty bad in that area).
Yes, M7 does have its idiosyncrasies. :)
Benjamin> First, let's see if it's related to the imageblit. Can you
Benjamin> re-apply the reverted patch and apply this little hack on top
Benjamin> :
I'm starting a compile now. It'll be a couple of hours until I can
reboot.
-JimC
--
James Cloos <[email protected]> OpenPGP: 1024D/ED7DAEA6
>>>>> "Benjamin" == Benjamin Herrenschmidt <[email protected]> writes:
Benjamin> First, let's see if it's related to the imageblit. Can you
Benjamin> re-apply the reverted patch and apply this little hack on top:
Benjamin> +#if 0
Benjamin> /* We only do 1 bpp color expansion for now */
Benjamin> if (info->flags & FBINFO_HWACCEL_DISABLED || image->depth != 1)
Benjamin> goto fallback;
Benjamin> @@ -275,6 +276,7 @@ void radeonfb_imageblit(struct fb_info *
Benjamin> return;
Benjamin> fallback:
Benjamin> +#endif
Benjamin> And let me know if that makes it not lockup.
Indeed, if-zero-ing out that block does work.
Since that if essentially removes to call to radeonfb_prim_imageblit(),
I wonder whether:
/* X here pads width to a multiple of 32 and uses the clipper to
* adjust the result. Is that really necessary ? Things seem to
* work ok for me without that and the doco doesn't seem to imply
* there is such a restriction.
*/
is relevant?
Or whether the card is unhappy with the creg settings. It does seem,
now that I read the code, that:
void radeon_fifo_update_and_wait(struct radeonfb_info *rinfo, int entries)
{
int i;
for (i=0; i<2000000; i++) {
rinfo->fifo_free = INREG(RBBM_STATUS) & 0x7f;
if (rinfo->fifo_free >= entries)
return;
udelay(10);
}
printk(KERN_ERR "radeonfb: FIFO Timeout !\n");
/* XXX Todo: attempt to reset the engine */
}
is in play. It is probably spinning through all of the 2000000 possible
udelay(10) calls. I don't think I ever gave it twenty seconds before
giving up. And certainly not forty seconds, if the freeze happens after
setting the DST_Y_X register.
-JimC
--
James Cloos <[email protected]> OpenPGP: 1024D/ED7DAEA6
> /* X here pads width to a multiple of 32 and uses the clipper to
> * adjust the result. Is that really necessary ? Things seem to
> * work ok for me without that and the doco doesn't seem to imply
> * there is such a restriction.
> */
>
> is relevant?
Possibly.
> Or whether the card is unhappy with the creg settings. It does seem,
> now that I read the code, that:
>
> void radeon_fifo_update_and_wait(struct radeonfb_info *rinfo, int entries)
> {
> int i;
>
> for (i=0; i<2000000; i++) {
> rinfo->fifo_free = INREG(RBBM_STATUS) & 0x7f;
> if (rinfo->fifo_free >= entries)
> return;
> udelay(10);
> }
> printk(KERN_ERR "radeonfb: FIFO Timeout !\n");
> /* XXX Todo: attempt to reset the engine */
> }
>
> is in play. It is probably spinning through all of the 2000000 possible
> udelay(10) calls. I don't think I ever gave it twenty seconds before
> giving up. And certainly not forty seconds, if the freeze happens after
> setting the DST_Y_X register.
Well, setting DST_Y_X is what triggers the transfer. The above means
that the FIFO isn't emptying (ie, the engine is locked up).
There's a few things we can do from here:
- We can try indeed to do the alignment things though that involves
using the clipper etc... and thus complicates things significantly
- We can also try putting a few more bubbles in the FIFO in case that's
where the problem is
- We can blacklist that chip for imageblit (it's not a huge improvement
on x86 anyway).
- We can be smart, reduce the timeout above, and "detect" the lockup,
when it happens, reset the engine and disable the acceleration that
locked up.
Now, the problem is ... My second son was just born last wed. so I'm
pretty unavailable right now. Thus, for .29, I'm tempted to go for the
simpler approach which is to blacklist M7's from imageblit acceleration.
Cheers,
Ben.
Benjamin Herrenschmidt <[email protected]> writes:
> Now, the problem is ... My second son was just born last wed. so I'm
> pretty unavailable right now. Thus, for .29, I'm tempted to go for the
> simpler approach which is to blacklist M7's from imageblit acceleration.
I am also experiencing this problem on a PowerBook G4 with an M10.
0000:00:10.0 VGA compatible controller [0300]: ATI Technologies Inc RV350 [Mobility Radeon 9600 M10] [1002:4e50]
Here are the fonts I'm using:
http://ondioline.org/~paul/consolefonts/
--
Paul Collins
Wellington, New Zealand
Dag vijandelijk luchtschip de huismeester is dood
On Mon, 2008-11-03 at 20:01 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <[email protected]> writes:
> > Now, the problem is ... My second son was just born last wed. so I'm
> > pretty unavailable right now. Thus, for .29, I'm tempted to go for the
> > simpler approach which is to blacklist M7's from imageblit acceleration.
>
> I am also experiencing this problem on a PowerBook G4 with an M10.
Interesting... I actually developed it on a PowerBook G4 with an M10 !
Maybe not the same revision though or maybe not the same fonts.
I'll have to dig a bit, I'm not sure I'll be able to do a proper
fix in time for the current -rc's so we might have to back the
patch off. We'll see what I can come up with this week, I want to
try reproducing here and experimenting if doing that alignment
& clipping helps.
> 0000:00:10.0 VGA compatible controller [0300]: ATI Technologies Inc RV350 [Mobility Radeon 9600 M10] [1002:4e50]
>
> Here are the fonts I'm using:
>
> http://ondioline.org/~paul/consolefonts/
>
Thanks.
One thing you may want to try .. it will result in crap results on
screen but would help telling us if that's the cause, is to hack
radeonfb to round the image size to a multiple of 32 and see if that
stops the lockup.
Cheers,
Ben.
>>>>> "Benjamin" == Benjamin Herrenschmidt <[email protected]> writes:
>> is in play. It is probably spinning through all of the 2000000 possible
>> udelay(10) calls. I don't think I ever gave it twenty seconds before
>> giving up. And certainly not forty seconds, if the freeze happens after
>> setting the DST_Y_X register.
Benjamin> Well, setting DST_Y_X is what triggers the transfer. The above
Benjamin> means that the FIFO isn't emptying (ie, the engine is locked up).
I gave it another try over the weekend and let it sit for five minutes.
The reset message never appeared.
Benjamin> - We can blacklist that chip for imageblit (it's not a huge
Benjamin> improvement on x86 anyway).
No objections here.
Benjamin> - We can be smart, reduce the timeout above, and "detect" the
Benjamin> lockup, when it happens, reset the engine and disable the
Benjamin> acceleration that locked up.
Given Paul's report, that seems like the long term solution.
Benjamin> Now, the problem is ... My second son was just born last
Benjamin> wed. so I'm pretty unavailable right now.
Congrats!
Benjamin> Thus, for .29, I'm tempted to go for the simpler approach
Benjamin> which is to blacklist M7's from imageblit acceleration.
Again, that is fine by me. Otherwise I'll just leave the #if0 commit
in my compile clone.
-JimC
--
James Cloos <[email protected]> OpenPGP: 1024D/ED7DAEA6
On Mon, 2008-11-03 at 10:33 -0500, James Cloos wrote:
> >>>>> "Benjamin" == Benjamin Herrenschmidt <[email protected]> writes:
>
> >> is in play. It is probably spinning through all of the 2000000 possible
> >> udelay(10) calls. I don't think I ever gave it twenty seconds before
> >> giving up. And certainly not forty seconds, if the freeze happens after
> >> setting the DST_Y_X register.
>
> Benjamin> Well, setting DST_Y_X is what triggers the transfer. The above
> Benjamin> means that the FIFO isn't emptying (ie, the engine is locked up).
>
> I gave it another try over the weekend and let it sit for five minutes.
> The reset message never appeared.
Because it's probably going to lockup trying to display it :-) I need to
make that stuff a bit more robust, by disabling acceleration if the
engine locks up for example.
> Again, that is fine by me. Otherwise I'll just leave the #if0 commit
> in my compile clone.
Allright, we'll see what I come up with, and at worst, I'll just revert
the whole thing.
Ben.
Benjamin Herrenschmidt <[email protected]> writes:
> One thing you may want to try .. it will result in crap results on
> screen but would help telling us if that's the cause, is to hack
> radeonfb to round the image size to a multiple of 32 and see if that
> stops the lockup.
I took a guess at how to do this and ended up with the patch below.
With the patch applied, the screen turns almost completely to gibberish
at console handover and the machine hangs. At the top I get two lines
of old output from the previous boot. (I'm pretty sure it's hung
because the optical drive init happens after console handover, and I
don't get the usual chunka-chunka noise.)
The corruption I get is very similar to what I got when I originally
reported the problem when I was using my patched-in 12x24 font. (I
created the .psf version later and switched back to default 8x16 to
verify my problem was the same as James's.)
diff --git a/drivers/video/aty/radeon_accel.c b/drivers/video/aty/radeon_accel.c
index 8718f73..848e9bc 100644
--- a/drivers/video/aty/radeon_accel.c
+++ b/drivers/video/aty/radeon_accel.c
@@ -208,7 +208,11 @@ static void radeonfb_prim_imageblit(struct radeonfb_info *rinfo,
* work ok for me without that and the doco doesn't seem to imply
* there is such a restriction.
*/
- OUTREG(DST_WIDTH_HEIGHT, (image->width << 16) | image->height);
+ {
+ /* Hack attack. */
+ int width = ((image->width - 1) / 32 + 1) * 32;
+ OUTREG(DST_WIDTH_HEIGHT, (width << 16) | image->height);
+ }
src_bytes = (((image->width * image->depth) + 7) / 8) * image->height;
dwords = (src_bytes + 3) / 4;
--
Paul Collins
Wellington, New Zealand
Dag vijandelijk luchtschip de huismeester is dood
On Tue, 2008-11-04 at 19:49 +1300, Paul Collins wrote:
> + int width = ((image->width - 1) / 32 + 1) * 32;
Heh, it would have been easier to do
width = (image->width | 0x1f) + 1;
BTW. Does it always lockup or only when using this special font (without
the hack that is ?)
Cheers,
Ben.
On Tue, 2008-11-04 at 19:49 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <[email protected]> writes:
>
> > One thing you may want to try .. it will result in crap results on
> > screen but would help telling us if that's the cause, is to hack
> > radeonfb to round the image size to a multiple of 32 and see if that
> > stops the lockup.
>
> I took a guess at how to do this and ended up with the patch below.
>
> With the patch applied, the screen turns almost completely to gibberish
> at console handover and the machine hangs. At the top I get two lines
> of old output from the previous boot. (I'm pretty sure it's hung
> because the optical drive init happens after console handover, and I
> don't get the usual chunka-chunka noise.)
>
> The corruption I get is very similar to what I got when I originally
> reported the problem when I was using my patched-in 12x24 font. (I
> created the .psf version later and switched back to default 8x16 to
> verify my problem was the same as James's.)
>
>
> diff --git a/drivers/video/aty/radeon_accel.c b/drivers/video/aty/radeon_accel.c
> index 8718f73..848e9bc 100644
> --- a/drivers/video/aty/radeon_accel.c
> +++ b/drivers/video/aty/radeon_accel.c
> @@ -208,7 +208,11 @@ static void radeonfb_prim_imageblit(struct radeonfb_info *rinfo,
> * work ok for me without that and the doco doesn't seem to imply
> * there is such a restriction.
> */
> - OUTREG(DST_WIDTH_HEIGHT, (image->width << 16) | image->height);
> + {
> + /* Hack attack. */
> + int width = ((image->width - 1) / 32 + 1) * 32;
> + OUTREG(DST_WIDTH_HEIGHT, (width << 16) | image->height);
> + }
>
> src_bytes = (((image->width * image->depth) + 7) / 8) * image->height;
> dwords = (src_bytes + 3) / 4;
Oh and you also need to change the src_bytes calculation
Ben.
>
On Mon, 2008-11-03 at 20:01 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <[email protected]> writes:
> > Now, the problem is ... My second son was just born last wed. so I'm
> > pretty unavailable right now. Thus, for .29, I'm tempted to go for the
> > simpler approach which is to blacklist M7's from imageblit acceleration.
>
> I am also experiencing this problem on a PowerBook G4 with an M10.
>
> 0000:00:10.0 VGA compatible controller [0300]: ATI Technologies Inc RV350 [Mobility Radeon 9600 M10] [1002:4e50]
>
> Here are the fonts I'm using:
>
> http://ondioline.org/~paul/consolefonts/
>
For some reason, that font gives me "Bad font file format".
Cheers,
Ben.
Benjamin Herrenschmidt <[email protected]> writes:
> On Mon, 2008-11-03 at 20:01 +1300, Paul Collins wrote:
>> I am also experiencing this problem on a PowerBook G4 with an M10.
>>
>> 0000:00:10.0 VGA compatible controller [0300]: ATI Technologies Inc RV350 [Mobility Radeon 9600 M10] [1002:4e50]
>>
>> Here are the fonts I'm using:
>>
>> http://ondioline.org/~paul/consolefonts/
>
> For some reason, that font gives me "Bad font file format".
Odd. I'm using setfont from Debian's kbd 1.14.1-4 to load it.
P.S. I haven't had time to play with the 32-roundup thing any further yet.
--
Paul Collins
Wellington, New Zealand
Dag vijandelijk luchtschip de huismeester is dood
On Wed, 2008-11-05 at 23:28 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <[email protected]> writes:
>
> > On Mon, 2008-11-03 at 20:01 +1300, Paul Collins wrote:
> >> I am also experiencing this problem on a PowerBook G4 with an M10.
> >>
> >> 0000:00:10.0 VGA compatible controller [0300]: ATI Technologies Inc RV350 [Mobility Radeon 9600 M10] [1002:4e50]
> >>
> >> Here are the fonts I'm using:
> >>
> >> http://ondioline.org/~paul/consolefonts/
> >
> > For some reason, that font gives me "Bad font file format".
>
> Odd. I'm using setfont from Debian's kbd 1.14.1-4 to load it.
>
> P.S. I haven't had time to play with the 32-roundup thing any further yet.
Hopefully I'll have some time today. I've been using consolechars -f,
I'll try with setfont.
Cheers,
Ben.
On Wed, 2008-11-05 at 23:28 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <[email protected]> writes:
>
> > On Mon, 2008-11-03 at 20:01 +1300, Paul Collins wrote:
> >> I am also experiencing this problem on a PowerBook G4 with an M10.
> >>
> >> 0000:00:10.0 VGA compatible controller [0300]: ATI Technologies Inc RV350 [Mobility Radeon 9600 M10] [1002:4e50]
> >>
> >> Here are the fonts I'm using:
> >>
> >> http://ondioline.org/~paul/consolefonts/
> >
> > For some reason, that font gives me "Bad font file format".
>
> Odd. I'm using setfont from Debian's kbd 1.14.1-4 to load it.
>
> P.S. I haven't had time to play with the 32-roundup thing any further yet.
Ok so I can't get it to die on my M9 (rv250) tipb here, I can try next
week on a albook with an M10 (rv350). Just to make sure I'm doing the
same as you, does it boot fine if you don't change the font and does it
lockup if, after it's fully booted, you do a setfont neepalt10x20.psf ?
Cheers,
Ben.
Benjamin Herrenschmidt <[email protected]> writes:
> Ok so I can't get it to die on my M9 (rv250) tipb here, I can try next
> week on a albook with an M10 (rv350). Just to make sure I'm doing the
> same as you, does it boot fine if you don't change the font and does it
> lockup if, after it's fully booted, you do a setfont neepalt10x20.psf ?
Yes. It boots fine with the regular 8x16 font and locks up when I do
"setfont neepalt10x20.psf".
--
Paul Collins
Wellington, New Zealand
Dag vijandelijk luchtschip de huismeester is dood
Benjamin Herrenschmidt <[email protected]> writes:
> Heh, it would have been easier to do
>
> width = (image->width | 0x1f) + 1;
Benjamin Herrenschmidt <[email protected]> writes:
> Oh and you also need to change the src_bytes calculation
With the slightly less embarrassing patch below applied and using the
default 8x16 font, the machine boots successfully with the expected
gibberish console display. I can then setfont one of the large fonts
and it does not lock up. The gibberish changes and if I do dmesg I even
get a few chunks of legible output: http://ondioline.org/~paul/corruption.png
diff --git a/drivers/video/aty/radeon_accel.c b/drivers/video/aty/radeon_accel.c
index 8718f73..6dbd24a 100644
--- a/drivers/video/aty/radeon_accel.c
+++ b/drivers/video/aty/radeon_accel.c
@@ -176,6 +176,7 @@ static void radeonfb_prim_imageblit(struct radeonfb_info *rinfo,
{
unsigned int src_bytes, dwords;
u32 *bits;
+ int width;
radeonfb_set_creg(rinfo, DP_GUI_MASTER_CNTL, &rinfo->dp_gui_mc_cache,
rinfo->dp_gui_mc_base |
@@ -208,9 +209,11 @@ static void radeonfb_prim_imageblit(struct radeonfb_info *rinfo,
* work ok for me without that and the doco doesn't seem to imply
* there is such a restriction.
*/
- OUTREG(DST_WIDTH_HEIGHT, (image->width << 16) | image->height);
+ /* Let us pad. */
+ width = (image->width | 0x1f) + 1;
+ OUTREG(DST_WIDTH_HEIGHT, (width << 16) | image->height);
- src_bytes = (((image->width * image->depth) + 7) / 8) * image->height;
+ src_bytes = (((width * image->depth) + 7) / 8) * image->height;
dwords = (src_bytes + 3) / 4;
bits = (u32*)(image->data);
--
Paul Collins
Wellington, New Zealand
Dag vijandelijk luchtschip de huismeester is dood
On Thu, 2008-11-06 at 19:00 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <[email protected]> writes:
> > Heh, it would have been easier to do
> >
> > width = (image->width | 0x1f) + 1;
> Benjamin Herrenschmidt <[email protected]> writes:
> > Oh and you also need to change the src_bytes calculation
>
> With the slightly less embarrassing patch below applied and using the
> default 8x16 font, the machine boots successfully with the expected
> gibberish console display. I can then setfont one of the large fonts
> and it does not lock up. The gibberish changes and if I do dmesg I even
> get a few chunks of legible output: http://ondioline.org/~paul/corruption.png
Ok. Well, that's both good and bad news. Good news is, we have a handle
on what the bug can be, ie, the comment in X might refer to an actual HW
limitation/bug, though I find it strange that it works fine in some
cases and not others.
The bad news is, I yet to figure out if we have a proper way out of it
using the clipping etc... that doesn't involve making the acceleration
totally worthless in the first place since it's already not very
interesting on a few platforms.
I'll dig but I'm afraid chances are that I'll disable the acceleration
by default unless some special command line arg is passed so that David
can still get his faster console on sparc.
Maybe using a completely different approach such as caching pre-expanded
glyphs would lead to better results... though I think it's harder to
implement with fbcon.
Cheers,
Ben.
>
> diff --git a/drivers/video/aty/radeon_accel.c b/drivers/video/aty/radeon_accel.c
> index 8718f73..6dbd24a 100644
> --- a/drivers/video/aty/radeon_accel.c
> +++ b/drivers/video/aty/radeon_accel.c
> @@ -176,6 +176,7 @@ static void radeonfb_prim_imageblit(struct radeonfb_info *rinfo,
> {
> unsigned int src_bytes, dwords;
> u32 *bits;
> + int width;
>
> radeonfb_set_creg(rinfo, DP_GUI_MASTER_CNTL, &rinfo->dp_gui_mc_cache,
> rinfo->dp_gui_mc_base |
> @@ -208,9 +209,11 @@ static void radeonfb_prim_imageblit(struct radeonfb_info *rinfo,
> * work ok for me without that and the doco doesn't seem to imply
> * there is such a restriction.
> */
> - OUTREG(DST_WIDTH_HEIGHT, (image->width << 16) | image->height);
> + /* Let us pad. */
> + width = (image->width | 0x1f) + 1;
> + OUTREG(DST_WIDTH_HEIGHT, (width << 16) | image->height);
>
> - src_bytes = (((image->width * image->depth) + 7) / 8) * image->height;
> + src_bytes = (((width * image->depth) + 7) / 8) * image->height;
> dwords = (src_bytes + 3) / 4;
> bits = (u32*)(image->data);
>
