[U-Boot] Add video support for mx51evk

[U-Boot] Add video support for mx51evk

[Frias Renato-B13784]

Hello Stefano,

This is my first post to the list, I work as Field Engineer for
Freescale, 
based in Brazil.

I'll be submitting couple patches to add support for the wvga lcd on the

mx51evk board, these are based on the work you did for vision2 board.

However, in order to add support for the second display inteface on the
imx51,
I had to change the Framebuffer driver. Thus, support for vision2 will
break,
to restore it all is needed is to add couple arguments to mx51_fb_init
call. 

Please review the patches and let me know if I should have done things
in a
different way.

There is also a second issue where I would like to know your thoughts.
Very 
early on system initialization, when LCD is enabled, there is a call to 
"lcd_setmem" from board.c. By that time, the video variables,
"panel_info", are
not set yet. Thus U-Boot doesn't reserve the appropriate amount of
memory for 
the display. I was going to set "panel_info" variable on mx51evk.c, but
I would like to know how you solved it for vision2 first.  

Thanks!
Renato Frias

[U-Boot] Add video support for mx51evk

[Stefano Babic]

On 11/02/2010 06:29 AM, Frias Renato-B13784 wrote:
> Hello Stefano,
> 

Hi Ricardo,

> 
> However, in order to add support for the second display inteface on the
> imx51,
> I had to change the Framebuffer driver. Thus, support for vision2 will
> break,
> to restore it all is needed is to add couple arguments to mx51_fb_init call.

I see, you add the display interface to the parameter list of the
probe() function.

However, it is really better to make the modification for the vision2
inside the same patchset. This guarantees that both boards work when
your patches go to mainline.

> 
> Please review the patches and let me know if I should have done things in a
> different way.

ok

> 
> There is also a second issue where I would like to know your thoughts. Very
> early on system initialization, when LCD is enabled, there is a call to
> "lcd_setmem" from board.c. By that time, the video variables,
> "panel_info", are
> not set yet. Thus U-Boot doesn't reserve the appropriate amount of
> memory for
> the display. I was going to set "panel_info" variable on mx51evk.c, but
> I would like to know how you solved it for vision2 first. 

Thanks for pointing out, I have already seen the point, but at the end I
forget to fix it ;-). Probably because there is no side-effects on this
board, but this does not mean that the issue should not fixed for
vision2, too.

The problem arises from the fact that the setup of the display
parameters was static and everything was solved at compile time. With
the framebuffer for the MX51, I needed the possibility to change
dinamically the parameters, because the board can have different LCD
displays.

Consider this, I do not think the actual computation in lcd_setmem() is
correct. We need to compute the maximum amount of memory to be reserved
to the framebuffer, not the value requested by the current display
interface. We could add a CONFIG_SYS_VIDEO_SIZE that contains the
maximum amount of memory needed, because the value is strictly
board-dependent, and change lcd_setmem to use it. Anatolij, what do you
think about this ?

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] Add video support for mx51evk

[Wolfgang Denk]

Dear Stefano Babic,

In message <4CCFB244.2000909@denx.de> you wrote:
>
> Consider this, I do not think the actual computation in lcd_setmem() is
> correct. We need to compute the maximum amount of memory to be reserved
> to the framebuffer, not the value requested by the current display
> interface. We could add a CONFIG_SYS_VIDEO_SIZE that contains the
> maximum amount of memory needed, because the value is strictly
> board-dependent, and change lcd_setmem to use it. Anatolij, what do you
> think about this ?

Why cannot you determine the exact amount needed at runtime?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
GUIs  are  virtually  useless.  Learn  tools.  They're  configurable,
scriptable, automatable, cron-able, interoperable, etc. We don't need
no brain-dead winslurping monolithic claptrap.
                               -- Tom Christiansen in 371140df at csnews

[U-Boot] Add video support for mx51evk

[Stefano Babic]

On 11/02/2010 09:35 AM, Wolfgang Denk wrote:
> Dear Stefano Babic,
> 
> In message <4CCFB244.2000909@denx.de> you wrote:
>>
>> Consider this, I do not think the actual computation in lcd_setmem() is
>> correct. We need to compute the maximum amount of memory to be reserved
>> to the framebuffer, not the value requested by the current display
>> interface. We could add a CONFIG_SYS_VIDEO_SIZE that contains the
>> maximum amount of memory needed, because the value is strictly
>> board-dependent, and change lcd_setmem to use it. Anatolij, what do you
>> think about this ?
> 
> Why cannot you determine the exact amount needed at runtime?

