| Message ID | 1356521265-22749-5-git-send-email-prathyush.k@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2012/12/26 Prathyush K <prathyush.k@samsung.com> > From: Akshu Agrawal <akshu.a@samsung.com> > > If any fimd channel was already active, initializing iommu will result > in a PAGE FAULT (e.g. 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. > > Signed-off-by: Akshu Agrawal <akshu.a@samsung.com> > Signed-off-by: Prathyush K <prathyush.k@samsung.com> > --- > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 31 > +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index f88eaa4..3aeedf5 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -733,6 +733,28 @@ out: > return IRQ_HANDLED; > } > > +static void fimd_clear_channel(struct device *dev) > +{ > + struct fimd_context *ctx = get_fimd_context(dev); > + 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(dev); > +} > + > static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device > *dev) > { > DRM_DEBUG_KMS("%s\n", __FILE__); > @@ -755,9 +777,14 @@ static int fimd_subdrv_probe(struct drm_device > *drm_dev, struct device *dev) > drm_dev->vblank_disable_allowed = 1; > > /* attach this sub driver to iommu mapping if supported. */ > - if (is_drm_iommu_supported(drm_dev)) > + if (is_drm_iommu_supported(drm_dev)) { > + /* > + * If any channel is already active, iommu will throw > + * a PAGE FAULT when enabled. So clear any channel if > enabled. > + */ > + fimd_clear_channel(dev); > Right, we had this issue that when booted with iommu, page fault occurs. But the reason we didn't disable all dma channel is that this makes the screen to be blinked. Actually, we are using some codes internally to avoid the blinking issue but this code is not generic(enabling iommu at vsync). Anyway, this patch should be applied for now. But let's find a more generic way to avoid that issue later. > drm_iommu_attach_device(drm_dev, dev); > - > + } > return 0; > } > > -- > 1.8.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
On Wed, Dec 26, 2012 at 3:27 AM, Prathyush K <prathyush.k@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.g. 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. > Will that work if another drm exynos platform driver (let's say hdmi) starts before fimd and enables the iommu? Stéphane > Signed-off-by: Akshu Agrawal <akshu.a@samsung.com> > Signed-off-by: Prathyush K <prathyush.k@samsung.com> > --- > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index f88eaa4..3aeedf5 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -733,6 +733,28 @@ out: > return IRQ_HANDLED; > } > > +static void fimd_clear_channel(struct device *dev) > +{ > + struct fimd_context *ctx = get_fimd_context(dev); > + 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(dev); > +} > + > static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev) > { > DRM_DEBUG_KMS("%s\n", __FILE__); > @@ -755,9 +777,14 @@ static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev) > drm_dev->vblank_disable_allowed = 1; > > /* attach this sub driver to iommu mapping if supported. */ > - if (is_drm_iommu_supported(drm_dev)) > + if (is_drm_iommu_supported(drm_dev)) { > + /* > + * If any channel is already active, iommu will throw > + * a PAGE FAULT when enabled. So clear any channel if enabled. > + */ > + fimd_clear_channel(dev); > drm_iommu_attach_device(drm_dev, dev); > - > + } > return 0; > } > > -- > 1.8.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jan 3, 2013 at 2:33 AM, Stéphane Marchesin < stephane.marchesin@gmail.com> wrote: > On Wed, Dec 26, 2012 at 3:27 AM, Prathyush K <prathyush.k@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.g. 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. > > > > Will that work if another drm exynos platform driver (let's say hdmi) > starts before fimd and enables the iommu? > > Stéphane > > How can hdmi enable fimd's iommu? iommu initialization has two parts in drm. A common mapping gets created first. This mapping is common to all devices of drm - fimd, mixer, exynos-drm etc. Then each device - calls attach_device to turn on its own iommu. so even if hdmi starts before fimd, hdmi will turn on its own iommu and will not affect fimd. Hope that clears your doubt. Regards, Prathyush > > Signed-off-by: Akshu Agrawal <akshu.a@samsung.com> > > Signed-off-by: Prathyush K <prathyush.k@samsung.com> > > --- > > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 31 > +++++++++++++++++++++++++++++-- > > 1 file changed, 29 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > > index f88eaa4..3aeedf5 100644 > > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > > @@ -733,6 +733,28 @@ out: > > return IRQ_HANDLED; > > } > > > > +static void fimd_clear_channel(struct device *dev) > > +{ > > + struct fimd_context *ctx = get_fimd_context(dev); > > + 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(dev); > > +} > > + > > static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device > *dev) > > { > > DRM_DEBUG_KMS("%s\n", __FILE__); > > @@ -755,9 +777,14 @@ static int fimd_subdrv_probe(struct drm_device > *drm_dev, struct device *dev) > > drm_dev->vblank_disable_allowed = 1; > > > > /* attach this sub driver to iommu mapping if supported. */ > > - if (is_drm_iommu_supported(drm_dev)) > > + if (is_drm_iommu_supported(drm_dev)) { > > + /* > > + * If any channel is already active, iommu will throw > > + * a PAGE FAULT when enabled. So clear any channel if > enabled. > > + */ > > + fimd_clear_channel(dev); > > drm_iommu_attach_device(drm_dev, dev); > > - > > + } > > return 0; > > } > > > > -- > > 1.8.0 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index f88eaa4..3aeedf5 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -733,6 +733,28 @@ out: return IRQ_HANDLED; } +static void fimd_clear_channel(struct device *dev) +{ + struct fimd_context *ctx = get_fimd_context(dev); + 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(dev); +} + static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev) { DRM_DEBUG_KMS("%s\n", __FILE__); @@ -755,9 +777,14 @@ static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev) drm_dev->vblank_disable_allowed = 1; /* attach this sub driver to iommu mapping if supported. */ - if (is_drm_iommu_supported(drm_dev)) + if (is_drm_iommu_supported(drm_dev)) { + /* + * If any channel is already active, iommu will throw + * a PAGE FAULT when enabled. So clear any channel if enabled. + */ + fimd_clear_channel(dev); drm_iommu_attach_device(drm_dev, dev); - + } return 0; }