[PATCH v2 0/7] drm/tilcdc: LCDC Revision 1 related fixes

[PATCH v2 0/7] drm/tilcdc: LCDC Revision 1 related fixes

[Jyri Sarha]

Changes since first version of the series:

- Move tilcdc_regs.h changes from "drm/tilcdc: Enable palette loading
  for revision 2 LCDC too" to "drm/tilcdc: Add tilcdc_write_mask() to
  tilcdc_regs.h"

These patches are inspired by this series form Bartosz Golaszewski:
https://www.spinics.net/lists/arm-kernel/msg539629.html

The patches are based on drm-next plus the earlier patches that I plan
to send in a pull request for 4.10. The base + these patches are
pushed here:

https://github.com/jsarha/linux drm-next-tilcdc-for-4.10-wip

Bartosz, please test if this branch works for rev1 LCDC, with your dts
file!

I am still waiting for Russell King to send a pull request for his
fixes for tda998x for the removal of drm_connector_register() in the
driver. That is why I still have my "drm/i2c: tda998x: Remove obsolete
drm_connector_register() call"-patch on the top of the above branch.

Bartosz Golaszewski (1):
  drm: tilcdc: implement palette loading for rev1

Jyri Sarha (6):
  drm/tilcdc: Enable sync lost error and recovery handling for rev 1
    LCDC
  drm/tilcdc: Fix tilcdc_crtc_create() return value handling
  drm/tilcdc: Free palette dma memory in tilcdc_crtc_destroy()
  drm/tilcdc: Add tilcdc_write_mask() to tilcdc_regs.h
  drm/tilcdc: Enable palette loading for revision 2 LCDC too
  drm/tilcdc: Load palette at the end of mode_set_nofb()

 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 138 ++++++++++++++++++++++++++++-------
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  23 ++++--
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |   3 +-
 drivers/gpu/drm/tilcdc/tilcdc_regs.h |  14 ++++
 4 files changed, 144 insertions(+), 34 deletions(-)

-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/7] drm/tilcdc: Enable sync lost error and recovery handling for rev 1 LCDC

[Jyri Sarha]

Revision 1 LCDC support also sync lost errors and can benefit from
sync lost recovery routine.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 39 ++++++++++++++++++------------------
 drivers/gpu/drm/tilcdc/tilcdc_regs.h |  1 +
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index c787349..dfe5796 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -113,8 +113,8 @@ static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
 
 	if (priv->rev == 1) {
 		tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
-			LCDC_V1_UNDERFLOW_INT_ENA);
-		tilcdc_set(dev, LCDC_DMA_CTRL_REG,
+			LCDC_V1_UNDERFLOW_INT_ENA |
+			LCDC_V1_SYNC_LOST_ENA |
 			LCDC_V1_END_OF_FRAME_INT_ENA);
 	} else {
 		tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG,
@@ -131,8 +131,9 @@ static void tilcdc_crtc_disable_irqs(struct drm_device *dev)
 	/* disable irqs that we might have enabled: */
 	if (priv->rev == 1) {
 		tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
-			LCDC_V1_UNDERFLOW_INT_ENA | LCDC_V1_PL_INT_ENA);
-		tilcdc_clear(dev, LCDC_DMA_CTRL_REG,
+			LCDC_V1_UNDERFLOW_INT_ENA |
+			LCDC_V1_PL_INT_ENA |
+			LCDC_V1_SYNC_LOST_ENA |
 			LCDC_V1_END_OF_FRAME_INT_ENA);
 	} else {
 		tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
@@ -845,6 +846,21 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 		dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underflow",
 				    __func__, stat);
 
+	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, recovering", __func__, stat);
+			queue_work(system_wq,
+				   &tilcdc_crtc->recover_work);
+			tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
+				     LCDC_SYNC_LOST);
+			tilcdc_crtc->sync_lost_count = 0;
+		}
+	}
+
 	/* For revision 2 only */
 	if (priv->rev == 2) {
 		if (stat & LCDC_FRAME_DONE) {
@@ -852,21 +868,6 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 			wake_up(&tilcdc_crtc->frame_done_wq);
 		}
 
-		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, recovering", __func__, stat);
-				queue_work(system_wq,
-					   &tilcdc_crtc->recover_work);
-				tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
-					     LCDC_SYNC_LOST);
-				tilcdc_crtc->sync_lost_count = 0;
-			}
-		}
-
 		/* Indicate to LCDC that the interrupt service routine has
 		 * completed, see 13.3.6.1.6 in AM335x TRM.
 		 */
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
index f57c0d6..beb8c21 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
@@ -61,6 +61,7 @@
 #define LCDC_V2_UNDERFLOW_INT_ENA                BIT(5)
 #define LCDC_V1_PL_INT_ENA                       BIT(4)
 #define LCDC_V2_PL_INT_ENA                       BIT(6)
+#define LCDC_V1_SYNC_LOST_ENA                    BIT(5)
 #define LCDC_MONOCHROME_MODE                     BIT(1)
 #define LCDC_RASTER_ENABLE                       BIT(0)
 #define LCDC_TFT_ALT_ENABLE                      BIT(23)
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/7] drm: tilcdc: implement palette loading for rev1

[Jyri Sarha]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Revision 1 of the IP doesn't work if we don't load the palette (even
if it's not used, which is the case for the RGB565 format).

Add a function called from tilcdc_crtc_enable() which performs all
required actions if we're dealing with a rev1 chip.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 88 +++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index dfe5796..c13e7a3 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -21,11 +21,15 @@
 #include <drm/drm_flip_work.h>
 #include <drm/drm_plane_helper.h>
 #include <linux/workqueue.h>
+#include <linux/completion.h>
+#include <linux/dma-mapping.h>
 
 #include "tilcdc_drv.h"
 #include "tilcdc_regs.h"
 
-#define TILCDC_VBLANK_SAFETY_THRESHOLD_US 1000
+#define TILCDC_VBLANK_SAFETY_THRESHOLD_US	1000
+#define TILCDC_REV1_PALETTE_SIZE		32
+#define TILCDC_REV1_PALETTE_FIRST_ENTRY		0x4000
 
 struct tilcdc_crtc {
 	struct drm_crtc base;
@@ -56,6 +60,10 @@ struct tilcdc_crtc {
 	int sync_lost_count;
 	bool frame_intact;
 	struct work_struct recover_work;
+
+	dma_addr_t palette_dma_handle;
+	void *palette_base;
+	struct completion palette_loaded;
 };
 #define to_tilcdc_crtc(x) container_of(x, struct tilcdc_crtc, base)
 
@@ -105,6 +113,55 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
 	tilcdc_crtc->curr_fb = fb;
 }
 
+/*
+ * The driver currently only supports the RGB565 format for revision 1. For
+ * 16 bits-per-pixel the palette block is bypassed, but the first 32 bytes of
+ * the framebuffer are still considered palette. The first 16-bit entry must
+ * be 0x4000 while all other entries must be zeroed.
+ */
+static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
+{
+	u32 dma_fb_base, dma_fb_ceiling, raster_ctl;
+	struct tilcdc_crtc *tilcdc_crtc;
+	struct drm_device *dev;
+	u16 *first_entry;
+
+	dev = crtc->dev;
+	tilcdc_crtc = to_tilcdc_crtc(crtc);
+	first_entry = tilcdc_crtc->palette_base;
+
+	*first_entry = TILCDC_REV1_PALETTE_FIRST_ENTRY;
+
+	dma_fb_base = tilcdc_read(dev, LCDC_DMA_FB_BASE_ADDR_0_REG);
+	dma_fb_ceiling = tilcdc_read(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG);
+	raster_ctl = tilcdc_read(dev, LCDC_RASTER_CTRL_REG);
+
+	/* Tell the LCDC where the palette is located. */
+	tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG,
+		     tilcdc_crtc->palette_dma_handle);
+	tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG,
+		     (u32)tilcdc_crtc->palette_dma_handle
+				+ TILCDC_REV1_PALETTE_SIZE - 1);
+
+	/* Load it. */
+	tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
+		     LCDC_PALETTE_LOAD_MODE(DATA_ONLY));
+	tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
+		   LCDC_PALETTE_LOAD_MODE(PALETTE_ONLY));
+
+	/* Enable the LCDC and wait for palette to be loaded. */
+	tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
+	tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+
+	wait_for_completion(&tilcdc_crtc->palette_loaded);
+
+	/* Restore the registers. */
+	tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+	tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, dma_fb_base);
+	tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG, dma_fb_ceiling);
+	tilcdc_write(dev, LCDC_RASTER_CTRL_REG, raster_ctl);
+}
+
 static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
 {
 	struct tilcdc_drm_private *priv = dev->dev_private;
@@ -160,6 +217,7 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+	struct tilcdc_drm_private *priv = dev->dev_private;
 
 	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
 	mutex_lock(&tilcdc_crtc->enable_lock);
@@ -172,6 +230,9 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
 
 	reset(crtc);
 
+	if (priv->rev == 1 && !completion_done(&tilcdc_crtc->palette_loaded))
+		tilcdc_crtc_load_palette(crtc);
+
 	tilcdc_crtc_enable_irqs(dev);
 
 	tilcdc_clear(dev, LCDC_DMA_CTRL_REG, LCDC_DUAL_FRAME_BUFFER_ENABLE);
@@ -213,6 +274,13 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool shutdown)
 				__func__);
 	}
 
+	/*
+	 * LCDC will not retain the palette when reset. Make sure it gets
+	 * reloaded on tilcdc_crtc_enable().
+	 */
+	if (priv->rev == 1)
+		reinit_completion(&tilcdc_crtc->palette_loaded);
+
 	drm_crtc_vblank_off(crtc);
 
 	tilcdc_crtc_disable_irqs(dev);
@@ -846,6 +914,14 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 		dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underflow",
 				    __func__, stat);
 
+	if (priv->rev == 1) {
+		if (stat & LCDC_PL_LOAD_DONE) {
+			complete(&tilcdc_crtc->palette_loaded);
+			tilcdc_clear(dev,
+				     LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
+		}
+	}
+
 	if (stat & LCDC_SYNC_LOST) {
 		dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
 				    __func__, stat);
@@ -890,6 +966,16 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)
 		return NULL;
 	}
 
+	if (priv->rev == 1) {
+		init_completion(&tilcdc_crtc->palette_loaded);
+		tilcdc_crtc->palette_base = dmam_alloc_coherent(dev->dev,
+					TILCDC_REV1_PALETTE_SIZE,
+					&tilcdc_crtc->palette_dma_handle,
+					GFP_KERNEL | __GFP_ZERO);
+		if (!tilcdc_crtc->palette_base)
+			return ERR_PTR(-ENOMEM);
+	}
+
 	crtc = &tilcdc_crtc->base;
 
 	ret = tilcdc_plane_init(dev, &tilcdc_crtc->primary);
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/7] drm/tilcdc: Fix tilcdc_crtc_create() return value handling

[Jyri Sarha]

Failed tilcdc_crtc_create() error handling was broken, this patch
should fix it.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 12 +++++++-----
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 11 ++++-------
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  2 +-
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index c13e7a3..67b50c0 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -953,7 +953,7 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 	return IRQ_HANDLED;
 }
 
-struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)
+int tilcdc_crtc_create(struct drm_device *dev)
 {
 	struct tilcdc_drm_private *priv = dev->dev_private;
 	struct tilcdc_crtc *tilcdc_crtc;
@@ -963,7 +963,7 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)
 	tilcdc_crtc = devm_kzalloc(dev->dev, sizeof(*tilcdc_crtc), GFP_KERNEL);
 	if (!tilcdc_crtc) {
 		dev_err(dev->dev, "allocation failed\n");
-		return NULL;
+		return -ENOMEM;
 	}
 
 	if (priv->rev == 1) {
@@ -973,7 +973,7 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)
 					&tilcdc_crtc->palette_dma_handle,
 					GFP_KERNEL | __GFP_ZERO);
 		if (!tilcdc_crtc->palette_base)
-			return ERR_PTR(-ENOMEM);
+			return -ENOMEM;
 	}
 
 	crtc = &tilcdc_crtc->base;
@@ -1016,13 +1016,15 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)
 		if (!crtc->port) { /* This should never happen */
 			dev_err(dev->dev, "Port node not found in %s\n",
 				dev->dev->of_node->full_name);
+			ret = -EINVAL;
 			goto fail;
 		}
 	}
 
-	return crtc;
+	priv->crtc = crtc;
+	return 0;
 
 fail:
 	tilcdc_crtc_destroy(crtc);
-	return NULL;
+	return -ENOMEM;
 }
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index af959df..28e97d5 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -153,13 +153,11 @@ static int tilcdc_commit(struct drm_device *dev,
 	.atomic_commit = tilcdc_commit,
 };
 
-static int modeset_init(struct drm_device *dev)
+static void modeset_init(struct drm_device *dev)
 {
 	struct tilcdc_drm_private *priv = dev->dev_private;
 	struct tilcdc_module *mod;
 
-	priv->crtc = tilcdc_crtc_create(dev);
-
 	list_for_each_entry(mod, &module_list, list) {
 		DBG("loading module: %s", mod->name);
 		mod->funcs->modeset_init(mod, dev);
@@ -170,8 +168,6 @@ static int modeset_init(struct drm_device *dev)
 	dev->mode_config.max_width = tilcdc_crtc_max_width(priv->crtc);
 	dev->mode_config.max_height = 2048;
 	dev->mode_config.funcs = &mode_config_funcs;
-
-	return 0;
 }
 
 #ifdef CONFIG_CPU_FREQ
@@ -370,11 +366,12 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
 		}
 	}
 
-	ret = modeset_init(ddev);
+	ret = tilcdc_crtc_create(ddev);
 	if (ret < 0) {
-		dev_err(dev, "failed to initialize mode setting\n");
+		dev_err(dev, "failed to create crtc\n");
 		goto init_failed;
 	}
+	modeset_init(ddev);
 
 	if (priv->is_componentized) {
 		ret = component_bind_all(dev, ddev);
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 283ff28..7913b0e 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -167,7 +167,7 @@ struct tilcdc_panel_info {
 
 #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
 
-struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev);
+int tilcdc_crtc_create(struct drm_device *dev);
 irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc);
 void tilcdc_crtc_update_clk(struct drm_crtc *crtc);
 void tilcdc_crtc_set_panel_info(struct drm_crtc *crtc,
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 4/7] drm/tilcdc: Free palette dma memory in tilcdc_crtc_destroy()

[Jyri Sarha]

We should free the palette dma memory too.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 67b50c0..6d2ce53b 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -355,6 +355,10 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
 	of_node_put(crtc->port);
 	drm_crtc_cleanup(crtc);
 	drm_flip_work_cleanup(&tilcdc_crtc->unref_work);
+
+	dmam_free_coherent(crtc->dev->dev, TILCDC_REV1_PALETTE_SIZE,
+			   tilcdc_crtc->palette_base,
+			   tilcdc_crtc->palette_dma_handle);
 }
 
 int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 5/7] drm/tilcdc: Add tilcdc_write_mask() to tilcdc_regs.h

[Jyri Sarha]

Add tilcdc_write_mask() for handling register field wider than one bit
and mask values for those fields.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_regs.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_regs.h b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
index beb8c21..56dbfbd 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_regs.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_regs.h
@@ -34,11 +34,14 @@
 
 /* LCDC DMA Control Register */
 #define LCDC_DMA_BURST_SIZE(x)                   ((x) << 4)
+#define LCDC_DMA_BURST_SIZE_MASK                 ((0x7) << 4)
 #define LCDC_DMA_BURST_1                         0x0
 #define LCDC_DMA_BURST_2                         0x1
 #define LCDC_DMA_BURST_4                         0x2
 #define LCDC_DMA_BURST_8                         0x3
 #define LCDC_DMA_BURST_16                        0x4
+#define LCDC_DMA_FIFO_THRESHOLD(x)               ((x) << 8)
+#define LCDC_DMA_FIFO_THRESHOLD_MASK             ((0x3) << 8)
 #define LCDC_V1_END_OF_FRAME_INT_ENA             BIT(2)
 #define LCDC_V2_END_OF_FRAME0_INT_ENA            BIT(8)
 #define LCDC_V2_END_OF_FRAME1_INT_ENA            BIT(9)
@@ -46,10 +49,12 @@
 
 /* LCDC Control Register */
 #define LCDC_CLK_DIVISOR(x)                      ((x) << 8)
+#define LCDC_CLK_DIVISOR_MASK                    ((0xFF) << 8)
 #define LCDC_RASTER_MODE                         0x01
 
 /* LCDC Raster Control Register */
 #define LCDC_PALETTE_LOAD_MODE(x)                ((x) << 20)
+#define LCDC_PALETTE_LOAD_MODE_MASK              ((0x3) << 20)
 #define PALETTE_AND_DATA                         0x00
 #define PALETTE_ONLY                             0x01
 #define DATA_ONLY                                0x02
@@ -75,7 +80,9 @@
 
 /* LCDC Raster Timing 2 Register */
 #define LCDC_AC_BIAS_TRANSITIONS_PER_INT(x)      ((x) << 16)
+#define LCDC_AC_BIAS_TRANSITIONS_PER_INT_MASK    ((0xF) << 16)
 #define LCDC_AC_BIAS_FREQUENCY(x)                ((x) << 8)
+#define LCDC_AC_BIAS_FREQUENCY_MASK              ((0xFF) << 8)
 #define LCDC_SYNC_CTRL                           BIT(25)
 #define LCDC_SYNC_EDGE                           BIT(24)
 #define LCDC_INVERT_PIXEL_CLOCK                  BIT(22)
@@ -140,6 +147,12 @@ static inline u32 tilcdc_read(struct drm_device *dev, u32 reg)
 	return ioread32(priv->mmio + reg);
 }
 
+static inline void tilcdc_write_mask(struct drm_device *dev, u32 reg,
+				     u32 val, u32 mask)
+{
+	tilcdc_write(dev, reg, (tilcdc_read(dev, reg) & ~mask) | (val & mask));
+}
+
 static inline void tilcdc_set(struct drm_device *dev, u32 reg, u32 mask)
 {
 	tilcdc_write(dev, reg, tilcdc_read(dev, reg) | mask);
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 6/7] drm/tilcdc: Enable palette loading for revision 2 LCDC too

[Jyri Sarha]

The LCDC revision 2 documentation also mentions the mandatory palette
for true color modes. Even if the rev 2 LCDC appears to work just fine
without the palette being loaded loading it helps in testing the
feature.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 88 ++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 6d2ce53b..1590c42 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -28,8 +28,8 @@
 #include "tilcdc_regs.h"
 
 #define TILCDC_VBLANK_SAFETY_THRESHOLD_US	1000
-#define TILCDC_REV1_PALETTE_SIZE		32
-#define TILCDC_REV1_PALETTE_FIRST_ENTRY		0x4000
+#define TILCDC_PALETTE_SIZE			32
+#define TILCDC_PALETTE_FIRST_ENTRY		0x4000
 
 struct tilcdc_crtc {
 	struct drm_crtc base;
@@ -62,7 +62,7 @@ struct tilcdc_crtc {
 	struct work_struct recover_work;
 
 	dma_addr_t palette_dma_handle;
-	void *palette_base;
+	u16 *palette_base;
 	struct completion palette_loaded;
 };
 #define to_tilcdc_crtc(x) container_of(x, struct tilcdc_crtc, base)
@@ -114,23 +114,17 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
 }
 
 /*
- * The driver currently only supports the RGB565 format for revision 1. For
- * 16 bits-per-pixel the palette block is bypassed, but the first 32 bytes of
- * the framebuffer are still considered palette. The first 16-bit entry must
- * be 0x4000 while all other entries must be zeroed.
+ * The driver currently only supports only true color formats. For
+ * true color the palette block is bypassed, but a 32 byte palette
+ * should still be loaded. The first 16-bit entry must be 0x4000 while
+ * all other entries must be zeroed.
  */
 static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
 {
 	u32 dma_fb_base, dma_fb_ceiling, raster_ctl;
-	struct tilcdc_crtc *tilcdc_crtc;
-	struct drm_device *dev;
-	u16 *first_entry;
-
-	dev = crtc->dev;
-	tilcdc_crtc = to_tilcdc_crtc(crtc);
-	first_entry = tilcdc_crtc->palette_base;
-
-	*first_entry = TILCDC_REV1_PALETTE_FIRST_ENTRY;
+	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	struct tilcdc_drm_private *priv = dev->dev_private;
 
 	dma_fb_base = tilcdc_read(dev, LCDC_DMA_FB_BASE_ADDR_0_REG);
 	dma_fb_ceiling = tilcdc_read(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG);
@@ -140,23 +134,34 @@ static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
 	tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG,
 		     tilcdc_crtc->palette_dma_handle);
 	tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG,
-		     (u32)tilcdc_crtc->palette_dma_handle
-				+ TILCDC_REV1_PALETTE_SIZE - 1);
+		     (u32) tilcdc_crtc->palette_dma_handle +
+		     TILCDC_PALETTE_SIZE - 1);
 
-	/* Load it. */
-	tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
-		     LCDC_PALETTE_LOAD_MODE(DATA_ONLY));
-	tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
-		   LCDC_PALETTE_LOAD_MODE(PALETTE_ONLY));
+	/* Set dma load mode for palette loading only. */
+	tilcdc_write_mask(dev, LCDC_RASTER_CTRL_REG,
+			  LCDC_PALETTE_LOAD_MODE(PALETTE_ONLY),
+			  LCDC_PALETTE_LOAD_MODE_MASK);
 
-	/* Enable the LCDC and wait for palette to be loaded. */
-	tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
+	/* Enable DMA Palette Loaded Interrupt */
+	if (priv->rev == 1)
+		tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
+	else
+		tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG, LCDC_V2_PL_INT_ENA);
+
+	/* Enable LCDC DMA and wait for palette to be loaded. */
+	tilcdc_clear_irqstatus(dev, 0xffffffff);
 	tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
 
 	wait_for_completion(&tilcdc_crtc->palette_loaded);
 
-	/* Restore the registers. */
+	/* Disable LCDC DMA and DMA Palette Loaded Interrupt. */
 	tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+	if (priv->rev == 1)
