drm/tilcdc: LCDC Revision 1 related fixes

PATCH Superseded Jyri Sarha 2 participants 19 messages Nov 16, 2016 – Nov 19, 2016

Subsystems (3)

DRM drivers DRM misc DRM drivers for TI lcdc

lore

Share & tools

lore

Jyri Sarha

author

2016-11-16 12:41 UTC #0 lore raw

v2 cover drm/tilcdc: LCDC Revision 1 related fixes

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(-)

Signature · 6 lines

1.9.1

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

Jyri Sarha

author

2016-11-16 12:41 UTC #1 lore raw

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

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

Signed-off-by Jyri Sarha <[email protected]>

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 39 ++++++++++++++++++------------------
 drivers/gpu/drm/tilcdc/tilcdc_regs.h |  1 +
 2 files changed, 21 insertions(+), 19 deletions(-)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +20 ~~-19~~

index c787349..dfe5796 100644
@@ -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.
 		 */

drivers/gpu/drm/tilcdc/tilcdc_regs.h +1 -0

index f57c0d6..beb8c21 100644
@@ -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)

Signature · 6 lines

1.9.1

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

Bartosz Golaszewski

2016-11-18 13:34 UTC #2 lore raw

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

2016-11-16 13:41 GMT+01:00 Jyri Sarha <[email protected]>:

Quoted 3 lines

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.

Quoted 23 lines

Signed-off-by Jyri Sarha <[email protected]>

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 39 ++++++++++++++++++------------------
 drivers/gpu/drm/tilcdc/tilcdc_regs.h |  1 +
 2 files changed, 21 insertions(+), 19 deletions(-)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +2 -2

index c787349..dfe5796 100644
@@ -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: */

drivers/gpu/drm/tilcdc/tilcdc_crtc.c static void tilcdc_crtc_disable_irqs(struct drm_device *dev) · line 139

Quoted 10 lines

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +3 -2

        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?

Quoted 8 lines

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +4 -0

@@ -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;

drivers/gpu/drm/tilcdc/tilcdc_crtc.c irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) · line 862

Quoted 10 lines

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +10 -0

+               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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2016-11-18 21:42 UTC #3 lore raw

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

Quoted 39 lines · Bartosz Golaszewski

2016-11-16 13:41 GMT+01:00 Jyri Sarha <[email protected]>:

Quoted 3 lines

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.

Quoted 30 lines

Signed-off-by Jyri Sarha <[email protected]>

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 39 ++++++++++++++++++------------------
 drivers/gpu/drm/tilcdc/tilcdc_regs.h |  1 +
 2 files changed, 21 insertions(+), 19 deletions(-)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +5 -4

index c787349..dfe5796 100644
@@ -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 |

Quoted 9 lines

drivers/gpu/drm/tilcdc/tilcdc_crtc.c static void tilcdc_crtc_disable_irqs(struct drm_device *dev) · line 139
Quoted 3 lines
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +0 -0
                        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!

Quoted 15 lines

Hint: maybe call it LCDC_V1_SYNC_LOST_INT_ENA?

Quoted 13 lines

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +9 -0

@@ -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);

Quoted 9 lines

drivers/gpu/drm/tilcdc/tilcdc_crtc.c irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) · line 862
Quoted 5 lines
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +5 -0
+                       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?

Quoted 3 lines

Thanks,
Bartosz Golaszewski

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

Jyri Sarha

author

2016-11-16 12:41 UTC #4 lore raw

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

In reply to Jyri Sarha

From: Bartosz Golaszewski <[email protected]>

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 <[email protected]>

Signed-off-by Jyri Sarha <[email protected]>

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 88 +++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +87 -1

index dfe5796..c13e7a3 100644
@@ -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);

Signature · 6 lines

1.9.1

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

Jyri Sarha

author

2016-11-16 12:41 UTC #5 lore raw

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

In reply to Jyri Sarha

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

Signed-off-by Jyri Sarha <[email protected]>

---
 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(-)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +7 -5

index c13e7a3..67b50c0 100644
@@ -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;
 }

drivers/gpu/drm/tilcdc/tilcdc_drv.c +4 -7

index af959df..28e97d5 100644
@@ -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);

drivers/gpu/drm/tilcdc/tilcdc_drv.h +1 -1

index 283ff28..7913b0e 100644
@@ -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,

Signature · 6 lines

1.9.1

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

Jyri Sarha

author

2016-11-16 12:41 UTC #6 lore raw

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

In reply to Jyri Sarha

We should free the palette dma memory too.

Signed-off-by Jyri Sarha <[email protected]>

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +4 -0

index 67b50c0..6d2ce53b 100644
@@ -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,

Signature · 6 lines

1.9.1

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

Jyri Sarha

author

2016-11-16 12:41 UTC #7 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/gpu/drm/tilcdc/tilcdc_regs.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

drivers/gpu/drm/tilcdc/tilcdc_regs.h +13 -0

index beb8c21..56dbfbd 100644
@@ -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);

Signature · 6 lines

1.9.1

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

Jyri Sarha

author

2016-11-16 12:41 UTC #8 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 88 ++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 44 deletions(-)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +44 ~~-44~~

index 6d2ce53b..1590c42 100644
@@ -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;

Signature · 6 lines

1.9.1

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

Jyri Sarha

author

2016-11-16 12:41 UTC #9 lore raw

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

In reply to 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 <[email protected]>

---
 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(-)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +13 ~~-20~~

index 1590c42..f3be171 100644
@@ -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;
 }

drivers/gpu/drm/tilcdc/tilcdc_drv.c +12 -0

