accelerated radeonfb produces artifacts on scrolling in 2.6.7

accelerated radeonfb produces artifacts on scrolling in 2.6.7

[Jurriaan]

The radeonfb driver in 2.6.7 produces some interesting artifacts on
scrolling, both scrolling horizontally and vertically.

When scrolling vertically (in mutt, in slrn, in less) some lines
move horizontally, and corruption occurs. Not all scrolling produces
artifacts, but fairly often.

When scrolling horizontally (most obvious in angband -mcu with the
option 'keep the screen centered' on) corruption appears at once.

Booting with 'noaccel' fixes the problems, but is slow, of course.

0000:01:00.0 VGA compatible controller: ATI Technologies Inc RV350 AQ [Radeon 9600]

Kernel command line: root=/dev/md3 video=radeonfb:1600x1200-16@85
radeonfb: Found Intel x86 BIOS ROM Image
radeonfb: Retreived PLL infos from BIOS
radeonfb: Reference=27.00 MHz (RefDiv=12) Memory=324.00 Mhz, System=182.00 MHz
radeonfb: Monitor 1 type CRT found
radeonfb: EDID probed
radeonfb: Monitor 2 type no found
radeonfb: ATI Radeon AQ  SDR SGRAM 128 MB
Console: switching to colour frame buffer device 133x54

Any hints would be appreciated.

Jurriaan
-- 
All lies all lies all schemes all schemes
Every winner means a loser in the western dream.
	News Model Army - Western Dream
Debian (Unstable) GNU/Linux 2.6.7-rc3-mm2 2x6078 bogomips load load 1.74


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: accelerated radeonfb produces artifacts on scrolling in 2.6.7

[David Eger]

On Wed, Jun 16, 2004 at 08:24:15PM +0200, Jurriaan wrote:
> The radeonfb driver in 2.6.7 produces some interesting artifacts on
> scrolling, both scrolling horizontally and vertically.

The corruption you are talking about is, I believe, caused by a couple of things:

(1) we're not issuing enough fifo_wait()'s around our accel engine
    and pan register writes.
(2) there's some disconnect between writing to fb memory, panning, and
    copyarea()/fillrect() calls

I sent a hack of a fix for this to Ben a week ago, adding a call to radeonfb_sync()
at the end of radeonfb_copyarea() and radeonfb_fillrect().  This seems to fix the
problem for me, but you *shouldn't* have to do this.  

I haven't tracked it any further than this.  My next guess would be auditing register 
writes and making sure there are enough fifo_wait()'s...

-dte


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: accelerated radeonfb produces artifacts on scrolling in 2.6.7

[Timothy Miller]

David Eger wrote:
> On Wed, Jun 16, 2004 at 08:24:15PM +0200, Jurriaan wrote:
> 
>>The radeonfb driver in 2.6.7 produces some interesting artifacts on
>>scrolling, both scrolling horizontally and vertically.
> 
> 
> The corruption you are talking about is, I believe, caused by a couple of things:
> 
> (1) we're not issuing enough fifo_wait()'s around our accel engine
>     and pan register writes.
> (2) there's some disconnect between writing to fb memory, panning, and
>     copyarea()/fillrect() calls
> 
> I sent a hack of a fix for this to Ben a week ago, adding a call to radeonfb_sync()
> at the end of radeonfb_copyarea() and radeonfb_fillrect().  This seems to fix the
> problem for me, but you *shouldn't* have to do this.  
> 
> I haven't tracked it any further than this.  My next guess would be auditing register 
> writes and making sure there are enough fifo_wait()'s...


Is this the case even with the off-by-one error in the bitblt code 
fixed?  In the 2.4 kernel, I got rid of all artifacts by fixing the 
off-by-one error.

In case, you don't know what I'm talking about, when you bitblt up or to 
the left on Radeon, x and y need to be adjusted by (w-1) and/or (h-1), 
respectively.  The code there, however, adjusted by w and/or h, which is 
off-by-one.



-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: accelerated radeonfb produces artifacts on scrolling in 2.6.7

