All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] drm/exynos: modify LCD I80 interface display
@ 2014-10-01  6:19 YoungJun Cho
  2014-10-01  6:19 ` [PATCH 1/7] drm/exynos: fimd: remove unnecessary waiting vblank routine YoungJun Cho
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: YoungJun Cho @ 2014-10-01  6:19 UTC (permalink / raw)
  To: airlied, dri-devel; +Cc: a.hajda, kyungmin.park, sw0312.kim

Hi,

This series modifies LCD I80 interface display for Exynos DRM driver.

This is based on exynos-drm-next branch.

Patches 1 and 2 modify trivial things.

Patch 3 changes ideal clock calculation standard as H/W guideline.

Patch 4 moves vblank handler in TE handler to privide proper VBLANK information.

Patch 5 arranges I80 interface interrupt configuration like RGB interface.

Patches 6 and 7 prevent showing the command mode panel garbage GRAM screen data.

I welcome any comments.

Thank you.
Best regards YJ

YoungJun Cho (7):
  drm/exynos: fimd: remove unnecessary waiting vblank routine
  drm/exynos: fimd: add fimd_channel_win() to clean up code
  drm/exynos: fimd: modify vclk calculation for I80 i/f
  drm/exynos: fimd: move handle vblank position in TE handler
  drm/exynos: fimd: modify I80 i/f interrupt relevant routine
  drm/exynos: dsi: move TE irq handler registration position
  drm/exynos: dsi: move DSIM_STATE_ENABLED set position

 drivers/gpu/drm/exynos/exynos_drm_dsi.c  |  24 +++--
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 155 +++++++++++++++++--------------
 2 files changed, 98 insertions(+), 81 deletions(-)

-- 
1.9.0

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/7] drm/exynos: fimd: remove unnecessary waiting vblank routine
  2014-10-01  6:19 [PATCH 0/7] drm/exynos: modify LCD I80 interface display YoungJun Cho
@ 2014-10-01  6:19 ` YoungJun Cho
  2014-11-13  9:00   ` Inki Dae
  2014-10-01  6:19 ` [PATCH 2/7] drm/exynos: fimd: add fimd_channel_win() to clean up code YoungJun Cho
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: YoungJun Cho @ 2014-10-01  6:19 UTC (permalink / raw)
  To: airlied, dri-devel; +Cc: a.hajda, kyungmin.park, sw0312.kim

The exynos_drm_crtc_dpms() waits until pended page flip
queue is empty, calls the drm_vblank_off() then calls
manager->ops->dpms() when mode is DRM_MODE_DPMS_OFF.
The fimd_dpms() is one of manager->ops->dpms()s and
finally calls fimd_window_suspend().
But there is no active window and vblank is already off
when it is called.
So addtional waiting vblank is not necessary any more.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 085b066..8b31b7e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -803,7 +803,6 @@ static void fimd_window_suspend(struct exynos_drm_manager *mgr)
 		if (win_data->enabled)
 			fimd_win_disable(mgr, i);
 	}
-	fimd_wait_for_vblank(mgr);
 }
 
 static void fimd_window_resume(struct exynos_drm_manager *mgr)
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/7] drm/exynos: fimd: add fimd_channel_win() to clean up code
  2014-10-01  6:19 [PATCH 0/7] drm/exynos: modify LCD I80 interface display YoungJun Cho
  2014-10-01  6:19 ` [PATCH 1/7] drm/exynos: fimd: remove unnecessary waiting vblank routine YoungJun Cho
@ 2014-10-01  6:19 ` YoungJun Cho
  2014-11-13  9:05   ` Inki Dae
  2014-10-01  6:19 ` [PATCH 3/7] drm/exynos: fimd: modify vclk calculation for I80 i/f YoungJun Cho
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: YoungJun Cho @ 2014-10-01  6:19 UTC (permalink / raw)
  To: airlied, dri-devel; +Cc: a.hajda, kyungmin.park, sw0312.kim

The ENWIN_F in WINCON# register and C#_EN_Fs in SHADOWCON register
should be always matched together, so adds fimd_channel_win()
to clean up code.
And this fimd_channel_win() should be called before unprotecting
window in fimd_win_commit().

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 62 ++++++++++++++++----------------
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 8b31b7e..b2f6007 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -214,6 +214,33 @@ static void fimd_wait_for_vblank(struct exynos_drm_manager *mgr)
 		DRM_DEBUG_KMS("vblank wait timed out.\n");
 }
 
+static void fimd_channel_win(struct fimd_context *ctx, int win, bool enable)
+{
+	u32 val;
+
+	/* for DMA output */
+	val = readl(ctx->regs + WINCON(win));
+
+	if (enable)
+		val |= WINCONx_ENWIN;
+	else
+		val &= ~WINCONx_ENWIN;
+
+	writel(val, ctx->regs + WINCON(win));
+
+	/* for shadow channel */
+	if (ctx->driver_data->has_shadowcon) {
+		val = readl(ctx->regs + SHADOWCON);
+
+		if (enable)
+			val |= SHADOWCON_CHx_ENABLE(win);
+		else
+			val &= ~SHADOWCON_CHx_ENABLE(win);
+
+		writel(val, ctx->regs + SHADOWCON);
+	}
+}
+
 static void fimd_clear_channel(struct exynos_drm_manager *mgr)
 {
 	struct fimd_context *ctx = mgr->ctx;
@@ -226,16 +253,7 @@ static void fimd_clear_channel(struct exynos_drm_manager *mgr)
 		u32 val = readl(ctx->regs + WINCON(win));
 
 		if (val & WINCONx_ENWIN) {
-			/* wincon */
-			val &= ~WINCONx_ENWIN;
-			writel(val, ctx->regs + WINCON(win));
-
-			/* unprotect windows */
-			if (ctx->driver_data->has_shadowcon) {
-				val = readl(ctx->regs + SHADOWCON);
-				val &= ~SHADOWCON_CHx_ENABLE(win);
-				writel(val, ctx->regs + SHADOWCON);
-			}
+			fimd_channel_win(ctx, win, false);
 			ch_enabled = 1;
 		}
 	}
@@ -730,20 +748,11 @@ static void fimd_win_commit(struct exynos_drm_manager *mgr, int zpos)
 	if (win != 0)
 		fimd_win_set_colkey(ctx, win);
 
-	/* wincon */
-	val = readl(ctx->regs + WINCON(win));
-	val |= WINCONx_ENWIN;
-	writel(val, ctx->regs + WINCON(win));
+	fimd_channel_win(ctx, win, true);
 
 	/* Enable DMA channel and unprotect windows */
 	fimd_shadow_protect_win(ctx, win, false);
 
-	if (ctx->driver_data->has_shadowcon) {
-		val = readl(ctx->regs + SHADOWCON);
-		val |= SHADOWCON_CHx_ENABLE(win);
-		writel(val, ctx->regs + SHADOWCON);
-	}
-
 	win_data->enabled = true;
 
 	if (ctx->i80_if)
@@ -755,7 +764,6 @@ static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
 	struct fimd_context *ctx = mgr->ctx;
 	struct fimd_win_data *win_data;
 	int win = zpos;
-	u32 val;
 
 	if (win == DEFAULT_ZPOS)
 		win = ctx->default_win;
@@ -774,17 +782,7 @@ static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
 	/* protect windows */
 	fimd_shadow_protect_win(ctx, win, true);
 
-	/* wincon */
-	val = readl(ctx->regs + WINCON(win));
-	val &= ~WINCONx_ENWIN;
-	writel(val, ctx->regs + WINCON(win));
-
-	/* unprotect windows */
-	if (ctx->driver_data->has_shadowcon) {
-		val = readl(ctx->regs + SHADOWCON);
-		val &= ~SHADOWCON_CHx_ENABLE(win);
-		writel(val, ctx->regs + SHADOWCON);
-	}
+	fimd_channel_win(ctx, win, false);
 
 	fimd_shadow_protect_win(ctx, win, false);
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/7] drm/exynos: fimd: modify vclk calculation for I80 i/f
  2014-10-01  6:19 [PATCH 0/7] drm/exynos: modify LCD I80 interface display YoungJun Cho
  2014-10-01  6:19 ` [PATCH 1/7] drm/exynos: fimd: remove unnecessary waiting vblank routine YoungJun Cho
  2014-10-01  6:19 ` [PATCH 2/7] drm/exynos: fimd: add fimd_channel_win() to clean up code YoungJun Cho
@ 2014-10-01  6:19 ` YoungJun Cho
  2014-11-13  9:12   ` Inki Dae
  2014-10-01  6:19 ` [PATCH 4/7] drm/exynos: fimd: move handle vblank position in TE handler YoungJun Cho
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: YoungJun Cho @ 2014-10-01  6:19 UTC (permalink / raw)
  To: airlied, dri-devel; +Cc: a.hajda, kyungmin.park, sw0312.kim

The I80 interface uses SYS_WE and SYS_CS to process
1 pixel data, so it requires the twice faster clock
than the pixel clock.
And the frame done interrupt should occurr prior to
the next TE signal, H/W guy recommends to use as 1.73
times faster clock frequency.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index b2f6007..05c2a97a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -81,6 +81,11 @@
 #define LCD_WR_HOLD(x)			((x) << 4)
 #define I80IFEN_ENABLE			(1 << 0)
 
+/* I80 interface clock */
+#define I80_DATA_SAMPLING_CYCLE		2
+#define I80_TE_PERIOD_US		1667
+#define I80_DATA_TRANSACTION_TIME_US	964
+
 /* FIMD has totally five hardware windows. */
 #define WINDOWS_NR	5
 
@@ -303,16 +308,25 @@ static void fimd_mgr_remove(struct exynos_drm_manager *mgr)
 static u32 fimd_calc_clkdiv(struct fimd_context *ctx,
 		const struct drm_display_mode *mode)
 {
-	unsigned long ideal_clk = mode->htotal * mode->vtotal * mode->vrefresh;
+	unsigned long ideal_clk;
 	u32 clkdiv;
 
 	if (ctx->i80_if) {
 		/*
-		 * The frame done interrupt should be occurred prior to the
-		 * next TE signal.
+		 * The I80 interface uses SYS_WE and SYS_CS to process 1 pixel
+		 * data, so it requires the twice faster clock than the pixel
+		 * clock[I80_DATA_SAMPLING_CYCLE].
+		 * And the frame done interrupt should occurr prior to the next
+		 * TE signal, H/W guy recommends to use as 1.73 times faster
+		 * frequency[I80_TE_PERIOD_US / I80_DATA_TRANSACTION_TIME_US].
 		 */
-		ideal_clk *= 2;
-	}
+		ideal_clk = mode->hdisplay * mode->vdisplay *
+				I80_DATA_SAMPLING_CYCLE *
+				I80_TE_PERIOD_US / I80_DATA_TRANSACTION_TIME_US;
+	} else
+		ideal_clk = mode->htotal * mode->vtotal;
+
+	ideal_clk *= mode->vrefresh;
 
 	/* Find the clock divider value that gets us closest to ideal_clk */
 	clkdiv = DIV_ROUND_UP(clk_get_rate(ctx->lcd_clk), ideal_clk);
@@ -431,7 +445,7 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
 		val |= VIDCON0_CLKSEL_LCD;
 
 	clkdiv = fimd_calc_clkdiv(ctx, mode);
-	if (clkdiv > 1)
+	if (clkdiv >= 1)
 		val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
 
 	writel(val, ctx->regs + VIDCON0);
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/7] drm/exynos: fimd: move handle vblank position in TE handler
  2014-10-01  6:19 [PATCH 0/7] drm/exynos: modify LCD I80 interface display YoungJun Cho
                   ` (2 preceding siblings ...)
  2014-10-01  6:19 ` [PATCH 3/7] drm/exynos: fimd: modify vclk calculation for I80 i/f YoungJun Cho
@ 2014-10-01  6:19 ` YoungJun Cho
  2014-11-14  1:20   ` Inki Dae
  2014-10-01  6:19 ` [PATCH 5/7] drm/exynos: fimd: modify I80 i/f interrupt relevant routine YoungJun Cho
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: YoungJun Cho @ 2014-10-01  6:19 UTC (permalink / raw)
  To: airlied, dri-devel; +Cc: a.hajda, kyungmin.park, sw0312.kim

For providing VBLANK information, drm_handle_vblank() should
be called properly, but it is blocked by wait_vsync_event
condition which is set by manager_ops->wait_for_vblank().
So moves it out from wait_vsync_event routine.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 05c2a97a..f062335 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -983,10 +983,10 @@ static void fimd_te_handler(struct exynos_drm_manager *mgr)
 	if (atomic_read(&ctx->wait_vsync_event)) {
 		atomic_set(&ctx->wait_vsync_event, 0);
 		wake_up(&ctx->wait_vsync_queue);
-
-		if (!atomic_read(&ctx->triggering))
-			drm_handle_vblank(ctx->drm_dev, ctx->pipe);
 	}
+
+	if (!atomic_read(&ctx->triggering))
+		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
 }
 
 static struct exynos_drm_manager_ops fimd_manager_ops = {
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/7] drm/exynos: fimd: modify I80 i/f interrupt relevant routine
  2014-10-01  6:19 [PATCH 0/7] drm/exynos: modify LCD I80 interface display YoungJun Cho
                   ` (3 preceding siblings ...)
  2014-10-01  6:19 ` [PATCH 4/7] drm/exynos: fimd: move handle vblank position in TE handler YoungJun Cho
@ 2014-10-01  6:19 ` YoungJun Cho
  2014-11-14  1:36   ` Inki Dae
  2014-10-01  6:19 ` [PATCH 6/7] drm/exynos: dsi: move TE irq handler registration position YoungJun Cho
  2014-10-01  6:19 ` [PATCH 7/7] drm/exynos: dsi: move DSIM_STATE_ENABLED set position YoungJun Cho
  6 siblings, 1 reply; 19+ messages in thread
From: YoungJun Cho @ 2014-10-01  6:19 UTC (permalink / raw)
  To: airlied, dri-devel; +Cc: a.hajda, kyungmin.park, sw0312.kim

For the I80 interface, the video interrupt pending register(VIDINTCON1)
should be handled in fimd_irq_handler() and the video interrupt control
register(VIDINTCON0) should be handled in fimd_enable_vblank() and
fimd_disable_vblank() like RGB interface.
So this patch moves each set / unset routines into proper positions.
And adds triggering unset routine in fimd_trigger() to exit from it
because there is a case like set config which requires triggering
but vblank is not enabled.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 60 ++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index f062335..c949099 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -463,12 +463,19 @@ static int fimd_enable_vblank(struct exynos_drm_manager *mgr)
 		val = readl(ctx->regs + VIDINTCON0);
 
 		val |= VIDINTCON0_INT_ENABLE;
-		val |= VIDINTCON0_INT_FRAME;
 
-		val &= ~VIDINTCON0_FRAMESEL0_MASK;
-		val |= VIDINTCON0_FRAMESEL0_VSYNC;
-		val &= ~VIDINTCON0_FRAMESEL1_MASK;
-		val |= VIDINTCON0_FRAMESEL1_NONE;
+		if (ctx->i80_if) {
+			val |= VIDINTCON0_INT_I80IFDONE;
+			val |= VIDINTCON0_INT_SYSMAINCON;
+			val &= ~VIDINTCON0_INT_SYSSUBCON;
+		} else {
+			val |= VIDINTCON0_INT_FRAME;
+
+			val &= ~VIDINTCON0_FRAMESEL0_MASK;
+			val |= VIDINTCON0_FRAMESEL0_VSYNC;
+			val &= ~VIDINTCON0_FRAMESEL1_MASK;
+			val |= VIDINTCON0_FRAMESEL1_NONE;
+		}
 
 		writel(val, ctx->regs + VIDINTCON0);
 	}
@@ -487,9 +494,15 @@ static void fimd_disable_vblank(struct exynos_drm_manager *mgr)
 	if (test_and_clear_bit(0, &ctx->irq_flags)) {
 		val = readl(ctx->regs + VIDINTCON0);
 
-		val &= ~VIDINTCON0_INT_FRAME;
 		val &= ~VIDINTCON0_INT_ENABLE;
 
+		if (ctx->i80_if) {
+			val &= ~VIDINTCON0_INT_I80IFDONE;
+			val &= ~VIDINTCON0_INT_SYSMAINCON;
+			val &= ~VIDINTCON0_INT_SYSSUBCON;
+		} else
+			val &= ~VIDINTCON0_INT_FRAME;
+
 		writel(val, ctx->regs + VIDINTCON0);
 	}
 }
@@ -945,16 +958,19 @@ static void fimd_trigger(struct device *dev)
 	void *timing_base = ctx->regs + driver_data->timing_base;
 	u32 reg;
 
+	/* Enters triggering mode */
 	atomic_set(&ctx->triggering, 1);
 
