> -----Original Message----- > From: djkurtz@xxxxxxxxxx [mailto:djkurtz@xxxxxxxxxx] On Behalf Of Daniel > Kurtz > Sent: Wednesday, April 30, 2014 3:21 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 Wed, Apr 30, 2014 at 1:44 PM, Inki Dae <inki.dae@xxxxxxxxxxx> wrote: > > 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. > > Right. So, I suggest moving drm_iommu_attach_device() from mgr_initialize > to fimd_probe(), after clearing the windows and waiting for vblank. > Again, Good idea but it doesn't consider drm_device object that it needs to check if iommu is supported or not - if iommu is not supported, drm_iommu_attach_device() shouldn't be called. At probe(), we couldn't get drm_device object. Anyway, there may be another better way. Thanks, Inki Dae > > > > 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