index 28e97d5..a7c91f7 100644
@@ -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,
 };
 
 /*

drivers/gpu/drm/tilcdc/tilcdc_drv.h +1 -0

index 7913b0e..5803848 100644
@@ -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);

Signature · 6 lines

1.9.1

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

Bartosz Golaszewski

2016-11-18 15:34 UTC #10 lore raw

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

2016-11-16 13:41 GMT+01:00 Jyri Sarha <[email protected]>:

Quoted 48 lines

Signed-off-by Jyri Sarha <[email protected]>

---
 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(-)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +9 -4

index 1590c42..f3be171 100644
@@ -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,

drivers/gpu/drm/tilcdc/tilcdc_crtc.c static void tilcdc_crtc_load_palette(struct drm_crtc *crtc) · line 169

Quoted 10 lines

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +0 -5

@@ -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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Bartosz Golaszewski

2016-11-18 16:10 UTC #11 lore raw

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

2016-11-18 16:34 GMT+01:00 Bartosz Golaszewski <[email protected]>:

Quoted 62 lines

2016-11-16 13:41 GMT+01:00 Jyri Sarha <[email protected]>:

Quoted 48 lines

Signed-off-by Jyri Sarha <[email protected]>

---
 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(-)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +9 -4

index 1590c42..f3be171 100644
@@ -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,

drivers/gpu/drm/tilcdc/tilcdc_crtc.c static void tilcdc_crtc_load_palette(struct drm_crtc *crtc) · line 169

Quoted 10 lines

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +0 -5

@@ -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,

Quoted 6 lines

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2016-11-18 21:53 UTC #12 lore raw

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

Quoted 70 lines · Bartosz Golaszewski

2016-11-18 16:34 GMT+01:00 Bartosz Golaszewski <[email protected]>:

Quoted 62 lines

2016-11-16 13:41 GMT+01:00 Jyri Sarha <[email protected]>:

Quoted 48 lines

Signed-off-by Jyri Sarha <[email protected]>

---
 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(-)

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +9 -4

index 1590c42..f3be171 100644
@@ -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,

drivers/gpu/drm/tilcdc/tilcdc_crtc.c static void tilcdc_crtc_load_palette(struct drm_crtc *crtc) · line 169

Quoted 10 lines

drivers/gpu/drm/tilcdc/tilcdc_crtc.c +0 -5

@@ -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,

Quoted 6 lines

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

Quoted 6 lines

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2016-11-19 15:58 UTC #13 lore raw

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

Quoted 6 lines · Jyri Sarha

Quoted 6 lines
Ok, got it. You need to set the palette loading mode back to 'palette
Quoted 5 lines
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);

Quoted 7 lines

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));

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Bartosz Golaszewski

2016-11-16 15:18 UTC #14 lore raw

In reply to Jyri Sarha

2016-11-16 13:40 GMT+01:00 Jyri Sarha <[email protected]>:

Quoted 9 lines

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

Quoted 9 lines

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2016-11-16 18:00 UTC #15 lore raw

Quoted 37 lines · Bartosz Golaszewski

2016-11-16 13:40 GMT+01:00 Jyri Sarha <[email protected]>:
Quoted 9 lines
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
Quoted 9 lines
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]

Quoted 2 lines

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.

Quoted 2 lines

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Bartosz Golaszewski

2016-11-17 11:31 UTC #16 lore raw

2016-11-16 19:00 GMT+01:00 Jyri Sarha <[email protected]>:

Quoted 38 lines · Bartosz Golaszewski

Quoted 37 lines
2016-11-16 13:40 GMT+01:00 Jyri Sarha <[email protected]>:
Quoted 9 lines
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
Quoted 9 lines
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]

Quoted 6 lines

Quoted 2 lines
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?).

Quoted 2 lines

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

Rather the interrupt is disabled after being enabled first.

Quoted 4 lines

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.

Quoted 5 lines

Quoted 2 lines
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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Jyri Sarha

author

2016-11-17 20:06 UTC #17 lore raw

Quoted 46 lines · Bartosz Golaszewski

2016-11-16 19:00 GMT+01:00 Jyri Sarha <[email protected]>:
Quoted 38 lines · Bartosz Golaszewski
Quoted 37 lines
2016-11-16 13:40 GMT+01:00 Jyri Sarha <[email protected]>:
Quoted 9 lines
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
Quoted 9 lines
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]
Quoted 6 lines
Quoted 2 lines
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?

Quoted 9 lines

I'm only getting three right after drm initialization and then, something seems to be disabling them (maybe as the result of vblank timeout?).
Quoted 2 lines
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]?

Quoted 7 lines

Quoted 4 lines
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.

Quoted 9 lines

Quoted 5 lines
Quoted 2 lines
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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Bartosz Golaszewski

2016-11-18 10:31 UTC #18 lore raw

2016-11-17 21:06 GMT+01:00 Jyri Sarha <[email protected]>:

Quoted 94 lines · Bartosz Golaszewski

Quoted 46 lines
2016-11-16 19:00 GMT+01:00 Jyri Sarha <[email protected]>:
Quoted 38 lines · Bartosz Golaszewski
Quoted 37 lines
2016-11-16 13:40 GMT+01:00 Jyri Sarha <[email protected]>:
Quoted 9 lines
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
Quoted 9 lines
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]
Quoted 6 lines
Quoted 2 lines
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?
Quoted 9 lines
I'm only getting three right after drm initialization and then, something seems to be disabling them (maybe as the result of vblank timeout?).
Quoted 2 lines
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]?
Quoted 7 lines
Quoted 4 lines
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.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
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:

Quoted 4 lines

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel