Re: scroll modes

[Knut Petersen <Knut_Petersen@t-online.de>]

Hi Tony

first of all, in updatescrollmode we introduce a the local fh, but lateron
vc->vc_font.height is often read again. Fixing that would be a cosmetic 
change, as
the compiler should optimize that anyway.

>diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
>index 1a8f0ea..5ebe68f 100644
>--- a/drivers/video/console/fbcon.c
>+++ b/drivers/video/console/fbcon.c
>@@ -1972,12 +1972,6 @@ static __inline__ void updatescrollmode(
> 	int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) &&
> 		!(cap & FBINFO_HWACCEL_DISABLED);
> 
>-	p->vrows = vyres/fh;
>-	if (yres > (fh * (vc->vc_rows + 1)))
>-		p->vrows -= (yres - (fh * vc->vc_rows)) / fh;
>-	if ((yres % fh) && (vyres % fh < yres % fh))
>-		p->vrows--;
>-
> 	if (good_wrap || good_pan) {
> 		if (reading_fast || fast_copyarea)
> 			p->scrollmode = good_wrap ?
>@@ -1991,6 +1985,16 @@ static __inline__ void updatescrollmode(
> 		else
> 			p->scrollmode = SCROLL_REDRAW;
> 	}
>+
>+	p->vrows = vyres/fh;
>  
>
ok, does not change anything.

>+
>+	if (p->scrollmode != SCROLL_WRAP_MOVE &&
>+	    (yres > (fh * (vc->vc_rows + 1))))
>+		    p->vrows -= (yres - (fh * vc->vc_rows)) / fh;
>+
>  
>
1024x768, vyres 2048, seems to  work with and without that extra
"p->scrollmode != SCROLL_WRAP_MOVE", but could you please
explain when  "yres > (fh * (vc->vc_rows + 1))" will be true?

>+	if ((yres % fh) && (vyres % fh < yres % fh))
>+		p->vrows--;
>  
>
ok, does not change anything.

Further comments to updatescrollmode:

      int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) &&
              divides(ywrap, vc->vc_font.height) &&
              divides(vc->vc_font.height, vyres);

is broken as it does not check that vyres * vxres == whole available video
memory. Or is there hardware that has an adjustable wrap address? Hmm, I
even can think of a way to reprogram the cyberblade/i1 to wrap at the 1, 2,
4 or 8MB boundaries while the system is running.

Even if that additional check would be introduced it would have problems
- with drivers like vesafb that do not map all available memory
- with drivers that do not know about all available memory
- with drivers that allow users to specify arbitraty values for the 
video memory size.

I don´t like the whole concept of updatescrollmode based on the evaluation
of various flags, asumptions and values.

Scrolling with YWRAP_MOVE is faster than scrolling with YPAN_MOVE,
often much faster. Execution time of  a "cat testfile" drops for mode 
1024x768-32
from 1.225s to 0.835s with cyblafb.

But I don´t like that the bottom status line of my curses based editor 
jumps while
scrolling up/down. So I have several options:

  - Don´t use ywrap scrolling
  - Ignore the anoying flicker of the base line
  - change the working YWRAP_MOVE
  - submit an alternative YWRAP_SOMETHING

But how should that YWRAP_SOMETHING be selected by updatescrollmode?
The individual driver does know best about the hardware about user 
preferences
and of the supported scroll modes in any situation.

========================================================
Wouldn´t it be easier and cleaner to introduce a new optional fb_ops 
function
fb_scrollmode(...) that would, if implemented in the driver, replace and 
extend
the functionality  implemented in updatescrollmode?!
========================================================

cu,
 Knut



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click