+		tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
+	else
+		tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, LCDC_V2_PL_INT_ENA);
+
+	/* Restore the registers. */
 	tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, dma_fb_base);
 	tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG, dma_fb_ceiling);
 	tilcdc_write(dev, LCDC_RASTER_CTRL_REG, raster_ctl);
@@ -217,7 +222,6 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-	struct tilcdc_drm_private *priv = dev->dev_private;
 
 	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
 	mutex_lock(&tilcdc_crtc->enable_lock);
@@ -230,7 +234,7 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
 
 	reset(crtc);
 
-	if (priv->rev == 1 && !completion_done(&tilcdc_crtc->palette_loaded))
+	if (!completion_done(&tilcdc_crtc->palette_loaded))
 		tilcdc_crtc_load_palette(crtc);
 
 	tilcdc_crtc_enable_irqs(dev);
@@ -278,8 +282,7 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool shutdown)
 	 * LCDC will not retain the palette when reset. Make sure it gets
 	 * reloaded on tilcdc_crtc_enable().
 	 */
-	if (priv->rev == 1)
-		reinit_completion(&tilcdc_crtc->palette_loaded);
+	reinit_completion(&tilcdc_crtc->palette_loaded);
 
 	drm_crtc_vblank_off(crtc);
 
@@ -356,7 +359,7 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
 	drm_crtc_cleanup(crtc);
 	drm_flip_work_cleanup(&tilcdc_crtc->unref_work);
 
-	dmam_free_coherent(crtc->dev->dev, TILCDC_REV1_PALETTE_SIZE,
+	dmam_free_coherent(crtc->dev->dev, TILCDC_PALETTE_SIZE,
 			   tilcdc_crtc->palette_base,
 			   tilcdc_crtc->palette_dma_handle);
 }
@@ -918,12 +921,10 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 		dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underflow",
 				    __func__, stat);
 
-	if (priv->rev == 1) {
-		if (stat & LCDC_PL_LOAD_DONE) {
-			complete(&tilcdc_crtc->palette_loaded);
-			tilcdc_clear(dev,
-				     LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
-		}
+	if (stat & LCDC_PL_LOAD_DONE) {
+		complete(&tilcdc_crtc->palette_loaded);
+		tilcdc_clear(dev,
+			     LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
 	}
 
 	if (stat & LCDC_SYNC_LOST) {
@@ -970,15 +971,14 @@ int tilcdc_crtc_create(struct drm_device *dev)
 		return -ENOMEM;
 	}
 
-	if (priv->rev == 1) {
-		init_completion(&tilcdc_crtc->palette_loaded);
-		tilcdc_crtc->palette_base = dmam_alloc_coherent(dev->dev,
-					TILCDC_REV1_PALETTE_SIZE,
+	init_completion(&tilcdc_crtc->palette_loaded);
+	tilcdc_crtc->palette_base = dmam_alloc_coherent(dev->dev,
+					TILCDC_PALETTE_SIZE,
 					&tilcdc_crtc->palette_dma_handle,
 					GFP_KERNEL | __GFP_ZERO);
-		if (!tilcdc_crtc->palette_base)
-			return -ENOMEM;
-	}
+	if (!tilcdc_crtc->palette_base)
+		return -ENOMEM;
+	*tilcdc_crtc->palette_base = TILCDC_PALETTE_FIRST_ENTRY;
 
 	crtc = &tilcdc_crtc->base;
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 7/7] drm/tilcdc: Load palette at the end of mode_set_nofb()

[Jyri Sarha]

Load palette at the end of mode_set_nofb() and only if the palette has
not been loaded since last runtime resume. Moving the palette loading
to mode_set_nofb() saves us from storing and restoring of LCDC dma
addresses that were just recently updated.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 33 +++++++++++++--------------------
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 12 ++++++++++++
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  1 +
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 1590c42..f3be171 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -113,6 +113,13 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
 	tilcdc_crtc->curr_fb = fb;
 }
 
+void tilcdc_crtc_reload_palette(struct drm_crtc *crtc)
+{
+	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+
+	reinit_completion(&tilcdc_crtc->palette_loaded);
+}
+
 /*
  * The driver currently only supports only true color formats. For
  * true color the palette block is bypassed, but a 32 byte palette
@@ -121,14 +128,12 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
  */
 static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
 {
-	u32 dma_fb_base, dma_fb_ceiling, raster_ctl;
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct tilcdc_drm_private *priv = dev->dev_private;
 
-	dma_fb_base = tilcdc_read(dev, LCDC_DMA_FB_BASE_ADDR_0_REG);
-	dma_fb_ceiling = tilcdc_read(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG);
-	raster_ctl = tilcdc_read(dev, LCDC_RASTER_CTRL_REG);
+	if (completion_done(&tilcdc_crtc->palette_loaded))
+		return;
 
 	/* Tell the LCDC where the palette is located. */
 	tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG,
@@ -160,11 +165,6 @@ static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
 		tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
 	else
 		tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, LCDC_V2_PL_INT_ENA);
-
-	/* Restore the registers. */
-	tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, dma_fb_base);
-	tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG, dma_fb_ceiling);
-	tilcdc_write(dev, LCDC_RASTER_CTRL_REG, raster_ctl);
 }
 
 static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
@@ -234,9 +234,6 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
 
 	reset(crtc);
 
-	if (!completion_done(&tilcdc_crtc->palette_loaded))
-		tilcdc_crtc_load_palette(crtc);
-
 	tilcdc_crtc_enable_irqs(dev);
 
 	tilcdc_clear(dev, LCDC_DMA_CTRL_REG, LCDC_DUAL_FRAME_BUFFER_ENABLE);
@@ -278,12 +275,6 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool shutdown)
 				__func__);
 	}
 
-	/*
-	 * LCDC will not retain the palette when reset. Make sure it gets
-	 * reloaded on tilcdc_crtc_enable().
-	 */
-	reinit_completion(&tilcdc_crtc->palette_loaded);
-
 	drm_crtc_vblank_off(crtc);
 
 	tilcdc_crtc_disable_irqs(dev);
@@ -675,10 +666,12 @@ static void tilcdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
 
 	drm_framebuffer_reference(fb);
 
-	set_scanout(crtc, fb);
-
 	tilcdc_crtc_set_clk(crtc);
 
+	tilcdc_crtc_load_palette(crtc);
+
+	set_scanout(crtc, fb);
+
 	crtc->hwmode = crtc->state->adjusted_mode;
 }
 
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 28e97d5..a7c91f7 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -637,8 +637,20 @@ static int tilcdc_pm_resume(struct device *dev)
 }
 #endif
 
+static int tilcdc_pm_runtime_resume(struct device *dev)
+{
+	struct drm_device *ddev = dev_get_drvdata(dev);
+	struct tilcdc_drm_private *priv = ddev->dev_private;
+
+	if (priv->crtc)
+		tilcdc_crtc_reload_palette(priv->crtc);
+
+	return 0;
+}
+
 static const struct dev_pm_ops tilcdc_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(tilcdc_pm_suspend, tilcdc_pm_resume)
+	.runtime_resume = tilcdc_pm_runtime_resume,
 };
 
 /*
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 7913b0e..5803848 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -180,6 +180,7 @@ void tilcdc_crtc_set_simulate_vesa_sync(struct drm_crtc *crtc,
 int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
 		struct drm_framebuffer *fb,
 		struct drm_pending_vblank_event *event);
+void tilcdc_crtc_reload_palette(struct drm_crtc *crtc);
 
 int tilcdc_plane_init(struct drm_device *dev, struct drm_plane *plane);
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/7] drm/tilcdc: LCDC Revision 1 related fixes

[Bartosz Golaszewski]

2016-11-16 13:40 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
> Changes since first version of the series:
>
> - Move tilcdc_regs.h changes from "drm/tilcdc: Enable palette loading
>   for revision 2 LCDC too" to "drm/tilcdc: Add tilcdc_write_mask() to
>   tilcdc_regs.h"
>
> These patches are inspired by this series form Bartosz Golaszewski:
> https://www.spinics.net/lists/arm-kernel/msg539629.html
>
> The patches are based on drm-next plus the earlier patches that I plan
> to send in a pull request for 4.10. The base + these patches are
> pushed here:
>
> https://github.com/jsarha/linux drm-next-tilcdc-for-4.10-wip
>
> Bartosz, please test if this branch works for rev1 LCDC, with your dts
> file!
>

Hi Jyri,

with your changes I'm getting the following warning on initialization:

[drm] Initialized
[drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[drm] No driver support for vblank timestamp query.
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_atomic_helper.c:1141
drm_atomic_helper_wait_for_vblanks+0x23c/0x24c
[CRTC:24] vblank wait timed out
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.9.0-rc4-00939-ge79af2c #60
Hardware name: Generic DA850/OMAP-L138/AM18x
Backtrace:
    [snip]

and the same when running simple modetest (no vsynced page flipping).

The default resolution still works and I can start a graphical environment.

Thanks,
Bartosz
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/7] drm/tilcdc: LCDC Revision 1 related fixes

[Jyri Sarha]

On 11/16/16 17:18, Bartosz Golaszewski wrote:
> 2016-11-16 13:40 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>> Changes since first version of the series:
>>
>> - Move tilcdc_regs.h changes from "drm/tilcdc: Enable palette loading
>>   for revision 2 LCDC too" to "drm/tilcdc: Add tilcdc_write_mask() to
>>   tilcdc_regs.h"
>>
>> These patches are inspired by this series form Bartosz Golaszewski:
>> https://www.spinics.net/lists/arm-kernel/msg539629.html
>>
>> The patches are based on drm-next plus the earlier patches that I plan
>> to send in a pull request for 4.10. The base + these patches are
>> pushed here:
>>
>> https://github.com/jsarha/linux drm-next-tilcdc-for-4.10-wip
>>
>> Bartosz, please test if this branch works for rev1 LCDC, with your dts
>> file!
>>
> 
> Hi Jyri,
> 
> with your changes I'm getting the following warning on initialization:
> 
> [drm] Initialized
> [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [drm] No driver support for vblank timestamp query.
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_atomic_helper.c:1141
> drm_atomic_helper_wait_for_vblanks+0x23c/0x24c
> [CRTC:24] vblank wait timed out
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.9.0-rc4-00939-ge79af2c #60
> Hardware name: Generic DA850/OMAP-L138/AM18x
> Backtrace:
>     [snip]
> 
> and the same when running simple modetest (no vsynced page flipping).
> 

Weird. I have never seen that warning. Are you receiving
LCDC_END_OF_FRAME0 interrupts at all?

Am I missing some interrupt enable bit for rev 1 LCDC?

AFAIK the drm_crtc_send_vblank_event() should get called for the drm
atomic state even when the  LCDC_END_OF_FRAME0 interrupt is received and
the (mandatory) framebuffer is on the screen.

> The default resolution still works and I can start a graphical environment.
> 

Was this with the panel hack or using dumb-vga-dac bridge?

Cheers,
Jyri
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/7] drm/tilcdc: LCDC Revision 1 related fixes

[Bartosz Golaszewski]

2016-11-16 19:00 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
> On 11/16/16 17:18, Bartosz Golaszewski wrote:
>> 2016-11-16 13:40 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>>> Changes since first version of the series:
>>>
>>> - Move tilcdc_regs.h changes from "drm/tilcdc: Enable palette loading
>>>   for revision 2 LCDC too" to "drm/tilcdc: Add tilcdc_write_mask() to
>>>   tilcdc_regs.h"
>>>
>>> These patches are inspired by this series form Bartosz Golaszewski:
>>> https://www.spinics.net/lists/arm-kernel/msg539629.html
>>>
>>> The patches are based on drm-next plus the earlier patches that I plan
>>> to send in a pull request for 4.10. The base + these patches are
>>> pushed here:
>>>
>>> https://github.com/jsarha/linux drm-next-tilcdc-for-4.10-wip
>>>
>>> Bartosz, please test if this branch works for rev1 LCDC, with your dts
>>> file!
>>>
>>
>> Hi Jyri,
>>
>> with your changes I'm getting the following warning on initialization:
>>
>> [drm] Initialized
>> [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
>> [drm] No driver support for vblank timestamp query.
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_atomic_helper.c:1141
>> drm_atomic_helper_wait_for_vblanks+0x23c/0x24c
>> [CRTC:24] vblank wait timed out
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.9.0-rc4-00939-ge79af2c #60
>> Hardware name: Generic DA850/OMAP-L138/AM18x
>> Backtrace:
>>     [snip]
>>
>> and the same when running simple modetest (no vsynced page flipping).
>>
>
> Weird. I have never seen that warning. Are you receiving
> LCDC_END_OF_FRAME0 interrupts at all?
>

I'm only getting three right after drm initialization and then,
something seems to be disabling them (maybe as the result of vblank
timeout?).

> Am I missing some interrupt enable bit for rev 1 LCDC?
>

Rather the interrupt is disabled after being enabled first.

> AFAIK the drm_crtc_send_vblank_event() should get called for the drm
> atomic state even when the  LCDC_END_OF_FRAME0 interrupt is received and
> the (mandatory) framebuffer is on the screen.
>

I'll investigate that, thanks for the hint.

>> The default resolution still works and I can start a graphical environment.
>>
>
> Was this with the panel hack or using dumb-vga-dac bridge?
>

Yes, still the panel hack. I'll test the vga-dac after I can sort out
this issue.

Thanks,
Bartosz
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/7] drm/tilcdc: LCDC Revision 1 related fixes

[Jyri Sarha]

On 11/17/16 13:31, Bartosz Golaszewski wrote:
> 2016-11-16 19:00 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>> On 11/16/16 17:18, Bartosz Golaszewski wrote:
>>> 2016-11-16 13:40 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>>>> Changes since first version of the series:
>>>>
>>>> - Move tilcdc_regs.h changes from "drm/tilcdc: Enable palette loading
>>>>   for revision 2 LCDC too" to "drm/tilcdc: Add tilcdc_write_mask() to
>>>>   tilcdc_regs.h"
>>>>
>>>> These patches are inspired by this series form Bartosz Golaszewski:
>>>> https://www.spinics.net/lists/arm-kernel/msg539629.html
>>>>
>>>> The patches are based on drm-next plus the earlier patches that I plan
>>>> to send in a pull request for 4.10. The base + these patches are
>>>> pushed here:
>>>>
>>>> https://github.com/jsarha/linux drm-next-tilcdc-for-4.10-wip
>>>>
>>>> Bartosz, please test if this branch works for rev1 LCDC, with your dts
>>>> file!
>>>>
>>>
>>> Hi Jyri,
>>>
>>> with your changes I'm getting the following warning on initialization:
>>>
>>> [drm] Initialized
>>> [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
>>> [drm] No driver support for vblank timestamp query.
>>> ------------[ cut here ]------------
>>> WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_atomic_helper.c:1141
>>> drm_atomic_helper_wait_for_vblanks+0x23c/0x24c
>>> [CRTC:24] vblank wait timed out
>>> Modules linked in:
>>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.9.0-rc4-00939-ge79af2c #60
>>> Hardware name: Generic DA850/OMAP-L138/AM18x
>>> Backtrace:
>>>     [snip]
>>>
>>> and the same when running simple modetest (no vsynced page flipping).
>>>
>>
>> Weird. I have never seen that warning. Are you receiving
>> LCDC_END_OF_FRAME0 interrupts at all?
>>
> 
> I'm only getting three right after drm initialization and then,
> something seems to be disabling them (maybe as the result of vblank
> timeout?).
> 
>> Am I missing some interrupt enable bit for rev 1 LCDC?
>>
> 
> Rather the interrupt is disabled after being enabled first.
> 

I double checked the my code, but could not find anything wrong with
interrupt enables and disables (but I found something else to fix[1]).

Could you check from debug-fs if the LCDC_RASTER_CTRL_REG bit 3 is
really 1 (frame done interrupt enabled) when ever display is on [2]?

>> AFAIK the drm_crtc_send_vblank_event() should get called for the drm
>> atomic state even when the  LCDC_END_OF_FRAME0 interrupt is received and
>> the (mandatory) framebuffer is on the screen.
>>
> 
> I'll investigate that, thanks for the hint.
> 
>>> The default resolution still works and I can start a graphical environment.
>>>
>>
>> Was this with the panel hack or using dumb-vga-dac bridge?
>>
> 
> Yes, still the panel hack. I'll test the vga-dac after I can sort out
> this issue.
> 

Ok, but do not expect that using vga-dac would make much difference.

Cheers,
Jyri

[1] I found that I left the disabling of palette loaded irq for rev 1 in
irc routine. My latest version removed the clearing all together.
Calling the complete more than once should not make any harm, and the
palette loaded interrupt appears to come only once anyway. I also added
a timeout for wait_for_completion() because we do not want things to
hang forever even if the interrupt never arrives. However, these changes
should not make any difference on how the driver works on LCDC rev 1.
The latest version, that is also rebased on the latest drm-next, is now in:

https://github.com/jsarha/linux drm-next-tilcdc-for-4.10-wip

[2] cat /sys/kernel/debug/dri/64/regs (as root)

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/7] drm/tilcdc: LCDC Revision 1 related fixes

[Bartosz Golaszewski]

2016-11-17 21:06 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
> On 11/17/16 13:31, Bartosz Golaszewski wrote:
>> 2016-11-16 19:00 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>>> On 11/16/16 17:18, Bartosz Golaszewski wrote:
>>>> 2016-11-16 13:40 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>>>>> Changes since first version of the series:
>>>>>
>>>>> - Move tilcdc_regs.h changes from "drm/tilcdc: Enable palette loading
>>>>>   for revision 2 LCDC too" to "drm/tilcdc: Add tilcdc_write_mask() to
>>>>>   tilcdc_regs.h"
>>>>>
>>>>> These patches are inspired by this series form Bartosz Golaszewski:
>>>>> https://www.spinics.net/lists/arm-kernel/msg539629.html
>>>>>
>>>>> The patches are based on drm-next plus the earlier patches that I plan
>>>>> to send in a pull request for 4.10. The base + these patches are
>>>>> pushed here:
>>>>>
>>>>> https://github.com/jsarha/linux drm-next-tilcdc-for-4.10-wip
>>>>>
>>>>> Bartosz, please test if this branch works for rev1 LCDC, with your dts
>>>>> file!
>>>>>
>>>>
>>>> Hi Jyri,
>>>>
>>>> with your changes I'm getting the following warning on initialization:
>>>>
>>>> [drm] Initialized
>>>> [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
>>>> [drm] No driver support for vblank timestamp query.
>>>> ------------[ cut here ]------------
>>>> WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_atomic_helper.c:1141
>>>> drm_atomic_helper_wait_for_vblanks+0x23c/0x24c
>>>> [CRTC:24] vblank wait timed out
>>>> Modules linked in:
>>>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.9.0-rc4-00939-ge79af2c #60
>>>> Hardware name: Generic DA850/OMAP-L138/AM18x
>>>> Backtrace:
>>>>     [snip]
>>>>
>>>> and the same when running simple modetest (no vsynced page flipping).
>>>>
>>>
>>> Weird. I have never seen that warning. Are you receiving
>>> LCDC_END_OF_FRAME0 interrupts at all?
>>>
>>
>> I'm only getting three right after drm initialization and then,
>> something seems to be disabling them (maybe as the result of vblank
>> timeout?).
>>
>>> Am I missing some interrupt enable bit for rev 1 LCDC?
>>>
>>
>> Rather the interrupt is disabled after being enabled first.
>>
>
> I double checked the my code, but could not find anything wrong with
> interrupt enables and disables (but I found something else to fix[1]).
>
> Could you check from debug-fs if the LCDC_RASTER_CTRL_REG bit 3 is
> really 1 (frame done interrupt enabled) when ever display is on [2]?
>
>>> AFAIK the drm_crtc_send_vblank_event() should get called for the drm
>>> atomic state even when the  LCDC_END_OF_FRAME0 interrupt is received and
>>> the (mandatory) framebuffer is on the screen.
>>>
>>
>> I'll investigate that, thanks for the hint.
>>
>>>> The default resolution still works and I can start a graphical environment.
>>>>
>>>
>>> Was this with the panel hack or using dumb-vga-dac bridge?
>>>
>>
>> Yes, still the panel hack. I'll test the vga-dac after I can sort out
>> this issue.
>>
>
> Ok, but do not expect that using vga-dac would make much difference.
>
> Cheers,
> Jyri
>
> [1] I found that I left the disabling of palette loaded irq for rev 1 in
> irc routine. My latest version removed the clearing all together.
> Calling the complete more than once should not make any harm, and the
> palette loaded interrupt appears to come only once anyway. I also added
> a timeout for wait_for_completion() because we do not want things to
> hang forever even if the interrupt never arrives. However, these changes
> should not make any difference on how the driver works on LCDC rev 1.
> The latest version, that is also rebased on the latest drm-next, is now in:
>
> https://github.com/jsarha/linux drm-next-tilcdc-for-4.10-wip
>
> [2] cat /sys/kernel/debug/dri/64/regs (as root)
>

Hi Jyri,

the current version hangs after

    [drm] Initialized
    [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
    [drm] No driver support for vblank timestamp query.

I'll try applying one patch at a time and when I find the one that
breaks stuff I'll try to pinpoint the place where it fails.

Thanks,
Bartosz
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/7] drm/tilcdc: Enable sync lost error and recovery handling for rev 1 LCDC

[Bartosz Golaszewski]

2016-11-16 13:41 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
> Revision 1 LCDC support also sync lost errors and can benefit from
> sync lost recovery routine.
>

Hi Jyri,

I think I found the issue with this patch. Please see below.

> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 39 ++++++++++++++++++------------------
>  drivers/gpu/drm/tilcdc/tilcdc_regs.h |  1 +
>  2 files changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index c787349..dfe5796 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -113,8 +113,8 @@ static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
>
>         if (priv->rev == 1) {
>                 tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
> -                       LCDC_V1_UNDERFLOW_INT_ENA);
> -               tilcdc_set(dev, LCDC_DMA_CTRL_REG,
> +                       LCDC_V1_UNDERFLOW_INT_ENA |
> +                       LCDC_V1_SYNC_LOST_ENA |
>                         LCDC_V1_END_OF_FRAME_INT_ENA);
>         } else {
>                 tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG,
> @@ -131,8 +131,9 @@ static void tilcdc_crtc_disable_irqs(struct drm_device *dev)
>         /* disable irqs that we might have enabled: */
>         if (priv->rev == 1) {
>                 tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
> -                       LCDC_V1_UNDERFLOW_INT_ENA | LCDC_V1_PL_INT_ENA);
> -               tilcdc_clear(dev, LCDC_DMA_CTRL_REG,
> +                       LCDC_V1_UNDERFLOW_INT_ENA |
> +                       LCDC_V1_PL_INT_ENA |
> +                       LCDC_V1_SYNC_LOST_ENA |
>                         LCDC_V1_END_OF_FRAME_INT_ENA);
>         } else {
>                 tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,

This is what breaks the driver. The END_OF_FRAME interrupt is enabled
by writing to the LCD DMA control register on rev 1, not LCDC RASTER
control. It should be left as it is - you only need to enable the
SYNC_LOST interrupt.

Hint: maybe call it LCDC_V1_SYNC_LOST_INT_ENA?

> @@ -845,6 +846,21 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
>                 dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underflow",
>                                     __func__, stat);
>
> +       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, recovering", __func__, stat);
> +                       queue_work(system_wq,
> +                                  &tilcdc_crtc->recover_work);
> +                       tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
> +                                    LCDC_SYNC_LOST);
> +                       tilcdc_crtc->sync_lost_count = 0;
> +               }
> +       }

The SYNC_LOST bit still needs clearing here - otherwise the flood
never stops even after recovery work completes.

Thanks,
Bartosz Golaszewski
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 7/7] drm/tilcdc: Load palette at the end of mode_set_nofb()

[Bartosz Golaszewski]

2016-11-16 13:41 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
> Load palette at the end of mode_set_nofb() and only if the palette has
> not been loaded since last runtime resume. Moving the palette loading
> to mode_set_nofb() saves us from storing and restoring of LCDC dma
> addresses that were just recently updated.
>
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 33 +++++++++++++--------------------
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 12 ++++++++++++
>  drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  1 +
>  3 files changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 1590c42..f3be171 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -113,6 +113,13 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
>         tilcdc_crtc->curr_fb = fb;
>  }
>
> +void tilcdc_crtc_reload_palette(struct drm_crtc *crtc)
> +{
> +       struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
> +
> +       reinit_completion(&tilcdc_crtc->palette_loaded);
> +}
> +
>  /*
>   * The driver currently only supports only true color formats. For
>   * true color the palette block is bypassed, but a 32 byte palette
> @@ -121,14 +128,12 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
>   */
>  static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
>  {
> -       u32 dma_fb_base, dma_fb_ceiling, raster_ctl;
>         struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
>         struct drm_device *dev = crtc->dev;
>         struct tilcdc_drm_private *priv = dev->dev_private;
>
> -       dma_fb_base = tilcdc_read(dev, LCDC_DMA_FB_BASE_ADDR_0_REG);
> -       dma_fb_ceiling = tilcdc_read(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG);
> -       raster_ctl = tilcdc_read(dev, LCDC_RASTER_CTRL_REG);
> +       if (completion_done(&tilcdc_crtc->palette_loaded))
> +               return;
>
>         /* Tell the LCDC where the palette is located. */
>         tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG,
> @@ -160,11 +165,6 @@ static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
>                 tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
>         else
>                 tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, LCDC_V2_PL_INT_ENA);
> -
> -       /* Restore the registers. */
> -       tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, dma_fb_base);
> -       tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG, dma_fb_ceiling);
> -       tilcdc_write(dev, LCDC_RASTER_CTRL_REG, raster_ctl);
>  }
>

