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(-)
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)
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
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
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);
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,
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,
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);
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;
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);
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
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
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
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
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
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
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
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)
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@lists.freedesktop.org
-
Bartosz Golaszewski -
Jyri Sarha