-	reg = readl(ctx->regs + VIDINTCON0);
-	reg |= (VIDINTCON0_INT_ENABLE | VIDINTCON0_INT_I80IFDONE |
-						VIDINTCON0_INT_SYSMAINCON);
-	writel(reg, ctx->regs + VIDINTCON0);
-
 	reg = readl(timing_base + TRIGCON);
 	reg |= (TRGMODE_I80_RGB_ENABLE_I80 | SWTRGCMD_I80_RGB_ENABLE);
 	writel(reg, timing_base + TRIGCON);
+
+	/*
+	 * Exits triggering mode if vblank is not enabled yet, because when the
+	 * VIDINTCON0 register is not set, it can not exit from triggering mode.
+	 */
+	if (!test_bit(0, &ctx->irq_flags))
+		atomic_set(&ctx->triggering, 0);
 }
 
 static void fimd_te_handler(struct exynos_drm_manager *mgr)
@@ -966,9 +982,9 @@ static void fimd_te_handler(struct exynos_drm_manager *mgr)
 		return;
 
 	 /*
-	 * Skips to trigger if in triggering state, because multiple triggering
-	 * requests can cause panel reset.
-	 */
+	  * Skips triggering if in triggering mode, because multiple triggering
+	  * requests can cause panel reset.
+	  */
 	if (atomic_read(&ctx->triggering))
 		return;
 
@@ -1023,21 +1039,13 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
 	if (ctx->pipe < 0 || !ctx->drm_dev)
 		goto out;
 
