RE: [PATCH 3/3] OMAP: use generic panel driver in board files

2010-11-03 Thread Taneja, Archit
Hi,

linux-omap-ow...@vger.kernel.org wrote:
 Introduce some fields in struct panel, which will be used to
 match the right panel configurations in generic panel driver.
 
 Still keep sharp_ls_panel, since the sharp_ls_panel driver
 contains blacklight control driver code which will be moved
 out later. Then we can use generic driver for sharp_ls_panel.
 
 Signed-off-by: Bryan Wu bryan...@canonical.com

[snip]

 diff --git a/arch/arm/plat-omap/include/plat/display.h
 b/arch/arm/plat-omap/include/plat/display.h
 index c915a66..6e1fbbd 100644
 --- a/arch/arm/plat-omap/include/plat/display.h
 +++ b/arch/arm/plat-omap/include/plat/display.h
 @@ -346,6 +346,24 @@ struct omap_overlay_manager {
   int (*disable)(struct omap_overlay_manager *mgr);  };
 
 +struct omap_display_panel {
 + struct omap_video_timings timings;
 +
 + int acbi;   /* ac-bias pin transitions per interrupt */ +   /* 
 Unit: line
 clocks */ +   int acb;/* ac-bias pin frequency */
 +
 + enum omap_panel_config config;
 +
 + int power_on_delay;
 + int power_off_delay;
 + /*
 +  * Used to match device to panel configuration
 +  * when use generic panel driver
 +  */
 + const char *name;
 +};
 +
  struct omap_dss_device {
   struct device dev;
 
 @@ -395,15 +413,7 @@ struct omap_dss_device {
   } venc;
   } phy;
 
 - struct {
 - struct omap_video_timings timings;
 -
 - int acbi;   /* ac-bias pin transitions per
 interrupt */
 - /* Unit: line clocks */
 - int acb;/* ac-bias pin frequency */
 -
 - enum omap_panel_config config;
 - } panel;
 + struct omap_display_panel panel;
 
   struct {
   u8 pixel_size;

I don't think that changing the omap_dss_device structure is the best way
to go about this. Members like power_on_delay and power_off_delay may not
be required by all types of panels. Same goes for the new name member since
its only used by the dummy dpi panels.

I think it would be better to go with the approach taken in panel-taal.c
where a struct panel_config exists to take care of extra requirements.

The omap_dss_device struct should contain only those members which would be
needed by the interface driver files like dsi.c, dpi.c etc. The rest should be
taken care of the panel drivers internally.

Archit--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/3] OMAP: use generic panel driver in board files

2010-11-03 Thread Tomi Valkeinen
On Wed, 2010-11-03 at 13:56 +0100, ext Taneja, Archit wrote:
 Hi,
 
 linux-omap-ow...@vger.kernel.org wrote:
  Introduce some fields in struct panel, which will be used to
  match the right panel configurations in generic panel driver.
  
  Still keep sharp_ls_panel, since the sharp_ls_panel driver
  contains blacklight control driver code which will be moved
  out later. Then we can use generic driver for sharp_ls_panel.
  
  Signed-off-by: Bryan Wu bryan...@canonical.com
 
 [snip]
 
  diff --git a/arch/arm/plat-omap/include/plat/display.h
  b/arch/arm/plat-omap/include/plat/display.h
  index c915a66..6e1fbbd 100644
  --- a/arch/arm/plat-omap/include/plat/display.h
  +++ b/arch/arm/plat-omap/include/plat/display.h
  @@ -346,6 +346,24 @@ struct omap_overlay_manager {
  int (*disable)(struct omap_overlay_manager *mgr);  };
  
  +struct omap_display_panel {
  +   struct omap_video_timings timings;
  +
  +   int acbi;   /* ac-bias pin transitions per interrupt */ +   /* 
  Unit: line
  clocks */ + int acb;/* ac-bias pin frequency */
  +
  +   enum omap_panel_config config;
  +
  +   int power_on_delay;
  +   int power_off_delay;
  +   /*
  +* Used to match device to panel configuration
  +* when use generic panel driver
  +*/
  +   const char *name;
  +};
  +
   struct omap_dss_device {
  struct device dev;
  
  @@ -395,15 +413,7 @@ struct omap_dss_device {
  } venc;
  } phy;
  
  -   struct {
  -   struct omap_video_timings timings;
  -
  -   int acbi;   /* ac-bias pin transitions per
  interrupt */
  -   /* Unit: line clocks */
  -   int acb;/* ac-bias pin frequency */
  -
  -   enum omap_panel_config config;
  -   } panel;
  +   struct omap_display_panel panel;
  
  struct {
  u8 pixel_size;
 
 I don't think that changing the omap_dss_device structure is the best way
 to go about this. Members like power_on_delay and power_off_delay may not
 be required by all types of panels. Same goes for the new name member since
 its only used by the dummy dpi panels.
 
 I think it would be better to go with the approach taken in panel-taal.c
 where a struct panel_config exists to take care of extra requirements.
 
 The omap_dss_device struct should contain only those members which would be
 needed by the interface driver files like dsi.c, dpi.c etc. The rest should be
 taken care of the panel drivers internally.

Yes, the configuration for the panel is panel driver specific features,
and should not be in the standard structs. No need to add anything to
display.h.

In fact, there are things in display.h that should be removed, and this
configurable panel driver could do that also. omap_dss_device contains
reset_gpio, max_backlight_level, platform_enable/disable and
set/get_backlight, which should actually be in panel spesific struct.

 Tomi


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] OMAP: use generic panel driver in board files

2010-11-03 Thread Bryan Wu
On Wed, Nov 3, 2010 at 11:08 AM, Tomi Valkeinen
tomi.valkei...@nokia.com wrote:
 On Wed, 2010-11-03 at 13:56 +0100, ext Taneja, Archit wrote:
 Hi,

 linux-omap-ow...@vger.kernel.org wrote:
  Introduce some fields in struct panel, which will be used to
  match the right panel configurations in generic panel driver.
 
  Still keep sharp_ls_panel, since the sharp_ls_panel driver
  contains blacklight control driver code which will be moved
  out later. Then we can use generic driver for sharp_ls_panel.
 
  Signed-off-by: Bryan Wu bryan...@canonical.com

 [snip]

  diff --git a/arch/arm/plat-omap/include/plat/display.h
  b/arch/arm/plat-omap/include/plat/display.h
  index c915a66..6e1fbbd 100644
  --- a/arch/arm/plat-omap/include/plat/display.h
  +++ b/arch/arm/plat-omap/include/plat/display.h
  @@ -346,6 +346,24 @@ struct omap_overlay_manager {
      int (*disable)(struct omap_overlay_manager *mgr);  };
 
  +struct omap_display_panel {
  +   struct omap_video_timings timings;
  +
  +   int acbi;       /* ac-bias pin transitions per interrupt */ +   /* 
  Unit: line
  clocks */ + int acb;        /* ac-bias pin frequency */
  +
  +   enum omap_panel_config config;
  +
  +   int power_on_delay;
  +   int power_off_delay;
  +   /*
  +    * Used to match device to panel configuration
  +    * when use generic panel driver
  +    */
  +   const char *name;
  +};
  +
   struct omap_dss_device {
      struct device dev;
 
  @@ -395,15 +413,7 @@ struct omap_dss_device {
              } venc;
      } phy;
 
  -   struct {
  -           struct omap_video_timings timings;
  -
  -           int acbi;       /* ac-bias pin transitions per
  interrupt */
  -           /* Unit: line clocks */
  -           int acb;        /* ac-bias pin frequency */
  -
  -           enum omap_panel_config config;
  -   } panel;
  +   struct omap_display_panel panel;
 
      struct {
              u8 pixel_size;

 I don't think that changing the omap_dss_device structure is the best way
 to go about this. Members like power_on_delay and power_off_delay may not
 be required by all types of panels. Same goes for the new name member since
 its only used by the dummy dpi panels.

 I think it would be better to go with the approach taken in panel-taal.c
 where a struct panel_config exists to take care of extra requirements.

 The omap_dss_device struct should contain only those members which would be
 needed by the interface driver files like dsi.c, dpi.c etc. The rest should 
 be
 taken care of the panel drivers internally.

 Yes, the configuration for the panel is panel driver specific features,
 and should not be in the standard structs. No need to add anything to
 display.h.

 In fact, there are things in display.h that should be removed, and this
 configurable panel driver could do that also. omap_dss_device contains
 reset_gpio, max_backlight_level, platform_enable/disable and
 set/get_backlight, which should actually be in panel spesific struct.


After a quick study of panel-taal.c driver and board-4430sdp.c, I
believe we can pass the information from *data pointer of struct
omap_dss_device. And keep the all panel specific configuration in
driver.

I will keep the struct omap_dss_device untouched.

Thanks,
-Bryan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] OMAP: use generic panel driver in board files

2010-11-02 Thread Bryan Wu
Introduce some fields in struct panel, which will be used to match the right
panel configurations in generic panel driver.

Still keep sharp_ls_panel, since the sharp_ls_panel driver contains blacklight
control driver code which will be moved out later. Then we can use generic 
driver
for sharp_ls_panel.

Signed-off-by: Bryan Wu bryan...@canonical.com
---
 arch/arm/mach-omap2/board-3430sdp.c   |1 +
 arch/arm/mach-omap2/board-am3517evm.c |4 +++-
 arch/arm/mach-omap2/board-cm-t35.c|4 +++-
 arch/arm/mach-omap2/board-devkit8000.c|2 ++
 arch/arm/mach-omap2/board-igep0020.c  |1 +
 arch/arm/mach-omap2/board-omap3beagle.c   |1 +
 arch/arm/mach-omap2/board-omap3evm.c  |1 +
 arch/arm/mach-omap2/board-omap3stalker.c  |2 ++
 arch/arm/plat-omap/include/plat/display.h |   28 +++-
 9 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/board-3430sdp.c 
b/arch/arm/mach-omap2/board-3430sdp.c
index 4e3742c..529a016 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -273,6 +273,7 @@ static struct omap_dss_device sdp3430_lcd_device = {
 static struct omap_dss_device sdp3430_dvi_device = {
.name   = dvi,
.driver_name= generic_panel,
+   .panel.name = dvi,
.type   = OMAP_DISPLAY_TYPE_DPI,
.phy.dpi.data_lines = 24,
.platform_enable= sdp3430_panel_enable_dvi,
diff --git a/arch/arm/mach-omap2/board-am3517evm.c 
b/arch/arm/mach-omap2/board-am3517evm.c
index 0739950..e435e66 100644
--- a/arch/arm/mach-omap2/board-am3517evm.c
+++ b/arch/arm/mach-omap2/board-am3517evm.c
@@ -306,7 +306,8 @@ static void am3517_evm_panel_disable_lcd(struct 
omap_dss_device *dssdev)
 static struct omap_dss_device am3517_evm_lcd_device = {
.type   = OMAP_DISPLAY_TYPE_DPI,
.name   = lcd,
-   .driver_name= sharp_lq_panel,
+   .driver_name= generic_panel,
+   .panel.name = sharp_lq,
.phy.dpi.data_lines = 16,
.platform_enable= am3517_evm_panel_enable_lcd,
.platform_disable   = am3517_evm_panel_disable_lcd,
@@ -350,6 +351,7 @@ static struct omap_dss_device am3517_evm_dvi_device = {
.type   = OMAP_DISPLAY_TYPE_DPI,
.name   = dvi,
.driver_name= generic_panel,
+   .panel.name = dvi,
.phy.dpi.data_lines = 24,
.platform_enable= am3517_evm_panel_enable_dvi,
.platform_disable   = am3517_evm_panel_disable_dvi,
diff --git a/arch/arm/mach-omap2/board-cm-t35.c 
b/arch/arm/mach-omap2/board-cm-t35.c
index 63f764e..afadebb 100644
--- a/arch/arm/mach-omap2/board-cm-t35.c
+++ b/arch/arm/mach-omap2/board-cm-t35.c
@@ -353,7 +353,8 @@ static void cm_t35_panel_disable_tv(struct omap_dss_device 
*dssdev)
 
 static struct omap_dss_device cm_t35_lcd_device = {
.name   = lcd,
-   .driver_name= toppoly_tdo35s_panel,
+   .driver_name= generic_panel,
+   .panel.name = toppoly_tdo35s,
.type   = OMAP_DISPLAY_TYPE_DPI,
.phy.dpi.data_lines = 18,
.platform_enable= cm_t35_panel_enable_lcd,
@@ -363,6 +364,7 @@ static struct omap_dss_device cm_t35_lcd_device = {
 static struct omap_dss_device cm_t35_dvi_device = {
.name   = dvi,
.driver_name= generic_panel,
+   .panel.name = dvi,
.type   = OMAP_DISPLAY_TYPE_DPI,
.phy.dpi.data_lines = 24,
.platform_enable= cm_t35_panel_enable_dvi,
diff --git a/arch/arm/mach-omap2/board-devkit8000.c 
b/arch/arm/mach-omap2/board-devkit8000.c
index 067f437..3a5937e 100644
--- a/arch/arm/mach-omap2/board-devkit8000.c
+++ b/arch/arm/mach-omap2/board-devkit8000.c
@@ -152,6 +152,7 @@ static struct regulator_consumer_supply 
devkit8000_vio_supply =
 static struct omap_dss_device devkit8000_lcd_device = {
.name   = lcd,
.driver_name= generic_panel,
+   .panel.name = dvi,
.type   = OMAP_DISPLAY_TYPE_DPI,
.phy.dpi.data_lines = 24,
.reset_gpio = -EINVAL, /* will be replaced */
@@ -161,6 +162,7 @@ static struct omap_dss_device devkit8000_lcd_device = {
 static struct omap_dss_device devkit8000_dvi_device = {
.name   = dvi,
.driver_name= generic_panel,
+   .panel.name = dvi,
.type   = OMAP_DISPLAY_TYPE_DPI,
.phy.dpi.data_lines = 24,
.reset_gpio = -EINVAL, /* will be replaced */
diff --git a/arch/arm/mach-omap2/board-igep0020.c 
b/arch/arm/mach-omap2/board-igep0020.c
index 5e035a5..4c83ef0 100644