Skip to contentSkip to conversation

drm/tilcdc Fixes and cleanups

PATCH In review Jyri Sarha 2 participants 10 messages Jun 17, 2016

Subsystems (4)
lore
Share & tools
lore

Jyri Sarha

author
#0 lore raw

v5 cover drm/tilcdc Fixes and cleanups

Some fixes and cleanups that should get merged to tilcdc even if my atomic changes are still a work in progress.

Yet one more round, sorry for spamming.

Changes since v4:
- Split
"drm/tilcdc: Move LCDC_SYNC_LOST handling inside if (ver == 2) statement" out of
"drm/tilcdc: Write to LCDC_END_OF_INT_IND_REG at the end of IRQ"

Changes since v3:
- "drm/tilcdc: Write to LCDC_END_OF_INT_IND_REG at the end of IRQ"
- Do not bail out in the middle of irq routine, use full

    "if (rev == 2)"-statement in stead

Changes since v2:
- "drm/tilcdc: Restore old dpms state in pm_resume()"
- Improve description
- "drm/tilcdc: Write to LCDC_END_OF_INT_IND_REG at the end of IRQ
- Handle LCDC_FIFO_UNDERFLOW also for v1 silicon
- "drm/tilcdc: Move waiting of LCDC_FRAME_DONE IRQ into stop()"
- Improve description
- Add "drm/tilcdc: Increase time out for waiting frame done interrupt"

Changes since first version:
- "drm/tilcdc: Restore old dpms state in pm_resume()"
- Fix typos from description and subject
- Add "drm/tilcdc: Call drm_crtc_vblank_on() and *_off() in start() and stop()"

Jyri Sarha (8):
drm/tilcdc: Restore old dpms state in pm_resume() drm/tilcdc: Move LCDC_SYNC_LOST handling inside if (ver == 2)
statement
drm/tilcdc: Write to LCDC_END_OF_INT_IND_REG at the end of IRQ
function
drm/tilcdc: Move waiting of LCDC_FRAME_DONE IRQ into stop() drm/tilcdc: Increase time out for waiting frame done interrupt drm/tilcdc: Call drm_crtc_vblank_on() and *_off() in start() and

    stop()

drm/tilcdc: Refer to panel.txt and tfp410.txt bindings in tilcdc.txt drm/tilcdc: Avoid error print by of_graph_get_next_endpoint()

 .../devicetree/bindings/display/tilcdc/tilcdc.txt  |  4 ++
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c               | 78 +++++++++++++---------
 drivers/gpu/drm/tilcdc/tilcdc_drv.c                |  3 +
 drivers/gpu/drm/tilcdc/tilcdc_drv.h                |  2 +
 drivers/gpu/drm/tilcdc/tilcdc_external.c           | 13 +++-
 5 files changed, 67 insertions(+), 33 deletions(-)
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#1 lore raw

PATCH v5 1/8 drm/tilcdc: Restore old dpms state in pm_resume()

Restore old dpms state in pm_resume(). The dpms is turned off in pm_suspend() and it should be restored to its original state in pm_resume(). Without this patch the display is left blanked after a suspend/resume cycle.

Fixes commit 614b3cfeb8d2 ("drm/tilcdc: disable the lcd controller/dma engine when suspend invoked")

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 +++++++
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 3 +++
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  | 2 ++
 3 files changed, 12 insertions(+)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +7 -0
index 79027b1..4d8f9a5 100644
@@ -246,6 +246,13 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode)
 	}
 }
 
+int tilcdc_crtc_current_dpms_state(struct drm_crtc *crtc)
+{
+	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+
+	return tilcdc_crtc->dpms;
+}
+
 static bool tilcdc_crtc_mode_fixup(struct drm_crtc *crtc,
 		const struct drm_display_mode *mode,
 		struct drm_display_mode *adjusted_mode)
drivers/gpu/drm/tilcdc/tilcdc_drv.c +3 -0
index 308e197..148b1ed 100644
@@ -598,6 +598,7 @@ static int tilcdc_pm_suspend(struct device *dev)
 	}
 
 	/* Disable the LCDC controller, to avoid locking up the PRCM */
+	priv->saved_dpms_state = tilcdc_crtc_current_dpms_state(priv->crtc);
 	tilcdc_crtc_dpms(priv->crtc, DRM_MODE_DPMS_OFF);
 
 	/* Save register state: */
@@ -628,6 +629,8 @@ static int tilcdc_pm_resume(struct device *dev)
 					     priv->saved_register[n++]);
 	}
 
+	tilcdc_crtc_dpms(priv->crtc, priv->saved_dpms_state);
+
 	drm_kms_helper_poll_enable(ddev);
 
 	return 0;
drivers/gpu/drm/tilcdc/tilcdc_drv.h +2 -0
index c1de18b..3b52ce8 100644
@@ -67,6 +67,7 @@ struct tilcdc_drm_private {
 
 	/* register contents saved across suspend/resume: */
 	u32 *saved_register;
+	int saved_dpms_state;
 	bool ctx_valid;
 
 #ifdef CONFIG_CPU_FREQ
@@ -172,5 +173,6 @@ void tilcdc_crtc_set_simulate_vesa_sync(struct drm_crtc *crtc,
 int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode);
 int tilcdc_crtc_max_width(struct drm_crtc *crtc);
 void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode);
+int tilcdc_crtc_current_dpms_state(struct drm_crtc *crtc);
 
 #endif /* __TILCDC_DRV_H__ */
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#2 lore raw

PATCH v5 2/8 drm/tilcdc: Move LCDC_SYNC_LOST handling inside if (ver == 2) statement

In reply to Jyri Sarha

Move LCDC_SYNC_LOST handling inside if (ver == 2) statement. LCDC_SYNC_LOST interrupt status bit is only defined for version 2 silicon.

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +10 -11
index 4d8f9a5..6309d3c 100644
@@ -731,18 +731,17 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 			wake_up(&tilcdc_crtc->frame_done_wq);
 		}
 		tilcdc_write(dev, LCDC_END_OF_INT_IND_REG, 0);
-	}
 
-	if (stat & LCDC_SYNC_LOST) {
-		dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
-				    __func__, stat);
-		tilcdc_crtc->frame_intact = false;
-		if (tilcdc_crtc->sync_lost_count++ > SYNC_LOST_COUNT_LIMIT) {
-			dev_err(dev->dev,
-				"%s(0x%08x): Sync lost flood detected, disabling the interrupt",
-				__func__, stat);
-			tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
-				     LCDC_SYNC_LOST);
+		if (stat & LCDC_SYNC_LOST) {
+			dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
+					    __func__, stat);
+			tilcdc_crtc->frame_intact = false;
+			if (tilcdc_crtc->sync_lost_count++ >
+			    SYNC_LOST_COUNT_LIMIT) {
+				dev_err(dev->dev, "%s(0x%08x): Sync lost flood detected, disabling the interrupt", __func__, stat);
+				tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
+					     LCDC_SYNC_LOST);
+			}
 		}
 	}
 
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#3 lore raw

PATCH v5 3/8 drm/tilcdc: Write to LCDC_END_OF_INT_IND_REG at the end of IRQ function

In reply to Jyri Sarha

Reorder the IRQ function so that the write to LCDC_END_OF_INT_IND_REG is done last. The write to LCDC_END_OF_INT_IND_REG indicates to LCDC that the interrupt service routine has completed (see section 13.3.6.1.6 in AM335x TRM). This is needed if LCDC's ipgvmodirq module is configured for pulse interrupts.

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +10 -5
index 6309d3c..65284e9 100644
@@ -725,12 +725,16 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 			tilcdc_crtc->frame_intact = true;
 	}
 
+	if (stat & LCDC_FIFO_UNDERFLOW)
+		dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underfow",
+				    __func__, stat);
+
+	/* For revision 2 only */
 	if (priv->rev == 2) {
 		if (stat & LCDC_FRAME_DONE) {
 			tilcdc_crtc->frame_done = true;
 			wake_up(&tilcdc_crtc->frame_done_wq);
 		}
-		tilcdc_write(dev, LCDC_END_OF_INT_IND_REG, 0);
 
 		if (stat & LCDC_SYNC_LOST) {
 			dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
@@ -743,11 +747,12 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 					     LCDC_SYNC_LOST);
 			}
 		}
-	}
 
-	if (stat & LCDC_FIFO_UNDERFLOW)
-		dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underfow",
-				    __func__, stat);
+		/* Indicate to LCDC that the interrupt service routine has
+		 * completed, see 13.3.6.1.6 in AM335x TRM.
+		 */
+		tilcdc_write(dev, LCDC_END_OF_INT_IND_REG, 0);
+	}
 
 	return IRQ_HANDLED;
 }
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#4 lore raw

PATCH v5 4/8 drm/tilcdc: Move waiting of LCDC_FRAME_DONE IRQ into stop()

In reply to Jyri Sarha

Move wait queue waiting of LCDC_FRAME_DONE IRQ from tilcdc_crtc_dpms() into stop() function. This is just a cleanup and enables independent use of stop() function.

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +16 -15
index 65284e9..4d90509 100644
@@ -113,9 +113,25 @@ static void start(struct drm_crtc *crtc)
 
 static void stop(struct drm_crtc *crtc)
 {
+	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
+	struct tilcdc_drm_private *priv = dev->dev_private;
 
+	tilcdc_crtc->frame_done = false;
 	tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+
+	/*
+	 * if necessary wait for framedone irq which will still come
+	 * before putting things to sleep..
+	 */
+	if (priv->rev == 2) {
+		int ret = wait_event_timeout(tilcdc_crtc->frame_done_wq,
+					     tilcdc_crtc->frame_done,
+					     msecs_to_jiffies(50));
+		if (ret == 0)
+			dev_err(dev->dev, "%s: timeout waiting for framedone\n",
+				__func__);
+	}
 }
 
 static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
