Patchwork dovefb: add more config options to the display controller (DCON)

login
register
mail settings
Submitter Peter Liao
Date Aug. 16, 2010, 9:15 a.m.
Message ID <689C75E0D210324F8BE88961219F681618132E32BB@SC-VEXCH2.marvell.com>
Download mbox | patch
Permalink /patch/61801/
State Not Applicable
Headers show

Comments

Peter Liao - Aug. 16, 2010, 9:15 a.m.
Dear Eric:
Thanks for your suggestion. Your changes will impact a lot to our current implementations, and all of customer and AE are familiar with current implementation already and it can satisfy all of our requirements. We implement the code referring a lot of feedback from customer, design flexibility and based on our supporting experiences. It is painful to us to change the code following your suggestions. I am sorry that we cannot give you any support and we also do not suggest you to do this way. If you do so, you have to revise your entire kernel for the platforms you have and make sure all functions works also with Marvell provided xorg driver. We have limited resources to revise the driver co-working with you in current stage. Please keep the current implementation as much as possible, otherwise you may get a lot f issues to sync up our new code drop. Hope you can understand. Thanks again for your suggestion.

-----Original Message-----
From: Green Wan
Sent: Monday, August 16, 2010 4:59 PM
To: Eric Miao; kernel-team@lists.ubuntu.com
Cc: Saeed Bishara; Peter Liao; Lea Li
Subject: RE: [PATCH] dovefb: add more config options to the display controller (DCON)

Hi Eric,  =)

1. I exported some paths in /sys/devices/platform/dovedcon/ to configure all possible DCON's status. For example, output LCD0 signal to portA and portB at the same time. Is it what you're referring to?

2. Yes, I think lcd0_enable & lcd1_enable can be decided by the pointer we pass into lcd_platform_init(). But once we rely on passing pointers, doesn't it mean that we can't dynamically enable/disable lcd0 and lcd1 with different bootargs? Actually, I wanted to do auto detect and enable/disable lcd0 automatically before. But turn out I found some h/w can't be detected correctly.

3. Yes, for H/W aspect, DCON is quite independent to LCD output signal. For example, system could enable lcd0 and configure it output to portB. I don't know whether it's suitable to tight these two modules' configuration up together. How do you think of it?

BRs,
Green


-----Original Message-----
From: eric.y.miao@gmail.com [mailto:eric.y.miao@gmail.com] On Behalf Of Eric Miao
Sent: Monday, August 16, 2010 4:20 PM
To: kernel-team@lists.ubuntu.com
Cc: Saeed Bishara; Peter Liao; Green Wan; Lea Li
Subject: [PATCH] dovefb: add more config options to the display controller (DCON)

It would be good if all the possibilities of PortA and PortB combinations in the
Display Controller can be exposed, instead of simply having either port_a or
port_b being enabled or not. Idea?

One more thing is - can be decide lcd0_enable and lcd1_enable depending
on the dovefb_mach_info * pointer passed to clcd_platform_init(), it seems to
get overly designed to have these two parameters. And lcd0_enable/lcd1_enable
seems to have no relationship with DCON configuration, and some of the code
can be clean'ed up?

Green?


commit 84a7f186460e6b5d1428de51cd7093404724125d
Author: Eric Miao <eric.miao@canonical.com>
Date:   Sat Aug 14 21:06:46 2010 +0800

    dovefb: add more config options to the display controller (DCON)

    Signed-off-by: Eric Miao <eric.miao@canonical.com>

 #endif /* _KERNEL_ */

Patch

