Skip to contentSkip to conversation

ARM: OMAP3: Fix omapfb's problem on OMAP3430sdp

PATCH In review stanley.miao 6 participants 12 messages Sep 2, 2008 – Sep 8, 2008 Framebuffer layer

lore
Share & tools
lore

stanley.miao

author
#0 lore raw

PATCH 1/1 ARM: OMAP3: Fix omapfb's problem on OMAP3430sdp

In RGB565 mode, red and blue color are filtered, so the picture on LCD looks green and dark.

Fix omapfb's problem on OMAP3430sdp.

Signed-off-by Stanley Miao <[email protected]>

---

(quoted diff) +0 -0

b/drivers/video/omap/lcd_2430sdp.c
index 9af6cd0..896a236 100644

drivers/video/omap/lcd_2430sdp.c +5 -0
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/delay.h>
 #include <linux/i2c/twl4030.h>
 
 #include <mach/gpio.h>

@@ -45,6 +46,10 @@ static unsigned enable_gpio;
 #define ENABLE_VAUX3_DEDICATED	0x03
 #define ENABLE_VAUX3_DEV_GRP	0x20
 
+#define ENABLE_VPLL2_DEDICATED          0x05
+#define ENABLE_VPLL2_DEV_GRP            0xE0
+#define TWL4030_VPLL2_DEV_GRP           0x33
+#define TWL4030_VPLL2_DEDICATED         0x36
 
 #define t2_out(c, r, v) twl4030_i2c_write_u8(c, r, v)
 

@@ -76,6 +81,11 @@ static int sdp2430_panel_enable(struct lcd_panel

*panel)
{
u8 ded_val, ded_reg;
u8 grp_val, grp_reg;
+
+ if(!is_sil_rev_less_than(OMAP3430_REV_ES2_0)) {
+ t2_out(PM_RECEIVER, ENABLE_VPLL2_DEDICATED,TWL4030_VPLL2_DEDICATED);
+ t2_out(PM_RECEIVER, ENABLE_VPLL2_DEV_GRP,TWL4030_VPLL2_DEV_GRP);
+ }

if (machine_is_omap_3430sdp()) {
ded_reg = TWL4030_VAUX3_DEDICATED;
@@ -104,6 +114,11 @@ static void sdp2430_panel_disable(struct lcd_panel *panel)
{
omap_set_gpio_dataout(enable_gpio, 0); omap_set_gpio_dataout(backlight_gpio, 0);
+ if(!is_sil_rev_less_than(OMAP3430_REV_ES2_0)) {
+ t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEDICATED);
+ t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEV_GRP);
+ mdelay(4);
+ }
}

static unsigned long sdp2430_panel_get_caps(struct lcd_panel *panel)

#1 lore raw
Quoted 6 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of stanley.miao

Quoted 6 lines

In RGB565 mode, red and blue color are filtered, so the picture on LCD looks green and dark.

Fix omapfb's problem on OMAP3430sdp.

Signed-off-by Stanley Miao <[email protected]>

Tested by me. I get a nice penguin. But no blue background that I usually see on TI kernels. I have no expertise on this, so this is just an observation.

Regards,
Anand

Quoted 62 lines

---

drivers/video/omap/lcd_2430sdp.c +10 -0
index 9af6cd0..896a236 100644
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/delay.h>
 #include <linux/i2c/twl4030.h>
 
 #include <mach/gpio.h>

@@ -45,6 +46,10 @@ static unsigned enable_gpio;
 #define ENABLE_VAUX3_DEDICATED	0x03
 #define ENABLE_VAUX3_DEV_GRP	0x20
 
+#define ENABLE_VPLL2_DEDICATED          0x05
+#define ENABLE_VPLL2_DEV_GRP            0xE0
+#define TWL4030_VPLL2_DEV_GRP           0x33
+#define TWL4030_VPLL2_DEDICATED         0x36
 
 #define t2_out(c, r, v) twl4030_i2c_write_u8(c, r, v)
 

@@ -76,6 +81,11 @@ static int sdp2430_panel_enable(struct lcd_panel
 *panel)
 {
 	u8 ded_val, ded_reg;
 	u8 grp_val, grp_reg;
+	
+	if(!is_sil_rev_less_than(OMAP3430_REV_ES2_0)) {
+		t2_out(PM_RECEIVER, 
 ENABLE_VPLL2_DEDICATED,TWL4030_VPLL2_DEDICATED);
+		t2_out(PM_RECEIVER, 
 ENABLE_VPLL2_DEV_GRP,TWL4030_VPLL2_DEV_GRP);
+	}
drivers/video/omap/lcd_2430sdp.c +0 -0
 	if (machine_is_omap_3430sdp()) {
 		ded_reg = TWL4030_VAUX3_DEDICATED;

@@ -104,6 +114,11 @@ static void sdp2430_panel_disable(struct

lcd_panel
*panel)
{

drivers/video/omap/lcd_2430sdp.c +5 -1
 	omap_set_gpio_dataout(enable_gpio, 0);
 	omap_set_gpio_dataout(backlight_gpio, 0);
+	if(!is_sil_rev_less_than(OMAP3430_REV_ES2_0)) {
+		t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEDICATED);
+		t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEV_GRP);
+		mdelay(4);
+	}
 }
 
 static unsigned long sdp2430_panel_get_caps(struct lcd_panel *panel)
--

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

#2 lore raw
In reply to stanley.miao
Quoted 33 lines · stanley.miao

In RGB565 mode, red and blue color are filtered, so the picture on LCD looks green and dark.

Fix omapfb's problem on OMAP3430sdp.

Signed-off-by Stanley Miao <[email protected]>

---

drivers/video/omap/lcd_2430sdp.c +5 -0
index 9af6cd0..896a236 100644
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/delay.h>
 #include <linux/i2c/twl4030.h>
 
 #include <mach/gpio.h>

@@ -45,6 +46,10 @@ static unsigned enable_gpio;
 #define ENABLE_VAUX3_DEDICATED 0x03
 #define ENABLE_VAUX3_DEV_GRP   0x20
 
+#define ENABLE_VPLL2_DEDICATED          0x05
+#define ENABLE_VPLL2_DEV_GRP            0xE0
+#define TWL4030_VPLL2_DEV_GRP           0x33
+#define TWL4030_VPLL2_DEDICATED         0x36
 
 #define t2_out(c, r, v) twl4030_i2c_write_u8(c, r, v)

drivers/video/omap/lcd_2430sdp.c static int sdp2430_panel_enable(struct lcd_panel · line 84

Quoted 4 lines
drivers/video/omap/lcd_2430sdp.c +0 -0
@@ -76,6 +81,11 @@ static int sdp2430_panel_enable(struct lcd_panel
 *panel)
 {
       u8 ded_val, ded_reg;
       u8 grp_val, grp_reg;

The below code will execute in 2430SDP also because is_sil_rev_less_than() will be false there.
You need some mechanism to avoid that right?

drivers/video/omap/lcd_2430sdp.c static void sdp2430_panel_disable(struct lcd_panel · line 117

Quoted 13 lines
drivers/video/omap/lcd_2430sdp.c +5 -0
+
+       if(!is_sil_rev_less_than(OMAP3430_REV_ES2_0)) {
+               t2_out(PM_RECEIVER, ENABLE_VPLL2_DEDICATED,TWL4030_VPLL2_DEDICATED);
+               t2_out(PM_RECEIVER, ENABLE_VPLL2_DEV_GRP,TWL4030_VPLL2_DEV_GRP);
+       }
 
       if (machine_is_omap_3430sdp()) {

               ded_reg = TWL4030_VAUX3_DEDICATED;
drivers/video/omap/lcd_2430sdp.c +0 -0
@@ -104,6 +114,11 @@ static void sdp2430_panel_disable(struct lcd_panel
 *panel)
 {
       omap_set_gpio_dataout(enable_gpio, 0);
       omap_set_gpio_dataout(backlight_gpio, 0);

Here too the same

Quoted 13 lines
drivers/video/omap/lcd_2430sdp.c +5 -0
+       if(!is_sil_rev_less_than(OMAP3430_REV_ES2_0)) {
+               t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEDICATED);
+               t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEV_GRP);
+               mdelay(4);
+       }
 }

static unsigned long sdp2430_panel_get_caps(struct lcd_panel *panel)
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#3 lore raw

Thanks for correcting me.

Re-send this patch.

Fix omapfb's problem on OMAP3430sdp.

Signed-off-by Stanley Miao <[email protected]>

---

(quoted diff) +0 -0

b/drivers/video/omap/lcd_2430sdp.c
index 9af6cd0..e4213b3 100644

drivers/video/omap/lcd_2430sdp.c +5 -0
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/delay.h>
 #include <linux/i2c/twl4030.h>
 
 #include <mach/gpio.h>

@@ -45,6 +46,10 @@ static unsigned enable_gpio;
 #define ENABLE_VAUX3_DEDICATED	0x03
 #define ENABLE_VAUX3_DEV_GRP	0x20
 
+#define ENABLE_VPLL2_DEDICATED          0x05
+#define ENABLE_VPLL2_DEV_GRP            0xE0
+#define TWL4030_VPLL2_DEV_GRP           0x33
+#define TWL4030_VPLL2_DEDICATED         0x36
 
 #define t2_out(c, r, v) twl4030_i2c_write_u8(c, r, v)
 

@@ -76,6 +81,11 @@ static int sdp2430_panel_enable(struct lcd_panel

*panel)
{
u8 ded_val, ded_reg;
u8 grp_val, grp_reg;
+
+ if(is_sil_rev_greater_than(OMAP3430_REV_ES1_0)) {
+ t2_out(PM_RECEIVER, ENABLE_VPLL2_DEDICATED,TWL4030_VPLL2_DEDICATED);
+ t2_out(PM_RECEIVER, ENABLE_VPLL2_DEV_GRP,TWL4030_VPLL2_DEV_GRP);
+ }

if (machine_is_omap_3430sdp()) {
ded_reg = TWL4030_VAUX3_DEDICATED;
@@ -104,6 +114,11 @@ static void sdp2430_panel_disable(struct lcd_panel *panel)
{
omap_set_gpio_dataout(enable_gpio, 0); omap_set_gpio_dataout(backlight_gpio, 0);
+ if(is_sil_rev_greater_than(OMAP3430_REV_ES1_0)) {
+ t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEDICATED);
+ t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEV_GRP);
+ mdelay(4);
+ }
}

static unsigned long sdp2430_panel_get_caps(struct lcd_panel *panel)

#4 lore raw

Review static int sdp2430_panel_enable(struct lcd_panel · line 86

Quoted 7 lines · stanley.miao
(quoted diff) +2 -0
@@ -76,6 +81,11 @@ static int sdp2430_panel_enable(struct lcd_panel
 *panel)
 {
 	u8 ded_val, ded_reg;
 	u8 grp_val, grp_reg;
+	
+	if(is_sil_rev_greater_than(OMAP3430_REV_ES1_0)) {

missing space "if ()".

Review static void sdp2430_panel_disable(struct lcd_panel · line 117

Quoted 5 lines
(quoted diff) +0 -0
@@ -104,6 +114,11 @@ static void sdp2430_panel_disable(struct lcd_panel
 *panel)
 {
 	omap_set_gpio_dataout(enable_gpio, 0);
 	omap_set_gpio_dataout(backlight_gpio, 0);

nothing to do with your patch, but these should be converted to
gpiolib.

Review static void sdp2430_panel_disable(struct lcd_panel · line 118

Quoted 1 line
(quoted diff) +1 -0
+	if(is_sil_rev_greater_than(OMAP3430_REV_ES1_0)) {

missing space again. Please, run scripts/checkpatch.pl --strict patch.diff before sending your patches so we avoid obvious style mistakes.

Signature · 1 line
balbi
author
#5 lore raw

Thanks. I resolved the style problem. If gpio function needs to be changed, I think it should be in a single patch.

Re-send the patch again.

Fix omapfb's problem on OMAP3430

Signed-off-by Stanley.Miao <[email protected]>
---
 drivers/video/omap/lcd_2430sdp.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)
(quoted diff) +0 -0

b/drivers/video/omap/lcd_2430sdp.c
index 9af6cd0..cf8d2a5 100644

drivers/video/omap/lcd_2430sdp.c +5 -0
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/delay.h>
 #include <linux/i2c/twl4030.h>
 
 #include <mach/gpio.h>

@@ -45,6 +46,10 @@ static unsigned enable_gpio;
 #define ENABLE_VAUX3_DEDICATED	0x03
 #define ENABLE_VAUX3_DEV_GRP	0x20
 
+#define ENABLE_VPLL2_DEDICATED          0x05
+#define ENABLE_VPLL2_DEV_GRP            0xE0
+#define TWL4030_VPLL2_DEV_GRP           0x33
+#define TWL4030_VPLL2_DEDICATED         0x36
 
 #define t2_out(c, r, v) twl4030_i2c_write_u8(c, r, v)
 

@@ -77,6 +82,13 @@ static int sdp2430_panel_enable(struct lcd_panel

*panel)
u8 ded_val, ded_reg;
u8 grp_val, grp_reg;

+ if (is_sil_rev_greater_than(OMAP3430_REV_ES1_0)) {
+ t2_out(PM_RECEIVER, ENABLE_VPLL2_DEDICATED,
+ TWL4030_VPLL2_DEDICATED);
+ t2_out(PM_RECEIVER, ENABLE_VPLL2_DEV_GRP,
+ TWL4030_VPLL2_DEV_GRP);
+ }
+
if (machine_is_omap_3430sdp()) {
ded_reg = TWL4030_VAUX3_DEDICATED;
ded_val = ENABLE_VAUX3_DEDICATED;
@@ -104,6 +116,11 @@ static void sdp2430_panel_disable(struct lcd_panel *panel)
{
omap_set_gpio_dataout(enable_gpio, 0); omap_set_gpio_dataout(backlight_gpio, 0);
+ if (is_sil_rev_greater_than(OMAP3430_REV_ES1_0)) {
+ t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEDICATED);
+ t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEV_GRP);
+ mdelay(4);
+ }
}

static unsigned long sdp2430_panel_get_caps(struct lcd_panel *panel)

Signature · 1 line
1.5.6.3
#8 lore raw
In reply to stanley.miao
Quoted 1 line · stanley.miao

+       if(!is_sil_rev_less_than(OMAP3430_REV_ES2_0)) {

But checkpatch.pl rejected that, yes? Fixed in a later version I see.

... and in "static int sdp2430_panel_enable(...)" this would seem to be wrong. But for that matter, lcd_2430sdp.c should not be relevant for any 3430 board anyway...

If that file is, oddly, being shared between two boards ... then put that code in the "if (machine_is_omap_3430sdp())" block. And someone should consider renaming the file if it's not specific to the 2430 SDP.

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

author
#9 lore raw
Quoted 5 lines · David Brownell
Quoted 1 line · stanley.miao

+ if(!is_sil_rev_less_than(OMAP3430_REV_ES2_0)) {

But checkpatch.pl rejected that, yes? Fixed in a later version I see.

It has been fixed in a later version.

Quoted 4 lines

... and in "static int sdp2430_panel_enable(...)" this would seem to be wrong. But for that matter, lcd_2430sdp.c should not be relevant for any 3430 board anyway...

this was shared between two boards.

Quoted 5 lines

If that file is, oddly, being shared between two boards ... then put that code in the "if (machine_is_omap_3430sdp())" block. And someone should consider renaming the file if it's not specific to the 2430 SDP.

OMAP3430_ES1 doesn't have this problem.

Stanley.

Quoted 3 lines
author
#11 lore raw
In reply to David Brownell
Quoted 11 lines · David Brownell
Quoted 7 lines · stanley.miao
Quoted 5 lines

If that file is, oddly, being shared between two boards ... then put that code in the "if (machine_is_omap_3430sdp())" block. And someone should consider renaming the file if it's not specific to the 2430 SDP.

OMAP3430_ES1 doesn't have this problem.

Not the point. Why have 2430 chips run code that only makes sense on 3430 silicion?

Have you noticed my later version ? I have modified it to if (is_sil_rev_greater_than(OMAP3430_REV_ES1_0)).

Stanley.

Quoted 1 line
#10 lore raw
In reply to stanley.miao
Quoted 7 lines · stanley.miao
Quoted 5 lines

If that file is, oddly, being shared between two boards ... then put that code in the "if (machine_is_omap_3430sdp())" block.  And someone should consider renaming the file if it's not specific to the 2430 SDP.

OMAP3430_ES1 doesn't have this problem.

Not the point. Why have 2430 chips run code that only makes sense on 3430 silicion?

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

#6 lore raw
In reply to stanley.miao
Quoted 50 lines · stanley.miao

Thanks. I resolved the style problem. If gpio function needs to be changed, I think it should be in a single patch.

Re-send the patch again.

Fix omapfb's problem on OMAP3430

Signed-off-by Stanley.Miao <[email protected]>
---
 drivers/video/omap/lcd_2430sdp.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)
drivers/video/omap/lcd_2430sdp.c +12 -0
index 9af6cd0..cf8d2a5 100644
@@ -23,6 +23,7 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/delay.h>
 #include <linux/i2c/twl4030.h>
 
 #include <mach/gpio.h>

@@ -45,6 +46,10 @@ static unsigned enable_gpio;
 #define ENABLE_VAUX3_DEDICATED	0x03
 #define ENABLE_VAUX3_DEV_GRP	0x20
 
+#define ENABLE_VPLL2_DEDICATED          0x05
+#define ENABLE_VPLL2_DEV_GRP            0xE0
+#define TWL4030_VPLL2_DEV_GRP           0x33
+#define TWL4030_VPLL2_DEDICATED         0x36
 
 #define t2_out(c, r, v) twl4030_i2c_write_u8(c, r, v)
 

@@ -77,6 +82,13 @@ static int sdp2430_panel_enable(struct lcd_panel
 *panel)
 	u8 ded_val, ded_reg;
 	u8 grp_val, grp_reg;
 
+	if (is_sil_rev_greater_than(OMAP3430_REV_ES1_0)) {
+		t2_out(PM_RECEIVER, ENABLE_VPLL2_DEDICATED,
+					TWL4030_VPLL2_DEDICATED);
+		t2_out(PM_RECEIVER, ENABLE_VPLL2_DEV_GRP,
+					TWL4030_VPLL2_DEV_GRP);
+	}
+
 	if (machine_is_omap_3430sdp()) {
 		ded_reg = TWL4030_VAUX3_DEDICATED;
Quoted 1 line

ded_val = ENABLE_VAUX3_DEDICATED;

How about putting the is_sel_rev_greater_than test within the machine_is_omap3430sdp test?

Tony

Quoted 20 lines
drivers/video/omap/lcd_2430sdp.c +5 -0
@@ -104,6 +116,11 @@ static void sdp2430_panel_disable(struct lcd_panel
 *panel)
 {
 	omap_set_gpio_dataout(enable_gpio, 0);
 	omap_set_gpio_dataout(backlight_gpio, 0);
+	if (is_sil_rev_greater_than(OMAP3430_REV_ES1_0)) {
+		t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEDICATED);
+		t2_out(PM_RECEIVER, 0x0, TWL4030_VPLL2_DEV_GRP);
+		mdelay(4);
+	}
 }

static unsigned long sdp2430_panel_get_caps(struct lcd_panel *panel)

Signature · 6 lines
1.5.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#7 lore raw
Quoted 13 lines · Tony Lindgren
Quoted 12 lines · stanley.miao

Thanks. I resolved the style problem. If gpio function needs to be changed, I think it should be in a single patch.

Re-send the patch again.

Fix omapfb's problem on OMAP3430

Signed-off-by Stanley.Miao <[email protected]>
---
 drivers/video/omap/lcd_2430sdp.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

[...]

Quoted 8 lines
Quoted 8 lines

#define t2_out(c, r, v) twl4030_i2c_write_u8(c, r, v)

(quoted diff) +0 -0
@@ -77,6 +82,13 @@ static int sdp2430_panel_enable(struct lcd_panel
 *panel)
 	u8 ded_val, ded_reg;
 	u8 grp_val, grp_reg;
Quoted 13 lines
Quoted 10 lines
(quoted diff) +7 -0
+	if (is_sil_rev_greater_than(OMAP3430_REV_ES1_0)) {
+		t2_out(PM_RECEIVER, ENABLE_VPLL2_DEDICATED,
+					TWL4030_VPLL2_DEDICATED);
+		t2_out(PM_RECEIVER, ENABLE_VPLL2_DEV_GRP,
+					TWL4030_VPLL2_DEV_GRP);
+	}
+
 	if (machine_is_omap_3430sdp()) {
 		ded_reg = TWL4030_VAUX3_DEDICATED;

ded_val = ENABLE_VAUX3_DEDICATED;

How about putting the is_sel_rev_greater_than test within the machine_is_omap3430sdp test?

Good, this will run fewer instructions on 2430sdp.

Stanley.

Quoted 2 lines

Tony