Patchwork [U-Boot,2/5] lcd: add option for board specific splash screen preparation

login
register
mail settings
Submitter Nikita Kiryanov
Date Dec. 23, 2012, 7:03 a.m.
Message ID <1356246228-26732-3-git-send-email-nikita@compulab.co.il>
Download mbox | patch
Permalink /patch/207948/
State Superseded
Delegated to: Tom Rini
Headers show

Comments

Nikita Kiryanov - Dec. 23, 2012, 7:03 a.m.
Currently there is no logical place to put the code that prepares the
splash image data. The splash image data should be ready in memory
before bmp_display() is called, and after the environment is ready
(since lcd.c looks for the splash image in an address specified by
the environment variable "splashimage").

Our window of opportunity in board_init_r() is therefore: between
env_relocate() and bmp_display(), and from the available options
only the lcd related functions in drv_lcd_init() seem appropriate
for such lcd oriented code.

Add the option to prepare the splash image data in lcd_logo() right
before it is sent to be displayed.

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 README        |    8 ++++++++
 common/lcd.c  |   15 +++++++++++++++
 include/lcd.h |    1 +
 3 files changed, 24 insertions(+)
Jeroen Hofstee - Jan. 20, 2013, 8:34 p.m.
On 12/23/2012 08:03 AM, Nikita Kiryanov wrote:
> Currently there is no logical place to put the code that prepares the
> splash image data. The splash image data should be ready in memory
> before bmp_display() is called, and after the environment is ready
> (since lcd.c looks for the splash image in an address specified by
> the environment variable "splashimage").
>
> Our window of opportunity in board_init_r() is therefore: between
> env_relocate() and bmp_display(), and from the available options
> only the lcd related functions in drv_lcd_init() seem appropriate
> for such lcd oriented code.
>
> Add the option to prepare the splash image data in lcd_logo() right
> before it is sent to be displayed.
>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
>   README        |    8 ++++++++
>   common/lcd.c  |   15 +++++++++++++++
>   include/lcd.h |    1 +
>   3 files changed, 24 insertions(+)
>
> diff --git a/README b/README
> index 78f40df..cffc016 100644
> --- a/README
> +++ b/README
> @@ -1541,6 +1541,14 @@ CBFS (Coreboot Filesystem) support
>   			=> vertically centered image
>   			   at x = dspWidth - bmpWidth - 9
>   
> +		CONFIG_SPLASH_SCREEN_PREPARE
> +
> +		If this option is set then the board_splash_screen_prepare()
> +		function, which must be defined in your code, is called as part
> +		of the splash screen display sequence. It gives the board an
> +		opportunity to prepare the splash image data before it is
> +		processed and sent to the frame buffer by U-Boot.
> +
>   - Gzip compressed BMP image support: CONFIG_VIDEO_BMP_GZIP
>   
>   		If this option is set, additionally to standard BMP
> diff --git a/common/lcd.c b/common/lcd.c
> index 66d4f94..129cf7e 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -1034,6 +1034,18 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
>   }
>   #endif
>   
> +#ifdef CONFIG_SPLASH_SCREEN_PREPARE
> +static inline int splash_screen_prepare(void)
> +{
> +	return board_splash_screen_prepare();
> +}
> +#else
> +static inline int splash_screen_prepare(void)
> +{
> +	return 0;
> +}
> +#endif
> +
>   static void *lcd_logo(void)
>   {
>   #ifdef CONFIG_SPLASH_SCREEN
> @@ -1045,6 +1057,9 @@ static void *lcd_logo(void)
>   		int x = 0, y = 0;
>   		do_splash = 0;
>   
> +		if (splash_screen_prepare())
> +			return (void *)lcd_base;
> +
>   		addr = simple_strtoul (s, NULL, 16);
>   #ifdef CONFIG_SPLASH_SCREEN_ALIGN
>   		s = getenv("splashpos");
> diff --git a/include/lcd.h b/include/lcd.h
> index c24164a..4ac4ddd 100644
> --- a/include/lcd.h
> +++ b/include/lcd.h
> @@ -47,6 +47,7 @@ extern struct vidinfo panel_info;
>   
>   extern void lcd_ctrl_init (void *lcdbase);
>   extern void lcd_enable (void);
> +extern int board_splash_screen_prepare(void);
>   
>   /* setcolreg used in 8bpp/16bpp; initcolregs used in monochrome */
>   extern void lcd_setcolreg (ushort regno,
Other boards seem to do this in lcd_enable. Perhaps that is also an option.

Regards,
Jeroen
Nikita Kiryanov - Jan. 21, 2013, 7:51 a.m.
Hi Jeroen,

On 01/20/2013 10:34 PM, Jeroen Hofstee wrote:
[...]
>> diff --git a/include/lcd.h b/include/lcd.h
>> index c24164a..4ac4ddd 100644
>> --- a/include/lcd.h
>> +++ b/include/lcd.h
>> @@ -47,6 +47,7 @@ extern struct vidinfo panel_info;
>>   extern void lcd_ctrl_init (void *lcdbase);
>>   extern void lcd_enable (void);
>> +extern int board_splash_screen_prepare(void);
>>   /* setcolreg used in 8bpp/16bpp; initcolregs used in monochrome */
>>   extern void lcd_setcolreg (ushort regno,
> Other boards seem to do this in lcd_enable. Perhaps that is also an option.

The problem with doing it in lcd_enable is that it's akin to a side
effect, given what the function's name advertises. Preparing the splash
image can, however, be a natural part of the process that displays it.

>
> Regards,
> Jeroen
Jeroen Hofstee - Jan. 21, 2013, 7:14 p.m.
Hello Nikita,

On 01/21/2013 08:51 AM, Nikita Kiryanov wrote:
> Hi Jeroen,
>
> On 01/20/2013 10:34 PM, Jeroen Hofstee wrote:
> [...]
>>> diff --git a/include/lcd.h b/include/lcd.h
>>> index c24164a..4ac4ddd 100644
>>> --- a/include/lcd.h
>>> +++ b/include/lcd.h
>>> @@ -47,6 +47,7 @@ extern struct vidinfo panel_info;
>>>   extern void lcd_ctrl_init (void *lcdbase);
>>>   extern void lcd_enable (void);
>>> +extern int board_splash_screen_prepare(void);
>>>   /* setcolreg used in 8bpp/16bpp; initcolregs used in monochrome */
>>>   extern void lcd_setcolreg (ushort regno,
>> Other boards seem to do this in lcd_enable. Perhaps that is also an 
>> option.
>
> The problem with doing it in lcd_enable is that it's akin to a side
> effect, given what the function's name advertises. Preparing the splash
> image can, however, be a natural part of the process that displays it.
>
mmm, I am not so sure I agree that loading a bitmap in lcd_enable is
a _problem_, while loading it in show logo and requiring a CONFIG_* is
_natural_.

But anyway, can't this at least be changed to a __weak function, so the
CONFIG and ifdef stuff can be dropped?

Regards,
Jeroen
Nikita Kiryanov - Jan. 23, 2013, 8:31 a.m.
On 01/21/2013 09:14 PM, Jeroen Hofstee wrote:
> Hello Nikita,
>
> On 01/21/2013 08:51 AM, Nikita Kiryanov wrote:
>> Hi Jeroen,
>>
>> On 01/20/2013 10:34 PM, Jeroen Hofstee wrote:
>> [...]
>>>> diff --git a/include/lcd.h b/include/lcd.h
>>>> index c24164a..4ac4ddd 100644
>>>> --- a/include/lcd.h
>>>> +++ b/include/lcd.h
>>>> @@ -47,6 +47,7 @@ extern struct vidinfo panel_info;
>>>>   extern void lcd_ctrl_init (void *lcdbase);
>>>>   extern void lcd_enable (void);
>>>> +extern int board_splash_screen_prepare(void);
>>>>   /* setcolreg used in 8bpp/16bpp; initcolregs used in monochrome */
>>>>   extern void lcd_setcolreg (ushort regno,
>>> Other boards seem to do this in lcd_enable. Perhaps that is also an
>>> option.
>>
>> The problem with doing it in lcd_enable is that it's akin to a side
>> effect, given what the function's name advertises. Preparing the splash
>> image can, however, be a natural part of the process that displays it.
>>
> mmm, I am not so sure I agree that loading a bitmap in lcd_enable is
> a _problem_, while loading it in show logo and requiring a CONFIG_* is
> _natural_.

Well, it is a problem. If we don't respect the abstractions we create
then things like function names become meaningless. A function called
"lcd_enable" should do just that- enable lcd. Not load stuff from
storage to memory or manipulate BMPs.

>
> But anyway, can't this at least be changed to a __weak function, so the
> CONFIG and ifdef stuff can be dropped?

The motivation behind the CONFIG was to make it a documentable user 
setting, rather than an undocumented feature buried in the code.

>
> Regards,
> Jeroen
>
>
Jeroen Hofstee - Jan. 23, 2013, 10:13 p.m.
Hello Nikita,

On 01/23/2013 09:31 AM, Nikita Kiryanov wrote:
> On 01/21/2013 09:14 PM, Jeroen Hofstee wrote:
>> Hello Nikita,
>>
>> On 01/21/2013 08:51 AM, Nikita Kiryanov wrote:
>>> Hi Jeroen,
>>>
>>> On 01/20/2013 10:34 PM, Jeroen Hofstee wrote:
>>> [...]
>>>>> diff --git a/include/lcd.h b/include/lcd.h
>>>>> index c24164a..4ac4ddd 100644
>>>>> --- a/include/lcd.h
>>>>> +++ b/include/lcd.h
>>>>> @@ -47,6 +47,7 @@ extern struct vidinfo panel_info;
>>>>>   extern void lcd_ctrl_init (void *lcdbase);
>>>>>   extern void lcd_enable (void);
>>>>> +extern int board_splash_screen_prepare(void);
>>>>>   /* setcolreg used in 8bpp/16bpp; initcolregs used in monochrome */
>>>>>   extern void lcd_setcolreg (ushort regno,
>>>> Other boards seem to do this in lcd_enable. Perhaps that is also an
>>>> option.
>>>
>>> The problem with doing it in lcd_enable is that it's akin to a side
>>> effect, given what the function's name advertises. Preparing the splash
>>> image can, however, be a natural part of the process that displays it.
>>>
>> mmm, I am not so sure I agree that loading a bitmap in lcd_enable is
>> a _problem_, while loading it in show logo and requiring a CONFIG_* is
>> _natural_.
>
> Well, it is a problem. If we don't respect the abstractions we create
> then things like function names become meaningless. A function called
> "lcd_enable" should do just that- enable lcd. Not load stuff from
> storage to memory or manipulate BMPs.
>
my point is that lcd_clear will e.g. call lcd_logo. Although I haven't 
tested it,
it seems you're make a side effect of a function only called once a side 
effect
of another function (which could be called multiple times). So you make 
things
even worse (loading an bitmap while the function is just named to 
display it).

>>
>> But anyway, can't this at least be changed to a __weak function, so the
>> CONFIG and ifdef stuff can be dropped?
>
> The motivation behind the CONFIG was to make it a documentable user 
> setting,
> rather than an undocumented feature buried in the code.
>
then document the callback...

I don't see the improvement of this patch..

Regards,
Jeroen
Igor Grinberg - Jan. 24, 2013, 8:35 a.m.
On 01/24/13 00:13, Jeroen Hofstee wrote:
> Hello Nikita,
> 
> On 01/23/2013 09:31 AM, Nikita Kiryanov wrote:
>> On 01/21/2013 09:14 PM, Jeroen Hofstee wrote:
>>> Hello Nikita,
>>>
>>> On 01/21/2013 08:51 AM, Nikita Kiryanov wrote:
>>>> Hi Jeroen,
>>>>
>>>> On 01/20/2013 10:34 PM, Jeroen Hofstee wrote:
>>>> [...]
>>>>>> diff --git a/include/lcd.h b/include/lcd.h
>>>>>> index c24164a..4ac4ddd 100644
>>>>>> --- a/include/lcd.h
>>>>>> +++ b/include/lcd.h
>>>>>> @@ -47,6 +47,7 @@ extern struct vidinfo panel_info;
>>>>>>   extern void lcd_ctrl_init (void *lcdbase);
>>>>>>   extern void lcd_enable (void);
>>>>>> +extern int board_splash_screen_prepare(void);
>>>>>>   /* setcolreg used in 8bpp/16bpp; initcolregs used in monochrome */
>>>>>>   extern void lcd_setcolreg (ushort regno,
>>>>> Other boards seem to do this in lcd_enable. Perhaps that is also an
>>>>> option.
>>>>
>>>> The problem with doing it in lcd_enable is that it's akin to a side
>>>> effect, given what the function's name advertises. Preparing the splash
>>>> image can, however, be a natural part of the process that displays it.
>>>>
>>> mmm, I am not so sure I agree that loading a bitmap in lcd_enable is
>>> a _problem_, while loading it in show logo and requiring a CONFIG_* is
>>> _natural_.
>>
>> Well, it is a problem. If we don't respect the abstractions we create
>> then things like function names become meaningless. A function called
>> "lcd_enable" should do just that- enable lcd. Not load stuff from
>> storage to memory or manipulate BMPs.
>>
> my point is that lcd_clear will e.g. call lcd_logo. Although I haven't tested it,
> it seems you're make a side effect of a function only called once a side effect
> of another function (which could be called multiple times). So you make things
> even worse (loading an bitmap while the function is just named to display it).

So what's your point? Do you think we should add a splash screen specific
callback inside the board.c U-Boot boot flow?
Please, be more specific, as both approaches are not suitable according
to what was said above...

> 
>>>
>>> But anyway, can't this at least be changed to a __weak function, so the
>>> CONFIG and ifdef stuff can be dropped?
>>
>> The motivation behind the CONFIG was to make it a documentable user setting,
>> rather than an undocumented feature buried in the code.
>>
> then document the callback...

Sorry, may be I've missed something, but I can't see any callback being
documented in the README file...

> 
> I don't see the improvement of this patch..

What does that suppose to mean? Either be constructive or don't bother...
I'd like to hear Anatolij's opinion on this.
Jeroen Hofstee - Jan. 24, 2013, 10:34 p.m.
Hello Igor,

On 01/24/2013 09:35 AM, Igor Grinberg wrote:
> On 01/24/13 00:13, Jeroen Hofstee wrote:
>> Hello Nikita,
>>
>> On 01/23/2013 09:31 AM, Nikita Kiryanov wrote:
>>> On 01/21/2013 09:14 PM, Jeroen Hofstee wrote:
>>>>
>>>> mmm, I am not so sure I agree that loading a bitmap in lcd_enable is
>>>> a _problem_, while loading it in show logo and requiring a CONFIG_* is
>>>> _natural_.
>>> Well, it is a problem. If we don't respect the abstractions we create
>>> then things like function names become meaningless. A function called
>>> "lcd_enable" should do just that- enable lcd. Not load stuff from
>>> storage to memory or manipulate BMPs.
>>>
>> my point is that lcd_clear will e.g. call lcd_logo. Although I haven't tested it,
>> it seems you're make a side effect of a function only called once a side effect
>> of another function (which could be called multiple times). So you make things
>> even worse (loading an bitmap while the function is just named to display it).
> So what's your point? Do you think we should add a splash screen specific
> callback inside the board.c U-Boot boot flow?
no.
> Please, be more specific, as both approaches are not suitable according
> to what was said above...

lets see, drv_lcd_init calls lcd_init. which does

lcd_ctrl_init(lcdbase);
lcd_is_enabled = 1;
lcd_clear();
lcd_enable();

After this patch, lcd_clear calls lcd_logo which calls
board_splash_screen_prepare in its turn. In my mind this
still leaves allot of side effects. If you want to prepare
for displaying and not have it as a side effect of a function
which is named to do something else, it should be in
between lcd_ctrl_init and lcd_clear in my mind.

>
>>>> But anyway, can't this at least be changed to a __weak function, so the
>>>> CONFIG and ifdef stuff can be dropped?
>>> The motivation behind the CONFIG was to make it a documentable user setting,
>>> rather than an undocumented feature buried in the code.
>>>
>> then document the callback...
> Sorry, may be I've missed something, but I can't see any callback being
> documented in the README file...
>
>> I don't see the improvement of this patch..
> What does that suppose to mean? Either be constructive or don't bother...
This means, as I hopefully explained a bit more clearly now, that
the patch makes the loading of a bitmap a side effect of lcd_clear,
while the intention was to make it a more natural call sequence.
(which can simply be done by putting it somewhere else as
mentioned above)

btw, I think, loading the image in lcd_enable() won't even work
since lcd_enable is actually before lcd_clear. Scanning some
boards which load in lcd_enable, they seem to call bmp_display
manually. So that makes this patch no longer optional, but is
actually required and is an improvement....
> I'd like to hear Anatolij's opinion on this.
>
yes, me too. I like the __weak far more than requiring a CONFIG_to
enable a callback. I cannot think of a reason why these __weak
functions cannot be documented. So that's up to Anatolij.

Regards,
Jeroen
Igor Grinberg - Jan. 25, 2013, 6:45 a.m.
On 01/25/13 00:34, Jeroen Hofstee wrote:
> Hello Igor,
> 
> On 01/24/2013 09:35 AM, Igor Grinberg wrote:
>> On 01/24/13 00:13, Jeroen Hofstee wrote:
>>> Hello Nikita,
>>>
>>> On 01/23/2013 09:31 AM, Nikita Kiryanov wrote:
>>>> On 01/21/2013 09:14 PM, Jeroen Hofstee wrote:
>>>>>
>>>>> mmm, I am not so sure I agree that loading a bitmap in lcd_enable is
>>>>> a _problem_, while loading it in show logo and requiring a CONFIG_* is
>>>>> _natural_.
>>>> Well, it is a problem. If we don't respect the abstractions we create
>>>> then things like function names become meaningless. A function called
>>>> "lcd_enable" should do just that- enable lcd. Not load stuff from
>>>> storage to memory or manipulate BMPs.
>>>>
>>> my point is that lcd_clear will e.g. call lcd_logo. Although I haven't tested it,
>>> it seems you're make a side effect of a function only called once a side effect
>>> of another function (which could be called multiple times). So you make things
>>> even worse (loading an bitmap while the function is just named to display it).
>> So what's your point? Do you think we should add a splash screen specific
>> callback inside the board.c U-Boot boot flow?
> no.
>> Please, be more specific, as both approaches are not suitable according
>> to what was said above...
> 
> lets see, drv_lcd_init calls lcd_init. which does
> 
> lcd_ctrl_init(lcdbase);
> lcd_is_enabled = 1;
> lcd_clear();
> lcd_enable();
> 
> After this patch, lcd_clear calls lcd_logo which calls
> board_splash_screen_prepare in its turn.

That said, lcd_clear() calls lcd_logo()...
This is the real source of confusion here - the side effect,
and not the fact that lcd_logo() calls the board_splash_screen_prepare()...
Being that a problem not directly related to Nikita's patch set, I don't
think we should delay it any further.

> In my mind this
> still leaves allot of side effects. If you want to prepare
> for displaying and not have it as a side effect of a function
> which is named to do something else, it should be in
> between lcd_ctrl_init and lcd_clear in my mind.

I think not, lcd_logo() fits perfectly for loading the splash screen.
The fact that lcd_logo() is called from lcd_clear(), IMO,
would be the one that should be dealt with if you want to remove the
confusion ("the side effect"). But that is not related to Nikita's
patch set.

> 
>>
>>>>> But anyway, can't this at least be changed to a __weak function, so the
>>>>> CONFIG and ifdef stuff can be dropped?
>>>> The motivation behind the CONFIG was to make it a documentable user setting,
>>>> rather than an undocumented feature buried in the code.
>>>>
>>> then document the callback...
>> Sorry, may be I've missed something, but I can't see any callback being
>> documented in the README file...
>>
>>> I don't see the improvement of this patch..
>> What does that suppose to mean? Either be constructive or don't bother...
> This means, as I hopefully explained a bit more clearly now, that
> the patch makes the loading of a bitmap a side effect of lcd_clear,
> while the intention was to make it a more natural call sequence.
> (which can simply be done by putting it somewhere else as
> mentioned above)

As explained above, the patch only uses lcd_logo(), which is meant to
display the splash screen, and add the loading functionality.
The fact that the lcd_logo() is called from lcd_clear() is the one
you should blame. And as already said, not related to this patch.

> 
> btw, I think, loading the image in lcd_enable() won't even work
> since lcd_enable is actually before lcd_clear. Scanning some
> boards which load in lcd_enable, they seem to call bmp_display
> manually. So that makes this patch no longer optional, but is
> actually required and is an improvement....

Ok. So we agree that this can't be done in lcd_enable().

>> I'd like to hear Anatolij's opinion on this.
>>
> yes, me too. I like the __weak far more than requiring a CONFIG_to
> enable a callback. I cannot think of a reason why these __weak
> functions cannot be documented. So that's up to Anatolij.

Usually, I also like the __weak approach, but this time the intention was
to document the feature and hopefully stop the lcd_*() API abuse.
Jeroen Hofstee - Jan. 26, 2013, 1:33 p.m.
Hello Igor,

On 01/25/2013 07:45 AM, Igor Grinberg wrote:
> On 01/25/13 00:34, Jeroen Hofstee wrote:
>> Hello Igor,
>>
>> On 01/24/2013 09:35 AM, Igor Grinberg wrote:
>>> On 01/24/13 00:13, Jeroen Hofstee wrote:
>>>> Hello Nikita,
>>>>
>>>> On 01/23/2013 09:31 AM, Nikita Kiryanov wrote:
>>>>> On 01/21/2013 09:14 PM, Jeroen Hofstee wrote:
>>>>>> mmm, I am not so sure I agree that loading a bitmap in lcd_enable is
>>>>>> a _problem_, while loading it in show logo and requiring a CONFIG_* is
>>>>>> _natural_.
>>>>> Well, it is a problem. If we don't respect the abstractions we create
>>>>> then things like function names become meaningless. A function called
>>>>> "lcd_enable" should do just that- enable lcd. Not load stuff from
>>>>> storage to memory or manipulate BMPs.
>>>>>
>>>> my point is that lcd_clear will e.g. call lcd_logo. Although I haven't tested it,
>>>> it seems you're make a side effect of a function only called once a side effect
>>>> of another function (which could be called multiple times). So you make things
>>>> even worse (loading an bitmap while the function is just named to display it).
>>> So what's your point? Do you think we should add a splash screen specific
>>> callback inside the board.c U-Boot boot flow?
>> no.
>>> Please, be more specific, as both approaches are not suitable according
>>> to what was said above...
>> lets see, drv_lcd_init calls lcd_init. which does
>>
>> lcd_ctrl_init(lcdbase);
>> lcd_is_enabled = 1;
>> lcd_clear();
>> lcd_enable();
>>
>> After this patch, lcd_clear calls lcd_logo which calls
>> board_splash_screen_prepare in its turn.
> That said, lcd_clear() calls lcd_logo()...
> This is the real source of confusion here - the side effect,
> and not the fact that lcd_logo() calls the board_splash_screen_prepare()...
> Being that a problem not directly related to Nikita's patch set, I don't
> think we should delay it any further.
>
>> In my mind this
>> still leaves allot of side effects. If you want to prepare
>> for displaying and not have it as a side effect of a function
>> which is named to do something else, it should be in
>> between lcd_ctrl_init and lcd_clear in my mind.
> I think not, lcd_logo() fits perfectly for loading the splash screen.
> The fact that lcd_logo() is called from lcd_clear(), IMO,
> would be the one that should be dealt with if you want to remove the
> confusion ("the side effect"). But that is not related to Nikita's
> patch set.
>

Given that I now know that lcd_enable is not an alternative to this
patch, but adding a callback is the only method to load a bitmap
and have it shown, I understand now that this patch is not just a
formal / cleanup change, but adds an actually missing feature.
So I am fine with having it inside lcd_logo.

>> btw, I think, loading the image in lcd_enable() won't even work
>> since lcd_enable is actually before lcd_clear. Scanning some
>> boards which load in lcd_enable, they seem to call bmp_display
>> manually. So that makes this patch no longer optional, but is
>> actually required and is an improvement....
> Ok. So we agree that this can't be done in lcd_enable().
>
yes.
>>> I'd like to hear Anatolij's opinion on this.
>>>
>> yes, me too. I like the __weak far more than requiring a CONFIG_to
>> enable a callback. I cannot think of a reason why these __weak
>> functions cannot be documented. So that's up to Anatolij.
> Usually, I also like the __weak approach, but this time the intention was
> to document the feature and hopefully stop the lcd_*() API abuse.
>
>
Regards,
Jeroen

Patch

diff --git a/README b/README
index 78f40df..cffc016 100644
--- a/README
+++ b/README
@@ -1541,6 +1541,14 @@  CBFS (Coreboot Filesystem) support
 			=> vertically centered image
 			   at x = dspWidth - bmpWidth - 9
 
+		CONFIG_SPLASH_SCREEN_PREPARE
+
+		If this option is set then the board_splash_screen_prepare()
+		function, which must be defined in your code, is called as part
+		of the splash screen display sequence. It gives the board an
+		opportunity to prepare the splash image data before it is
+		processed and sent to the frame buffer by U-Boot.
+
 - Gzip compressed BMP image support: CONFIG_VIDEO_BMP_GZIP
 
 		If this option is set, additionally to standard BMP
diff --git a/common/lcd.c b/common/lcd.c
index 66d4f94..129cf7e 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -1034,6 +1034,18 @@  int lcd_display_bitmap(ulong bmp_image, int x, int y)
 }
 #endif
 
+#ifdef CONFIG_SPLASH_SCREEN_PREPARE
+static inline int splash_screen_prepare(void)
+{
+	return board_splash_screen_prepare();
+}
+#else
+static inline int splash_screen_prepare(void)
+{
+	return 0;
+}
+#endif
+
 static void *lcd_logo(void)
 {
 #ifdef CONFIG_SPLASH_SCREEN
@@ -1045,6 +1057,9 @@  static void *lcd_logo(void)
 		int x = 0, y = 0;
 		do_splash = 0;
 
+		if (splash_screen_prepare())
+			return (void *)lcd_base;
+
 		addr = simple_strtoul (s, NULL, 16);
 #ifdef CONFIG_SPLASH_SCREEN_ALIGN
 		s = getenv("splashpos");
diff --git a/include/lcd.h b/include/lcd.h
index c24164a..4ac4ddd 100644
--- a/include/lcd.h
+++ b/include/lcd.h
@@ -47,6 +47,7 @@  extern struct vidinfo panel_info;
 
 extern void lcd_ctrl_init (void *lcdbase);
 extern void lcd_enable (void);
+extern int board_splash_screen_prepare(void);
 
 /* setcolreg used in 8bpp/16bpp; initcolregs used in monochrome */
 extern void lcd_setcolreg (ushort regno,