Display related board specific boot args

Display related board specific boot args

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 817 bytes --]

Hi Tony,

As you probably know, Overo is a very basic omap board to which you can
attach different add-on modules, like LCD module. In the current overo
board file we always add multiple LCD devices, of which the user should
only use one (the one on his module). This model of allowing multiple
LCD devices on the same video bus is causing headaches in the dss land,
and I'm removing support for it.

So what we should do is add only the LCD device that you actually have
attached. With DT this will probably be handled with separate dts files,
depending on the add-on module.

How should this be handled with board files? Is it ok to add a board
specific kernel boot argument, parsed in the board file, which tells the
add-on board?

We have similar situation in some other boards also.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: Display related board specific boot args

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [130322 01:30]:
> Hi Tony,
> 
> As you probably know, Overo is a very basic omap board to which you can
> attach different add-on modules, like LCD module. In the current overo
> board file we always add multiple LCD devices, of which the user should
> only use one (the one on his module). This model of allowing multiple
> LCD devices on the same video bus is causing headaches in the dss land,
> and I'm removing support for it.

OK yes sounds like they should be all selectable, and only the selected
one gets initialized.
 
> So what we should do is add only the LCD device that you actually have
> attached. With DT this will probably be handled with separate dts files,
> depending on the add-on module.
> 
> How should this be handled with board files? Is it ok to add a board
> specific kernel boot argument, parsed in the board file, which tells the
> add-on board?

At this point I'd stay away adding any kind of custom handling to the
board-*.c files as that will make the task of removing them harder. And
a kernel boot argument is something we'd have to support in the future.
So if something gets added, it should be a Linux generic boot argument.

Some of these add on boards can be detected over i2c, but it seems as
DT is the way to go in the long run in this case.
 
> We have similar situation in some other boards also.

Yeah. There's the capebus coming that might even allow hotplugging
some of these add on boards eventually.

How about just make PANEL_LGPHILIPS_LB035Q02 depends on !PANEL_XYZ_DPI
in drivers/video/omap2/displays/Kconfig and add some comments for now?

That way people can still select LB035Q02 but not for generic
configurations.

Regards,

Tony

Re: Display related board specific boot args

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 2522 bytes --]

On 2013-03-22 17:18, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [130322 01:30]:
>> Hi Tony,
>>
>> As you probably know, Overo is a very basic omap board to which you can
>> attach different add-on modules, like LCD module. In the current overo
>> board file we always add multiple LCD devices, of which the user should
>> only use one (the one on his module). This model of allowing multiple
>> LCD devices on the same video bus is causing headaches in the dss land,
>> and I'm removing support for it.
> 
> OK yes sounds like they should be all selectable, and only the selected
> one gets initialized.
>  
>> So what we should do is add only the LCD device that you actually have
>> attached. With DT this will probably be handled with separate dts files,
>> depending on the add-on module.
>>
>> How should this be handled with board files? Is it ok to add a board
>> specific kernel boot argument, parsed in the board file, which tells the
>> add-on board?
> 
> At this point I'd stay away adding any kind of custom handling to the
> board-*.c files as that will make the task of removing them harder. And
> a kernel boot argument is something we'd have to support in the future.
> So if something gets added, it should be a Linux generic boot argument.
> 
> Some of these add on boards can be detected over i2c, but it seems as
> DT is the way to go in the long run in this case.
>  
>> We have similar situation in some other boards also.
> 
> Yeah. There's the capebus coming that might even allow hotplugging
> some of these add on boards eventually.
> 
> How about just make PANEL_LGPHILIPS_LB035Q02 depends on !PANEL_XYZ_DPI
> in drivers/video/omap2/displays/Kconfig and add some comments for now?
> 
> That way people can still select LB035Q02 but not for generic
> configurations.

Well, I don't want to disable a panel driver totally because of one
board. And it's more complex than that. For overo we have three displays
on the same DPI bus (using the names assigned in the board file): dvi,
lcd43, lcd35. So even if the LB035Q02 would be disabled, we have dvi and
lcd43 which conflict.

I also would stay away from custom boot args. So I guess that leaves us
only the compile time option. Either so that the overo board-file needs
to be modified by the user to change the add-on board, or a Kconfig
option to select the add-on board under "TI OMAP2/3/4 Specific Features"
menu.

Would the Kconfig option be acceptable?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: Display related board specific boot args

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [130322 08:34]:
> On 2013-03-22 17:18, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [130322 01:30]:
> >> Hi Tony,
> >>
> >> As you probably know, Overo is a very basic omap board to which you can
> >> attach different add-on modules, like LCD module. In the current overo
> >> board file we always add multiple LCD devices, of which the user should
> >> only use one (the one on his module). This model of allowing multiple
> >> LCD devices on the same video bus is causing headaches in the dss land,
> >> and I'm removing support for it.
> > 
> > OK yes sounds like they should be all selectable, and only the selected
> > one gets initialized.
> >  
> >> So what we should do is add only the LCD device that you actually have
> >> attached. With DT this will probably be handled with separate dts files,
> >> depending on the add-on module.
> >>
> >> How should this be handled with board files? Is it ok to add a board
> >> specific kernel boot argument, parsed in the board file, which tells the
> >> add-on board?
> > 
> > At this point I'd stay away adding any kind of custom handling to the
> > board-*.c files as that will make the task of removing them harder. And
> > a kernel boot argument is something we'd have to support in the future.
> > So if something gets added, it should be a Linux generic boot argument.
> > 
> > Some of these add on boards can be detected over i2c, but it seems as
> > DT is the way to go in the long run in this case.
> >  
> >> We have similar situation in some other boards also.
> > 
> > Yeah. There's the capebus coming that might even allow hotplugging
> > some of these add on boards eventually.
> > 
> > How about just make PANEL_LGPHILIPS_LB035Q02 depends on !PANEL_XYZ_DPI
> > in drivers/video/omap2/displays/Kconfig and add some comments for now?
> > 
> > That way people can still select LB035Q02 but not for generic
> > configurations.
> 
> Well, I don't want to disable a panel driver totally because of one
> board. And it's more complex than that. For overo we have three displays
> on the same DPI bus (using the names assigned in the board file): dvi,
> lcd43, lcd35. So even if the LB035Q02 would be disabled, we have dvi and
> lcd43 which conflict.
> 
> I also would stay away from custom boot args. So I guess that leaves us
> only the compile time option. Either so that the overo board-file needs
> to be modified by the user to change the add-on board, or a Kconfig
> option to select the add-on board under "TI OMAP2/3/4 Specific Features"
> menu.
> 
> Would the Kconfig option be acceptable?

Yes I think that's better than cmdline option in this case considering that
the cmdline option won't be needed at some point in the future.

Regards,

Tony

Re: Display related board specific boot args

[Andy Gross]

On 03/22/2013 10:36 AM, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [130322 08:34]:
>> On 2013-03-22 17:18, Tony Lindgren wrote:
>>> * Tomi Valkeinen <tomi.valkeinen@ti.com> [130322 01:30]:
>>>> Hi Tony,
>>>>
>>>> As you probably know, Overo is a very basic omap board to which you can
>>>> attach different add-on modules, like LCD module. In the current overo
>>>> board file we always add multiple LCD devices, of which the user should
>>>> only use one (the one on his module). This model of allowing multiple
>>>> LCD devices on the same video bus is causing headaches in the dss land,
>>>> and I'm removing support for it.
>>>
>>> OK yes sounds like they should be all selectable, and only the selected
>>> one gets initialized.
>>>
>>>> So what we should do is add only the LCD device that you actually have
>>>> attached. With DT this will probably be handled with separate dts files,
>>>> depending on the add-on module.
>>>>
>>>> How should this be handled with board files? Is it ok to add a board
>>>> specific kernel boot argument, parsed in the board file, which tells the
>>>> add-on board?
>>>
>>> At this point I'd stay away adding any kind of custom handling to the
>>> board-*.c files as that will make the task of removing them harder. And
>>> a kernel boot argument is something we'd have to support in the future.
>>> So if something gets added, it should be a Linux generic boot argument.
>>>
>>> Some of these add on boards can be detected over i2c, but it seems as
>>> DT is the way to go in the long run in this case.
>>>
>>>> We have similar situation in some other boards also.
>>>
>>> Yeah. There's the capebus coming that might even allow hotplugging
>>> some of these add on boards eventually.
>>>
>>> How about just make PANEL_LGPHILIPS_LB035Q02 depends on !PANEL_XYZ_DPI
>>> in drivers/video/omap2/displays/Kconfig and add some comments for now?
>>>
>>> That way people can still select LB035Q02 but not for generic
>>> configurations.
>>
>> Well, I don't want to disable a panel driver totally because of one
>> board. And it's more complex than that. For overo we have three displays
>> on the same DPI bus (using the names assigned in the board file): dvi,
>> lcd43, lcd35. So even if the LB035Q02 would be disabled, we have dvi and
>> lcd43 which conflict.
>>
>> I also would stay away from custom boot args. So I guess that leaves us
>> only the compile time option. Either so that the overo board-file needs
>> to be modified by the user to change the add-on board, or a Kconfig
>> option to select the add-on board under "TI OMAP2/3/4 Specific Features"
>> menu.
>>
>> Would the Kconfig option be acceptable?
>
> Yes I think that's better than cmdline option in this case considering that
> the cmdline option won't be needed at some point in the future.

Barring the hotplug infrastructure, the Kconfig sounds like the best 
option to me.


BR,
Andy

Overo expansion board selection with Kconfig (Was: Re: Display related board specific boot args)

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 6073 bytes --]

On 2013-03-22 17:36, Tony Lindgren wrote:

>> Would the Kconfig option be acceptable?
> 
> Yes I think that's better than cmdline option in this case considering that
> the cmdline option won't be needed at some point in the future.

Here's a diff for the changes for Overo. This is not a real patch yet,
just an RFC for the naming of the Kconfig options, and if this approach
is generally ok. Some other boards need similar setup also, but I think
Overo is the most complex case.

I only handle the displays here, and only three overo expansion boards.
I didn't find any of the expansion boards having analog TV out, so I removed
it.

I'm not sure if MACH_XXX style options should be reserved for boards only. It
just seemed like a logical way to extend the MACH_OVERO with
MACH_OVERO_EXPANSION_XXX.

 Tomi


diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 8111cd9..f30aadb 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -238,6 +238,28 @@ config MACH_OVERO
 	default y
 	select OMAP_PACKAGE_CBB
 
+choice
+	depends on MACH_OVERO
+	default MACH_OVERO_EXPANSION_PALO43
+	prompt "Overo Expansion Board"
+
+config MACH_OVERO_EXPANSION_PALO43
+	bool "Gumstix Overo Palo43 Expansion Board"
+	help
+	  Palo43 Expansion board with Samsung 4.3" 480x272 LCD.
+
+config MACH_OVERO_EXPANSION_PALO35
+	bool "Gumstix Overo Palo35 Expansion Board"
+	help
+	  Palo35 Expansion board with LG 3.5" 320x240 LCD.
+
+config MACH_OVERO_EXPANSION_SUMMIT
+	bool "Gumstix Overo Summit Expansion Board"
+	help
+	  Summit expansion board with DVI output.
+
+endchoice
+
 config MACH_OMAP3EVM
 	bool "OMAP 3530 EVM board"
 	depends on ARCH_OMAP3
diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
index 86bab51..d9df651 100644
--- a/arch/arm/mach-omap2/board-overo.c
+++ b/arch/arm/mach-omap2/board-overo.c
@@ -145,9 +145,6 @@ static void __init overo_init_smsc911x(void)
 static inline void __init overo_init_smsc911x(void) { return; }
 #endif
 
-/* DSS */
-static int lcd_enabled;
-static int dvi_enabled;
 
 #define OVERO_GPIO_LCD_EN 144
 #define OVERO_GPIO_LCD_BL 145
@@ -168,6 +165,8 @@ static void __init overo_display_init(void)
 	gpio_export(OVERO_GPIO_LCD_BL, 0);
 }
 
+#if defined(CONFIG_MACH_OVERO_EXPANSION_SUMMIT)
+
 static struct tfp410_platform_data dvi_panel = {
 	.i2c_bus_num		= 3,
 	.power_down_gpio	= -1,
@@ -181,23 +180,56 @@ static struct omap_dss_device overo_dvi_device = {
 	.phy.dpi.data_lines	= 24,
 };
 
-static struct omap_dss_device overo_tv_device = {
-	.name = "tv",
-	.driver_name = "venc",
-	.type = OMAP_DISPLAY_TYPE_VENC,
-	.phy.venc.type = OMAP_DSS_VENC_TYPE_SVIDEO,
+static struct omap_dss_device *overo_dss_devices[] = {
+	&overo_dvi_device,
 };
 
+static struct omap_dss_board_info overo_dss_data = {
+	.num_devices	= ARRAY_SIZE(overo_dss_devices),
+	.devices	= overo_dss_devices,
+	.default_device	= &overo_dvi_device,
+};
+
+#elif defined(CONFIG_MACH_OVERO_EXPANSION_PALO35)
+
 static int overo_panel_enable_lcd(struct omap_dss_device *dssdev)
 {
-	if (dvi_enabled) {
-		printk(KERN_ERR "cannot enable LCD, DVI is enabled\n");
-		return -EINVAL;
-	}
+	gpio_set_value(OVERO_GPIO_LCD_EN, 1);
+	gpio_set_value(OVERO_GPIO_LCD_BL, 1);
+	return 0;
+}
+
+static void overo_panel_disable_lcd(struct omap_dss_device *dssdev)
+{
+	gpio_set_value(OVERO_GPIO_LCD_EN, 0);
+	gpio_set_value(OVERO_GPIO_LCD_BL, 0);
+}
+
+static struct omap_dss_device overo_lcd35_device = {
+	.type			= OMAP_DISPLAY_TYPE_DPI,
+	.name			= "lcd35",
+	.driver_name		= "lgphilips_lb035q02_panel",
+	.phy.dpi.data_lines	= 24,
+	.platform_enable	= overo_panel_enable_lcd,
+	.platform_disable	= overo_panel_disable_lcd,
+};
+
+static struct omap_dss_device *overo_dss_devices[] = {
+	&overo_lcd35_device,
+};
 
+static struct omap_dss_board_info overo_dss_data = {
+	.num_devices	= ARRAY_SIZE(overo_dss_devices),
+	.devices	= overo_dss_devices,
+	.default_device	= &overo_lcd35_device,
+};
+
+#elif defined(CONFIG_MACH_OVERO_EXPANSION_PALO43)
+
+static int overo_panel_enable_lcd(struct omap_dss_device *dssdev)
+{
 	gpio_set_value(OVERO_GPIO_LCD_EN, 1);
 	gpio_set_value(OVERO_GPIO_LCD_BL, 1);
-	lcd_enabled = 1;
 	return 0;
 }
 
@@ -205,7 +237,6 @@ static void overo_panel_disable_lcd(struct omap_dss_device *dssdev)
 {
 	gpio_set_value(OVERO_GPIO_LCD_EN, 0);
 	gpio_set_value(OVERO_GPIO_LCD_BL, 0);
-	lcd_enabled = 0;
 }
 
 static struct panel_generic_dpi_data lcd43_panel = {
@@ -222,34 +253,18 @@ static struct omap_dss_device overo_lcd43_device = {
 	.phy.dpi.data_lines	= 24,
 };
 
-#if defined(CONFIG_PANEL_LGPHILIPS_LB035Q02) || \
-	defined(CONFIG_PANEL_LGPHILIPS_LB035Q02_MODULE)
-static struct omap_dss_device overo_lcd35_device = {
-	.type			= OMAP_DISPLAY_TYPE_DPI,
-	.name			= "lcd35",
-	.driver_name		= "lgphilips_lb035q02_panel",
-	.phy.dpi.data_lines	= 24,
-	.platform_enable	= overo_panel_enable_lcd,
-	.platform_disable	= overo_panel_disable_lcd,
-};
-#endif
-
 static struct omap_dss_device *overo_dss_devices[] = {
-	&overo_dvi_device,
-	&overo_tv_device,
-#if defined(CONFIG_PANEL_LGPHILIPS_LB035Q02) || \
-	defined(CONFIG_PANEL_LGPHILIPS_LB035Q02_MODULE)
-	&overo_lcd35_device,
-#endif
 	&overo_lcd43_device,
 };
 
 static struct omap_dss_board_info overo_dss_data = {
 	.num_devices	= ARRAY_SIZE(overo_dss_devices),
 	.devices	= overo_dss_devices,
-	.default_device	= &overo_dvi_device,
+	.default_device	= &overo_lcd43_device,
 };
 
+#endif
+
 static struct mtd_partition overo_nand_partitions[] = {
 	{
 		.name           = "xloader",
@@ -438,8 +453,7 @@ static int __init overo_i2c_init(void)
 }
 
 static struct spi_board_info overo_spi_board_info[] __initdata = {
-#if defined(CONFIG_PANEL_LGPHILIPS_LB035Q02) || \
-	defined(CONFIG_PANEL_LGPHILIPS_LB035Q02_MODULE)
+#ifdef MACH_OVERO_EXPANSION_PALO35
 	{
 		.modalias		= "lgphilips_lb035q02_panel-spi",
 		.bus_num		= 1,



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]