[Jurriaan]

From: Timothy Miller <miller@techsource.com>
Date: Wed, Jun 16, 2004 at 03:55:32PM -0400
> 
> 
> David Eger wrote:
> >On Wed, Jun 16, 2004 at 08:24:15PM +0200, Jurriaan wrote:
> >
> >>The radeonfb driver in 2.6.7 produces some interesting artifacts on
> >>scrolling, both scrolling horizontally and vertically.
> >
> >
> >The corruption you are talking about is, I believe, caused by a couple of 
> >things:
> >
> >(1) we're not issuing enough fifo_wait()'s around our accel engine
> >    and pan register writes.
> >(2) there's some disconnect between writing to fb memory, panning, and
> >    copyarea()/fillrect() calls
> >
> >I sent a hack of a fix for this to Ben a week ago, adding a call to 
> >radeonfb_sync()
> >at the end of radeonfb_copyarea() and radeonfb_fillrect().  This seems to 
> >fix the
> >problem for me, but you *shouldn't* have to do this.  
> >
> >I haven't tracked it any further than this.  My next guess would be 
> >auditing register writes and making sure there are enough fifo_wait()'s...
> 
> 
> Is this the case even with the off-by-one error in the bitblt code 
> fixed?  In the 2.4 kernel, I got rid of all artifacts by fixing the 
> off-by-one error.
> 
> In case, you don't know what I'm talking about, when you bitblt up or to 
> the left on Radeon, x and y need to be adjusted by (w-1) and/or (h-1), 
> respectively.  The code there, however, adjusted by w and/or h, which is 
> off-by-one.
> 

You mean this code? I see (w-1) and (h-1) in there.

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 */
		| GMC_SRC_DSTCOLOR
		| ROP3_S 
		| DP_SRC_RECT );
	OUTREG(DP_WRITE_MSK, 0xffffffff);
	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, (sy << 16) | sx);
	OUTREG(DST_Y_X, (dy << 16) | dx);
	OUTREG(DST_HEIGHT_WIDTH, (h << 16) | w);
}


void radeonfb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
{
	struct radeonfb_info *rinfo = info->par;
	struct fb_copyarea modded;
	u32 vxres, vyres;
	modded.sx = area->sx;
	modded.sy = area->sy;
	modded.dx = area->dx;
	modded.dy = area->dy;
	modded.width  = area->width;
	modded.height = area->height;
  
	if (info->state != FBINFO_STATE_RUNNING)
		return;
	if (radeon_accel_disabled()) {
		cfb_copyarea(info, area);
		return;
	}

	vxres = info->var.xres;
	vyres = info->var.yres;

	if(!modded.width || !modded.height ||
	   modded.sx >= vxres || modded.sy >= vyres ||
	   modded.dx >= vxres || modded.dy >= vyres)
		return;
  
	if(modded.sx + modded.width > vxres)  modded.width = vxres - modded.sx;
	if(modded.dx + modded.width > vxres)  modded.width = vxres - modded.dx;
	if(modded.sy + modded.height > vyres) modded.height = vyres - modded.sy;
	if(modded.dy + modded.height > vyres) modded.height = vyres - modded.dy;
  
	radeonfb_prim_copyarea(rinfo, &modded);
}

HTH,
Jurriaan
-- 
hundred-and-one symptoms of being an internet addict:
17. You turn on your intercom when leaving the room so you can hear if new
e-mail arrives.
Debian (Unstable) GNU/Linux 2.6.7 2x6078 bogomips load 1.90


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: accelerated radeonfb produces artifacts on scrolling in 2.6.7

[Timothy Miller]

Jurriaan wrote:

>>Is this the case even with the off-by-one error in the bitblt code 
>>fixed?  In the 2.4 kernel, I got rid of all artifacts by fixing the 
>>off-by-one error.

> You mean this code? I see (w-1) and (h-1) in there.


> 
> 	if ( xdir < 0 ) { sx += w-1; dx += w-1; }
> 	if ( ydir < 0 ) { sy += h-1; dy += h-1; }


Yes, that's the code, and it's correct.



-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

[PATCH] fix radeonfb panning and make it play nice with copyarea()

[David Eger]

/me looks at vanilla 2.6.7

I believe the following patch will fix the bug you describe.
(part of this patch I sent to benh before, but it never made it to 2.6.7)

Please try the following bugfix.  It works for me.
If it works for you, I'll ask Andrew/James to merge.

-dte


radeonfb: fix panning corruption on a large virtual screen,
  Make panning and copyarea() play nicely with each other.

Signed-off-by: David Eger <eger@havoc.gtf.org>

# drivers/video/aty/radeon_base.c
#   2004/06/17 03:47:12+02:00 eger@rosencrantz.theboonies.us +13 -0
#   add some fifo_wait()s to lick this corruption problem
# 
# drivers/video/aty/radeon_accel.c
#   2004/06/17 03:47:12+02:00 eger@rosencrantz.theboonies.us +2 -2
#   make radeon accel play nice when the user wants to have a large virtual
#   screen to pan on.  fix previous drain bramage.
# 
diff -Nru a/drivers/video/aty/radeon_accel.c b/drivers/video/aty/radeon_accel.c
--- a/drivers/video/aty/radeon_accel.c	2004-06-17 03:52:48 +02:00
+++ b/drivers/video/aty/radeon_accel.c	2004-06-17 03:52:48 +02:00
@@ -35,8 +35,8 @@
 		return;
 	}
 
-	vxres = info->var.xres;
-	vyres = info->var.yres;
+	vxres = info->var.xres_virtual;
+	vyres = info->var.yres_virtual;
 
 	memcpy(&modded, region, sizeof(struct fb_fillrect));
 
@@ -101,8 +102,8 @@
 		return;
 	}
 
