Re: radeonfb lockup in .28-rc (bisected)

Re: radeonfb lockup in .28-rc (bisected)

[David Miller]

From: James Cloos <cloos@jhcloos.com>
Date: Mon, 27 Oct 2008 19:45:58 -0400

> Commit b1ee26bab1 breaks radeonfb on my inspiron 8100 (P3-M with a
> Mobility M7 LW [7500] (1002:4c57 1028:00e6)).

Please quote at least the headerline of the commit so that it can
come into our memory quickly when reading your report.

Here it is for the others:

commit b1ee26bab14886350ba12a5c10cbc0696ac679bf
Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date:   Wed Oct 15 22:03:46 2008 -0700

    radeonfb: accelerate imageblit and other improvements
    
    Implement support for HW color expansion of 1bpp images, along with some
    improvements to the FIFO handling and other accel operations.
    
    The offset fixup code is now unnecessary as the fbcon core will call our
    set_par upon switch back from KD_GRAPHICS before anything else happens.  I
    removed it as it would slow down accel operations.
    
    The fifo wait has been improved to avoid hitting the HW register as often,
    and the various accel ops are now performing better caching of register
    values.
    
    Overall, this improve accel performances.  The imageblit acceleration does
    result in a small overall regression in performances on some machines (on
    the order of 5% on some x86), probably becaus the SW path provides a
    better bus utilisation, but I decided to ingnore that as the performances
    is still very good, and on the other hand, some machines such as some
    sparc64 get a 3 fold performance improvement.
    
    Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
    Acked-by: David S. Miller <davem@davemloft.net>
    Cc: Krzysztof Halasa <khc@pm.waw.pl>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

> 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.

The actual key here is that when setfont runs, the framebuffer layer sets
the acceleration options for the framebuffer for the first time to their
final settings.

> The differences between rc2 and rc2+revert are limited
> to some changes in the size of the kernel:
> 
> -Memory: 512232k/524200k available (4376k kernel code, 11428k reserved, 1707k data, 320k init, 0k highmem)
> +Memory: 512272k/524200k available (4354k kernel code, 11388k reserved, 1693k data, 316k init, 0k highmem)

That's just all due to the text size change because of the different
acceleration code.

Re: radeonfb lockup in .28-rc (bisected)

[Benjamin Herrenschmidt]

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.

Re: radeonfb lockup in .28-rc (bisected)

[James Cloos]

>>>>> "David" == David Miller <davem@davemloft.net> writes:

David> Please quote at least the headerline of the commit so that it can
David> come into our memory quickly when reading your report.

Good point.

David> The actual key here is that when setfont runs, the framebuffer
David> layer sets the acceleration options for the framebuffer for the
David> first time to their final settings.

Explains why it breaks.  Thanks!

>> -Memory: 512232k/524200k available (4376k kernel code, 11428k reserved, 1707k data, 320k init, 0k highmem)
>> +Memory: 512272k/524200k available (4354k kernel code, 11388k reserved, 1693k data, 316k init, 0k highmem)

David> That's just all due to the text size change because of the
David> different acceleration code.

I left a bit too much unsaid there, but that was the point I was
making.  Ie, nothing of issue.

Incidently, it only took 14 hours of compiling to accomplish the
bisect.  And that was after a s/=m$/=n/. on my .27 .config.

Anyone bisecting should know that the new HID code is not bisect-safe.
The dell, lg, bright and sony hid drivers all are auto Y if hid is Y and
embedded is N and fail to link (when bisecting) due to missing symbols.

-JimC (still waking up)
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

Re: radeonfb lockup in .28-rc (bisected)

[James Cloos]

>>>>> "Benjamin" == Benjamin Herrenschmidt <benh@kernel.crashing.org> 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 <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

Re: radeonfb lockup in .28-rc (bisected)

[James Cloos]

>>>>> "Benjamin" == Benjamin Herrenschmidt <benh@kernel.crashing.org> 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 <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Benjamin Herrenschmidt]

>         /* 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.

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Paul Collins]

Benjamin Herrenschmidt <benh@kernel.crashing.org> 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

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Benjamin Herrenschmidt]

On Mon, 2008-11-03 at 20:01 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> 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.

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[James Cloos]

>>>>> "Benjamin" == Benjamin Herrenschmidt <benh@kernel.crashing.org> 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 <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Benjamin Herrenschmidt]

On Mon, 2008-11-03 at 10:33 -0500, James Cloos wrote:
> >>>>> "Benjamin" == Benjamin Herrenschmidt <benh@kernel.crashing.org> 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.

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Paul Collins]

Benjamin Herrenschmidt <benh@kernel.crashing.org> 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

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Benjamin Herrenschmidt]

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.

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Benjamin Herrenschmidt]

On Tue, 2008-11-04 at 19:49 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> 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.

> 

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Benjamin Herrenschmidt]

On Mon, 2008-11-03 at 20:01 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> 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.

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Paul Collins]

Benjamin Herrenschmidt <benh@kernel.crashing.org> 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

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Benjamin Herrenschmidt]

On Wed, 2008-11-05 at 23:28 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> 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.

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Benjamin Herrenschmidt]

On Wed, 2008-11-05 at 23:28 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> 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.

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Paul Collins]

Benjamin Herrenschmidt <benh@kernel.crashing.org> 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

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Paul Collins]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> Heh, it would have been easier to do
>
> 	width = (image->width | 0x1f) + 1; 
Benjamin Herrenschmidt <benh@kernel.crashing.org> 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

Re: [Linux-fbdev-devel] radeonfb lockup in .28-rc (bisected)

[Benjamin Herrenschmidt]

On Thu, 2008-11-06 at 19:00 +1300, Paul Collins wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> > Heh, it would have been easier to do
> >
> > 	width = (image->width | 0x1f) + 1; 
> Benjamin Herrenschmidt <benh@kernel.crashing.org> 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);
>