@@ -212,22 +228,7 @@ void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode)
 		pm_runtime_get_sync(dev->dev);
 		start(crtc);
 	} else {
-		tilcdc_crtc->frame_done = false;
 		stop(crtc);
-
-		/*
-		 * if necessary wait for framedone irq which will still come
-		 * before putting things to sleep..
-		 */
-		if (priv->rev == 2) {
-			int ret = wait_event_timeout(
-					tilcdc_crtc->frame_done_wq,
-					tilcdc_crtc->frame_done,
-					msecs_to_jiffies(50));
-			if (ret == 0)
-				dev_err(dev->dev, "timeout waiting for framedone\n");
-		}
-
 		pm_runtime_put_sync(dev->dev);
 
 		if (tilcdc_crtc->next_fb) {
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#5 lore raw

PATCH v5 5/8 drm/tilcdc: Increase time out for waiting frame done interrupt

In reply to Jyri Sarha

Increase time out for waiting frame done interrupt. 50ms is long enough for the usual display modes (50 Hz or higher refresh rate), but it may be a bit tight for some unusual mode.

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +1 -1
index 4d90509..419e43f 100644
@@ -127,7 +127,7 @@ static void stop(struct drm_crtc *crtc)
 	if (priv->rev == 2) {
 		int ret = wait_event_timeout(tilcdc_crtc->frame_done_wq,
 					     tilcdc_crtc->frame_done,
-					     msecs_to_jiffies(50));
+					     msecs_to_jiffies(500));
 		if (ret == 0)
 			dev_err(dev->dev, "%s: timeout waiting for framedone\n",
 				__func__);
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#6 lore raw

PATCH v5 6/8 drm/tilcdc: Call drm_crtc_vblank_on() and *_off() in start() and stop()

In reply to Jyri Sarha

Add drm_crtc_vblank_on() and *_off() calls to start() and stop() functions, to make sure any vblank waits etc. gets properly cleaned up.

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +4 -0
index 419e43f..80ae134 100644
@@ -109,6 +109,8 @@ static void start(struct drm_crtc *crtc)
 	tilcdc_clear(dev, LCDC_DMA_CTRL_REG, LCDC_DUAL_FRAME_BUFFER_ENABLE);
 	tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_PALETTE_LOAD_MODE(DATA_ONLY));
 	tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+
+	drm_crtc_vblank_on(crtc);
 }
 
 static void stop(struct drm_crtc *crtc)
@@ -132,6 +134,8 @@ static void stop(struct drm_crtc *crtc)
 			dev_err(dev->dev, "%s: timeout waiting for framedone\n",
 				__func__);
 	}
+
+	drm_crtc_vblank_off(crtc);
 }
 
 static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#7 lore raw

PATCH v5 7/8 drm/tilcdc: Refer to panel.txt and tfp410.txt bindings in tilcdc.txt

In reply to Jyri Sarha

The legacy panel.txt and tfp410.txt bindings are still the only supported way to connect lcd panel and tfp410 DVI encoder to tilcdc.

---
 Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt | 4 ++++
 1 file changed, 4 insertions(+)
Documentation/devicetree/bindings/display/tilcdc/tilcdc.txt +4 -0
index 2136ee8..6efa4c5 100644
@@ -24,6 +24,10 @@ Optional nodes:
    binding follows Documentation/devicetree/bindings/graph.txt and
    suppors a single port with a single endpoint.
 
+ - See also Documentation/devicetree/bindings/display/tilcdc/panel.txt and
+   Documentation/devicetree/bindings/display/tilcdc/tfp410.txt for connecting
+   tfp410 DVI encoder or lcd panel to lcdc
+
 Example:
 
 	fb: fb@4830e000 {
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#8 lore raw

PATCH v5 8/8 drm/tilcdc: Avoid error print by of_graph_get_next_endpoint()

In reply to Jyri Sarha

Avoid error print by of_graph_get_next_endpoint() if there is no ports present.

---
 drivers/gpu/drm/tilcdc/tilcdc_external.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_external.c +11 -2
index 03acb4f..ad3db4d 100644
@@ -138,12 +138,21 @@ static int dev_match_of(struct device *dev, void *data)
 int tilcdc_get_external_components(struct device *dev,
 				   struct component_match **match)
 {
+	struct device_node *node;
 	struct device_node *ep = NULL;
 	int count = 0;
 
-	while ((ep = of_graph_get_next_endpoint(dev->of_node, ep))) {
-		struct device_node *node;
+	/* Avoid error print by of_graph_get_next_endpoint() if there
+	 * is no ports present.
+	 */
+	node = of_get_child_by_name(dev->of_node, "ports");
+	if (!node)
+		node = of_get_child_by_name(dev->of_node, "port");
+	if (!node)
+		return 0;
+	of_node_put(node);
 
+	while ((ep = of_graph_get_next_endpoint(dev->of_node, ep))) {
 		node = of_graph_get_remote_port_parent(ep);
 		if (!node && !of_device_is_available(node)) {
 			of_node_put(node);
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Reviewed-by on the v5 series
#9 lore raw
In reply to Jyri Sarha
Quoted 4 lines · Jyri Sarha

Some fixes and cleanups that should get merged to tilcdc even if my atomic changes are still a work in progress.

Yet one more round, sorry for spamming.

I think these look fine:

Tomi

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel