Structs for platform data of omapdss panels are found in headers in the
'include/video/' path. Board files populate these structs with platform
specific values, and the panel driver uses these to configure the panel.
Currently, each panel has it's own header in the above path. Move all the
omapdss panel platform data structs to a single header omap-panel-data.h.
This is useful because:
- All other omapdss panel drivers will be modified to use platform data. This
would lead to a lot of panel headers usable only by omapdss. A lot of these
platform data structs are trivial, and don't really need a separate header.
- Platform data would be eventually removed, and platform information would be
passed via device tree. Therefore, omapdss panel platform data structs are
temporary, and will be easier to remove if they are all in the same header.
- All board files will have to include the same header to configure a panel's
platform data, that makes the board files more consistent.
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-2430sdp.c | 2 +-
arch/arm/mach-omap2/board-3430sdp.c | 2 +-
arch/arm/mach-omap2/board-am3517evm.c | 3 +-
arch/arm/mach-omap2/board-apollon.c | 2 +-
arch/arm/mach-omap2/board-cm-t35.c | 3 +-
arch/arm/mach-omap2/board-devkit8000.c | 3 +-
arch/arm/mach-omap2/board-h4.c | 2 +-
arch/arm/mach-omap2/board-igep0020.c | 2 +-
arch/arm/mach-omap2/board-ldp.c | 2 +-
arch/arm/mach-omap2/board-omap3beagle.c | 2 +-
arch/arm/mach-omap2/board-omap3evm.c | 2 +-
arch/arm/mach-omap2/board-omap3stalker.c | 3 +-
arch/arm/mach-omap2/board-overo.c | 3 +-
arch/arm/mach-omap2/dss-common.c | 4 +-
drivers/video/omap2/displays/panel-generic-dpi.c | 2 +-
drivers/video/omap2/displays/panel-n8x0.c | 2 +-
drivers/video/omap2/displays/panel-picodlp.c | 2 +-
drivers/video/omap2/displays/panel-taal.c | 2 +-
drivers/video/omap2/displays/panel-tfp410.c | 2 +-
include/video/omap-panel-data.h | 101 ++++++++++++++++++++++
include/video/omap-panel-generic-dpi.h | 37 --------
include/video/omap-panel-n8x0.h | 13 ---
include/video/omap-panel-nokia-dsi.h | 32 -------
include/video/omap-panel-picodlp.h | 23 -----
include/video/omap-panel-tfp410.h | 35 --------
25 files changed, 120 insertions(+), 166 deletions(-)
create mode 100644 include/video/omap-panel-data.h
delete mode 100644 include/video/omap-panel-generic-dpi.h
delete mode 100644 include/video/omap-panel-n8x0.h
delete mode 100644 include/video/omap-panel-nokia-dsi.h
delete mode 100644 include/video/omap-panel-picodlp.h
delete mode 100644 include/video/omap-panel-tfp410.h
@@ -1,37 +0,0 @@-/*- * Header for generic DPI panel driver- *- * Copyright (C) 2010 Canonical Ltd.- * Author: Bryan Wu <bryan.wu@canonical.com>- *- * This program is free software; you can redistribute it and/or modify it- * under the terms of the GNU General Public License version 2 as published by- * the Free Software Foundation.- *- * This program is distributed in the hope that it will be useful, but WITHOUT- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for- * more details.- *- * You should have received a copy of the GNU General Public License along with- * this program. If not, see <http://www.gnu.org/licenses/>.- */--#ifndef __OMAP_PANEL_GENERIC_DPI_H-#define __OMAP_PANEL_GENERIC_DPI_H--struct omap_dss_device;--/**- * struct panel_generic_dpi_data - panel driver configuration data- * @name: panel name- * @platform_enable: platform specific panel enable function- * @platform_disable: platform specific panel disable function- */-struct panel_generic_dpi_data {- const char *name;- int (*platform_enable)(struct omap_dss_device *dssdev);- void (*platform_disable)(struct omap_dss_device *dssdev);-};--#endif /* __OMAP_PANEL_GENERIC_DPI_H */
@@ -1,23 +0,0 @@-/*- * panel data for picodlp panel- *- * Copyright (C) 2011 Texas Instruments- *- * Author: Mayuresh Janorkar <mayur@ti.com>- *- * This program is free software; you can redistribute it and/or modify- * it under the terms of the GNU General Public License version 2 as- * published by the Free Software Foundation.- */-#ifndef __PANEL_PICODLP_H-#define __PANEL_PICODLP_H-/**- * struct : picodlp panel data- * picodlp_adapter_id: i2c_adapter number for picodlp- */-struct picodlp_panel_data {- int picodlp_adapter_id;- int emu_done_gpio;- int pwrgood_gpio;-};-#endif /* __PANEL_PICODLP_H */
@@ -1,35 +0,0 @@-/*- * Header for TFP410 chip driver- *- * Copyright (C) 2011 Texas Instruments Inc- * Author: Tomi Valkeinen <tomi.valkeinen@ti.com>- *- * This program is free software; you can redistribute it and/or modify it- * under the terms of the GNU General Public License version 2 as published by- * the Free Software Foundation.- *- * This program is distributed in the hope that it will be useful, but WITHOUT- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for- * more details.- *- * You should have received a copy of the GNU General Public License along with- * this program. If not, see <http://www.gnu.org/licenses/>.- */--#ifndef __OMAP_PANEL_TFP410_H-#define __OMAP_PANEL_TFP410_H--struct omap_dss_device;--/**- * struct tfp410_platform_data - panel driver configuration data- * @i2c_bus_num: i2c bus id for the panel- * @power_down_gpio: gpio number for PD pin (or -1 if not available)- */-struct tfp410_platform_data {- int i2c_bus_num;- int power_down_gpio;-};--#endif /* __OMAP_PANEL_TFP410_H */
From: Tomi Valkeinen <redacted>
The generic dpi panel driver leaves gpio configurations to the platform_enable
and disable calls in the platform's board file. These should happen in the
panel driver itself.
Add a generic way of passing gpio information to the generic dpi panel driver
via it's platform_data. This information includes the number of gpios used by
the panel, the gpio number and logic level (active high/low) for each gpio. This
gpio data will be used by the driver to request and configure the gpios required
by the panel.
This will help in removing the need for the panel drivers to have platform
related callbacks.
Signed-off-by: Tomi Valkeinen <redacted>
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-generic-dpi.c | 25 ++++++++++++++++++++--
include/video/omap-panel-data.h | 7 ++++++
2 files changed, 30 insertions(+), 2 deletions(-)
The 2430sdp board file currently requests gpios required to configure the NEC
DPI panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the generic dpi panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to generic dpi panel's platform data so that it's
passed to the panel driver.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-2430sdp.c | 43 +++++------------------------------
1 file changed, 6 insertions(+), 37 deletions(-)
The devkit8000 board file currently requests gpios required to configure the
innolux DPI panel, and provides platform_enable/disable callbacks to configure
them.
These tasks have been moved to the generic dpi panel driver itself and should
be removed from the board files.
Remove the gpio request and the platform callbacks from the board file.
Configure the gpio information in generic dpi panel's platform data so that it's
passed to the panel driver.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-devkit8000.c | 27 +++------------------------
1 file changed, 3 insertions(+), 24 deletions(-)
@@ -219,13 +203,8 @@ static int devkit8000_twl_gpio_setup(struct device *dev,gpio_leds[2].gpio=gpio+TWL4030_GPIO_MAX+1;/* TWL4030_GPIO_MAX + 0 is "LCD_PWREN" (out, active high) */-devkit8000_lcd_device.reset_gpio=gpio+TWL4030_GPIO_MAX+0;-ret=gpio_request_one(devkit8000_lcd_device.reset_gpio,-GPIOF_OUT_INIT_LOW,"LCD_PWREN");-if(ret<0){-devkit8000_lcd_device.reset_gpio=-EINVAL;-printk(KERN_ERR"Failed to request GPIO for LCD_PWRN\n");-}+lcd_panel.num_gpios=1;+lcd_panel.gpios[0]=gpio+TWL4030_GPIO_MAX+0;/* gpio + 7 is "DVI_PD" (out, active low) */dvi_panel.power_down_gpio=gpio+7;
The cm-t35 board file currently requests gpios required to configure the tdo35s
panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the generic dpi panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to generic dpi panel's platform data so that it's
passed to the panel driver.
Note: In cm_t35_init_display(), the gpios were disabled, and the LCD_EN gpio was
enabled after a 50 millisecond delay. This code has been removed and is not
taken care of in the generic panel driver. The impact of this needs to be
tested. The panel's gpios are also not exported any more. Hence, they can't be
accessed via sysfs interface.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-cm-t35.c | 46 ++++--------------------------------
1 file changed, 5 insertions(+), 41 deletions(-)
The apollon board file currently configures the LCD_PWR_EN gpio by muxing the
corresponding pin to gpio 11, and configuring it in PULL UP mode.
Remove this muxing from the board file. Add the gpio information to generic dpi
panel's platform data so that it's passed to the panel driver. The panel driver
will take care of requesting and setting the LCD_PWR_EN gpio.
Note: This should be tested to ensure that setting the GPIO is equivalent to
configuring the GPIO in PULL UP mode. Also, this GPIO was just set once during
init, and never cleared, where as now the gpio will toggle everytime the panel
is disabled/enabled. The impact of this needs to be tested.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-apollon.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
The am3517 board file currently requests gpios required to configure the sharp
lq DPI panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the generic dpi panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to generic dpi panel's platform data so that it's
passed to the panel driver.
Note: It's not clear why the GPIOs were muxed as input signals in PULL down mode
in am3517_evm_display_init(). Also, only the LCD_PANEL_PWR was toggled in the
platform_enable/disable calls, the generic DPI panel driver will now toggle all
the three gpios on panel's disable/enable. We need to test if these changes to
see if they have any impact or not.
Cc: Tony Lindgren <tony@atomide.com>
Cc: Vaibhav Hiremath <redacted>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-am3517evm.c | 63 ++++-----------------------------
1 file changed, 6 insertions(+), 57 deletions(-)
The ldp board file currently requests gpios required to configure the NEC DPI
panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the generic dpi panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Configure the gpio information in generic dpi panel's platform data so that it's
passed to the panel driver.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-ldp.c | 61 ++++++---------------------------------
1 file changed, 9 insertions(+), 52 deletions(-)
The lgphilips panel driver leaves gpio configurations to the platform_enable
and disable calls in the platform's board file. These should happen in the
panel driver itself.
Use the platform data as defined for generic dpi panels to pass gpio information
to the lgphilips driver.
This will help in removing the need for the panel drivers to have platform
related callbacks.
Signed-off-by: Archit Taneja <redacted>
---
.../omap2/displays/panel-lgphilips-lb035q02.c | 38 +++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
The overo board file currently requests gpios required by the lb035q02 panel,
and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the lb035q02 dpi panel driver itself and should
be removed from the board files.
The lb035q02 panel driver uses generic dpi panel's platform data struct
internally. Remove the gpio requests and the platform callbacks from the board
file. Add the gpio information to the generic dpi panel platform data struct so
that it's passed to the panel driver.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-overo.c | 58 +++++++++----------------------------
1 file changed, 14 insertions(+), 44 deletions(-)
The lgphilips panel driver now manages the gpios required to configure the
panel. This was previously done in omap_dss_device's platform_enable/disable
callbacks defined in board files using this panel.
All the board files using this panel now pass the gpio information as platform
data via the panel_generic_dpi_data struct, which is needed by the panel driver
to configure the gpios connected to the panel. Hence, the
platform_enable/disable ops can be safely removed now.
Signed-off-by: Archit Taneja <redacted>
---
.../omap2/displays/panel-lgphilips-lb035q02.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
The generic dpi panel driver now sets the gpios required to configure the panel.
This was previously done in platform_enable/disable callbacks in board files.
All the board files using generic dpi panel now correctly pass the gpio related
information as platform data, which is needed by the panel driver to configure
the panel. Hence, the platform_enable/disable ops can be safely removed now.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-generic-dpi.c | 12 +---
.../video/omap2/displays/panel-sharp-ls037v7dw01.c | 71 +++++++++++++++++---
include/video/omap-panel-data.h | 20 ++++--
3 files changed, 77 insertions(+), 26 deletions(-)
The omap3evm board file currently requests gpios required by the sharp_ls dpi
panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the sharp_ls panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to panel_sharp_ls037v7dw01_data so that it's passed
to the panel driver.
Note: The GPIOs OMAP3EVM_LCD_PANEL_ENVDD and OMAP3EVM_LCD_PANEL_BKLIGHT_GPIO
aren't directly connected to the sharp panel, hence they aren't passed to the
panel driver as platform data. These are set to a default value such that LCD
is enabled and backlight is on. These used to previously toggle through the
platform_enable/disable callbacks, but now these are always on. This needs to
be revisited.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-omap3evm.c | 59 ++++++++++++----------------------
1 file changed, 20 insertions(+), 39 deletions(-)
The omap3430sdp board file currently requests gpios required by the sharp_ls dpi
panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the sharp_ls panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to panel_sharp_ls037v7dw01_data so that it's
passed to the panel driver.
Out of sharp panel's configurable pins, all apart from resb_gpio are managed by
a CPLD on the display and set to a default value. Only the configurable pin is
passed to platform data.
The backlight GPIO doesn't go directly to the sharp panel, it is used to set up
a voltage supply which goes to the LED+ pin of the panel, hence it isn't passed
to panel as platform data, and configured in the board file itself. The
backlight used to previously toggle through the platform_enable/disable
callbacks, but now it is always on. This needs to be revisited.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-3430sdp.c | 42 +++++++++++++++--------------------
1 file changed, 18 insertions(+), 24 deletions(-)
The sharp-ls panel driver now manages the gpios required to configure the panel.
This was previously done in omap_dss_device's platform_enable/disable callbacks
defined in board files using this panel.
All the board files using this panel now pass the gpio information as platform
data via the panel_sharp_ls037v7dw01_data struct, which is needed by the panel
driver to configure the gpios connected to the panel. Hence, the
platform_enable/disable ops can be safely removed now.
Signed-off-by: Archit Taneja <redacted>
---
.../video/omap2/displays/panel-sharp-ls037v7dw01.c | 11 -----------
1 file changed, 11 deletions(-)
@@ -125,12 +125,6 @@ static int sharp_ls_power_on(struct omap_dss_device *dssdev)/* wait couple of vsyncs until enabling the LCD */msleep(50);-if(dssdev->platform_enable){-r=dssdev->platform_enable(dssdev);-if(r)-gotoerr1;-}-if(gpio_is_valid(pd->resb_gpio))gpio_set_value_cansleep(pd->resb_gpio,1);
@@ -138,8 +132,6 @@ static int sharp_ls_power_on(struct omap_dss_device *dssdev)gpio_set_value_cansleep(pd->ini_gpio,1);return0;-err1:-omapdss_dpi_display_disable(dssdev);err0:returnr;}
@@ -157,9 +149,6 @@ static void sharp_ls_power_off(struct omap_dss_device *dssdev)if(gpio_is_valid(pd->resb_gpio))gpio_set_value_cansleep(pd->resb_gpio,0);-if(dssdev->platform_disable)-dssdev->platform_disable(dssdev);-/* wait at least 5 vsyncs after disabling the LCD */msleep(100);
The acx565akm panel driver leaves gpio configurations to the platform_enable
and disable calls in the platform's board file. These should happen in the panel
driver itself.
Create a platform data struct for the panel, this contains the reset gpio number
used by the panel driver, this struct will be passed to the panel driver as
platform data. The driver will request and configure the reset gpio rather than
leaving it to platform callbacks in board files.
This will help in removing the need for the panel drivers to have platform
related callbacks.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-acx565akm.c | 48 ++++++++++++++++--------
include/video/omap-panel-data.h | 8 ++++
2 files changed, 41 insertions(+), 15 deletions(-)
The rx-51 board file currently requests gpios required by the acx565akm panel,
and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the acx565akm panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file. Pass
the panel_acx565akm_data instance 'lcd_data' to omap_dss_device instead of
passing the gpio number in omap_dss_device's reset_gpio.
Add the gpio information to panel_acx565akm_data so that it's passed to the
panel driver.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-rx51-video.c | 26 +++++++-------------------
1 file changed, 7 insertions(+), 19 deletions(-)
The nec-nl8048hl11-01 panel driver leaves gpio configurations to the
platform_enable and disable calls in the platform's board file. These should
happen in the panel driver itself.
Create a platform data struct for the panel, this contains the gpio numbers
used by the panel driver, this struct will be passed to the panel driver as
platform data. The driver will request and configure these gpios rather than
leaving it to platform callbacks in board files.
This will help in removing the need for the panel drivers to have platform
related callbacks.
Signed-off-by: Archit Taneja <redacted>
---
.../omap2/displays/panel-nec-nl8048hl11-01b.c | 45 +++++++++++++++-----
include/video/omap-panel-data.h | 10 +++++
2 files changed, 44 insertions(+), 11 deletions(-)
The zoom board file currently requests gpios required by the nec-nl8048hl11-01
dpi panel, and provides dummy platform_enable/disable callbacks.
gpio request and configuration have been moved to the nec-nl8048hl11-01 panel
driver itself and shouldn't be done in the board files.
Remove the gpio requests and the platform callbacks from the board file. Add the
gpio information to panel_nec_nl8048_data so that it's passed to the panel
driver.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-zoom-display.c | 38 ++++++++++--------------------
1 file changed, 13 insertions(+), 25 deletions(-)
The nec-nl8048 panel driver now manages the gpios required to configure the
panel. This was previously done in omap_dss_device's platform_enable/disable
callbacks defined in board files using this panel.
All the board files using this panel now pass the gpio information as platform
data via the panel_nec_nl8048_data struct, which is needed by the panel driver
to configure the gpios connected to the panel. Hence, the
platform_enable/disable ops can be safely removed now.
Signed-off-by: Archit Taneja <redacted>
---
.../omap2/displays/panel-nec-nl8048hl11-01b.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
The tpo-td043mtea1 panel driver leaves gpio configurations to the
platform_enable and disable calls in the platform's board file. These should
happen in the panel driver itself.
Create a platform data struct for the panel, this contains the reset gpio
number used by the panel driver, this struct will be passed to the panel driver
as platform data. The driver will request and configure the reset gpio rather
than leaving it to platform callbacks in board files.
This will help in removing the need for the panel drivers to have platform
related callbacks.
Signed-off-by: Archit Taneja <redacted>
---
.../video/omap2/displays/panel-tpo-td043mtea1.c | 36 ++++++++++++--------
include/video/omap-panel-data.h | 8 +++++
2 files changed, 29 insertions(+), 15 deletions(-)
@@ -290,8 +296,8 @@ static int tpo_td043_power_on(struct tpo_td043_device *tpo_td043)/* wait for panel to stabilize */msleep(160);-if(gpio_is_valid(nreset_gpio))-gpio_set_value(nreset_gpio,1);+if(gpio_is_valid(tpo_td043->nreset_gpio))+gpio_set_value(tpo_td043->nreset_gpio,1);tpo_td043_write(tpo_td043->spi,2,TPO_R02_MODE(tpo_td043->mode)|TPO_R02_NCLK_RISING);
@@ -308,16 +314,14 @@ static int tpo_td043_power_on(struct tpo_td043_device *tpo_td043)staticvoidtpo_td043_power_off(structtpo_td043_device*tpo_td043){-intnreset_gpio=tpo_td043->nreset_gpio;-if(!tpo_td043->powered_on)return;tpo_td043_write(tpo_td043->spi,3,TPO_R03_VAL_STANDBY|TPO_R03_EN_PWM);-if(gpio_is_valid(nreset_gpio))-gpio_set_value(nreset_gpio,0);+if(gpio_is_valid(tpo_td043->nreset_gpio))+gpio_set_value(tpo_td043->nreset_gpio,0);/* wait for at least 2 vsyncs before cutting off power */msleep(50);
The omap3pandora board file currently passes the reset gpio number to the
tpo-td043mtea1 panel driver via the reset_gpio field in omap_dss_device.
Platform related information should be passed via the panel driver's platform
data struct.
Add the reset gpio information to panel_tpo_td043_data so that it's passed to
the panel driver.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-omap3pandora.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
The tpo-td043 panel driver now manages the gpios required to configure the panel.
This was previously done in omap_dss_device's platform_enable/disable callbacks
defined in board files using this panel.
All the board files using this panel now pass the gpio information as platform
data via the panel_tpo_td043_data struct, which is needed by the panel driver to
configure the gpios connected to the panel. Hence, the platform_enable/disable
ops can be safely removed now.
Signed-off-by: Archit Taneja <redacted>
---
.../video/omap2/displays/panel-tpo-td043mtea1.c | 9 ---------
1 file changed, 9 deletions(-)
The picodlp panel driver leaves gpio requests to the platform's board file.
These should happen in the panel driver itself.
A platform data struct called picodlp_panel_data already exists to hold gpio
numbers and other platform data. Request all the gpios in the panel driver so
that the board files which use the the panel don't need to do it.
This will help in removing the need for the panel drivers to have platform
related callbacks.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-picodlp.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
The dss-common file currently requests gpios required by the picodlp DPI
panel on the 4430sdp/blaze board. It also requests DISPLAY_SEL_GPIO and
DLP_POWER_ON_GPIO gpios which are board specific gpios to switch between lcd2
panel and picodlp, and setting intermediate power supplies for picodlp
respectively. These gpios are toggled through platform_enable/disable functions
called by the picodlp driver.
Remove the gpio requests for the gpios which are already requested by the panel
driver, and remove the platform callback functions and set the platform specific
gpios in such a way that lcd2 panel is selected for the LCD2 overlay manager and
the power supplies for picodlp are disabled.
Note: We need to revisit this so that we can enable and switch to picodlp if
that's the only panel driver available for the LCD2 overlay manager.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/dss-common.c | 53 ++++++++++++--------------------------
1 file changed, 16 insertions(+), 37 deletions(-)
@@ -232,17 +199,26 @@ static struct omap_dss_board_info sdp4430_dss_data = {.default_device=&sdp4430_lcd_device,};+/*+*weselectLCD2bydefault(insteadofPicoDLP)bysettingDISPLAY_SEL_GPIO.+*SettingDLP_POWER_ONgpioenablestheVDLP_2V5VDLP_1V8andVDLP_1V0rails+*usedbypicodlponthe4430sdpplatform.KeepthisgpiodisabledasLCD2is+*selectedbydefault+*/void__initomap_4430sdp_display_init(void){intr;-/* Enable LCD2 by default (instead of Pico DLP) */r=gpio_request_one(DISPLAY_SEL_GPIO,GPIOF_OUT_INIT_HIGH,"display_sel");if(r)pr_err("%s: Could not get display_sel GPIO\n",__func__);-sdp4430_picodlp_init();+r=gpio_request_one(DLP_POWER_ON_GPIO,GPIOF_OUT_INIT_LOW,+"DLP POWER ON");+if(r)+pr_err("%s: Could not get DLP POWER ON GPIO\n",__func__);+omap_display_init(&sdp4430_dss_data);/**OMAP4460SDP/BlazeandOMAP4430ES2.3SDP/Blazeboardsand
@@ -262,12 +238,15 @@ void __init omap_4430sdp_display_init_of(void){intr;-/* Enable LCD2 by default (instead of Pico DLP) */r=gpio_request_one(DISPLAY_SEL_GPIO,GPIOF_OUT_INIT_HIGH,"display_sel");if(r)pr_err("%s: Could not get display_sel GPIO\n",__func__);-sdp4430_picodlp_init();+r=gpio_request_one(DLP_POWER_ON_GPIO,GPIOF_OUT_INIT_LOW,+"DLP POWER ON");+if(r)+pr_err("%s: Could not get DLP POWER ON GPIO\n",__func__);+omap_display_init(&sdp4430_dss_data);}
The picodlp panel driver now manages the gpios required to configure the
panel. This was previously done in omap_dss_device's platform_enable/disable
callbacks defined in board files using this panel.
All the board files using this panel now pass the gpio information as platform
data via the panel_generic_dpi_data struct, which is needed by the panel driver
to configure the gpios connected to the panel. Hence, the
platform_enable/disable ops can be safely removed now.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-picodlp.c | 12 ------------
1 file changed, 12 deletions(-)
The n8x0 panel driver leaves gpio configurations to the platform_enable and
disable calls in the platform's board file. These should happen in the panel
driver itself.
A platform data struct called panel_n8x0_data already exists to hold gpio
numbers and other platform data. However, the gpio requests are expected to be
done in the board file and not the panel driver.
Request all the gpios in the panel driver so that the board files which use
the the panel don't need to do it. This will help in removing the need for the
panel drivers to have platform related callbacks.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-n8x0.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
The n8x0 panel driver now manages the gpios required to configure the panel.
This was previously done in panel_n8x0_data's platform_enable/disable callbacks
defined in board files using this panel.
All the board files using this panel now pass the gpio information as platform
data via the panel_n8x0_data struct, which is needed by the panel driver to
configure the gpios connected to the panel. Hence, the platform_enable/disable
ops can be safely removed now.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/displays/panel-n8x0.c | 12 ------------
include/video/omap-panel-data.h | 2 --
2 files changed, 14 deletions(-)
The omap_dss_device's platform_enable/disable callbacks don't do anything for
any of the boards. The platform calls from the VENC driver will also be removed
in the future. Remove these calls from the board which have a VENC device.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/board-3430sdp.c | 11 -----------
arch/arm/mach-omap2/board-am3517evm.c | 11 -----------
arch/arm/mach-omap2/board-cm-t35.c | 11 -----------
arch/arm/mach-omap2/board-omap3evm.c | 11 -----------
arch/arm/mach-omap2/board-omap3stalker.c | 11 -----------
5 files changed, 55 deletions(-)
The platform_enable/disable callbacks in board files for VENC omap_dss_device
instances don't do anything. Hence, we can remove these callbacks from the VENC
driver.
Signed-off-by: Archit Taneja <redacted>
---
drivers/video/omap2/dss/venc.c | 9 ---------
1 file changed, 9 deletions(-)
None of the omapdss panel drivers call platform_enable/disable callbacks, and
none of the omap board files define these callbacks for any omap_dss_device.
Hence these callbacks can be removed form the omap_dss_device struct.
Signed-off-by: Archit Taneja <redacted>
---
include/video/omapdss.h | 4 ----
1 file changed, 4 deletions(-)
gpio reset info is passed to the tfp410 panel driver via the panel's platform
data struct 'tfp410_platform_data'. The tfp driver doesn't use the reset_gpio
field in the omap4_panda_dvi_device struct. Remove this field.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <redacted>
---
arch/arm/mach-omap2/dss-common.c | 1 -
1 file changed, 1 deletion(-)
The reset_gpio field isn't used by any panel driver to retrieve a reset gpio
number. All the panel drivers receive gpio data from their corresponding
platform_data structs. Remove the reset_gpio field.
Signed-off-by: Archit Taneja <redacted>
---
include/video/omapdss.h | 2 --
1 file changed, 2 deletions(-)
From: Igor Grinberg <hidden> Date: 2013-02-13 15:16:32
Hi Archit,
On 02/13/13 16:21, Archit Taneja wrote:
The cm-t35 board file currently requests gpios required to configure the tdo35s
panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the generic dpi panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to generic dpi panel's platform data so that it's
passed to the panel driver.
Note: In cm_t35_init_display(), the gpios were disabled, and the LCD_EN gpio was
enabled after a 50 millisecond delay. This code has been removed and is not
taken care of in the generic panel driver. The impact of this needs to be
tested. The panel's gpios are also not exported any more. Hence, they can't be
accessed via sysfs interface.
Indeed, there is an impact - the LCD no longer works.
The reason for the LCD_EN gpio being pushed high after the 50ms delay,
is to get the LCD out of reset, so the SPI transaction will succeed
and initialize the LCD.
Now, when you remove the gpio handling for the LCD_EN pin,
the LCD no longer works.
I don't agree with this breakage.
From: Tomi Valkeinen <hidden> Date: 2013-02-13 15:28:19
On 2013-02-13 17:16, Igor Grinberg wrote:
Hi Archit,
On 02/13/13 16:21, Archit Taneja wrote:
quoted
The cm-t35 board file currently requests gpios required to configure the tdo35s
panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the generic dpi panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to generic dpi panel's platform data so that it's
passed to the panel driver.
Note: In cm_t35_init_display(), the gpios were disabled, and the LCD_EN gpio was
enabled after a 50 millisecond delay. This code has been removed and is not
taken care of in the generic panel driver. The impact of this needs to be
tested. The panel's gpios are also not exported any more. Hence, they can't be
accessed via sysfs interface.
Indeed, there is an impact - the LCD no longer works.
The reason for the LCD_EN gpio being pushed high after the 50ms delay,
is to get the LCD out of reset, so the SPI transaction will succeed
and initialize the LCD.
Now, when you remove the gpio handling for the LCD_EN pin,
the LCD no longer works.
So between what is the sleep done? It's not clear from the code. LCD_EN
needs to be 0 for 50ms, or...?
If the panel requires specific reset handling, does it work right even
currently when the panel is turned off and later turned on? The msleep
is only used at boot time.
Tomi
From: Tomi Valkeinen <hidden> Date: 2013-02-13 15:59:58
On 2013-02-13 17:28, Tomi Valkeinen wrote:
On 2013-02-13 17:16, Igor Grinberg wrote:
quoted
Hi Archit,
On 02/13/13 16:21, Archit Taneja wrote:
quoted
The cm-t35 board file currently requests gpios required to configure the tdo35s
panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the generic dpi panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to generic dpi panel's platform data so that it's
passed to the panel driver.
Note: In cm_t35_init_display(), the gpios were disabled, and the LCD_EN gpio was
enabled after a 50 millisecond delay. This code has been removed and is not
taken care of in the generic panel driver. The impact of this needs to be
tested. The panel's gpios are also not exported any more. Hence, they can't be
accessed via sysfs interface.
Indeed, there is an impact - the LCD no longer works.
The reason for the LCD_EN gpio being pushed high after the 50ms delay,
is to get the LCD out of reset, so the SPI transaction will succeed
and initialize the LCD.
Now, when you remove the gpio handling for the LCD_EN pin,
the LCD no longer works.
So between what is the sleep done? It's not clear from the code. LCD_EN
needs to be 0 for 50ms, or...?
If the panel requires specific reset handling, does it work right even
currently when the panel is turned off and later turned on? The msleep
is only used at boot time.
Okay, so I just realized there's an spi backlight driver used here, and
that backlight driver is actually handling the SPI transactions with the
panel, instead of the panel driver. So this looks quite messed up.
For a quick solution, can we just set the LCD_EN at boot time (with the
msleep), and not touch it after that?
Tomi
From: Tony Lindgren <tony@atomide.com> Date: 2013-02-13 16:46:47
* Archit Taneja [off-list ref] [130213 06:26]:
init functions in omap board files request panel specific gpios, and provide
functions which omapdss panel drivers call to enable or disable them.
Instead of the board files requesting these gpios, they should just pass the
platform specific data(like the gpio numbers), the panel should retrieve the
platform data and request the gpios. Doing this prevents the need of the panel
driver calling platform functions in board files.
Panel drivers have their own platform data struct, and the board files populate
these structs and pass the pointer to the 'data' field of omap_dss_device. This
work will make it easier for the panel drivers be more adaptable to the
DT model.
There is also removal of passing panel reset_gpio numbers through
omap_dss_device struct directly, reset gpios are passed through platform data
only.
To avoid merge conflicts and dependencies between drivers and core
Soc code, please break thes kind of patches into following parts:
1. Any platform_data header changes needed so both I and Tomi
can pull it in as needed.
2. Changes to DSS drivers. Please keep stubs around for the
board specific callback functions so omap2plus_defconfig
won't break with just #1 merged into arm soc tree.
3. All the arch/arm/*omap* changes based on #1 above to
drop obsolete callback functions and add new pdata if still
needed. This needs to build and boot on #1 so I can merge
this in via arm soc tree.
4. Any .dts changes needed.
Regards,
Tony
Hi,
On Wednesday 13 February 2013 11:05 PM, Aaro Koskinen wrote:
Hi,
On Wed, Feb 13, 2013 at 07:52:19PM +0530, Archit Taneja wrote:
quoted
@@ -444,6 +445,20 @@ static int n8x0_panel_probe(struct omap_dss_device *dssdev) dssdev->ctrl.rfbi_timings = n8x0_panel_timings; dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;+ if (gpio_is_valid(bdata->panel_reset)) {+ r = devm_gpio_request_one(&dssdev->dev, bdata->panel_reset,+ GPIOF_OUT_INIT_LOW, "PANEL RESET");+ if (r)+ return r;+ }++ if (gpio_is_valid(bdata->ctrl_pwrdown)) {+ r = devm_gpio_request_one(&dssdev->dev, bdata->ctrl_pwrdown,+ GPIOF_OUT_INIT_LOW, "PANEL PWRDOWN");+ if (r)+ return r;+ }+
In the error case, the other GPIO is not freed. Also maybe you should
free them on module removal, because now the module owns the GPIOs.
Wouldn't the usage of devm_* functions take care of this? If the device
isn't registered successfully, then all allocations/requests done using
devm_* functions will be free'd automatically.
Archit
From: Igor Grinberg <hidden> Date: 2013-02-14 06:56:03
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 02/13/13 17:59, Tomi Valkeinen wrote:
On 2013-02-13 17:28, Tomi Valkeinen wrote:
quoted
On 2013-02-13 17:16, Igor Grinberg wrote:
quoted
Hi Archit,
On 02/13/13 16:21, Archit Taneja wrote:
quoted
The cm-t35 board file currently requests gpios required to configure the tdo35s
panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the generic dpi panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to generic dpi panel's platform data so that it's
passed to the panel driver.
Note: In cm_t35_init_display(), the gpios were disabled, and the LCD_EN gpio was
enabled after a 50 millisecond delay. This code has been removed and is not
taken care of in the generic panel driver. The impact of this needs to be
tested. The panel's gpios are also not exported any more. Hence, they can't be
accessed via sysfs interface.
Indeed, there is an impact - the LCD no longer works.
The reason for the LCD_EN gpio being pushed high after the 50ms delay,
is to get the LCD out of reset, so the SPI transaction will succeed
and initialize the LCD.
Now, when you remove the gpio handling for the LCD_EN pin,
the LCD no longer works.
So between what is the sleep done? It's not clear from the code. LCD_EN
needs to be 0 for 50ms, or...?
If the panel requires specific reset handling, does it work right even
currently when the panel is turned off and later turned on? The msleep
is only used at boot time.
Okay, so I just realized there's an spi backlight driver used here, and
that backlight driver is actually handling the SPI transactions with the
panel, instead of the panel driver. So this looks quite messed up.
Yep, it always was.
The whole DSS specific panel handling inside the
drivers/video/omap2/displays is a mess.
Those panels can be (and are) used not only with OMAP based boards.
For a quick solution, can we just set the LCD_EN at boot time (with the
msleep), and not touch it after that?
That would be sensible for now, so this series can be merged.
As a more appropriate (and long term) solution,
I plan on moving the panel reset pin handling to the spi backlight
driver itself.
- --
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRHIqCAAoJEBDE8YO64Efamw0P/R2wt0tpCI1ecrnutVGMX4bF
Pyjk2B65uDWqoqZ/cpJUqnvmupXl5UdrA7eqKjTBh1A+g81UVFcNDMuJsbPIIiYI
1pimZieAq0T6Vag00PKImKlkhJYfC7JVBbESij/NONlzYtPkbZ91Y+Ik4DZXnyZf
1TS4GbHQ25tjl73PkwlzLUcJIDIogsimSrkM+aWXPE8LmvrBEQs0LhAObPsAFtgL
1An3hvA2Tkhh9QgerWQd9YiqX994tv1PGRLBEXTbjh1yihzKSNleuvw3NdM+wf9i
9Y5l9IV6L2dtYBMLCpzkiGQBDdOzoq+fObRnSgK6Kr1mfXot+MAlLrk9gCeWcq1b
c2oU/imKWB4sZys21pTnjIxAIzzRDoGW40qXuibTW4DoAYaVHuEBPtphjMVCBCcQ
sJaIVXpsChQ3vvtHOgllnInMjCnlXJ3Piqr4y5glTPxu9mZHdPr6VDpWdqRmvyr9
V7fRQztwXB3Td+SZVDD1yBqoXKlKCX4QPlLAqH3FI9s1WhDHcJePcoDJY0/QyXB1
IeQRlEwBBEZAYy/kr9/pwbZzXeh5V5dK6wAq8aT+thS22zl3nJbKjW//vN06+ib+
WAnHRSZ8iCbUX2cRVF1k+DCQOTi8QCbI6WTcLsgenLeSrbEuzilfgsrsvd6LHfjD
oTODiiD9QInP2sBfknUp
=tWsB
-----END PGP SIGNATURE-----
Why the get_panel_data function is needed, isn't the cast unnecessary?
the 'data' member of omap_dss_device has the type 'void *', we need to
cast it to access the panel_acx565akm_data struct pointer.
You don't need an explicit cast to assign a void pointer to a pointer to
something else (or vice versa, I think).
I remember us having similar constructs in some other panel drivers
also. I think they are unnecessary also.
Tomi
From: Tomi Valkeinen <hidden> Date: 2013-02-14 07:09:03
On 2013-02-14 08:56, Igor Grinberg wrote:
On 02/13/13 17:59, Tomi Valkeinen wrote:
quoted
Okay, so I just realized there's an spi backlight driver used here, and
that backlight driver is actually handling the SPI transactions with the
panel, instead of the panel driver. So this looks quite messed up.
Yep, it always was.
The whole DSS specific panel handling inside the
drivers/video/omap2/displays is a mess.
Well, that's not mess itself, it's just omap specific panel framework.
But dividing single device handling into two separate places is a mess.
Those panels can be (and are) used not only with OMAP based boards.
True, but as there's no generic panel framework, that's the best we can
do. But see CDF (common display framework) discussions if you're
interested in what's hopefully coming soon.
quoted
For a quick solution, can we just set the LCD_EN at boot time (with the
msleep), and not touch it after that?
That would be sensible for now, so this series can be merged.
As a more appropriate (and long term) solution,
I plan on moving the panel reset pin handling to the spi backlight
driver itself.
Well, if you must. But I suggest moving the whole panel handling into a
(omap specific) panel driver, as it's done for other panels. That way
you'll have a proper panel driver for it, for omap, and when CDF comes,
you'll get a platform independent panel driver for it.
Of course, if you have multiple platforms already using that backlight
driver, the omap specific approach may not be enticing. So perhaps it's
easier to just do the quick fix and wait for CDF.
Tomi
Why the get_panel_data function is needed, isn't the cast unnecessary?
the 'data' member of omap_dss_device has the type 'void *', we need to
cast it to access the panel_acx565akm_data struct pointer.
You don't need an explicit cast to assign a void pointer to a pointer to
something else (or vice versa, I think).
I remember us having similar constructs in some other panel drivers
also. I think they are unnecessary also.
Hi,
On Wednesday 13 February 2013 10:16 PM, Tony Lindgren wrote:
* Archit Taneja [off-list ref] [130213 06:26]:
quoted
init functions in omap board files request panel specific gpios, and provide
functions which omapdss panel drivers call to enable or disable them.
Instead of the board files requesting these gpios, they should just pass the
platform specific data(like the gpio numbers), the panel should retrieve the
platform data and request the gpios. Doing this prevents the need of the panel
driver calling platform functions in board files.
Panel drivers have their own platform data struct, and the board files populate
these structs and pass the pointer to the 'data' field of omap_dss_device. This
work will make it easier for the panel drivers be more adaptable to the
DT model.
There is also removal of passing panel reset_gpio numbers through
omap_dss_device struct directly, reset gpios are passed through platform data
only.
To avoid merge conflicts and dependencies between drivers and core
Soc code, please break thes kind of patches into following parts:
1. Any platform_data header changes needed so both I and Tomi
can pull it in as needed.
2. Changes to DSS drivers. Please keep stubs around for the
board specific callback functions so omap2plus_defconfig
won't break with just #1 merged into arm soc tree.
The build won't break, and the kernel will boot up properly, but the
panels won't work till the time #3 is also merged,
3. All the arch/arm/*omap* changes based on #1 above to
drop obsolete callback functions and add new pdata if still
needed. This needs to build and boot on #1 so I can merge
this in via arm soc tree.
4. Any .dts changes needed.
We don't have any .dts changes for DSS as of now.
I'll split the patches accordingly.
Thanks,
Archit
From: Igor Grinberg <hidden> Date: 2013-02-14 08:37:27
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 02/14/13 09:09, Tomi Valkeinen wrote:
On 2013-02-14 08:56, Igor Grinberg wrote:
quoted
On 02/13/13 17:59, Tomi Valkeinen wrote:
quoted
quoted
Okay, so I just realized there's an spi backlight driver used here, and
that backlight driver is actually handling the SPI transactions with the
panel, instead of the panel driver. So this looks quite messed up.
Yep, it always was.
The whole DSS specific panel handling inside the
drivers/video/omap2/displays is a mess.
Well, that's not mess itself, it's just omap specific panel framework.
But dividing single device handling into two separate places is a mess.
Yes, you are right it is not the mess, but it prevents the panel to
be used on other systems and that is BAD.
At the very least, drivers/video/backlight is a generic place that can be
used not just on OMAP.
And since the toppoly was and is used on other systems, why the hell
should anyone duplicate the driver just to please the OMAP specific
panel framework? The real problem is that this framework should not be
OMAP specific...
Of course I'm aware of the fact that currently there is no generic
panel framework, but forging something OMAP specific which is obviously
used on most of the other architectures/platforms (and I mean
panel<->controller relations), is not a good way to go.
Although, I'm also aware of the fact that most things are done this way:
do several specific drivers/frameworks, find the common stuff, and extract
it into a core driver/framework. So I don't want to blame anyone - that's
just the way how we do things, right?
quoted
Those panels can be (and are) used not only with OMAP based boards.
True, but as there's no generic panel framework, that's the best we can
do. But see CDF (common display framework) discussions if you're
interested in what's hopefully coming soon.
Yep, I've seen the CDF discussion and I think this is a good way to go.
quoted
quoted
For a quick solution, can we just set the LCD_EN at boot time (with the
msleep), and not touch it after that?
That would be sensible for now, so this series can be merged.
As a more appropriate (and long term) solution,
I plan on moving the panel reset pin handling to the spi backlight
driver itself.
Well, if you must. But I suggest moving the whole panel handling into a
(omap specific) panel driver, as it's done for other panels. That way
you'll have a proper panel driver for it, for omap, and when CDF comes,
you'll get a platform independent panel driver for it.
You can't just move generic architecture/platform independent stuff
into OMAP specific framework... Just think about this... It's insane.
Of course, if you have multiple platforms already using that backlight
driver, the omap specific approach may not be enticing. So perhaps it's
easier to just do the quick fix and wait for CDF.
That is exactly what I am talking about.
In addition, AFAIR, the reset pin is the property of the toppoly panel
hardware, so that is why I think, we should let the toppoly driver
(currently spi backlight, later hopefully CDF) handle it correctly
along with the spi sequences.
- --
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRHKJGAAoJEBDE8YO64EfacksP/j79jaDbnXpFcwT9KlInp8OE
e+XNi5Vt8zbqhj4gHtxZlN/eIQsVRfuivm9CTp5aJSZHBDAJlPNKobmwjFrDLO9V
RtYTwLAcuWyOdnutIQ52xNwXSntQknd8yxm1qJZMEBjEP+mcQxISWXXMsdxlQEiT
emNtU42W16ZOR34kHUoVfYLkV0v02/JVygt3oaU71+mrNBOt+5L6cHcXaQZPKSes
LUOcyz0qJfzKbnmmZnP/+clTIids83u8rVCNZ1/JoIIlR4rvtNcxRM8Apa8KFJx/
PVT38ds62F0L0qbxL3UmI1uJS2KuEHuJyjYo0uDeQqeeSyz7Q3ZG4TwAJYkWZdWQ
TdFbVrsXbK408FT33VIP4rOzDjqO93IK6f5ld0tZoIvL59NLwgXIejJn6jTNNcU4
p25mUXGSDnaZrNU5cC7d/MzSMt60XQx3UiHjEXD3eJAT33yb+DdBaQwloMCXJQOx
vnseFqhuAzgFHd9LEl47LBg7eXudjaSvWYfJOV0SoB9s7QM8m/YUhnmqmtvdCqZL
fKMJcAjCgm0BG2P6ss79sl6P4XDoBF1LOwSwz4dRmocA3TP7vBNkuRoK08vQe6gv
Qi7hJ05ioa8THt77FxMHtf+ZrO34/L6gHxZqrOD++OgPPdL6qtegemyp4IaKKbUg
q3Mpgsr4ODyStdjEXxTC
=lIHm
-----END PGP SIGNATURE-----
From: Tomi Valkeinen <hidden> Date: 2013-02-14 09:09:07
On 2013-02-14 10:37, Igor Grinberg wrote:
On 02/14/13 09:09, Tomi Valkeinen wrote:
quoted
On 2013-02-14 08:56, Igor Grinberg wrote:
quoted
On 02/13/13 17:59, Tomi Valkeinen wrote:
quoted
quoted
quoted
Okay, so I just realized there's an spi backlight driver used here, and
that backlight driver is actually handling the SPI transactions with the
panel, instead of the panel driver. So this looks quite messed up.
Yep, it always was.
The whole DSS specific panel handling inside the
drivers/video/omap2/displays is a mess.
quoted
Well, that's not mess itself, it's just omap specific panel framework.
But dividing single device handling into two separate places is a mess.
Yes, you are right it is not the mess, but it prevents the panel to
be used on other systems and that is BAD.
At the very least, drivers/video/backlight is a generic place that can be
used not just on OMAP.
True, it's generic, but does it work reliably? The panel hardware is now
partly handled in the backlight driver, and partly in the omap's panel
driver (and wherever on other platforms).
At least currently there's a dependency between the two, as the LCD_EN
gpio is handled by the panel driver, which affects the functioning of
the backlight driver. Is it ensured that the panel driver does not
disable the panel when the backlight driver does spi transactions?
That's what I meant with the mess, it's difficult to make it work
reliably. I know that for some panels such a two-driver approach would
not work at all. Although I guess it's working well enough for you for
this panel.
Thinking about it, if you do move the gpio handling to the backlight
driver, the panel driver will only handle the DPI video stream. Then it
should not have any effect on the SPI side (presuming the panel doesn't
use the pixel clock as func clock), although there's probably still
possibility for display artifacts on enable and disable, if the order of
operations goes the wrong way.
And since the toppoly was and is used on other systems, why the hell
should anyone duplicate the driver just to please the OMAP specific
panel framework? The real problem is that this framework should not be
Not to please. To make it reliable.
OMAP specific...
Of course I'm aware of the fact that currently there is no generic
panel framework, but forging something OMAP specific which is obviously
used on most of the other architectures/platforms (and I mean
panel<->controller relations), is not a good way to go.
Well, if duplicating the code gives us reliable drivers, versus
unreliable without duplicating, then... I don't see it as that bad.
Although, I'm also aware of the fact that most things are done this way:
do several specific drivers/frameworks, find the common stuff, and extract
it into a core driver/framework. So I don't want to blame anyone - that's
just the way how we do things, right?
If it was easy, somebody would've done it.
quoted
quoted
quoted
For a quick solution, can we just set the LCD_EN at boot time (with the
msleep), and not touch it after that?
That would be sensible for now, so this series can be merged.
As a more appropriate (and long term) solution,
I plan on moving the panel reset pin handling to the spi backlight
driver itself.
quoted
Well, if you must. But I suggest moving the whole panel handling into a
(omap specific) panel driver, as it's done for other panels. That way
you'll have a proper panel driver for it, for omap, and when CDF comes,
you'll get a platform independent panel driver for it.
You can't just move generic architecture/platform independent stuff
into OMAP specific framework... Just think about this... It's insane.
As I said, reliable vs unreliable. That's not insane.
But again, if you can handle this particular panel reliably with the
two-driver approach, I'm fine with it.
In addition, AFAIR, the reset pin is the property of the toppoly panel
hardware, so that is why I think, we should let the toppoly driver
(currently spi backlight, later hopefully CDF) handle it correctly
along with the spi sequences.
From: Igor Grinberg <hidden> Date: 2013-02-14 09:43:15
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 02/14/13 11:09, Tomi Valkeinen wrote:
On 2013-02-14 10:37, Igor Grinberg wrote:
quoted
On 02/14/13 09:09, Tomi Valkeinen wrote:
quoted
On 2013-02-14 08:56, Igor Grinberg wrote:
quoted
On 02/13/13 17:59, Tomi Valkeinen wrote:
quoted
quoted
quoted
Okay, so I just realized there's an spi backlight driver used here, and
that backlight driver is actually handling the SPI transactions with the
panel, instead of the panel driver. So this looks quite messed up.
Yep, it always was.
The whole DSS specific panel handling inside the
drivers/video/omap2/displays is a mess.
quoted
Well, that's not mess itself, it's just omap specific panel framework.
But dividing single device handling into two separate places is a mess.
Yes, you are right it is not the mess, but it prevents the panel to
be used on other systems and that is BAD.
At the very least, drivers/video/backlight is a generic place that can be
used not just on OMAP.
True, it's generic, but does it work reliably? The panel hardware is now
partly handled in the backlight driver, and partly in the omap's panel
driver (and wherever on other platforms).
It works reliably on other platforms, but not on OMAP - because
we need to cope with the OMAP specific framework...
At least currently there's a dependency between the two, as the LCD_EN
gpio is handled by the panel driver, which affects the functioning of
the backlight driver. Is it ensured that the panel driver does not
disable the panel when the backlight driver does spi transactions?
That's what I meant with the mess, it's difficult to make it work
reliably. I know that for some panels such a two-driver approach would
not work at all. Although I guess it's working well enough for you for
this panel.
Yep, that is correct - this is the mess.
Thinking about it, if you do move the gpio handling to the backlight
driver, the panel driver will only handle the DPI video stream. Then it
should not have any effect on the SPI side (presuming the panel doesn't
use the pixel clock as func clock), although there's probably still
possibility for display artifacts on enable and disable, if the order of
operations goes the wrong way.
Yep, again, that is correct.
quoted
And since the toppoly was and is used on other systems, why the hell
should anyone duplicate the driver just to please the OMAP specific
panel framework? The real problem is that this framework should not be
Not to please. To make it reliable.
Well, there are multiple ways to make it reliable.
And I don't think that the best would be: make it OMAP specific.
quoted
OMAP specific...
Of course I'm aware of the fact that currently there is no generic
panel framework, but forging something OMAP specific which is obviously
used on most of the other architectures/platforms (and I mean
panel<->controller relations), is not a good way to go.
Well, if duplicating the code gives us reliable drivers, versus
unreliable without duplicating, then... I don't see it as that bad.
Hmmm... I don't think this fits the mainline (upstream) philosophy.
This can be also extrapolated into: let's make our own Linux ARM fork
so it will be more reliable...
This is the way how vendor specific kernel releases work.
quoted
Although, I'm also aware of the fact that most things are done this way:
do several specific drivers/frameworks, find the common stuff, and extract
it into a core driver/framework. So I don't want to blame anyone - that's
just the way how we do things, right?
If it was easy, somebody would've done it.
In fact this is done all the time on multiple drivers and frameworks.
Also, I don't say this is easy, but I also don't think this too hard.
It is also a function of resources (time/will/experience/etc.).
quoted
quoted
quoted
quoted
For a quick solution, can we just set the LCD_EN at boot time (with the
msleep), and not touch it after that?
That would be sensible for now, so this series can be merged.
As a more appropriate (and long term) solution,
I plan on moving the panel reset pin handling to the spi backlight
driver itself.
quoted
Well, if you must. But I suggest moving the whole panel handling into a
(omap specific) panel driver, as it's done for other panels. That way
you'll have a proper panel driver for it, for omap, and when CDF comes,
you'll get a platform independent panel driver for it.
You can't just move generic architecture/platform independent stuff
into OMAP specific framework... Just think about this... It's insane.
As I said, reliable vs unreliable. That's not insane.
But again, if you can handle this particular panel reliably with the
two-driver approach, I'm fine with it.
Again, it works reliably on other platforms,
why would OMAP be an exception?
quoted
In addition, AFAIR, the reset pin is the property of the toppoly panel
hardware, so that is why I think, we should let the toppoly driver
(currently spi backlight, later hopefully CDF) handle it correctly
along with the spi sequences.
From: Tomi Valkeinen <hidden> Date: 2013-02-14 10:59:08
On 2013-02-14 11:43, Igor Grinberg wrote:
quoted
True, it's generic, but does it work reliably? The panel hardware is now
partly handled in the backlight driver, and partly in the omap's panel
driver (and wherever on other platforms).
It works reliably on other platforms, but not on OMAP - because
we need to cope with the OMAP specific framework...
How do you handle the gpios on other platform? Those are the ones
causing the issues here, right?
Or is there something else with OMAP DSS that you need to specifically
cope with?
quoted
Thinking about it, if you do move the gpio handling to the backlight
driver, the panel driver will only handle the DPI video stream. Then it
should not have any effect on the SPI side (presuming the panel doesn't
use the pixel clock as func clock), although there's probably still
possibility for display artifacts on enable and disable, if the order of
operations goes the wrong way.
Yep, again, that is correct.
It's correct that there may be artifacts? How do you manage the ordering
of the operations on other platforms?
quoted
quoted
And since the toppoly was and is used on other systems, why the hell
should anyone duplicate the driver just to please the OMAP specific
panel framework? The real problem is that this framework should not be
quoted
Not to please. To make it reliable.
Well, there are multiple ways to make it reliable.
And I don't think that the best would be: make it OMAP specific.
I'm not saying it's the best option. I'm saying it's a realistic option
to get it working.
quoted
Well, if duplicating the code gives us reliable drivers, versus
unreliable without duplicating, then... I don't see it as that bad.
Hmmm... I don't think this fits the mainline (upstream) philosophy.
This can be also extrapolated into: let's make our own Linux ARM fork
so it will be more reliable...
This is the way how vendor specific kernel releases work.
Well, we are talking about a smallish driver here. Not an arch fork. If
the options are a) platform specific driver that works, or b) generic
driver that's not reliable, or c) no driver at all, I can't really see
why a) would be such a horrible option for the time being.
But this discussion is getting a bit out of hand. It sounds to me that
for this panel in question we can manage with the current approach, so
this whole line of discussion doesn't matter for this specific problem.
quoted
If it was easy, somebody would've done it.
In fact this is done all the time on multiple drivers and frameworks.
Also, I don't say this is easy, but I also don't think this too hard.
It is also a function of resources (time/will/experience/etc.).
I think the CDF discussions have already proven that it is quite hard.
But feel free to contribute.
And I've talked about a common display framework already years ago, and
I've tried to design OMAP DSS panels from start in such a way that they
try to depend on OMAP DSS features as little as possible, to make it
easier to generalize them. Just to prove I'm not indifferent about the
issue =).
Tomi
From: Igor Grinberg <hidden> Date: 2013-02-14 12:37:37
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 02/14/13 12:59, Tomi Valkeinen wrote:
On 2013-02-14 11:43, Igor Grinberg wrote:
quoted
quoted
True, it's generic, but does it work reliably? The panel hardware is now
partly handled in the backlight driver, and partly in the omap's panel
driver (and wherever on other platforms).
It works reliably on other platforms, but not on OMAP - because
we need to cope with the OMAP specific framework...
How do you handle the gpios on other platform? Those are the ones
causing the issues here, right?
Well, I'm also talking about something that is a history already.
Remember, we had multiple panel drivers inside the
video/omap2/displays and then they were consolidated into the
"generic dpi/dsi/whatever".
And yes you are right, on the platforms I'm aware of, the GPIO is not
handled. Apparently its hardware default (pull resistor) is always on...
Or is there something else with OMAP DSS that you need to specifically
cope with?
The fact there is a need to create a OMAP specific driver.
I'm not talking about the generic driver which only needs to have the
controller specific data (e.g. porches, pixel clock, bus width).
The generic driver was one of the good ways to go.
quoted
quoted
Thinking about it, if you do move the gpio handling to the backlight
driver, the panel driver will only handle the DPI video stream. Then it
should not have any effect on the SPI side (presuming the panel doesn't
use the pixel clock as func clock), although there's probably still
possibility for display artifacts on enable and disable, if the order of
operations goes the wrong way.
Yep, again, that is correct.
It's correct that there may be artifacts? How do you manage the ordering
of the operations on other platforms?
Yep, there might be artifacts if the ordering is incorrect.
The ordering is something that should be solved by the CDF.
quoted
quoted
quoted
And since the toppoly was and is used on other systems, why the hell
should anyone duplicate the driver just to please the OMAP specific
panel framework? The real problem is that this framework should not be
quoted
Not to please. To make it reliable.
Well, there are multiple ways to make it reliable.
And I don't think that the best would be: make it OMAP specific.
I'm not saying it's the best option. I'm saying it's a realistic option
to get it working.
quoted
quoted
Well, if duplicating the code gives us reliable drivers, versus
unreliable without duplicating, then... I don't see it as that bad.
Hmmm... I don't think this fits the mainline (upstream) philosophy.
This can be also extrapolated into: let's make our own Linux ARM fork
so it will be more reliable...
This is the way how vendor specific kernel releases work.
Well, we are talking about a smallish driver here. Not an arch fork. If
the options are a) platform specific driver that works, or b) generic
driver that's not reliable, or c) no driver at all, I can't really see
why a) would be such a horrible option for the time being.
I think there is b.5) where the driver is both generic and reliable,
but I haven't looked into this deep enough.
But this discussion is getting a bit out of hand. It sounds to me that
for this panel in question we can manage with the current approach, so
this whole line of discussion doesn't matter for this specific problem.
quoted
quoted
If it was easy, somebody would've done it.
In fact this is done all the time on multiple drivers and frameworks.
Also, I don't say this is easy, but I also don't think this too hard.
It is also a function of resources (time/will/experience/etc.).
I think the CDF discussions have already proven that it is quite hard.
But feel free to contribute.
I will contribute as much as I can in terms of my paid work.
I always do...
And I've talked about a common display framework already years ago, and
I've tried to design OMAP DSS panels from start in such a way that they
try to depend on OMAP DSS features as little as possible, to make it
easier to generalize them. Just to prove I'm not indifferent about the
issue =).
Yep, your work was fruitful and no one can take it from you!
That's why I said that I'm not trying to blame or accuse anyone.
That is more of a wish that the CDF will come as quickly as it can.
Because currently it is clear that there are cases where we don't
have a proper support...
- --
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRHNqRAAoJEBDE8YO64Efac08QAKBgU7xFkdBHU0ekAgRIafhO
kd0EgPXwPZDvuAaoj5pzdMI65alRSuoQxp5ohHyz0aFfGRk0Q66f6/zXzTsGXPLP
pRTkqGGTHD5Qtv2IvhHPvLMRGW+87QiuK0NU0BJqV/1JRPz1UyKMJrwc5U3Acf4J
B7bQnWLlIcPftIt2zHwuvZZMYZUL8f5/dXGWMhxmTF7rse7YcTjCwQO4eabTIX5r
/GUanF+qlwKJzk7nxQR1DFQdN+7Vv8vdumJi38sq9fouLgXlhmzOgXvxMgT7lq8T
l+7Bg7l+E9yrHaXmWf3zjZBAk6/wBnzzrmmK5V6Gjg2PiEh6Rs5ARWPrdc/FQCpt
4bXTKEktLonHj2rya8intrPLw7lP4AQN81QsQOLeRIPGoSCcDsi4RGAFrWdXR0Xt
Du9ea/liAD3+qiNIErQcTlR5zDKAIhMvWycG+ZFj0kVFUFb6p/F1TKcqVy0O7HAG
SLSsrCyO8jA/UxAKQifhiUU2Hoq9a3VhQi507lbXiN0NTsk686mp/W8YJPB/di+v
44wayaE5Kyw5iawLj9WUyTpw29sOysiewp+z7XuaF3fM5lWeMy2e0/mNbe4iCcCG
LYMl0NWcQn0S7okkQz5HHmCJp3L3/Se5sMfGKJRzpG0vK1DR+m8nJh0MGbEu/14C
B9+3VrImRZS8te/wnPqE
ºnK
-----END PGP SIGNATURE-----
From: Aaro Koskinen <aaro.koskinen@iki.fi> Date: 2013-02-14 12:45:38
On Thu, Feb 14, 2013 at 12:04:32PM +0530, Archit Taneja wrote:
On Wednesday 13 February 2013 11:05 PM, Aaro Koskinen wrote:
quoted
On Wed, Feb 13, 2013 at 07:52:19PM +0530, Archit Taneja wrote:
quoted
@@ -444,6 +445,20 @@ static int n8x0_panel_probe(struct omap_dss_device *dssdev) dssdev->ctrl.rfbi_timings = n8x0_panel_timings; dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;+ if (gpio_is_valid(bdata->panel_reset)) {+ r = devm_gpio_request_one(&dssdev->dev, bdata->panel_reset,+ GPIOF_OUT_INIT_LOW, "PANEL RESET");+ if (r)+ return r;+ }++ if (gpio_is_valid(bdata->ctrl_pwrdown)) {+ r = devm_gpio_request_one(&dssdev->dev, bdata->ctrl_pwrdown,+ GPIOF_OUT_INIT_LOW, "PANEL PWRDOWN");+ if (r)+ return r;+ }+
In the error case, the other GPIO is not freed. Also maybe you should
free them on module removal, because now the module owns the GPIOs.
Wouldn't the usage of devm_* functions take care of this? If the
device isn't registered successfully, then all allocations/requests
done using devm_* functions will be free'd automatically.
Sorry, I didn't realized they are devm_* now. You are right.
A.
From: Tomi Valkeinen <hidden> Date: 2013-02-14 12:52:34
On 2013-02-14 14:37, Igor Grinberg wrote:
On 02/14/13 12:59, Tomi Valkeinen wrote:
quoted
On 2013-02-14 11:43, Igor Grinberg wrote:
quoted
quoted
quoted
True, it's generic, but does it work reliably? The panel hardware is now
partly handled in the backlight driver, and partly in the omap's panel
driver (and wherever on other platforms).
It works reliably on other platforms, but not on OMAP - because
we need to cope with the OMAP specific framework...
quoted
How do you handle the gpios on other platform? Those are the ones
causing the issues here, right?
Well, I'm also talking about something that is a history already.
Remember, we had multiple panel drivers inside the
video/omap2/displays and then they were consolidated into the
"generic dpi/dsi/whatever".
Sorry, I miss the point. Was that a bad thing? Didn't it simplify the
task for you with simple panels? It could've been taken even further,
though (see below).
And yes you are right, on the platforms I'm aware of, the GPIO is not
handled. Apparently its hardware default (pull resistor) is always on...
Ok, so the simple fix of setting the GPIOs only in the board file is
acceptable for now.
Can the LCD_BL_GPIO be handled by the omap panel driver? Otherwise the
backlight will supposedly be always on. Is it just a simple switch for
the BL power, which does not affect the SPI in any way?
quoted
Or is there something else with OMAP DSS that you need to specifically
cope with?
The fact there is a need to create a OMAP specific driver.
I'm not talking about the generic driver which only needs to have the
controller specific data (e.g. porches, pixel clock, bus width).
The generic driver was one of the good ways to go.
Well, we could also have an even more generic driver that takes the
video timings from the board file as platform data. Then all you would
need to do is to define the timings in the board file, as I think is
done for other platforms also.
I'm not very fond of that idea, as I think hardcoded device specific
data should not be given as parameters, but they should be handled by
the device driver internally, as it should know that device specific
data already.
But, in practice, making this kind of even more generic panel driver
will probably make life easier for everyone, so I think we'll have one
with CDF.
Tomi
From: Igor Grinberg <hidden> Date: 2013-02-14 13:51:00
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 02/14/13 14:52, Tomi Valkeinen wrote:
On 2013-02-14 14:37, Igor Grinberg wrote:
quoted
On 02/14/13 12:59, Tomi Valkeinen wrote:
quoted
On 2013-02-14 11:43, Igor Grinberg wrote:
quoted
quoted
quoted
True, it's generic, but does it work reliably? The panel hardware is now
partly handled in the backlight driver, and partly in the omap's panel
driver (and wherever on other platforms).
It works reliably on other platforms, but not on OMAP - because
we need to cope with the OMAP specific framework...
quoted
How do you handle the gpios on other platform? Those are the ones
causing the issues here, right?
Well, I'm also talking about something that is a history already.
Remember, we had multiple panel drivers inside the
video/omap2/displays and then they were consolidated into the
"generic dpi/dsi/whatever".
Sorry, I miss the point. Was that a bad thing? Didn't it simplify the
task for you with simple panels? It could've been taken even further,
though (see below).
Yes it was a good thing (I have already told this below).
quoted
And yes you are right, on the platforms I'm aware of, the GPIO is not
handled. Apparently its hardware default (pull resistor) is always on...
Ok, so the simple fix of setting the GPIOs only in the board file is
acceptable for now.
Yep. I also told this already in one of the previous emails.
Can the LCD_BL_GPIO be handled by the omap panel driver? Otherwise the
backlight will supposedly be always on. Is it just a simple switch for
the BL power, which does not affect the SPI in any way?
Yes, it can for now.
Also, I think we should also take into account the backlight framework,
including PMW.
quoted
quoted
Or is there something else with OMAP DSS that you need to specifically
cope with?
The fact there is a need to create a OMAP specific driver.
I'm not talking about the generic driver which only needs to have the
controller specific data (e.g. porches, pixel clock, bus width).
The generic driver was one of the good ways to go.
Well, we could also have an even more generic driver that takes the
video timings from the board file as platform data. Then all you would
need to do is to define the timings in the board file, as I think is
done for other platforms also.
Yep. That sounds reasonable also for cases where the bus width is the
hardware (board) property.
I'm not very fond of that idea, as I think hardcoded device specific
data should not be given as parameters, but they should be handled by
the device driver internally, as it should know that device specific
data already.
Yes, that is why I think the generic driver for "simple" panels
was a good idea. There was some flexibility missing though.
For example the resolution setting which in turn drags another set
of timings and pixel clock.
There are panels that support more than one resolution.
But, in practice, making this kind of even more generic panel driver
will probably make life easier for everyone, so I think we'll have one
with CDF.
I'm looking forward to see it happening!
- --
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRHOvEAAoJEBDE8YO64EfaCZ0P/32SYC7MZRJMfUVrnzZtJZpn
9BKzrvO0h/GKZMbKoKKNFwmvlTxEocELLkljF35ipbc51C/dpD45ZmMBvu4s8owE
6bopGw6ssTahTKk0zY5PekTamZT7UlY86hI9ZcZBxSzY8xaj7UCSPnJ3qzY2ZGzA
4heJW01mlHr9i1qkOzxz0IHA2CdQmQltAsW12NFCWYBRpDfhrYtECwhlnvLXHEzy
nqXNpRHOnrL/hqvJwor1X+D/O1JlyxUiVr6+7sAZqXxvv/iMX8Nc1XeTObgGbse6
1bpipPD57etqISsN1g9ur1tU5f6KvoR4IA35RaCCkBFINIXMEBl7kr5X9Bcg3giE
3pz23k91KRloOcXJ0OvLHp7RnSrwswU/4C3Cse+95cY0wyChaVlwJtySEnfGALWV
aiuw89v6DXaC5WDQq55UtTc3qjc23Ehut8i184PAv5HKrDHLLjVg4HqgGjC9uGEn
JKOxOnfMTstqyKC2whW+Pep3g4npJAHsn/0QmpdSJQ/vEwm7CmXBZeR6DwcTSLAT
xR1SUWwHav2HqpYUz4y7TgIPuwsR+VNKn/mSOTbhbLB4s9bowPMxMiqz2AQn3ige
YyMTo7KnObivXZydrVrx0/e/DJC4ENGpE3macVQytr0BpjJhK1+XiNMHa17+Mymm
U45VDKrUgaYcYXW2L4JN
=xeSz
-----END PGP SIGNATURE-----
Why the get_panel_data function is needed, isn't the cast unnecessary?
the 'data' member of omap_dss_device has the type 'void *', we need to
cast it to access the panel_acx565akm_data struct pointer.
You don't need an explicit cast to assign a void pointer to a pointer to
something else (or vice versa, I think).
I remember us having similar constructs in some other panel drivers
also. I think they are unnecessary also.
I was considering keeping the get_panel_data() funcs in the panel
drivers though. This way, whenever the way of retrieving platform data
changes because of DT or CDF or something else, we would just need to
modify the get_panel_data func.
Archit
Why the get_panel_data function is needed, isn't the cast unnecessary?
the 'data' member of omap_dss_device has the type 'void *', we need to
cast it to access the panel_acx565akm_data struct pointer.
You don't need an explicit cast to assign a void pointer to a pointer to
something else (or vice versa, I think).
I remember us having similar constructs in some other panel drivers
also. I think they are unnecessary also.
I was considering keeping the get_panel_data() funcs in the panel
drivers though. This way, whenever the way of retrieving platform data
changes because of DT or CDF or something else, we would just need to
modify the get_panel_data func.
I think it's simpler if we manage the fetching of the platform data in
probe one, and just store a struct panel_acx565akm_data or such to the
panel driver's data.
For DT we don't have platform data, but we can create the same platform
data struct from the DT properties.
So the above would become something like:
struct acx565akm_device *md = &acx_dev;
struct panel_acx565akm_data *panel_data = md->pdata;
Tomi
Why the get_panel_data function is needed, isn't the cast unnecessary?
the 'data' member of omap_dss_device has the type 'void *', we need to
cast it to access the panel_acx565akm_data struct pointer.
You don't need an explicit cast to assign a void pointer to a pointer to
something else (or vice versa, I think).
I remember us having similar constructs in some other panel drivers
also. I think they are unnecessary also.
I was considering keeping the get_panel_data() funcs in the panel
drivers though. This way, whenever the way of retrieving platform data
changes because of DT or CDF or something else, we would just need to
modify the get_panel_data func.
I think it's simpler if we manage the fetching of the platform data in
probe one, and just store a struct panel_acx565akm_data or such to the
panel driver's data.
For DT we don't have platform data, but we can create the same platform
data struct from the DT properties.
So the above would become something like:
struct acx565akm_device *md = &acx_dev;
struct panel_acx565akm_data *panel_data = md->pdata;
From: Tomi Valkeinen <hidden> Date: 2013-04-03 12:02:02
On 2013-02-14 15:51, Igor Grinberg wrote:
On 02/14/13 14:52, Tomi Valkeinen wrote:
quoted
On 2013-02-14 14:37, Igor Grinberg wrote:
quoted
On 02/14/13 12:59, Tomi Valkeinen wrote:
quoted
On 2013-02-14 11:43, Igor Grinberg wrote:
quoted
quoted
quoted
True, it's generic, but does it work reliably? The panel hardware is now
partly handled in the backlight driver, and partly in the omap's panel
driver (and wherever on other platforms).
It works reliably on other platforms, but not on OMAP - because
we need to cope with the OMAP specific framework...
quoted
How do you handle the gpios on other platform? Those are the ones
causing the issues here, right?
Well, I'm also talking about something that is a history already.
Remember, we had multiple panel drivers inside the
video/omap2/displays and then they were consolidated into the
"generic dpi/dsi/whatever".
quoted
Sorry, I miss the point. Was that a bad thing? Didn't it simplify the
task for you with simple panels? It could've been taken even further,
though (see below).
Yes it was a good thing (I have already told this below).
quoted
quoted
And yes you are right, on the platforms I'm aware of, the GPIO is not
handled. Apparently its hardware default (pull resistor) is always on...
quoted
Ok, so the simple fix of setting the GPIOs only in the board file is
acceptable for now.
Yep. I also told this already in one of the previous emails.
quoted
Can the LCD_BL_GPIO be handled by the omap panel driver? Otherwise the
backlight will supposedly be always on. Is it just a simple switch for
the BL power, which does not affect the SPI in any way?
Yes, it can for now.
Also, I think we should also take into account the backlight framework,
including PMW.
I've updated this patch to set the LCD EN gpio once at boot time, and pass the
LCD BL gpio to the panel driver. Updated patch below.
---
commit a58a72363aa4359cdb75878de1517bd50faf9eb4
Author: Tomi Valkeinen [off-list ref]
Date: Mon Dec 3 16:05:06 2012 +0530
arm: omap: board-cm-t35: use generic dpi panel's gpio handling
The cm-t35 board file currently requests gpios required to configure the tdo35s
panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the generic dpi panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to generic dpi panel's platform data so that it's
passed to the panel driver.
Note: Only BL enable gpio is handled in the panel driver. The LCD enable
GPIO is handled in the board file at init time, as there's a 50 ms delay
required when using the GPIO, and the panel driver doesn't know about
that.
Cc: Tony Lindgren [off-list ref]
Cc: Igor Grinberg [off-list ref]
Signed-off-by: Tomi Valkeinen [off-list ref]
From: Tomi Valkeinen <hidden> Date: 2013-04-03 12:28:08
Hi Tony,
On 2013-02-13 18:46, Tony Lindgren wrote:
* Archit Taneja [off-list ref] [130213 06:26]:
quoted
init functions in omap board files request panel specific gpios, and provide
functions which omapdss panel drivers call to enable or disable them.
Instead of the board files requesting these gpios, they should just pass the
platform specific data(like the gpio numbers), the panel should retrieve the
platform data and request the gpios. Doing this prevents the need of the panel
driver calling platform functions in board files.
Panel drivers have their own platform data struct, and the board files populate
these structs and pass the pointer to the 'data' field of omap_dss_device. This
work will make it easier for the panel drivers be more adaptable to the
DT model.
There is also removal of passing panel reset_gpio numbers through
omap_dss_device struct directly, reset gpios are passed through platform data
only.
To avoid merge conflicts and dependencies between drivers and core
Soc code, please break thes kind of patches into following parts:
1. Any platform_data header changes needed so both I and Tomi
can pull it in as needed.
2. Changes to DSS drivers. Please keep stubs around for the
board specific callback functions so omap2plus_defconfig
won't break with just #1 merged into arm soc tree.
3. All the arch/arm/*omap* changes based on #1 above to
drop obsolete callback functions and add new pdata if still
needed. This needs to build and boot on #1 so I can merge
this in via arm soc tree.
4. Any .dts changes needed.
Tony, I've split these patches as follows:
Platform data header file changes:
git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
Board file changes (based on header changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
DSS panel changes (based on header changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10/1-panel-cleanup
Removing unused fields from header files (based on panel changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10/2-late-panel-cleanup
The 2-late-panel-cleanup breaks compilation if the arch changes are not
merged, so I'll leave that until they have been merged.
Do you mind if I add the board-cleanup branch temporarily to omapdss's
for-next, to simplify testing? When everything looks ok, I'll remove it
and pass the branch to you to be handled through l-o.
Tomi
From: Tony Lindgren <tony@atomide.com> Date: 2013-04-03 15:46:45
* Tomi Valkeinen [off-list ref] [130403 05:32]:
Hi Tony,
On 2013-02-13 18:46, Tony Lindgren wrote:
quoted
* Archit Taneja [off-list ref] [130213 06:26]:
quoted
init functions in omap board files request panel specific gpios, and provide
functions which omapdss panel drivers call to enable or disable them.
Instead of the board files requesting these gpios, they should just pass the
platform specific data(like the gpio numbers), the panel should retrieve the
platform data and request the gpios. Doing this prevents the need of the panel
driver calling platform functions in board files.
Panel drivers have their own platform data struct, and the board files populate
these structs and pass the pointer to the 'data' field of omap_dss_device. This
work will make it easier for the panel drivers be more adaptable to the
DT model.
There is also removal of passing panel reset_gpio numbers through
omap_dss_device struct directly, reset gpios are passed through platform data
only.
To avoid merge conflicts and dependencies between drivers and core
Soc code, please break thes kind of patches into following parts:
1. Any platform_data header changes needed so both I and Tomi
can pull it in as needed.
2. Changes to DSS drivers. Please keep stubs around for the
board specific callback functions so omap2plus_defconfig
won't break with just #1 merged into arm soc tree.
3. All the arch/arm/*omap* changes based on #1 above to
drop obsolete callback functions and add new pdata if still
needed. This needs to build and boot on #1 so I can merge
this in via arm soc tree.
4. Any .dts changes needed.
Tony, I've split these patches as follows:
Platform data header file changes:
git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
Board file changes (based on header changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
DSS panel changes (based on header changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10/1-panel-cleanup
Removing unused fields from header files (based on panel changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10/2-late-panel-cleanup
The 2-late-panel-cleanup breaks compilation if the arch changes are not
merged, so I'll leave that until they have been merged.
Do you mind if I add the board-cleanup branch temporarily to omapdss's
for-next, to simplify testing? When everything looks ok, I'll remove it
and pass the branch to you to be handled through l-o.
Sure please go ahead. There are still some board-*.c related patches
that I have not merged, but we'll see those merge conflicts before
next as I usually do a merge with next before sending out pull reqs.
Regards,
Tony
From: Igor Grinberg <hidden> Date: 2013-04-04 07:17:01
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
[...]
quoted
quoted
Can the LCD_BL_GPIO be handled by the omap panel driver? Otherwise the
backlight will supposedly be always on. Is it just a simple switch for
the BL power, which does not affect the SPI in any way?
Yes, it can for now.
Also, I think we should also take into account the backlight framework,
including PMW.
I've updated this patch to set the LCD EN gpio once at boot time, and pass the
LCD BL gpio to the panel driver. Updated patch below.
---
commit a58a72363aa4359cdb75878de1517bd50faf9eb4
Author: Tomi Valkeinen [off-list ref]
Date: Mon Dec 3 16:05:06 2012 +0530
arm: omap: board-cm-t35: use generic dpi panel's gpio handling
The cm-t35 board file currently requests gpios required to configure the tdo35s
panel, and provides platform_enable/disable callbacks to configure them.
These tasks have been moved to the generic dpi panel driver itself and shouldn't
be done in the board files.
Remove the gpio requests and the platform callbacks from the board file.
Add the gpio information to generic dpi panel's platform data so that it's
passed to the panel driver.
Note: Only BL enable gpio is handled in the panel driver. The LCD enable
GPIO is handled in the board file at init time, as there's a 50 ms delay
required when using the GPIO, and the panel driver doesn't know about
that.
Cc: Tony Lindgren [off-list ref]
Cc: Igor Grinberg [off-list ref]
Signed-off-by: Tomi Valkeinen [off-list ref]
Looks good, thanks!
Acked-by: Igor Grinberg <redacted>
From: Tomi Valkeinen <hidden> Date: 2013-04-15 09:29:59
Hi Tony,
On 2013-04-03 18:46, Tony Lindgren wrote:
quoted
Tony, I've split these patches as follows:
Platform data header file changes:
git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
Board file changes (based on header changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
DSS panel changes (based on header changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10/1-panel-cleanup
Removing unused fields from header files (based on panel changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10/2-late-panel-cleanup
The 2-late-panel-cleanup breaks compilation if the arch changes are not
merged, so I'll leave that until they have been merged.
Do you mind if I add the board-cleanup branch temporarily to omapdss's
for-next, to simplify testing? When everything looks ok, I'll remove it
and pass the branch to you to be handled through l-o.
Sure please go ahead. There are still some board-*.c related patches
that I have not merged, but we'll see those merge conflicts before
next as I usually do a merge with next before sending out pull reqs.
The code seems to work ok, so I think we can declare the branches
stable. I've pushed new for-next branch that does not contain the arch
files anymore. In fact, I removed all omapdss patches also, as omapdss
should be merged through the drm tree, and I don't want to have
conflicts with drm's for-next branch.
So, to recap, the common header changes are located in:
git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
And the branch for linux-omap is:
git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
After merging those, some displays won't start anymore until the omapdss
changes are in, but things should still compile.
Tomi
From: Tony Lindgren <tony@atomide.com> Date: 2013-04-15 21:20:22
* Tomi Valkeinen [off-list ref] [130415 02:34]:
Hi Tony,
On 2013-04-03 18:46, Tony Lindgren wrote:
quoted
quoted
Tony, I've split these patches as follows:
Platform data header file changes:
git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
Board file changes (based on header changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
DSS panel changes (based on header changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10/1-panel-cleanup
Removing unused fields from header files (based on panel changes):
git://gitorious.org/linux-omap-dss2/linux.git 3.10/2-late-panel-cleanup
The 2-late-panel-cleanup breaks compilation if the arch changes are not
merged, so I'll leave that until they have been merged.
Do you mind if I add the board-cleanup branch temporarily to omapdss's
for-next, to simplify testing? When everything looks ok, I'll remove it
and pass the branch to you to be handled through l-o.
Sure please go ahead. There are still some board-*.c related patches
that I have not merged, but we'll see those merge conflicts before
next as I usually do a merge with next before sending out pull reqs.
The code seems to work ok, so I think we can declare the branches
stable. I've pushed new for-next branch that does not contain the arch
files anymore. In fact, I removed all omapdss patches also, as omapdss
should be merged through the drm tree, and I don't want to have
conflicts with drm's for-next branch.
So, to recap, the common header changes are located in:
git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
And the branch for linux-omap is:
git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
After merging those, some displays won't start anymore until the omapdss
changes are in, but things should still compile.
Sounds like it's best that you merge those branches via your
tree as the conflicts have been already resolved in linux next.
Regards,
Tony
From: Tomi Valkeinen <hidden> Date: 2013-04-16 04:20:10
On 2013-04-16 00:20, Tony Lindgren wrote:
quoted
So, to recap, the common header changes are located in:
git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
And the branch for linux-omap is:
git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
After merging those, some displays won't start anymore until the omapdss
changes are in, but things should still compile.
Sounds like it's best that you merge those branches via your
tree as the conflicts have been already resolved in linux next.
The dss changes are going through drm tree this time, as there are some
drm dependencies also, and I've already sent the pull request for those.
And when I asked Dave Airlie if he can merge the dss changes, I didn't
talk about a big chunk of arch file changes getting included.
Also, the whole division to two independent branches was done only to
make it possible for the arch changes to go through l-o tree.
There will probably be more these kind of changes in the future, so I
think we should agree how to handle those and stick to the plan.
Dividing the arch file changes to a separate branch is often quite
laborious, and I'd rather not do that for nothing.
Tomi
From: Tony Lindgren <tony@atomide.com> Date: 2013-04-18 00:34:20
* Tomi Valkeinen [off-list ref] [130415 21:25]:
On 2013-04-16 00:20, Tony Lindgren wrote:
quoted
quoted
So, to recap, the common header changes are located in:
git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
And the branch for linux-omap is:
git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
After merging those, some displays won't start anymore until the omapdss
changes are in, but things should still compile.
Sounds like it's best that you merge those branches via your
tree as the conflicts have been already resolved in linux next.
The dss changes are going through drm tree this time, as there are some
drm dependencies also, and I've already sent the pull request for those.
And when I asked Dave Airlie if he can merge the dss changes, I didn't
talk about a big chunk of arch file changes getting included.
OK sorry I did not know that and was assuming you will be sending
a pull request to Linus.
Also, the whole division to two independent branches was done only to
make it possible for the arch changes to go through l-o tree.
Yup. That probably caused you to fix up few other things while doing
it though ;)
There will probably be more these kind of changes in the future, so I
think we should agree how to handle those and stick to the plan.
Dividing the arch file changes to a separate branch is often quite
laborious, and I'd rather not do that for nothing.
Thanks for doing all that. And yes, let's plan on always separating
driver changes from arch/arm changes to cut away the dependencies.
Just one request: Let's do branches like this early on before -rc6,
not what might be five days before the merge window potentially
opens..
I've pulled them into omap-for-v3.10/dss and will send a pull
request for Arnd and Olof.
Regards,
Tony
From: Tomi Valkeinen <hidden> Date: 2013-04-18 03:40:08
On 2013-04-18 03:34, Tony Lindgren wrote:
Just one request: Let's do branches like this early on before -rc6,
not what might be five days before the merge window potentially
opens..
Agreed, it got a bit late. But the branch itself has been stable and in
linux-next for some time.
Perhaps it would've been better to merge it to l-o earlier, instead of
me pushing it to linux-next via my tree. Any possible found problems
(there weren't any this time) could've been fixed with a new fixes branch.
I've pulled them into omap-for-v3.10/dss and will send a pull
request for Arnd and Olof.