[PATCH] radeonfb(): memmove() fix -- this one works ;-)

[PATCH] radeonfb(): memmove() fix -- this one works ;-)

[David Eger]

Dear Ben,

Please apply this patch to the radeonfb driver (I think of you as the
maintainer for this driver) and then forward to James and Linus :-)

This fixes the same problem as mentioned before -- overlapping
regions sent to copyarea() get fux0r3d.  This patch corrects my earlier
braindamage of setting up w, h, dx, etc. and then forgetting to use them
later on.  Ah, having two versions of the same code (userspace and kernel
space) -- little details like that just float past the eye...  no problem
with fbcon at all, as i previously speculated.  apologies to james.

-David


--- 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);
 }
 
 



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Mon, 2004-04-19 at 11:46, David Eger wrote:
> Dear Ben,
> 
> Please apply this patch to the radeonfb driver (I think of you as the
> maintainer for this driver) and then forward to James and Linus :-)
> 
> This fixes the same problem as mentioned before -- overlapping
> regions sent to copyarea() get fux0r3d.  This patch corrects my earlier
> braindamage of setting up w, h, dx, etc. and then forgetting to use them
> later on.  Ah, having two versions of the same code (userspace and kernel
> space) -- little details like that just float past the eye...  no problem
> with fbcon at all, as i previously speculated.  apologies to james.

Excellent. Thanks.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

,On Mon, 2004-04-19 at 11:46, David Eger wrote:
> Dear Ben,
> 
> Please apply this patch to the radeonfb driver (I think of you as the
> maintainer for this driver) and then forward to James and Linus :-)
> 
> This fixes the same problem as mentioned before -- overlapping
> regions sent to copyarea() get fux0r3d.  This patch corrects my earlier
> braindamage of setting up w, h, dx, etc. and then forgetting to use them
> later on.  Ah, having two versions of the same code (userspace and kernel
> space) -- little details like that just float past the eye...  no problem
> with fbcon at all, as i previously speculated.  apologies to james.

BTW, James, did we decide what is the best way to set fbcon accel
mode ? For drivers with copyarea accelerated, we want YMOVE. I'd like
a patch to send Linus/Andrew asap along with some pending radeonfb
fixes for that.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> BTW, James, did we decide what is the best way to set fbcon accel
> mode ? For drivers with copyarea accelerated, we want YMOVE. I'd like
> a patch to send Linus/Andrew asap along with some pending radeonfb
> fixes for that.

I had to do some thinking about this. Here is explation ofthe scrolling 
code. It really needs to be cleaned up. So renamings would make sense 
here.

In the original code we had:

These we set by the driver.

SCROLL_YREDRAW		( __SCROLL_YFIXED  | __SCROLL_YREDRAW )     Lots use this
SCROLL_YNOMOVE		( __SCROLL_YNOMOVE | __SCROLL_YPANREDRAW )  Lots use this
SCROLL_YNOPARTIAL	  __SCROLL_YNOPARTIAL			    (controlfb)

fbcon.c -> updatescrollmode.

__SCROLL_YFIXED		--> Does not update. Test to see if it is set then exits
__SCROLL_YWRAP		--> sets if we have ywrapstep and conditions are right
__SCROLL_YPAN		--> sets if we have ypanstep and conditions are right
__SCROLL_YNOMOVE	--> test for this and if yes then
__SCROLL_YREDRAW	-->			set __SCROLL_YREDRAW
__SCROLL_YMOVE		-->		      else set __SCROLL_YMOVE

__SCROLL_YMASK		Mask to clear out extra bits.

Not used in updatescrollmode

__SCROLL_YPANREDRAW	--> Set by SCROLL_YNOMOVE
__SCROLL_YNOPARTIAL	--> Set by SCROLL_YNOPARTIAL. Its deals for the case 
			    when we can only only pan the whole screen, 
			    not a region of the screen.
---------------------------------------------------------------------------------
Let go over what happened in the old code.

So if the driver set SCROLL_YNOPARTIAL then __SCROLL_YNOPARTIAL was set. This was
to deal with hardware panning flickering. Only the controlfb used it. Is this 
problem still present? If not this could go away. The goal is:

	Handle screen flickering.

The second one is the driver using SCROLL_YREDRAW. This sets __SCROLL_YFIXED and
__SCROLL_YREDRAW. When going into updatescrollmode nothing happens since the first
thing we do is test for __SCROLL_YFIXED. So the goal is to override the hardware 
panning or wrap. The system is set up to only __SCROLL_YREDRAW which just redraws
all the font images. In this case only __SCROLL_YREDRAW is important. So the 
goal is:
	
	if driver set SCROLL_YREDRAW
		Use hardware image drawing	// Only use this

The 3rd and final one is where we set the scrollmode to __SCROLL_YNOMOVE and 
__SCROLL_YPANREDRAW. __SCROLL_YNOMOVE again is one of those variables used only
in updatescrollmode. First we test to see if hardware panning or hardware wrapping. 
Use that. If we don't have that ablity then we test the __SCROLL_YNOMOVE to see
if we just draw all the font images or use a copyarea operation with the flag 
is set to __SCROLL_YMOVE. So the goal is:

	if Ywrap
		Use hardware ywrap
	else if ypan 
	  	Use hardware panning
	else if driver set SCROLL_YNOMOVE
		Use hardware image drawing
	else 
		Use hardware copyarea

I like to see this cleaned up now. The question is what is the proper order of all
this stuff. For use we have the following conditions.

Hardware panning. Test for fb_pan_display and ypanstep. 
Hardware wrapping. Test for ywrapstep.
Hardware copyarea.
Hardware image drawing.

   So if the we have the case of both copyarea and imageblit hardware accelerated we
need to use copyarea for scrolling since that will be faster. Its the same case
for if we only have copyarea accelerating. 
   The question is if we have only hardware imageblit should we use that? There is
a cost to assembly a image for the hardware to draw. The question is the assembly
of the image less costly than a soft copyarea?
   As for hardware panning should we prefer it above everthing? 
   






-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Wed, 2004-04-21 at 08:52, James Simmons wrote:
> > BTW, James, did we decide what is the best way to set fbcon accel
> > mode ? For drivers with copyarea accelerated, we want YMOVE. I'd like
> > a patch to send Linus/Andrew asap along with some pending radeonfb
> > fixes for that.
> 
> I had to do some thinking about this. Here is explation ofthe scrolling 
> code. It really needs to be cleaned up. So renamings would make sense 
> here.

> .../...

Why not simply

 1) Document properly what those various options do
 2) Let the driver provide it's "preferred" scroll mode ? It's sort-of
    a stack violation since it really only affects fbcon at this point,
    but since it's just an "hint", it's perfectly acceptable.

(Note: I'm not sure about controlfb at this point, I have to dig
out some old HW and test)

Also, we sort-of lost the ability to have fbcon specific accels in the
drivers... It would have been nice to be able to store the font in
vram and use blits to composite text, but that would have meant having
fbcon-related hooks =P Or maybe we can find a clean way to express that
to fbcon without clobbering fbdev's with fbcon-related datas ? Simply by
having fbcon request a vres big enough to store the font data outside
of the visible area and using copyarea ?

Ben.

> In the original code we had:
> 
> These we set by the driver.
> 
> SCROLL_YREDRAW		( __SCROLL_YFIXED  | __SCROLL_YREDRAW )     Lots use this
> SCROLL_YNOMOVE		( __SCROLL_YNOMOVE | __SCROLL_YPANREDRAW )  Lots use this
> SCROLL_YNOPARTIAL	  __SCROLL_YNOPARTIAL			    (controlfb)
> 
> fbcon.c -> updatescrollmode.
> 
> __SCROLL_YFIXED		--> Does not update. Test to see if it is set then exits
> __SCROLL_YWRAP		--> sets if we have ywrapstep and conditions are right
> __SCROLL_YPAN		--> sets if we have ypanstep and conditions are right
> __SCROLL_YNOMOVE	--> test for this and if yes then
> __SCROLL_YREDRAW	-->			set __SCROLL_YREDRAW
> __SCROLL_YMOVE		-->		      else set __SCROLL_YMOVE
> 
> __SCROLL_YMASK		Mask to clear out extra bits.
> 
> Not used in updatescrollmode
> 
> __SCROLL_YPANREDRAW	--> Set by SCROLL_YNOMOVE
> __SCROLL_YNOPARTIAL	--> Set by SCROLL_YNOPARTIAL. Its deals for the case 
> 			    when we can only only pan the whole screen, 
> 			    not a region of the screen.
> ---------------------------------------------------------------------------------
> Let go over what happened in the old code.
> 
> So if the driver set SCROLL_YNOPARTIAL then __SCROLL_YNOPARTIAL was set. This was
> to deal with hardware panning flickering. Only the controlfb used it. Is this 
> problem still present? If not this could go away. The goal is:
> 
> 	Handle screen flickering.
> 
> The second one is the driver using SCROLL_YREDRAW. This sets __SCROLL_YFIXED and
> __SCROLL_YREDRAW. When going into updatescrollmode nothing happens since the first
> thing we do is test for __SCROLL_YFIXED. So the goal is to override the hardware 
> panning or wrap. The system is set up to only __SCROLL_YREDRAW which just redraws
> all the font images. In this case only __SCROLL_YREDRAW is important. So the 
> goal is:
> 	
> 	if driver set SCROLL_YREDRAW
> 		Use hardware image drawing	// Only use this
> 
> The 3rd and final one is where we set the scrollmode to __SCROLL_YNOMOVE and 
> __SCROLL_YPANREDRAW. __SCROLL_YNOMOVE again is one of those variables used only
> in updatescrollmode. First we test to see if hardware panning or hardware wrapping. 
> Use that. If we don't have that ablity then we test the __SCROLL_YNOMOVE to see
> if we just draw all the font images or use a copyarea operation with the flag 
> is set to __SCROLL_YMOVE. So the goal is:
> 
> 	if Ywrap
> 		Use hardware ywrap
> 	else if ypan 
> 	  	Use hardware panning
> 	else if driver set SCROLL_YNOMOVE
> 		Use hardware image drawing
> 	else 
> 		Use hardware copyarea
> 
> I like to see this cleaned up now. The question is what is the proper order of all
> this stuff. For use we have the following conditions.
> 
> Hardware panning. Test for fb_pan_display and ypanstep. 
> Hardware wrapping. Test for ywrapstep.
> Hardware copyarea.
> Hardware image drawing.
> 
>    So if the we have the case of both copyarea and imageblit hardware accelerated we
> need to use copyarea for scrolling since that will be faster. Its the same case
> for if we only have copyarea accelerating. 
>    The question is if we have only hardware imageblit should we use that? There is
> a cost to assembly a image for the hardware to draw. The question is the assembly
> of the image less costly than a soft copyarea?
>    As for hardware panning should we prefer it above everthing? 
>    



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

> I find that a soft copyarea is always expensive than a soft imageblit as a
> copyarea will involve an fb read (10-20x slower than fb write).  Tested
> this before with an old pci nvidia card. Not o sure with newer hardware.

I confirm, reading from the fb shall be avoided at all cost

> >    As for hardware panning should we prefer it above everthing?
> 
> Probably not.  Hardware panning + slow ymove will result in an unsmooth
> scrolling action.
> 
> I prefer an algo like this:
> 
> if (yres < vyres/2 || !ypanstep || soft_copyarea)
>      yredraw
> else
>      ymove
> 
> Tony
> 
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Geert Uytterhoeven]

On Wed, 21 Apr 2004, Benjamin Herrenschmidt wrote:
> > I find that a soft copyarea is always expensive than a soft imageblit as a
> > copyarea will involve an fb read (10-20x slower than fb write).  Tested
> > this before with an old pci nvidia card. Not o sure with newer hardware.
>
> I confirm, reading from the fb shall be avoided at all cost

This is true for PCI/AGP, but may not be true for unified memory systems.

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: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Wed, 2004-04-21 at 18:28, Geert Uytterhoeven wrote:
> On Wed, 21 Apr 2004, Benjamin Herrenschmidt wrote:
> > > I find that a soft copyarea is always expensive than a soft imageblit as a
> > > copyarea will involve an fb read (10-20x slower than fb write).  Tested
> > > this before with an old pci nvidia card. Not o sure with newer hardware.
> >
> > I confirm, reading from the fb shall be avoided at all cost
> 
> This is true for PCI/AGP, but may not be true for unified memory systems.

Right, but that confirms my opinion that the choice of the technique used
shall be under control of the driver

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Geert Uytterhoeven]

On Wed, 21 Apr 2004, Benjamin Herrenschmidt wrote:
> On Wed, 2004-04-21 at 18:28, Geert Uytterhoeven wrote:
> > On Wed, 21 Apr 2004, Benjamin Herrenschmidt wrote:
> > > > I find that a soft copyarea is always expensive than a soft imageblit as a
> > > > copyarea will involve an fb read (10-20x slower than fb write).  Tested
> > > > this before with an old pci nvidia card. Not o sure with newer hardware.
> > >
> > > I confirm, reading from the fb shall be avoided at all cost
> >
> > This is true for PCI/AGP, but may not be true for unified memory systems.
>
> Right, but that confirms my opinion that the choice of the technique used
> shall be under control of the driver

Agreed. So a hint would be the most appropriate.

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: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> Why not simply
> 
>  1) Document properly what those various options do
>  2) Let the driver provide it's "preferred" scroll mode ? It's sort-of
>     a stack violation since it really only affects fbcon at this point,
>     but since it's just an "hint", it's perfectly acceptable.
> 
> (Note: I'm not sure about controlfb at this point, I have to dig
> out some old HW and test)
> 
> Also, we sort-of lost the ability to have fbcon specific accels in the
> drivers... 

I agree. I don't doubt that in time we will find that the low level driver 
has to deal with this. Also it would really nice if userland new which is 
better for when it would using the accel engine.

> It would have been nice to be able to store the font in
> vram and use blits to composite text, but that would have meant having
> fbcon-related hooks =P Or maybe we can find a clean way to express that
> to fbcon without clobbering fbdev's with fbcon-related datas ? Simply by
> having fbcon request a vres big enough to store the font data outside
> of the visible area and using copyarea ?

Struct fb_pixmap. We have data pointer *addr. The only problem is using 
the memory space to the right of the visible framebuffer

------------|
|	|   |
|	|   |
--------------

At the bottom of the framebuffer it would be no problem. We could even use 
struct fb_pixmap with texture maps. Store fonts as texture maps and just 
draw them to a surface. You can do lots of fancy tricks. The only issue I 
have thought about is we might need to had a hook for fb_get_buffer_offset.
Not to big a deal.



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> > > This is true for PCI/AGP, but may not be true for unified memory systems.
> >
> > Right, but that confirms my opinion that the choice of the technique used
> > shall be under control of the driver
> 
> Agreed. So a hint would be the most appropriate.

Hi Antonino. Haven't heard from you in a while.

I agree. What if we add to vmode in struct fb_screeninfo.
We have 

FB_VMODE_YWRAP		256
FB_VMODE_SMOOTH_XPAN	512

We can add more fields. Of course we have to figure out what the common 
case would be.




-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Antonino A. Daplas]

On Thu, 2004-04-22 at 02:13, James Simmons wrote:
> 
> > > > This is true for PCI/AGP, but may not be true for unified memory systems.
> > >
> > > Right, but that confirms my opinion that the choice of the technique used
> > > shall be under control of the driver
> > 
> > Agreed. So a hint would be the most appropriate.
> 
> Hi Antonino. Haven't heard from you in a while.

Yeah, been busy.

> 
> I agree. What if we add to vmode in struct fb_screeninfo.
> We have 
> 
> FB_VMODE_YWRAP		256
> FB_VMODE_SMOOTH_XPAN	512
> 
> We can add more fields. Of course we have to figure out what the common 
> case would be.
> 

We can add:

FB_VMODE_YPAN
FB_VMODE_REDRAW /* will use imageblit */
FB_VMODE_MOVE   /* will use copyarea */

Personally, I think FB_VMODE_REDRAW should be the default as it is 
faster than FB_VMODE_MOVE for most PCI cards.

Just to be clear.

1.  If without a ypan or ywrap, fbcon can either do a redraw or a move

2.  If with ypan or ywrap, then fbcon can only do a move.

Ypan + redraw (FB_VMODE_MOVE | FB_VMODE_REDRAW) is not supported, and I
consider this combination to give the fastest and smoothest scrolling
action if driver does not have any hardware acceleration in most cards.
Perhaps this can be added?

Tony
 






-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

> Struct fb_pixmap. We have data pointer *addr. The only problem is using 
> the memory space to the right of the visible framebuffer
> 
> ------------|
> |	|   |
> |	|   |
> --------------
> 
> At the bottom of the framebuffer it would be no problem. We could even use 
> struct fb_pixmap with texture maps. Store fonts as texture maps and just 
> draw them to a surface. You can do lots of fancy tricks. The only issue I 
> have thought about is we might need to had a hook for fb_get_buffer_offset.
> Not to big a deal.

The choice of where to allocate that pixmap may not be trivial. It would
be interesting to have a call to the fbdev to deal with that. If we call
this pixmap the "work area" for example (or whatever fancy name you can
find), we could have a call "prepare_work_area" to the fbdev, giving it
the size requested by the client (based on the font size).

That should stay optional as it's not interesting for drivers without a
fast copyarea of course.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Geert Uytterhoeven]

On Wed, 22 Apr 2004, Antonino A. Daplas wrote:
> On Thu, 2004-04-22 at 02:13, James Simmons wrote:
> > > > > This is true for PCI/AGP, but may not be true for unified memory systems.
> > > >
> > > > Right, but that confirms my opinion that the choice of the technique used
> > > > shall be under control of the driver
> > >
> > > Agreed. So a hint would be the most appropriate.
> >
> > Hi Antonino. Haven't heard from you in a while.
>
> Yeah, been busy.
>
> > I agree. What if we add to vmode in struct fb_screeninfo.
> > We have
> >
> > FB_VMODE_YWRAP		256
> > FB_VMODE_SMOOTH_XPAN	512
> >
> > We can add more fields. Of course we have to figure out what the common
> > case would be.
> >
>
> We can add:
>
> FB_VMODE_YPAN
> FB_VMODE_REDRAW /* will use imageblit */
> FB_VMODE_MOVE   /* will use copyarea */
>
> Personally, I think FB_VMODE_REDRAW should be the default as it is
> faster than FB_VMODE_MOVE for most PCI cards.
>
> Just to be clear.
>
> 1.  If without a ypan or ywrap, fbcon can either do a redraw or a move
>
> 2.  If with ypan or ywrap, then fbcon can only do a move.
>
> Ypan + redraw (FB_VMODE_MOVE | FB_VMODE_REDRAW) is not supported, and I
> consider this combination to give the fastest and smoothest scrolling
> action if driver does not have any hardware acceleration in most cards.
> Perhaps this can be added?

Perhaps a different approach in announcing these capabilities is more
appropriate? The above is very console-centric. And since it's set by the
fbdev, conceptually it does not belong in fb_var_screeninfo, but in
fb_fix_screeninfo.

What we really want to know here is whether redrawing is faster than copying,
i.e. whether reading from the frame buffer is fast or slow, and whether
fb_copyarea and fb_imageblit are fast or slow.

Right now we have in fb_fix_screeninfo:
  - [xy]panstep, for hardware panning
  - ywrapstep, for hardware ywrap

We can add a capability field with some flags that indicates that
  - fb_copyarea is accelerated or not
  - fb_imageblit is accelerated or not
  - fb_write is much faster than fb_read
(I don't think it matters much to announce an accelerated fb_fillrect, although
 we may want to do that for consistency)

I chose the above so we have reasonable defaults if the capability field is 0.

All other logic can be derived by fbcon from these capabilities, right?

E.g. wrap and pan are always faster than copying, since (usually) they involve
only one register access, while copying is O(n).

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: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

Okay. This is my first attempt to address this problem. There is not test 
for accelerated copyarea or image drawing as we lack the flag. I haven't 
figured where to put it yet. We have 3 choices.


fb_var_screeninfo
fb_fix_screeninfo
fb_info


diff -urN -X /home/jsimmons/dontdiff linus-2.6/drivers/video/console/fbcon.c fbdev-2.6/drivers/video/console/fbcon.c
--- linus-2.6/drivers/video/console/fbcon.c	2004-04-26 14:49:24.000000000 -0700
+++ fbdev-2.6/drivers/video/console/fbcon.c	2004-04-25 13:44:51.000000000 -0700
@@ -605,10 +605,6 @@
 	/* on which frame buffer will we open this console? */
 	info = registered_fb[(int) con2fb_map[unit]];
 	
-	if (info->var.accel_flags)
-		fb_display[unit].scrollmode = SCROLL_YNOMOVE;
-	else
-		fb_display[unit].scrollmode = SCROLL_YREDRAW;
 	fbcon_set_display(vc, init, !init);
 }
 
@@ -622,21 +618,19 @@
 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;
+		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;
+		m = SCROLL_YPAN;
+	if (info->fix.accel == FB_ACCEL_NONE)
+		m = SCROLL_YREDRAW;
 	else
-		m = __SCROLL_YMOVE;
-	p->scrollmode = (p->scrollmode & ~__SCROLL_YMASK) | m;
+		m = SCROLL_YMOVE;
+	p->scrollmode = (p->scrollmode & ~SCROLL_YMASK) | m;
 }
 
 static void fbcon_set_display(struct vc_data *vc, int init, int logo)
@@ -1322,7 +1316,7 @@
 {
 	struct fb_info *info = registered_fb[(int) con2fb_map[vc->vc_num]];
 	struct display *p = &fb_display[vc->vc_num];
-	int scroll_partial = !(p->scrollmode & __SCROLL_YNOPARTIAL);
+	int scroll_partial = !(p->scrollmode & SCROLL_YNOPARTIAL);
 
 	if (!info->fbops->fb_blank && console_blanked)
 		return 0;
@@ -1346,15 +1340,15 @@
 			fbcon_softback_note(vc, t, count);
 		if (logo_shown >= 0)
 			goto redraw_up;
-		switch (p->scrollmode & __SCROLL_YMASK) {
-		case __SCROLL_YMOVE:
+		switch (p->scrollmode & SCROLL_YMASK) {
+		case SCROLL_YMOVE:
 			accel_bmove(vc, info, t + count, 0, t, 0,
 					 b - t - count, vc->vc_cols);
 			accel_clear(vc, info, b - count, 0, count,
 					 vc->vc_cols);
 			break;
 
-		case __SCROLL_YWRAP:
+		case SCROLL_YWRAP:
 			if (b - t - count > 3 * vc->vc_rows >> 2) {
 				if (t > 0)
 					fbcon_bmove(vc, 0, 0, count, 0, t,
@@ -1364,7 +1358,7 @@
 					fbcon_bmove(vc, b - count, 0, b, 0,
 						    vc->vc_rows - b,
 						    vc->vc_cols);
-			} else if (p->scrollmode & __SCROLL_YPANREDRAW)
+			} else if (p->scrollmode & SCROLL_YREDRAW)
 				goto redraw_up;
 			else
 				fbcon_bmove(vc, t + count, 0, t, 0,
@@ -1372,7 +1366,7 @@
 			fbcon_clear(vc, b - count, 0, count, vc->vc_cols);
 			break;
 
-		case __SCROLL_YPAN:
+		case SCROLL_YPAN:
 			if ((p->yscroll + count <=
 			     2 * (p->vrows - vc->vc_rows))
 			    && ((!scroll_partial && (b - t == vc->vc_rows))
@@ -1387,7 +1381,7 @@
 					fbcon_bmove(vc, b - count, 0, b, 0,
 						    vc->vc_rows - b,
 						    vc->vc_cols);
-			} else if (p->scrollmode & __SCROLL_YPANREDRAW)
+			} else if (p->scrollmode & SCROLL_YREDRAW)
 				goto redraw_up;
 			else
 				fbcon_bmove(vc, t + count, 0, t, 0,
@@ -1395,7 +1389,7 @@
 			fbcon_clear(vc, b - count, 0, count, vc->vc_cols);
 			break;
 
-		case __SCROLL_YREDRAW:
+		case SCROLL_YREDRAW:
 		      redraw_up:
 			fbcon_redraw(vc, p, t, b - t - count,
 				     count * vc->vc_cols);
@@ -1413,14 +1407,14 @@
 	case SM_DOWN:
 		if (count > vc->vc_rows)	/* Maximum realistic size */
 			count = vc->vc_rows;
-		switch (p->scrollmode & __SCROLL_YMASK) {
-		case __SCROLL_YMOVE:
+		switch (p->scrollmode & SCROLL_YMASK) {
+		case SCROLL_YMOVE:
 			accel_bmove(vc, info, t, 0, t + count, 0,
 					 b - t - count, vc->vc_cols);
 			accel_clear(vc, info, t, 0, count, vc->vc_cols);
 			break;
 
-		case __SCROLL_YWRAP:
+		case SCROLL_YWRAP:
 			if (b - t - count > 3 * vc->vc_rows >> 2) {
 				if (vc->vc_rows - b > 0)
 					fbcon_bmove(vc, b, 0, b - count, 0,
@@ -1430,7 +1424,7 @@
 				if (t > 0)
 					fbcon_bmove(vc, count, 0, 0, 0, t,
 						    vc->vc_cols);
-			} else if (p->scrollmode & __SCROLL_YPANREDRAW)
+			} else if (p->scrollmode & SCROLL_YREDRAW)
 				goto redraw_down;
 			else
 				fbcon_bmove(vc, t, 0, t + count, 0,
@@ -1438,7 +1432,7 @@
 			fbcon_clear(vc, t, 0, count, vc->vc_cols);
 			break;
 
-		case __SCROLL_YPAN:
+		case SCROLL_YPAN:
 			if ((count - p->yscroll <= p->vrows - vc->vc_rows)
 			    && ((!scroll_partial && (b - t == vc->vc_rows))
 				|| (scroll_partial
@@ -1452,7 +1446,7 @@
 				if (t > 0)
 					fbcon_bmove(vc, count, 0, 0, 0, t,
 						    vc->vc_cols);
-			} else if (p->scrollmode & __SCROLL_YPANREDRAW)
+			} else if (p->scrollmode & SCROLL_YREDRAW)
 				goto redraw_down;
 			else
 				fbcon_bmove(vc, t, 0, t + count, 0,
@@ -1460,7 +1454,7 @@
 			fbcon_clear(vc, t, 0, count, vc->vc_cols);
 			break;
 
-		case __SCROLL_YREDRAW:
+		case SCROLL_YREDRAW:
 		      redraw_down:
 			fbcon_redraw(vc, p, b - 1, b - t - count,
 				     -count * vc->vc_cols);
@@ -1604,11 +1598,11 @@
 	if (info)
 		info->var.yoffset = p->yscroll = 0;
         fbcon_resize(vc, vc->vc_cols, vc->vc_rows);
-	switch (p->scrollmode & __SCROLL_YMASK) {
-	case __SCROLL_YWRAP:
+	switch (p->scrollmode & SCROLL_YMASK) {
+	case SCROLL_YWRAP:
 		scrollback_phys_max = p->vrows - vc->vc_rows;
 		break;
-	case __SCROLL_YPAN:
+	case SCROLL_YPAN:
 		scrollback_phys_max = p->vrows - 2 * vc->vc_rows;
 		if (scrollback_phys_max < 0)
 			scrollback_phys_max = 0;
@@ -2191,11 +2185,11 @@
 
 	offset = p->yscroll - scrollback_current;
 	limit = p->vrows;
-	switch (p->scrollmode && __SCROLL_YMASK) {
-	case __SCROLL_YWRAP:
+	switch (p->scrollmode && SCROLL_YMASK) {
+	case SCROLL_YWRAP:
 		info->var.vmode |= FB_VMODE_YWRAP;
 		break;
-	case __SCROLL_YPAN:
+	case SCROLL_YPAN:
 		limit -= vc->vc_rows;
 		info->var.vmode &= ~FB_VMODE_YWRAP;
 		break;
diff -urN -X /home/jsimmons/dontdiff linus-2.6/drivers/video/console/fbcon.h fbdev-2.6/drivers/video/console/fbcon.h
--- linus-2.6/drivers/video/console/fbcon.h	2004-04-26 14:49:24.000000000 -0700
+++ fbdev-2.6/drivers/video/console/fbcon.h	2004-04-25 13:45:51.000000000 -0700
@@ -69,39 +69,12 @@
      */
      
 /* Internal flags */
-#define __SCROLL_YPAN		0x001
-#define __SCROLL_YWRAP		0x002
-#define __SCROLL_YMOVE		0x003
-#define __SCROLL_YREDRAW	0x004
-#define __SCROLL_YMASK		0x00f
-#define __SCROLL_YFIXED		0x010
-#define __SCROLL_YNOMOVE	0x020
-#define __SCROLL_YPANREDRAW	0x040
-#define __SCROLL_YNOPARTIAL	0x080
-
-/* Only these should be used by the drivers */
-/* Which one should you use? If you have a fast card and slow bus,
-   then probably just 0 to indicate fbcon should choose between
-   YWRAP/YPAN+MOVE/YMOVE. On the other side, if you have a fast bus
-   and even better if your card can do fonting (1->8/32bit painting),
-   you should consider either SCROLL_YREDRAW (if your card is
-   able to do neither YPAN/YWRAP), or SCROLL_YNOMOVE.
-   The best is to test it with some real life scrolling (usually, not
-   all lines on the screen are filled completely with non-space characters,
-   and REDRAW performs much better on such lines, so don't cat a file
-   with every line covering all screen columns, it would not be the right
-   benchmark).
- */
-#define SCROLL_YREDRAW		(__SCROLL_YFIXED|__SCROLL_YREDRAW)
-#define SCROLL_YNOMOVE		(__SCROLL_YNOMOVE|__SCROLL_YPANREDRAW)
-
-/* SCROLL_YNOPARTIAL, used in combination with the above, is for video
-   cards which can not handle using panning to scroll a portion of the
-   screen without excessive flicker.  Panning will only be used for
-   whole screens.
- */
-/* Namespace consistency */
-#define SCROLL_YNOPARTIAL	__SCROLL_YNOPARTIAL
+#define SCROLL_YPAN		0x001
+#define SCROLL_YWRAP		0x002
+#define SCROLL_YMOVE		0x003
+#define SCROLL_YREDRAW		0x004
+#define SCROLL_YNOPARTIAL	0x005
+#define SCROLL_YMASK		0x00f
 
 extern int fb_console_init(void);
 



-------------------------------------------------------
This SF.net email is sponsored by: The Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Tue, 2004-04-27 at 10:19, James Simmons wrote:
> Okay. This is my first attempt to address this problem. There is not test 
> for accelerated copyarea or image drawing as we lack the flag. I haven't 
> figured where to put it yet. We have 3 choices.
> 
> 
> fb_var_screeninfo
> fb_fix_screeninfo
> fb_info

Well... fix is what gets filled after a mode is set, let's put it in there
along with the linewidth etc...

Ben.




-------------------------------------------------------
This SF.net email is sponsored by: The Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Geert Uytterhoeven]

On Tue, 27 Apr 2004, Benjamin Herrenschmidt wrote:
> On Tue, 2004-04-27 at 10:19, James Simmons wrote:
> > Okay. This is my first attempt to address this problem. There is not test
> > for accelerated copyarea or image drawing as we lack the flag. I haven't
> > figured where to put it yet. We have 3 choices.
> >
> >
> > fb_var_screeninfo
> > fb_fix_screeninfo
> > fb_info
>
> Well... fix is what gets filled after a mode is set, let's put it in there
> along with the linewidth etc...

Indeed. BTW, no comments on my proposal to add fb_fix_screeninfo.capabilities?

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 Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[David Eger]

Geert wrote:
> Indeed. BTW, no comments on my proposal to add fb_fix_screeninfo.capabilities?

referring to his earlier post where he suggested:
> What we really want to know here is whether redrawing is faster than
> copying, i.e. whether reading from the frame buffer is fast or slow,
> and whether fb_copyarea and fb_imageblit are fast or slow.
> 
> Right now we have in fb_fix_screeninfo:
>  - [xy]panstep, for hardware panning
>  - ywrapstep, for hardware ywrap
> 
> We can add a capability field with some flags that indicates that
> - fb_copyarea is accelerated or not
> - fb_imageblit is accelerated or not
> - fb_write is much faster than fb_read
> (I don't think it matters much to announce an accelerated fb_fillrect, 
> although we may want to do that for consistency)
> 
> I chose the above so we have reasonable defaults if the capability field 
> is 0.
> 
> All other logic can be derived by fbcon from these capabilities, right?

I like the approach.  It sounds like this is the sensible replacement for
the vague semantics we have now in var.accel_flags / FB_ACCELF_TEXT. 

I'm currently running through the radeon code trying to figure who all 
twiddles var before it gets set to registers and how to rework fbcon to be
sensible...

-dte

---
getting bounce messages from me? let me know and you'll go on my whitelist


-------------------------------------------------------
This SF.net email is sponsored by: The Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Geert Uytterhoeven]

On Tue, 27 Apr 2004, David Eger wrote:
> Geert wrote:
> > Indeed. BTW, no comments on my proposal to add fb_fix_screeninfo.capabilities?
>
> referring to his earlier post where he suggested:
> > What we really want to know here is whether redrawing is faster than
> > copying, i.e. whether reading from the frame buffer is fast or slow,
> > and whether fb_copyarea and fb_imageblit are fast or slow.
> >
> > Right now we have in fb_fix_screeninfo:
> >  - [xy]panstep, for hardware panning
> >  - ywrapstep, for hardware ywrap
> >
> > We can add a capability field with some flags that indicates that
> > - fb_copyarea is accelerated or not
> > - fb_imageblit is accelerated or not
> > - fb_write is much faster than fb_read
> > (I don't think it matters much to announce an accelerated fb_fillrect,
> > although we may want to do that for consistency)
> >
> > I chose the above so we have reasonable defaults if the capability field
> > is 0.
> >
> > All other logic can be derived by fbcon from these capabilities, right?
>
> I like the approach.  It sounds like this is the sensible replacement for
> the vague semantics we have now in var.accel_flags / FB_ACCELF_TEXT.

var.accel_flags / FB_ACCELF_TEXT is something different: you need to be able to
disable hardware acceleration before your application touches the accel engine.

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 Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Tue, 2004-04-27 at 19:57, Geert Uytterhoeven wrote:

> var.accel_flags / FB_ACCELF_TEXT is something different: you need to be able to
> disable hardware acceleration before your application touches the accel engine.

That was true in 2.4, I don't think this can apply reasonably well in 2.6.
In fact, from experience, the var tends to be "lost" easily, I'm not sure
why exactly.

On the other hand, I tend to think that if you have a KD_TEXT console,
accel is under fbcon control, if you have a KD_GRAPHICS console, then
the accel engine is under you control. I don't think any else make any
sense in fact

We really need to simplify all of that stuff, it's way too messy at the
moment.

Ben.




-------------------------------------------------------
This SF.net email is sponsored by: The Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

> I like the approach.  It sounds like this is the sensible replacement for
> the vague semantics we have now in var.accel_flags / FB_ACCELF_TEXT. 
> 
> I'm currently running through the radeon code trying to figure who all 
> twiddles var before it gets set to registers and how to rework fbcon to be
> sensible...

The whole var thing is too messy right now. Especially, the lack of a
per-VT var right now makes things even more difficult

Ben.




-------------------------------------------------------
This SF.net email is sponsored by: The Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Geert Uytterhoeven]

On Tue, 27 Apr 2004, Benjamin Herrenschmidt wrote:
> On Tue, 2004-04-27 at 19:57, Geert Uytterhoeven wrote:
> > var.accel_flags / FB_ACCELF_TEXT is something different: you need to be able to
> > disable hardware acceleration before your application touches the accel engine.
>
> That was true in 2.4, I don't think this can apply reasonably well in 2.6.
> In fact, from experience, the var tends to be "lost" easily, I'm not sure
> why exactly.
>
> On the other hand, I tend to think that if you have a KD_TEXT console,
> accel is under fbcon control, if you have a KD_GRAPHICS console, then
> the accel engine is under you control. I don't think any else make any
> sense in fact

You're right, I forgot about that.

> We really need to simplify all of that stuff, it's way too messy at the
> moment.

Agreed.

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 Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Geert Uytterhoeven]

On Tue, 27 Apr 2004, Benjamin Herrenschmidt wrote:
> > I like the approach.  It sounds like this is the sensible replacement for
> > the vague semantics we have now in var.accel_flags / FB_ACCELF_TEXT.
> >
> > I'm currently running through the radeon code trying to figure who all
> > twiddles var before it gets set to registers and how to rework fbcon to be
> > sensible...
>
> The whole var thing is too messy right now. Especially, the lack of a
> per-VT var right now makes things even more difficult

Agreed, too. It took me much effort to make the console subsystem aware of
different-sized VTs a few years ago, so we should make use of that :-)

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 Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> > I'm currently running through the radeon code trying to figure who all 
> > twiddles var before it gets set to registers and how to rework fbcon to be
> > sensible...
> 
> The whole var thing is too messy right now. Especially, the lack of a
> per-VT var right now makes things even more difficult

the real problem is the VT system was designed around VGA text mode. For 
example alot of the scrolling code in fbcon should be in the generic VT 
layer. 






-------------------------------------------------------
This SF.net email is sponsored by: The Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> Agreed, too. It took me much effort to make the console subsystem aware of
> different-sized VTs a few years ago, so we should make use of that :-)

For 2.7.X I plan to take it all the way. You will see.





-------------------------------------------------------
This SF.net email is sponsored by: The Robotic Monkeys at ThinkGeek
For a limited time only, get FREE Ground shipping on all orders of $35
or more. Hurry up and shop folks, this offer expires April 30th!
http://www.thinkgeek.com/freeshipping/?cpg=12297

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[John Zielinski]

Benjamin Herrenschmidt wrote:

>The whole var thing is too messy right now. Especially, the lack of a
>per-VT var right now makes things even more difficult
>  
>
I'm currently in the middle of adding a per-VT var to my kernel.  
They'll be in addition to the fb's current var so that when a fb using 
program releases the frame buffer I can restore the VT's var instead of 
depending on the app to do it right.  I'll post some patches when I get 
things working properly. 

John




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> >The whole var thing is too messy right now. Especially, the lack of a
> >per-VT var right now makes things even more difficult
> >  
> >
> I'm currently in the middle of adding a per-VT var to my kernel.  
> They'll be in addition to the fb's current var so that when a fb using 
> program releases the frame buffer I can restore the VT's var instead of 
> depending on the app to do it right.  I'll post some patches when I get 
> things working properly. 

This breaks the way linux works. It is up to the program to restore the 
console. The same case for vgacon. The X server is the one that restores 
the graphics hardware back to the VGA text mode. This patch would not ne 
accepted for a stable release. 




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[John Zielinski]

Geert Uytterhoeven wrote:

>>On the other hand, I tend to think that if you have a KD_TEXT console,
>>accel is under fbcon control, if you have a KD_GRAPHICS console, then
>>the accel engine is under you control. I don't think any else make any
>>sense in fact
>>    
>>
>
>You're right, I forgot about that.
>  
>
That's a good idea.  I'm tinkering with the idea that when an app 
switches to KD_GRAPHICS that it lock out all other attempts to access 
the framebuffer.  I ran two copies of fbtest about 5 seconds apart and 
the results were interesting....  :)

So using KD_TEXT/KD_GRAPHICS as the access control mechanism between 
multiple applications and fbcon might be the way to go.  It already 
tells the console to not to write to the screen so it seems like a 
natural extension of the concept.

John




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[John Zielinski]

James Simmons wrote:

>This breaks the way linux works. It is up to the program to restore the 
>console. The same case for vgacon. The X server is the one that restores 
>the graphics hardware back to the VGA text mode. This patch would not ne 
>accepted for a stable release. 
>  
>
No, this would not be for the stable release.  But what I'm saying is 
that it's nice that a program cleans up after itself but we shouldn't 
rely on it.  Take resources like memory, file handles, sockets, etc.  
Most program release those resources when the shut down normally but 
what if they seg fault or exit unexpectedly?  The kernel cleans up after 
the process regardless.

So if a fb program restores the mode properly then we should do nothing 
but if it doesn't either because of a bad programming or an unexpected 
termination then we should reset the mode to what the user has selected 
for their console.

John




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Wed, 2004-04-28 at 08:33, James Simmons wrote:
> > >The whole var thing is too messy right now. Especially, the lack of a
> > >per-VT var right now makes things even more difficult
> > >  
> > >
> > I'm currently in the middle of adding a per-VT var to my kernel.  
> > They'll be in addition to the fb's current var so that when a fb using 
> > program releases the frame buffer I can restore the VT's var instead of 
> > depending on the app to do it right.  I'll post some patches when I get 
> > things working properly. 
> 
> This breaks the way linux works. It is up to the program to restore the 
> console. The same case for vgacon. The X server is the one that restores 
> the graphics hardware back to the VGA text mode. This patch would not ne 
> accepted for a stable release. 

I don't agree. While it's true that we shouldn't bother saving/restoring
the var for a KD_GRAPHICS VT which is under app control, we should do it
for fbcon controlled consoles.

Right now, it just doesn't work properly and you know it. It's actually
a regression from 2.4 and I don't think his patch will be more invasive
than the rest of the changes you have been doing lately ;)

Face it, the current fbcon resize mecanism is just broken. It cannot
work properly without full mode management in all drivers. A per-VT var
along with fbcon properly adapting it's size to the VT's var would bring
us back to 2.4 level of functionality at least, which is a good thing.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

> So if a fb program restores the mode properly then we should do nothing 
> but if it doesn't either because of a bad programming or an unexpected 
> termination then we should reset the mode to what the user has selected 
> for their console.

Which is why I've been adding calls from the VT subsystem to the console
when a program enter/leaves KD_GRAPHICS (via the blank() callback to which
I added a parameter) so fbcon can reprogram the mode & the engine. That
fixed various problems with XFree not cleaning things properly.

The proper policy here is simple. Whoever gets control of a console is
responsible for setting it up the way he needs. When leaving, it is not
requires to restore the mode it got on entry, but it should try at least
to leave the card in a sane state (accel engine not running). Though we
should probably add more security there to the fbdev's so they actually
make sure of that since it's always possible to kill a userland program
without letting it cleanup.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> a regression from 2.4 and I don't think his patch will be more invasive
> than the rest of the changes you have been doing lately ;)
> 
> Face it, the current fbcon resize mecanism is just broken. It cannot
> work properly without full mode management in all drivers. A per-VT var
> along with fbcon properly adapting it's size to the VT's var would bring
> us back to 2.4 level of functionality at least, which is a good thing.

I refuse to have hacks to deal with a cripple VT system. What is 
acceptable is that we add what we need to struct vc_data. You do that and 
I have no problem then. From what I can see we need the following.

1) Color depth.
2) Frequency.
3) Over scan color.

Anything else? 

Personally I rather add these to struct vc_data. Consider if we ever had 
hardware that supported different hardware text modes. Say some at 
different color depths. I rather do this!!





-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> No, this would not be for the stable release.  But what I'm saying is 
> that it's nice that a program cleans up after itself but we shouldn't 
> rely on it.  Take resources like memory, file handles, sockets, etc.  
> Most program release those resources when the shut down normally but 
> what if they seg fault or exit unexpectedly?  The kernel cleans up after 
> the process regardless.
> 
> So if a fb program restores the mode properly then we should do nothing 
> but if it doesn't either because of a bad programming or an unexpected 
> termination then we should reset the mode to what the user has selected 
> for their console.

Trust me. I wish the console would cleanup itself. It would take a good 
amount of work to redo.



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Wed, 2004-04-28 at 09:18, James Simmons wrote:
> > a regression from 2.4 and I don't think his patch will be more invasive
> > than the rest of the changes you have been doing lately ;)
> > 
> > Face it, the current fbcon resize mecanism is just broken. It cannot
> > work properly without full mode management in all drivers. A per-VT var
> > along with fbcon properly adapting it's size to the VT's var would bring
> > us back to 2.4 level of functionality at least, which is a good thing.
> 
> I refuse to have hacks to deal with a cripple VT system. What is 
> acceptable is that we add what we need to struct vc_data. You do that and 
> I have no problem then. From what I can see we need the following.
> 
> 1) Color depth.
> 2) Frequency.
> 3) Over scan color.

No, no, no ... We don't need those, we need a var structure, that's all.

> Anything else? 
> 
> Personally I rather add these to struct vc_data. Consider if we ever had 
> hardware that supported different hardware text modes. Say some at 
> different color depths. I rather do this!!





-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Wed, 2004-04-28 at 09:24, James Simmons wrote:
> > No, this would not be for the stable release.  But what I'm saying is 
> > that it's nice that a program cleans up after itself but we shouldn't 
> > rely on it.  Take resources like memory, file handles, sockets, etc.  
> > Most program release those resources when the shut down normally but 
> > what if they seg fault or exit unexpectedly?  The kernel cleans up after 
> > the process regardless.
> > 
> > So if a fb program restores the mode properly then we should do nothing 
> > but if it doesn't either because of a bad programming or an unexpected 
> > termination then we should reset the mode to what the user has selected 
> > for their console.
> 
> Trust me. I wish the console would cleanup itself. It would take a good 
> amount of work to redo.

Nah, it's a lot easier to restore the state you need than to
save/restore everything. It's not even always possible to save/restore
everything (think: write only registers).

I got that pretty much working lately with the new blank() argument
anyway, though some drivers need to be told about it FB_ACTIVATE_FORCE I
suppose.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> > I refuse to have hacks to deal with a cripple VT system. What is 
> > acceptable is that we add what we need to struct vc_data. You do that and 
> > I have no problem then. From what I can see we need the following.
> > 
> > 1) Color depth.
> > 2) Frequency.
> > 3) Over scan color.
> 
> No, no, no ... We don't need those, we need a var structure, that's all.

No. Look the logic should go from the upper VT layer to the lower driver 
to accomplish something. Not the reverse.

struct vc_data -> native console driver structure --> fbdev

Not.

fbdev --> native console driver --> struct vc_data.

This was the way it uses to be. If this is the case then I suggest we 
write our own vt.c and vt_ioctl.c for the fbcon layer. We shouldn't even
use struct vc_data then. In fact each fbdev driver should be independent 
console drivers. That is how 2.4.X was. Each driver was nearly its own 
console driver. We need to go one way or the other. 


 



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

> No. Look the logic should go from the upper VT layer to the lower driver 
> to accomplish something. Not the reverse.
> 
> struct vc_data -> native console driver structure --> fbdev

But the upper VT layer has no clue about what a graphic mode can be and
we know well enough that we _need_ to be able to enforce graphics mode
or want to play with them by manipulating var structures, without losing
the console.

Your logic is only ever valid once every single fbdev is capable of
doing proper mode management & validation. And even then, there isn't
a 1:1 relationchip between a mode and an fbcon size.

> Not.
> 
> fbdev --> native console driver --> struct vc_data.
> 
> This was the way it uses to be. If this is the case then I suggest we 
> write our own vt.c and vt_ioctl.c for the fbcon layer. We shouldn't even
> use struct vc_data then. In fact each fbdev driver should be independent 
> console drivers. That is how 2.4.X was. Each driver was nearly its own 
> console driver. We need to go one way or the other. 

I agree we need to go away and I'm not proposing adding anything to
fb_info nor whatever in this regard. It has to be all self-contained
in the console subsystem, that is fbcon. And we already have a data
structure holding per-console fbcon-specific data, it's called
struct display.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[John Zielinski]

Benjamin Herrenschmidt wrote:

>>1) Color depth.
>>2) Frequency.
>>3) Over scan color.
>>    
>>
>
>No, no, no ... We don't need those, we need a var structure, that's all.
>  
>

Yes, we need a var structure.  How else would you set one console to 
1920x540p and another to 1920x1080i (540 doublescan)?  Or try different 
timings?  I want my first console at a sane video mode like 480p while 
the others can I can play with.  If the HDTV I'm testing looses sync I 
can always switch to console one and restore all the consoles back to 
480p and try again without having to shell in via serial or over a 
network.  A second computer may not always be close at hand.

John




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> Nah, it's a lot easier to restore the state you need than to
> save/restore everything. It's not even always possible to save/restore
> everything (think: write only registers).

If this is true then it would be better if we have the below logic.

struct vc_data --> native driver layer --> fbdev.

instead of save/restore the var for each VC.





-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Wed, 2004-04-28 at 09:57, James Simmons wrote:
> > Nah, it's a lot easier to restore the state you need than to
> > save/restore everything. It's not even always possible to save/restore
> > everything (think: write only registers).
> 
> If this is true then it would be better if we have the below logic.
> 
> struct vc_data --> native driver layer --> fbdev.
> 
> instead of save/restore the var for each VC.

I think we are not understanding eachother....

The save/restore thing is a different matter than the var thing
altogether....

The var for each vc is simply having a different mode for each VC
and not relying on the doggy magic mode setting done by fbcon to
set a mode matching the VC size. fbcon is responsible for setting
the proper mode for each VC upon entry to that VC, and so with the
above, that mean just having a var per VC.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[John Zielinski]

James Simmons wrote:

>Trust me. I wish the console would cleanup itself. It would take a good 
>amount of work to redo.
>  
>
I don't think it will be too much work.  Here's my line of thinking:

When booting fbcon sets all it's per VT var structures to what the 
driver returns as it's startup mode.  After that only fbset can change 
these per VT var structures by using a special flag it it's mode set 
call.  An fb program would change the mode any which way it wants which 
would change a the "current" var structure.  When that fb program ends 
or goes back the KD_TEXT then fbcon just sets the mode to what it has in 
it's var for the particular VT number.

At no point does fbcon try to save the current hardware state.  All it 
does is put things back to the way they were at boot or to what the user 
chose after boot with fbset.

Oh, almost forgot.  Changing the resolution using the console set rows 
and columns ioctl and changing resolution  would be equivilent to fbset 
changing the res and be stored in the per VT var structures as well.

Now I don't understand why this info needs to move back to the vt 
layer.  Let's say we had another console driver that ran a vector 
display or a pixel board or whatever other exotic display.  Different 
resolutions, timings, colour depths etc would not apply to those 
devices.  The laser vector type display might have stuff like the 
rotation speed of various mirrors.  (this is an example, I'm not saying 
it's a practical application).   That display would have it's own fbset 
type utility to set those things.  So the low level console driver 
should be where the mode is actually stored (again on a per VT basis).

But storing some of this info in the vc_data like you said would be 
useful when one console takes over for another then it can set it self 
up in the same general mode that the last console would be in although 
all the timings might be off.

John





-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Antonino A. Daplas]

On Tuesday 27 April 2004 16:51, Geert Uytterhoeven wrote:
> On Tue, 27 Apr 2004, Benjamin Herrenschmidt wrote:
> > On Tue, 2004-04-27 at 10:19, James Simmons wrote:
> > > Okay. This is my first attempt to address this problem. There is not
> > > test for accelerated copyarea or image drawing as we lack the flag. I
> > > haven't figured where to put it yet. We have 3 choices.
> > >
> > >
> > > fb_var_screeninfo
> > > fb_fix_screeninfo
> > > fb_info
> >
> > Well... fix is what gets filled after a mode is set, let's put it in
> > there along with the linewidth etc...
>
> Indeed. BTW, no comments on my proposal to add
> fb_fix_screeninfo.capabilities?
>

Since accelerated drawing functions have no relevence whatsoever in userspace, 
placing it in fb_info is better.  But I won't disagree if it's placed in 
fb_fix_screeninfo.

Tony




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[John Zielinski]

James Simmons wrote:

>No. Look the logic should go from the upper VT layer to the lower driver 
>to accomplish something. Not the reverse.
>
>struct vc_data -> native console driver structure --> fbdev
>
>  
>
The problem with that is how would something like fbset work in that 
situation?   The current way of selecing number of rows/columns through 
the vt layer and hoping for the best in what mode you actually get is 
way too limited to be the only mechanism.

Don't misunderstand me here.  I do want to be able to tell the vt to 
switch to 1024x768@85-32 and have it propagate down and set the right 
mode.  This shouldn't be a problem once we get a mechanism in place to 
load a user's mode database into the kernel at run time.  Then the 
timings will be exactly what I want when I say 1024x768@85-32.

>Not.
>
>fbdev --> native console driver --> struct vc_data.
>
>  
>
But how do I fiddle with this to get things just right in the first 
place?   Right now that requires the the reversed path starting at the 
fbdev shown immediately above. 

What we need is a vt call that says I'm switching to 1024 x 768 with 
this mode data that I wan't to pass down to the fbdev.  That way the vt 
adjusts it's row/column settings based on the font size and then passes 
the mode data down.  That's how the fbset (and other) mode tuning 
utilities should work and should only be used by mode tuning utilities.  
Once the user is happy they would include their tuned mode in their 
database for other apps to use.

All other apps should use the generic vt call to ask for 1024x768 at 
85Hz in 32bpp and the kernel would give them the mode from the users 
database.


>This was the way it uses to be. If this is the case then I suggest we 
>write our own vt.c and vt_ioctl.c for the fbcon layer. We shouldn't even
>use struct vc_data then. In fact each fbdev driver should be independent 
>console drivers. That is how 2.4.X was. Each driver was nearly its own 
>console driver. We need to go one way or the other. 
>

I like your way, from vt down to the fbdev.  I just want a mechanism for 
fbset and other mode tuning utilities to work.

John




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[John Zielinski]

James Simmons wrote:

>If this is true then it would be better if we have the below logic.
>
>struct vc_data --> native driver layer --> fbdev.
>
>instead of save/restore the var for each VC.
>  
>

I don't mean save as in record the current hardware state.  I mean save 
as in remember the last mode the user had active on the console before a 
graphics app starts.  This only has to be done 1) on boot and 2) after 
running fbset or calling VT_RESIZE.

Once we get the users mode db into the kernel then we only need to 
remember the generic stuff (x, y, Hz, bpp) per vt as the user will have 
the ability to map:

(x, y, Hz, bpp) -> (exact timing options)

Either way, regardless if we remember a per vt var or (x, y, Hz, bpp), I 
still would like the vt/fbcon to reset the the last mode on switch from 
KD_GRAPHICS to KD_TEXT.

John




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

> 
> Either way, regardless if we remember a per vt var or (x, y, Hz, bpp), I 
> still would like the vt/fbcon to reset the the last mode on switch from 
> KD_GRAPHICS to KD_TEXT.

That is done in fbcon in the unblank callback, though it uses the currently
crappy way that fbcon has to build up the var structure

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Alex Stewart]

>> > 1) Color depth.
>> > 2) Frequency.
>> > 3) Over scan color.
>>
>> No, no, no ... We don't need those, we need a var structure, that's
>> all.

I agree, a var structure is how video modes are represented, and should be
the way they're always represented.  Anything else will either be
inadequate or horribly duplicative.  I also think that storing pixel
resolutions and depths (let alone frequencies, etc) in a layer that
shouldn't even know what a framebuffer is generally a poor way to do
things.

> No. Look the logic should go from the upper VT layer to the lower driver
>  to accomplish something. Not the reverse.
>
> struct vc_data -> native console driver structure --> fbdev
>
> Not.
>
> fbdev --> native console driver --> struct vc_data.

Heh, well, I hope I'm not stirring things up too much here if I say I
think both of those paths are wrong :)

Actually, I think we're going round and round here partly because nobody
has really sat down and thought about what roles different components
should be playing in the overall system.

The way I see it, we have the following components, which perform the
following general tasks (note, I'm talking intended functionality, not
necessarily exact implementation):

framebuffer device driver (fbdev) - manages the actual hardware.
fbcon - Represents a text console using a framebuffer for display.
console - upper layer (generic) console system.
vt - allows multiple things (consoles, other apps) to access the same
display and input hardware.
other apps - VT-aware userland framebuffer apps.

Now one of the problems I see with things at the moment, is that some of
these components (vt and the console stuff in particular) have very
ambiguous and sometimes conflicting heirarchies in the current
implementation.  I think it would be good to start by defining what the
right relationships between these components should be.

The way I see it, if we formalize the boundaries between these components
a bit better, the best arrangement is something like the following:

1. fbdev sits on the bottom, talks to the hardware. (this is pretty obvious)

2. vt sits on top of fbdev.  Its purpose is to arbitrate access to
hardware resources.  It does not care about consoles, or display settings,
just who's allowed to talk to the device drivers right now.

3. fbcon sits on top of vt.  It uses vt to obtain access to fbdev, just
like any other application should.  vt should not really need to care that
this is a console as opposed to any other application which just wants to
access the framebuffer.

4. console sits on top of fbcon (for display) and vt (for keyboard access).

5. other apps sit on top of vt, to talk to fbdev (and possibly keyboard,
etc).
 _____________________________
|     console     |     |     |
|-----------------|other|other|
|      fbcon      | app | app |
|-----------------+-----+-----|
| (1) | (2) | (3) | (4) | (5) |
| :::::::::::: vt ::::::::::::|
|--------------.--------------'
|     fbdev    |   keyboard   |
`--------------'--------------'

Now, first of all, I realize this is not the way things currently work.  I
think this is part of the problem.  In the proposed heirarchy, fbcon and
the rest of the console are treated exactly the same as any other
application from the standpoint of accessing the framebuffer. 
Applications do not need to know or care whether there's a console running
on this framebuffer or not if they're just wanting to draw on a
framebuffer, and vt doesn't have to know or care what a "video mode" is.

If we want to set the video mode used by a framebuffer console, we should
tell fbcon.  fbcon is (by definition) the only component which has (or
should have) a concept of "video mode" and also has a concept of
"console".  fbcon sets the appropriate var using fbdev (assuming vt says
it is the currently active app for that screen), and then remembers what
the right var is for future reference.  Any time vt changes to another VT,
and then changes back to this one, fbdev gets notified that this VT has
become active again, and can re-set fbdev's var to the right values.

This of course requires that we have some appropriate way for something
like fbset to "tell fbcon" to change the video mode.  I suggest it just be
done through a fbcon-specific console ioctl (console passes it through to
fbcon, fbcon interprets it and communicates with fbdev as appropriate). 
vt never comes into the picture.

By moving the console and fbcon code out to the same layer as other
VT-aware framebuffer applications, the architecture becomes much cleaner,
and a bunch of the nagging little issues we're currently discussing become
a lot easier to deal with.

So now you can all tell me how much crap you think this suggestion is..

-alex




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ; -)

[Geert Uytterhoeven]

On Wed, 28 Apr 2004, Antonino A. Daplas wrote:
> On Tuesday 27 April 2004 16:51, Geert Uytterhoeven wrote:
> > On Tue, 27 Apr 2004, Benjamin Herrenschmidt wrote:
> > > On Tue, 2004-04-27 at 10:19, James Simmons wrote:
> > > > Okay. This is my first attempt to address this problem. There is not
> > > > test for accelerated copyarea or image drawing as we lack the flag. I
> > > > haven't figured where to put it yet. We have 3 choices.
> > > >
> > > >
> > > > fb_var_screeninfo
> > > > fb_fix_screeninfo
> > > > fb_info
> > >
> > > Well... fix is what gets filled after a mode is set, let's put it in
> > > there along with the linewidth etc...
> >
> > Indeed. BTW, no comments on my proposal to add
> > fb_fix_screeninfo.capabilities?
> >
>
> Since accelerated drawing functions have no relevence whatsoever in userspace,
> placing it in fb_info is better.  But I won't disagree if it's placed in
> fb_fix_screeninfo.

Point taken.

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: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Geert Uytterhoeven]

On Wed, 28 Apr 2004, Benjamin Herrenschmidt wrote:
> > No. Look the logic should go from the upper VT layer to the lower driver
> > to accomplish something. Not the reverse.
> >
> > struct vc_data -> native console driver structure --> fbdev
>
> But the upper VT layer has no clue about what a graphic mode can be and
> we know well enough that we _need_ to be able to enforce graphics mode
> or want to play with them by manipulating var structures, without losing
> the console.
>
> Your logic is only ever valid once every single fbdev is capable of
> doing proper mode management & validation. And even then, there isn't
> a 1:1 relationchip between a mode and an fbcon size.
>
> > Not.
> >
> > fbdev --> native console driver --> struct vc_data.
> >
> > This was the way it uses to be. If this is the case then I suggest we
> > write our own vt.c and vt_ioctl.c for the fbcon layer. We shouldn't even
> > use struct vc_data then. In fact each fbdev driver should be independent
> > console drivers. That is how 2.4.X was. Each driver was nearly its own
> > console driver. We need to go one way or the other.
>
> I agree we need to go away and I'm not proposing adding anything to
> fb_info nor whatever in this regard. It has to be all self-contained
> in the console subsystem, that is fbcon. And we already have a data
> structure holding per-console fbcon-specific data, it's called
> struct display.

Agreed. We need a per-VT var.

The way I see it (and have always seen it), fbcon implements a text console _on
top of_ fbdev, just like vgacon implements a text console on top of the VGA
hardware. Not the other way around!

You give fbcon the number of columns and rows, and a font, and it will build a
text console using the current var (of the VT).

<sidenote>
In extremis, this means I wouldn't have a problem if you use stty to set a
80x25 console with a 8x16 font on a 1280x1024 screen, and get text in the
upper left 640x400 corner only. Or try a 160x100 console on a 640x480 screen,
of which only 80x32 characters are visible.

This would IMHO be much more logical than the way it is now.
</sidenote>

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: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Wed, 2004-04-28 at 18:41, Geert Uytterhoeven wrote:

> Agreed. We need a per-VT var.
> 
> The way I see it (and have always seen it), fbcon implements a text console _on
> top of_ fbdev, just like vgacon implements a text console on top of the VGA
> hardware. Not the other way around!

Then I must say we are in violent agreement :)

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> > No. Look the logic should go from the upper VT layer to the lower driver 
> > to accomplish something. Not the reverse.
> > 
> > struct vc_data -> native console driver structure --> fbdev
> 
> But the upper VT layer has no clue about what a graphic mode can be and
> we know well enough that we _need_ to be able to enforce graphics mode
> or want to play with them by manipulating var structures, without losing
> the console.
> 
> Your logic is only ever valid once every single fbdev is capable of
> doing proper mode management & validation. And even then, there isn't
> a 1:1 relationchip between a mode and an fbcon size.

  But we do have the proper data already. Its called modedb. I have two 
reasons for why I hate to have a var in struct display. One is that it
makes fbcon heavier. You now have a extra var in every struct display. 
Usually the user has several if not all VCs at the same resolution. That 
means there is alot of duplication of the same data for now reason. If we 
use modedb to fill in the extra data then we don't need the extra data in
each struct display. The bonus there is permentally stored data to represent 
that one mode to everybody!! Much lighter.
  Second I have seen the fbset method to set the VC fail miserably. Whereas I 
have never seen using modedb at boot time fail unless the mode is not supported.  
There have been many times where using fbset has left my screen blank :-( 
  I'm going to create a patch that uses modedb in fbcon to set the mode. 
This will solve all our problems for the most part.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> >No. Look the logic should go from the upper VT layer to the lower driver 
> >to accomplish something. Not the reverse.
> >
> >struct vc_data -> native console driver structure --> fbdev
> >
> >  
> >
> The problem with that is how would something like fbset work in that 
> situation?   The current way of selecing number of rows/columns through 
> the vt layer and hoping for the best in what mode you actually get is 
> way too limited to be the only mechanism.
> 
> Don't misunderstand me here.  I do want to be able to tell the vt to 
> switch to 1024x768@85-32 and have it propagate down and set the right 
> mode.  This shouldn't be a problem once we get a mechanism in place to 
> load a user's mode database into the kernel at run time.  Then the 
> timings will be exactly what I want when I say 1024x768@85-32.

But we do have that mechanism. Its call modedb. The only thing missing in 
the VT layer is the depth value whcich we should have anyways. BTW VT220 
did support different depths. I have t find out what the esc code was for 
that. I have the manual here. The other piece of data is the frequency. 
That is harder to deal with.I have to talk to a friend who use to program 
Vt220 about this. 



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> > Agreed. We need a per-VT var.
> > 
> > The way I see it (and have always seen it), fbcon implements a text console _on
> > top of_ fbdev, just like vgacon implements a text console on top of the VGA
> > hardware. Not the other way around!
> 
> Then I must say we are in violent agreement :)

How do you feel about using modedb handling in fbcon. Let me create this 
patch and test it out. If it works then we can resolve this issue.





-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> I don't mean save as in record the current hardware state.  I mean save 
> as in remember the last mode the user had active on the console before a 
> graphics app starts.  This only has to be done 1) on boot and 2) after 
> running fbset or calling VT_RESIZE.
> 
> Once we get the users mode db into the kernel then we only need to 
> remember the generic stuff (x, y, Hz, bpp) per vt as the user will have 
> the ability to map:
> 
> (x, y, Hz, bpp) -> (exact timing options)

Modedb!!!!



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ; -)

[James Simmons]

> > Since accelerated drawing functions have no relevence whatsoever in userspace,
> > placing it in fb_info is better.  But I won't disagree if it's placed in
> > fb_fix_screeninfo.
> 
> Point taken.

So shoudl be palce it in fb_info then? Are we in agreement on this?




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> >> > 1) Color depth.
> >> > 2) Frequency.
> >> > 3) Over scan color.
> >>
> >> No, no, no ... We don't need those, we need a var structure, that's
> >> all.
> 
> I agree, a var structure is how video modes are represented, and should be
> the way they're always represented.  Anything else will either be
> inadequate or horribly duplicative.  I also think that storing pixel
> resolutions and depths (let alone frequencies, etc) in a layer that
> shouldn't even know what a framebuffer is generally a poor way to do
> things.

The tty layer alreasy stores the resolution. See asm/termios.h. What it 
lacks is depth and frequency. I have to do some digging to see if the 
VT220 dealt with that data. I bet it did.

> Heh, well, I hope I'm not stirring things up too much here if I say I
> think both of those paths are wrong :)

I have to look over what you said in detail. I have to get to coding a fix 
for the fbcon resize issue.





-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Geert Uytterhoeven]

On Wed, 28 Apr 2004, James Simmons wrote:
> > > No. Look the logic should go from the upper VT layer to the lower driver
> > > to accomplish something. Not the reverse.
> > >
> > > struct vc_data -> native console driver structure --> fbdev
> >
> > But the upper VT layer has no clue about what a graphic mode can be and
> > we know well enough that we _need_ to be able to enforce graphics mode
> > or want to play with them by manipulating var structures, without losing
> > the console.
> >
> > Your logic is only ever valid once every single fbdev is capable of
> > doing proper mode management & validation. And even then, there isn't
> > a 1:1 relationchip between a mode and an fbcon size.
>
>   But we do have the proper data already. Its called modedb. I have two

So I cannot have a mode that's not in modedb, since it won't survive switching
VTs.

> reasons for why I hate to have a var in struct display. One is that it
> makes fbcon heavier. You now have a extra var in every struct display.

... vs. a (non __init) modedb that stays in RAM after bootup.

Besides the initial mode (modedb, EDID, kernel arg, ...), I'd like to keep mode
databases in userspace where possible.

> Usually the user has several if not all VCs at the same resolution. That
> means there is alot of duplication of the same data for now reason. If we

Alternatively, make the vars in struct display pointers to reference counted
data, so initially they all point to the same var. But I don't know if it's
worth the extra effort.

> use modedb to fill in the extra data then we don't need the extra data in
> each struct display. The bonus there is permentally stored data to represent
> that one mode to everybody!! Much lighter.

But I'm still limited to the modes in modedb.

>   Second I have seen the fbset method to set the VC fail miserably. Whereas I
> have never seen using modedb at boot time fail unless the mode is not supported.
> There have been many times where using fbset has left my screen blank :-(

I consider it a bug in the fbdev driver if you manage to end up with a blank
screen (not taking into account modes beyond your monitor's limitations if
monspecs are not implemented/working).

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: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> > > Your logic is only ever valid once every single fbdev is capable of
> > > doing proper mode management & validation. And even then, there isn't
> > > a 1:1 relationchip between a mode and an fbcon size.
> >
> >   But we do have the proper data already. Its called modedb. I have two
> 
> So I cannot have a mode that's not in modedb, since it won't survive switching
> VTs.

Well its a bug in fbdev that we can't alter the modedb. Especially that we 
have in struct fb_info struct fb_monspecs  which has a modedb field. We 
need to add the functionally to export the drivers modedb to userland and 
vice versa.

> > reasons for why I hate to have a var in struct display. One is that it
> > makes fbcon heavier. You now have a extra var in every struct display.
> 
> ... vs. a (non __init) modedb that stays in RAM after bootup.
> 
> Besides the initial mode (modedb, EDID, kernel arg, ...), I'd like to keep mode
> databases in userspace where possible.

That kind of sucks for modular fbdev drivers. Also with EDID data the 
driver creates the respected modedb it can work with. Consider the case 
where we have a fbdev driver all happy. Now we unplug the monitor and plug 
in a different kind. With the new monitor the old mode doesn't work. How 
do we handle this? Even if you say you can load the new database from 
userland before we change monitors we still have to create a database.
With EDID we can generate those modes. But its driver side. 

> > Usually the user has several if not all VCs at the same resolution. That
> > means there is alot of duplication of the same data for now reason. If we
> 
> Alternatively, make the vars in struct display pointers to reference counted
> data, so initially they all point to the same var. But I don't know if it's
> worth the extra effort.

Thats a possibity. Of course we still have modedb and it is needed for the 
above reasons.

> But I'm still limited to the modes in modedb.

Not if we fix modedb to take userland modes.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Alex Stewart]

> The tty layer alreasy stores the resolution. See asm/termios.h. What it
> lacks is depth and frequency. I have to do some digging to see if the
> VT220 dealt with that data. I bet it did.

Well, for what it's worth, I think that's wrong too.

-alex






-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> > The tty layer alreasy stores the resolution. See asm/termios.h. What it
> > lacks is depth and frequency. I have to do some digging to see if the
> > VT220 dealt with that data. I bet it did.
> 
> Well, for what it's worth, I think that's wrong too.

Well it's the POSIX standard. 



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[John Zielinski]

James Simmons wrote:

>But we do have that mechanism. Its call modedb. The only thing missing in 
>the VT layer is the depth value whcich we should have anyways. BTW VT220 
>  
>
The one limitation is that it has to be compiled into the kernel.  I was 
working on a solution for that but my devel box died on me and I lost 
some of that code.  I mentioned it a while back in this list.  Basically 
the kernel would read in the modedb from a initramfs image file very 
early in the boot process.

John




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[John Zielinski]

Geert Uytterhoeven wrote:

>So I cannot have a mode that's not in modedb, since it won't survive switching
>VTs.
>
>  
>
It would be possible.  Let's say that the future fbset can modify the 
kernels modedb and that the vt driver can also swtich to a mode by an 
alias string.  All fbset would have to do is create a new modedb entry 
called "vt02_temp_mode" and then tell the vt to switch VT 2 to 
"vt02_temp_mode".  The vt driver remembers that VT 2 is now set to 
"vt02_temp_mode" (via a pointer or a numeric modedb index or something) 
so when you switch to back to VT 2 you get your custom mode again.

The VT's vc_data struct would have an extra pointer/index variable.  If 
it's non zero it uses that to reference the proper modedb entry.  If 
it's zero then it uses it's x,y,Hz,bpp variables to reference the proper 
modedb.  This way you could also have multiple modes with the same 
x,y,Hz,bpp but different timing and sync options.

This approach would also get rid of the need for a special mode set api 
for fbset to tweak the current mode timing.  It would just use the 
standard kernel modedb editing API.

John




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

>   But we do have the proper data already. Its called modedb. I have two 
> reasons for why I hate to have a var in struct display. One is that it
> makes fbcon heavier. You now have a extra var in every struct display. 
> Usually the user has several if not all VCs at the same resolution. That 
> means there is alot of duplication of the same data for now reason. If we 
> use modedb to fill in the extra data then we don't need the extra data in
> each struct display. The bonus there is permentally stored data to represent 
> that one mode to everybody!! Much lighter.

I don't agree... again. What is the semantics _users_ are the most
comfortable with ? an stty size or a resolution ? All OSes so far
let user pick up resolutions (w/h/freq), this is the thing users are
getting used to :) That's how we should, via fbset or whatever other
tool we have, set a resolution. The actual fbcon size is just a side
effect of the resolution and font size.

Modedb isn't always a solution with some fancy displays too...

Also, if I ever start doing dual head, I'll have interesting side
effects to deal with, which means for example  that enabling mirroring
will possibly change the resolution of the primary head. That means that
I'll have to trigger a mode change in console based on ... a resolution,
not a console size. I just want the console size to adapt.

Now, wether this mode is common to all VCs or per-VC is a different
story, I tend to like when it's per-VC for various reasons, one of them
beeing that if I set something that doesn't work, I can always go back
to my working VC :)

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Thu, 2004-04-29 at 02:48, James Simmons wrote:
> > > Agreed. We need a per-VT var.
> > > 
> > > The way I see it (and have always seen it), fbcon implements a text console _on
> > > top of_ fbdev, just like vgacon implements a text console on top of the VGA
> > > hardware. Not the other way around!
> > 
> > Then I must say we are in violent agreement :)
> 
> How do you feel about using modedb handling in fbcon. Let me create this 
> patch and test it out. If it works then we can resolve this issue.

No, no, no...

First, we would have to fix all drivers to properly probe monitors or
provide some valid monitor data for things that cannot be probed. Then,
I don't think modedb is really an fbcon matter. I completely agree with
Geert here, I think the current scheme is backward.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Alex Stewart]

>> The tty layer alreasy stores the resolution. See asm/termios.h. What
>> it lacks is depth and frequency. I have to do some digging to see if
>> the VT220 dealt with that data. I bet it did.
>>
>> Well, for what it's worth, I think that's wrong too.
>
> Well it's the POSIX standard.

Yeah, I know, but that doesn't mean I have to agree with it.  :)

Anyway, my point is just that ttys should only care about characters.  In
the console system, fbcon is what does the translation from a
character-based display to a pixel-based display.  Thus, nothing above
fbcon should have any notion (or any need to have a notion) of pixels,
color depths, frequency timings, or any of that.

-alex




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> > How do you feel about using modedb handling in fbcon. Let me create this 
> > patch and test it out. If it works then we can resolve this issue.
> 
> No, no, no...
> 
> First, we would have to fix all drivers to properly probe monitors or
> provide some valid monitor data for things that cannot be probed. Then,
> I don't think modedb is really an fbcon matter. I completely agree with
> Geert here, I think the current scheme is backward.

No we don't. We have the default modedb. I already tested a patch using 
modedb. It works like a charm :-) I can stty all I want!!!! 



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> I don't agree... again. What is the semantics _users_ are the most
> comfortable with ? an stty size or a resolution ? All OSes so far
> let user pick up resolutions (w/h/freq), this is the thing users are
> getting used to :) 

  Fbcon is a graphical emulation of a VT220. You set a terminal size with 
stty. I do it all the time when logged in with a serial console. I'm going
to work on vgacon to also be able to change the size of text terminal. The 
same goes for sticon etc. See we have one tool that works on ANY terminal
if resizing is supported. Otherwise we need to make stiset and a vgaset.
Yuck!!!
   Now if the user want to program a graphics application and run it on 
the console then fine. I plan to put in sysfs a way to cat a string to
change the resolution using modedb. Then you get the function you want 
above.

> Modedb isn't always a solution with some fancy displays too...

Then you add to the modedb. Right now we lack that functionalty.

> Also, if I ever start doing dual head, I'll have interesting side
> effects to deal with, which means for example  that enabling mirroring
> will possibly change the resolution of the primary head. That means that
> I'll have to trigger a mode change in console based on ... a resolution,
> not a console size. I just want the console size to adapt.

I take up the challenge. I have dual head matrox card to experiment with.



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Otto Solares]

On Thu, Apr 29, 2004 at 01:26:57AM +0100, James Simmons wrote:
> I plan to put in sysfs a way to cat a string to
> change the resolution using modedb.

nice!.

> Then you add to the modedb. Right now we lack that functionalty.

Is not modedb marked __init so it's lost to userspace?  If not how
can i use modedb from userspace?

-otto



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Thu, 2004-04-29 at 10:02, James Simmons wrote:
> > > How do you feel about using modedb handling in fbcon. Let me create this 
> > > patch and test it out. If it works then we can resolve this issue.
> > 
> > No, no, no...
> > 
> > First, we would have to fix all drivers to properly probe monitors or
> > provide some valid monitor data for things that cannot be probed. Then,
> > I don't think modedb is really an fbcon matter. I completely agree with
> > Geert here, I think the current scheme is backward.
> 
> No we don't. We have the default modedb. I already tested a patch using 
> modedb. It works like a charm :-) I can stty all I want!!!! 

On your monitor... then most TFT owners will curse you :)

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ; -)

[Geert Uytterhoeven]

On Wed, 28 Apr 2004, Otto Solares wrote:
> On Thu, Apr 29, 2004 at 01:26:57AM +0100, James Simmons wrote:
> > I plan to put in sysfs a way to cat a string to
> > change the resolution using modedb.
>
> nice!.
>
> > Then you add to the modedb. Right now we lack that functionalty.
>
> Is not modedb marked __init so it's lost to userspace?  If not how
> can i use modedb from userspace?

Not anymore (since 2.6.5-rc1).

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: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> > No we don't. We have the default modedb. I already tested a patch using 
> > modedb. It works like a charm :-) I can stty all I want!!!! 
> 
> On your monitor... then most TFT owners will curse you :)

As we move from ioctls to sysfs we will also move from using 
fb_var_screeninfo as the userland interface to using mode strings.
That means when someone creates a string  like

834x721@81 

then for non TFT monitors it should fail. For TFT monitors it will work.
If someoone using stty ask for exotic mode then if the monitor is TFT then 
it should work. Else it should fail. 







-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Otto Solares]

On Thu, Apr 29, 2004 at 07:01:00PM +0100, James Simmons wrote:
> 
> > > No we don't. We have the default modedb. I already tested a patch using 
> > > modedb. It works like a charm :-) I can stty all I want!!!! 
> > 
> > On your monitor... then most TFT owners will curse you :)
> 
> As we move from ioctls to sysfs we will also move from using 
> fb_var_screeninfo as the userland interface to using mode strings.
> That means when someone creates a string  like
> 
> 834x721@81 
> 
> then for non TFT monitors it should fail. For TFT monitors it will work.
> If someoone using stty ask for exotic mode then if the monitor is TFT then 
> it should work. Else it should fail. 

James: I have seen sysfs support for fbdev in the -mm series
from Kronoz I think, this sysfs interface that will obsolete
var_screeninfo and fix_screeninfo is 2.6 material? or we have
to wait until 2.7 starts?

-otto



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Otto Solares]

On Thu, Apr 29, 2004 at 09:48:13PM +0200, Kronos wrote:
> Il Thu, Apr 29, 2004 at 12:11:42PM -0600, Otto Solares ha scritto: 
> > On Thu, Apr 29, 2004 at 07:01:00PM +0100, James Simmons wrote:
> > > 
> > > > > No we don't. We have the default modedb. I already tested a patch using 
> > > > > modedb. It works like a charm :-) I can stty all I want!!!! 
> > > > 
> > > > On your monitor... then most TFT owners will curse you :)
> > > 
> > > As we move from ioctls to sysfs we will also move from using 
> > > fb_var_screeninfo as the userland interface to using mode strings.
> > > That means when someone creates a string  like
> > > 
> > > 834x721@81 
> > > 
> > > then for non TFT monitors it should fail. For TFT monitors it will work.
> > > If someoone using stty ask for exotic mode then if the monitor is TFT then 
> > > it should work. Else it should fail. 
> > 
> > James: I have seen sysfs support for fbdev in the -mm series
> > from Kronoz I think, this sysfs interface that will obsolete
> > var_screeninfo and fix_screeninfo is 2.6 material?
> 
> Well, my patch uses class_simple to expose the framebuffer device so
> that udev can create /dev/fbN. Drivers can't touch the class_device
> (yet), for example radeonfb attaches edid info to the pci device, not to
> the framebuffer device. After the exams I will cook up a patch that
> should improve that.

Excellent news!

> > or we have to wait until 2.7 starts?
> 
> I think that for fully dynamic fb_info we should wait 2.7, the rest can
> go in 2.6. Resolution change via sysfs shouln't have a big impact on
> fbdev core.

I just would suggest as userspace fbdev consumer that is a pain to
do development using two different interfaces, if sysfs interface
will obsolete the ioctl one, it must have all the mechanics that
are present in the ioctl one so no mix of interfaces would be
required.

-otto



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

On Fri, 2004-04-30 at 04:01, James Simmons wrote:
> > > No we don't. We have the default modedb. I already tested a patch using 
> > > modedb. It works like a charm :-) I can stty all I want!!!! 
> > 
> > On your monitor... then most TFT owners will curse you :)
> 
> As we move from ioctls to sysfs we will also move from using 
> fb_var_screeninfo as the userland interface to using mode strings.
> That means when someone creates a string  like
> 
> 834x721@81 
> 
> then for non TFT monitors it should fail. For TFT monitors it will work.
> If someoone using stty ask for exotic mode then if the monitor is TFT then 
> it should work. Else it should fail. 

Maybe... though for a TFT, you

1) need more infos than that to set a proper mode
2) don't care bout the refresh rate once you have that proper mode set
since you use the HW scaler

Ben.
 



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> Excellent news!

Its going to be a little bit before this is complete. I plan for 2.6.X to 
have this functionality.

> I just would suggest as userspace fbdev consumer that is a pain to
> do development using two different interfaces, if sysfs interface
> will obsolete the ioctl one, it must have all the mechanics that
> are present in the ioctl one so no mix of interfaces would be
> required.

Yeah that is going to be a problem for a while. 



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[James Simmons]

> > > On your monitor... then most TFT owners will curse you :)
> > 
> > As we move from ioctls to sysfs we will also move from using 
> > fb_var_screeninfo as the userland interface to using mode strings.
> > That means when someone creates a string  like
> > 
> > 834x721@81 
> > 
> > then for non TFT monitors it should fail. For TFT monitors it will work.
> > If someoone using stty ask for exotic mode then if the monitor is TFT then 
> > it should work. Else it should fail. 
> 
> Maybe... though for a TFT, you
> 
> 1) need more infos than that to set a proper mode

What do I need? I like to see this idea work.

> 2) don't care bout the refresh rate once you have that proper mode set
> since you use the HW scaler

Okay. You don't have to pass in a mode string with a refresh rate anyways.
If it's not a tft then we can pick one basded on user preference.





-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Re: [PATCH] radeonfb(): memmove() fix -- this one works ;-)

[Benjamin Herrenschmidt]

> r mode
> 
> What do I need? I like to see this idea work.

Well... If you have the full EDID block etc..., then you have all you
need there . 

> > 2) don't care bout the refresh rate once you have that proper mode set
> > since you use the HW scaler
> 
> Okay. You don't have to pass in a mode string with a refresh rate anyways.
> If it's not a tft then we can pick one basded on user preference.
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>



-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click