Hello,
On 2015-06-11 17:04, Inki Dae wrote:
On 2015년 06월 03일 17:26, Marek Szyprowski wrote:
One should not do any assumptions on the stare of the fimd hardware
during driver initialization, so to properly reset fimd before enabling
IOMMU, one should ensure that all power domains and clocks are really
enabled. This patch adds pm_runtime and clocks management in the
fimd_clear_channel() function to ensure that any access to fimd
registers will be performed with clocks and power domains enabled.
Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
Tested-by: Javier Martinez Canillas <javier.martinez@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 96618534358e..3ec9d4299a86 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -242,12 +242,21 @@ static void fimd_enable_shadow_channel_path(struct fimd_context *ctx,
writel(val, ctx->regs + SHADOWCON);
}
+static int fimd_enable_vblank(struct exynos_drm_crtc *crtc);
+static void fimd_disable_vblank(struct exynos_drm_crtc *crtc);
You can remove abvoe declarations. See the below comment.
+
static void fimd_clear_channel(struct fimd_context *ctx)
{
unsigned int win, ch_enabled = 0;
DRM_DEBUG_KMS("%s\n", __FILE__);
+ /* Hardware is in unknown state, so ensure it gets enabled properly */
+ pm_runtime_get_sync(ctx->dev);
+
+ clk_prepare_enable(ctx->bus_clk);
+ clk_prepare_enable(ctx->lcd_clk);
+
/* Check if any channel is enabled. */
for (win = 0; win < WINDOWS_NR; win++) {
u32 val = readl(ctx->regs + WINCON(win));
@@ -265,12 +274,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
/* Wait for vsync, as disable channel takes effect at next vsync */
if (ch_enabled) {
- unsigned int state = ctx->suspended;
-
- ctx->suspended = 0;
+ ctx->suspended = false;
+ fimd_enable_vblank(ctx->crtc);
I think you can call enable_vblank callback instead of
fimd_enable_vblank function because ctx object has exynos_crtc object.
i.e.,
struct exynos_drm_crtc_ops *ops = ctx->crtc->ops;
...
if (ops->enable_vblank)
ops->enable_vblank(ctx->crtc);
...
Well, I don't like such indirect calls to known functions, but if you prefer
this approach I will send an updated patch in a minute. There is also
alternative
way of getting rid of forward declarations. Code of fimd_enable_vblank and
fimd_disable_vblank can be moved closer to fimd_wait_vblank function.
I will also send such alternative patch. Feel free to select the one
that better
fits your preferences.
fimd_wait_for_vblank(ctx->crtc);
- ctx->suspended = state;
+ fimd_disable_vblank(ctx->crtc);
Ditto.
Thanks,
Inki Dae
+ ctx->suspended = true;
}
+
+ clk_disable_unprepare(ctx->lcd_clk);
+ clk_disable_unprepare(ctx->bus_clk);
+
+ pm_runtime_put(ctx->dev);
}
static int fimd_iommu_attach_devices(struct fimd_context *ctx,
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