-	if (ctx->i80_if) {
-		/* unset I80 frame done interrupt */
-		val = readl(ctx->regs + VIDINTCON0);
-		val &= ~(VIDINTCON0_INT_I80IFDONE | VIDINTCON0_INT_SYSMAINCON);
-		writel(val, ctx->regs + VIDINTCON0);
+	drm_handle_vblank(ctx->drm_dev, ctx->pipe);
+	exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
 
-		/* exit triggering mode */
+	if (ctx->i80_if) {
+		/* Exits triggering mode */
 		atomic_set(&ctx->triggering, 0);
-
-		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
-		exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
 	} else {
-		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
-		exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
-
 		/* set wait vsync event to zero and wake up queue. */
 		if (atomic_read(&ctx->wait_vsync_event)) {
 			atomic_set(&ctx->wait_vsync_event, 0);
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/7] drm/exynos: dsi: move TE irq handler registration position
  2014-10-01  6:19 [PATCH 0/7] drm/exynos: modify LCD I80 interface display YoungJun Cho
                   ` (4 preceding siblings ...)
  2014-10-01  6:19 ` [PATCH 5/7] drm/exynos: fimd: modify I80 i/f interrupt relevant routine YoungJun Cho
@ 2014-10-01  6:19 ` YoungJun Cho
  2014-11-14  1:49   ` Inki Dae
  2014-10-01  6:19 ` [PATCH 7/7] drm/exynos: dsi: move DSIM_STATE_ENABLED set position YoungJun Cho
  6 siblings, 1 reply; 19+ messages in thread
From: YoungJun Cho @ 2014-10-01  6:19 UTC (permalink / raw)
  To: airlied, dri-devel; +Cc: a.hajda, kyungmin.park, sw0312.kim

The drm_helper_hpd_irq_event() does dpms control and panel is
initialized and displayed on by it.
So should register TE irq handler(exynos_dsi_te_irq_handler())
beforehand.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 24741d8..ded69df 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1143,6 +1143,7 @@ static int exynos_dsi_init(struct exynos_dsi *dsi)
 static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
 {
 	int ret;
+	int te_gpio_irq;
 
 	dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
 	if (!gpio_is_valid(dsi->te_gpio)) {
@@ -1157,14 +1158,10 @@ static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
 		goto out;
 	}
 
-	/*
-	 * This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
-	 * calls drm_panel_init() first then calls mipi_dsi_attach() in probe().
-	 * It means that te_gpio is invalid when exynos_dsi_enable_irq() is
-	 * called by drm_panel_init() before panel is attached.
-	 */
-	ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
-					exynos_dsi_te_irq_handler, NULL,
+	te_gpio_irq = gpio_to_irq(dsi->te_gpio);
+
+	irq_set_status_flags(te_gpio_irq, IRQ_NOAUTOEN);
+	ret = request_threaded_irq(te_gpio_irq, exynos_dsi_te_irq_handler, NULL,
 					IRQF_TRIGGER_RISING, "TE", dsi);
 	if (ret) {
 		dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
@@ -1195,9 +1192,6 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 	dsi->mode_flags = device->mode_flags;
 	dsi->panel_node = device->dev.of_node;
 
-	if (dsi->connector.dev)
-		drm_helper_hpd_irq_event(dsi->connector.dev);
-
 	/*
 	 * This is a temporary solution and should be made by more generic way.
 	 *
@@ -1211,6 +1205,9 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 			return ret;
 	}
 
+	if (dsi->connector.dev)
+		drm_helper_hpd_irq_event(dsi->connector.dev);
+
 	return 0;
 }
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/7] drm/exynos: dsi: move DSIM_STATE_ENABLED set position
  2014-10-01  6:19 [PATCH 0/7] drm/exynos: modify LCD I80 interface display YoungJun Cho
                   ` (5 preceding siblings ...)
  2014-10-01  6:19 ` [PATCH 6/7] drm/exynos: dsi: move TE irq handler registration position YoungJun Cho
@ 2014-10-01  6:19 ` YoungJun Cho
  2014-11-14  1:53   ` Inki Dae
  6 siblings, 1 reply; 19+ messages in thread
From: YoungJun Cho @ 2014-10-01  6:19 UTC (permalink / raw)
  To: airlied, dri-devel; +Cc: a.hajda, kyungmin.park, sw0312.kim

The command mode panel should draw image earlier than the display
on command execution to prevent showing garbage GRAM screen data.
So should set dsi->state as DSIM_STATE_ENABLED between calling
exynos_dsi_set_display_enable() and drm_panel_enable() to transmit
image data before executing display on command.
And moves the display on command execution routine from prepare()
to enable() in drm_panel_funcs also.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index ded69df..f304a20 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1366,16 +1366,17 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
 	exynos_dsi_set_display_mode(dsi);
 	exynos_dsi_set_display_enable(dsi, true);
 
+	dsi->state |= DSIM_STATE_ENABLED;
+
 	ret = drm_panel_enable(dsi->panel);
 	if (ret < 0) {
+		dsi->state &= ~DSIM_STATE_ENABLED;
 		exynos_dsi_set_display_enable(dsi, false);
 		drm_panel_unprepare(dsi->panel);
 		exynos_dsi_poweroff(dsi);
 		return ret;
 	}
 
-	dsi->state |= DSIM_STATE_ENABLED;
-
 	return 0;
 }
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/7] drm/exynos: fimd: remove unnecessary waiting vblank routine
  2014-10-01  6:19 ` [PATCH 1/7] drm/exynos: fimd: remove unnecessary waiting vblank routine YoungJun Cho
@ 2014-11-13  9:00   ` Inki Dae
  0 siblings, 0 replies; 19+ messages in thread
From: Inki Dae @ 2014-11-13  9:00 UTC (permalink / raw)
  To: YoungJun Cho; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

On 2014년 10월 01일 15:19, YoungJun Cho wrote:
> The exynos_drm_crtc_dpms() waits until pended page flip
> queue is empty, calls the drm_vblank_off() then calls
> manager->ops->dpms() when mode is DRM_MODE_DPMS_OFF.
> The fimd_dpms() is one of manager->ops->dpms()s and
> finally calls fimd_window_suspend().
> But there is no active window and vblank is already off
> when it is called.
> So addtional waiting vblank is not necessary any more.

Applied.

Thanks,
Inki Dae

> 
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 085b066..8b31b7e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -803,7 +803,6 @@ static void fimd_window_suspend(struct exynos_drm_manager *mgr)
>  		if (win_data->enabled)
>  			fimd_win_disable(mgr, i);
>  	}
> -	fimd_wait_for_vblank(mgr);
>  }
>  
>  static void fimd_window_resume(struct exynos_drm_manager *mgr)
> 

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/7] drm/exynos: fimd: add fimd_channel_win() to clean up code
  2014-10-01  6:19 ` [PATCH 2/7] drm/exynos: fimd: add fimd_channel_win() to clean up code YoungJun Cho
@ 2014-11-13  9:05   ` Inki Dae
  0 siblings, 0 replies; 19+ messages in thread
From: Inki Dae @ 2014-11-13  9:05 UTC (permalink / raw)
  To: YoungJun Cho; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

On 2014년 10월 01일 15:19, YoungJun Cho wrote:
> The ENWIN_F in WINCON# register and C#_EN_Fs in SHADOWCON register
> should be always matched together, so adds fimd_channel_win()
> to clean up code.
> And this fimd_channel_win() should be called before unprotecting
> window in fimd_win_commit().

Sorry for late. below is my comment.

> 
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 62 ++++++++++++++++----------------
>  1 file changed, 30 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 8b31b7e..b2f6007 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -214,6 +214,33 @@ static void fimd_wait_for_vblank(struct exynos_drm_manager *mgr)
>  		DRM_DEBUG_KMS("vblank wait timed out.\n");
>  }
>  
> +static void fimd_channel_win(struct fimd_context *ctx, int win, bool enable)
> +{
> +	u32 val;
> +
> +	/* for DMA output */
> +	val = readl(ctx->regs + WINCON(win));
> +
> +	if (enable)
> +		val |= WINCONx_ENWIN;
> +	else
> +		val &= ~WINCONx_ENWIN;
> +
> +	writel(val, ctx->regs + WINCON(win));
> +
> +	/* for shadow channel */
> +	if (ctx->driver_data->has_shadowcon) {
> +		val = readl(ctx->regs + SHADOWCON);
> +
> +		if (enable)
> +			val |= SHADOWCON_CHx_ENABLE(win);
> +		else
> +			val &= ~SHADOWCON_CHx_ENABLE(win);
> +
> +		writel(val, ctx->regs + SHADOWCON);
> +	}
> +}


This function includes two functionalities. One is controlling DMA
output. Other is controlling shadow channel. How about using separated
two functions instead? And let's call shadowcon control function only if
has_shadowcon is 1.

Thanks,
Inki Dae


> +
>  static void fimd_clear_channel(struct exynos_drm_manager *mgr)
>  {
>  	struct fimd_context *ctx = mgr->ctx;
> @@ -226,16 +253,7 @@ static void fimd_clear_channel(struct exynos_drm_manager *mgr)
>  		u32 val = readl(ctx->regs + WINCON(win));
>  
>  		if (val & WINCONx_ENWIN) {
> -			/* wincon */
> -			val &= ~WINCONx_ENWIN;
> -			writel(val, ctx->regs + WINCON(win));
> -
> -			/* unprotect windows */
> -			if (ctx->driver_data->has_shadowcon) {
> -				val = readl(ctx->regs + SHADOWCON);
> -				val &= ~SHADOWCON_CHx_ENABLE(win);
> -				writel(val, ctx->regs + SHADOWCON);
> -			}
> +			fimd_channel_win(ctx, win, false);
>  			ch_enabled = 1;
>  		}
>  	}
> @@ -730,20 +748,11 @@ static void fimd_win_commit(struct exynos_drm_manager *mgr, int zpos)
>  	if (win != 0)
>  		fimd_win_set_colkey(ctx, win);
>  
> -	/* wincon */
> -	val = readl(ctx->regs + WINCON(win));
> -	val |= WINCONx_ENWIN;
> -	writel(val, ctx->regs + WINCON(win));
> +	fimd_channel_win(ctx, win, true);
>  
>  	/* Enable DMA channel and unprotect windows */
>  	fimd_shadow_protect_win(ctx, win, false);
>  
> -	if (ctx->driver_data->has_shadowcon) {
> -		val = readl(ctx->regs + SHADOWCON);
> -		val |= SHADOWCON_CHx_ENABLE(win);
> -		writel(val, ctx->regs + SHADOWCON);
> -	}
> -
>  	win_data->enabled = true;
>  
>  	if (ctx->i80_if)
> @@ -755,7 +764,6 @@ static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
>  	struct fimd_context *ctx = mgr->ctx;
>  	struct fimd_win_data *win_data;
>  	int win = zpos;
> -	u32 val;
>  
>  	if (win == DEFAULT_ZPOS)
>  		win = ctx->default_win;
> @@ -774,17 +782,7 @@ static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
>  	/* protect windows */
>  	fimd_shadow_protect_win(ctx, win, true);
>  
> -	/* wincon */
> -	val = readl(ctx->regs + WINCON(win));
> -	val &= ~WINCONx_ENWIN;
> -	writel(val, ctx->regs + WINCON(win));
> -
> -	/* unprotect windows */
> -	if (ctx->driver_data->has_shadowcon) {
> -		val = readl(ctx->regs + SHADOWCON);
> -		val &= ~SHADOWCON_CHx_ENABLE(win);
> -		writel(val, ctx->regs + SHADOWCON);
> -	}
> +	fimd_channel_win(ctx, win, false);
>  
>  	fimd_shadow_protect_win(ctx, win, false);
>  
> 

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/7] drm/exynos: fimd: modify vclk calculation for I80 i/f
  2014-10-01  6:19 ` [PATCH 3/7] drm/exynos: fimd: modify vclk calculation for I80 i/f YoungJun Cho
@ 2014-11-13  9:12   ` Inki Dae
  2014-11-13  9:54     ` YoungJun Cho
  0 siblings, 1 reply; 19+ messages in thread
From: Inki Dae @ 2014-11-13  9:12 UTC (permalink / raw)
  To: YoungJun Cho; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

On 2014년 10월 01일 15:19, YoungJun Cho wrote:
> The I80 interface uses SYS_WE and SYS_CS to process
> 1 pixel data, so it requires the twice faster clock
> than the pixel clock.
> And the frame done interrupt should occurr prior to
> the next TE signal, H/W guy recommends to use as 1.73
> times faster clock frequency.
> 
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index b2f6007..05c2a97a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -81,6 +81,11 @@
>  #define LCD_WR_HOLD(x)			((x) << 4)
>  #define I80IFEN_ENABLE			(1 << 0)
>  
> +/* I80 interface clock */
> +#define I80_DATA_SAMPLING_CYCLE		2
> +#define I80_TE_PERIOD_US		1667
> +#define I80_DATA_TRANSACTION_TIME_US	964
> +
>  /* FIMD has totally five hardware windows. */
>  #define WINDOWS_NR	5
>  
> @@ -303,16 +308,25 @@ static void fimd_mgr_remove(struct exynos_drm_manager *mgr)
>  static u32 fimd_calc_clkdiv(struct fimd_context *ctx,
>  		const struct drm_display_mode *mode)
>  {
> -	unsigned long ideal_clk = mode->htotal * mode->vtotal * mode->vrefresh;
> +	unsigned long ideal_clk;
>  	u32 clkdiv;
>  
>  	if (ctx->i80_if) {
>  		/*
> -		 * The frame done interrupt should be occurred prior to the
> -		 * next TE signal.
> +		 * The I80 interface uses SYS_WE and SYS_CS to process 1 pixel
> +		 * data, so it requires the twice faster clock than the pixel
> +		 * clock[I80_DATA_SAMPLING_CYCLE].
> +		 * And the frame done interrupt should occurr prior to the next
> +		 * TE signal, H/W guy recommends to use as 1.73 times faster
> +		 * frequency[I80_TE_PERIOD_US / I80_DATA_TRANSACTION_TIME_US].
>  		 */
> -		ideal_clk *= 2;
> -	}
> +		ideal_clk = mode->hdisplay * mode->vdisplay *
> +				I80_DATA_SAMPLING_CYCLE *
> +				I80_TE_PERIOD_US / I80_DATA_TRANSACTION_TIME_US;

How about using one constant directly?

I.e.,
#define RECOMMENDED_VAR	1.73

 ... I80_DATA_SAMPLING_CYCLE * RECOMMENDED_VAR

Is there any case that the te period and data transaction time should be
changed?


Thanks,
Inki Dae


> +	} else
> +		ideal_clk = mode->htotal * mode->vtotal;
> +
> +	ideal_clk *= mode->vrefresh;
>  
>  	/* Find the clock divider value that gets us closest to ideal_clk */
>  	clkdiv = DIV_ROUND_UP(clk_get_rate(ctx->lcd_clk), ideal_clk);
> @@ -431,7 +445,7 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
>  		val |= VIDCON0_CLKSEL_LCD;
>  
>  	clkdiv = fimd_calc_clkdiv(ctx, mode);
> -	if (clkdiv > 1)
> +	if (clkdiv >= 1)
>  		val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
>  
>  	writel(val, ctx->regs + VIDCON0);
> 

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/7] drm/exynos: fimd: modify vclk calculation for I80 i/f
  2014-11-13  9:12   ` Inki Dae
@ 2014-11-13  9:54     ` YoungJun Cho
  2014-11-14  1:25       ` Inki Dae
  0 siblings, 1 reply; 19+ messages in thread
From: YoungJun Cho @ 2014-11-13  9:54 UTC (permalink / raw)
  To: Inki Dae; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

Hi Inki,

On 11/13/2014 06:12 PM, Inki Dae wrote:
> On 2014년 10월 01일 15:19, YoungJun Cho wrote:
>> The I80 interface uses SYS_WE and SYS_CS to process
>> 1 pixel data, so it requires the twice faster clock
>> than the pixel clock.
>> And the frame done interrupt should occurr prior to
>> the next TE signal, H/W guy recommends to use as 1.73
>> times faster clock frequency.
>>
>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>> Acked-by: Inki Dae <inki.dae@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c | 26 ++++++++++++++++++++------
>>   1 file changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index b2f6007..05c2a97a 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -81,6 +81,11 @@
>>   #define LCD_WR_HOLD(x)			((x) << 4)
>>   #define I80IFEN_ENABLE			(1 << 0)
>>
>> +/* I80 interface clock */
>> +#define I80_DATA_SAMPLING_CYCLE		2
>> +#define I80_TE_PERIOD_US		1667
>> +#define I80_DATA_TRANSACTION_TIME_US	964
>> +
>>   /* FIMD has totally five hardware windows. */
>>   #define WINDOWS_NR	5
>>
>> @@ -303,16 +308,25 @@ static void fimd_mgr_remove(struct exynos_drm_manager *mgr)
>>   static u32 fimd_calc_clkdiv(struct fimd_context *ctx,
>>   		const struct drm_display_mode *mode)
>>   {
>> -	unsigned long ideal_clk = mode->htotal * mode->vtotal * mode->vrefresh;
>> +	unsigned long ideal_clk;
>>   	u32 clkdiv;
>>
>>   	if (ctx->i80_if) {
>>   		/*
>> -		 * The frame done interrupt should be occurred prior to the
>> -		 * next TE signal.
>> +		 * The I80 interface uses SYS_WE and SYS_CS to process 1 pixel
>> +		 * data, so it requires the twice faster clock than the pixel
>> +		 * clock[I80_DATA_SAMPLING_CYCLE].
>> +		 * And the frame done interrupt should occurr prior to the next
>> +		 * TE signal, H/W guy recommends to use as 1.73 times faster
>> +		 * frequency[I80_TE_PERIOD_US / I80_DATA_TRANSACTION_TIME_US].
>>   		 */
>> -		ideal_clk *= 2;
>> -	}
>> +		ideal_clk = mode->hdisplay * mode->vdisplay *
>> +				I80_DATA_SAMPLING_CYCLE *
>> +				I80_TE_PERIOD_US / I80_DATA_TRANSACTION_TIME_US;
>
> How about using one constant directly?
>
> I.e.,
> #define RECOMMENDED_VAR	1.73
>
>   ... I80_DATA_SAMPLING_CYCLE * RECOMMENDED_VAR
>
> Is there any case that the te period and data transaction time should be
> changed?
>

At first time, I did.

But it made following build break :
   drivers/built-in.o: In function `fimd_commit':
   exynos_adc.c:(.text+0x878bc): undefined reference to `__aeabi_ui2d'
   exynos_adc.c:(.text+0x878d0): undefined reference to `__aeabi_dmul'
   exynos_adc.c:(.text+0x878d4): undefined reference to `__aeabi_d2uiz'
   make: *** [vmlinux] Error 1

It came from using floating point value.
So I changed it.

Do you have any good idea ?

Thank you.
Best regards YJ

>
> Thanks,
> Inki Dae
>
>
>> +	} else
>> +		ideal_clk = mode->htotal * mode->vtotal;
>> +
>> +	ideal_clk *= mode->vrefresh;
>>
>>   	/* Find the clock divider value that gets us closest to ideal_clk */
>>   	clkdiv = DIV_ROUND_UP(clk_get_rate(ctx->lcd_clk), ideal_clk);
>> @@ -431,7 +445,7 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
>>   		val |= VIDCON0_CLKSEL_LCD;
>>
>>   	clkdiv = fimd_calc_clkdiv(ctx, mode);
>> -	if (clkdiv > 1)
>> +	if (clkdiv >= 1)
>>   		val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
>>
>>   	writel(val, ctx->regs + VIDCON0);
>>
>
>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] drm/exynos: fimd: move handle vblank position in TE handler
  2014-10-01  6:19 ` [PATCH 4/7] drm/exynos: fimd: move handle vblank position in TE handler YoungJun Cho
@ 2014-11-14  1:20   ` Inki Dae
  0 siblings, 0 replies; 19+ messages in thread
From: Inki Dae @ 2014-11-14  1:20 UTC (permalink / raw)
  To: YoungJun Cho; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

On 2014년 10월 01일 15:19, YoungJun Cho wrote:
> For providing VBLANK information, drm_handle_vblank() should
> be called properly, but it is blocked by wait_vsync_event
> condition which is set by manager_ops->wait_for_vblank().
> So moves it out from wait_vsync_event routine.

Applied.

Thanks,
Inki Dae

> 
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 05c2a97a..f062335 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -983,10 +983,10 @@ static void fimd_te_handler(struct exynos_drm_manager *mgr)
>  	if (atomic_read(&ctx->wait_vsync_event)) {
>  		atomic_set(&ctx->wait_vsync_event, 0);
>  		wake_up(&ctx->wait_vsync_queue);
> -
> -		if (!atomic_read(&ctx->triggering))
> -			drm_handle_vblank(ctx->drm_dev, ctx->pipe);
>  	}
> +
> +	if (!atomic_read(&ctx->triggering))
> +		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
>  }
>  
>  static struct exynos_drm_manager_ops fimd_manager_ops = {
> 

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/7] drm/exynos: fimd: modify vclk calculation for I80 i/f
  2014-11-13  9:54     ` YoungJun Cho
@ 2014-11-14  1:25       ` Inki Dae
  0 siblings, 0 replies; 19+ messages in thread
From: Inki Dae @ 2014-11-14  1:25 UTC (permalink / raw)
  To: YoungJun Cho; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

On 2014년 11월 13일 18:54, YoungJun Cho wrote:
> Hi Inki,
> 
> On 11/13/2014 06:12 PM, Inki Dae wrote:
>> On 2014년 10월 01일 15:19, YoungJun Cho wrote:
>>> The I80 interface uses SYS_WE and SYS_CS to process
>>> 1 pixel data, so it requires the twice faster clock
>>> than the pixel clock.
>>> And the frame done interrupt should occurr prior to
>>> the next TE signal, H/W guy recommends to use as 1.73
>>> times faster clock frequency.
>>>
>>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>>> Acked-by: Inki Dae <inki.dae@samsung.com>
>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c | 26
>>> ++++++++++++++++++++------
>>>   1 file changed, 20 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> index b2f6007..05c2a97a 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> @@ -81,6 +81,11 @@
>>>   #define LCD_WR_HOLD(x)            ((x) << 4)
>>>   #define I80IFEN_ENABLE            (1 << 0)
>>>
>>> +/* I80 interface clock */
>>> +#define I80_DATA_SAMPLING_CYCLE        2
>>> +#define I80_TE_PERIOD_US        1667
>>> +#define I80_DATA_TRANSACTION_TIME_US    964
>>> +
>>>   /* FIMD has totally five hardware windows. */
>>>   #define WINDOWS_NR    5
>>>
>>> @@ -303,16 +308,25 @@ static void fimd_mgr_remove(struct
>>> exynos_drm_manager *mgr)
>>>   static u32 fimd_calc_clkdiv(struct fimd_context *ctx,
>>>           const struct drm_display_mode *mode)
>>>   {
>>> -    unsigned long ideal_clk = mode->htotal * mode->vtotal *
>>> mode->vrefresh;
>>> +    unsigned long ideal_clk;
>>>       u32 clkdiv;
>>>
>>>       if (ctx->i80_if) {
>>>           /*
>>> -         * The frame done interrupt should be occurred prior to the
>>> -         * next TE signal.
>>> +         * The I80 interface uses SYS_WE and SYS_CS to process 1 pixel
>>> +         * data, so it requires the twice faster clock than the pixel
>>> +         * clock[I80_DATA_SAMPLING_CYCLE].
>>> +         * And the frame done interrupt should occurr prior to the next
>>> +         * TE signal, H/W guy recommends to use as 1.73 times faster
>>> +         * frequency[I80_TE_PERIOD_US / I80_DATA_TRANSACTION_TIME_US].
>>>            */
>>> -        ideal_clk *= 2;
>>> -    }
>>> +        ideal_clk = mode->hdisplay * mode->vdisplay *
>>> +                I80_DATA_SAMPLING_CYCLE *
>>> +                I80_TE_PERIOD_US / I80_DATA_TRANSACTION_TIME_US;
>>
>> How about using one constant directly?
>>
>> I.e.,
>> #define RECOMMENDED_VAR    1.73
>>
>>   ... I80_DATA_SAMPLING_CYCLE * RECOMMENDED_VAR
>>
>> Is there any case that the te period and data transaction time should be
>> changed?
>>
> 
> At first time, I did.
> 
> But it made following build break :
>   drivers/built-in.o: In function `fimd_commit':
>   exynos_adc.c:(.text+0x878bc): undefined reference to `__aeabi_ui2d'
>   exynos_adc.c:(.text+0x878d0): undefined reference to `__aeabi_dmul'
>   exynos_adc.c:(.text+0x878d4): undefined reference to `__aeabi_d2uiz'
>   make: *** [vmlinux] Error 1
> 
> It came from using floating point value.
> So I changed it.

Ah, sorry. I missed it. I think there would be better idea. Let's try to
find the idea.

Thanks,
Inki Dae

> 
> Do you have any good idea ?
> 
> Thank you.
> Best regards YJ
> 
>>
>> Thanks,
>> Inki Dae
>>
>>
>>> +    } else
>>> +        ideal_clk = mode->htotal * mode->vtotal;
>>> +
>>> +    ideal_clk *= mode->vrefresh;
>>>
>>>       /* Find the clock divider value that gets us closest to
>>> ideal_clk */
>>>       clkdiv = DIV_ROUND_UP(clk_get_rate(ctx->lcd_clk), ideal_clk);
>>> @@ -431,7 +445,7 @@ static void fimd_commit(struct exynos_drm_manager
>>> *mgr)
>>>           val |= VIDCON0_CLKSEL_LCD;
>>>
>>>       clkdiv = fimd_calc_clkdiv(ctx, mode);
>>> -    if (clkdiv > 1)
>>> +    if (clkdiv >= 1)
>>>           val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
>>>
>>>       writel(val, ctx->regs + VIDCON0);
>>>
>>
>>
> 
> 

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/7] drm/exynos: fimd: modify I80 i/f interrupt relevant routine
  2014-10-01  6:19 ` [PATCH 5/7] drm/exynos: fimd: modify I80 i/f interrupt relevant routine YoungJun Cho
@ 2014-11-14  1:36   ` Inki Dae
  2014-11-16  0:53     ` YoungJun Cho
  0 siblings, 1 reply; 19+ messages in thread
From: Inki Dae @ 2014-11-14  1:36 UTC (permalink / raw)
  To: YoungJun Cho; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

On 2014년 10월 01일 15:19, YoungJun Cho wrote:
> For the I80 interface, the video interrupt pending register(VIDINTCON1)
> should be handled in fimd_irq_handler() and the video interrupt control
> register(VIDINTCON0) should be handled in fimd_enable_vblank() and
> fimd_disable_vblank() like RGB interface.
> So this patch moves each set / unset routines into proper positions.
> And adds triggering unset routine in fimd_trigger() to exit from it
> because there is a case like set config which requires triggering
> but vblank is not enabled.

Reasonable but how about separating this patch into two patches. One is
set/unset properly the registers relevant to interrupt, and other?

It seems that your patch includes some codes not relevant to above
description. And below is my comment.

Thanks,
Inki Dae

> 
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 60 ++++++++++++++++++--------------
>  1 file changed, 34 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index f062335..c949099 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -463,12 +463,19 @@ static int fimd_enable_vblank(struct exynos_drm_manager *mgr)
>  		val = readl(ctx->regs + VIDINTCON0);
>  
>  		val |= VIDINTCON0_INT_ENABLE;
> -		val |= VIDINTCON0_INT_FRAME;
>  
> -		val &= ~VIDINTCON0_FRAMESEL0_MASK;
> -		val |= VIDINTCON0_FRAMESEL0_VSYNC;
> -		val &= ~VIDINTCON0_FRAMESEL1_MASK;
> -		val |= VIDINTCON0_FRAMESEL1_NONE;
> +		if (ctx->i80_if) {
> +			val |= VIDINTCON0_INT_I80IFDONE;
> +			val |= VIDINTCON0_INT_SYSMAINCON;
> +			val &= ~VIDINTCON0_INT_SYSSUBCON;
> +		} else {
> +			val |= VIDINTCON0_INT_FRAME;
> +
> +			val &= ~VIDINTCON0_FRAMESEL0_MASK;
> +			val |= VIDINTCON0_FRAMESEL0_VSYNC;
> +			val &= ~VIDINTCON0_FRAMESEL1_MASK;
> +			val |= VIDINTCON0_FRAMESEL1_NONE;
> +		}
>  
>  		writel(val, ctx->regs + VIDINTCON0);
>  	}
> @@ -487,9 +494,15 @@ static void fimd_disable_vblank(struct exynos_drm_manager *mgr)
>  	if (test_and_clear_bit(0, &ctx->irq_flags)) {
>  		val = readl(ctx->regs + VIDINTCON0);
>  
> -		val &= ~VIDINTCON0_INT_FRAME;
>  		val &= ~VIDINTCON0_INT_ENABLE;
>  
> +		if (ctx->i80_if) {
> +			val &= ~VIDINTCON0_INT_I80IFDONE;
> +			val &= ~VIDINTCON0_INT_SYSMAINCON;
> +			val &= ~VIDINTCON0_INT_SYSSUBCON;
> +		} else
> +			val &= ~VIDINTCON0_INT_FRAME;
> +
>  		writel(val, ctx->regs + VIDINTCON0);
>  	}
>  }
> @@ -945,16 +958,19 @@ static void fimd_trigger(struct device *dev)
>  	void *timing_base = ctx->regs + driver_data->timing_base;
>  	u32 reg;
>  
> +	/* Enters triggering mode */
>  	atomic_set(&ctx->triggering, 1);
>  
> -	reg = readl(ctx->regs + VIDINTCON0);
> -	reg |= (VIDINTCON0_INT_ENABLE | VIDINTCON0_INT_I80IFDONE |
> -						VIDINTCON0_INT_SYSMAINCON);
> -	writel(reg, ctx->regs + VIDINTCON0);
> -
>  	reg = readl(timing_base + TRIGCON);
>  	reg |= (TRGMODE_I80_RGB_ENABLE_I80 | SWTRGCMD_I80_RGB_ENABLE);
>  	writel(reg, timing_base + TRIGCON);
> +
> +	/*
> +	 * Exits triggering mode if vblank is not enabled yet, because when the
> +	 * VIDINTCON0 register is not set, it can not exit from triggering mode.
> +	 */
> +	if (!test_bit(0, &ctx->irq_flags))
> +		atomic_set(&ctx->triggering, 0);

