Hi Daniel, Please use only text format. :) >From: djkurtz@xxxxxxxxxx [mailto:djkurtz@xxxxxxxxxx] On Behalf Of Daniel Kurtz >Sent: Wednesday, April 30, 2014 1:57 PM >To: Inki Dae >Cc: David Airlie; dri-devel; Akshu Agrawal; Prathyush K >Subject: Re: [PATCH v2] drm/exynos: fimd: clear channel before enabling iommu > > > >On Tue, Apr 29, 2014 at 9:38 AM, Inki Dae <inki.dae@xxxxxxxxxxx> wrote: >From: Akshu Agrawal <akshu.a@xxxxxxxxxxx> > >If any fimd channel was already active, initializing iommu will result >in a PAGE FAULT (e.e. u-boot could have turned on the display and >not disabled it before the kernel starts). This patch checks if any >channel is active before initializing iommu and disables it. > >Changelog v2: >- consider SoC without SHADOWCON register > >Signed-off-by: Akshu Agrawal <akshu.a@xxxxxxxxxxx> >Signed-off-by: Prathyush K <prathyush.k@xxxxxxxxxxx> >Signed-off-by: Inki Dae <inki.dae@xxxxxxxxxxx> >--- > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 70 +++++++++++++++++++++--------- > 1 file changed, 50 insertions(+), 20 deletions(-) > >diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >index 40fd6cc..ef21ce4 100644 >--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >@@ -143,6 +143,48 @@ static inline struct fimd_driver_data *drm_fimd_get_driver_data( > return (struct fimd_driver_data *)of_id->data; > } > >+static void fimd_wait_for_vblank(struct exynos_drm_manager *mgr) >+{ >+ struct fimd_context *ctx = mgr->ctx; >+ >+ if (ctx->suspended) >+ return; >+ > >Hi Inki, > >I think you need this to guarantee that the irq is actually enabled: > Right, it needs to make sure that the irq is enabled. > drm_vblank_get(ctx->drm_dev, ctx->pipe); > >+ atomic_set(&ctx->wait_vsync_event, 1); >+ >+ /* >+ * wait for FIMD to signal VSYNC interrupt or return after >+ * timeout which is set to 50ms (refresh rate of 20). >+ */ >+ if (!wait_event_timeout(ctx->wait_vsync_queue, >+ !atomic_read(&ctx->wait_vsync_event), >+ HZ/20)) >+ DRM_DEBUG_KMS("vblank wait timed out.\n"); > > drm_vblank_put(ctx->drm_dev, ctx->pipe); > >+} >+ >+ >+static void fimd_clear_channel(struct exynos_drm_manager *mgr) >+{ >+ struct fimd_context *ctx = mgr->ctx; >+ int win, ch_enabled = 0; >+ >+ DRM_DEBUG_KMS("%s\n", __FILE__); >+ >+ /* Check if any channel is enabled. */ >+ for (win = 0; win < WINDOWS_NR; win++) { >+ u32 val = readl(ctx->regs + SHADOWCON); >+ if (val & SHADOWCON_CHx_ENABLE(win)) { >+ val &= ~SHADOWCON_CHx_ENABLE(win); >+ writel(val, ctx->regs + SHADOWCON); >+ ch_enabled = 1; >+ } >+ } >+ >+ /* Wait for vsync, as disable channel takes effect at next vsync */ >+ if (ch_enabled) >+ fimd_wait_for_vblank(mgr); >+} >+ > static int fimd_mgr_initialize(struct exynos_drm_manager *mgr, > struct drm_device *drm_dev, int pipe) > { >@@ -169,8 +211,15 @@ static int fimd_mgr_initialize(struct exynos_drm_manager *mgr, > drm_dev->vblank_disable_allowed = true; > > /* attach this sub driver to iommu mapping if supported. */ >- if (is_drm_iommu_supported(ctx->drm_dev)) >+ if (is_drm_iommu_supported(ctx->drm_dev)) { >+ /* >+ * If any channel is already active, iommu will throw >+ * a PAGE FAULT when enabled. So clear any channel if enabled. >+ */ >+ if (ctx->driver_data->has_shadowcon) >+ fimd_clear_channel(mgr); > >We already disable all overlays at the end of fimd_probe() by calling fimd_clear_win(). >Perhaps all that was missing was just waiting until the next vblank to ensure that these clears have all completed? > No, fimd_mgr_initialize() will be called at load() so the iommu unit for fimd ip will be tried to be enabled prior to fimd_clear_channel() call - before all dma channels are disabled. In this case, page fault could be incurred if fimd ip was already on by bootloader. Thanks, Inki Dae >Best Regards, >-Daniel > > drm_iommu_attach_device(ctx->drm_dev, ctx->dev); >+ } > > return 0; > } >@@ -324,25 +373,6 @@ static void fimd_disable_vblank(struct exynos_drm_manager *mgr) > } > } > >-static void fimd_wait_for_vblank(struct exynos_drm_manager *mgr) >-{ >- struct fimd_context *ctx = mgr->ctx; >- >- if (ctx->suspended) >- return; >- >- atomic_set(&ctx->wait_vsync_event, 1); >- >- /* >- * wait for FIMD to signal VSYNC interrupt or return after >- * timeout which is set to 50ms (refresh rate of 20). >- */ >- if (!wait_event_timeout(ctx->wait_vsync_queue, >- !atomic_read(&ctx->wait_vsync_event), >- HZ/20)) >- DRM_DEBUG_KMS("vblank wait timed out.\n"); >-} >- > static void fimd_win_mode_set(struct exynos_drm_manager *mgr, > struct exynos_drm_overlay *overlay) > { >-- >1.7.9.5 > >_______________________________________________ >dri-devel mailing list >dri-devel@xxxxxxxxxxxxxxxxxxxxx >http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
