diff mbox

[U-Boot,RFC] : always relocate u-boot before the framebuffer

Message ID 20130102201725.DC80A20035D@gemini.denx.de
State RFC
Delegated to: Anatolij Gustschin
Headers show

Commit Message

Wolfgang Denk Jan. 2, 2013, 8:17 p.m. UTC
Dear Tom,

In message <20130102154854.GC14738@bill-the-cat> you wrote:
> 
> > Please see my responses.  This is definitely NOT a good idea, it will
> > break most (all?) boards that use CONFIG_FB_ADDR in the way it was
> > intended for.
> 
> I think this means we've got people not understanding what the variable
> IS for.  And at the high level, the idea of "lets transition from U-Boot
> to Linux without a flicker" is good.  So, what is the variables we
> should be using for this, today?  Or do we need to add and document
> more?  Or are we all just failing to RTFM here?

I think the key problem is insufficient documentation of what
CONFIG_FB_ADDR is intended for (i. e. graphics controllers with
external video RAM).  Eventualy a patch as attached might help?

The idea of "lets transition from U-Boot to Linux without a flicker"
is as old as PPCBoot (i. e. even predates U-Boot).  The standard
mechanism as implemented should work out of the box, assuming that
both U-Boot and Linux use the same configuration of the graphics
controller (resulting in the same size of the needed frame buffer
memory).  And if they use different configurations, you won't be able
to pass a pre-initialized frame buffer ayway.

The problem Jeroen ran into is/was caused by the fct that U-Boot and
Linux computed different sizes for the frame buffer.  I think this is
either a bug, or a difference in the configuration, but Jeroen did not
reply to my question for the reason for this difference yet.

Best regards,

Wolfgang Denk

Suggested doc patch:

From ede2e170bd6c4b4ab6a3b7752400eb8afe0fdec9 Mon Sep 17 00:00:00 2001
From: Wolfgang Denk <wd@denx.de>
Date: Wed, 2 Jan 2013 21:10:29 +0100
Subject: [PATCH] VIDEO: better document the correct use of CONFIG_FB_ADDR

Signed-off-by: Wolfgang Denk <wd@denx.de>
cc: Anatolij Gustschin <agust@denx.de>
---
 README | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Jeroen Hofstee Jan. 3, 2013, 10:27 a.m. UTC | #1
Hello Tom, Wolfgang,

On 01/02/2013 09:17 PM, Wolfgang Denk wrote:
> Dear Tom,
>
> In message <20130102154854.GC14738@bill-the-cat> you wrote:
>> I think this means we've got people not understanding what the variable
>> IS for.  And at the high level, the idea of "lets transition from U-Boot
>> to Linux without a flicker" is good.  So, what is the variables we
>> should be using for this, today?  Or do we need to add and document
>> more?  Or are we all just failing to RTFM here?
I did read the document. Especially "Then system will reserve the
frame buffer address to defined address instead of lcd_setmem"
is a bit misleading, given that nothing is "reserved" it is just assumed
to be free.
> I think the key problem is insufficient documentation of what
> CONFIG_FB_ADDR is intended for (i. e. graphics controllers with
> external video RAM).  Eventualy a patch as attached might help?
>
> The idea of "lets transition from U-Boot to Linux without a flicker"
> is as old as PPCBoot (i. e. even predates U-Boot).  The standard
> mechanism as implemented should work out of the box, assuming that
> both U-Boot and Linux use the same configuration of the graphics
> controller (resulting in the same size of the needed frame buffer
> memory).  And if they use different configurations, you won't be able
> to pass a pre-initialized frame buffer ayway.
>
> The problem Jeroen ran into is/was caused by the fct that U-Boot and
> Linux computed different sizes for the frame buffer.  I think this is
> either a bug, or a difference in the configuration, but Jeroen did not
> reply to my question for the reason for this difference yet.
>
I encountered this issue on a omap board / dss. Currently the amount
of video ram is passed with a vram= argument to linux and allocated
at the end of the ram. This is 16Mb by default I have CONFIG_FB_ADDR
set to end of RAM minus that. U-boot then relocates _into_ my frame
buffer and all goes well since I have a small lcd and only use the first
500k or so.

So the intention was to fix that while preserving the frame buffer and I
don't want linux to steal so much memory. The minimum amount of
memory the dss accepts is 2MB though (for reason unknown to me).
Without the fb address set U-boot will allocate it at 1MB before the end
of the ram, since it does not have the 2MB minimum. With the fb address
set to -2MB u-boot will allocate itself over it, hence the patch, do 
actually
reserve the frame buffer (well in the most simplistic way, by placing
everything before it).

As Wolfgang suggested, maybe I shouldn't point at U-boot but blame
linux and get rid of the 2MB alignment. I haven't checked yet, but I
don't see why that should not work. So this is resolved for my
case (thanks for the suggestion) for now until I meet

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg80440.html

> diff --git a/README b/README
> index 78f40df..f84108e 100644
> --- a/README
> +++ b/README
> @@ -2695,11 +2695,14 @@ FIT uImage format:
>   - Frame Buffer Address:
>   		CONFIG_FB_ADDR
>   
> -		Define CONFIG_FB_ADDR if you want to use specific
> -		address for frame buffer.
> -		Then system will reserve the frame buffer address to
> -		defined address instead of lcd_setmem (this function
> -		grabs the memory for frame buffer by panel's size).
> +                Define CONFIG_FB_ADDR if you want to use specific
> +                address for frame buffer.  This is typically the case
> +                when using a graphics controller has separate video
> +                memory.  U-Boot will then reserve the frame buffer at
> +                the given address instead of dynamically reserving it
> +                in system RAM by calling lcd_setmem(), which grabs
> +                the memory for the frame buffer depending on the
> +                configured panel size.
>   
>   		Please see board_init_f function.
>   
Maybe it is clearer as "U-boot will then place the frame buffer at ... 
instead of reserving"

Regards,
Jeroen
Wolfgang Denk Jan. 3, 2013, 10:41 a.m. UTC | #2
Dear Jeroen,

In message <50E55D24.8000809@myspectrum.nl> you wrote:
> 
> So the intention was to fix that while preserving the frame buffer and I
> don't want linux to steal so much memory. The minimum amount of
> memory the dss accepts is 2MB though (for reason unknown to me).
> Without the fb address set U-boot will allocate it at 1MB before the end
> of the ram, since it does not have the 2MB minimum. With the fb address
> set to -2MB u-boot will allocate itself over it, hence the patch, do 
> actually
> reserve the frame buffer (well in the most simplistic way, by placing
> everything before it).
> 
> As Wolfgang suggested, maybe I shouldn't point at U-boot but blame
> linux and get rid of the 2MB alignment. I haven't checked yet, but I
> don't see why that should not work. So this is resolved for my
> case (thanks for the suggestion) for now until I meet
> 
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg80440.html

Hm...

In any case the code should behave the same in U-Boot and Linux.  If
we can lift this restriction in Linux, then OK.  If not, then the FB
allocation in U-Boot (by lcd_setmem()) should result in the same size
as allocated by Linux.  After all, the allocated size should be a
"resasonable" one ;-)

> > +                Define CONFIG_FB_ADDR if you want to use specific
> > +                address for frame buffer.  This is typically the case
> > +                when using a graphics controller has separate video
> > +                memory.  U-Boot will then reserve the frame buffer at
> > +                the given address instead of dynamically reserving it
> > +                in system RAM by calling lcd_setmem(), which grabs
> > +                the memory for the frame buffer depending on the
> > +                configured panel size.
> >   
> >   		Please see board_init_f function.
> >   
> Maybe it is clearer as "U-boot will then place the frame buffer at ... 
> instead of reserving"

OK, will repost.

Best regards,

Wolfgang Denk
Jeroen Hofstee Jan. 3, 2013, 6:10 p.m. UTC | #3
Hi,

On 01/03/2013 11:41 AM, Wolfgang Denk wrote:
> In message <50E55D24.8000809@myspectrum.nl> you wrote:
>> As Wolfgang suggested, maybe I shouldn't point at U-boot but blame
>> linux and get rid of the 2MB alignment. I haven't checked yet, but I
>> don't see why that should not work. So this is resolved for my
>> case (thanks for the suggestion) for now until I meet
>>
>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg80440.html
> Hm...
>
> In any case the code should behave the same in U-Boot and Linux.  If
> we can lift this restriction in Linux, then OK.  If not, then the FB
> allocation in U-Boot (by lcd_setmem()) should result in the same size
> as allocated by Linux.  After all, the allocated size should be a
> "resasonable" one ;-)

For completeness, removing size = ALIGN(size, SZ_2M); from vram.c
in the linux dss driver and passing the correct vram= works fine.
The frame buffer is then at the same physical address and I regain
15MB of memory. So solved as far as I am concerned till proven that
it really hurts performance.

Motivation for the 2mb alignment is at:
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-December/035013.html

Regards,
Jeroen
Wolfgang Denk Jan. 3, 2013, 8:28 p.m. UTC | #4
Dear Jeroen Hofstee,

In message <50E5C9A2.7090904@myspectrum.nl> you wrote:
> 
> > In any case the code should behave the same in U-Boot and Linux.  If
> > we can lift this restriction in Linux, then OK.  If not, then the FB
> > allocation in U-Boot (by lcd_setmem()) should result in the same size
> > as allocated by Linux.  After all, the allocated size should be a
> > "resasonable" one ;-)
> 
> For completeness, removing size = ALIGN(size, SZ_2M); from vram.c
> in the linux dss driver and passing the correct vram= works fine.

Hm... I'm not sure if this is the right approach, then.  If mainline
Linux insists on such a 2 MB alignment, we shoud  rather teach
lcd_setmem() to follow tha rule, too.  That should solve the problem
as well (and probably more efficiently, especially if other features
like pRAM or shared log buffer reserve high memory as well).


Anatolij, what do you think should be done here?


> The frame buffer is then at the same physical address and I regain
> 15MB of memory. So solved as far as I am concerned till proven that
> it really hurts performance.

I can't grok this, though.  I could understand if you say you saved
up to 2 MB by lifting the 2 MB alignment requirement - but 15 MB?
Please elucidate where this number is coming from.

Best regards,

Wolfgang Denk
Jeroen Hofstee Jan. 4, 2013, 8:29 p.m. UTC | #5
Hi Wolfgang,

On 01/03/2013 09:28 PM, Wolfgang Denk wrote:
>> The frame buffer is then at the same physical address and I regain
>> 15MB of memory. So solved as far as I am concerned till proven that
>> it really hurts performance.
> I can't grok this, though.  I could understand if you say you saved
> up to 2 MB by lifting the 2 MB alignment requirement - but 15 MB?
> Please elucidate where this number is coming from.
>

I had the frame buffer at 16mb before the end of the ram to allow
u-boot, heap etc to relocated itself into the end of the frame buffer
/ ram. Without the CONFIG_FB_ADDR, the frame buffer it is located
at 1mb before the end and u-boot relocates itself before it. So now
linux has 15mb more since the framebuffer is at the 1mb before the
end of ram and not 16mb as it used to be.

Regards,
Jeroen
Jeroen Hofstee Jan. 5, 2013, 10:07 a.m. UTC | #6
Hi,

On 01/03/2013 09:28 PM, Wolfgang Denk wrote:
> Hm... I'm not sure if this is the right approach, then.  If mainline
> Linux insists on such a 2 MB alignment, we shoud  rather teach
> lcd_setmem() to follow tha rule, too.  That should solve the problem
> as well (and probably more efficiently, especially if other features
> like pRAM or shared log buffer reserve high memory as well).
>
I just noticed this is already possible since the recently introduced
dcache for lcd.c also adds a CONFIG_LCD_ALIGNMENT. Since I am at
rc2 I had not seen it yet.

Regards,
Jeroen
Wolfgang Denk Jan. 5, 2013, 7:18 p.m. UTC | #7
Dear Jeroen,

In message <50E7FB65.9000709@myspectrum.nl> you wrote:
> 
> > Hm... I'm not sure if this is the right approach, then.  If mainline
> > Linux insists on such a 2 MB alignment, we shoud  rather teach
> > lcd_setmem() to follow tha rule, too.  That should solve the problem
> > as well (and probably more efficiently, especially if other features
> > like pRAM or shared log buffer reserve high memory as well).
> >
> I just noticed this is already possible since the recently introduced
> dcache for lcd.c also adds a CONFIG_LCD_ALIGNMENT. Since I am at
> rc2 I had not seen it yet.

Ah, so we have a working solution already in place?  Excellent.

Do you think you have a chance to test if this works for you, too?

Thanks in advance.

Best regards,

Wolfgang Denk
diff mbox

Patch

diff --git a/README b/README
index 78f40df..f84108e 100644
--- a/README
+++ b/README
@@ -2695,11 +2695,14 @@  FIT uImage format:
 - Frame Buffer Address:
 		CONFIG_FB_ADDR
 
-		Define CONFIG_FB_ADDR if you want to use specific
-		address for frame buffer.
-		Then system will reserve the frame buffer address to
-		defined address instead of lcd_setmem (this function
-		grabs the memory for frame buffer by panel's size).
+                Define CONFIG_FB_ADDR if you want to use specific
+                address for frame buffer.  This is typically the case
+                when using a graphics controller has separate video
+                memory.  U-Boot will then reserve the frame buffer at
+                the given address instead of dynamically reserving it
+                in system RAM by calling lcd_setmem(), which grabs
+                the memory for the frame buffer depending on the
+                configured panel size.
 
 		Please see board_init_f function.