| Message ID | AANLkTinzgzmNm3mgiPNHn18BkbtAEm8L0rH-rsBjLNde@mail.gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Mon, Aug 16, 2010 at 4:59 PM, Green Wan <gwan@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@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> > > 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_ */ >
Is this for Lucid or Maverick? I guess it is for Lucid. So SRU justification will be needed for stable maintainers review. -Bryan On 08/16/2010 04:20 PM, Eric Miao wrote: > 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> > > 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_ */ >
On Mon, Aug 16, 2010 at 5:15 PM, Peter Liao <pliao@marvell.com> wrote: > 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. > Hi Peter, I fully understand that. It's no kidding to satisfy all the customers :-) So this is actually intended for the upstream activities, and that's why we are so eager to see dove support being fully integrated into mainline. Just hope you will consider something like this when doing upstream work. And for your current release, if there are some benefits of this patch, I still hope they can be picked up safely. > -----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> > > 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_ */ >
On Mon, Aug 16, 2010 at 5:29 PM, Bryan Wu <bryan.wu@canonical.com> wrote: > Is this for Lucid or Maverick? I guess it is for Lucid. > > So SRU justification will be needed for stable maintainers review. > Just general RFC stage, and it's actually a small feature enhancement. This is actually nice to have when doing the enabling work for different display configurations of a new board. And I just want to throw a stone out and see how such activities can be on-going. > -Bryan > > On 08/16/2010 04:20 PM, Eric Miao wrote: >> 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> >> >> 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_ */ >> > >
Dear Eric: As Green mentioned, we will take a look when we have time. Of course, we will pick up your patch is it is beauty and good. :) Regarding to merging code back to mainline, the main issue here is that we have to merge one uniform LCD driver supporting all of our various SoC with similar LCD controller design first. Thanks anyway. -----Original Message----- From: eric.y.miao@gmail.com [mailto:eric.y.miao@gmail.com] On Behalf Of Eric Miao Sent: Monday, August 16, 2010 5:45 PM To: Peter Liao Cc: Green Wan; kernel-team@lists.ubuntu.com; Saeed Bishara; Lea Li Subject: Re: [PATCH] dovefb: add more config options to the display controller (DCON) On Mon, Aug 16, 2010 at 5:15 PM, Peter Liao <pliao@marvell.com> wrote: > 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. > Hi Peter, I fully understand that. It's no kidding to satisfy all the customers :-) So this is actually intended for the upstream activities, and that's why we are so eager to see dove support being fully integrated into mainline. Just hope you will consider something like this when doing upstream work. And for your current release, if there are some benefits of this patch, I still hope they can be picked up safely. > -----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> > > 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_ */ >
On 08/16/2010 01:20 AM, Eric Miao wrote: > 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> > > 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_ */ > Eric, I can't tell if this is a request to have this patch applied to Lucid or this is just a discussion of a potential patch. Brad
On Tue, Aug 17, 2010 at 2:56 AM, Brad Figg <brad.figg@canonical.com> wrote: > On 08/16/2010 01:20 AM, Eric Miao wrote: >> 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> >> >> 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_ */ >> > > Eric, > > I can't tell if this is a request to have this patch applied to > Lucid or this is just a discussion of a potential patch. > Brad, Sorry I should have made it clear. It was supposed to be RFC only and not targeting for Lucid. And since it's a small feature enhancement, I'd expect this to be in Maverick if the review is positive. This will be used in hedley to enable their quite unique display configuration though.
On 08/16/2010 07:05 PM, Eric Miao wrote: > On Tue, Aug 17, 2010 at 2:56 AM, Brad Figg<brad.figg@canonical.com> wrote: >> On 08/16/2010 01:20 AM, Eric Miao wrote: >>> 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> >>> >>> 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_ */ >>> >> >> Eric, >> >> I can't tell if this is a request to have this patch applied to >> Lucid or this is just a discussion of a potential patch. >> > > Brad, > > Sorry I should have made it clear. It was supposed to be RFC only > and not targeting for Lucid. > > And since it's a small feature enhancement, I'd expect this to be in > Maverick if the review is positive. This will be used in hedley to > enable their quite unique display configuration though. Eric, No problem. Just wanted to make sure so there was no misunderstanding. Brad
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);
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_ */