-	vxres = info->var.xres;
-	vyres = info->var.yres;
+	vxres = info->var.xres_virtual;
+	vyres = info->var.yres_virtual;
 
 	if(!modded.width || !modded.height ||
 	   modded.sx >= vxres || modded.sy >= vyres ||
diff -Nru a/drivers/video/aty/radeon_base.c b/drivers/video/aty/radeon_base.c
--- a/drivers/video/aty/radeon_base.c	2004-06-17 03:52:48 +02:00
+++ b/drivers/video/aty/radeon_base.c	2004-06-17 03:52:48 +02:00
@@ -855,6 +855,7 @@
         if (rinfo->asleep)
         	return 0;
 
+	radeon_fifo_wait(2);
         OUTREG(CRTC_OFFSET, ((var->yoffset * var->xres_virtual + var->xoffset)
 			     * var->bits_per_pixel / 8) & ~7);
         return 0;
@@ -884,6 +885,7 @@
 			if (rc)
 				return rc;
 
+			radeon_fifo_wait(2);
 			if (value & 0x01) {
 				tmp = INREG(LVDS_GEN_CNTL);
 
@@ -960,6 +962,7 @@
                         break;
         }
 
+	radeon_fifo_wait(1);
 	switch (rinfo->mon1_type) {
 		case MT_LCD:
 			OUTREG(LVDS_GEN_CNTL, val2);
@@ -1018,6 +1021,7 @@
         if (!rinfo->asleep) {
         	u32 dac_cntl2, vclk_cntl = 0;
         	
+		radeon_fifo_wait(9);
 		if (rinfo->is_mobility) {
 			vclk_cntl = INPLL(VCLK_ECP_CNTL);
 			OUTPLL(VCLK_ECP_CNTL, vclk_cntl & ~PIXCLK_DAC_ALWAYS_ONb);
@@ -1109,6 +1113,8 @@
 {
 	int i;
 
+	radeon_fifo_wait(20);
+	
 	/* Workaround from XFree */
 	if (rinfo->is_mobility) {
 	        /* A temporal workaround for the occational blanking on certain laptop panels. 
@@ -1195,6 +1201,8 @@
 {
 	struct radeonfb_info *rinfo = (struct radeonfb_info *)data;
 
+	radeon_fifo_wait(3);
+
 	OUTREG(LVDS_GEN_CNTL, rinfo->pending_lvds_gen_cntl);
 	if (rinfo->pending_pixclks_cntl) {
 		OUTPLL(PIXCLKS_CNTL, rinfo->pending_pixclks_cntl);
@@ -1219,6 +1227,7 @@
 
 	radeon_screen_blank(rinfo, VESA_POWERDOWN);
 
+	radeon_fifo_wait(31);
 	for (i=0; i<10; i++)
 		OUTREG(common_regs[i].reg, common_regs[i].val);
 
@@ -1246,6 +1255,7 @@
 	radeon_write_pll_regs(rinfo, mode);
 
 	if ((primary_mon == MT_DFP) || (primary_mon == MT_LCD)) {
+		radeon_fifo_wait(10);
 		OUTREG(FP_CRTC_H_TOTAL_DISP, mode->fp_crtc_h_total_disp);
 		OUTREG(FP_CRTC_V_TOTAL_DISP, mode->fp_crtc_v_total_disp);
 		OUTREG(FP_H_SYNC_STRT_WID, mode->fp_h_sync_strt_wid);
@@ -1285,6 +1295,7 @@
 
 	radeon_screen_blank(rinfo, VESA_NO_BLANKING);
 
+	radeon_fifo_wait(2);
 	OUTPLL(VCLK_ECP_CNTL, mode->vclk_ecp_cntl);
 	
 	return;
@@ -1858,6 +1870,7 @@
 	del_timer_sync(&rinfo->lvds_timer);
 
 	lvds_gen_cntl |= (LVDS_BL_MOD_EN | LVDS_BLON);
+	radeon_fifo_wait(3);
 	if (on && (level > BACKLIGHT_OFF)) {
 		lvds_gen_cntl |= LVDS_DIGON;
 		if (!(lvds_gen_cntl & LVDS_ON)) {
@@ -2130,6 +2143,7 @@
           u32 tom = INREG(NB_TOM);
           tmp = ((((tom >> 16) - (tom & 0xffff) + 1) << 6) * 1024);
  
+ 		radeon_fifo_wait(6);
           OUTREG(MC_FB_LOCATION, tom);
           OUTREG(DISPLAY_BASE_ADDR, (tom & 0xffff) << 16);
           OUTREG(CRTC2_DISPLAY_BASE_ADDR, (tom & 0xffff) << 16);


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: [PATCH] fix radeonfb panning and make it play nice with copyarea()

[Jurriaan]

From: David Eger <eger@havoc.gtf.org>
Date: Wed, Jun 16, 2004 at 10:21:00PM -0400
> 
> /me looks at vanilla 2.6.7
> 
> I believe the following patch will fix the bug you describe.
> (part of this patch I sent to benh before, but it never made it to 2.6.7)
> 
> Please try the following bugfix.  It works for me.
> If it works for you, I'll ask Andrew/James to merge.
> 
> -dte
> 
> 
> radeonfb: fix panning corruption on a large virtual screen,
>   Make panning and copyarea() play nicely with each other.
> 
> Signed-off-by: David Eger <eger@havoc.gtf.org>
> 
> # drivers/video/aty/radeon_base.c
> #   2004/06/17 03:47:12+02:00 eger@rosencrantz.theboonies.us +13 -0
> #   add some fifo_wait()s to lick this corruption problem
> # 
> # drivers/video/aty/radeon_accel.c
> #   2004/06/17 03:47:12+02:00 eger@rosencrantz.theboonies.us +2 -2
> #   make radeon accel play nice when the user wants to have a large virtual
> #   screen to pan on.  fix previous drain bramage.
> # 

Unfortunately, this doesn't fix it for me.

horizontal scrolling in angband is still very broken, and vertical
scrolling (even as simple as recalling previous command-lines in bash)
is also still broken.

For example, recalling the previous commandline goes OK the first time,
and the second time, part of the line is shifted some 120 characters to
the right.

Sorry,
Jurriaan
-- 
Cole's Law:
  Thinly sliced cabbage.
Debian (Unstable) GNU/Linux 2.6.7 2x6078 bogomips load 0.14


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: [PATCH] fix radeonfb panning and make it play nice with copyarea()

[David Eger]

Dear Andrew,

While the patch I sent Juriaan didn't work for him, it is a needed bugfix.
Please apply to -mm.  I'm continuing to try to track down his issue...

-dte


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Re: [PATCH] fix radeonfb panning and make it play nice with copyarea()

[David Eger]

On Thu, Jun 17, 2004 at 07:19:31AM +0200, Jurriaan wrote:
> 
> Unfortunately, this doesn't fix it for me.
> 
> horizontal scrolling in angband is still very broken, and vertical
> scrolling (even as simple as recalling previous command-lines in bash)
> is also still broken.

/me tries 16 bpp for the first time.  
ah. you do have a different problem.
sorry about that, i'll take a look into it...
  ( i ported the accel fns for radeon from the XFree project,
    so i've got a special interest in getting this right... )

-dte

[PATCH] radeonfb: 16bpp accel broken. a work-around

[David Eger]

Dear Andrew, 

I've tracked Jurriaan's radeonfb problem down to copyarea() malfunctioning 
in 16bpp mode.  Here's a hack to just do a software copyarea() in 16bpp mode.
No more screen garbage, but then it's slow :-/
for instance, cat' ing the MAINTAINERS file:
   8bpp -  ~1.5 sec
  16bpp - ~19 sec  (with yucky fix)
  32bpp -  ~6 sec

-dte


radeonfb: 16bpp work-around - copyarea() doesn't work

Signed-off-by: David Eger <eger@havoc.gtf.org>

diff -Nru a/drivers/video/aty/radeon_accel.c b/drivers/video/aty/radeon_accel.c
--- a/drivers/video/aty/radeon_accel.c	2004-06-25 09:15:24 +02:00
+++ b/drivers/video/aty/radeon_accel.c	2004-06-25 09:15:24 +02:00
@@ -99,7 +99,9 @@
   
 	if (info->state != FBINFO_STATE_RUNNING)
 		return;
-	if (radeon_accel_disabled()) {
+	if (radeon_accel_disabled() 
+		|| info->var.bits_per_pixel == 16) { 
+		/* hack alert: why is 16bpp broken? dunno */
 		cfb_copyarea(info, area);
 		return;
 	}
diff -Nru a/drivers/video/aty/radeon_base.c b/drivers/video/aty/radeon_base.c
--- a/drivers/video/aty/radeon_base.c	2004-06-25 09:15:24 +02:00
+++ b/drivers/video/aty/radeon_base.c	2004-06-25 09:15:24 +02:00
@@ -1573,6 +1573,12 @@
 	}
 #endif
 
+	/* for some reason, copyarea is broken at 16bpp... :-/ */
+	if (mode->bits_per_pixel == 16)
+		info->flags &= ~FBINFO_HWACCEL_COPYAREA;
+	else
+		info->flags |= FBINFO_HWACCEL_COPYAREA;
+	
 	/* Clear surface registers */
 	for (i=0; i<8; i++) {
 		newmode.surf_lower_bound[i] = 0;


-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com

Re: [PATCH] radeonfb: 16bpp accel broken. a work-around

[Andrew Morton]

David Eger <eger@havoc.gtf.org> wrote:
>
> I've tracked Jurriaan's radeonfb problem down to copyarea() malfunctioning 
>  in 16bpp mode.  Here's a hack to just do a software copyarea() in 16bpp mode.
>  No more screen garbage, but then it's slow :-/
>  for instance, cat' ing the MAINTAINERS file:
>     8bpp -  ~1.5 sec
>    16bpp - ~19 sec  (with yucky fix)
>    32bpp -  ~6 sec
> 
>  -dte
> 
> 
>  radeonfb: 16bpp work-around - copyarea() doesn't work

err, why not fix copyarea()?


-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com