Agree, we can do it, and it is better - I do not think we need really to
change dinamically (I mean, in the same u-boot session) the LCD
connected to the framebuffer, reason that requires to reserve the
maximum amount of memory.

We could introduce a weak function, that a board maintainer can decide
to implement or not. This maintains the compatibility with the most
drivers where vpanel is static initialized.

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] Add video support for mx51evk

[Anatolij Gustschin]

Hi,

On Tue, 02 Nov 2010 07:40:04 +0100
Stefano Babic <sbabic@denx.de> wrote:
...
> > There is also a second issue where I would like to know your thoughts. Very
> > early on system initialization, when LCD is enabled, there is a call to
> > "lcd_setmem" from board.c. By that time, the video variables,
> > "panel_info", are
> > not set yet. Thus U-Boot doesn't reserve the appropriate amount of
> > memory for
> > the display. I was going to set "panel_info" variable on mx51evk.c, but
> > I would like to know how you solved it for vision2 first. 
> 
> Thanks for pointing out, I have already seen the point, but at the end I
> forget to fix it ;-). Probably because there is no side-effects on this
> board, but this does not mean that the issue should not fixed for
> vision2, too.
> 
> The problem arises from the fact that the setup of the display
> parameters was static and everything was solved at compile time. With
> the framebuffer for the MX51, I needed the possibility to change
> dinamically the parameters, because the board can have different LCD
> displays.
> 
> Consider this, I do not think the actual computation in lcd_setmem() is
> correct. We need to compute the maximum amount of memory to be reserved
> to the framebuffer, not the value requested by the current display
> interface. We could add a CONFIG_SYS_VIDEO_SIZE that contains the
> maximum amount of memory needed, because the value is strictly
> board-dependent, and change lcd_setmem to use it. Anatolij, what do you
> think about this ?

We should reserve the amount of memory we actually need for the
used display configuration, I think. Reserving the maximum amount
would be needed if we have to support switching the display
resolution at runtime, but I don't think that this is needed in
your case. Therefore I prefer the solution that reserves the
actually needed amount of memory.

Best regards,
Anatolij

[U-Boot] Add video support for mx51evk

[Wolfgang Denk]

Dear Stefano Babic,

In message <4CCFD2FB.6060000@denx.de> you wrote:
>
> > Why cannot you determine the exact amount needed at runtime?
> 
> Agree, we can do it, and it is better - I do not think we need really to
> change dinamically (I mean, in the same u-boot session) the LCD
> connected to the framebuffer, reason that requires to reserve the
> maximum amount of memory.

Right. If we keep in mind the possibility to share the frame buffer
with Linux (to allow for a flicker-free display of a splash screen
loaded very early in U-Boot) we should stick with the memory map we
have: pRAM on top, frame buffer below, U-Boot code below, etc.  That
means that size changes for the frame buffer (like for pRAM) take only
effect after a reset.

> We could introduce a weak function, that a board maintainer can decide
> to implement or not. This maintains the compatibility with the most
> drivers where vpanel is static initialized.

In which way is this board dependent?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Philosophy:  A route of many roads leading from nowhere to nothing.
- Ambrose Bierce

[U-Boot] Add video support for mx51evk

[Stefano Babic]

On 11/02/2010 10:42 AM, Wolfgang Denk wrote:
> Dear Stefano Babic,
> 

Hi Wolfgang,

>> We could introduce a weak function, that a board maintainer can decide
>> to implement or not. This maintains the compatibility with the most
>> drivers where vpanel is static initialized.
> 
> In which way is this board dependent?

Because each board uses a different LCD with a different resolution, and
this requires a different amount of memory. This is not different as we
see in Linux, because the framebuffer properties are set in the board
file before to be passed to the framebuffer driver.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] Add video support for mx51evk

[Wolfgang Denk]

Dear Stefano Babic,

In message <4CCFDEBA.2070203@denx.de> you wrote:
>
> >> We could introduce a weak function, that a board maintainer can decide
> >> to implement or not. This maintains the compatibility with the most
> >> drivers where vpanel is static initialized.
> > 
> > In which way is this board dependent?
> 
> Because each board uses a different LCD with a different resolution, and
> this requires a different amount of memory. This is not different as we
> see in Linux, because the framebuffer properties are set in the board
> file before to be passed to the framebuffer driver.

Well, if _that_ was what we are using, the memory requirements were
fixed, and we could use compile-time constants.

But what you discussed before was the option to _switch_ resolutions,
so there has to be a way to select the cosen resolution (in an
environment variable?), and then this setting is used for the memory
calculations.

And then the calculation depends only on the current setting - which
is _not_ board dependent.

Boards would start with a (board dependent) default setting, though.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Steal five dollars and you were a petty  thief.  Steal  thousands  of
dollars and you are either a government or a hero.
                                   - Terry Pratchett, _Going_Postal_

[U-Boot] Add video support for mx51evk

[Stefano Babic]

On 11/02/2010 02:34 PM, Wolfgang Denk wrote:

>> Because each board uses a different LCD with a different resolution, and
>> this requires a different amount of memory. This is not different as we
>> see in Linux, because the framebuffer properties are set in the board
>> file before to be passed to the framebuffer driver.
> 
> Well, if _that_ was what we are using, the memory requirements were
> fixed, and we could use compile-time constants.

I was not complete: a single board can be connected to several LCDs, of
course not at the same type. The decision which LCD must be used should
be taken baing on an environment variable.

> 
> But what you discussed before was the option to _switch_ resolutions,
> so there has to be a way to select the cosen resolution (in an
> environment variable?),

Yes, this is what I meant....

> and then this setting is used for the memory
> calculations.

Correct.

> 
> And then the calculation depends only on the current setting - which
> is _not_ board dependent.

Yes, calculation is not board dependent and must remain in lcd_setmem().
What I meant as "board dependent" is really the LCD settings, in terms
of display resolution and bits x pixel. The proposed wek function can
set the panel_info structure, that lcd_setmem() uses for computation.

> 
> Boards would start with a (board dependent) default setting, though.

Yes. What we need is a way to get the board default setting to allow
lcd_setmem() to compute the required memory.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] Add video support for mx51evk

[Wolfgang Denk]

Dear Stefano Babic,

In message <4CD035F7.9070609@denx.de> you wrote:
> 
> > And then the calculation depends only on the current setting - which
> > is _not_ board dependent.
> 
> Yes, calculation is not board dependent and must remain in lcd_setmem().
> What I meant as "board dependent" is really the LCD settings, in terms
> of display resolution and bits x pixel. The proposed wek function can
> set the panel_info structure, that lcd_setmem() uses for computation.

Why do we need a board dependent function for that? We can adjust the
setings by storing their value in an environment variable, say some-
thing like 800x600x16 at 60Hz or copying what Linux does with the "video-
mode=" boot argument (we already have drivers/video/videomodes.c).
That should be precise enough, and is completely bosrd-independent.


> > Boards would start with a (board dependent) default setting, though.
> 
> Yes. What we need is a way to get the board default setting to allow
> lcd_setmem() to compute the required memory.

Let's re-use existing interfaces (especially if we then can pass the
very same variable as boot argument to Linux).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
CONSUMER NOTICE:  Because  of  the  "Uncertainty  Principle,"  It  Is
Impossible  for  the  Consumer  to  Find  Out  at  the Same Time Both
Precisely Where This Product Is and How Fast It Is Moving.

[U-Boot] Add video support for mx51evk

[Renato Frias]

Hello Stefano Babic,

On Tue, Nov 2, 2010 at 4:40 AM, Stefano Babic <sbabic@denx.de> wrote:
>
> However, it is really better to make the modification for the vision2
> inside the same patchset. This guarantees that both boards work when
> your patches go to mainline.

Ok! Should I send the patch for vision2, also?

> Thanks for pointing out, I have already seen the point, but at the end I
> forget to fix it ;-). Probably because there is no side-effects on this
> board, but this does not mean that the issue should not fixed for
> vision2, too.

This issue affects the video mx51evk, at least for the patch I sent.
If I don't edit lcd.c the board resets when writing to the
framebuffer, upon boot.

Best Regards,
Renato

[U-Boot] Add video support for mx51evk

[Frias Renato-B13784]

Hello Stefano,

This is my first post to the list, I work as Field Engineer for
Freescale, 
based in Brazil.

I'll be submitting couple patches to add support for the wvga lcd on the

mx51evk board, these are based on the work you did for vision2 board.

However, in order to add support for the second display inteface on the
imx51,
I had to change the Framebuffer driver. Thus, support for vision2 will
break,
to restore it all is needed is to add couple arguments to mx51_fb_init
call. 

Please review the patches and let me know if I should have done things
in a
different way.

There is also a second issue where I would like to know your thoughts.
Very 
early on system initialization, when LCD is enabled, there is a call to 
"lcd_setmem" from board.c. By that time, the video variables,
"panel_info", are
not set yet. Thus U-Boot doesn't reserve the appropriate amount of
memory for 
the display. I was going to set "panel_info" variable on mx51evk.c, but
I would like to know how you solved it for vision2 first.  

Thanks!
Renato Frias