Fixes for various issues which are to Power On/Off sequence, Layer update, waiting for vblank in exynos mixer driver.
v2: 1) Repalce mixer_enable_vblank with drm_vblank_get.
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)
On 2014년 06월 23일 14:32, Rahul Sharma wrote:
Rahul, it looks good to me and ok as is. But above codes don't consider Exynos4 series. In case of Exynos4xxx SoC, MXR_CFG_LAYER_UPDATE_COUNT_MASK and MXR_CFG_LAYER_UPDATE of MIXER_CFG register are reserved fields. So can you work that patch to be considered for Exynos4xxx SoC? That patch would be additional one.
Anyway, will apply it as is, and I will wait for the additional patch.
Thanks, Inki Dae
}
static void mixer_graph_buffer(struct mixer_context *ctx, int win)
Am 24.06.2014 07:21, schrieb Inki Dae:
If it's not too late, could you fix up "multiple" in the subject? :)
Cheers, Andreas
Thanks Inki,
One more thing. mixer_layer_update is only called on for mixer version; MXR_VER_16_0_33_0, MXR_VER_128_0_0_184. This condition should have taken care of Exynos4 scenarios. What you say?
Regards, Rahul Sharma.
On 24 June 2014 20:20, Inki Dae inki.dae@samsung.com wrote:
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..7529946 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);
+ drm_vblank_get(mgr->crtc->dev, mixer_ctx->pipe); + 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"); + + drm_vblank_put(mgr->crtc->dev, mixer_ctx->pipe); }
static void mixer_window_suspend(struct exynos_drm_manager *mgr)
dri-devel@lists.freedesktop.org
-
Andreas Färber -
Inki Dae -
Rahul Sharma