Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 11/12] OMAPDSS: DPI: verify if DSI PLL is operational
From: Tomi Valkeinen @ 2012-10-30 16:10 UTC (permalink / raw)
  To: archit, linux-omap, linux-fbdev; +Cc: rob, Tomi Valkeinen
In-Reply-To: <1351613409-21186-1-git-send-email-tomi.valkeinen@ti.com>

The SoCs that have DSI module should have a working DSI PLL. However,
some rare boards have not connected the powers to the DSI PLL.

This patch adds a function that tries to power up the DSI PLL, and
reports if that doesn't succeed. DPI uses this function to fall back to
PRCM clocks if DSI PLL doesn't work.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dpi.c |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index fc1ec7d..267caf0 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -361,6 +361,28 @@ void omapdss_dpi_set_data_lines(struct omap_dss_device *dssdev, int data_lines)
 }
 EXPORT_SYMBOL(omapdss_dpi_set_data_lines);
 
+static int __init dpi_verify_dsi_pll(struct platform_device *dsidev)
+{
+	int r;
+
+	/* do initial setup with the PLL to see if it is operational */
+
+	r = dsi_runtime_get(dsidev);
+	if (r)
+		return r;
+
+	r = dsi_pll_init(dsidev, 0, 1);
+	if (r) {
+		dsi_runtime_put(dsidev);
+		return r;
+	}
+
+	dsi_pll_uninit(dsidev, true);
+	dsi_runtime_put(dsidev);
+
+	return 0;
+}
+
 static int __init dpi_init_display(struct omap_dss_device *dssdev)
 {
 	DSSDBG("init_display\n");
@@ -383,6 +405,11 @@ static int __init dpi_init_display(struct omap_dss_device *dssdev)
 		enum omap_dss_clk_source dispc_fclk_src  			dssdev->clocks.dispc.dispc_fclk_src;
 		dpi.dsidev = dpi_get_dsidev(dispc_fclk_src);
+
+		if (dpi_verify_dsi_pll(dpi.dsidev)) {
+			dpi.dsidev = NULL;
+			DSSWARN("DSI PLL not operational\n");
+		}
 	}
 
 	return 0;
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 12/12] OMAPDSS: DPI: always use DSI PLL if available
From: Tomi Valkeinen @ 2012-10-30 16:10 UTC (permalink / raw)
  To: archit, linux-omap, linux-fbdev; +Cc: rob, Tomi Valkeinen
In-Reply-To: <1351613409-21186-1-git-send-email-tomi.valkeinen@ti.com>

We currently get the decision whether to use PRCM or DSI PLL clock for
DPI from the board file. This is not a good way to handle it, and it
won't work with device tree.

This patch changes DPI to always use DSI PLL if it's available.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dpi.c |   64 ++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index 267caf0..32e7dd5 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -49,28 +49,30 @@ static struct {
 	struct omap_dss_output output;
 } dpi;
 
-static struct platform_device *dpi_get_dsidev(enum omap_dss_clk_source clk)
+static struct platform_device *dpi_get_dsidev(enum omap_channel channel)
 {
-	int dsi_module;
-
-	dsi_module = clk = OMAP_DSS_CLK_SRC_DSI_PLL_HSDIV_DISPC ? 0 : 1;
-
-	return dsi_get_dsidev_from_id(dsi_module);
+	switch (channel) {
+	case OMAP_DSS_CHANNEL_LCD:
+		return dsi_get_dsidev_from_id(0);
+	case OMAP_DSS_CHANNEL_LCD2:
+		return dsi_get_dsidev_from_id(1);
+	default:
+		return NULL;
+	}
 }
 
-static bool dpi_use_dsi_pll(struct omap_dss_device *dssdev)
+static enum omap_dss_clk_source dpi_get_alt_clk_src(enum omap_channel channel)
 {
-	if (dssdev->clocks.dispc.dispc_fclk_src =
-			OMAP_DSS_CLK_SRC_DSI_PLL_HSDIV_DISPC ||
-			dssdev->clocks.dispc.dispc_fclk_src =
-			OMAP_DSS_CLK_SRC_DSI2_PLL_HSDIV_DISPC ||
-			dssdev->clocks.dispc.channel.lcd_clk_src =
-			OMAP_DSS_CLK_SRC_DSI_PLL_HSDIV_DISPC ||
-			dssdev->clocks.dispc.channel.lcd_clk_src =
-			OMAP_DSS_CLK_SRC_DSI2_PLL_HSDIV_DISPC)
-		return true;
-	else
-		return false;
+	switch (channel) {
+	case OMAP_DSS_CHANNEL_LCD:
+		return OMAP_DSS_CLK_SRC_DSI_PLL_HSDIV_DISPC;
+	case OMAP_DSS_CHANNEL_LCD2:
+		return OMAP_DSS_CLK_SRC_DSI2_PLL_HSDIV_DISPC;
+	default:
+		/* this shouldn't happen */
+		WARN_ON(1);
+		return OMAP_DSS_CLK_SRC_FCK;
+	}
 }
 
 static int dpi_set_dsi_clk(struct omap_dss_device *dssdev,
@@ -92,7 +94,7 @@ static int dpi_set_dsi_clk(struct omap_dss_device *dssdev,
 		return r;
 
 	dss_select_lcd_clk_source(mgr->id,
-			dssdev->clocks.dispc.channel.lcd_clk_src);
+			dpi_get_alt_clk_src(mgr->id));
 
 	dpi.mgr_config.clock_info = dispc_cinfo;
 
@@ -385,6 +387,8 @@ static int __init dpi_verify_dsi_pll(struct platform_device *dsidev)
 
 static int __init dpi_init_display(struct omap_dss_device *dssdev)
 {
+	struct platform_device *dsidev;
+
 	DSSDBG("init_display\n");
 
 	if (dss_has_feature(FEAT_DPI_USES_VDDS_DSI) &&
@@ -401,17 +405,23 @@ static int __init dpi_init_display(struct omap_dss_device *dssdev)
 		dpi.vdds_dsi_reg = vdds_dsi;
 	}
 
-	if (dpi_use_dsi_pll(dssdev)) {
-		enum omap_dss_clk_source dispc_fclk_src -			dssdev->clocks.dispc.dispc_fclk_src;
-		dpi.dsidev = dpi_get_dsidev(dispc_fclk_src);
+	/*
+	 * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
+	 * source for DPI is SoC integration detail, not something that should
+	 * be configured in the dssdev
+	 */
+	dsidev = dpi_get_dsidev(dssdev->channel);
 
-		if (dpi_verify_dsi_pll(dpi.dsidev)) {
-			dpi.dsidev = NULL;
-			DSSWARN("DSI PLL not operational\n");
-		}
+	if (dpi_verify_dsi_pll(dsidev)) {
+		dsidev = NULL;
+		DSSWARN("DSI PLL not operational\n");
 	}
 
+	if (dsidev)
+		DSSDBG("using DSI PLL for DPI clock\n");
+
+	dpi.dsidev = dsidev;
+
 	return 0;
 }
 
-- 
1.7.10.4


^ permalink raw reply related

* Re: [RFC 0/5] Generic panel framework
From: Laurent Pinchart @ 2012-10-30 16:23 UTC (permalink / raw)
  To: Jun Nie
  Cc: Tomi Valkeinen, linux-fbdev, dri-devel, linux-leds, linux-media,
	Bryan Wu, Richard Purdie, Marcus Lorentzon, Sumit Semwal,
	Archit Taneja, Sebastien Guiriec, Inki Dae, Kyungmin Park
In-Reply-To: <CAGA24MLnW-i0koFuAsnFQ2mNnrLupkmbxW5T8WYiV3QuoA2vig@mail.gmail.com>

Hi Jun,

I've finally been able to resume my work on the panel framework (I hope to 
post a v2 at the end of the week).

On Thursday 23 August 2012 14:23:01 Jun Nie wrote:
> Hi Laurent,
>     Do you plan to add an API to get and parse EDID to mode list?

An API to get the raw EDID data is likely needed. Parsing EDID data in the 
panel driver and providing the modes to the caller isn't enough, as EDID 
contains more than just video modes. I'm not sure whether a driver for an 
EDID-aware panel should parse the EDID data internally and provide both modes 
and raw EDID data, or only raw EDID data.

> video mode is tightly coupled with panel that is capable of hot-plug.
> Or you are busy on modifying EDID parsing code for sharing it amoung
> DRM/FB/etc? I see you mentioned this in Mar.

That's needed as well, but -ENOTIME :-S

> It is great if you are considering add more info into video mode, such as
> pixel repeating, 3D timing related parameter.

Please have a look at "[PATCH 2/2 v6] of: add generic videomode description" 
on dri-devel. There's a proposal for a common video mode structure.

> I have some code for CEA modes filtering and 3D parsing, but still tight
> coupled with FB and with a little hack style.
> 
>     My HDMI driver is implemented as lcd device as you mentioned here.
> But more complex than other lcd devices for a kthread is handling
> hot-plug/EDID/HDCP/ASoC etc.
> 
>     I also feel a little weird to add code parsing HDMI audio related
> info in fbmod.c in my current implementation, thought it is the only
> place to handle EDID in kernel. Your panel framework provide a better
> place to add panel related audio/HDCP code. panel notifier can also
> trigger hot-plug related feature, such as HDCP start.

That's a good idea. I was wondering whether to put the common EDID parser in 
drivers/gpu/drm, drivers/video or drivers/media. Putting it wherever the panel 
framework will be might be a good option as well.

>     Looking forward to your hot-plug panel patch. Or I can help add it
> if you would like me to.

I'll try to post a v2 at the end of the week, but likely without much hot-plug 
support. Patches and enhancement proposals will be welcome.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [RFC 0/5] Generic panel framework
From: Laurent Pinchart @ 2012-10-30 16:35 UTC (permalink / raw)
  To: Zhou Zhu
  Cc: Jun Nie, Tomi Valkeinen, linux-fbdev, dri-devel, linux-leds,
	linux-media, Bryan Wu, Richard Purdie, Marcus Lorentzon,
	Sumit Semwal, Archit Taneja, Sebastien Guiriec, Inki Dae,
	Kyungmin Park
In-Reply-To: <CAJATT-5=kQzUaubL--oRJdm6u8Z10Hus+SMLt3zG1ZSi4QUVWw@mail.gmail.com>

Hi Zhou,

On Tuesday 04 September 2012 16:20:38 Zhou Zhu wrote:
> Hi Laurent,
> 
> Basically I agree that we need a common panel framework. I just have
> some questions:
> 1.  I think we should add color format in videomode - if we use such
> common video mode structure shared across subsystems.
> In HDMI, colors are bind with timings tightly. We need a combined
> videomode with timing and color format together.

What kind of color formats do you have in mind ?

> 2. I think we should add "set_videomode" interface. It helps HDMI
> monitors to set EDIDs.

For panels that support several video modes, sure, we need a way to set the 
video mode. I don't have access to any such panel though, that's why the 
operation has been left out. It wouldn't be difficult to add it when a real 
use case will come up.

What do you mean exactly about HDMI monitors setting EDID ?

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [PATCH] da8xx: Add CDTech_S035Q01 panel (used by LCD3 bone cape)
From: Pantelis Antoniou @ 2012-10-30 18:12 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Pantelis Antoniou, linux-fbdev, linux-kernel, Koen Kooi,
	Matt Porter, Russ Dill, linux-omap

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/video/da8xx-fb.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index c661665..c1f5d30 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -298,6 +298,20 @@ static struct da8xx_panel known_lcd_panels[] = {
 		.pxl_clk = 30000000,
 		.invert_pxl_clk = 0,
 	},
+	[5] = {
+		/* CDTech S035Q01 */
+		.name = "CDTech_S035Q01",
+		.width = 320,
+		.height = 240,
+		.hfp = 58,
+		.hbp = 21,
+		.hsw = 47,
+		.vfp = 23,
+		.vbp = 11,
+		.vsw = 2,
+		.pxl_clk = 8000000,
+		.invert_pxl_clk = 0,
+	},
 };
 
 /* Enable the Raster Engine of the LCD Controller */
-- 
1.7.12


^ permalink raw reply related

* [PATCH] da8xx: Fix revision check on the da8xx driver
From: Pantelis Antoniou @ 2012-10-30 18:14 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Pantelis Antoniou, linux-fbdev, linux-kernel, Koen Kooi,
	Matt Porter, Russ Dill, linux-omap

The revision check fails for the beaglebone; Add new revision ID.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/video/da8xx-fb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 80665f6..866d804 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1283,6 +1283,7 @@ static int __devinit fb_probe(struct platform_device *device)
 		lcd_revision = LCD_VERSION_1;
 		break;
 	case 0x4F200800:
+	case 0x4F201000:
 		lcd_revision = LCD_VERSION_2;
 		break;
 	default:
-- 
1.7.12


^ permalink raw reply related

* [PATCH] pwm-backlight: Pinctrl-fy
From: Pantelis Antoniou @ 2012-10-30 18:14 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Pantelis Antoniou, Richard Purdie, Florian Tobias Schandinat,
	linux-fbdev, linux-kernel, Koen Kooi, Matt Porter, Russ Dill,
	linux-omap

Enable pinctrl for pwm-backlight.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/video/backlight/pwm_bl.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 0c91023..f3b6194 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -20,6 +20,8 @@
 #include <linux/pwm.h>
 #include <linux/pwm_backlight.h>
 #include <linux/slab.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/err.h>
 
 struct pwm_bl_data {
 	struct pwm_device	*pwm;
@@ -180,9 +182,14 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 	struct backlight_properties props;
 	struct backlight_device *bl;
 	struct pwm_bl_data *pb;
+	struct pinctrl *pinctrl;
 	unsigned int max;
 	int ret;
 
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl))
+		dev_warn(&pdev->dev, "unable to select pin group\n");
+
 	if (!data) {
 		ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
 		if (ret < 0) {
-- 
1.7.12


^ permalink raw reply related

* [PATCH] da8xx: De-constify members in the platform config.
From: Pantelis Antoniou @ 2012-10-30 18:15 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Pantelis Antoniou, linux-fbdev, linux-kernel, Koen Kooi,
	Matt Porter, Russ Dill, linux-omap

There's no need for this to be const. It interferes with
creating the platform data dynamically.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 include/video/da8xx-fb.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/video/da8xx-fb.h b/include/video/da8xx-fb.h
index 5a0e4f9..a512d6b 100644
--- a/include/video/da8xx-fb.h
+++ b/include/video/da8xx-fb.h
@@ -35,9 +35,9 @@ struct display_panel {
 };
 
 struct da8xx_lcdc_platform_data {
-	const char manu_name[10];
+	char manu_name[10];
 	void *controller_data;
-	const char type[25];
+	char type[25];
 	void (*panel_power_ctrl)(int);
 };
 
-- 
1.7.12


^ permalink raw reply related

* [PATCH] da8xx-fb: add panel definition for beaglebone LCD7 cape
From: Pantelis Antoniou @ 2012-10-30 18:17 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Koen Kooi, linux-fbdev, linux-kernel, Matt Porter, Russ Dill,
	linux-omap

From: Koen Kooi <koen@dominion.thruhere.net>

Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
---
 drivers/video/da8xx-fb.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 4462d9e..c661665 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -284,6 +284,20 @@ static struct da8xx_panel known_lcd_panels[] = {
 		.pxl_clk = 56000000,
 		.invert_pxl_clk = 0,
 	},
+	/* ThreeFive S9700RTWV35TR */
+	[4] = {
+		.name = "TFC_S9700RTWV35TR_01B",
+		.width = 800,
+		.height = 480,
+		.hfp = 39,
+		.hbp = 39,
+		.hsw = 47,
+		.vfp = 13,
+		.vbp = 29,
+		.vsw = 2,
+		.pxl_clk = 30000000,
+		.invert_pxl_clk = 0,
+	},
 };
 
 /* Enable the Raster Engine of the LCD Controller */
-- 
1.7.12


^ permalink raw reply related

* [PATCH] tps65217: Allow placement elsewhere than parent mfd device
From: Pantelis Antoniou @ 2012-10-30 18:17 UTC (permalink / raw)
  To: Richard Purdie
  Cc: Pantelis Antoniou, Florian Tobias Schandinat, Grant Likely,
	linux-fbdev, devicetree-discuss, linux-kernel, Koen Kooi,
	Matt Porter, Russ Dill, linux-omap

The current code expect the configuration of the backlight to stay
constant after initialization. This patch allows to move it around.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/video/backlight/tps65217_bl.c | 103 ++++++++++++++++++++++++++++++----
 1 file changed, 92 insertions(+), 11 deletions(-)

diff --git a/drivers/video/backlight/tps65217_bl.c b/drivers/video/backlight/tps65217_bl.c
index 7088163..69c1dfe 100644
--- a/drivers/video/backlight/tps65217_bl.c
+++ b/drivers/video/backlight/tps65217_bl.c
@@ -24,8 +24,11 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/of_i2c.h>
 
 struct tps65217_bl {
+	struct i2c_client *i2c_client;
 	struct tps65217 *tps;
 	struct device *dev;
 	struct backlight_device *bl;
@@ -98,8 +101,6 @@ static int tps65217_bl_update_status(struct backlight_device *bl)
 			return rc;
 		}
 
-		dev_dbg(tps65217_bl->dev, "brightness set to %d\n", brightness);
-
 		if (!tps65217_bl->is_enabled)
 			rc = tps65217_bl_enable(tps65217_bl);
 	} else {
@@ -187,14 +188,69 @@ static int tps65217_bl_hw_init(struct tps65217_bl *tps65217_bl,
 
 #ifdef CONFIG_OF
 static struct tps65217_bl_pdata *
-tps65217_bl_parse_dt(struct platform_device *pdev)
+tps65217_bl_parse_dt(struct platform_device *pdev, struct tps65217 **tpsp,
+		int *brightnessp)
 {
-	struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
-	struct device_node *node = of_node_get(tps->dev->of_node);
+	struct i2c_client *i2c_client;
+	struct tps65217 *tps;
+	struct device_node *node, *rnode, *pnode;
 	struct tps65217_bl_pdata *pdata, *err;
+	u32 tps_handle;
 	u32 val;
 
-	node = of_find_node_by_name(node, "backlight");
+	tps = NULL;
+	node = NULL;
+	*brightnessp = 0;
+
+	/* our node (compatible) */
+	pnode = pdev->dev.of_node;
+	if (pnode != NULL &&
+		of_property_read_u32(pnode, "tps", &tps_handle) = 0) {
+		/* we are not instantiated from the mfd */
+		node = of_find_node_by_phandle(tps_handle);
+		if (node = NULL) {
+			dev_err(&pdev->dev, "failed to find the tps node\n");
+			err = ERR_PTR(-EINVAL);
+			goto err;
+		}
+		i2c_client = of_find_i2c_device_by_node(node);
+		if (i2c_client = NULL) {
+			dev_err(&pdev->dev, "failed to find the i2c device "
+					"of tps node\n");
+			err = ERR_PTR(-EINVAL);
+			goto err;
+		}
+		/* yeah this is gross; the whole concept is */
+		tps = i2c_get_clientdata(i2c_client);
+		if (tps = NULL) {
+			dev_err(&pdev->dev, "failed to get tps structure\n");
+			err = ERR_PTR(-EINVAL);
+			goto err;
+		}
+
+		/* read default brightness */
+		val = 0;
+		of_property_read_u32(pnode, "brightness", &val);
+		if (val >= 100)
+			val = 100;
+
+		*brightnessp = val;
+
+		/* no need for this anymore */
+		of_node_put(node);
+
+		dev_info(&pdev->dev, "got tps=%p from handle 0x%x\n", tps, tps_handle);
+	}
+
+	if (tps = NULL)
+		tps = dev_get_drvdata(pdev->dev.parent);
+
+	rnode = of_node_get(tps->dev->of_node);
+
+	node = of_find_node_by_name(rnode, "backlight");
+	of_node_put(rnode);
+	rnode = NULL;
+
 	if (!node)
 		return ERR_PTR(-ENODEV);
 
@@ -247,6 +303,7 @@ tps65217_bl_parse_dt(struct platform_device *pdev)
 
 	of_node_put(node);
 
+	*tpsp = tps;
 	return pdata;
 
 err:
@@ -254,9 +311,16 @@ err:
 
 	return err;
 }
+
+static struct of_device_id tps65217_backlight_of_match[] = {
+	{ .compatible = "tps65217-backlight" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tps65217_backlight_of_match);
 #else
 static struct tps65217_bl_pdata *
-tps65217_bl_parse_dt(struct platform_device *pdev)
+tps65217_bl_parse_dt(struct platform_device *pdev, struct tps65217 **tpsp,
+		int *brightnessp)
 {
 	return NULL;
 }
@@ -265,13 +329,16 @@ tps65217_bl_parse_dt(struct platform_device *pdev)
 static int tps65217_bl_probe(struct platform_device *pdev)
 {
 	int rc;
-	struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
+	struct tps65217 *tps;
 	struct tps65217_bl *tps65217_bl;
 	struct tps65217_bl_pdata *pdata;
 	struct backlight_properties bl_props;
+	int brightness = 0;
 
-	if (tps->dev->of_node) {
-		pdata = tps65217_bl_parse_dt(pdev);
+	tps = NULL;
+
+	if (pdev->dev.of_node) {
+		pdata = tps65217_bl_parse_dt(pdev, &tps, &brightness);
 		if (IS_ERR(pdata))
 			return PTR_ERR(pdata);
 	} else {
@@ -281,6 +348,14 @@ static int tps65217_bl_probe(struct platform_device *pdev)
 		}
 
 		pdata = pdev->dev.platform_data;
+
+		/* get the parent device */
+		tps = dev_get_drvdata(pdev->dev.parent);
+	}
+
+	if (tps = NULL) {
+		dev_err(&pdev->dev, "failed to find tps\n");
+		return -EINVAL;
 	}
 
 	tps65217_bl = devm_kzalloc(&pdev->dev, sizeof(*tps65217_bl),
@@ -311,9 +386,14 @@ static int tps65217_bl_probe(struct platform_device *pdev)
 		return PTR_ERR(tps65217_bl->bl);
 	}
 
-	tps65217_bl->bl->props.brightness = 0;
+	tps65217_bl->bl->props.brightness = brightness;
 	platform_set_drvdata(pdev, tps65217_bl);
 
+	/* update with initial settings */
+	tps65217_bl_update_status(tps65217_bl->bl);
+
+	dev_info(&pdev->dev, "OK.\n");
+
 	return 0;
 }
 
@@ -332,6 +412,7 @@ static struct platform_driver tps65217_bl_driver = {
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "tps65217-bl",
+		.of_match_table	= of_match_ptr(tps65217_backlight_of_match),
 	},
 };
 
-- 
1.7.12


^ permalink raw reply related

* [PATCH] da8xx: Allow use by am33xx based devices
From: Pantelis Antoniou @ 2012-10-30 18:18 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Pantelis Antoniou, linux-fbdev, linux-kernel, Koen Kooi,
	Matt Porter, Russ Dill, linux-omap

This driver can be used for AM33xx devices, like the popular beaglebone.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/video/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 9791d10..e7868d8 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2202,7 +2202,7 @@ config FB_SH7760
 
 config FB_DA8XX
 	tristate "DA8xx/OMAP-L1xx Framebuffer support"
-	depends on FB && ARCH_DAVINCI_DA8XX
+	depends on FB && (ARCH_DAVINCI_DA8XX || SOC_AM33XX)
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
-- 
1.7.12


^ permalink raw reply related

* [PATCH] da8xx: Add standard panel definition
From: Pantelis Antoniou @ 2012-10-30 18:19 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Pantelis Antoniou, linux-fbdev, linux-kernel, Koen Kooi,
	Matt Porter, Russ Dill, linux-omap

Add standard panel definition that can work for the beaglebone
DVI cape.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/video/da8xx-fb.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 866d804..4462d9e 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -270,6 +270,20 @@ static struct da8xx_panel known_lcd_panels[] = {
 		.pxl_clk = 7833600,
 		.invert_pxl_clk = 0,
 	},
+	[3] = {
+		 /* 1024 x 768 @ 60 Hz  Reduced blanking VESA CVT 0.79M3-R */
+		.name = "1024x768@60",
+		.width = 1024,
+		.height = 768,
+		.hfp = 48,
+		.hbp = 80,
+		.hsw = 32,
+		.vfp = 3,
+		.vbp = 15,
+		.vsw = 4,
+		.pxl_clk = 56000000,
+		.invert_pxl_clk = 0,
+	},
 };
 
 /* Enable the Raster Engine of the LCD Controller */
-- 
1.7.12


^ permalink raw reply related

* RE: [PATCH] da8xx: Fix revision check on the da8xx driver
From: Manjunathappa, Prakash @ 2012-10-31  3:51 UTC (permalink / raw)
  To: Pantelis Antoniou, Florian Tobias Schandinat
  Cc: linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Koen Kooi, Porter, Matt, Dill, Russ, linux-omap@vger.kernel.org
In-Reply-To: <1351698984-4007-1-git-send-email-panto@antoniou-consulting.com>

On Wed, Oct 31, 2012 at 21:26:24, Pantelis Antoniou wrote:
> The revision check fails for the beaglebone; Add new revision ID.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/video/da8xx-fb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 80665f6..866d804 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -1283,6 +1283,7 @@ static int __devinit fb_probe(struct platform_device *device)
>  		lcd_revision = LCD_VERSION_1;
>  		break;
>  	case 0x4F200800:
> +	case 0x4F201000:

Thanks for Correcting. This is the LCDC revision on am335x silicon in comparison
with to one read(0x4F200800) on emulator platform.

Acked-by: Manjunathappa, Prakash <prakash.pm@ti.com>

>  		lcd_revision = LCD_VERSION_2;
>  		break;
>  	default:
> -- 
> 1.7.12
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply

* RE: [PATCH] omap2-clk: Add missing lcdc clock definition
From: Hiremath, Vaibhav @ 2012-10-31  5:09 UTC (permalink / raw)
  To: Paul Walmsley, Tony Lindgren
  Cc: Pantelis Antoniou, linux-omap@vger.kernel.org,
	linux-kernel@vger.kernel.org, Koen Kooi, Porter, Matt, Dill, Russ,
	linux-fbdev@vger.kernel.org
In-Reply-To: <alpine.DEB.2.00.1210302324530.6152@utopia.booyaka.com>

On Wed, Oct 31, 2012 at 04:56:40, Paul Walmsley wrote:
> + Vaibhav Hiremath
> 
> On Tue, 30 Oct 2012, Tony Lindgren wrote:
> 
> > * Pantelis Antoniou <panto@antoniou-consulting.com> [121030 11:04]:
> > > Looks like the lcdc clock definition got dropped.
> > > It is required for the LCD controller to work. Reintroduce.
> > 
> > This looks like a regression, can you also add the commit
> > causing it?
> 
> Looks like probably a new "feature," in that this clock didn't exist in 
> the original check-in.  Would be good to get Vaibhav's opinion on this; 
> also the common clock patches will need to be updated.
> 

Thanks Paul for looping me in, something went wrong with my l-o subscription, 
so I didn't receive these Patches. 

As far as lck clock node is concerned, we had deliberately dropped all leaf-
node clocks from the clock tree, please refer to the description mentioned 
in -
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/101987.html


From LCDC driver perspective, driver is using,

fb_clk = clk_get(&device->dev, NULL);

This I feel needs to be corrected for valid name as per Spec (mostly I would 
vote for "fck") and then every platform should make sure that it returns 
valid clock-node for it.

Change in Driver would be,

fb_clk = clk_get(&device->dev, "fck");


Thanks,
Vaibhav

> 
> - Paul
> 


^ permalink raw reply

* RE: [PATCH] da8xx: Allow use by am33xx based devices
From: Manjunathappa, Prakash @ 2012-10-31  5:23 UTC (permalink / raw)
  To: Pantelis Antoniou, Florian Tobias Schandinat
  Cc: linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Koen Kooi, Porter, Matt, Dill, Russ, linux-omap@vger.kernel.org,
	davinci-linux-open-source@linux.davincidsp.com
In-Reply-To: <1351698968-3965-1-git-send-email-panto@antoniou-consulting.com>

Hi,

On Wed, Oct 31, 2012 at 21:26:08, Pantelis Antoniou wrote:
> This driver can be used for AM33xx devices, like the popular beaglebone.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/video/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 9791d10..e7868d8 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2202,7 +2202,7 @@ config FB_SH7760
>  
>  config FB_DA8XX
>  	tristate "DA8xx/OMAP-L1xx Framebuffer support"
> -	depends on FB && ARCH_DAVINCI_DA8XX
> +	depends on FB && (ARCH_DAVINCI_DA8XX || SOC_AM33XX)

Agreed this is present on da8xx and am33xx, but moving forward for
supporting DT, we should be avoiding these dependencies. So instead
change this to remove machine dependencies.

Thanks,
Prakash

>  	select FB_CFB_FILLRECT
>  	select FB_CFB_COPYAREA
>  	select FB_CFB_IMAGEBLIT
> -- 
> 1.7.12
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply

* RE: [PATCH] omap2-clk: Add missing lcdc clock definition
From: Paul Walmsley @ 2012-10-31  5:49 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: Tony Lindgren, Pantelis Antoniou, linux-omap@vger.kernel.org,
	linux-kernel@vger.kernel.org, Koen Kooi, Porter, Matt, Dill, Russ,
	linux-fbdev@vger.kernel.org
In-Reply-To: <79CD15C6BA57404B839C016229A409A83EB529E4@DBDE01.ent.ti.com>

On Wed, 31 Oct 2012, Hiremath, Vaibhav wrote:

> As far as lck clock node is concerned, we had deliberately dropped all leaf-
> node clocks from the clock tree, please refer to the description mentioned 
> in -
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/101987.html

Ach, should have remembered that :-(  Indeed there is an LCDC hwmod:

static struct omap_hwmod am33xx_lcdc_hwmod = {
	.name		= "lcdc",
	.class		= &am33xx_lcdc_hwmod_class,
	.clkdm_name	= "lcdc_clkdm",
	.mpu_irqs	= am33xx_lcdc_irqs,
	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
	.main_clk	= "lcd_gclk",
	.prcm		= {
		.omap4	= {
			.clkctrl_offs	= AM33XX_CM_PER_LCDC_CLKCTRL_OFFSET,
			.modulemode	= MODULEMODE_SWCTRL,
		},
	},
};

> >From LCDC driver perspective, driver is using,
> 
> fb_clk = clk_get(&device->dev, NULL);
> 
> This I feel needs to be corrected for valid name as per Spec (mostly I would 
> vote for "fck") and then every platform should make sure that it returns 
> valid clock-node for it.
> 
> Change in Driver would be,
> 
> fb_clk = clk_get(&device->dev, "fck");

Indeed.


- Paul

^ permalink raw reply

* Re: [PATCH 01/12] OMAPFB: remove use of extended edid block
From: Archit Taneja @ 2012-10-31  6:22 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <1351613409-21186-2-git-send-email-tomi.valkeinen@ti.com>

On Tuesday 30 October 2012 09:39 PM, Tomi Valkeinen wrote:
> It seems that using the second EDID block causes more problems than is
> of any help. The first mode in the extended block will get
> FB_MODE_IS_FIRST set, which will override the first mode from the first
> EDID block, thus making the default videomode selection not to work
> properly.
>
> This patch removes the use of the extended edid block for now.

Looks like you had posted this one(and also the next one)in the previous 
cleanup/fixes series also.

Archit


^ permalink raw reply

* Re: [PATCH 01/12] OMAPFB: remove use of extended edid block
From: Tomi Valkeinen @ 2012-10-31  6:23 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <5090C0CD.7070104@ti.com>

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

On 2012-10-31 08:10, Archit Taneja wrote:
> On Tuesday 30 October 2012 09:39 PM, Tomi Valkeinen wrote:
>> It seems that using the second EDID block causes more problems than is
>> of any help. The first mode in the extended block will get
>> FB_MODE_IS_FIRST set, which will override the first mode from the first
>> EDID block, thus making the default videomode selection not to work
>> properly.
>>
>> This patch removes the use of the extended edid block for now.
> 
> Looks like you had posted this one(and also the next one)in the previous
> cleanup/fixes series also.

Ah, so I did. Too many work branches =). Thanks for pointing it out.

 Tomi




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply

* RE: [PATCH] omap2-clk: Add missing lcdc clock definition
From: Hiremath, Vaibhav @ 2012-10-31  6:38 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Tony Lindgren, Pantelis Antoniou, linux-omap@vger.kernel.org,
	linux-kernel@vger.kernel.org, Koen Kooi, Porter, Matt, Dill, Russ,
	linux-fbdev@vger.kernel.org
In-Reply-To: <alpine.DEB.2.00.1210310547560.8340@utopia.booyaka.com>

On Wed, Oct 31, 2012 at 11:19:44, Paul Walmsley wrote:
> On Wed, 31 Oct 2012, Hiremath, Vaibhav wrote:
> 
> > As far as lck clock node is concerned, we had deliberately dropped all leaf-
> > node clocks from the clock tree, please refer to the description mentioned 
> > in -
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/101987.html
> 
> Ach, should have remembered that :-(  Indeed there is an LCDC hwmod:
> 
> static struct omap_hwmod am33xx_lcdc_hwmod = {
> 	.name		= "lcdc",
> 	.class		= &am33xx_lcdc_hwmod_class,
> 	.clkdm_name	= "lcdc_clkdm",
> 	.mpu_irqs	= am33xx_lcdc_irqs,
> 	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
> 	.main_clk	= "lcd_gclk",
> 	.prcm		= {
> 		.omap4	= {
> 			.clkctrl_offs	= AM33XX_CM_PER_LCDC_CLKCTRL_OFFSET,
> 			.modulemode	= MODULEMODE_SWCTRL,
> 		},
> 	},
> };
> 
> > >From LCDC driver perspective, driver is using,
> > 
> > fb_clk = clk_get(&device->dev, NULL);
> > 
> > This I feel needs to be corrected for valid name as per Spec (mostly I would 
> > vote for "fck") and then every platform should make sure that it returns 
> > valid clock-node for it.
> > 
> > Change in Driver would be,
> > 
> > fb_clk = clk_get(&device->dev, "fck");
> 

Ok, thanks. Let me submit patch for this.

Thanks,
Vaibhav

> Indeed.
> 
> 
> - Paul
> 


^ permalink raw reply

* Re: [PATCH 08/12] OMAPDSS: setup default dss fck
From: Archit Taneja @ 2012-10-31  6:43 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <1351613409-21186-9-git-send-email-tomi.valkeinen@ti.com>

On Tuesday 30 October 2012 09:40 PM, Tomi Valkeinen wrote:
> We don't currently set the dss fck when starting up. This is not a
> problem, as we setup the fck later when configuring the pixel clocks. Or
> this is how it was for omap2, for the rest of the omaps this may not be
> so.
>
> For DSI, HDMI and also for DPI when using DSI PLL, we don't need to
> change the dss fck, and thus it may be left unconfigured. Usually the
> dss fck is already setup fine by default, but we can't trust this.
>
> This patch sets the dss fck to maximum at probe time.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/dss.c |   36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
>
> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
> index 5affa86..034cc1a 100644
> --- a/drivers/video/omap2/dss/dss.c
> +++ b/drivers/video/omap2/dss/dss.c
> @@ -485,6 +485,36 @@ unsigned long dss_get_dpll4_rate(void)
>   		return 0;
>   }
>
> +static int dss_setup_default_clock(void)
> +{
> +	unsigned long max_dss_fck, prate;
> +	unsigned fck_div;
> +	struct dss_clock_info dss_cinfo = { 0 };
> +	int r;
> +
> +	if (dss.dpll4_m4_ck = NULL)
> +		return 0;
> +
> +	max_dss_fck = dss_feat_get_param_max(FEAT_PARAM_DSS_FCK);
> +
> +	prate = dss_get_dpll4_rate();

Not related to this patch, but maybe we could change the 
dss_get_dpll4_rate() name and dss.dpll4_m4_clk to something better. 
Maybe something like dss_fck_parent?

> +
> +	fck_div = DIV_ROUND_UP(prate * dss.feat->dss_fck_multiplier,
> +			max_dss_fck);
> +
> +	dss_cinfo.fck_div = fck_div;
> +
> +	r = dss_calc_clock_rates(&dss_cinfo);
> +	if (r)
> +		return r;
> +
> +	r = dss_set_clock_div(&dss_cinfo);
> +	if (r)
> +		return r;
> +
> +	return 0;
> +}
> +
>   int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
>   		struct dispc_clock_info *dispc_cinfo)
>   {
> @@ -913,6 +943,10 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
>   	dss.lcd_clk_source[0] = OMAP_DSS_CLK_SRC_FCK;
>   	dss.lcd_clk_source[1] = OMAP_DSS_CLK_SRC_FCK;
>
> +	r = dss_setup_default_clock();
> +	if (r)
> +		goto err_setup_clocks;

Maybe it's safer to call this before we do a dss_runtime_get(). On 
OMAP4, DSS_FCLK is needed to access registers also. Changing it's rate 
might not be liked by the DSS HW. Also, it seems more logical to call it 
after dss_get_clocks() in omap_dsshw_probe(), then we sort of group the 
clock related stuff together.

Archit

> +
>   	rev = dss_read_reg(DSS_REVISION);
>   	printk(KERN_INFO "OMAP DSS rev %d.%d\n",
>   			FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
> @@ -923,6 +957,8 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
>
>   	return 0;
>
> +err_setup_clocks:
> +	dss_runtime_put();
>   err_runtime_get:
>   	pm_runtime_disable(&pdev->dev);
>   	dss_put_clocks();
>


^ permalink raw reply

* Re: [PATCH 09/12] OMAPDSS: hide dss_select_dispc_clk_source()
From: Archit Taneja @ 2012-10-31  6:54 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <1351613409-21186-10-git-send-email-tomi.valkeinen@ti.com>

On Tuesday 30 October 2012 09:40 PM, Tomi Valkeinen wrote:
> dss.c currently exposes functions to configure the dispc source clock
> and lcd source clock. There are configured separately from the output
> drivers.
>
> However, there is no safe way for the output drivers to handle dispc
> clock, as it's shared between the outputs. Thus, if, say, the DSI driver
> sets up DSI PLL and configures both the dispc and lcd clock sources to
> that DSI PLL, the resulting dispc clock could be too low for, say, HDMI.
>
> Thus the output drivers should really only be concerned about the lcd
> clock, which is what the output drivers actually use. There's lot to do
> to clean up the dss clock handling, but this patch takes one step
> forward and removes the use of dss_select_dispc_clk_source() from the
> output drivers.
>
> After this patch, the output drivers only configure the lcd source
> clock. On omap4+ the dispc src clock is never changed from the default
> PRCM source. On omap3, where the dispc and lcd clocks are actually the
> same, setting the lcd clock source sets the dispc clock source.

Maybe we could have one call to dss_select_dispc_clk_source() in 
omap_dsshw_porbe(). This is not necessary now, but if we support a 
splash screen on bootloader, and skip the hwmod resets, we might want to 
switch back our dispc clock source to PRCM if the output drivers don't 
to it. This is just a point though, we don't necessarily need it right now.

Archit

^ permalink raw reply

* Re: [PATCH 05/12] OMAPDSS: DSI: skip odd dividers when pck >= 100MHz
From: Archit Taneja @ 2012-10-31  6:57 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <1351613409-21186-6-git-send-email-tomi.valkeinen@ti.com>

On Tuesday 30 October 2012 09:40 PM, Tomi Valkeinen wrote:
> The DSI PLL and HSDivider can be used to generate the pixel clock for
> LCD overlay manager, which then goes to DPI output. On the DPI output
> pin the voltage of the signal is shifted from the OMAP's internal
> minimal voltage to 1.8V range. The shifting is not instant, and the
> higher the clock frequency, the less time there is to shift the signal
> to nominal voltage.
>
> If the HSDivider's divider is greater than one and odd, the resulting
> pixel clock does not have 50% duty cycle. For example, with a divider of
> 3, the duty cycle is 33%.
>
> When combining high frequency (in the area of 140MHz+) and non-50% duty
> cycle, it has been observed the the shifter does not have enough time to
> shift the voltage enough, and this leads to bad signal which is rejected
> by monitors.

Is this something seen on OMAP3 also? I guess it must be since it's the 
same DSI IP.

>
> As a workaround this patch makes the divider calculation skip all odd
> dividers when the required pixel clock is over 100MHz.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/dsi.c |    5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 7d0db2b..d0e35da 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -1386,6 +1386,11 @@ retry:
>   				cur.dsi_pll_hsdiv_dispc_clk >   					cur.clkin4ddr / cur.regm_dispc;
>
> +				if (cur.regm_dispc > 1 &&
> +						cur.regm_dispc % 2 != 0 &&
> +						req_pck >= 1000000)
> +					continue;
> +

Why do we do the req_pck check here? Can't we do it much earlier? We 
could bail out right in the beginning of dsi_pll_calc_clock_div_pck() if 
we see that req_pck is greater than 100 Mhz.

Also, we could maybe have a comment (or in the commit message) saying 
that we chose the 100 Mhz to make it a safe bet.

Archit


^ permalink raw reply

* Re: [PATCH 09/12] OMAPDSS: hide dss_select_dispc_clk_source()
From: Tomi Valkeinen @ 2012-10-31  7:17 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <5090CB17.4070400@ti.com>

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

On 2012-10-31 08:54, Archit Taneja wrote:
> On Tuesday 30 October 2012 09:40 PM, Tomi Valkeinen wrote:
>> dss.c currently exposes functions to configure the dispc source clock
>> and lcd source clock. There are configured separately from the output
>> drivers.
>>
>> However, there is no safe way for the output drivers to handle dispc
>> clock, as it's shared between the outputs. Thus, if, say, the DSI driver
>> sets up DSI PLL and configures both the dispc and lcd clock sources to
>> that DSI PLL, the resulting dispc clock could be too low for, say, HDMI.
>>
>> Thus the output drivers should really only be concerned about the lcd
>> clock, which is what the output drivers actually use. There's lot to do
>> to clean up the dss clock handling, but this patch takes one step
>> forward and removes the use of dss_select_dispc_clk_source() from the
>> output drivers.
>>
>> After this patch, the output drivers only configure the lcd source
>> clock. On omap4+ the dispc src clock is never changed from the default
>> PRCM source. On omap3, where the dispc and lcd clocks are actually the
>> same, setting the lcd clock source sets the dispc clock source.
> 
> Maybe we could have one call to dss_select_dispc_clk_source() in
> omap_dsshw_porbe(). This is not necessary now, but if we support a
> splash screen on bootloader, and skip the hwmod resets, we might want to
> switch back our dispc clock source to PRCM if the output drivers don't
> to it. This is just a point though, we don't necessarily need it right now.

If we're showing the image from the bootloader, we can't change the
clock sources if we want to keep the display working. We either need to
accept and use the config the bootloader did, or reset the dss and start
over with our config.

But you're right, even if we don't support the splash screen from the
bootloader, we can't be sure if the dss hw was reset or not, and what
the clock source is. So it probably is safer to set the clock source at
probe time (and actually all other configs also).

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply

* Re: [PATCH 05/12] OMAPDSS: DSI: skip odd dividers when pck >= 100MHz
From: Tomi Valkeinen @ 2012-10-31  7:26 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <5090C8F9.4060103@ti.com>

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

On 2012-10-31 08:45, Archit Taneja wrote:
> On Tuesday 30 October 2012 09:40 PM, Tomi Valkeinen wrote:
>> The DSI PLL and HSDivider can be used to generate the pixel clock for
>> LCD overlay manager, which then goes to DPI output. On the DPI output
>> pin the voltage of the signal is shifted from the OMAP's internal
>> minimal voltage to 1.8V range. The shifting is not instant, and the
>> higher the clock frequency, the less time there is to shift the signal
>> to nominal voltage.
>>
>> If the HSDivider's divider is greater than one and odd, the resulting
>> pixel clock does not have 50% duty cycle. For example, with a divider of
>> 3, the duty cycle is 33%.
>>
>> When combining high frequency (in the area of 140MHz+) and non-50% duty
>> cycle, it has been observed the the shifter does not have enough time to
>> shift the voltage enough, and this leads to bad signal which is rejected
>> by monitors.
> 
> Is this something seen on OMAP3 also? I guess it must be since it's the
> same DSI IP.

I have not seen this on OMAP3, but I'm 99% sure the same problem happens
there. But I guess there are many small things affecting the signal
quality, it could be that on omap3 beagleboard the resulting signal
voltage is still inside the standard range, even if odd dividers weaken it.

And I also think that we have the same problem with logic and pixel
clock dividers. My understanding is that all these simple dividers (i.e.
not a PLL or such) are made the same way, and, for example, divider of 3
is produced by keeping the output clock low for 2 cycles of the original
clock, and high for 1 cycle. Which leads to 33% duty cycle.

However, as the actual problem only materializes with high frequencies,
in practice we don't have a problem with pck or lck dividers. The reason
is that if we used pcd or lcd of 3, and the resulting pixel clock would
be > 100, the incoming DSS func clock would be around 300. Which is much
over the limit, and thus this scenario doesn't happen.

>> As a workaround this patch makes the divider calculation skip all odd
>> dividers when the required pixel clock is over 100MHz.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/video/omap2/dss/dsi.c |    5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/video/omap2/dss/dsi.c
>> b/drivers/video/omap2/dss/dsi.c
>> index 7d0db2b..d0e35da 100644
>> --- a/drivers/video/omap2/dss/dsi.c
>> +++ b/drivers/video/omap2/dss/dsi.c
>> @@ -1386,6 +1386,11 @@ retry:
>>                   cur.dsi_pll_hsdiv_dispc_clk =
>>                       cur.clkin4ddr / cur.regm_dispc;
>>
>> +                if (cur.regm_dispc > 1 &&
>> +                        cur.regm_dispc % 2 != 0 &&
>> +                        req_pck >= 1000000)
>> +                    continue;
>> +
> 
> Why do we do the req_pck check here? Can't we do it much earlier? We
> could bail out right in the beginning of dsi_pll_calc_clock_div_pck() if
> we see that req_pck is greater than 100 Mhz.

I think you misunderstood the patch. We don't skip or fail calculations
for pck > 100. What we do is we skip odd dividers if pck > 100.

> Also, we could maybe have a comment (or in the commit message) saying
> that we chose the 100 Mhz to make it a safe bet.

Hmm, yes, I should point out that 100MHz is just a guesstimate.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ permalink raw reply

* Re: [PATCH 08/12] OMAPDSS: setup default dss fck
From: Tomi Valkeinen @ 2012-10-31  7:32 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev, rob
In-Reply-To: <5090C5B3.6020502@ti.com>

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

On 2012-10-31 08:31, Archit Taneja wrote:
> On Tuesday 30 October 2012 09:40 PM, Tomi Valkeinen wrote:
>> We don't currently set the dss fck when starting up. This is not a
>> problem, as we setup the fck later when configuring the pixel clocks. Or
>> this is how it was for omap2, for the rest of the omaps this may not be
>> so.
>>
>> For DSI, HDMI and also for DPI when using DSI PLL, we don't need to
>> change the dss fck, and thus it may be left unconfigured. Usually the
>> dss fck is already setup fine by default, but we can't trust this.
>>
>> This patch sets the dss fck to maximum at probe time.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/video/omap2/dss/dss.c |   36
>> ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>>
>> diff --git a/drivers/video/omap2/dss/dss.c
>> b/drivers/video/omap2/dss/dss.c
>> index 5affa86..034cc1a 100644
>> --- a/drivers/video/omap2/dss/dss.c
>> +++ b/drivers/video/omap2/dss/dss.c
>> @@ -485,6 +485,36 @@ unsigned long dss_get_dpll4_rate(void)
>>           return 0;
>>   }
>>
>> +static int dss_setup_default_clock(void)
>> +{
>> +    unsigned long max_dss_fck, prate;
>> +    unsigned fck_div;
>> +    struct dss_clock_info dss_cinfo = { 0 };
>> +    int r;
>> +
>> +    if (dss.dpll4_m4_ck == NULL)
>> +        return 0;
>> +
>> +    max_dss_fck = dss_feat_get_param_max(FEAT_PARAM_DSS_FCK);
>> +
>> +    prate = dss_get_dpll4_rate();
> 
> Not related to this patch, but maybe we could change the
> dss_get_dpll4_rate() name and dss.dpll4_m4_clk to something better.
> Maybe something like dss_fck_parent?

I agree. Or, even better, we should fix the omap clk data/code so that
we could set the dss fck, without this trickery. But I have no idea if
that's easy or difficult.

>> +
>> +    fck_div = DIV_ROUND_UP(prate * dss.feat->dss_fck_multiplier,
>> +            max_dss_fck);
>> +
>> +    dss_cinfo.fck_div = fck_div;
>> +
>> +    r = dss_calc_clock_rates(&dss_cinfo);
>> +    if (r)
>> +        return r;
>> +
>> +    r = dss_set_clock_div(&dss_cinfo);
>> +    if (r)
>> +        return r;
>> +
>> +    return 0;
>> +}
>> +
>>   int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info
>> *dss_cinfo,
>>           struct dispc_clock_info *dispc_cinfo)
>>   {
>> @@ -913,6 +943,10 @@ static int __init omap_dsshw_probe(struct
>> platform_device *pdev)
>>       dss.lcd_clk_source[0] = OMAP_DSS_CLK_SRC_FCK;
>>       dss.lcd_clk_source[1] = OMAP_DSS_CLK_SRC_FCK;
>>
>> +    r = dss_setup_default_clock();
>> +    if (r)
>> +        goto err_setup_clocks;
> 
> Maybe it's safer to call this before we do a dss_runtime_get(). On
> OMAP4, DSS_FCLK is needed to access registers also. Changing it's rate
> might not be liked by the DSS HW. Also, it seems more logical to call it
> after dss_get_clocks() in omap_dsshw_probe(), then we sort of group the
> clock related stuff together.

Yes, good point. I don't think DSS has any problems with the clock
changing, as long as it's not outputting an image (but I'm not sure). In
any case, it makes sense to setup the clocks dss_get_clocks as you said.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

^ 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