Re: [RFC v2 3/3] video: exynos: Making s6e8ax0 panel driver compliant with CDF

2013-02-14 Thread Vikas Sajjan
Hi Mr. Lee,

thanks for the review.

On 14 February 2013 07:30, Donghwa Lee dh09@samsung.com wrote:
 On Wed, Feb 13, 2013 at 19:01, Vikas Sajjan wrote:

 Made necessary changes in s6e8ax0 panel driver as per the  CDF-T.
 It also removes the dependency on backlight and lcd framework

 Signed-off-by: Vikas Sajjanvikas.saj...@linaro.org
 ---
   drivers/video/exynos/s6e8ax0.c |  848
 +---
   1 file changed, 444 insertions(+), 404 deletions(-)

 diff --git a/drivers/video/exynos/s6e8ax0.c
 b/drivers/video/exynos/s6e8ax0.c
 index 7f7b25f..5a17e3c 100644
 --- a/drivers/video/exynos/s6e8ax0.c
 +++ b/drivers/video/exynos/s6e8ax0.c
 @@ -25,6 +25,7 @@
   #include linux/backlight.h
   #include linux/regulator/consumer.h
   +#include video/display.h
   #include video/mipi_display.h
   #include video/exynos_mipi_dsim.h
   @@ -38,8 +39,7 @@
   #define POWER_IS_OFF(pwr) ((pwr) == FB_BLANK_POWERDOWN)
   #define POWER_IS_NRM(pwr) ((pwr) == FB_BLANK_NORMAL)
   -#define lcd_to_master(a) (a-dsim_dev-master)
 -#define lcd_to_master_ops(a)   ((lcd_to_master(a))-master_ops)
 +#define to_panel(p) container_of(p, struct s6e8ax0, entity)
 enum {
 DSIM_NONE_STATE = 0,
 @@ -47,20 +47,34 @@ enum {
 DSIM_FRAME_DONE = 2,
   };
   +/* This structure defines all the properties of a backlight */
 +struct backlight_prop {
 +   /* Current User requested brightness (0 - max_brightness) */
 +   int brightness;
 +   /* Maximal value for brightness (read-only) */
 +   int max_brightness;
 +};
 +
 +struct panel_platform_data {
 +   unsigned intreset_delay;
 +   unsigned intpower_on_delay;
 +   unsigned intpower_off_delay;
 +   const char  *video_source_name;
 +};
 +
   struct s6e8ax0 {
 -   struct device   *dev;
 -   unsigned intpower;
 -   unsigned intid;
 -   unsigned intgamma;
 -   unsigned intacl_enable;
 -   unsigned intcur_acl;
 -
 -   struct lcd_device   *ld;
 -   struct backlight_device *bd;
 -
 -   struct mipi_dsim_lcd_device *dsim_dev;
 -   struct lcd_platform_data*ddi_pd;
 +   struct platform_device  *pdev;
 +   struct video_source *src;
 +   struct display_entity   entity;
 +   unsigned intpower;
 +   unsigned intid;
 +   unsigned intgamma;
 +   unsigned intacl_enable;
 +   unsigned intcur_acl;
 +   boolpanel_reverse;
 +   struct lcd_platform_data*plat_data;
 struct mutexlock;
 +   struct backlight_prop   bl_prop;
 bool  enabled;
   };


 Could this panel driver use only CDF?
 Does not consider the compatibility with backlight and lcd framework?

as of now CDF does not support backlight and lcd framework functionalities.
Once CDF has the support, we modify the driver to support both CDF and
non CDF way, there by maintaining the backward compatibility with
backlight and lcd framework.

 -static const unsigned char s6e8ax0_22_gamma_30[] = {
 +static unsigned char s6e8ax0_22_gamma_30[] = {
 0xfa, 0x01, 0x60, 0x10, 0x60, 0xf5, 0x00, 0xff, 0xad, 0xaf,
 0xbA, 0xc3, 0xd8, 0xc5, 0x9f, 0xc6, 0x9e, 0xc1, 0xdc, 0xc0,
 0x00, 0x61, 0x00, 0x5a, 0x00, 0x74,
   };

 In all case, you had changed data type to 'static unsigned char'.
 Is it need to change all case? Otherwise, for the unity of the code?

in the CDF-T proposed by Mr. Tomi Valkeinen, the prototype for
dcs_write looks as below

int (*dcs_write)(struct video_source *src, int channel, u8 *data, size_t len);

It does not have const for the 3rd parameter (u8 *data ), and in our
driver we have all the arrays as const.
Just to silence the compiler warnings, i had removed the const keyword.


 Thank you,
 Donghwa Lee





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


[RFC v2 3/3] video: exynos: Making s6e8ax0 panel driver compliant with CDF

2013-02-13 Thread Vikas Sajjan
Made necessary changes in s6e8ax0 panel driver as per the  CDF-T.
It also removes the dependency on backlight and lcd framework

Signed-off-by: Vikas Sajjan vikas.saj...@linaro.org
---
 drivers/video/exynos/s6e8ax0.c |  848 +---
 1 file changed, 444 insertions(+), 404 deletions(-)

diff --git a/drivers/video/exynos/s6e8ax0.c b/drivers/video/exynos/s6e8ax0.c
index 7f7b25f..5a17e3c 100644
--- a/drivers/video/exynos/s6e8ax0.c
+++ b/drivers/video/exynos/s6e8ax0.c
@@ -25,6 +25,7 @@
 #include linux/backlight.h
 #include linux/regulator/consumer.h
 
+#include video/display.h
 #include video/mipi_display.h
 #include video/exynos_mipi_dsim.h
 
@@ -38,8 +39,7 @@
 #define POWER_IS_OFF(pwr)  ((pwr) == FB_BLANK_POWERDOWN)
 #define POWER_IS_NRM(pwr)  ((pwr) == FB_BLANK_NORMAL)
 
-#define lcd_to_master(a)   (a-dsim_dev-master)
-#define lcd_to_master_ops(a)   ((lcd_to_master(a))-master_ops)
+#define to_panel(p) container_of(p, struct s6e8ax0, entity)
 
 enum {
DSIM_NONE_STATE = 0,
@@ -47,20 +47,34 @@ enum {
DSIM_FRAME_DONE = 2,
 };
 
+/* This structure defines all the properties of a backlight */
+struct backlight_prop {
+   /* Current User requested brightness (0 - max_brightness) */
+   int brightness;
+   /* Maximal value for brightness (read-only) */
+   int max_brightness;
+};
+
+struct panel_platform_data {
+   unsigned intreset_delay;
+   unsigned intpower_on_delay;
+   unsigned intpower_off_delay;
+   const char  *video_source_name;
+};
+
 struct s6e8ax0 {
-   struct device   *dev;
-   unsigned intpower;
-   unsigned intid;
-   unsigned intgamma;
-   unsigned intacl_enable;
-   unsigned intcur_acl;
-
-   struct lcd_device   *ld;
-   struct backlight_device *bd;
-
-   struct mipi_dsim_lcd_device *dsim_dev;
-   struct lcd_platform_data*ddi_pd;
+   struct platform_device  *pdev;
+   struct video_source *src;
+   struct display_entity   entity;
+   unsigned intpower;
+   unsigned intid;
+   unsigned intgamma;
+   unsigned intacl_enable;
+   unsigned intcur_acl;
+   boolpanel_reverse;
+   struct lcd_platform_data*plat_data;
struct mutexlock;
+   struct backlight_prop   bl_prop;
bool  enabled;
 };
 
@@ -70,192 +84,194 @@ static struct regulator_bulk_data supplies[] = {
{ .supply = vci, },
 };
 
-static void s6e8ax0_regulator_enable(struct s6e8ax0 *lcd)
+static void s6e8ax0_regulator_enable(struct s6e8ax0 *panel)
 {
int ret = 0;
-   struct lcd_platform_data *pd = NULL;
+   struct lcd_platform_data *plat_data = NULL;
 
-   pd = lcd-ddi_pd;
-   mutex_lock(lcd-lock);
-   if (!lcd-enabled) {
+   plat_data = panel-plat_data;
+
+   mutex_lock(panel-lock);
+
+   if (!panel-enabled) {
ret = regulator_bulk_enable(ARRAY_SIZE(supplies), supplies);
if (ret)
goto out;
-
-   lcd-enabled = true;
+   panel-enabled = true;
}
-   msleep(pd-power_on_delay);
+   msleep(plat_data-power_on_delay);
 out:
-   mutex_unlock(lcd-lock);
+   mutex_unlock(panel-lock);
 }
 
-static void s6e8ax0_regulator_disable(struct s6e8ax0 *lcd)
+static void s6e8ax0_regulator_disable(struct s6e8ax0 *panel)
 {
int ret = 0;
 
-   mutex_lock(lcd-lock);
-   if (lcd-enabled) {
+   mutex_lock(panel-lock);
+
+   if (panel-enabled) {
ret = regulator_bulk_disable(ARRAY_SIZE(supplies), supplies);
if (ret)
goto out;
 
-   lcd-enabled = false;
+   panel-enabled = false;
}
 out:
-   mutex_unlock(lcd-lock);
+   mutex_unlock(panel-lock);
 }
 
-static const unsigned char s6e8ax0_22_gamma_30[] = {
+static unsigned char s6e8ax0_22_gamma_30[] = {
0xfa, 0x01, 0x60, 0x10, 0x60, 0xf5, 0x00, 0xff, 0xad, 0xaf,
0xbA, 0xc3, 0xd8, 0xc5, 0x9f, 0xc6, 0x9e, 0xc1, 0xdc, 0xc0,
0x00, 0x61, 0x00, 0x5a, 0x00, 0x74,
 };
 
-static const unsigned char s6e8ax0_22_gamma_50[] = {
+static unsigned char s6e8ax0_22_gamma_50[] = {
0xfa, 0x01, 0x60, 0x10, 0x60, 0xe8, 0x1f, 0xf7, 0xad, 0xc0,
0xb5, 0xc4, 0xdc, 0xc4, 0x9e, 0xc6, 0x9c, 0xbb, 0xd8, 0xbb,
0x00, 0x70, 0x00, 0x68, 0x00, 0x86,
 };
 
-static const unsigned char s6e8ax0_22_gamma_60[] = {
+static unsigned char s6e8ax0_22_gamma_60[] = {
0xfa, 0x01, 0x60, 0x10, 0x60, 0xde, 0x1f, 0xef, 0xad, 0xc4,
0xb3, 0xc3, 0xdd, 0xc4, 0x9e, 0xc6, 0x9c, 0xbc, 0xd6, 0xba,
0x00, 0x75, 0x00, 0x6e, 0x00, 0x8d,
 };
 
-static const unsigned char s6e8ax0_22_gamma_70[] = {
+static unsigned char 

Re: [RFC v2 3/3] video: exynos: Making s6e8ax0 panel driver compliant with CDF

2013-02-13 Thread Donghwa Lee

On Wed, Feb 13, 2013 at 19:01, Vikas Sajjan wrote:

Made necessary changes in s6e8ax0 panel driver as per the  CDF-T.
It also removes the dependency on backlight and lcd framework

Signed-off-by: Vikas Sajjanvikas.saj...@linaro.org
---
  drivers/video/exynos/s6e8ax0.c |  848 +---
  1 file changed, 444 insertions(+), 404 deletions(-)

diff --git a/drivers/video/exynos/s6e8ax0.c b/drivers/video/exynos/s6e8ax0.c
index 7f7b25f..5a17e3c 100644
--- a/drivers/video/exynos/s6e8ax0.c
+++ b/drivers/video/exynos/s6e8ax0.c
@@ -25,6 +25,7 @@
  #include linux/backlight.h
  #include linux/regulator/consumer.h
  
+#include video/display.h

  #include video/mipi_display.h
  #include video/exynos_mipi_dsim.h
  
@@ -38,8 +39,7 @@

  #define POWER_IS_OFF(pwr) ((pwr) == FB_BLANK_POWERDOWN)
  #define POWER_IS_NRM(pwr) ((pwr) == FB_BLANK_NORMAL)
  
-#define lcd_to_master(a)	(a-dsim_dev-master)

-#define lcd_to_master_ops(a)   ((lcd_to_master(a))-master_ops)
+#define to_panel(p) container_of(p, struct s6e8ax0, entity)
  
  enum {

DSIM_NONE_STATE = 0,
@@ -47,20 +47,34 @@ enum {
DSIM_FRAME_DONE = 2,
  };
  
+/* This structure defines all the properties of a backlight */

+struct backlight_prop {
+   /* Current User requested brightness (0 - max_brightness) */
+   int brightness;
+   /* Maximal value for brightness (read-only) */
+   int max_brightness;
+};
+
+struct panel_platform_data {
+   unsigned intreset_delay;
+   unsigned intpower_on_delay;
+   unsigned intpower_off_delay;
+   const char  *video_source_name;
+};
+
  struct s6e8ax0 {
-   struct device   *dev;
-   unsigned intpower;
-   unsigned intid;
-   unsigned intgamma;
-   unsigned intacl_enable;
-   unsigned intcur_acl;
-
-   struct lcd_device   *ld;
-   struct backlight_device *bd;
-
-   struct mipi_dsim_lcd_device *dsim_dev;
-   struct lcd_platform_data*ddi_pd;
+   struct platform_device  *pdev;
+   struct video_source *src;
+   struct display_entity   entity;
+   unsigned intpower;
+   unsigned intid;
+   unsigned intgamma;
+   unsigned intacl_enable;
+   unsigned intcur_acl;
+   boolpanel_reverse;
+   struct lcd_platform_data*plat_data;
struct mutexlock;
+   struct backlight_prop   bl_prop;
bool  enabled;
  };
  

Could this panel driver use only CDF?
Does not consider the compatibility with backlight and lcd framework?

-static const unsigned char s6e8ax0_22_gamma_30[] = {
+static unsigned char s6e8ax0_22_gamma_30[] = {
0xfa, 0x01, 0x60, 0x10, 0x60, 0xf5, 0x00, 0xff, 0xad, 0xaf,
0xbA, 0xc3, 0xd8, 0xc5, 0x9f, 0xc6, 0x9e, 0xc1, 0xdc, 0xc0,
0x00, 0x61, 0x00, 0x5a, 0x00, 0x74,
  };

In all case, you had changed data type to 'static unsigned char'.
Is it need to change all case? Otherwise, for the unity of the code?


Thank you,
Donghwa Lee


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