Fixes for various issues which are to Power On/Off sequence, Layer update, waiting for vblank in exynos mixer driver.
This series is based on exynos-drm-fixes branch in Inki dae's tree.
Rahul Sharma (5): drm/exynos: set power state variable after enabling clocks and power drm/exynos: stop mixer before gating clocks during poweroff drm/exynos: allow mulitple layer updates per vsync for mixer drm/exynos: soft reset mixer before reconfigure after power-on drm/exynos: enable vsync interrupt while waiting for vblank
drivers/gpu/drm/exynos/exynos_mixer.c | 50 +++++++++++++++++++++++---------- drivers/gpu/drm/exynos/regs-mixer.h | 1 + 2 files changed, 36 insertions(+), 15 deletions(-)
Power state variable holds the state of the mixer device. Power on and power off functions are toggling these variable at wrong place.
State variable should be changed to true only after Runtime PM and clocks are enabled. Else it may result to a situation where mixer registers are accessed with device power enabled. Similar logic for poweroff sequence.
Signed-off-by: Rahul Sharma rahul.sharma@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 4c5aed7..c00abbe 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1061,7 +1061,7 @@ static void mixer_poweron(struct exynos_drm_manager *mgr) mutex_unlock(&ctx->mixer_mutex); return; } - ctx->powered = true; + mutex_unlock(&ctx->mixer_mutex);
pm_runtime_get_sync(ctx->dev); @@ -1072,6 +1072,10 @@ static void mixer_poweron(struct exynos_drm_manager *mgr) clk_prepare_enable(res->sclk_mixer); }
+ mutex_lock(&ctx->mixer_mutex); + ctx->powered = true; + mutex_unlock(&ctx->mixer_mutex); + mixer_reg_write(res, MXR_INT_EN, ctx->int_en); mixer_win_reset(ctx);
@@ -1084,14 +1088,20 @@ static void mixer_poweroff(struct exynos_drm_manager *mgr) struct mixer_resources *res = &ctx->mixer_res;
mutex_lock(&ctx->mixer_mutex); - if (!ctx->powered) - goto out; + if (!ctx->powered) { + mutex_unlock(&ctx->mixer_mutex); + return; + } mutex_unlock(&ctx->mixer_mutex);
mixer_window_suspend(mgr);
ctx->int_en = mixer_reg_read(res, MXR_INT_EN);
+ mutex_lock(&ctx->mixer_mutex); + ctx->powered = false; + mutex_unlock(&ctx->mixer_mutex); + clk_disable_unprepare(res->mixer); if (ctx->vp_enabled) { clk_disable_unprepare(res->vp); @@ -1099,12 +1109,6 @@ static void mixer_poweroff(struct exynos_drm_manager *mgr) }
pm_runtime_put_sync(ctx->dev); - - mutex_lock(&ctx->mixer_mutex); - ctx->powered = false; - -out: - mutex_unlock(&ctx->mixer_mutex); }
static void mixer_dpms(struct exynos_drm_manager *mgr, int mode)
Mixer should be power gated only after it is gracefully stopped. The recommended sequence is to Stop the mixer and wait till it enters to IDLE state before gating the clocks and power to the mixer.
Signed-off-by: Rahul Sharma rahul.sharma@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 15 +++++++++++++++ drivers/gpu/drm/exynos/regs-mixer.h | 1 + 2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index c00abbe..d359501 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -377,6 +377,20 @@ static void mixer_run(struct mixer_context *ctx) mixer_regs_dump(ctx); }
+static void mixer_stop(struct mixer_context *ctx) +{ + struct mixer_resources *res = &ctx->mixer_res; + int timeout = 20; + + mixer_reg_writemask(res, MXR_STATUS, 0, MXR_STATUS_REG_RUN); + + while (!(mixer_reg_read(res, MXR_STATUS) & MXR_STATUS_REG_IDLE) && + --timeout) + usleep_range(10000, 12000); + + mixer_regs_dump(ctx); +} + static void vp_video_buffer(struct mixer_context *ctx, int win) { struct mixer_resources *res = &ctx->mixer_res; @@ -1094,6 +1108,7 @@ static void mixer_poweroff(struct exynos_drm_manager *mgr) } mutex_unlock(&ctx->mixer_mutex);
+ mixer_stop(ctx); mixer_window_suspend(mgr);
ctx->int_en = mixer_reg_read(res, MXR_INT_EN); diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h index 4537026..5f32e1a 100644 --- a/drivers/gpu/drm/exynos/regs-mixer.h +++ b/drivers/gpu/drm/exynos/regs-mixer.h @@ -78,6 +78,7 @@ #define MXR_STATUS_BIG_ENDIAN (1 << 3) #define MXR_STATUS_ENDIAN_MASK (1 << 3) #define MXR_STATUS_SYNC_ENABLE (1 << 2) +#define MXR_STATUS_REG_IDLE (1 << 1) #define MXR_STATUS_REG_RUN (1 << 0)
/* bits for MXR_CFG */
Allowing only one layer update per vsync can cause issues while there are update available for both layers. There is a good amount of possibility to loose updates if we allow single update per vsync.
Signed-off-by: Rahul Sharma rahul.sharma@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index d359501..6773b03 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -511,13 +511,8 @@ static void vp_video_buffer(struct mixer_context *ctx, int win) static void mixer_layer_update(struct mixer_context *ctx) { struct mixer_resources *res = &ctx->mixer_res; - u32 val; - - val = mixer_reg_read(res, MXR_CFG);
- /* allow one update per vsync only */ - if (!(val & MXR_CFG_LAYER_UPDATE_COUNT_MASK)) - mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE); + mixer_reg_writemask(res, MXR_CFG, ~0, MXR_CFG_LAYER_UPDATE); }
static void mixer_graph_buffer(struct mixer_context *ctx, int win)
Mixer soft reset is a recommended step before reconfiguring the mixer after power on. Mixer looses the previous state of DMAs if soft reset. This is the recommendation from the hardware team.
Signed-off-by: Rahul Sharma rahul.sharma@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 6773b03..6f18581 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1085,6 +1085,8 @@ static void mixer_poweron(struct exynos_drm_manager *mgr) ctx->powered = true; mutex_unlock(&ctx->mixer_mutex);
+ mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET); + mixer_reg_write(res, MXR_INT_EN, ctx->int_en); mixer_win_reset(ctx);
mixer_wait_for_vblank function expects that the upcoming vsync interrupt handler routine will clear the wait_vsync_event atomic variable.
For this to happen, interrupts should be enabled and disabled properly.
Signed-off-by: Rahul Sharma rahul.sharma@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 6f18581..7927f2e 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1019,6 +1019,8 @@ static void mixer_wait_for_vblank(struct exynos_drm_manager *mgr) } mutex_unlock(&mixer_ctx->mixer_mutex);
+ mixer_enable_vblank(mgr); + atomic_set(&mixer_ctx->wait_vsync_event, 1);
/* @@ -1029,6 +1031,8 @@ static void mixer_wait_for_vblank(struct exynos_drm_manager *mgr) !atomic_read(&mixer_ctx->wait_vsync_event), HZ/20)) DRM_DEBUG_KMS("vblank wait timed out.\n"); + + mixer_disable_vblank(mgr); }
static void mixer_window_suspend(struct exynos_drm_manager *mgr)
Hi All,
On 18 June 2014 17:04, Rahul Sharma rahul.sharma@samsung.com wrote:
mixer_wait_for_vblank function expects that the upcoming vsync interrupt handler routine will clear the wait_vsync_event atomic variable.
For this to happen, interrupts should be enabled and disabled properly.
Signed-off-by: Rahul Sharma rahul.sharma@samsung.com
drivers/gpu/drm/exynos/exynos_mixer.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 6f18581..7927f2e 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1019,6 +1019,8 @@ static void mixer_wait_for_vblank(struct exynos_drm_manager *mgr) } mutex_unlock(&mixer_ctx->mixer_mutex);
mixer_enable_vblank(mgr);atomic_set(&mixer_ctx->wait_vsync_event, 1); /*@@ -1029,6 +1031,8 @@ static void mixer_wait_for_vblank(struct exynos_drm_manager *mgr) !atomic_read(&mixer_ctx->wait_vsync_event), HZ/20)) DRM_DEBUG_KMS("vblank wait timed out.\n");
mixer_disable_vblank(mgr);}
I found issue with this patch. It is causing deadlock by bypassing vblank refcounting and manually disabling the Interrupt.
I switched to following and it is back on track.
drm_vblank_get(mgr->crtc->dev, mixer_ctx->pipe); drm_vblank_put(mgr->crtc->dev, mixer_ctx->pipe);
I will include this change in next version.
Regards, Rahul Sharma.
static void mixer_window_suspend(struct exynos_drm_manager *mgr)
1.7.9.5
dri-devel@lists.freedesktop.org
-
Rahul Sharma