Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH v5 00/12] video: da8xx-fb: am335x DT support
From: Afzal Mohammed @ 2013-01-28  9:32 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette

Hi,

This series adds DT support to da8xx-fb driver (device found on
DaVinci and AM335x SoC's). It does certain cleanup's in the process.

This series as compared to previous version goes back to v2 way of
configuring pixel clock rate. i.e. set divider if rate is within
the range that is configurable with existing input clock rate, else
change input clock rate as required instead of modeling CCF clock
nodes in the driver (more details in 12/12)

This makes use of Steffen Trumtrar's v17 of display timing DT support.

Testing has been done on AM335x SoC based boards like AM335x EVM. It
has also been verified that display on DA850 EVM (non-DT boot) works
as earlier.

This series is based on v3.8-rc3,
 and is dependent on,
1. Series v17 "of: add display helper" by,
        Steffen Trumtrar <s.trumtrar@pengutronix.de>
2. Patch "da8xx: Allow use by am33xx based devices" by,
        Pantelis Antoniou <panto@antoniou-consulting.com>
3. Series v3 "video: da8xx-fb: runtime timing configuration" by,
        me (Afzal Mohammed <afzal@ti.com>)

To test this series on AM335x based boards,
1. Series "HWMOD fixes for AM33xx PWM submodules and device tree nodes" by,
        Philip, Avinash <avinashphilip@ti.com>
as well as following,
2. Series v2 "ARM: dts: AM33XX: lcdc support",
3. Patch v2 "ARM: OMAP2+: dpll: am335x - avoid freqsel",
4. Patch v2 "ARM: OMAP2+: clock: DEFINE_STRUCT_CLK_FLAGS helper",
5. Patch v2 "ARM: AM33XX: clock: SET_RATE_PARENT in lcd path" by,
	me (Afzal Mohammed <afzal@ti.com>)
would be needed.

All above dependencies along with those required for testing is available
@ git://gitorious.org/x0148406-public/linux-kernel.git tags/da8xx-fb-dt-v5

Regards
Afzal

v5: use v2 method of configuring pixel clock rate instead of modeling
    CCF clock nodes in driver, i.e. set divider if rate is within
    the range that is configurable with existing input clock rate,
    else change input clock rate as required.
v4: use new registration for clock divider having minimum divider
    requirement and have ifdef'ery in a better way
v3: model CCF clock divider with parent propogation if CCF selected
v2: 2 new patches - one to configure clock rate properly (12/12)and
    other to make io operations safe (1/12)



Afzal Mohammed (11):
  video: da8xx-fb: make io operations safe
  video: da8xx-fb: enable sync lost intr for v2 ip
  video: da8xx-fb: use devres
  video: da8xx-fb: ensure non-null cfg in pdata
  video: da8xx-fb: reorganize panel detection
  video: da8xx-fb: minimal dt support
  video: da8xx-fb: invoke platform callback safely
  video: da8xx-fb: obtain fb_videomode info from dt
  video: da8xx-fb: ensure pdata only for non-dt
  video: da8xx-fb: setup struct lcd_ctrl_config for dt
  video: da8xx-fb: set upstream clock rate (if reqd)

Manjunathappa, Prakash (1):
  video: da8xx-fb: fix 24bpp raster configuration

 .../devicetree/bindings/video/fb-da8xx.txt         |  37 ++++
 drivers/video/da8xx-fb.c                           | 226 ++++++++++++++-------
 2 files changed, 194 insertions(+), 69 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt

-- 
1.7.12


^ permalink raw reply

* [PATCH v5 01/12] video: da8xx-fb: make io operations safe
From: Afzal Mohammed @ 2013-01-28  9:32 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

Replace __raw_readl/__raw_writel with readl/writel; this driver is
reused on ARMv7 (AM335x SoC).

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---

v2: new patch

 drivers/video/da8xx-fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 720604c..35a33ca 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -141,12 +141,12 @@ static int frame_done_flag;
 
 static inline unsigned int lcdc_read(unsigned int addr)
 {
-	return (unsigned int)__raw_readl(da8xx_fb_reg_base + (addr));
+	return (unsigned int)readl(da8xx_fb_reg_base + (addr));
 }
 
 static inline void lcdc_write(unsigned int val, unsigned int addr)
 {
-	__raw_writel(val, da8xx_fb_reg_base + (addr));
+	writel(val, da8xx_fb_reg_base + (addr));
 }
 
 struct da8xx_fb_par {
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 02/12] video: da8xx-fb: fix 24bpp raster configuration
From: Afzal Mohammed @ 2013-01-28  9:32 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette, Manjunathappa, Prakash
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

From: "Manjunathappa, Prakash" <prakash.pm@ti.com>

Set only LCD_V2_TFT_24BPP_MODE bit for 24bpp and LCD_V2_TFT_24BPP_UNPACK
bit along with LCD_V2_TFT_24BPP_MODE for 32bpp configuration.

Patch is tested on am335x-evm for 24bpp and da850-evm for 16bpp
configurations.

Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 35a33ca..7f92f37 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -550,10 +550,10 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
 	case 4:
 	case 16:
 		break;
-	case 24:
-		reg |= LCD_V2_TFT_24BPP_MODE;
 	case 32:
 		reg |= LCD_V2_TFT_24BPP_UNPACK;
+	case 24:
+		reg |= LCD_V2_TFT_24BPP_MODE;
 		break;
 
 	case 8:
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 03/12] video: da8xx-fb: enable sync lost intr for v2 ip
From: Afzal Mohammed @ 2013-01-28  9:32 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

interrupt handler is checking for sync lost interrupt, but it was not
enabled, enable it.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 7f92f37..ca69e01 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -318,7 +318,7 @@ static void lcd_blit(int load_mode, struct da8xx_fb_par *par)
 			reg_int = lcdc_read(LCD_INT_ENABLE_SET_REG) |
 				LCD_V2_END_OF_FRAME0_INT_ENA |
 				LCD_V2_END_OF_FRAME1_INT_ENA |
-				LCD_FRAME_DONE;
+				LCD_FRAME_DONE | LCD_SYNC_LOST;
 			lcdc_write(reg_int, LCD_INT_ENABLE_SET_REG);
 		}
 		reg_dma |= LCD_DUAL_FRAME_BUFFER_ENABLE;
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 04/12] video: da8xx-fb: use devres
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

Replace existing resource handling in the driver with managed device
resource.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index ca69e01..7a32e83 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1036,12 +1036,9 @@ static int fb_remove(struct platform_device *dev)
 				  par->p_palette_base);
 		dma_free_coherent(NULL, par->vram_size, par->vram_virt,
 				  par->vram_phys);
-		free_irq(par->irq, par);
 		pm_runtime_put_sync(&dev->dev);
 		pm_runtime_disable(&dev->dev);
 		framebuffer_release(info);
-		iounmap(da8xx_fb_reg_base);
-		release_mem_region(lcdc_regs->start, resource_size(lcdc_regs));
 
 	}
 	return 0;
@@ -1265,7 +1262,6 @@ static int fb_probe(struct platform_device *device)
 	struct fb_info *da8xx_fb_info;
 	struct clk *fb_clk = NULL;
 	struct da8xx_fb_par *par;
-	resource_size_t len;
 	int ret, i;
 	unsigned long ulcm;
 
@@ -1275,29 +1271,16 @@ static int fb_probe(struct platform_device *device)
 	}
 
 	lcdc_regs = platform_get_resource(device, IORESOURCE_MEM, 0);
-	if (!lcdc_regs) {
-		dev_err(&device->dev,
-			"Can not get memory resource for LCD controller\n");
-		return -ENOENT;
-	}
-
-	len = resource_size(lcdc_regs);
-
-	lcdc_regs = request_mem_region(lcdc_regs->start, len, lcdc_regs->name);
-	if (!lcdc_regs)
-		return -EBUSY;
-
-	da8xx_fb_reg_base = ioremap(lcdc_regs->start, len);
+	da8xx_fb_reg_base = devm_request_and_ioremap(&device->dev, lcdc_regs);
 	if (!da8xx_fb_reg_base) {
-		ret = -EBUSY;
-		goto err_request_mem;
+		dev_err(&device->dev, "memory resource setup failed\n");
+		return -EADDRNOTAVAIL;
 	}
 
-	fb_clk = clk_get(&device->dev, "fck");
+	fb_clk = devm_clk_get(&device->dev, "fck");
 	if (IS_ERR(fb_clk)) {
 		dev_err(&device->dev, "Can not get device clock\n");
-		ret = -ENODEV;
-		goto err_ioremap;
+		return -ENODEV;
 	}
 
 	pm_runtime_enable(&device->dev);
@@ -1458,7 +1441,7 @@ static int fb_probe(struct platform_device *device)
 		lcdc_irq_handler = lcdc_irq_handler_rev02;
 	}
 
-	ret = request_irq(par->irq, lcdc_irq_handler, 0,
+	ret = devm_request_irq(&device->dev, par->irq, lcdc_irq_handler, 0,
 			DRIVER_NAME, par);
 	if (ret)
 		goto irq_freq;
@@ -1488,12 +1471,6 @@ err_pm_runtime_disable:
 	pm_runtime_put_sync(&device->dev);
 	pm_runtime_disable(&device->dev);
 
-err_ioremap:
-	iounmap(da8xx_fb_reg_base);
-
-err_request_mem:
-	release_mem_region(lcdc_regs->start, len);
-
 	return ret;
 }
 
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 05/12] video: da8xx-fb: ensure non-null cfg in pdata
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Mike Turquette, Florian Tobias Schandinat, Rob Herring,
	Tomi Valkeinen
In-Reply-To: <cover.1359356015.git.afzal-l0cyMroinI0@public.gmane.org>

Ensure that platform data contains pointer for lcd_ctrl_config.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 7a32e83..3b146bc 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1320,6 +1320,11 @@ static int fb_probe(struct platform_device *device)
 
 	lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
 
+	if (!lcd_cfg) {
+		ret = -EINVAL;
+		goto err_pm_runtime_disable;
+	}
+
 	da8xx_fb_info = framebuffer_alloc(sizeof(struct da8xx_fb_par),
 					&device->dev);
 	if (!da8xx_fb_info) {
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 06/12] video: da8xx-fb: reorganize panel detection
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Mike Turquette, Florian Tobias Schandinat, Rob Herring,
	Tomi Valkeinen
In-Reply-To: <cover.1359356015.git.afzal-l0cyMroinI0@public.gmane.org>

Move panel detection to a separate function, this helps in readability
as well as makes DT support cleaner.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 3b146bc..b6ea5e9 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1253,6 +1253,27 @@ static struct fb_ops da8xx_fb_ops = {
 	.fb_blank = cfb_blank,
 };
 
+static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
+{
+	struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
+	struct fb_videomode *lcdc_info;
+	int i;
+
+	for (i = 0, lcdc_info = known_lcd_panels;
+		i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
+		if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
+			break;
+	}
+
+	if (i = ARRAY_SIZE(known_lcd_panels)) {
+		dev_err(&dev->dev, "no panel found\n");
+		return NULL;
+	}
+	dev_info(&dev->dev, "found %s panel\n", lcdc_info->name);
+
+	return lcdc_info;
+}
+
 static int fb_probe(struct platform_device *device)
 {
 	struct da8xx_lcdc_platform_data *fb_pdata @@ -1262,7 +1283,7 @@ static int fb_probe(struct platform_device *device)
 	struct fb_info *da8xx_fb_info;
 	struct clk *fb_clk = NULL;
 	struct da8xx_fb_par *par;
-	int ret, i;
+	int ret;
 	unsigned long ulcm;
 
 	if (fb_pdata = NULL) {
@@ -1270,6 +1291,10 @@ static int fb_probe(struct platform_device *device)
 		return -ENOENT;
 	}
 
+	lcdc_info = da8xx_fb_get_videomode(device);
+	if (lcdc_info = NULL)
+		return -ENODEV;
+
 	lcdc_regs = platform_get_resource(device, IORESOURCE_MEM, 0);
 	da8xx_fb_reg_base = devm_request_and_ioremap(&device->dev, lcdc_regs);
 	if (!da8xx_fb_reg_base) {
@@ -1303,21 +1328,6 @@ static int fb_probe(struct platform_device *device)
 		break;
 	}
 
-	for (i = 0, lcdc_info = known_lcd_panels;
-		i < ARRAY_SIZE(known_lcd_panels);
-		i++, lcdc_info++) {
-		if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
-			break;
-	}
-
-	if (i = ARRAY_SIZE(known_lcd_panels)) {
-		dev_err(&device->dev, "GLCD: No valid panel found\n");
-		ret = -ENODEV;
-		goto err_pm_runtime_disable;
-	} else
-		dev_info(&device->dev, "GLCD: Found %s panel\n",
-					fb_pdata->type);
-
 	lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
 
 	if (!lcd_cfg) {
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 07/12] video: da8xx-fb: minimal dt support
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

Driver is provided a means to have the probe triggered by DT.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 Documentation/devicetree/bindings/video/fb-da8xx.txt | 16 ++++++++++++++++
 drivers/video/da8xx-fb.c                             |  7 +++++++
 2 files changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt

diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt
new file mode 100644
index 0000000..581e014
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt
@@ -0,0 +1,16 @@
+TI LCD Controller on DA830/DA850/AM335x SoC's
+
+Required properties:
+- compatible:
+	DA830 - "ti,da830-lcdc"
+	AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc"
+- reg: Address range of lcdc register set
+- interrupts: lcdc interrupt
+
+Example:
+
+lcdc@4830e000 {
+	compatible = "ti,am3352-lcdc", "ti,da830-lcdc";
+	reg =  <0x4830e000 0x1000>;
+	interrupts = <36>;
+};
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index b6ea5e9..08ee8eb 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1595,6 +1595,12 @@ static int fb_resume(struct platform_device *dev)
 #define fb_resume NULL
 #endif
 
+static const struct of_device_id da8xx_fb_of_match[] = {
+	{.compatible = "ti,da830-lcdc", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, da8xx_fb_of_match);
+
 static struct platform_driver da8xx_fb_driver = {
 	.probe = fb_probe,
 	.remove = fb_remove,
@@ -1603,6 +1609,7 @@ static struct platform_driver da8xx_fb_driver = {
 	.driver = {
 		   .name = DRIVER_NAME,
 		   .owner = THIS_MODULE,
+		   .of_match_table = of_match_ptr(da8xx_fb_of_match),
 		   },
 };
 
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 08/12] video: da8xx-fb: invoke platform callback safely
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

Ensure that platform data is present before checking whether platform
callback is present (the one used to control backlight). So far this
was not an issue as driver was purely non-DT triggered, but now DT
support has been added.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 08ee8eb..0beed20 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1347,7 +1347,7 @@ static int fb_probe(struct platform_device *device)
 	par->dev = &device->dev;
 	par->lcdc_clk = fb_clk;
 	par->lcd_fck_rate = clk_get_rate(fb_clk);
-	if (fb_pdata->panel_power_ctrl) {
+	if (fb_pdata && fb_pdata->panel_power_ctrl) {
 		par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
 		par->panel_power_ctrl(1);
 	}
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 09/12] video: da8xx-fb: obtain fb_videomode info from dt
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

Obtain fb_videomode details for the connected lcd panel using the
display timing details present in DT.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 .../devicetree/bindings/video/fb-da8xx.txt          | 21 +++++++++++++++++++++
 drivers/video/da8xx-fb.c                            | 17 +++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt
index 581e014..0741f78 100644
--- a/Documentation/devicetree/bindings/video/fb-da8xx.txt
+++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt
@@ -6,6 +6,12 @@ Required properties:
 	AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc"
 - reg: Address range of lcdc register set
 - interrupts: lcdc interrupt
+- display-timings: typical videomode of lcd panel, represented as child.
+  Refer Documentation/devicetree/bindings/video/display-timing.txt for
+  display timing binding details. If multiple videomodes are mentioned
+  in display timings node, typical videomode has to be mentioned as the
+  native mode or it has to be first child (driver cares only for native
+  videomode).
 
 Example:
 
@@ -13,4 +19,19 @@ lcdc@4830e000 {
 	compatible = "ti,am3352-lcdc", "ti,da830-lcdc";
 	reg =  <0x4830e000 0x1000>;
 	interrupts = <36>;
+	display-timings {
+		800x480p62 {
+			clock-frequency = <30000000>;
+			hactive = <800>;
+			vactive = <480>;
+			hfront-porch = <39>;
+			hback-porch = <39>;
+			hsync-len = <47>;
+			vback-porch = <29>;
+			vfront-porch = <13>;
+			vsync-len = <2>;
+			hsync-active = <1>;
+			vsync-active = <1>;
+		};
+	};
 };
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0beed20..0c68712 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -36,6 +36,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/lcm.h>
+#include <video/of_display_timing.h>
 #include <video/da8xx-fb.h>
 #include <asm/div64.h>
 
@@ -1257,8 +1258,24 @@ static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
 {
 	struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
 	struct fb_videomode *lcdc_info;
+	struct device_node *np = dev->dev.of_node;
 	int i;
 
+	if (np) {
+		lcdc_info = devm_kzalloc(&dev->dev,
+					 sizeof(struct fb_videomode),
+					 GFP_KERNEL);
+		if (!lcdc_info) {
+			dev_err(&dev->dev, "memory allocation failed\n");
+			return NULL;
+		}
+		if (of_get_fb_videomode(np, lcdc_info, OF_USE_NATIVE_MODE)) {
+			dev_err(&dev->dev, "timings not available in DT\n");
+			return NULL;
+		}
+		return lcdc_info;
+	}
+
 	for (i = 0, lcdc_info = known_lcd_panels;
 		i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
 		if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 10/12] video: da8xx-fb: ensure pdata only for non-dt
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

This driver is DT probe-able, hence ensure presence of platform data
only for non-DT boot.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0c68712..1c1a616 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1303,7 +1303,7 @@ static int fb_probe(struct platform_device *device)
 	int ret;
 	unsigned long ulcm;
 
-	if (fb_pdata = NULL) {
+	if (fb_pdata = NULL && !device->dev.of_node) {
 		dev_err(&device->dev, "Can not get platform data\n");
 		return -ENOENT;
 	}
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 11/12] video: da8xx-fb: setup struct lcd_ctrl_config for dt
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

strcut lcd_ctrl_config information required for driver is currently
obtained via platform data. To handle DT probing, create
lcd_ctrl_config and populate it with default values, these values are
sufficient for the panels so far used with this controller to work.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 1c1a616..5455682 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1254,6 +1254,35 @@ static struct fb_ops da8xx_fb_ops = {
 	.fb_blank = cfb_blank,
 };
 
+static struct lcd_ctrl_config *da8xx_fb_create_cfg(struct platform_device *dev)
+{
+	struct lcd_ctrl_config *cfg;
+
+	cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode), GFP_KERNEL);
+	if (!cfg) {
+		dev_err(&dev->dev, "memory allocation failed\n");
+		return NULL;
+	}
+
+	/* default values */
+
+	if (lcd_revision = LCD_VERSION_1)
+		cfg->bpp = 16;
+	else
+		cfg->bpp = 32;
+
+	/*
+	 * For panels so far used with this LCDC, below statement is sufficient.
+	 * For new panels, if required, struct lcd_ctrl_cfg fields to be updated
+	 * with additional/modified values. Those values would have to be then
+	 * obtained from dt(requiring new dt bindings).
+	 */
+
+	cfg->panel_shade = COLOR_ACTIVE;
+
+	return cfg;
+}
+
 static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
 {
 	struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
@@ -1345,7 +1374,10 @@ static int fb_probe(struct platform_device *device)
 		break;
 	}
 
-	lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
+	if (device->dev.of_node)
+		lcd_cfg = da8xx_fb_create_cfg(device);
+	else
+		lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
 
 	if (!lcd_cfg) {
 		ret = -EINVAL;
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 12/12] video: da8xx-fb: set upstream clock rate (if reqd)
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

LCDC IP has a clock divider to adjust pixel clock, this limits pixel
clock range to fck/255 - fck/2(fck - rate of input clock to LCDC IP).
In the case of AM335x, where this IP is present, default fck is not
sufficient to provide normal pixel clock rates, hence rendering this
driver unusable on AM335x.

If input clock too is configurable, allowable range of pixel clock
would increase. Here initially it is checked whether with present fck,
divider in IP could be configured to obtain required rate, if not,
fck is adjusted. This makes it usable on AM335x.

Note:
Another solution would be to model an inherited basic clock divider of
CCF, an advantage would be a better possible resolution for pixel clk.
And trying to instantiate a CCF clock would mean that to be consistent,
3 bits being turned on to enable clocks of LCDC IP would have to be
modeled as gate clocks. Now that would bring in a total of 4 clocks,
including necessity to create a new inherited divider clock, and that
mean a branch of clock tree would be present in LCDC driver. This
would add complexity to LCDC driver bringing in considerable amount
of clock handling code, and this would not bring in much advantage
for existing use cases other than providing a higher resolution of
pixel clock. And existing use cases work without relying on clock
modeling. Another fact is that out of the two platform's using this
driver DaVinci is not yet converted to CCF. In future if higher
resolution of pixel clock is required, and probably after DaVinci is
CCF'ed, modeling clock nodes inside driver may be considered.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---

v5: use v2 method of configuring pixel clock rate instead of modeling
    CCF clock nodes in driver, i.e. set divider if rate is within
    the range that is configurable with existing input clock rate,
    else change input clock rate as required.
v4: use new registration for clock divider having minimum divider
    requirement and have ifdef'ery in a better way
v3: model CCF clock divider with parent propogation if CCF selected
v2: new patch

 drivers/video/da8xx-fb.c | 76 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 58 insertions(+), 18 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 5455682..09dfa12 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -133,6 +133,9 @@
 #define WSI_TIMEOUT	50
 #define PALETTE_SIZE	256
 
+#define	CLK_MIN_DIV	2
+#define	CLK_MAX_DIV	255
+
 static void __iomem *da8xx_fb_reg_base;
 static struct resource *lcdc_regs;
 static unsigned int lcd_revision;
@@ -683,23 +686,21 @@ static void da8xx_fb_lcd_reset(void)
 	}
 }
 
