From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Thu, 27 Sep 2012 12:15:15 -0700 Subject: [U-Boot] [PATCH v2 11/19] tegra: Add LCD support to Nvidia boards In-Reply-To: <500DB320.60003@wwwdotorg.org> References: <1339604395-6621-1-git-send-email-sjg@chromium.org> <1339604395-6621-12-git-send-email-sjg@chromium.org> <4FDA7829.6020106@wwwdotorg.org> <500DB320.60003@wwwdotorg.org> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Stephen, On Mon, Jul 23, 2012 at 1:25 PM, Stephen Warren wrote: > On 07/10/2012 10:58 PM, Simon Glass wrote: >> Hi Stephen, >> >> On Fri, Jun 15, 2012 at 1:47 AM, Stephen Warren > > 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