I think this code would make for other trigger requested while triggering.

>  }
>  
>  static void fimd_te_handler(struct exynos_drm_manager *mgr)
> @@ -966,9 +982,9 @@ static void fimd_te_handler(struct exynos_drm_manager *mgr)
>  		return;
>  
>  	 /*
> -	 * Skips to trigger if in triggering state, because multiple triggering
> -	 * requests can cause panel reset.
> -	 */
> +	  * Skips triggering if in triggering mode, because multiple triggering
> +	  * requests can cause panel reset.
> +	  */
>  	if (atomic_read(&ctx->triggering))
>  		return;
>  
> @@ -1023,21 +1039,13 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>  	if (ctx->pipe < 0 || !ctx->drm_dev)
>  		goto out;
>  
> -	if (ctx->i80_if) {
> -		/* unset I80 frame done interrupt */
> -		val = readl(ctx->regs + VIDINTCON0);
> -		val &= ~(VIDINTCON0_INT_I80IFDONE | VIDINTCON0_INT_SYSMAINCON);
> -		writel(val, ctx->regs + VIDINTCON0);
> +	drm_handle_vblank(ctx->drm_dev, ctx->pipe);
> +	exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>  
> -		/* exit triggering mode */
> +	if (ctx->i80_if) {
> +		/* Exits triggering mode */
>  		atomic_set(&ctx->triggering, 0);
> -
> -		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
> -		exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>  	} else {
> -		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
> -		exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
> -
>  		/* set wait vsync event to zero and wake up queue. */
>  		if (atomic_read(&ctx->wait_vsync_event)) {
>  			atomic_set(&ctx->wait_vsync_event, 0);
> 

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/7] drm/exynos: dsi: move TE irq handler registration position
  2014-10-01  6:19 ` [PATCH 6/7] drm/exynos: dsi: move TE irq handler registration position YoungJun Cho
@ 2014-11-14  1:49   ` Inki Dae
  2014-11-14  2:08     ` YoungJun Cho
  0 siblings, 1 reply; 19+ messages in thread
From: Inki Dae @ 2014-11-14  1:49 UTC (permalink / raw)
  To: YoungJun Cho; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

On 2014년 10월 01일 15:19, YoungJun Cho wrote:
> The drm_helper_hpd_irq_event() does dpms control and panel is
> initialized and displayed on by it.
> So should register TE irq handler(exynos_dsi_te_irq_handler())
> beforehand.

This patch also includes some codes not relevant to register TE irq
handler before drm_helper_hpd_irq_event call. How about separating this
patch into two patches?

And below is my comment.

Thanks,
Inki Dae

> 
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 24741d8..ded69df 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1143,6 +1143,7 @@ static int exynos_dsi_init(struct exynos_dsi *dsi)
>  static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
>  {
>  	int ret;
> +	int te_gpio_irq;
>  
>  	dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
>  	if (!gpio_is_valid(dsi->te_gpio)) {
> @@ -1157,14 +1158,10 @@ static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
>  		goto out;
>  	}
>  
> -	/*
> -	 * This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
> -	 * calls drm_panel_init() first then calls mipi_dsi_attach() in probe().
> -	 * It means that te_gpio is invalid when exynos_dsi_enable_irq() is
> -	 * called by drm_panel_init() before panel is attached.
> -	 */
> -	ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
> -					exynos_dsi_te_irq_handler, NULL,
> +	te_gpio_irq = gpio_to_irq(dsi->te_gpio);
> +
> +	irq_set_status_flags(te_gpio_irq, IRQ_NOAUTOEN);

I think with IRQ_NOAUTOEN, te irq wouldn't be enabled automatically. So
shouldn't we call enable_irq() and disable_irq() somewhere?

> +	ret = request_threaded_irq(te_gpio_irq, exynos_dsi_te_irq_handler, NULL,
>  					IRQF_TRIGGER_RISING, "TE", dsi);
>  	if (ret) {
>  		dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
> @@ -1195,9 +1192,6 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>  	dsi->mode_flags = device->mode_flags;
>  	dsi->panel_node = device->dev.of_node;
>  
> -	if (dsi->connector.dev)
> -		drm_helper_hpd_irq_event(dsi->connector.dev);
> -
>  	/*
>  	 * This is a temporary solution and should be made by more generic way.
>  	 *
> @@ -1211,6 +1205,9 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>  			return ret;
>  	}
>  
> +	if (dsi->connector.dev)
> +		drm_helper_hpd_irq_event(dsi->connector.dev);
> +
>  	return 0;
>  }
>  
> 

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 7/7] drm/exynos: dsi: move DSIM_STATE_ENABLED set position
  2014-10-01  6:19 ` [PATCH 7/7] drm/exynos: dsi: move DSIM_STATE_ENABLED set position YoungJun Cho
@ 2014-11-14  1:53   ` Inki Dae
  0 siblings, 0 replies; 19+ messages in thread
From: Inki Dae @ 2014-11-14  1:53 UTC (permalink / raw)
  To: YoungJun Cho; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

On 2014년 10월 01일 15:19, YoungJun Cho wrote:
> The command mode panel should draw image earlier than the display
> on command execution to prevent showing garbage GRAM screen data.
> So should set dsi->state as DSIM_STATE_ENABLED between calling
> exynos_dsi_set_display_enable() and drm_panel_enable() to transmit
> image data before executing display on command.
> And moves the display on command execution routine from prepare()
> to enable() in drm_panel_funcs also.

Applied.

Thanks,
Inki Dae

> 
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index ded69df..f304a20 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1366,16 +1366,17 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
>  	exynos_dsi_set_display_mode(dsi);
>  	exynos_dsi_set_display_enable(dsi, true);
>  
> +	dsi->state |= DSIM_STATE_ENABLED;
> +
>  	ret = drm_panel_enable(dsi->panel);
>  	if (ret < 0) {
> +		dsi->state &= ~DSIM_STATE_ENABLED;
>  		exynos_dsi_set_display_enable(dsi, false);
>  		drm_panel_unprepare(dsi->panel);
>  		exynos_dsi_poweroff(dsi);
>  		return ret;
>  	}
>  
> -	dsi->state |= DSIM_STATE_ENABLED;
> -
>  	return 0;
>  }
>  
> 

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/7] drm/exynos: dsi: move TE irq handler registration position
  2014-11-14  1:49   ` Inki Dae
@ 2014-11-14  2:08     ` YoungJun Cho
  0 siblings, 0 replies; 19+ messages in thread
From: YoungJun Cho @ 2014-11-14  2:08 UTC (permalink / raw)
  To: Inki Dae; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

Hi Inki,

On 11/14/2014 10:49 AM, Inki Dae wrote:
> On 2014년 10월 01일 15:19, YoungJun Cho wrote:
>> The drm_helper_hpd_irq_event() does dpms control and panel is
>> initialized and displayed on by it.
>> So should register TE irq handler(exynos_dsi_te_irq_handler())
>> beforehand.
>
> This patch also includes some codes not relevant to register TE irq
> handler before drm_helper_hpd_irq_event call. How about separating this
> patch into two patches?
>
> And below is my comment.
>
> Thanks,
> Inki Dae
>
>>
>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>> Acked-by: Inki Dae <inki.dae@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 19 ++++++++-----------
>>   1 file changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index 24741d8..ded69df 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -1143,6 +1143,7 @@ static int exynos_dsi_init(struct exynos_dsi *dsi)
>>   static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
>>   {
>>   	int ret;
>> +	int te_gpio_irq;
>>
>>   	dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
>>   	if (!gpio_is_valid(dsi->te_gpio)) {
>> @@ -1157,14 +1158,10 @@ static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
>>   		goto out;
>>   	}
>>
>> -	/*
>> -	 * This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
>> -	 * calls drm_panel_init() first then calls mipi_dsi_attach() in probe().
>> -	 * It means that te_gpio is invalid when exynos_dsi_enable_irq() is
>> -	 * called by drm_panel_init() before panel is attached.
>> -	 */
>> -	ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
>> -					exynos_dsi_te_irq_handler, NULL,
>> +	te_gpio_irq = gpio_to_irq(dsi->te_gpio);
>> +
>> +	irq_set_status_flags(te_gpio_irq, IRQ_NOAUTOEN);
>
> I think with IRQ_NOAUTOEN, te irq wouldn't be enabled automatically. So
> shouldn't we call enable_irq() and disable_irq() somewhere?

The te_gpio irq triggering is done by exynos_dsi_enable_irq() and 
exynos_dsi_disable_irq() like dsi irq.

And I'll separate patch with others also.

Thank you.
Best regards YJ

>
>> +	ret = request_threaded_irq(te_gpio_irq, exynos_dsi_te_irq_handler, NULL,
>>   					IRQF_TRIGGER_RISING, "TE", dsi);
>>   	if (ret) {
>>   		dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
>> @@ -1195,9 +1192,6 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>>   	dsi->mode_flags = device->mode_flags;
>>   	dsi->panel_node = device->dev.of_node;
>>
>> -	if (dsi->connector.dev)
>> -		drm_helper_hpd_irq_event(dsi->connector.dev);
>> -
>>   	/*
>>   	 * This is a temporary solution and should be made by more generic way.
>>   	 *
>> @@ -1211,6 +1205,9 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>>   			return ret;
>>   	}
>>
>> +	if (dsi->connector.dev)
>> +		drm_helper_hpd_irq_event(dsi->connector.dev);
>> +
>>   	return 0;
>>   }
>>
>>
>
>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/7] drm/exynos: fimd: modify I80 i/f interrupt relevant routine
  2014-11-14  1:36   ` Inki Dae
@ 2014-11-16  0:53     ` YoungJun Cho
  0 siblings, 0 replies; 19+ messages in thread
From: YoungJun Cho @ 2014-11-16  0:53 UTC (permalink / raw)
  To: Inki Dae; +Cc: sw0312.kim, dri-devel, a.hajda, kyungmin.park

Hi Inki,

On 11/14/2014 10:36 AM, Inki Dae wrote:
> On 2014년 10월 01일 15:19, YoungJun Cho wrote:
>> For the I80 interface, the video interrupt pending register(VIDINTCON1)
>> should be handled in fimd_irq_handler() and the video interrupt control
>> register(VIDINTCON0) should be handled in fimd_enable_vblank() and
>> fimd_disable_vblank() like RGB interface.
>> So this patch moves each set / unset routines into proper positions.
>> And adds triggering unset routine in fimd_trigger() to exit from it
>> because there is a case like set config which requires triggering
>> but vblank is not enabled.
>
> Reasonable but how about separating this patch into two patches. One is
> set/unset properly the registers relevant to interrupt, and other?
>
> It seems that your patch includes some codes not relevant to above
> description. And below is my comment.
>
> Thanks,
> Inki Dae
>
>>
>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>> Acked-by: Inki Dae <inki.dae@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c | 60 ++++++++++++++++++--------------
>>   1 file changed, 34 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index f062335..c949099 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -463,12 +463,19 @@ static int fimd_enable_vblank(struct exynos_drm_manager *mgr)
>>   		val = readl(ctx->regs + VIDINTCON0);
>>
>>   		val |= VIDINTCON0_INT_ENABLE;
>> -		val |= VIDINTCON0_INT_FRAME;
>>
>> -		val &= ~VIDINTCON0_FRAMESEL0_MASK;
>> -		val |= VIDINTCON0_FRAMESEL0_VSYNC;
>> -		val &= ~VIDINTCON0_FRAMESEL1_MASK;
>> -		val |= VIDINTCON0_FRAMESEL1_NONE;
>> +		if (ctx->i80_if) {
>> +			val |= VIDINTCON0_INT_I80IFDONE;
>> +			val |= VIDINTCON0_INT_SYSMAINCON;
>> +			val &= ~VIDINTCON0_INT_SYSSUBCON;
>> +		} else {
>> +			val |= VIDINTCON0_INT_FRAME;
>> +
>> +			val &= ~VIDINTCON0_FRAMESEL0_MASK;
>> +			val |= VIDINTCON0_FRAMESEL0_VSYNC;
>> +			val &= ~VIDINTCON0_FRAMESEL1_MASK;
>> +			val |= VIDINTCON0_FRAMESEL1_NONE;
>> +		}
>>
>>   		writel(val, ctx->regs + VIDINTCON0);
>>   	}
>> @@ -487,9 +494,15 @@ static void fimd_disable_vblank(struct exynos_drm_manager *mgr)
>>   	if (test_and_clear_bit(0, &ctx->irq_flags)) {
>>   		val = readl(ctx->regs + VIDINTCON0);
>>
>> -		val &= ~VIDINTCON0_INT_FRAME;
>>   		val &= ~VIDINTCON0_INT_ENABLE;
>>
>> +		if (ctx->i80_if) {
>> +			val &= ~VIDINTCON0_INT_I80IFDONE;
>> +			val &= ~VIDINTCON0_INT_SYSMAINCON;
>> +			val &= ~VIDINTCON0_INT_SYSSUBCON;
>> +		} else
>> +			val &= ~VIDINTCON0_INT_FRAME;
>> +
>>   		writel(val, ctx->regs + VIDINTCON0);
>>   	}
>>   }
>> @@ -945,16 +958,19 @@ static void fimd_trigger(struct device *dev)
>>   	void *timing_base = ctx->regs + driver_data->timing_base;
>>   	u32 reg;
>>
>> +	/* Enters triggering mode */
>>   	atomic_set(&ctx->triggering, 1);
>>
>> -	reg = readl(ctx->regs + VIDINTCON0);
>> -	reg |= (VIDINTCON0_INT_ENABLE | VIDINTCON0_INT_I80IFDONE |
>> -						VIDINTCON0_INT_SYSMAINCON);
>> -	writel(reg, ctx->regs + VIDINTCON0);
>> -
>>   	reg = readl(timing_base + TRIGCON);
>>   	reg |= (TRGMODE_I80_RGB_ENABLE_I80 | SWTRGCMD_I80_RGB_ENABLE);
>>   	writel(reg, timing_base + TRIGCON);
>> +
>> +	/*
>> +	 * Exits triggering mode if vblank is not enabled yet, because when the
>> +	 * VIDINTCON0 register is not set, it can not exit from triggering mode.
>> +	 */
>> +	if (!test_bit(0, &ctx->irq_flags))
>> +		atomic_set(&ctx->triggering, 0);
>
> I think this code would make for other trigger requested while triggering.
>

I missed this comment.

After modifying this patch, the I80 interface works with vblank enable / 
disable.
And there is a case like set config which requires triggering to update 
frame buffer but vblank is not enabled yet. So this exception routine is 
required to escape from triggering mode.

The connector DPMS is off earlier than CRTC DPMS. So I think the TE 
interrupt is stopped earlier than vblank is disabled.

Thank you.
Best regards YJ

>>   }
>>
>>   static void fimd_te_handler(struct exynos_drm_manager *mgr)
>> @@ -966,9 +982,9 @@ static void fimd_te_handler(struct exynos_drm_manager *mgr)
>>   		return;
>>
>>   	 /*
>> -	 * Skips to trigger if in triggering state, because multiple triggering
>> -	 * requests can cause panel reset.
>> -	 */
>> +	  * Skips triggering if in triggering mode, because multiple triggering
>> +	  * requests can cause panel reset.
>> +	  */
>>   	if (atomic_read(&ctx->triggering))
>>   		return;
>>
>> @@ -1023,21 +1039,13 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>>   	if (ctx->pipe < 0 || !ctx->drm_dev)
>>   		goto out;
>>
>> -	if (ctx->i80_if) {
>> -		/* unset I80 frame done interrupt */
>> -		val = readl(ctx->regs + VIDINTCON0);
>> -		val &= ~(VIDINTCON0_INT_I80IFDONE | VIDINTCON0_INT_SYSMAINCON);
>> -		writel(val, ctx->regs + VIDINTCON0);
>> +	drm_handle_vblank(ctx->drm_dev, ctx->pipe);
>> +	exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>>
>> -		/* exit triggering mode */
>> +	if (ctx->i80_if) {
>> +		/* Exits triggering mode */
>>   		atomic_set(&ctx->triggering, 0);
>> -
>> -		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
>> -		exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>>   	} else {
>> -		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
>> -		exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>> -
>>   		/* set wait vsync event to zero and wake up queue. */
>>   		if (atomic_read(&ctx->wait_vsync_event)) {
>>   			atomic_set(&ctx->wait_vsync_event, 0);
>>
>
>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2014-11-16  0:53 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-01  6:19 [PATCH 0/7] drm/exynos: modify LCD I80 interface display YoungJun Cho
2014-10-01  6:19 ` [PATCH 1/7] drm/exynos: fimd: remove unnecessary waiting vblank routine YoungJun Cho
2014-11-13  9:00   ` Inki Dae
2014-10-01  6:19 ` [PATCH 2/7] drm/exynos: fimd: add fimd_channel_win() to clean up code YoungJun Cho
2014-11-13  9:05   ` Inki Dae
2014-10-01  6:19 ` [PATCH 3/7] drm/exynos: fimd: modify vclk calculation for I80 i/f YoungJun Cho
2014-11-13  9:12   ` Inki Dae
2014-11-13  9:54     ` YoungJun Cho
2014-11-14  1:25       ` Inki Dae
2014-10-01  6:19 ` [PATCH 4/7] drm/exynos: fimd: move handle vblank position in TE handler YoungJun Cho
2014-11-14  1:20   ` Inki Dae
2014-10-01  6:19 ` [PATCH 5/7] drm/exynos: fimd: modify I80 i/f interrupt relevant routine YoungJun Cho
2014-11-14  1:36   ` Inki Dae
2014-11-16  0:53     ` YoungJun Cho
2014-10-01  6:19 ` [PATCH 6/7] drm/exynos: dsi: move TE irq handler registration position YoungJun Cho
2014-11-14  1:49   ` Inki Dae
2014-11-14  2:08     ` YoungJun Cho
2014-10-01  6:19 ` [PATCH 7/7] drm/exynos: dsi: move DSIM_STATE_ENABLED set position YoungJun Cho
2014-11-14  1:53   ` Inki Dae

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.