-static inline unsigned da8xx_fb_calc_clk_divider(struct da8xx_fb_par *par,
-						 unsigned pixclock)
-{
-	return par->lcd_fck_rate / (PICOS2KHZ(pixclock) * 1000);
-}
-
-static inline unsigned da8xx_fb_round_clk(struct da8xx_fb_par *par,
-					  unsigned pixclock)
+static int da8xx_fb_config_clk_divider(struct da8xx_fb_par *par,
+					      unsigned div, unsigned rate)
 {
-	unsigned div;
+	int ret;
 
-	div = da8xx_fb_calc_clk_divider(par, pixclock);
-	return KHZ2PICOS(par->lcd_fck_rate / (1000 * div));
-}
+	if (par->lcd_fck_rate != rate) {
+		ret = clk_set_rate(par->lcdc_clk, rate);
+		if (IS_ERR_VALUE(ret)) {
+			dev_err(par->dev,
+				"unable to set clock rate at %u\n", rate);
+			return ret;
+		}
+		par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
+	}
 
-static inline void da8xx_fb_config_clk_divider(unsigned div)
-{
 	/* Configure the LCD clock divisor. */
 	lcdc_write(LCD_CLK_DIVISOR(div) |
 			(LCD_RASTER_MODE & 0x1), LCD_CTRL_REG);
@@ -707,14 +708,49 @@ static inline void da8xx_fb_config_clk_divider(unsigned div)
 	if (lcd_revision = LCD_VERSION_2)
 		lcdc_write(LCD_V2_DMA_CLK_EN | LCD_V2_LIDD_CLK_EN |
 				LCD_V2_CORE_CLK_EN, LCD_CLK_ENABLE_REG);
+
+	return 0;
+}
+
+static unsigned int da8xx_fb_calc_clk_divider(struct da8xx_fb_par *par,
+					      unsigned pixclock,
+					      unsigned *rate)
+{
+	unsigned div;
+
+	pixclock = PICOS2KHZ(pixclock) * 1000;
+
+	*rate = par->lcd_fck_rate;
+
+	if (pixclock < (*rate / CLK_MAX_DIV)) {
+		*rate = clk_round_rate(par->lcdc_clk, pixclock * CLK_MAX_DIV);
+		div = CLK_MAX_DIV;
+	} else if (pixclock > (*rate / CLK_MIN_DIV)) {
+		*rate = clk_round_rate(par->lcdc_clk, pixclock * CLK_MIN_DIV);
+		div = CLK_MIN_DIV;
+	} else {
+		div = *rate / pixclock;
+	}
+
+	return div;
 }
 
-static inline void da8xx_fb_calc_config_clk_divider(struct da8xx_fb_par *par,
+static inline int da8xx_fb_calc_config_clk_divider(struct da8xx_fb_par *par,
 						    struct fb_videomode *mode)
 {
-	unsigned div = da8xx_fb_calc_clk_divider(par, mode->pixclock);
+	unsigned rate;
+	unsigned div = da8xx_fb_calc_clk_divider(par, mode->pixclock, &rate);
 
-	da8xx_fb_config_clk_divider(div);
+	return da8xx_fb_config_clk_divider(par, div, rate);
+}
+
+static inline unsigned da8xx_fb_round_clk(struct da8xx_fb_par *par,
+					  unsigned pixclock)
+{
+	unsigned div, rate;
+
+	div = da8xx_fb_calc_clk_divider(par, pixclock, &rate);
+	return KHZ2PICOS(rate / (1000 * div));
 }
 
 static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
@@ -723,7 +759,11 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
 	u32 bpp;
 	int ret = 0;
 
-	da8xx_fb_calc_config_clk_divider(par, panel);
+	ret = da8xx_fb_calc_config_clk_divider(par, panel);
+	if (IS_ERR_VALUE(ret)) {
+		dev_err(par->dev, "unable to configure clock\n");
+		return ret;
+	}
 
 	if (panel->sync & FB_SYNC_CLK_INVERT)
 		lcdc_write((lcdc_read(LCD_RASTER_TIMING_2_REG) |
-- 
1.7.12


^ permalink raw reply related

* RE: [PATCH 1/4] drm/tilcdc: add TI LCD Controller DRM driver (v3)
From: Mohammed, Afzal @ 2013-01-28  9:56 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel@lists.freedesktop.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, patches@linaro.org,
	linux-fbdev@vger.kernel.org, Mike Turquette, Nori, Sekhar
In-Reply-To: <CAF6AEGtrHBxvnp052K6z3aj3dAxJf1hy7H72EoAWr6PhTtwr8Q@mail.gmail.com>

SGkgUm9iLA0KDQpPbiBGcmksIEphbiAyNSwgMjAxMyBhdCAyMDoyMjo1NSwgUm9iIENsYXJrIHdy
b3RlOg0KPiBPbiBGcmksIEphbiAyNSwgMjAxMyBhdCA4OjE1IEFNLCBNb2hhbW1lZCwgQWZ6YWwg
PGFmemFsQHRpLmNvbT4gd3JvdGU6DQoNCj4gPiBJdCdzIG5vdCBhYm91dCBiZWluZyBzaW1wbGUs
IGJ1dCBub3QgZG9pbmcgdGhlIHdyb25nIHdheSwgaGVyZSB5b3UgYXJlDQo+ID4gcmVseWluZyBv
biBhIHBsYXRmb3JtIHNwZWNpZmljIGNsb2NrIGluIGEgZHJpdmVyLCB0aGluayBhYm91dCB0aGUg
Y2FzZQ0KPiA+IHdoZXJlIHNhbWUgSVAgaXMgdXNlZCBvbiBhbm90aGVyIHBsYXRmb3JtLiBFaXRo
ZXIgd2F5IGl0IGlzIG5vdCBhIGdvb2QNCj4gPiB0aGluZyB0byBoYW5kbGUgcGxhdGZvcm0gc3Bl
Y2lmaWMgZGV0YWlscyAoYWJvdXQgZGlzcF9jbGspIGluIGRyaXZlci4NCg0KPiBSaWdodCwgYnV0
IEkgd2FzIHRyeWluZyB0byB1bmRlcnN0YW5kIHdoYXQgd2FzIHRoZSBiZW5lZml0IHRoYXQgdGhl
DQo+IGFkZGVkIGNvbXBsZXhpdHkgaXMuICBJIGRpZG4ndCByZWFsaXplIG9uIGRhdmluY2kgdGhh
dCB5b3UgYXJlIGxpbWl0ZWQNCg0KSGVyZSBJIGFtIHJlZmVycmluZyB0byB1c2FnZSBvZiBkaXNw
X2NsaywNCg0KRHJpdmVyIGlzIG5vdCBzdXBwb3NlZCB0byBkbyBwbGF0Zm9ybSBoYWNrcyAtIGhl
cmUgeW91IGFyZSB0cnlpbmcgdG8NCmNvbmZpZ3VyZSBzb21ldGhpbmcgKFBMTCkgaW4geW91ciBk
cml2ZXIgd2hpY2ggaXMgbm90IHBhcnQgb2YgTENEQyBJUC4NCkFuZCBMQ0RDIElQIGlzIG5vdCBh
d2FyZSBvZiBQTEwgd2hpY2ggaXMgYSBwbGF0Zm9ybSBzcGVjaWZpYyBtYXR0ZXINCihleGlzdGVu
dCBvbmx5IGluIEFNMzM1eCksIGl0IGlzIG9ubHkgYXdhcmUgb2YgZnVuY3Rpb25hbCBjbG9jay4N
Cg0KWW91IGNhbiBzZXQgdGhlIHJhdGUgb24gImZjayIgKGZ1bmN0aW9uYWwgY2xvY2spIGluIEFN
MzM1eCB1c2luZyBteSBwYXRjaCwNCiJBUk06IEFNMzNYWDogY2xvY2s6IFNFVF9SQVRFX1BBUkVO
VCBpbiBsY2QgcGF0aCIsIGFuZCB0aGVyZQ0Kd291bGQgbm90IGJlIGFueSBuZWVkIGZvciBkcml2
ZXIgdG8gYmUgYXdhcmUgb2YgcGxhdGZvcm0gc3BlY2lmaWMgUExMLg0KDQo+ID4+ID4+ICsgICAg
IHByaXYtPmNsayA9IGNsa19nZXQoZGV2LT5kZXYsICJmY2siKTsNCg0KPiA+PiA+PiArICAgICBw
cml2LT5kaXNwX2NsayA9IGNsa19nZXQoZGV2LT5kZXYsICJkcGxsX2Rpc3BfY2siKTsNCg0KPiBh
dCB0aGUgbW9tZW50IGFsbCB0aGlzIGRpc2N1c3Npb24gaXMgYSBiaXQgbW9vdC4gIEknZCBwcm9w
b3NlIGxlYXZpbmcNCj4gdGhlIGRyaXZlciBhcyBpdCBpcyBmb3Igbm93LCBiZWNhdXNlIHRoYXQg
YXQgbGVhc3QgbWFrZXMgaXQgdXNlZnVsIG9uDQo+IGFtMzN4eC4gIEFuZCB3aGVuIENDRiBhbmQg
ZGF2aW5jaSBoYXZlIHRoZSBuZWVkZWQgc3VwcG9ydCBpbiBwbGFjZSwNCg0KTGV0J3MgZm9yZ2V0
IGFib3V0IGxldmVyYWdpbmcgQ0NGIGluIGRyaXZlciwgYnV0IHNhbmUgc29sdXRpb24gdy5yLnQg
UExMDQpjb25maWd1cmF0aW9uIHdvdWxkIGJlIHRvIGRvIGFzIG1lbnRpb25lZCBhYm92ZS4NCg0K
UmVnYXJkcw0KQWZ6YWwNCg0K

^ permalink raw reply

* Re: [git pull] fbcon locking fixes.
From: Maarten Lankhorst @ 2013-01-28 12:45 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Linus Torvalds, m.b.lankhorst, linux-fbdev@vger.kernel.org,
	Linux Kernel Mailing List, DRI mailing list, Andrew Morton
In-Reply-To: <CAPM=9txUNG_CeK+YBhcA_47BVv4Z2GAmEC_J59cY+D9nRgv54A@mail.gmail.com>

Hey,

Op 25-01-13 02:45, Dave Airlie schreef:
> On Fri, Jan 25, 2013 at 11:06 AM, Dave Airlie <airlied@gmail.com> wrote:
>> On Fri, Jan 25, 2013 at 10:53 AM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>> On Thu, Jan 24, 2013 at 4:42 PM, Dave Airlie <airlied@linux.ie> wrote:
>>>> These patches have been sailing around long enough, waiting for a maintainer
>>>> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.
>>> Last this was tried, these patches failed miserably.
>>>
>>> They caused instant lockdep splat and then a total lockup with efifb.
>>> It may be that Takashi's patch helps fix that problem, but it's in no
>>> way clear that it does, so the patch series isn't at all obviously
>>> stable.
>>>
>>> Yes, lockdep is indeed "kinda useful", and there clearly are locking
>>> problems in fbdev. But I'm not seeing myself pulling these for 3.8.
>>> They've been too problematic to pull in at this late stage.
>>>
>> Okay I'll fix the efifb problem and then maybe queue them for -next.
> Okay I've just sent out another fbcon patch to fix the locking harder.
>
> There was a path going into set_con2fb_path if an fb driver was
> already registered, I just pushed the locking out further on anyone
> going in there.
>
> it boots on my EFI macbook here.
>
I cherry picked those patches to my tree, and the full series no longer triggers a lockdep warning.
It also no longer locks up during modprobing or vga-switcheroo either.

Tested-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>

~Maarten

^ permalink raw reply

* Re: [PATCH 1/4] drm/tilcdc: add TI LCD Controller DRM driver (v3)
From: Rob Clark @ 2013-01-28 16:37 UTC (permalink / raw)
  To: Mohammed, Afzal
  Cc: dri-devel@lists.freedesktop.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, patches@linaro.org,
	linux-fbdev@vger.kernel.org, Mike Turquette, Nori, Sekhar
In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A93EA944D2@DBDE01.ent.ti.com>

On Mon, Jan 28, 2013 at 3:56 AM, Mohammed, Afzal <afzal@ti.com> wrote:
> Hi Rob,
>
> On Fri, Jan 25, 2013 at 20:22:55, Rob Clark wrote:
>> On Fri, Jan 25, 2013 at 8:15 AM, Mohammed, Afzal <afzal@ti.com> wrote:
>
>> > It's not about being simple, but not doing the wrong way, here you are
>> > relying on a platform specific clock in a driver, think about the case
>> > where same IP is used on another platform. Either way it is not a good
>> > thing to handle platform specific details (about disp_clk) in driver.
>
>> Right, but I was trying to understand what was the benefit that the
>> added complexity is.  I didn't realize on davinci that you are limited
>
> Here I am referring to usage of disp_clk,
>
> Driver is not supposed to do platform hacks - here you are trying to
> configure something (PLL) in your driver which is not part of LCDC IP.
> And LCDC IP is not aware of PLL which is a platform specific matter
> (existent only in AM335x), it is only aware of functional clock.
>
> You can set the rate on "fck" (functional clock) in AM335x using my patch,
> "ARM: AM33XX: clock: SET_RATE_PARENT in lcd path", and there
> would not be any need for driver to be aware of platform specific PLL.

right, but I think it would be better to just make another patch that
changes tilcdc to just set rate on fck after that patch is merged.  I
mean, I'd rather have the driver at least usable on AM33xx until then,
rather than broken for everyone.

BR,
-R

>> >> >> +     priv->clk = clk_get(dev->dev, "fck");
>
>> >> >> +     priv->disp_clk = clk_get(dev->dev, "dpll_disp_ck");
>
>> at the moment all this discussion is a bit moot.  I'd propose leaving
>> the driver as it is for now, because that at least makes it useful on
>> am33xx.  And when CCF and davinci have the needed support in place,
>
> Let's forget about leveraging CCF in driver, but sane solution w.r.t PLL
> configuration would be to do as mentioned above.
>
> Regards
> Afzal
>

^ permalink raw reply

* Re: [PATCH 1/4] pwm_backlight: Fix PWM levels support in non DT case
From: Thierry Reding @ 2013-01-28 21:01 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Richard Purdie, Grant Likely, Rob Landley,
	Florian Tobias Schandinat, Andrew Morton, devicetree-discuss,
	linux-doc, linux-kernel, linux-fbdev
In-Reply-To: <1358861996-27194-2-git-send-email-peter.ujfalusi@ti.com>

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

On Tue, Jan 22, 2013 at 02:39:53PM +0100, Peter Ujfalusi wrote:
> It is expected that board files would have:
> static unsigned int bl_levels[] = { 0, 50, 100, 150, 200, 250, };
> 
> static struct platform_pwm_backlight_data bl_data = {
> 	.levels = bl_levels,
> 	.max_brightness = ARRAY_SIZE(bl_levels),
> 	.dft_brightness = 4,
> 	.pwm_period_ns = 7812500,
> };
> 
> In this case the max_brightness would be out of range in the levels array.
> Decrement the received max_brightness in every case (DT or non DT) when the
> levels has been provided.

What's wrong with specifying .max_brightness = ARRAY_SIZE(bl_levels) - 1
instead?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Andrew Morton @ 2013-01-29  0:22 UTC (permalink / raw)
  To: Alexander Holler
  Cc: linux-kernel, linux-fbdev, Florian Tobias Schandinat,
	Bernie Thompson, Steve Glendinning, stable
In-Reply-To: <1359139768-32294-2-git-send-email-holler@ahsoftware.de>

On Fri, 25 Jan 2013 19:49:27 +0100
Alexander Holler <holler@ahsoftware.de> wrote:

> When a device was disconnected the driver may hang at waiting for urbs it never
> will get. Fix this by using a timeout while waiting for the used semaphore.
> 
> There is still a memory leak if a timeout happens, but at least the driver
> now continues his disconnect routine.
> 
> ...
>
> --- a/drivers/video/udlfb.c
> +++ b/drivers/video/udlfb.c
> @@ -1832,8 +1832,9 @@ static void dlfb_free_urb_list(struct dlfb_data *dev)
>  	/* keep waiting and freeing, until we've got 'em all */
>  	while (count--) {
>  
> -		/* Getting interrupted means a leak, but ok at disconnect */
> -		ret = down_interruptible(&dev->urbs.limit_sem);
> +		/* Timeout likely occurs at disconnect (resulting in a leak) */
> +		ret = down_timeout_killable(&dev->urbs.limit_sem,
> +						FREE_URB_TIMEOUT);
>  		if (ret)
>  			break;

This is rather a hack.  Do you have an understanding of the underlying
bug?  Why is the driver waiting for things which will never happen?


^ permalink raw reply

* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Alexander Holler @ 2013-01-29  0:56 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, linux-fbdev, Florian Tobias Schandinat,
	Bernie Thompson, Steve Glendinning, stable
In-Reply-To: <20130128162238.7fba92fe.akpm@linux-foundation.org>

Am 29.01.2013 01:22, schrieb Andrew Morton:
> On Fri, 25 Jan 2013 19:49:27 +0100
> Alexander Holler <holler@ahsoftware.de> wrote:
> 
>> When a device was disconnected the driver may hang at waiting for urbs it never
>> will get. Fix this by using a timeout while waiting for the used semaphore.
>>
>> There is still a memory leak if a timeout happens, but at least the driver
>> now continues his disconnect routine.
>>
>> ...
>>
>> --- a/drivers/video/udlfb.c
>> +++ b/drivers/video/udlfb.c
>> @@ -1832,8 +1832,9 @@ static void dlfb_free_urb_list(struct dlfb_data *dev)
>>  	/* keep waiting and freeing, until we've got 'em all */
>>  	while (count--) {
>>  
>> -		/* Getting interrupted means a leak, but ok at disconnect */
>> -		ret = down_interruptible(&dev->urbs.limit_sem);
>> +		/* Timeout likely occurs at disconnect (resulting in a leak) */
>> +		ret = down_timeout_killable(&dev->urbs.limit_sem,
>> +						FREE_URB_TIMEOUT);
>>  		if (ret)
>>  			break;
> 
> This is rather a hack.  Do you have an understanding of the underlying
> bug?  Why is the driver waiting for things which will never happen?

It is a hack, but I don't want to rewrite the whole driver. There is
already an attempt to do so, udl. The hack is a quick solution which
doesn't make something worse, just better. The only drawback might be
the few additional bytes for the implementation of
down_timeout_killable(), but I thought such should be already available,
and wondered it wasn't.

Fixing the underlying problem (including removing the leaks) would just
end up in another rewrite and I prefer to have a look at udl, maybe
fixing the current problems to use such a device as console there.

I've only posted this small patch series, because some (longterm) stable
kernels don't have udl, and smscufx, which is in large parts identical
to udlfb might want that patch too.

Just in case: I don't know anything about smscufx and have discovered
the similarities between those drivers only when I did a git grep to
search for something I fixed with a previous patch.

Regards,

Alexander

^ permalink raw reply

* [PATCH RESEND v4] video: imxfb: Do not crash on reboot
From: Fabio Estevam @ 2013-01-29  2:39 UTC (permalink / raw)
  To: linux-fbdev

From: Fabio Estevam <fabio.estevam@freescale.com>

Issuing a "reboot" command after the LCD times out causes the following
warnings:

Requesting system reboot
------------[ cut here ]------------
WARNING: at drivers/clk/clk.c:471 clk_disable+0x24/0x50()
Modules linked in:
[<c001ad90>] (unwind_backtrace+0x0/0xf4) from [<c0025aac>] (warn_slowpath_common+0x48/0x60)
[<c0025aac>] (warn_slowpath_common+0x48/0x60) from [<c0025ae0>] (warn_slowpath_null+0x1c/0x24)
[<c0025ae0>] (warn_slowpath_null+0x1c/0x24) from [<c03960a0>] (clk_disable+0x24/0x50)
[<c03960a0>] (clk_disable+0x24/0x50) from [<c02695a0>] (imxfb_disable_controller+0x48/0x7c)
[<c02695a0>] (imxfb_disable_controller+0x48/0x7c) from [<c029d838>] (platform_drv_shutdown+0x18/0x1c)
[<c029d838>] (platform_drv_shutdown+0x18/0x1c) from [<c02990fc>] (device_shutdown+0x48/0x14c)
[<c02990fc>] (device_shutdown+0x48/0x14c) from [<c003d09c>] (kernel_restart_prepare+0x2c/0x3c)
[<c003d09c>] (kernel_restart_prepare+0x2c/0x3c) from [<c003d0e4>] (kernel_restart+0xc/0x48)
[<c003d0e4>] (kernel_restart+0xc/0x48) from [<c003d1e8>] (sys_reboot+0xc0/0x1bc)
[<c003d1e8>] (sys_reboot+0xc0/0x1bc) from [<c0014ca0>] (ret_fast_syscall+0x0/0x2c)
---[ end trace da6b502ca79c854f ]---
------------[ cut here ]------------
WARNING: at drivers/clk/clk.c:380 clk_unprepare+0x1c/0x2c()
Modules linked in:
[<c001ad90>] (unwind_backtrace+0x0/0xf4) from [<c0025aac>] (warn_slowpath_common+0x48/0x60)
[<c0025aac>] (warn_slowpath_common+0x48/0x60) from [<c0025ae0>] (warn_slowpath_null+0x1c/0x24)
[<c0025ae0>] (warn_slowpath_null+0x1c/0x24) from [<c0396338>] (clk_unprepare+0x1c/0x2c)
[<c0396338>] (clk_unprepare+0x1c/0x2c) from [<c02695a8>] (imxfb_disable_controller+0x50/0x7c)
[<c02695a8>] (imxfb_disable_controller+0x50/0x7c) from [<c029d838>] (platform_drv_shutdown+0x18/0x1c)
[<c029d838>] (platform_drv_shutdown+0x18/0x1c) from [<c02990fc>] (device_shutdown+0x48/0x14c)
[<c02990fc>] (device_shutdown+0x48/0x14c) from [<c003d09c>] (kernel_restart_prepare+0x2c/0x3c)
[<c003d09c>] (kernel_restart_prepare+0x2c/0x3c) from [<c003d0e4>] (kernel_restart+0xc/0x48)
[<c003d0e4>] (kernel_restart+0xc/0x48) from [<c003d1e8>] (sys_reboot+0xc0/0x1bc)
[<c003d1e8>] (sys_reboot+0xc0/0x1bc) from [<c0014ca0>] (ret_fast_syscall+0x0/0x2c)
---[ end trace da6b502ca79c8550 ]---
------------[ cut here ]------------

This happens because "reboot" triggers imxfb_shutdown(), which calls
imxfb_disable_controller with the clocks already disabled.

To prevent this, add a clock enabled status so that we can check if the clocks
are enabled before disabling them. 

Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v3:
- Changed from 'clks_enabled' to 'enabled' 
Changes since v2:
- Use a better naming for the clk enabled variable
- Return immediately in imxfb_enable_controller/imxfb_disable_controller
if the the clocks are already enabled/disabled.
Changes since v1:
- Protect the whole function instead of only the clocks
 drivers/video/imxfb.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index cf2688d..ce7de9f 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -134,6 +134,7 @@ struct imxfb_info {
 	struct clk		*clk_ipg;
 	struct clk		*clk_ahb;
 	struct clk		*clk_per;
+	int			enabled;
 
 	/*
 	 * These are the addresses we mapped
@@ -513,6 +514,10 @@ static void imxfb_exit_backlight(struct imxfb_info *fbi)
 
 static void imxfb_enable_controller(struct imxfb_info *fbi)
 {
+
+	if (fbi->enabled)
+		return;
+
 	pr_debug("Enabling LCD controller\n");
 
 	writel(fbi->screen_dma, fbi->regs + LCDC_SSA);
@@ -533,6 +538,7 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 	clk_prepare_enable(fbi->clk_ipg);
 	clk_prepare_enable(fbi->clk_ahb);
 	clk_prepare_enable(fbi->clk_per);
+	fbi->enabled = 1;
 
 	if (fbi->backlight_power)
 		fbi->backlight_power(1);
@@ -542,6 +548,9 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 
 static void imxfb_disable_controller(struct imxfb_info *fbi)
 {
+	if (!fbi->enabled)
+		return;
+
 	pr_debug("Disabling LCD controller\n");
 
 	if (fbi->backlight_power)
@@ -552,6 +561,7 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 	clk_disable_unprepare(fbi->clk_per);
 	clk_disable_unprepare(fbi->clk_ipg);
 	clk_disable_unprepare(fbi->clk_ahb);
+	fbi->enabled = 0;
 
 	writel(0, fbi->regs + LCDC_RMCR);
 }
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH RESEND v2] video: Kconfig: Specify the SoCs that make use of FB_IMX
From: Fabio Estevam @ 2013-01-29  2:40 UTC (permalink / raw)
  To: linux-fbdev

From: Fabio Estevam <fabio.estevam@freescale.com>

FB_IMX is the framebuffer driver used by MX1, MX21, MX25 and MX27 processors.

Pass this information to the Kconfig text to make it clear.

Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:
- Add mx1/21 to the list.

 drivers/video/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 0cff083..f0c53b2 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -364,7 +364,7 @@ config FB_SA1100
 	  Y here.
 
 config FB_IMX
-	tristate "Freescale i.MX LCD support"
+	tristate "Freescale i.MX1/21/25/27 LCD support"
 	depends on FB && IMX_HAVE_PLATFORM_IMX_FB
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH RESEND v4] video: imxfb: Do not crash on reboot
From: Shawn Guo @ 2013-01-29  2:47 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1359427171-18109-1-git-send-email-festevam@gmail.com>

Andrew,

On Tue, Jan 29, 2013 at 12:39:31AM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Issuing a "reboot" command after the LCD times out causes the following
> warnings:

Please ignore this one.  It has hit mainline through arm-soc tree
with 3.8-rc series, as we categorized it as a critical bug fixing.

Shawn


^ permalink raw reply

* [PATCH 1/2] video: exynos_dp: add missing of_node_put()
From: Jingoo Han @ 2013-01-29  7:36 UTC (permalink / raw)
  To: linux-fbdev

of_find_node_by_name() returns a node pointer with refcount
incremented, use of_node_put() on it when done.

of_find_node_by_name() will call of_node_put() against
the node pass to from parameter, thus we also need to call
of_node_get(from) before calling of_find_node_by_name().

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 2ed9776..c7374c0 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -965,10 +965,11 @@ static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
 
 static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
 {
-	struct device_node *dp_phy_node;
+	struct device_node *dp_phy_node = of_node_get(dp->dev->of_node);
 	u32 phy_base;
+	int ret = 0;
 
-	dp_phy_node = of_find_node_by_name(dp->dev->of_node, "dptx-phy");
+	dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
 	if (!dp_phy_node) {
 		dev_err(dp->dev, "could not find dptx-phy node\n");
 		return -ENODEV;
@@ -976,22 +977,28 @@ static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
 
 	if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
 		dev_err(dp->dev, "faild to get reg for dptx-phy\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	if (of_property_read_u32(dp_phy_node, "samsung,enable-mask",
 				&dp->enable_mask)) {
 		dev_err(dp->dev, "faild to get enable-mask for dptx-phy\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	dp->phy_addr = ioremap(phy_base, SZ_4);
 	if (!dp->phy_addr) {
 		dev_err(dp->dev, "failed to ioremap dp-phy\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err;
 	}
 
-	return 0;
+err:
+	of_node_put(dp_phy_node);
+
+	return ret;
 }
 
 static void exynos_dp_phy_init(struct exynos_dp_device *dp)
-- 
1.7.2.5



^ permalink raw reply related

* [PATCH 2/2] video: exynos_dp: move disable_irq() to exynos_dp_suspend()
From: Jingoo Han @ 2013-01-29  7:38 UTC (permalink / raw)
  To: linux-fbdev

From: Ajay Kumar <ajaykumar.rs@samsung.com>

disable_irq() should be moved to exynos_dp_suspend(), because
enable_irq() is called at exynos_dp_resume().

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index c7374c0..9ba973a 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -1124,8 +1124,6 @@ static int exynos_dp_remove(struct platform_device *pdev)
 	struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
 	struct exynos_dp_device *dp = platform_get_drvdata(pdev);
 
-	disable_irq(dp->irq);
-
 	flush_work(&dp->hotplug_work);
 
 	if (pdev->dev.of_node) {
@@ -1138,7 +1136,6 @@ static int exynos_dp_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(dp->clock);
 
-
 	return 0;
 }
 
@@ -1148,6 +1145,8 @@ static int exynos_dp_suspend(struct device *dev)
 	struct exynos_dp_platdata *pdata = dev->platform_data;
 	struct exynos_dp_device *dp = dev_get_drvdata(dev);
 
+	disable_irq(dp->irq);
+
 	flush_work(&dp->hotplug_work);
 
 	if (dev->of_node) {
-- 
1.7.2.5



^ permalink raw reply related

* Re: [PATCH 1/4] pwm_backlight: Fix PWM levels support in non DT case
From: Peter Ujfalusi @ 2013-01-29  8:17 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Richard Purdie, Grant Likely, Rob Landley,
	Florian Tobias Schandinat, Andrew Morton, devicetree-discuss,
	linux-doc, linux-kernel, linux-fbdev
In-Reply-To: <20130128210123.GA24673@avionic-0098.mockup.avionic-design.de>

On 01/28/2013 10:01 PM, Thierry Reding wrote:
> On Tue, Jan 22, 2013 at 02:39:53PM +0100, Peter Ujfalusi wrote:
>> It is expected that board files would have:
>> static unsigned int bl_levels[] = { 0, 50, 100, 150, 200, 250, };
>>
>> static struct platform_pwm_backlight_data bl_data = {
>> 	.levels = bl_levels,
>> 	.max_brightness = ARRAY_SIZE(bl_levels),
>> 	.dft_brightness = 4,
>> 	.pwm_period_ns = 7812500,
>> };
>>
>> In this case the max_brightness would be out of range in the levels array.
>> Decrement the received max_brightness in every case (DT or non DT) when the
>> levels has been provided.
> 
> What's wrong with specifying .max_brightness = ARRAY_SIZE(bl_levels) - 1
> instead?

There is nothing wrong with that either but IMHO it is more natural for board
files to use just ARRAY_SIZE(bl_levels). In this way the handling of
data->max_brightness among non DT and DT booted kernel is more uniform in the
driver itself.
Right now all board files are using only the .max_brightness to specify the
maximum value, I could not find any users of .levels in the kernel.

-- 
Péter

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox