[PATCH V2 0/7] drm/tilcdc: bug fixes, mode selection improvements

[PATCH V2 0/7] drm/tilcdc: bug fixes, mode selection improvements

[Darren Etheridge]

The series of patches that follow are intended to address issues that
have been found in the tilcdc drm driver. The patchset enables support
for screen resolutions with horizontal resolutions greater than 1024
pixels.  The patchset also addresses a limitation where certain
monitor timings would overflow LCD controller timing registers causing
either no monitor signal or a very corrupted display.  This patchset
will stop monitor modes from being reported as valid if the lcd
controller cannot support them.  

V2:
	Fix typos in commit messages
	Add a patch that enables runtime modesetting to work correctly
	Fix an issue where the slave encoder can initialize before the i2c
		subsystem, resulting in no displays being configured.

Applies cleanly on drm-next.

Darren Etheridge (6):
  drm/tilcdc: support pixel widths greater than 1024
  drm/tilcdc: adding some more devicetree config
  drm/tilcdc: fixing off by one errors found on analyzer
  drm/tilcdc: adding more guards to prevent selection of invalid modes
  drm/tilcdc: whitespace fixes and tidyup
  drm/tilcdc fixing i2c/slave initialization race

Pantelis Antoniou (1):
  drm/tilcdc: Clear bits of register we're going to set.

 .../devicetree/bindings/drm/tilcdc/tilcdc.txt      |    8 ++
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c               |  117 +++++++++++++++++--
 drivers/gpu/drm/tilcdc/tilcdc_drv.c                |   25 ++++-
 drivers/gpu/drm/tilcdc/tilcdc_drv.h                |   24 ++++-
 drivers/gpu/drm/tilcdc/tilcdc_regs.h               |    1 +
 drivers/gpu/drm/tilcdc/tilcdc_slave.c              |   53 +++++----
 6 files changed, 189 insertions(+), 39 deletions(-)

[PATCH V2 1/7] drm/tilcdc: support pixel widths greater than 1024

[Darren Etheridge]

TI LCD controller version 2 has an extended eleventh
bit that enables horizontal resolutions greater than
1024 pixels to be specified (upto 2048).  This patch
adds support for setting this bit on LCDC V2.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |   15 +++++++++++++++
 drivers/gpu/drm/tilcdc/tilcdc_regs.h |    1 +
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 5dd3c7d..84fdf25 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -310,6 +310,21 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,
 		((vsw & 0x3f) << 10);
 	tilcdc_write(dev, LCDC_RASTER_TIMING_1_REG, reg);
 
+	/*
+	 * be sure to set Bit 10 for the V2 LCDC controller,
+	 * otherwise limited to 1024 pixels width, stopping
+	 * 1920x1080 being suppoted.
+	 */
+	if (priv->rev == 2) {
+		if ((mode->vdisplay - 1) & 0x400) {
+			tilcdc_set(dev, LCDC_RASTER_TIMING_2_REG,
+				LCDC_LPP_B10);
+		} else {
+			tilcdc_clear(dev, LCDC_RASTER_TIMING_2_REG,
+				LCDC_LPP_B10);
+		}
+	}
+
 	/* Configure display type: */
 	reg = tilcdc_read(dev, LCDC_RASTER_CTRL_REG) &
 		~(LCDC_TFT_MODE | LCDC_MONO_8BIT_MODE | LCDC_MONOCHROME_MODE |
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
index 17fd1b4..1bf5e25 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
@@ -80,6 +80,7 @@
 #define LCDC_INVERT_PIXEL_CLOCK                  BIT(22)
 #define LCDC_INVERT_HSYNC                        BIT(21)
 #define LCDC_INVERT_VSYNC                        BIT(20)
+#define LCDC_LPP_B10                             BIT(26)
 
 /* LCDC Block */
 #define LCDC_PID_REG                             0x0
-- 
1.7.0.4

[PATCH V2 2/7] drm/tilcdc: adding some more devicetree config

[Darren Etheridge]

Adding support for max-pixelclock and max-width device tree
entries. As some devices that use the tilcdc hardware module
have restrictions on the allowed/tested values.  Also update DT
bindings document to reflect new parameters.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
 .../devicetree/bindings/drm/tilcdc/tilcdc.txt      |    8 +++++++
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c               |   23 ++++++++++++++++++-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c                |   15 ++++++++++++-
 drivers/gpu/drm/tilcdc/tilcdc_drv.h                |   22 +++++++++++++++++++
 4 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/drm/tilcdc/tilcdc.txt b/Documentation/devicetree/bindings/drm/tilcdc/tilcdc.txt
index e5f1301..fff10da 100644
--- a/Documentation/devicetree/bindings/drm/tilcdc/tilcdc.txt
+++ b/Documentation/devicetree/bindings/drm/tilcdc/tilcdc.txt
@@ -10,6 +10,14 @@ Recommended properties:
    services interrupts for this device.
  - ti,hwmods: Name of the hwmod associated to the LCDC
 
+Optional properties:
+ - max-bandwidth: The maximum pixels per second that the memory
+   interface / lcd controller combination can sustain
+ - max-width: The maximum horizontal pixel width supported by
+   the lcd controller.
+ - max-pixelclock: The maximum pixel clock that can be supported
+   by the lcd controller in KHz.
+
 Example:
 
 	fb: fb@4830e000 {
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 84fdf25..05f2b14 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -448,10 +448,29 @@ int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode)
 	if (mode->vdisplay > 2048)
 		return MODE_VIRTUAL_Y;
 
+	/*
+	 * some devices have a maximum allowed pixel clock
+	 * configured from the DT
+	 */
+	if (mode->clock > priv->max_pixelclock) {
+		DBG("Pruning mode, pixel clock too high");
+		return MODE_CLOCK_HIGH;
+	}
+
+	/*
+	 * some devices further limit the max horizontal resolution
+	 * configured from the DT
+	 */
+	if (mode->hdisplay > priv->max_width)
+		return MODE_BAD_WIDTH;
+
 	/* filter out modes that would require too much memory bandwidth: */
-	bandwidth = mode->hdisplay * mode->vdisplay * drm_mode_vrefresh(mode);
-	if (bandwidth > priv->max_bandwidth)
+	bandwidth = mode->hdisplay * mode->vdisplay *
+		drm_mode_vrefresh(mode);
+	if (bandwidth > priv->max_bandwidth) {
+		DBG("Pruning mode, exceeds defined bandwidth limit");
 		return MODE_BAD;
+	}
 
 	return MODE_OK;
 }
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index f2a6528..1e8f273 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -212,7 +212,20 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 #endif
 
 	if (of_property_read_u32(node, "max-bandwidth", &priv->max_bandwidth))
-		priv->max_bandwidth = 1280 * 1024 * 60;
+		priv->max_bandwidth = TILCDC_DEFAULT_MAX_BANDWIDTH;
+
+	DBG("Maximum Bandwidth Value %d", priv->max_bandwidth);
+
+	if (of_property_read_u32(node, "ti,max-width", &priv->max_width))
+		priv->max_width = TILCDC_DEFAULT_MAX_WIDTH;
+
+	DBG("Maximum Horizontal Pixel Width Value %dpixels", priv->max_width);
+
+	if (of_property_read_u32(node, "ti,max-pixelclock",
+					&priv->max_pixelclock))
+		priv->max_pixelclock = TILCDC_DEFAULT_MAX_PIXELCLOCK;
+
+	DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock);
 
 	pm_runtime_enable(dev->dev);
 
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 0906843..66df316 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -34,6 +34,18 @@
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 
+/* Defaulting to pixel clock defined on AM335x */
+#define TILCDC_DEFAULT_MAX_PIXELCLOCK  126000
+/* Defaulting to max width as defined on AM335x */
+#define TILCDC_DEFAULT_MAX_WIDTH  2048
+/*
+ * This may need some tweaking, but want to allow at least 1280x1024@60
+ * with optimized DDR & EMIF settings tweaked 1920x1080@24 appears to
+ * be supportable
+ */
+#define TILCDC_DEFAULT_MAX_BANDWIDTH  (1280*1024*60)
+
+
 struct tilcdc_drm_private {
 	void __iomem *mmio;
 
@@ -43,6 +55,16 @@ struct tilcdc_drm_private {
 
 	/* don't attempt resolutions w/ higher W * H * Hz: */
 	uint32_t max_bandwidth;
+	/*
+	 * Pixel Clock will be restricted to some value as
+	 * defined in the device datasheet measured in KHz
+	 */
+	uint32_t max_pixelclock;
+	/*
+	 * Max allowable width is limited on a per device basis
+	 * measured in pixels
+	 */
+	uint32_t max_width;
 
 	/* register contents saved across suspend/resume: */
 	u32 saved_register[12];
-- 
1.7.0.4

[PATCH V2 3/7] drm/tilcdc: fixing off by one errors found on analyzer

[Darren Etheridge]

When hooking up to an HDMI analyzer noticed some timings were
off by one.  Referring to the hardware technical reference manual
for the lcd controller some of the timing registers use 0 to
represent 1.  This patch addresses that issue.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 05f2b14..4455a41 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -289,17 +289,22 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,
 	reg = tilcdc_read(dev, LCDC_RASTER_TIMING_2_REG) & ~0x000fff00;
 	reg |= LCDC_AC_BIAS_FREQUENCY(info->ac_bias) |
 		LCDC_AC_BIAS_TRANSITIONS_PER_INT(info->ac_bias_intrpt);
+
+	/*
+	 * subtract one from hfp, hbp, hsw because the hardware uses
+	 * a value of 0 as 1
+	 */
 	if (priv->rev == 2) {
-		reg |= (hfp & 0x300) >> 8;
-		reg |= (hbp & 0x300) >> 4;
-		reg |= (hsw & 0x3c0) << 21;
+		reg |= ((hfp-1) & 0x300) >> 8;
+		reg |= ((hbp-1) & 0x300) >> 4;
+		reg |= ((hsw-1) & 0x3c0) << 21;
 	}
 	tilcdc_write(dev, LCDC_RASTER_TIMING_2_REG, reg);
 
 	reg = (((mode->hdisplay >> 4) - 1) << 4) |
-		((hbp & 0xff) << 24) |
-		((hfp & 0xff) << 16) |
-		((hsw & 0x3f) << 10);
+		(((hbp-1) & 0xff) << 24) |
+		(((hfp-1) & 0xff) << 16) |
+		(((hsw-1) & 0x3f) << 10);
 	if (priv->rev == 2)
 		reg |= (((mode->hdisplay >> 4) - 1) & 0x40) >> 3;
 	tilcdc_write(dev, LCDC_RASTER_TIMING_0_REG, reg);
@@ -307,7 +312,7 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,
 	reg = ((mode->vdisplay - 1) & 0x3ff) |
 		((vbp & 0xff) << 24) |
 		((vfp & 0xff) << 16) |
-		((vsw & 0x3f) << 10);
+		(((vsw-1) & 0x3f) << 10);
 	tilcdc_write(dev, LCDC_RASTER_TIMING_1_REG, reg);
 
 	/*
-- 
1.7.0.4

[PATCH V2 4/7] drm/tilcdc: adding more guards to prevent selection of invalid modes

[Darren Etheridge]

The tilcdc has a number of limitations for the allowed sizes of
the various adjustable timing parameter.  Some modes are outside
of these timings.  This commit will prune modes that report timings
that will overflow the allowed sizes in the tilcdc.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |   46 ++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 4455a41..283e0a6 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -442,7 +442,12 @@ int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode)
 {
 	struct tilcdc_drm_private *priv = crtc->dev->dev_private;
 	unsigned int bandwidth;
+	uint32_t hbp, hfp, hsw, vbp, vfp, vsw;
 
+	/*
+	 * check to see if the width is within the range that
+	 * the LCD Controller physically supports
+	 */
 	if (mode->hdisplay > tilcdc_crtc_max_width(crtc))
 		return MODE_VIRTUAL_X;
 
@@ -453,6 +458,47 @@ int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode)
 	if (mode->vdisplay > 2048)
 		return MODE_VIRTUAL_Y;
 
+	DBG("Processing mode %dx%d@%d with pixel clock %d",
+		mode->hdisplay, mode->vdisplay,
+		drm_mode_vrefresh(mode), mode->clock);
+
+	hbp = mode->htotal - mode->hsync_end;
+	hfp = mode->hsync_start - mode->hdisplay;
+	hsw = mode->hsync_end - mode->hsync_start;
+	vbp = mode->vtotal - mode->vsync_end;
+	vfp = mode->vsync_start - mode->vdisplay;
+	vsw = mode->vsync_end - mode->vsync_start;
+
+	if ((hbp-1) & ~0x3ff) {
+		DBG("Pruning mode: Horizontal Back Porch out of range");
+		return MODE_HBLANK_WIDE;
+	}
+
+	if ((hfp-1) & ~0x3ff) {
+		DBG("Pruning mode: Horizontal Front Porch out of range");
+		return MODE_HBLANK_WIDE;
+	}
+
+	if ((hsw-1) & ~0x3ff) {
+		DBG("Pruning mode: Horizontal Sync Width out of range");
+		return MODE_HSYNC_WIDE;
+	}
+
+	if (vbp & ~0xff) {
+		DBG("Pruning mode: Vertical Back Porch out of range");
+		return MODE_VBLANK_WIDE;
+	}
+
+	if (vfp & ~0xff) {
+		DBG("Pruning mode: Vertical Front Porch out of range");
+		return MODE_VBLANK_WIDE;
+	}
+
+	if ((vsw-1) & ~0x3f) {
+		DBG("Pruning mode: Vertical Sync Width out of range");
+		return MODE_VSYNC_WIDE;
+	}
+
 	/*
 	 * some devices have a maximum allowed pixel clock
 	 * configured from the DT
-- 
1.7.0.4

[PATCH V2 5/7] drm/tilcdc: whitespace fixes and tidyup

[Darren Etheridge]

keeping checkpatch happy.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 283e0a6..6118651 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -42,7 +42,8 @@ struct tilcdc_crtc {
 
 static void unref_worker(struct work_struct *work)
 {
-	struct tilcdc_crtc *tilcdc_crtc = container_of(work, struct tilcdc_crtc, work);
+	struct tilcdc_crtc *tilcdc_crtc =
+		container_of(work, struct tilcdc_crtc, work);
 	struct drm_device *dev = tilcdc_crtc->base.dev;
 	struct drm_framebuffer *fb;
 
@@ -55,10 +56,12 @@ static void unref_worker(struct work_struct *work)
 static void set_scanout(struct drm_crtc *crtc, int n)
 {
 	static const uint32_t base_reg[] = {
-			LCDC_DMA_FB_BASE_ADDR_0_REG, LCDC_DMA_FB_BASE_ADDR_1_REG,
+			LCDC_DMA_FB_BASE_ADDR_0_REG,
+			LCDC_DMA_FB_BASE_ADDR_1_REG,
 	};
 	static const uint32_t ceil_reg[] = {
-			LCDC_DMA_FB_CEILING_ADDR_0_REG, LCDC_DMA_FB_CEILING_ADDR_1_REG,
+			LCDC_DMA_FB_CEILING_ADDR_0_REG,
+			LCDC_DMA_FB_CEILING_ADDR_1_REG,
 	};
 	static const uint32_t stat[] = {
 			LCDC_END_OF_FRAME0, LCDC_END_OF_FRAME1,
@@ -194,7 +197,8 @@ static void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode)
 		tilcdc_crtc->frame_done = false;
 		stop(crtc);
 
-		/* if necessary wait for framedone irq which will still come
+		/*
+		 * if necessary wait for framedone irq which will still come
 		 * before putting things to sleep..
 		 */
 		if (priv->rev == 2) {
@@ -504,7 +508,7 @@ int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode)
 	 * configured from the DT
 	 */
 	if (mode->clock > priv->max_pixelclock) {
-		DBG("Pruning mode, pixel clock too high");
+		DBG("Pruning mode: pixel clock too high");
 		return MODE_CLOCK_HIGH;
 	}
 
@@ -519,7 +523,7 @@ int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode)
 	bandwidth = mode->hdisplay * mode->vdisplay *
 		drm_mode_vrefresh(mode);
 	if (bandwidth > priv->max_bandwidth) {
-		DBG("Pruning mode, exceeds defined bandwidth limit");
+		DBG("Pruning mode: exceeds defined bandwidth limit");
 		return MODE_BAD;
 	}
 
-- 
1.7.0.4

[PATCH V2 6/7] drm/tilcdc fixing i2c/slave initialization race

[Darren Etheridge]

In certain senarios drm will initialize before i2c this means that i2c
slave devices like the nxp tda998x will fail to be probed.  This patch
detects this condition then defers the probe of the slave device and
the tilcdc main driver.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c   |   10 ++++++
 drivers/gpu/drm/tilcdc/tilcdc_drv.h   |    2 +-
 drivers/gpu/drm/tilcdc/tilcdc_slave.c |   53 ++++++++++++++++++---------------
 3 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 1e8f273..40b71da 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -26,6 +26,7 @@
 #include "drm_fb_helper.h"
 
 static LIST_HEAD(module_list);
+static bool slave_probing;
 
 void tilcdc_module_init(struct tilcdc_module *mod, const char *name,
 		const struct tilcdc_module_ops *funcs)
@@ -41,6 +42,11 @@ void tilcdc_module_cleanup(struct tilcdc_module *mod)
 	list_del(&mod->list);
 }
 
+void tilcdc_slave_probedefer(bool defered)
+{
+	slave_probing = defered;
+}
+
 static struct of_device_id tilcdc_of_match[];
 
 static struct drm_framebuffer *tilcdc_fb_create(struct drm_device *dev,
@@ -580,6 +586,10 @@ static int tilcdc_pdev_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
+	/* defer probing if slave is in deferred probing */
+	if (slave_probing == true)
+		return -EPROBE_DEFER;
+
 	return drm_platform_init(&tilcdc_driver, pdev);
 }
 
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 66df316..0938036 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -117,7 +117,7 @@ struct tilcdc_module {
 void tilcdc_module_init(struct tilcdc_module *mod, const char *name,
 		const struct tilcdc_module_ops *funcs);
 void tilcdc_module_cleanup(struct tilcdc_module *mod);
-
+void tilcdc_slave_probedefer(bool defered);
 
 /* Panel config that needs to be set in the crtc, but is not coming from
  * the mode timings.  The display module is expected to call
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
index 8bf4fd1..dfffaf0 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
@@ -298,6 +298,7 @@ static int slave_probe(struct platform_device *pdev)
 	struct tilcdc_module *mod;
 	struct pinctrl *pinctrl;
 	uint32_t i2c_phandle;
+	struct i2c_adapter *slavei2c;
 	int ret = -EINVAL;
 
 	/* bail out early if no DT data: */
@@ -306,44 +307,48 @@ static int slave_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
-	slave_mod = kzalloc(sizeof(*slave_mod), GFP_KERNEL);
-	if (!slave_mod)
-		return -ENOMEM;
-
-	mod = &slave_mod->base;
-
-	tilcdc_module_init(mod, "slave", &slave_module_ops);
-
-	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
-	if (IS_ERR(pinctrl))
-		dev_warn(&pdev->dev, "pins are not configured\n");
-
+	/* Bail out early if i2c not specified */
 	if (of_property_read_u32(node, "i2c", &i2c_phandle)) {
 		dev_err(&pdev->dev, "could not get i2c bus phandle\n");
-		goto fail;
+		return ret;
 	}
 
-	mod->preferred_bpp = slave_info.bpp;
-
 	i2c_node = of_find_node_by_phandle(i2c_phandle);
 	if (!i2c_node) {
 		dev_err(&pdev->dev, "could not get i2c bus node\n");
-		goto fail;
+		return ret;
 	}
 
-	slave_mod->i2c = of_find_i2c_adapter_by_node(i2c_node);
-	if (!slave_mod->i2c) {
+	/* but defer the probe if it can't be initialized it might come later */
+	slavei2c = of_find_i2c_adapter_by_node(i2c_node);
+	of_node_put(i2c_node);
+
+	if (!slavei2c) {
+		ret = -EPROBE_DEFER;
+		tilcdc_slave_probedefer(true);
 		dev_err(&pdev->dev, "could not get i2c\n");
-		goto fail;
+		return ret;
 	}
 
-	of_node_put(i2c_node);
+	slave_mod = kzalloc(sizeof(*slave_mod), GFP_KERNEL);
+	if (!slave_mod)
+		return -ENOMEM;
 
-	return 0;
+	mod = &slave_mod->base;
 
-fail:
-	slave_destroy(mod);
-	return ret;
+	mod->preferred_bpp = slave_info.bpp;
+
+	slave_mod->i2c = slavei2c;
+
+	tilcdc_module_init(mod, "slave", &slave_module_ops);
+
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl))
+		dev_warn(&pdev->dev, "pins are not configured\n");
+
+	tilcdc_slave_probedefer(false);
+
+	return 0;
 }
 
 static int slave_remove(struct platform_device *pdev)
-- 
1.7.0.4

[PATCH V2 7/7] drm/tilcdc: Clear bits of register we're going to set.

[Darren Etheridge]

From: Pantelis Antoniou <panto@antoniou-consulting.com>

Bits weren't cleared so resolution changes didn't work.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 6118651..eb06f70 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -299,6 +299,8 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,
 	 * a value of 0 as 1
 	 */
 	if (priv->rev == 2) {
+		/* clear bits we're going to set */
+		reg &= ~0x78000033;
 		reg |= ((hfp-1) & 0x300) >> 8;
 		reg |= ((hbp-1) & 0x300) >> 4;
 		reg |= ((hsw-1) & 0x3c0) << 21;
-- 
1.7.0.4

Re: [PATCH V2 0/7] drm/tilcdc: bug fixes, mode selection improvements

[Rob Clark]

On Fri, Jun 21, 2013 at 2:52 PM, Darren Etheridge <detheridge@ti.com> wrote:
> The series of patches that follow are intended to address issues that
> have been found in the tilcdc drm driver. The patchset enables support
> for screen resolutions with horizontal resolutions greater than 1024
> pixels.  The patchset also addresses a limitation where certain
> monitor timings would overflow LCD controller timing registers causing
> either no monitor signal or a very corrupted display.  This patchset
> will stop monitor modes from being reported as valid if the lcd
> controller cannot support them.
>
> V2:
>         Fix typos in commit messages
>         Add a patch that enables runtime modesetting to work correctly
>         Fix an issue where the slave encoder can initialize before the i2c
>                 subsystem, resulting in no displays being configured.
>
> Applies cleanly on drm-next.
>
> Darren Etheridge (6):
>   drm/tilcdc: support pixel widths greater than 1024
>   drm/tilcdc: adding some more devicetree config
>   drm/tilcdc: fixing off by one errors found on analyzer
>   drm/tilcdc: adding more guards to prevent selection of invalid modes
>   drm/tilcdc: whitespace fixes and tidyup
>   drm/tilcdc fixing i2c/slave initialization race
>
> Pantelis Antoniou (1):
>   drm/tilcdc: Clear bits of register we're going to set.

for the series:
Acked-by: Rob Clark <robdclark@gmail.com>

the i2c/slave race thing.. well, isn't pretty.. but right now I don't
see a better way.  I guess at some point we need to revisit how the
encoder-slave stuff gets loaded in drm

BR,
-R

>
>  .../devicetree/bindings/drm/tilcdc/tilcdc.txt      |    8 ++
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c               |  117 +++++++++++++++++--
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c                |   25 ++++-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.h                |   24 ++++-
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h               |    1 +
>  drivers/gpu/drm/tilcdc/tilcdc_slave.c              |   53 +++++----
>  6 files changed, 189 insertions(+), 39 deletions(-)
>

Re: [PATCH V2 0/7] drm/tilcdc: bug fixes, mode selection improvements

[Pantelis Antoniou]

Hi Rob,

On Jun 26, 2013, at 6:42 PM, Rob Clark wrote:

> On Fri, Jun 21, 2013 at 2:52 PM, Darren Etheridge <detheridge@ti.com> wrote:
>> The series of patches that follow are intended to address issues that
>> have been found in the tilcdc drm driver. The patchset enables support
>> for screen resolutions with horizontal resolutions greater than 1024
>> pixels.  The patchset also addresses a limitation where certain
>> monitor timings would overflow LCD controller timing registers causing
>> either no monitor signal or a very corrupted display.  This patchset
>> will stop monitor modes from being reported as valid if the lcd
>> controller cannot support them.
>> 
>> V2:
>>        Fix typos in commit messages
>>        Add a patch that enables runtime modesetting to work correctly
>>        Fix an issue where the slave encoder can initialize before the i2c
>>                subsystem, resulting in no displays being configured.
>> 
>> Applies cleanly on drm-next.
>> 
>> Darren Etheridge (6):
>>  drm/tilcdc: support pixel widths greater than 1024
>>  drm/tilcdc: adding some more devicetree config
>>  drm/tilcdc: fixing off by one errors found on analyzer
>>  drm/tilcdc: adding more guards to prevent selection of invalid modes
>>  drm/tilcdc: whitespace fixes and tidyup
>>  drm/tilcdc fixing i2c/slave initialization race
>> 
>> Pantelis Antoniou (1):
>>  drm/tilcdc: Clear bits of register we're going to set.
> 
> for the series:
> Acked-by: Rob Clark <robdclark@gmail.com>
> 

I have some more DT related patches. Turns out there are some modes
that just don't get right, so I've implemented mode black-lists &
white-lists based on DT properties. But they sure look generic to me.

Perhaps they should be better reworked and added as general helpers
for drivers to use.

> the i2c/slave race thing.. well, isn't pretty.. but right now I don't
> see a better way.  I guess at some point we need to revisit how the
> encoder-slave stuff gets loaded in drm
> 

Oh yeah, we depend on the order of initialization for this to work.

> BR,
> -R
> 

Regards

-- Pantelis

>> 
>> .../devicetree/bindings/drm/tilcdc/tilcdc.txt      |    8 ++
>> drivers/gpu/drm/tilcdc/tilcdc_crtc.c               |  117 +++++++++++++++++--
>> drivers/gpu/drm/tilcdc/tilcdc_drv.c                |   25 ++++-
>> drivers/gpu/drm/tilcdc/tilcdc_drv.h                |   24 ++++-
>> drivers/gpu/drm/tilcdc/tilcdc_regs.h               |    1 +
>> drivers/gpu/drm/tilcdc/tilcdc_slave.c              |   53 +++++----
>> 6 files changed, 189 insertions(+), 39 deletions(-)
>> 

Re: [PATCH V2 0/7] drm/tilcdc: bug fixes, mode selection improvements

[Darren Etheridge]

Rob Clark <robdclark@gmail.com> wrote on Wed [2013-Jun-26 11:42:44 -0400]:
> On Fri, Jun 21, 2013 at 2:52 PM, Darren Etheridge <detheridge@ti.com> wrote:
> > The series of patches that follow are intended to address issues that
> > have been found in the tilcdc drm driver. The patchset enables support
> > for screen resolutions with horizontal resolutions greater than 1024
> > pixels.  The patchset also addresses a limitation where certain
> > monitor timings would overflow LCD controller timing registers causing
> > either no monitor signal or a very corrupted display.  This patchset
> > will stop monitor modes from being reported as valid if the lcd
> > controller cannot support them.
> >
> > V2:
> >         Fix typos in commit messages
> >         Add a patch that enables runtime modesetting to work correctly
> >         Fix an issue where the slave encoder can initialize before the i2c
> >                 subsystem, resulting in no displays being configured.
> >
> for the series:
> Acked-by: Rob Clark <robdclark@gmail.com>
> 
> the i2c/slave race thing.. well, isn't pretty.. but right now I don't
> see a better way.  I guess at some point we need to revisit how the
> encoder-slave stuff gets loaded in drm
> 
Thanks Rob.  
Yes I agree with your assessment,  I looked at using the
module list as a way to indicate if a given module is deferred or not
but this had its own difficulties.  This patch was the easiest way to
defer the probing at the earliest possible point for both the slave
module and the main tilcdc driver.  

Reordering the drivers/Makefile was the other suggestion that was
given. By moving the Makefile drm entry to a point after the i2c entry
the order that the initilization happens is changed. This certainly
appeared to work when I tried it. However I was really concerned that this
would have horrible side effects for drivers that I had no knowledge about
and no way of testing.  So my provided solution while not elegant its
impact is at least localized to the tilcdc driver.

Darren