[PATCH] sisfb accel capabilities

[PATCH] sisfb accel capabilities

[David Eger]

Dear Thomas,

The following patch updates sisfb for the new fb accel capabilities
patch I'm sending upstream.  By default, this patch will rely on your
copyarea() and fillrect() accel functions instead of the panning. 
Please test the core patch + this patch, and let me know your console
works.  I'm interested to hear what speeds you see :-)

Also, using fbset to turn acceleration on and off ought to work, though
i still think this is better done via sysfs... just don't have time to
learn that ATM...

-dte

sisfb: update to new fbinfo.flags accel capabilities

Signed-off-by: David Eger <eger@havoc.gtf.org>
 
# drivers/video/sis/sis_main.h
#   2004/06/02 22:49:50+02:00 eger@rosencrantz.theboonies.us +1 -1
#   get rid of referece to setup variable
# 
# drivers/video/sis/sis_main.c
#   2004/06/02 22:49:49+02:00 eger@rosencrantz.theboonies.us +33 -35
#   migrate to new fbinfo->flags hwaccel semantics;
#   
#   get rid of attempt to do "wrap" in pan_display, 
#   as the code doesn't seem to indicate it actually ever worked.
#   wrap is just a weird amiga thing anyways ;-)
# 
# drivers/video/sis/sis_accel.h
#   2004/06/02 22:49:49+02:00 eger@rosencrantz.theboonies.us +0 -1
#   get rid of referece to setup variable
# 
# drivers/video/sis/sis_accel.c
#   2004/06/02 22:49:49+02:00 eger@rosencrantz.theboonies.us +0 -1
#   get rid of referece to setup variable
# 
diff -Nru a/drivers/video/sis/sis_accel.c b/drivers/video/sis/sis_accel.c
--- a/drivers/video/sis/sis_accel.c	Wed Jun  2 22:51:34 2004
+++ b/drivers/video/sis/sis_accel.c	Wed Jun  2 22:51:34 2004
@@ -79,7 +79,6 @@
 
 extern struct     video_info ivideo;
 extern VGA_ENGINE sisvga_engine;
-extern int sisfb_accel;
 
 static const int sisALUConv[] =
 {
diff -Nru a/drivers/video/sis/sis_accel.h b/drivers/video/sis/sis_accel.h
--- a/drivers/video/sis/sis_accel.h	Wed Jun  2 22:51:34 2004
+++ b/drivers/video/sis/sis_accel.h	Wed Jun  2 22:51:34 2004
@@ -525,7 +525,6 @@
                        int srcx, int height, int width);
 #endif
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,5,34)
-extern int sisfb_accel;
 void fbcon_sis_fillrect(struct fb_info *info, const struct fb_fillrect *rect);
 void fbcon_sis_copyarea(struct fb_info *info, const struct fb_copyarea *area);
 #endif
diff -Nru a/drivers/video/sis/sis_main.c b/drivers/video/sis/sis_main.c
--- a/drivers/video/sis/sis_main.c	Wed Jun  2 22:51:34 2004
+++ b/drivers/video/sis/sis_main.c	Wed Jun  2 22:51:34 2004
@@ -1104,10 +1104,19 @@
 		ivideo.org_x = ivideo.org_y = 0;
 		ivideo.video_linelength = ivideo.video_width * (ivideo.video_bpp >> 3);
 		ivideo.accel = 0;
+#if defined(STUPID_ACCELF_TEXT_SHIT) || LINUX_VERSION_CODE < KERNEL_VERSION(2,6,5)
+	if (var->accel_flags == FB_ACCELF_TEXT)
+		info->flags &= ~FBINFO_HWACCEL_DISABLED;
+	else
+		info->flags |= FBINFO_HWACCEL_DISABLED;
+#endif
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,5)
+		ivideo.accel = !(info->flags & FBINFO_HWACCEL_DISABLED);
+#else
 		if(sisfb_accel) {
 		   ivideo.accel = (var->accel_flags & FB_ACCELF_TEXT) ? -1 : 0;
 		}
-
+#endif
 		sisfb_set_vparms();
 
 		ivideo.current_width = ivideo.video_width;
@@ -1654,14 +1663,11 @@
 {
 	int err;
 
-	if (var->vmode & FB_VMODE_YWRAP) {
-		if (var->yoffset < 0 || var->yoffset >= fb_display[con].var.yres_virtual || var->xoffset)
-			return -EINVAL;
-	} else {
-		if (var->xoffset+fb_display[con].var.xres > fb_display[con].var.xres_virtual ||
-		    var->yoffset+fb_display[con].var.yres > fb_display[con].var.yres_virtual)
-			return -EINVAL;
-	}
+	if (var->vmode & FB_VMODE_YWRAP)
+		return -EINVAL;
+	if (var->xoffset+fb_display[con].var.xres > fb_display[con].var.xres_virtual ||
+	    var->yoffset+fb_display[con].var.yres > fb_display[con].var.yres_virtual)
+		return -EINVAL;
 
         if(con == currcon) {
 	   if((err = sisfb_pan_var(var)) < 0) return err;
@@ -1669,10 +1675,6 @@
 
 	fb_display[con].var.xoffset = var->xoffset;
 	fb_display[con].var.yoffset = var->yoffset;
-	if (var->vmode & FB_VMODE_YWRAP)
-		fb_display[con].var.vmode |= FB_VMODE_YWRAP;
-	else
-		fb_display[con].var.vmode &= ~FB_VMODE_YWRAP;
 
 	return 0;
 }
@@ -2060,30 +2062,21 @@
 {
 	int err;
 
+	if (var->vmode & FB_VMODE_YWRAP) 
+	    	return -EINVAL;
 	if (var->xoffset > (var->xres_virtual - var->xres))
 		return -EINVAL;
 	if (var->yoffset > (var->yres_virtual - var->yres))
 		return -EINVAL;
 
-	if (var->vmode & FB_VMODE_YWRAP) {
-		if (var->yoffset < 0 ||
-		    var->yoffset >= info->var.yres_virtual ||
-		    var->xoffset)
-		    	return -EINVAL;
-	} else {
-		if (var->xoffset + info->var.xres > info->var.xres_virtual ||
-		    var->yoffset + info->var.yres > info->var.yres_virtual)
-			return -EINVAL;
-	}
+	if (var->xoffset + info->var.xres > info->var.xres_virtual ||
+	    var->yoffset + info->var.yres > info->var.yres_virtual)
+		return -EINVAL;
     
 	if((err = sisfb_pan_var(var)) < 0) return err;
 
 	info->var.xoffset = var->xoffset;
 	info->var.yoffset = var->yoffset;
-	if (var->vmode & FB_VMODE_YWRAP)
-		info->var.vmode |= FB_VMODE_YWRAP;
-	else
-		info->var.vmode &= ~FB_VMODE_YWRAP;
 
 	return 0;
 }
@@ -2111,7 +2104,7 @@
 	mmio_off = PAGE_ALIGN((start & ~PAGE_MASK) + ivideo.heapstart);
 	if(off >= mmio_off) {
 		off -= mmio_off;		
-		if(info->var.accel_flags) return -EINVAL;
+		if (!(info->flags & FBINFO_HWACCEL_DISABLED)) return -EINVAL;
 
 		start = (unsigned long) ivideo.mmio_base;
 		len = PAGE_ALIGN((start & ~PAGE_MASK) + sisfb_mmio_size);
@@ -4897,12 +4890,12 @@
 	   	   }
 	        }
 
-		ivideo.accel = 0;
-		if(sisfb_accel) {
-		   ivideo.accel = -1;
-		   default_var.accel_flags |= FB_ACCELF_TEXT;
-		   sisfb_initaccel();
-		}
+		ivideo.accel = sisfb_accel;
+		sisfb_initaccel();
+
+#ifdef STUPID_ACCELF_TEXT_SHIT
+		default_var.accel_flags = sisfb_accel ? FB_ACCELF_TEXT : 0;
+#endif
 
 		if(sisfb_ypan) {
 		   /* Maximize regardless of sisfb_max at startup */
@@ -4914,7 +4907,12 @@
 	    	   }
 		}
 
-		sis_fb_info->flags = FBINFO_FLAG_DEFAULT;
+		sis_fb_info->flags = FBINFO_DEFAULT
+				   | FBINFO_HWACCEL_YPAN
+				   | FBINFO_HWACCEL_XPAN
+				   | FBINFO_HWACCEL_COPYAREA
+				   | FBINFO_HWACCEL_FILLRECT
+				   | ((sisfb_accel) ? 0 : FBINFO_HWACCEL_DISABLED);
 		sis_fb_info->var = default_var;
 		sis_fb_info->fix = sisfb_fix;
 		sis_fb_info->par = &ivideo;
diff -Nru a/drivers/video/sis/sis_main.h b/drivers/video/sis/sis_main.h
--- a/drivers/video/sis/sis_main.h	Wed Jun  2 22:51:34 2004
+++ b/drivers/video/sis/sis_main.h	Wed Jun  2 22:51:34 2004
@@ -402,7 +402,7 @@
 static int sisfb_fstn = 0;
 
 VGA_ENGINE sisvga_engine = UNKNOWN_VGA;
-int 	   sisfb_accel = -1;
+static int sisfb_accel = -1; /* synced to !(info.flags & FBINFO_HWACCEL_DISABLED */
 
 /* These are to adapted according to VGA_ENGINE type */
 static int sisfb_hwcursor_size = 0;


-------------------------------------------------------
This SF.Net email is sponsored by the new InstallShield X.
From Windows to Linux, servers to mobile, InstallShield X is the one
installation-authoring solution that does it all. Learn more and
evaluate today! http://www.installshield.com/Dev2Dev/0504

Re: [PATCH] sisfb accel capabilities

[Thomas Winischhofer]

David,

David Eger wrote:
> Dear Thomas,
> 
> The following patch updates sisfb for the new fb accel capabilities
> patch I'm sending upstream.  By default, this patch will rely on your
> copyarea() and fillrect() accel functions instead of the panning.

Could you give me any hint why this should be faster than panning?

Frankly, I don't think that's a good idea... panning is by far the 
fastest of all scrollmodes, especially when the virtual screen is much 
larger than the visible area. sisfb (unless told not to do so) maximizes 
the virtual y resolution to the available video memory, so by default 
that's eg. about 1024x16000 with 32MB of video ram (which is usual these 
days). The console is fast as hell here...

Perhaps you should make the decision whether to use either 
fillrect/copyarea or panning by the size of the virtual framebuffer? I 
understand panning is not as fast if the virtual screen is only little 
bigger than the visible area, but as soon as it has a certain size that 
makes copying the entire screen to the start of the framebuffer a seldom 
event, it's much faster.

Hm. On second thought, even if the virtual screen is only little larger 
than the visible screen, using panning will reduce the times where a 
copy of the entire screen is required. With SCROLL_ACCEL, you need a 
copy/move over the entire screen on every scrolling event; each 
additional line of text that fits into the virtual screen means calling 
copy/move one time less.

What am I missing here?

BTW:

1) fillrect and copyarea WERE (resp. ARE) being called here despite your 
previous statement that they were not. Fillrect is used to clear every 
line after pressing enter, and copyrect was used to copy the buffer to 
the start of the virtual screen when the end was reached.)

2) I still don't understand the meaning of FBINFO_HWACCEL_XPAN 
(especially as your sisfb patch sets it although sisfb did not support 
x-panning until yesterday evening...)


> Please test the core patch + this patch, and let me know your console
> works.  I'm interested to hear what speeds you see :-)

I will, but it will take some while. I need to merge your changes into 
my development version (which has been modified quite a bit). 
Furthermore I need to make sure it still compiles and works under 2.4. 
(You may have missed this fact but the driver is unified for both 2.4 
and 2.6. No time to maintain two separate versions.)

(BTW: The "get rid of reference to setup variable" and "YWARP" stuff was 
already removed there... So your patch boils down to a very few lines in 
sis_main.c only.)


> Also, using fbset to turn acceleration on and off ought to work, though
> i still think this is better done via sysfs... just don't have time to
> learn that ATM...

This depends on that STUPID_ACCELF_TEXT_SHIT, right? Nice name... ;) 
Note that not only fbset needs this, but DirectFB as well.

Thomas

-- 
Thomas Winischhofer
Vienna/Austria
thomas AT winischhofer DOT net	       *** http://www.winischhofer.net
twini AT xfree86 DOT org


-------------------------------------------------------
This SF.Net email is sponsored by the new InstallShield X.
From Windows to Linux, servers to mobile, InstallShield X is the one
installation-authoring solution that does it all. Learn more and
evaluate today! http://www.installshield.com/Dev2Dev/0504

Re: Re: [PATCH] sisfb accel capabilities

[David Eger]

Quoting Thomas Winischhofer <thomas@winischhofer.net>:

> David Eger wrote:
> > The following patch updates sisfb for the new fb accel capabilities
> > patch I'm sending upstream.  By default, this patch will rely on your
> > copyarea() and fillrect() accel functions instead of the panning.
>
> Could you give me any hint why this should be faster than panning?

The logic to prefer copyarea()/fillrect() to panning is in the main patch.  If
panning is faster than using these for most cards, then the default when both
are available should be panning as you say.  But I'll need numbers to determine
that ;-)  I think many drivers just map a chunk of video ram just big enough
for the requested resolution, so panning does zero for them :-/

> 1) fillrect and copyarea WERE (resp. ARE) being called here despite your
> previous statement that they were not. Fillrect is used to clear every
> line after pressing enter, and copyrect was used to copy the buffer to
> the start of the virtual screen when the end was reached.)

bizarre.  For radeonfb, fillrect() was called to clear the last line as you say,
but copyarea() never was.  I'll have to dig back through the old code to really
 grok your statement.

> 2) I still don't understand the meaning of FBINFO_HWACCEL_XPAN
> (especially as your sisfb patch sets it although sisfb did not support
> x-panning until yesterday evening...)

Good catch.  My patch should not set that flag, as it's inaccurate for your
driver.  My bad.  Good thing these are going through the individual driver
maintainers ;-)

> > Please test the core patch + this patch, and let me know your console
> > works.  I'm interested to hear what speeds you see :-)
>
> I will, but it will take some while. I need to merge your changes into
> my development version (which has been modified quite a bit).

Thanks.

-dte



-------------------------------------------------------
This SF.Net email is sponsored by the new InstallShield X.
From Windows to Linux, servers to mobile, InstallShield X is the one
installation-authoring solution that does it all. Learn more and
evaluate today! http://www.installshield.com/Dev2Dev/0504

Re: Re: [PATCH] sisfb accel capabilities

[Antonino A. Daplas]

On Thursday 03 June 2004 21:26, David Eger wrote:
> Quoting Thomas Winischhofer <thomas@winischhofer.net>:
> > David Eger wrote:
> > > The following patch updates sisfb for the new fb accel capabilities
> > > patch I'm sending upstream.  By default, this patch will rely on your
> > > copyarea() and fillrect() accel functions instead of the panning.
> >
> > Could you give me any hint why this should be faster than panning?
>
> The logic to prefer copyarea()/fillrect() to panning is in the main patch. 
> If panning is faster than using these for most cards, then the default when
> both are available should be panning as you say.  But I'll need numbers to
> determine that ;-)  I think many drivers just map a chunk of video ram just
> big enough for the requested resolution, so panning does zero for them :-/

I have benchamarked them before (sometime last year).  Panning (with vyres at 
least 2x yres) is significantly faster than without panning.  If I remember 
correctly, I tested at 1024x768, 8bpp, vyres = 4*yres, all drawing functions 
are accelerated.  Without panning, 'time cat /usr/src/linux/Documentation' 
finished in around 2 seconds.  With panning, it was done in 0.5 sec.

And if vyres = yres, then the driver should disable the panning/wrapping flag.  
Or fbcon should at least recognize that panning may be impossible or not very 
efficient if vyres < yres*2.

>
> > 1) fillrect and copyarea WERE (resp. ARE) being called here despite your
> > previous statement that they were not. Fillrect is used to clear every
> > line after pressing enter, and copyrect was used to copy the buffer to
> > the start of the virtual screen when the end was reached.)
>
> bizarre.  For radeonfb, fillrect() was called to clear the last line as you
> say, but copyarea() never was.  I'll have to dig back through the old code
> to really grok your statement.

Yes, this is true.  Once you've scrolled down to the end of virtual memory, 
the last screenful of data will be copied to the beginning of the 
framebuffer.  Then the whole scrolling process starts all over again. 

(This is what I was trying to point out before, panning will always involve a 
copyarea.  Also, instead of copying, maybe  the last screenful of data can be 
redrawn.  Advantage to framebuffers which has panning but with slow fb reads.  
Currently, the latter is not available).
  
Tony




-------------------------------------------------------
This SF.Net email is sponsored by the new InstallShield X.
From Windows to Linux, servers to mobile, InstallShield X is the one
installation-authoring solution that does it all. Learn more and
evaluate today! http://www.installshield.com/Dev2Dev/0504

Re: Re: [PATCH] sisfb accel capabilities

[Thomas Winischhofer]

David,

while modifying sisfb for your new flags, I went through the base patch 
as posted to Andrew (and surprisingly fast merged into 2.6.7-rc2-mm2) 
but failed to find any possibility for userland to disable acceleration 
(considering that ACCELF_TEXT is now "stupid shit" aka deprecated).

You mention that this should be done through sysfs. Where is the 
corresponding code?

Thomas

-- 
Thomas Winischhofer
Vienna/Austria
thomas AT winischhofer DOT net	       *** http://www.winischhofer.net
twini AT xfree86 DOT org


-------------------------------------------------------
This SF.Net email is sponsored by the new InstallShield X.
From Windows to Linux, servers to mobile, InstallShield X is the one
installation-authoring solution that does it all. Learn more and
evaluate today! http://www.installshield.com/Dev2Dev/0504

Re: Re: [PATCH] sisfb accel capabilities

[David Eger]

I guess I shouldn't mark it "stupid shit" until I actually have the replacement
code ready ;-)  You want me to revise the name of the #define?

Basically, I think the name FB_ACCELF_TEXT is silly and misleading:
+ it means the kernel is enabling the acceleration engine; not really to do with
_TEXT
+ it's not really a flag, since it and zero are the only valid settings
+ it doesn't really belong with the mode settings (i.e. 1024x768, 60 Hz 32bpp) 
which is what var is, mainly

But it is how things are done now.  So yes, fbset -accel 0 => var.accel_flags =
FB_ACCELF_TEXT => set_par() is still the only path that the drivers have to
enable/disable acceleration.  You'll note that the main patch does #define this
to be true for now.

I'm generating an alternate kernel at the moment with Antonino's logic to see
how panning and accel compares on my chip.  If it's faster, I should have a
patch with revised logic (favoring panning if there's sufficient yres_virtual)
by tomorrow..

-David


-------------------------------------------------------
This SF.Net email is sponsored by the new InstallShield X.
From Windows to Linux, servers to mobile, InstallShield X is the one
installation-authoring solution that does it all. Learn more and
evaluate today! http://www.installshield.com/Dev2Dev/0504

Re: Re: [PATCH] sisfb accel capabilities

[Geert Uytterhoeven]

On Thu, 3 Jun 2004, David Eger wrote:
> I guess I shouldn't mark it "stupid shit" until I actually have the replacement
> code ready ;-)  You want me to revise the name of the #define?

Yes, please do so. We need it for backwards compatibility.

> Basically, I think the name FB_ACCELF_TEXT is silly and misleading:
> + it means the kernel is enabling the acceleration engine; not really to do with
> _TEXT

Well, inside the kernel, the acceleration engine is used for acceleration the
text console emulation only.

> + it's not really a flag, since it and zero are the only valid settings

What other things do you do with a flag? Waiving? ;-)

> + it doesn't really belong with the mode settings (i.e. 1024x768, 60 Hz 32bpp)
> which is what var is, mainly

A long time ago, we also intended to use it to switch to real text mode on
cards that supported it[*]. But no one ever implemented that.

[*] Remember, fbdev originated on platforms without VGA graphics. Although some
graphics cards were based on VGA-compatible chips, they all booted in graphics
mode.

> But it is how things are done now.  So yes, fbset -accel 0 => var.accel_flags =

Indeed.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
This SF.Net email is sponsored by the new InstallShield X.
From Windows to Linux, servers to mobile, InstallShield X is the one
installation-authoring solution that does it all. Learn more and
evaluate today! http://www.installshield.com/Dev2Dev/0504

Re: Re: [PATCH] sisfb accel capabilities

[Thomas Winischhofer]

Geert Uytterhoeven wrote:
> On Thu, 3 Jun 2004, David Eger wrote:
> 
>>I guess I shouldn't mark it "stupid shit" until I actually have the replacement
>>code ready ;-)  You want me to revise the name of the #define?
> 
> 
> Yes, please do so. We need it for backwards compatibility.

ARGH.... well then, but please keep the #define STUPID... now that I 
committed an update of sisfb to Andrew.

Thomas


-- 
Thomas Winischhofer
Vienna/Austria
thomas AT winischhofer DOT net          http://www.winischhofer.net/
twini AT xfree86 DOT org


-------------------------------------------------------
This SF.Net email is sponsored by the new InstallShield X.
From Windows to Linux, servers to mobile, InstallShield X is the one
installation-authoring solution that does it all. Learn more and
evaluate today! http://www.installshield.com/Dev2Dev/0504

Re: Re: [PATCH] sisfb accel capabilities

[Geert Uytterhoeven]

On Thu, 3 Jun 2004, Thomas Winischhofer wrote:
> > Also, using fbset to turn acceleration on and off ought to work, though
> > i still think this is better done via sysfs... just don't have time to
> > learn that ATM...
>
> This depends on that STUPID_ACCELF_TEXT_SHIT, right? Nice name... ;)
> Note that not only fbset needs this, but DirectFB as well.

And XFree86, in fbdev-friendly mode.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
This SF.Net email is sponsored by the new InstallShield X.
From Windows to Linux, servers to mobile, InstallShield X is the one
installation-authoring solution that does it all. Learn more and
evaluate today! http://www.installshield.com/Dev2Dev/0504