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(-)
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)
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)
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);
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);
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);
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);
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);
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);
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 = {
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 = {
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);
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_ENABLE;val &= ~VIDINTCON0_INT_FRAME;
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) {
atomic_set(&ctx->triggering, 0);/* Exits triggering mode */
drm_handle_vblank(ctx->drm_dev, ctx->pipe);
} else {exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
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);
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_ENABLE;val &= ~VIDINTCON0_INT_FRAME;
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) {
atomic_set(&ctx->triggering, 0);/* Exits triggering mode */
drm_handle_vblank(ctx->drm_dev, ctx->pipe);
} else {exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
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);
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; }
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;
}
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; }
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; }
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) {
exynos_dsi_set_display_enable(dsi, false); drm_panel_unprepare(dsi->panel); exynos_dsi_poweroff(dsi); return ret; }dsi->state &= ~DSIM_STATE_ENABLED;
- dsi->state |= DSIM_STATE_ENABLED;
- return 0;
}
dri-devel@lists.freedesktop.org