diff --git a/arch/arm/mach-dove/clcd.c b/arch/arm/mach-dove/clcd.c
index 70e9f7d..b1a8364 100755
--- a/arch/arm/mach-dove/clcd.c
+++ b/arch/arm/mach-dove/clcd.c
@@ -528,8 +528,7 @@  static struct resource dcon_res[] = {
 };

 static struct dovedcon_mach_info dcon_data = {
-       .port_a = 1,
-       .port_b = 1,
+       .mode   = PORTA_ENABLE | PORTB_ENABLE,
 };

 static struct platform_device dcon_platform_device = {
@@ -565,7 +564,8 @@  int clcd_platform_init(struct dovefb_mach_info
*lcd0_dmi_data,
                       struct dovefb_mach_info *lcd0_vid_dmi_data,
                       struct dovefb_mach_info *lcd1_dmi_data,
                       struct dovefb_mach_info *lcd1_vid_dmi_data,
-                      struct dovebl_platform_data *backlight_data)
+                      struct dovebl_platform_data *backlight_data,
+                      struct dovedcon_mach_info *dcon)
 {
        u32 total_x, total_y, i;
        u64 div_result;
@@ -632,16 +632,21 @@  int clcd_platform_init(struct dovefb_mach_info
*lcd0_dmi_data,
                lcd_accurate_clock = 0;

 #ifdef CONFIG_FB_DOVE_DCON
-       if (lcd0_enable || lcd1_enable) {
-               if (lcd0_enable)
-                       dcon_data.port_a = 1;
-               if (lcd1_enable)
-                       dcon_data.port_b = 1;
+       if (dcon)
+               memcpy(&dcon_data, dcon, sizeof(*dcon));
+       else {
+               /* generate default according to cmdline */
+               if (lcd0_enable || lcd1_enable) {
+                       if (lcd0_enable)
+                               dcon_data.mode |= PORTA_ENABLE;
+                       if (lcd1_enable)
+                               dcon_data.mode |= PORTB_ENABLE;
 #ifdef CONFIG_FB_DOVE_CLCD_DCONB_BYPASS0
-               dcon_data.port_b = 1;
+                       dcon_data.mode |= PORTB_LCD0_BYPASS;
 #endif
-               platform_device_register(&dcon_platform_device);
+               }
        }
+       platform_device_register(&dcon_platform_device);
 #endif

 #ifdef CONFIG_BACKLIGHT_DOVE
diff --git a/arch/arm/mach-dove/dove-db-setup.c
b/arch/arm/mach-dove/dove-db-setup.c
index 52d66fe..bd38ca0 100755
--- a/arch/arm/mach-dove/dove-db-setup.c
+++ b/arch/arm/mach-dove/dove-db-setup.c
@@ -381,7 +381,7 @@  void __init dove_db_clcd_init(void) {
        }
        clcd_platform_init(lcd0_dmi, lcd0_vid_dmi,
                           &dove_db_lcd1_dmi, &dove_db_lcd1_vid_dmi,
-                          &dove_db_backlight_data);
+                          &dove_db_backlight_data, NULL);

 #endif /* CONFIG_FB_DOVE */
 }
diff --git a/arch/arm/mach-dove/dove-front-panel-common.c
b/arch/arm/mach-dove/dove-front-panel-common.c
index 3fddf67..9b97a22 100755
--- a/arch/arm/mach-dove/dove-front-panel-common.c
+++ b/arch/arm/mach-dove/dove-front-panel-common.c
@@ -237,6 +237,6 @@  void __init dove_fp_clcd_init(void) {
 #ifdef CONFIG_FB_DOVE
        clcd_platform_init(&dove_lcd0_dmi, &dove_lcd0_vid_dmi,
                           &dove_lcd1_dmi, &dove_lcd1_vid_dmi,
-                          &fp_backlight_data);
+                          &fp_backlight_data, NULL);
 #endif /* CONFIG_FB_DOVE */
 }
diff --git a/arch/arm/mach-dove/dove-rd-avng-setup.c
b/arch/arm/mach-dove/dove-rd-avng-setup.c
index a5e832e..947935a 100755
--- a/arch/arm/mach-dove/dove-rd-avng-setup.c
+++ b/arch/arm/mach-dove/dove-rd-avng-setup.c
@@ -230,7 +230,7 @@  void __init dove_rd_avng_clcd_init(void) {
 #ifdef CONFIG_FB_DOVE
        clcd_platform_init(&dove_rd_avng_lcd0_dmi, &dove_rd_avng_lcd0_vid_dmi,
                           &dove_rd_avng_lcd1_dmi, &dove_rd_avng_lcd1_vid_dmi,
-                          &dove_rd_avng_backlight_data);
+                          &dove_rd_avng_backlight_data, NULL);
 #endif /* CONFIG_FB_DOVE */
 }

diff --git a/arch/arm/mach-dove/dove-videoplug-setup.c
b/arch/arm/mach-dove/dove-videoplug-setup.c
index 37a366e..ff226a1 100644
--- a/arch/arm/mach-dove/dove-videoplug-setup.c
+++ b/arch/arm/mach-dove/dove-videoplug-setup.c
@@ -129,7 +129,7 @@  static struct dove_ssp_platform_data
dove_ssp_platform_data = {
 void __init dove_videoplug_clcd_init(void) {
 #ifdef CONFIG_FB_DOVE
        clcd_platform_init(&dove_videoplug_lcd0_dmi, &dove_videoplug_lcd0_vid_dmi,
-                          NULL, NULL, NULL);
+                          NULL, NULL, NULL, NULL);
 #endif /* CONFIG_FB_DOVE */
 }

diff --git a/drivers/video/marvell/dovedcon.c b/drivers/video/marvell/dovedcon.c
index 1f3fa24..4de60c8 100755
--- a/drivers/video/marvell/dovedcon.c
+++ b/drivers/video/marvell/dovedcon.c
@@ -15,7 +15,18 @@ 

 #include <video/dovedcon.h>

+#define DCON_CTRL0_VGA_CLK_DISABLE     (1 << 25)
+#define DCON_CTRL0_DCON_CLK_DISABLE    (1 << 24)
+#define DCON_CTRL0_DCON_RESET          (1 << 23)
+#define DCON_CTRL0_OUTPUT_DELAY                (1 << 22)
+#define DCON_CTRL0_LCD_DISABLE         (1 << 17)
+#define DCON_CTRL0_REVERSE_SCAN                (1 << 10)
+#define DCON_CTRL0_PORTB_SELECT                (3 << 8)
+#define DCON_CTRL0_PORTA_SELECT                (3 << 6)
+#define DCON_CTRL0_LBUF_EN             (1 << 5)
+
 #define VGA_CHANNEL_DEFAULT    0x90C78
+
 static int dovedcon_enable(struct dovedcon_info *ddi)
 {
        unsigned int channel_ctrl;
@@ -30,36 +41,44 @@  static int dovedcon_enable(struct dovedcon_info *ddi)
        /* enable lcd0 pass to PortB */
        ctrl0 &= ~(0x3 << 8);
        ctrl0 |= (0x1 << 8);
-       ddi->port_b = 1;
+       ddi->mode |= PORTB_ENABLE;
 #endif
-
-       /*
-        * Enable VGA clock, clear it to enable.
-        */
-       ctrl0 &= ~(ddi->port_b << 25);
-
-       /*
-        * Enable LCD clock, clear it to enable
-        */
-       ctrl0 &= ~(ddi->port_a << 24);

-       /*
-        * Enable LCD Parallel Interface, clear it to enable
-        */
-       ctrl0 &= ~(0x1 << 17);
+       /* Enable DCON clock if either port is enabled */
+       if (ddi->mode & (PORTA_ENABLE | PORTB_ENABLE))
+               ctrl0 &= ~DCON_CTRL0_DCON_CLK_DISABLE;

-       writel(ctrl0, ddi->reg_base+DCON_CTRL0);
+       if (ddi->mode & PORTA_ENABLE) {
+               /* Enable LCD Parallel Interface on PortA */
+               ctrl0 &= ~DCON_CTRL0_LCD_DISABLE;
+
+               /* Configure PortA mode */
+               ctrl0 &= ~DCON_CTRL0_PORTA_SELECT;
+               ctrl0 |= PORTA_MODE(ddi->mode) << 6;
+       }
+
+       if (ddi->mode & PORTB_ENABLE) {
+               /* Enable VGA clock on PortB */
+               ctrl0 &= ~DCON_CTRL0_VGA_CLK_DISABLE;
+
+               /* Configure PortB mode */
+               ctrl0 &= ~DCON_CTRL0_PORTB_SELECT;
+               ctrl0 |= PORTB_MODE(ddi->mode) << 8;
+       }
+
+       writel(ctrl0, ddi->reg_base + DCON_CTRL0);

        /*
         * Configure VGA data channel and power on them.
         */
-       if (ddi->port_b) {
+       if (ddi->mode & PORTB_ENABLE) {
                channel_ctrl = VGA_CHANNEL_DEFAULT;
-               writel(channel_ctrl, ddi->reg_base+DCON_VGA_DAC_CHANNEL_A_CTRL);
-               writel(channel_ctrl, ddi->reg_base+DCON_VGA_DAC_CHANNEL_B_CTRL);
-               writel(channel_ctrl, ddi->reg_base+DCON_VGA_DAC_CHANNEL_C_CTRL);
+               writel(channel_ctrl, ddi->reg_base + DCON_VGA_DAC_CHANNEL_A_CTRL);
+               writel(channel_ctrl, ddi->reg_base + DCON_VGA_DAC_CHANNEL_B_CTRL);
+               writel(channel_ctrl, ddi->reg_base + DCON_VGA_DAC_CHANNEL_C_CTRL);
        }

+       pr_debug("%s: DCON_CTRL0 = 0x%08x\n", __func__, ctrl0);
        return 0;
 }

@@ -171,7 +190,7 @@  static ssize_t dcon_show_pa_clk(struct device *dev,

        ddi = dev_get_drvdata(dev);

-       return sprintf(buf, "%d\n", ddi->port_a);
+       return sprintf(buf, "%d\n", (ddi->mode & PORTA_ENABLE) ? 1 : 0);
 }

 static ssize_t dcon_ena_pa_clk(struct device *dev,
@@ -179,38 +198,26 @@  static ssize_t dcon_ena_pa_clk(struct device *dev,
 {
        int rc;
        struct dovedcon_info *ddi;
-       unsigned long ena_clk;
+       unsigned long ena_clk, ctrl0;

        ddi = dev_get_drvdata(dev);
        rc = strict_strtoul(buf, 0, &ena_clk);
        if (rc)
                return rc;

-       rc = -ENXIO;
-
-       if (ddi->port_a != ena_clk) {
-               unsigned int ctrl0;
-
-               ddi->port_a = ena_clk;
-
-               /*
-                * Get current configuration of CTRL0
-                */
-               ctrl0 = readl(ddi->reg_base+DCON_CTRL0);
-
-               /* enable or disable LCD clk. */
-               if (0 == ddi->port_a)
-                       ctrl0 |= (0x1 << 24);
-               else
-                       ctrl0 &= ~(0x1 << 24);
-
-               /* Apply setting. */
-               writel(ctrl0, ddi->reg_base+DCON_CTRL0);
+       if ((ddi->mode & PORTA_ENABLE) && !ena_clk) {
+               ctrl0 = readl(ddi->reg_base + DCON_CTRL0);
+               ctrl0 &= ~DCON_CTRL0_DCON_CLK_DISABLE;
+               writel(ctrl0, ddi->reg_base + DCON_CTRL0);
        }

-       rc = count;
+       if (ena_clk && !(ddi->mode & PORTA_ENABLE)) {
+               ctrl0 = readl(ddi->reg_base + DCON_CTRL0);
+               ctrl0 |= DCON_CTRL0_DCON_CLK_DISABLE;
+               writel(ctrl0, ddi->reg_base + DCON_CTRL0);
+       }

-       return rc;
+       return count;
 }

 static ssize_t dcon_show_pb_clk(struct device *dev,
@@ -220,7 +227,7 @@  static ssize_t dcon_show_pb_clk(struct device *dev,

        ddi = dev_get_drvdata(dev);

-       return sprintf(buf, "%d\n", ddi->port_b);
+       return sprintf(buf, "%d\n", (ddi->mode & PORTB_ENABLE) ? 1 : 0);
 }

 static ssize_t dcon_ena_pb_clk(struct device *dev,
@@ -228,36 +235,26 @@  static ssize_t dcon_ena_pb_clk(struct device *dev,
 {
        int rc;
        struct dovedcon_info *ddi;
-       unsigned long ena_clk;
+       unsigned long ena_clk, ctrl0;

        ddi = dev_get_drvdata(dev);
        rc = strict_strtoul(buf, 0, &ena_clk);
        if (rc)
                return rc;

-       if (ddi->port_b != ena_clk) {
-               unsigned int ctrl0;
-
-               ddi->port_b = ena_clk;
-
-               /*
-                * Get current configuration of CTRL0
-                */
-               ctrl0 = readl(ddi->reg_base+DCON_CTRL0);
-
-               /* enable or disable LCD clk. */
-               if (0 == ddi->port_b)
-                       ctrl0 |= (0x1 << 25);
-               else
-                       ctrl0 &= ~(0x1 << 25);
-
-               /* Apply setting. */
-               writel(ctrl0, ddi->reg_base+DCON_CTRL0);
+       if ((ddi->mode & PORTB_ENABLE) && !ena_clk) {
+               ctrl0 = readl(ddi->reg_base + DCON_CTRL0);
+               ctrl0 &= ~DCON_CTRL0_VGA_CLK_DISABLE;
+               writel(ctrl0, ddi->reg_base + DCON_CTRL0);
        }

-       rc = count;
+       if (ena_clk && !(ddi->mode & PORTB_ENABLE)) {
+               ctrl0 = readl(ddi->reg_base + DCON_CTRL0);
+               ctrl0 |= DCON_CTRL0_VGA_CLK_DISABLE;
+               writel(ctrl0, ddi->reg_base + DCON_CTRL0);
+       }

-       return rc;
+       return count;
 }

 static ssize_t dcon_show_pa_mode(struct device *dev,
@@ -576,8 +573,7 @@  static int __init dovedcon_probe(struct
platform_device *pdev)
        if (!IS_ERR(ddi->clk))
                clk_enable(ddi->clk);

-       ddi->port_a = ddmi->port_a;
-       ddi->port_b = ddmi->port_b;
+       ddi->mode = ddmi->mode;

        /* Initialize DCON hardware */
        dovedcon_enable(ddi);
diff --git a/include/video/dovedcon.h b/include/video/dovedcon.h
index a9ec18b..3bfa02d 100644
--- a/include/video/dovedcon.h
+++ b/include/video/dovedcon.h
@@ -43,17 +43,29 @@ 

 #ifdef __KERNEL__

+#define PORTA_ENABLE           (1 << 15)
+#define PORTA_LCD0_BYPASS      (PORTA_ENABLE | 0)
+#define PORTA_OLPC_MODE                (PORTA_ENABLE | 1)
+#define PORTA_DUAL_VIEW                (PORTA_ENABLE | 2)
+#define PORTA_EXT_DCON         (PORTA_ENABLE | 3)
+
+#define PORTB_ENABLE           (1 << 31)
+#define PORTB_LCD1_BYPASS      (PORTB_ENABLE | (0 << 16))
+#define PORTB_LCD0_BYPASS      (PORTB_ENABLE | (1 << 16))
+#define PORTB_DCON_COPY                (PORTB_ENABLE | (3 << 16))
+
+#define PORTA_MODE(m)          ((m) & 0xf)
+#define PORTB_MODE(m)          (((m) >> 16) & 0xf)
+
 struct dovedcon_mach_info {
-       unsigned int port_a;
-       unsigned int port_b;
+       uint32_t        mode;
 };

 struct dovedcon_info {
        void                    *reg_base;
        struct clk              *clk;
-       struct notifier_block fb_notif;
-       unsigned int port_a;
-       unsigned int port_b;
+       uint32_t                mode;
+       struct notifier_block   fb_notif;
 };

 #define to_dcon_device(obj) container_of(obj, struct dovedcon_info, dev)
diff --git a/include/video/dovefb.h b/include/video/dovefb.h
index 8f9f058..b2fb7c4 100755
--- a/include/video/dovefb.h
+++ b/include/video/dovefb.h
@@ -20,6 +20,7 @@ 
 /*              Header Files                      */
 /* ---------------------------------------------- */
 #include <linux/fb.h>
+#include <video/dovedcon.h>

 /* ---------------------------------------------- */
 /*              IOCTL Definition                  */
@@ -476,7 +477,8 @@  int clcd_platform_init(struct dovefb_mach_info
*lcd0_dmi_data,
                       struct dovefb_mach_info *lcd0_vid_dmi_data,
                       struct dovefb_mach_info *lcd1_dmi_data,
                       struct dovefb_mach_info *lcd1_vid_dmi_data,
-                      struct dovebl_platform_data *backlight_data);
+                      struct dovebl_platform_data *backlight_data,
+                      struct dovedcon_mach_info *dcon);