-----Original Message----- From: djkurtz@google.com [mailto:djkurtz@google.com] 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@samsung.com wrote:
Hi Daniel,
Please use only text format. :)
From: djkurtz@google.com [mailto:djkurtz@google.com] 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@samsung.com wrote: From: Akshu Agrawal akshu.a@samsung.com
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@samsung.com Signed-off-by: Prathyush K prathyush.k@samsung.com Signed-off-by: Inki Dae inki.dae@samsung.com
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 ifenabled.
*/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.
It seems more clear. I tried to take care of patch already posted so I didn't consider a 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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel