[U-Boot] [PATCH 05/14] cfb_console: Fix function console_scrollup

[Anatolij Gustschin]

Hi Marek,

On Wed, 21 Mar 2012 11:20:38 +0100
Marek Vasut <marex at denx.de> wrote:

> Dear Anatolij Gustschin,
> 
> > Hi,
> > 
> > On Tue, 24 Jan 2012 15:28:02 +0100
> > 
> > Pali Rohár <pali.rohar at gmail.com> wrote:
> > >  * Use correct buffer size, do not damage screen output
> > > 
> > > Signed-off-by: Pali Rohár <pali.rohar at gmail.com>
> > > ---
> > > 
> > > Changes since original version:
> > >    - Fixed commit message
> > >  
> > >  drivers/video/cfb_console.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
> > > index 904caf7..9092399 100644
> > > --- a/drivers/video/cfb_console.c
> > > +++ b/drivers/video/cfb_console.c
> > > @@ -701,7 +701,7 @@ static void console_scrollup(void)
> > > 
> > >  		);
> > >  
> > >  #else
> > >  
> > >  	memcpyl(CONSOLE_ROW_FIRST, CONSOLE_ROW_SECOND,
> > > 
> > > -		CONSOLE_SCROLL_SIZE >> 2);
> > > +		CONSOLE_SCROLL_SIZE);
> > 
> > NAK. This change is wrong. CONSOLE_SCROLL_SIZE is the size of the
> > visible frame buffer - size of one row in bytes. We are using memcpyl()
> > here, so the division by 4 (size >> 2) is correct. With your change
> > we end up copying 4 times more data then needed.
> 
> What kind of a problem are we fixing here? And what are the symptoms of it?

Actually I'm not aware of any problem in current console_scrollup().
At least on two test setups with different framebuffer drivers and
in one setup with HW accelerated scrolling I didn't see any problems
with current code.

The description in the commit log of this patch:
"Use correct buffer size, do not damage screen output"
doesn't say much about the problem. If the GraphicDevice structure
returned by video_hw_init() is setup correctly, the scrolling should be
working fine. From the other patch [1] I can see that the structure
is setup as follows:

        /* fill in Graphic Device */
        gdev.frameAdrs = 0x8f9c0000;
        gdev.winSizeX = 800;
        gdev.winSizeY = 480;
        gdev.gdfBytesPP = 2;
        gdev.gdfIndex = GDF_16BIT_565RGB;
        memset((void *)gdev.frameAdrs, 0, 0xbb800);
        return (void *) &gdev;

Most likely using 0x8f9c0000 as framebuffer address is the first _big_
problem. The framebuffer address range is not allocated properly and
could be used by malloc area. The board has 256 MB of RAM starting at
0x80000000, if U-Boot relocated itself to the upper RAM, the problems
should be expected.

AFAIK the N900 port doesn't use a framebuffer driver but probably uses
pre-initialized display controller configuration. This should be fixed
first.

Thanks,
Anatolij