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

Green Wan gwan at marvell.com
Mon Aug 16 10:40:02 BST 2010


Hi Eric,

As for point 1, I got your points. Currently, all boards share same configuration. So I didn't specify all the configurable field in data structure. If wanna do this, there are many board setup files need to be revised. I think I can think about this refinement when I finish my current task. Thanks.

As for point 2, yes, you are right. Xorg has right to do the final decision to turn on/off any lcd output. Please be aware if you're running any display manager, your configuration script should be able to notify display manager. For example, using xrandr to turn off. Or I am afraid that display manager might store inconsistent information from you script done.

As for point 3, got to figure out more detail and criteria.

BRs,
Green

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

On Mon, Aug 16, 2010 at 4:59 PM, Green Wan <gwan at marvell.com> wrote:
> 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?

Yeah. Normally the board code knows the configuration so it can set
it up correctly on boot. The sysfs interface would be a nice plus then.

>
> 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.

So on those platforms where both lcd0 and lcd1 could be used, we need
to fill the platform data for both of them correctly, and they will be both
enabled by default. And it's up to the Xserver to detect EDID and which
screen to use, right? And if for power consumption consideration, i.e. when
LCD1 is not actually used and can be turned off, we could have pm scripts
to turn that off at run-time by sysfs interface (assumed there is a one).

>
> 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?

I think we can split them up more clearly if it's possible.

>
> BRs,
> Green
>
>
> -----Original Message-----
> From: eric.y.miao at gmail.com [mailto:eric.y.miao at gmail.com] On Behalf Of Eric Miao
> Sent: Monday, August 16, 2010 4:20 PM
> To: kernel-team at 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 at 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 at canonical.com>
>
> 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);
>
>
>  #endif /* _KERNEL_ */
>


More information about the kernel-team mailing list