[U-Boot] [PATCH v2 11/19] tegra: Add LCD support to Nvidia boards

[Simon Glass]

Hi Stephen,

On Mon, Jul 23, 2012 at 1:25 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 07/10/2012 10:58 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Fri, Jun 15, 2012 at 1:47 AM, Stephen Warren <swarren at wwwdotorg.org
>> <mailto:swarren at wwwdotorg.org>> wrote:
>>
>>     On 06/13/2012 10:19 AM, Simon Glass wrote:
>>     > Add calls to the LCD driver from Nvidia board code.
>>
>>     > diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
>>
>>     > @@ -87,6 +88,9 @@ int board_init(void)
>>
>>     > +#ifdef CONFIG_VIDEO_TEGRA2
>>     > +     tegra_lcd_check_next_stage(gd->blob, 0);
>>     > +#endif
>>
>>     This seems to be conflating video support with LCD support. It would be
>>     quite possible to have a board with no LCD, yet supporting display over
>>     HDMI for example. In other words, shouldn't the ifdef above be something
>>     more like:
>>
>>     #if define(CONFIG_LCD_SUPPORT)
>>             register_lcd_driver();
>>     #endif
>>     #if defined(CONFIG_VIDEO_TEGRA2)
>>             tegra_display_init(...);
>>     #endif
>>
>>     and internal to tegra_display_init(), the DT is searched for LCD
>>     controller nodes, and if any are found, they're matched to the LCD
>>     driver registered by the first call above.
>>
>>
>> Yes that sounds great, but again we don't really have this
>> infrastructure in U-Boot. We would be inventing it just for Tegra, and I
>> would prefer to wait until the device model stuff is done before being
>> too fancy.
>
> What is "the device model stuff" you mention?

There is an effort in U-Boot to create a unified device model, which
would permit things like the register_lcd_driver() call you show
above. At present we have hard-coded calls to various things in
U-Boot. This means that in many cases it is only possible to have one
driver of each time.

This also applied to LCD - at present it wouldn't be possible to
support HDMI separate from LCD, unless the HDMI stuff was perhaps done
through the separate CONFIG_VIDEO option. That would be odd though,
since we are using the display controller.

Maybe I am wrong, but this doesn't seem like the right time to address
this issue. Once the device model is in, we will find this sort of
thing much easier and cleaner.

>
>> We don't have an HDMI driver at present, so perhaps if/when that appears
>> in U-Boot it would be a good time to add support for that?
>
> Certainly adding code for HDMI can wait until later. However, I do think
> that we should use the correct ifdefs up-front, so that e.g. adding HDMI
> support only means adding a bunch of code, not going through the
> existing code and untangling conflated ifdefs.

Well you are right, it is conflated, although not only in the ifdefs!
I'm afraid that all the code currently deals with setting up the
display driver for an LCD - e.g. the code in display.c which sets up
clocks and LCD outputs/pinmux, plus the whole of
drivers/video/tegra.c. To be honest, the ifdefs are the least of our
problems.

For HDMI the whole thing would need another look I believe. But the
purpose of this series is to provide LCD support on Tegra and
unfortunately it isn't clear to me what the differences might be for
HDMI.

I will change this to use CONFIG_LCD since it is definitely more correct.

Regards,
Simon