Hi Jyri,

I don't know exactly why, but not restoring the RASTER CTRL register
here messes up simple modetest - the image is shifted vertically. The
rest of the patch seems fine.

Thanks,
Bartosz Golaszewski
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 7/7] drm/tilcdc: Load palette at the end of mode_set_nofb()

[Bartosz Golaszewski]

2016-11-18 16:34 GMT+01:00 Bartosz Golaszewski <bgolaszewski@baylibre.com>:
> 2016-11-16 13:41 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>> Load palette at the end of mode_set_nofb() and only if the palette has
>> not been loaded since last runtime resume. Moving the palette loading
>> to mode_set_nofb() saves us from storing and restoring of LCDC dma
>> addresses that were just recently updated.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 33 +++++++++++++--------------------
>>  drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 12 ++++++++++++
>>  drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  1 +
>>  3 files changed, 26 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> index 1590c42..f3be171 100644
>> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> @@ -113,6 +113,13 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
>>         tilcdc_crtc->curr_fb = fb;
>>  }
>>
>> +void tilcdc_crtc_reload_palette(struct drm_crtc *crtc)
>> +{
>> +       struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
>> +
>> +       reinit_completion(&tilcdc_crtc->palette_loaded);
>> +}
>> +
>>  /*
>>   * The driver currently only supports only true color formats. For
>>   * true color the palette block is bypassed, but a 32 byte palette
>> @@ -121,14 +128,12 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
>>   */
>>  static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
>>  {
>> -       u32 dma_fb_base, dma_fb_ceiling, raster_ctl;
>>         struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
>>         struct drm_device *dev = crtc->dev;
>>         struct tilcdc_drm_private *priv = dev->dev_private;
>>
>> -       dma_fb_base = tilcdc_read(dev, LCDC_DMA_FB_BASE_ADDR_0_REG);
>> -       dma_fb_ceiling = tilcdc_read(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG);
>> -       raster_ctl = tilcdc_read(dev, LCDC_RASTER_CTRL_REG);
>> +       if (completion_done(&tilcdc_crtc->palette_loaded))
>> +               return;
>>
>>         /* Tell the LCDC where the palette is located. */
>>         tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG,
>> @@ -160,11 +165,6 @@ static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
>>                 tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
>>         else
>>                 tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, LCDC_V2_PL_INT_ENA);
>> -
>> -       /* Restore the registers. */
>> -       tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, dma_fb_base);
>> -       tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG, dma_fb_ceiling);
>> -       tilcdc_write(dev, LCDC_RASTER_CTRL_REG, raster_ctl);
>>  }
>>
>
> Hi Jyri,
>
> I don't know exactly why, but not restoring the RASTER CTRL register
> here messes up simple modetest - the image is shifted vertically. The
> rest of the patch seems fine.
>
> Thanks,
> Bartosz Golaszewski

Ok, got it. You need to set the palette loading mode back to 'palette
and data' before returning. Just add this at the end:

tilcdc_write_mask(dev, LCDC_RASTER_CTRL_REG,
 LCDC_PALETTE_LOAD_MODE(PALETTE_AND_DATA),
 LCDC_PALETTE_LOAD_MODE_MASK);

Thanks,
Bartosz Golaszewski
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/7] drm/tilcdc: Enable sync lost error and recovery handling for rev 1 LCDC

[Jyri Sarha]

On 11/18/16 15:34, Bartosz Golaszewski wrote:
> 2016-11-16 13:41 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>> Revision 1 LCDC support also sync lost errors and can benefit from
>> sync lost recovery routine.
>>
> 
> Hi Jyri,
> 
> I think I found the issue with this patch. Please see below.
> 
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 39 ++++++++++++++++++------------------
>>  drivers/gpu/drm/tilcdc/tilcdc_regs.h |  1 +
>>  2 files changed, 21 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> index c787349..dfe5796 100644
>> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> @@ -113,8 +113,8 @@ static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
>>
>>         if (priv->rev == 1) {
>>                 tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
>> -                       LCDC_V1_UNDERFLOW_INT_ENA);
>> -               tilcdc_set(dev, LCDC_DMA_CTRL_REG,
>> +                       LCDC_V1_UNDERFLOW_INT_ENA |
>> +                       LCDC_V1_SYNC_LOST_ENA |
>>                         LCDC_V1_END_OF_FRAME_INT_ENA);
>>         } else {
>>                 tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG,
>> @@ -131,8 +131,9 @@ static void tilcdc_crtc_disable_irqs(struct drm_device *dev)
>>         /* disable irqs that we might have enabled: */
>>         if (priv->rev == 1) {
>>                 tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
>> -                       LCDC_V1_UNDERFLOW_INT_ENA | LCDC_V1_PL_INT_ENA);
>> -               tilcdc_clear(dev, LCDC_DMA_CTRL_REG,
>> +                       LCDC_V1_UNDERFLOW_INT_ENA |
>> +                       LCDC_V1_PL_INT_ENA |
>> +                       LCDC_V1_SYNC_LOST_ENA |
>>                         LCDC_V1_END_OF_FRAME_INT_ENA);
>>         } else {
>>                 tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
> 
> This is what breaks the driver. The END_OF_FRAME interrupt is enabled
> by writing to the LCD DMA control register on rev 1, not LCDC RASTER
> control. It should be left as it is - you only need to enable the
> SYNC_LOST interrupt.
> 

Argh, stupid me. I just over looked that there there were actually two
different registers here. Thanks a lot for finding this out for me!

> Hint: maybe call it LCDC_V1_SYNC_LOST_INT_ENA?
> 
>> @@ -845,6 +846,21 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
>>                 dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underflow",
>>                                     __func__, stat);
>>
>> +       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, recovering", __func__, stat);
>> +                       queue_work(system_wq,
>> +                                  &tilcdc_crtc->recover_work);
>> +                       tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
>> +                                    LCDC_SYNC_LOST);
>> +                       tilcdc_crtc->sync_lost_count = 0;
>> +               }
>> +       }
> 
> The SYNC_LOST bit still needs clearing here - otherwise the flood
> never stops even after recovery work completes.
> 

That is surprising. So turning off the raster and resetting the LCDC
does not get rid off sync lost error flood. But the flood stops if we
clear the IRQ here and does not reappear when we turn it back on after
the reset, does it?

> Thanks,
> Bartosz Golaszewski
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 7/7] drm/tilcdc: Load palette at the end of mode_set_nofb()

[Jyri Sarha]

On 11/18/16 18:10, Bartosz Golaszewski wrote:
> 2016-11-18 16:34 GMT+01:00 Bartosz Golaszewski <bgolaszewski@baylibre.com>:
>> 2016-11-16 13:41 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>>> Load palette at the end of mode_set_nofb() and only if the palette has
>>> not been loaded since last runtime resume. Moving the palette loading
>>> to mode_set_nofb() saves us from storing and restoring of LCDC dma
>>> addresses that were just recently updated.
>>>
>>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>>> ---
>>>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 33 +++++++++++++--------------------
>>>  drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 12 ++++++++++++
>>>  drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  1 +
>>>  3 files changed, 26 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>>> index 1590c42..f3be171 100644
>>> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>>> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>>> @@ -113,6 +113,13 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
>>>         tilcdc_crtc->curr_fb = fb;
>>>  }
>>>
>>> +void tilcdc_crtc_reload_palette(struct drm_crtc *crtc)
>>> +{
>>> +       struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
>>> +
>>> +       reinit_completion(&tilcdc_crtc->palette_loaded);
>>> +}
>>> +
>>>  /*
>>>   * The driver currently only supports only true color formats. For
>>>   * true color the palette block is bypassed, but a 32 byte palette
>>> @@ -121,14 +128,12 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
>>>   */
>>>  static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
>>>  {
>>> -       u32 dma_fb_base, dma_fb_ceiling, raster_ctl;
>>>         struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
>>>         struct drm_device *dev = crtc->dev;
>>>         struct tilcdc_drm_private *priv = dev->dev_private;
>>>
>>> -       dma_fb_base = tilcdc_read(dev, LCDC_DMA_FB_BASE_ADDR_0_REG);
>>> -       dma_fb_ceiling = tilcdc_read(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG);
>>> -       raster_ctl = tilcdc_read(dev, LCDC_RASTER_CTRL_REG);
>>> +       if (completion_done(&tilcdc_crtc->palette_loaded))
>>> +               return;
>>>
>>>         /* Tell the LCDC where the palette is located. */
>>>         tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG,
>>> @@ -160,11 +165,6 @@ static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
>>>                 tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
>>>         else
>>>                 tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG, LCDC_V2_PL_INT_ENA);
>>> -
>>> -       /* Restore the registers. */
>>> -       tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, dma_fb_base);
>>> -       tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG, dma_fb_ceiling);
>>> -       tilcdc_write(dev, LCDC_RASTER_CTRL_REG, raster_ctl);
>>>  }
>>>
>>
>> Hi Jyri,
>>
>> I don't know exactly why, but not restoring the RASTER CTRL register
>> here messes up simple modetest - the image is shifted vertically. The
>> rest of the patch seems fine.
>>
>> Thanks,
>> Bartosz Golaszewski
> 
> Ok, got it. You need to set the palette loading mode back to 'palette
> and data' before returning. Just add this at the end:
> 
> tilcdc_write_mask(dev, LCDC_RASTER_CTRL_REG,
>  LCDC_PALETTE_LOAD_MODE(PALETTE_AND_DATA),
>  LCDC_PALETTE_LOAD_MODE_MASK);

Really? I wonder why, because we anyway set it to data only when we turn
the display on. The raster is not turned on before that so the register
value should not matter. I need to investigate what really happens.

However, for now I think I should just add it. There should not be any
harm in doing that.

Thanks,
Jyri

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 7/7] drm/tilcdc: Load palette at the end of mode_set_nofb()

[Jyri Sarha]

On 11/18/16 23:53, Jyri Sarha wrote:
>> Ok, got it. You need to set the palette loading mode back to 'palette
>> > and data' before returning. Just add this at the end:
>> > 
>> > tilcdc_write_mask(dev, LCDC_RASTER_CTRL_REG,
>> >  LCDC_PALETTE_LOAD_MODE(PALETTE_AND_DATA),
>> >  LCDC_PALETTE_LOAD_MODE_MASK);
> Really? I wonder why, because we anyway set it to data only when we turn
> the display on. The raster is not turned on before that so the register
> value should not matter. I need to investigate what really happens.
> 
> However, for now I think I should just add it. There should not be any
> harm in doing that.
> 

Now I got it. The problem is of course the wrong usage of tilcdc_set()
for the palette load mode bit-field in tilcdc_crtc_enable(). I'll change:

tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
	LCDC_PALETTE_LOAD_MODE(DATA_ONLY));

to

tilcdc_write_mask(dev, LCDC_RASTER_CTRL_REG,
		LCDC_PALETTE_LOAD_MODE(DATA_ONLY),
		LCDC_PALETTE_LOAD_MODE_MASK);

in tilcdc_crtc_enable(). That should fix the problem.

Cheers,
Jyri
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel