[PATCH] drm/exynos: Fix cleanup of IOMMU related objects

[PATCH] drm/exynos: Fix cleanup of IOMMU related objects

[Marek Szyprowski]

Store the IOMMU mapping created by device core of each Exynos DRM
sub-device and restore it when Exynos DRM driver is unbound. This fixes
IOMMU initialization failure for the second time when deferred probe is
triggered from the bind() callback of master's compound DRM driver.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  5 +++--
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  5 +++--
 drivers/gpu/drm/exynos/exynos_drm_dma.c       | 22 +++++++++++--------
 drivers/gpu/drm/exynos/exynos_drm_drv.h       |  6 +++--
 drivers/gpu/drm/exynos/exynos_drm_fimc.c      |  5 +++--
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  5 +++--
 drivers/gpu/drm/exynos/exynos_drm_g2d.c       |  5 +++--
 drivers/gpu/drm/exynos/exynos_drm_gsc.c       |  5 +++--
 drivers/gpu/drm/exynos/exynos_drm_rotator.c   |  5 +++--
 drivers/gpu/drm/exynos/exynos_drm_scaler.c    |  6 +++--
 drivers/gpu/drm/exynos/exynos_mixer.c         |  7 ++++--
 11 files changed, 47 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 8428ae12dfa5..1f79bc2a881e 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -55,6 +55,7 @@ static const char * const decon_clks_name[] = {
 struct decon_context {
 	struct device			*dev;
 	struct drm_device		*drm_dev;
+	void				*dma_priv;
 	struct exynos_drm_crtc		*crtc;
 	struct exynos_drm_plane		planes[WINDOWS_NR];
 	struct exynos_drm_plane_config	configs[WINDOWS_NR];
@@ -644,7 +645,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
 
 	decon_clear_channels(ctx->crtc);
 
-	return exynos_drm_register_dma(drm_dev, dev);
+	return exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
 }
 
 static void decon_unbind(struct device *dev, struct device *master, void *data)
@@ -654,7 +655,7 @@ static void decon_unbind(struct device *dev, struct device *master, void *data)
 	decon_atomic_disable(ctx->crtc);
 
 	/* detach this sub driver from iommu mapping if supported. */
-	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
+	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
 }
 
 static const struct component_ops decon_component_ops = {
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index ff59c641fa80..1eed3327999f 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -40,6 +40,7 @@
 struct decon_context {
 	struct device			*dev;
 	struct drm_device		*drm_dev;
+	void				*dma_priv;
 	struct exynos_drm_crtc		*crtc;
 	struct exynos_drm_plane		planes[WINDOWS_NR];
 	struct exynos_drm_plane_config	configs[WINDOWS_NR];
@@ -127,13 +128,13 @@ static int decon_ctx_initialize(struct decon_context *ctx,
 
 	decon_clear_channels(ctx->crtc);
 
-	return exynos_drm_register_dma(drm_dev, ctx->dev);
+	return exynos_drm_register_dma(drm_dev, ctx->dev, &ctx->dma_priv);
 }
 
 static void decon_ctx_remove(struct decon_context *ctx)
 {
 	/* detach this sub driver from iommu mapping if supported. */
-	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
+	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
 }
 
 static u32 decon_calc_clkdiv(struct decon_context *ctx,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c
index 9ebc02768847..482bec7756fa 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
@@ -58,7 +58,7 @@ static inline void clear_dma_max_seg_size(struct device *dev)
  * mapping.
  */
 static int drm_iommu_attach_device(struct drm_device *drm_dev,
-				struct device *subdrv_dev)
+				struct device *subdrv_dev, void **dma_priv)
 {
 	struct exynos_drm_private *priv = drm_dev->dev_private;
 	int ret;
@@ -74,7 +74,8 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
 		return ret;
 
 	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
-		if (to_dma_iommu_mapping(subdrv_dev))
+		*dma_priv = to_dma_iommu_mapping(subdrv_dev);
+		if (*dma_priv)
 			arm_iommu_detach_device(subdrv_dev);
 
 		ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
@@ -98,19 +99,21 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
  * mapping
  */
 static void drm_iommu_detach_device(struct drm_device *drm_dev,
-				struct device *subdrv_dev)
+				    struct device *subdrv_dev, void **dma_priv)
 {
 	struct exynos_drm_private *priv = drm_dev->dev_private;
 
-	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU))
+	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
 		arm_iommu_detach_device(subdrv_dev);
-	else if (IS_ENABLED(CONFIG_IOMMU_DMA))
+		arm_iommu_attach_device(subdrv_dev, *dma_priv);
+	} else if (IS_ENABLED(CONFIG_IOMMU_DMA))
 		iommu_detach_device(priv->mapping, subdrv_dev);
 
 	clear_dma_max_seg_size(subdrv_dev);
 }
 
-int exynos_drm_register_dma(struct drm_device *drm, struct device *dev)
+int exynos_drm_register_dma(struct drm_device *drm, struct device *dev,
+			    void **dma_priv)
 {
 	struct exynos_drm_private *priv = drm->dev_private;
 
@@ -137,13 +140,14 @@ int exynos_drm_register_dma(struct drm_device *drm, struct device *dev)
 		priv->mapping = mapping;
 	}
 
-	return drm_iommu_attach_device(drm, dev);
+	return drm_iommu_attach_device(drm, dev, dma_priv);
 }
 
-void exynos_drm_unregister_dma(struct drm_device *drm, struct device *dev)
+void exynos_drm_unregister_dma(struct drm_device *drm, struct device *dev,
+			       void **dma_priv)
 {
 	if (IS_ENABLED(CONFIG_EXYNOS_IOMMU))
-		drm_iommu_detach_device(drm, dev);
+		drm_iommu_detach_device(drm, dev, dma_priv);
 }
 
 void exynos_drm_cleanup_dma(struct drm_device *drm)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index d4d21d8cfb90..6ae9056e7a18 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -223,8 +223,10 @@ static inline bool is_drm_iommu_supported(struct drm_device *drm_dev)
 	return priv->mapping ? true : false;
 }
 
-int exynos_drm_register_dma(struct drm_device *drm, struct device *dev);
-void exynos_drm_unregister_dma(struct drm_device *drm, struct device *dev);
+int exynos_drm_register_dma(struct drm_device *drm, struct device *dev,
+			    void **dma_priv);
+void exynos_drm_unregister_dma(struct drm_device *drm, struct device *dev,
+			       void **dma_priv);
 void exynos_drm_cleanup_dma(struct drm_device *drm);
 
 #ifdef CONFIG_DRM_EXYNOS_DPI
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 8ea2e1d77802..29ab8be8604c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -97,6 +97,7 @@ struct fimc_scaler {
 struct fimc_context {
 	struct exynos_drm_ipp ipp;
 	struct drm_device *drm_dev;
+	void		*dma_priv;
 	struct device	*dev;
 	struct exynos_drm_ipp_task	*task;
 	struct exynos_drm_ipp_formats	*formats;
@@ -1133,7 +1134,7 @@ static int fimc_bind(struct device *dev, struct device *master, void *data)
 
 	ctx->drm_dev = drm_dev;
 	ipp->drm_dev = drm_dev;
-	exynos_drm_register_dma(drm_dev, dev);
+	exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
 
 	exynos_drm_ipp_register(dev, ipp, &ipp_funcs,
 			DRM_EXYNOS_IPP_CAP_CROP | DRM_EXYNOS_IPP_CAP_ROTATE |
@@ -1153,7 +1154,7 @@ static void fimc_unbind(struct device *dev, struct device *master,
 	struct exynos_drm_ipp *ipp = &ctx->ipp;
 
 	exynos_drm_ipp_unregister(dev, ipp);
-	exynos_drm_unregister_dma(drm_dev, dev);
+	exynos_drm_unregister_dma(drm_dev, dev, &ctx->dma_priv);
 }
 
 static const struct component_ops fimc_component_ops = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 21aec38702fc..bb67cad8371f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -167,6 +167,7 @@ static struct fimd_driver_data exynos5420_fimd_driver_data = {
 struct fimd_context {
 	struct device			*dev;
 	struct drm_device		*drm_dev;
+	void				*dma_priv;
 	struct exynos_drm_crtc		*crtc;
 	struct exynos_drm_plane		planes[WINDOWS_NR];
 	struct exynos_drm_plane_config	configs[WINDOWS_NR];
@@ -1090,7 +1091,7 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 	if (is_drm_iommu_supported(drm_dev))
 		fimd_clear_channels(ctx->crtc);
 
-	return exynos_drm_register_dma(drm_dev, dev);
+	return exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
 }
 
 static void fimd_unbind(struct device *dev, struct device *master,
@@ -1100,7 +1101,7 @@ static void fimd_unbind(struct device *dev, struct device *master,
 
 	fimd_atomic_disable(ctx->crtc);
 
-	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
+	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
 
 	if (ctx->encoder)
 		exynos_dpi_remove(ctx->encoder);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 2a3382d43bc9..fcee33a43aca 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -232,6 +232,7 @@ struct g2d_runqueue_node {
 
 struct g2d_data {
 	struct device			*dev;
+	void				*dma_priv;
 	struct clk			*gate_clk;
 	void __iomem			*regs;
 	int				irq;
@@ -1409,7 +1410,7 @@ static int g2d_bind(struct device *dev, struct device *master, void *data)
 		return ret;
 	}
 
-	ret = exynos_drm_register_dma(drm_dev, dev);
+	ret = exynos_drm_register_dma(drm_dev, dev, &g2d->dma_priv);
 	if (ret < 0) {
 		dev_err(dev, "failed to enable iommu.\n");
 		g2d_fini_cmdlist(g2d);
@@ -1434,7 +1435,7 @@ static void g2d_unbind(struct device *dev, struct device *master, void *data)
 	priv->g2d_dev = NULL;
 
 	cancel_work_sync(&g2d->runqueue_work);
-	exynos_drm_unregister_dma(g2d->drm_dev, dev);
+	exynos_drm_unregister_dma(g2d->drm_dev, dev, &g2d->dma_priv);
 }
 
 static const struct component_ops g2d_component_ops = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 88b6fcaa20be..45e9aee8366a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -97,6 +97,7 @@ struct gsc_scaler {
 struct gsc_context {
 	struct exynos_drm_ipp ipp;
 	struct drm_device *drm_dev;
+	void		*dma_priv;
 	struct device	*dev;
 	struct exynos_drm_ipp_task	*task;
 	struct exynos_drm_ipp_formats	*formats;
@@ -1169,7 +1170,7 @@ static int gsc_bind(struct device *dev, struct device *master, void *data)
 
 	ctx->drm_dev = drm_dev;
 	ctx->drm_dev = drm_dev;
-	exynos_drm_register_dma(drm_dev, dev);
+	exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
 
 	exynos_drm_ipp_register(dev, ipp, &ipp_funcs,
 			DRM_EXYNOS_IPP_CAP_CROP | DRM_EXYNOS_IPP_CAP_ROTATE |
@@ -1189,7 +1190,7 @@ static void gsc_unbind(struct device *dev, struct device *master,
 	struct exynos_drm_ipp *ipp = &ctx->ipp;
 
 	exynos_drm_ipp_unregister(dev, ipp);
-	exynos_drm_unregister_dma(drm_dev, dev);
+	exynos_drm_unregister_dma(drm_dev, dev, &ctx->dma_priv);
 }
 
 static const struct component_ops gsc_component_ops = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
index b98482990d1a..dafa87b82052 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
@@ -56,6 +56,7 @@ struct rot_variant {
 struct rot_context {
 	struct exynos_drm_ipp ipp;
 	struct drm_device *drm_dev;
+	void		*dma_priv;
 	struct device	*dev;
 	void __iomem	*regs;
 	struct clk	*clock;
@@ -243,7 +244,7 @@ static int rotator_bind(struct device *dev, struct device *master, void *data)
 
 	rot->drm_dev = drm_dev;
 	ipp->drm_dev = drm_dev;
-	exynos_drm_register_dma(drm_dev, dev);
+	exynos_drm_register_dma(drm_dev, dev, &rot->dma_priv);
 
 	exynos_drm_ipp_register(dev, ipp, &ipp_funcs,
 			   DRM_EXYNOS_IPP_CAP_CROP | DRM_EXYNOS_IPP_CAP_ROTATE,
@@ -261,7 +262,7 @@ static void rotator_unbind(struct device *dev, struct device *master,
 	struct exynos_drm_ipp *ipp = &rot->ipp;
 
 	exynos_drm_ipp_unregister(dev, ipp);
-	exynos_drm_unregister_dma(rot->drm_dev, rot->dev);
+	exynos_drm_unregister_dma(rot->drm_dev, rot->dev, &rot->dma_priv);
 }
 
 static const struct component_ops rotator_component_ops = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_scaler.c b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
index 497973e9b2c5..93c43c8d914e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_scaler.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
@@ -39,6 +39,7 @@ struct scaler_data {
 struct scaler_context {
 	struct exynos_drm_ipp		ipp;
 	struct drm_device		*drm_dev;
+	void				*dma_priv;
 	struct device			*dev;
 	void __iomem			*regs;
 	struct clk			*clock[SCALER_MAX_CLK];
@@ -450,7 +451,7 @@ static int scaler_bind(struct device *dev, struct device *master, void *data)
 
 	scaler->drm_dev = drm_dev;
 	ipp->drm_dev = drm_dev;
-	exynos_drm_register_dma(drm_dev, dev);
+	exynos_drm_register_dma(drm_dev, dev, &scaler->dma_priv);
 
 	exynos_drm_ipp_register(dev, ipp, &ipp_funcs,
 			DRM_EXYNOS_IPP_CAP_CROP | DRM_EXYNOS_IPP_CAP_ROTATE |
@@ -470,7 +471,8 @@ static void scaler_unbind(struct device *dev, struct device *master,
 	struct exynos_drm_ipp *ipp = &scaler->ipp;
 
 	exynos_drm_ipp_unregister(dev, ipp);
-	exynos_drm_unregister_dma(scaler->drm_dev, scaler->dev);
+	exynos_drm_unregister_dma(scaler->drm_dev, scaler->dev,
+				  &scaler->dma_priv);
 }
 
 static const struct component_ops scaler_component_ops = {
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 38ae9c32feef..21b726baedea 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -94,6 +94,7 @@ struct mixer_context {
 	struct platform_device *pdev;
 	struct device		*dev;
 	struct drm_device	*drm_dev;
+	void			*dma_priv;
 	struct exynos_drm_crtc	*crtc;
 	struct exynos_drm_plane	planes[MIXER_WIN_NR];
 	unsigned long		flags;
@@ -894,12 +895,14 @@ static int mixer_initialize(struct mixer_context *mixer_ctx,
 		}
 	}
 
-	return exynos_drm_register_dma(drm_dev, mixer_ctx->dev);
+	return exynos_drm_register_dma(drm_dev, mixer_ctx->dev,
+				       &mixer_ctx->dma_priv);
 }
 
 static void mixer_ctx_remove(struct mixer_context *mixer_ctx)
 {
-	exynos_drm_unregister_dma(mixer_ctx->drm_dev, mixer_ctx->dev);
+	exynos_drm_unregister_dma(mixer_ctx->drm_dev, mixer_ctx->dev,
+				  &mixer_ctx->dma_priv);
 }
 
 static int mixer_enable_vblank(struct exynos_drm_crtc *crtc)
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/exynos: Fix cleanup of IOMMU related objects

[Inki Dae]

Hi Marek,

20. 3. 2. 오후 11:27에 Marek Szyprowski 이(가) 쓴 글:
> Store the IOMMU mapping created by device core of each Exynos DRM
> sub-device and restore it when Exynos DRM driver is unbound. This fixes
> IOMMU initialization failure for the second time when deferred probe is
> triggered from the bind() callback of master's compound DRM driver.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  5 +++--
>  drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_dma.c       | 22 +++++++++++--------
>  drivers/gpu/drm/exynos/exynos_drm_drv.h       |  6 +++--
>  drivers/gpu/drm/exynos/exynos_drm_fimc.c      |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c       |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_gsc.c       |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_rotator.c   |  5 +++--
>  drivers/gpu/drm/exynos/exynos_drm_scaler.c    |  6 +++--
>  drivers/gpu/drm/exynos/exynos_mixer.c         |  7 ++++--
>  11 files changed, 47 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index 8428ae12dfa5..1f79bc2a881e 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -55,6 +55,7 @@ static const char * const decon_clks_name[] = {
>  struct decon_context {
>  	struct device			*dev;
>  	struct drm_device		*drm_dev;
> +	void				*dma_priv;
>  	struct exynos_drm_crtc		*crtc;
>  	struct exynos_drm_plane		planes[WINDOWS_NR];
>  	struct exynos_drm_plane_config	configs[WINDOWS_NR];
> @@ -644,7 +645,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
>  
>  	decon_clear_channels(ctx->crtc);
>  
> -	return exynos_drm_register_dma(drm_dev, dev);
> +	return exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
>  }
>  
>  static void decon_unbind(struct device *dev, struct device *master, void *data)
> @@ -654,7 +655,7 @@ static void decon_unbind(struct device *dev, struct device *master, void *data)
>  	decon_atomic_disable(ctx->crtc);
>  
>  	/* detach this sub driver from iommu mapping if supported. */
> -	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
> +	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
>  }
>  
>  static const struct component_ops decon_component_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> index ff59c641fa80..1eed3327999f 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -40,6 +40,7 @@
>  struct decon_context {
>  	struct device			*dev;
>  	struct drm_device		*drm_dev;
> +	void				*dma_priv;
>  	struct exynos_drm_crtc		*crtc;
>  	struct exynos_drm_plane		planes[WINDOWS_NR];
>  	struct exynos_drm_plane_config	configs[WINDOWS_NR];
> @@ -127,13 +128,13 @@ static int decon_ctx_initialize(struct decon_context *ctx,
>  
>  	decon_clear_channels(ctx->crtc);
>  
> -	return exynos_drm_register_dma(drm_dev, ctx->dev);
> +	return exynos_drm_register_dma(drm_dev, ctx->dev, &ctx->dma_priv);
>  }
>  
>  static void decon_ctx_remove(struct decon_context *ctx)
>  {
>  	/* detach this sub driver from iommu mapping if supported. */
> -	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
> +	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
>  }
>  
>  static u32 decon_calc_clkdiv(struct decon_context *ctx,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> index 9ebc02768847..482bec7756fa 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> @@ -58,7 +58,7 @@ static inline void clear_dma_max_seg_size(struct device *dev)
>   * mapping.
>   */
>  static int drm_iommu_attach_device(struct drm_device *drm_dev,
> -				struct device *subdrv_dev)
> +				struct device *subdrv_dev, void **dma_priv)
>  {
>  	struct exynos_drm_private *priv = drm_dev->dev_private;
>  	int ret;
> @@ -74,7 +74,8 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>  		return ret;
>  
>  	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
> -		if (to_dma_iommu_mapping(subdrv_dev))
> +		*dma_priv = to_dma_iommu_mapping(subdrv_dev);
> +		if (*dma_priv)
>  			arm_iommu_detach_device(subdrv_dev);
>  
>  		ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
> @@ -98,19 +99,21 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>   * mapping
>   */
>  static void drm_iommu_detach_device(struct drm_device *drm_dev,
> -				struct device *subdrv_dev)
> +				    struct device *subdrv_dev, void **dma_priv)
>  {
>  	struct exynos_drm_private *priv = drm_dev->dev_private;
>  
> -	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU))
> +	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
>  		arm_iommu_detach_device(subdrv_dev);
> -	else if (IS_ENABLED(CONFIG_IOMMU_DMA))
> +		arm_iommu_attach_device(subdrv_dev, *dma_priv);

I don't see why arm_iommu_attach_device function should be called again at drm_iommu_detach_device function.
I think it should be no problem without arm_iommu_attach_device call.
If there is any problem without arm_iommu_attach_device function call then maybe getting wrong somewhere but not here.

Thanks,
Inki Dae

> +	} else if (IS_ENABLED(CONFIG_IOMMU_DMA))
>  		iommu_detach_device(priv->mapping, subdrv_dev);
>  
>  	clear_dma_max_seg_size(subdrv_dev);
>  }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/exynos: Fix cleanup of IOMMU related objects

[Marek Szyprowski]

Hi Inki,

On 05.03.2020 09:01, Inki Dae wrote:
> Hi Marek,
>
> 20. 3. 2. 오후 11:27에 Marek Szyprowski 이(가) 쓴 글:
>> Store the IOMMU mapping created by device core of each Exynos DRM
>> sub-device and restore it when Exynos DRM driver is unbound. This fixes
>> IOMMU initialization failure for the second time when deferred probe is
>> triggered from the bind() callback of master's compound DRM driver.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  5 +++--
>>   drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  5 +++--
>>   drivers/gpu/drm/exynos/exynos_drm_dma.c       | 22 +++++++++++--------
>>   drivers/gpu/drm/exynos/exynos_drm_drv.h       |  6 +++--
>>   drivers/gpu/drm/exynos/exynos_drm_fimc.c      |  5 +++--
>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  5 +++--
>>   drivers/gpu/drm/exynos/exynos_drm_g2d.c       |  5 +++--
>>   drivers/gpu/drm/exynos/exynos_drm_gsc.c       |  5 +++--
>>   drivers/gpu/drm/exynos/exynos_drm_rotator.c   |  5 +++--
>>   drivers/gpu/drm/exynos/exynos_drm_scaler.c    |  6 +++--
>>   drivers/gpu/drm/exynos/exynos_mixer.c         |  7 ++++--
>>   11 files changed, 47 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> index 8428ae12dfa5..1f79bc2a881e 100644
>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> @@ -55,6 +55,7 @@ static const char * const decon_clks_name[] = {
>>   struct decon_context {
>>   	struct device			*dev;
>>   	struct drm_device		*drm_dev;
>> +	void				*dma_priv;
>>   	struct exynos_drm_crtc		*crtc;
>>   	struct exynos_drm_plane		planes[WINDOWS_NR];
>>   	struct exynos_drm_plane_config	configs[WINDOWS_NR];
>> @@ -644,7 +645,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
>>   
>>   	decon_clear_channels(ctx->crtc);
>>   
>> -	return exynos_drm_register_dma(drm_dev, dev);
>> +	return exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
>>   }
>>   
>>   static void decon_unbind(struct device *dev, struct device *master, void *data)
>> @@ -654,7 +655,7 @@ static void decon_unbind(struct device *dev, struct device *master, void *data)
>>   	decon_atomic_disable(ctx->crtc);
>>   
>>   	/* detach this sub driver from iommu mapping if supported. */
>> -	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
>> +	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
>>   }
>>   
>>   static const struct component_ops decon_component_ops = {
>> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>> index ff59c641fa80..1eed3327999f 100644
>> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>> @@ -40,6 +40,7 @@
>>   struct decon_context {
>>   	struct device			*dev;
>>   	struct drm_device		*drm_dev;
>> +	void				*dma_priv;
>>   	struct exynos_drm_crtc		*crtc;
>>   	struct exynos_drm_plane		planes[WINDOWS_NR];
>>   	struct exynos_drm_plane_config	configs[WINDOWS_NR];
>> @@ -127,13 +128,13 @@ static int decon_ctx_initialize(struct decon_context *ctx,
>>   
>>   	decon_clear_channels(ctx->crtc);
>>   
>> -	return exynos_drm_register_dma(drm_dev, ctx->dev);
>> +	return exynos_drm_register_dma(drm_dev, ctx->dev, &ctx->dma_priv);
>>   }
>>   
>>   static void decon_ctx_remove(struct decon_context *ctx)
>>   {
>>   	/* detach this sub driver from iommu mapping if supported. */
>> -	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
>> +	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
>>   }
>>   
>>   static u32 decon_calc_clkdiv(struct decon_context *ctx,
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c
>> index 9ebc02768847..482bec7756fa 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
>> @@ -58,7 +58,7 @@ static inline void clear_dma_max_seg_size(struct device *dev)
>>    * mapping.
>>    */
>>   static int drm_iommu_attach_device(struct drm_device *drm_dev,
>> -				struct device *subdrv_dev)
>> +				struct device *subdrv_dev, void **dma_priv)
>>   {
>>   	struct exynos_drm_private *priv = drm_dev->dev_private;
>>   	int ret;
>> @@ -74,7 +74,8 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>>   		return ret;
>>   
>>   	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
>> -		if (to_dma_iommu_mapping(subdrv_dev))
>> +		*dma_priv = to_dma_iommu_mapping(subdrv_dev);
>> +		if (*dma_priv)
>>   			arm_iommu_detach_device(subdrv_dev);
>>   
>>   		ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
>> @@ -98,19 +99,21 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>>    * mapping
>>    */
>>   static void drm_iommu_detach_device(struct drm_device *drm_dev,
>> -				struct device *subdrv_dev)
>> +				    struct device *subdrv_dev, void **dma_priv)
>>   {
>>   	struct exynos_drm_private *priv = drm_dev->dev_private;
>>   
>> -	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU))
>> +	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
>>   		arm_iommu_detach_device(subdrv_dev);
>> -	else if (IS_ENABLED(CONFIG_IOMMU_DMA))
>> +		arm_iommu_attach_device(subdrv_dev, *dma_priv);
> I don't see why arm_iommu_attach_device function should be called again at drm_iommu_detach_device function.
> I think it should be no problem without arm_iommu_attach_device call.

If device is not attached again to the mapping created by the DMA 
framework, it will be later considered as a device without IOMMU.

> If there is any problem without arm_iommu_attach_device function call then maybe getting wrong somewhere but not here.

The problem will be during the second initialization of Exynos DRM, 
mainly if the first initialization is interrupted by deferred probe. 
This issue would be also visible when Exynos DRM is compiled as module 
and loaded, removed and loaded again. Sadly, due to some circular 
dependencies, this is not yet possible without the hacks.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/exynos: Fix cleanup of IOMMU related objects

[Lukasz Luba]

Hi Marek,

On 3/2/20 2:27 PM, Marek Szyprowski wrote:
> Store the IOMMU mapping created by device core of each Exynos DRM
> sub-device and restore it when Exynos DRM driver is unbound. This fixes
> IOMMU initialization failure for the second time when deferred probe is
> triggered from the bind() callback of master's compound DRM driver.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>


Attaching back the old mapping looks good. The problem with leakage
is also no longer reported (you can add it to the commit message).

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Regards,
Lukasz

> ---
>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  5 +++--
>   drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  5 +++--
>   drivers/gpu/drm/exynos/exynos_drm_dma.c       | 22 +++++++++++--------
>   drivers/gpu/drm/exynos/exynos_drm_drv.h       |  6 +++--
>   drivers/gpu/drm/exynos/exynos_drm_fimc.c      |  5 +++--
>   drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  5 +++--
>   drivers/gpu/drm/exynos/exynos_drm_g2d.c       |  5 +++--
>   drivers/gpu/drm/exynos/exynos_drm_gsc.c       |  5 +++--
>   drivers/gpu/drm/exynos/exynos_drm_rotator.c   |  5 +++--
>   drivers/gpu/drm/exynos/exynos_drm_scaler.c    |  6 +++--
>   drivers/gpu/drm/exynos/exynos_mixer.c         |  7 ++++--
>   11 files changed, 47 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index 8428ae12dfa5..1f79bc2a881e 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -55,6 +55,7 @@ static const char * const decon_clks_name[] = {
>   struct decon_context {
>   	struct device			*dev;
>   	struct drm_device		*drm_dev;
> +	void				*dma_priv;
>   	struct exynos_drm_crtc		*crtc;
>   	struct exynos_drm_plane		planes[WINDOWS_NR];
>   	struct exynos_drm_plane_config	configs[WINDOWS_NR];
> @@ -644,7 +645,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
>   
>   	decon_clear_channels(ctx->crtc);
>   
> -	return exynos_drm_register_dma(drm_dev, dev);
> +	return exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
>   }
>   
>   static void decon_unbind(struct device *dev, struct device *master, void *data)
> @@ -654,7 +655,7 @@ static void decon_unbind(struct device *dev, struct device *master, void *data)
>   	decon_atomic_disable(ctx->crtc);
>   
>   	/* detach this sub driver from iommu mapping if supported. */
> -	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
> +	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
>   }
>   
>   static const struct component_ops decon_component_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> index ff59c641fa80..1eed3327999f 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -40,6 +40,7 @@
>   struct decon_context {
>   	struct device			*dev;
>   	struct drm_device		*drm_dev;
> +	void				*dma_priv;
>   	struct exynos_drm_crtc		*crtc;
>   	struct exynos_drm_plane		planes[WINDOWS_NR];
>   	struct exynos_drm_plane_config	configs[WINDOWS_NR];
> @@ -127,13 +128,13 @@ static int decon_ctx_initialize(struct decon_context *ctx,
>   
>   	decon_clear_channels(ctx->crtc);
>   
> -	return exynos_drm_register_dma(drm_dev, ctx->dev);
> +	return exynos_drm_register_dma(drm_dev, ctx->dev, &ctx->dma_priv);
>   }
>   
>   static void decon_ctx_remove(struct decon_context *ctx)
>   {
>   	/* detach this sub driver from iommu mapping if supported. */
> -	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
> +	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
>   }
>   
>   static u32 decon_calc_clkdiv(struct decon_context *ctx,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> index 9ebc02768847..482bec7756fa 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> @@ -58,7 +58,7 @@ static inline void clear_dma_max_seg_size(struct device *dev)
>    * mapping.
>    */
>   static int drm_iommu_attach_device(struct drm_device *drm_dev,
> -				struct device *subdrv_dev)
> +				struct device *subdrv_dev, void **dma_priv)
>   {
>   	struct exynos_drm_private *priv = drm_dev->dev_private;
>   	int ret;
> @@ -74,7 +74,8 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>   		return ret;
>   
>   	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
> -		if (to_dma_iommu_mapping(subdrv_dev))
> +		*dma_priv = to_dma_iommu_mapping(subdrv_dev);
> +		if (*dma_priv)
>   			arm_iommu_detach_device(subdrv_dev);
>   
>   		ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
> @@ -98,19 +99,21 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>    * mapping
>    */
>   static void drm_iommu_detach_device(struct drm_device *drm_dev,
> -				struct device *subdrv_dev)
> +				    struct device *subdrv_dev, void **dma_priv)
>   {
>   	struct exynos_drm_private *priv = drm_dev->dev_private;
>   
> -	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU))
> +	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
>   		arm_iommu_detach_device(subdrv_dev);
> -	else if (IS_ENABLED(CONFIG_IOMMU_DMA))
> +		arm_iommu_attach_device(subdrv_dev, *dma_priv);
> +	} else if (IS_ENABLED(CONFIG_IOMMU_DMA))
>   		iommu_detach_device(priv->mapping, subdrv_dev);
>   
>   	clear_dma_max_seg_size(subdrv_dev);
>   }
>   
> -int exynos_drm_register_dma(struct drm_device *drm, struct device *dev)
> +int exynos_drm_register_dma(struct drm_device *drm, struct device *dev,
> +			    void **dma_priv)
>   {
>   	struct exynos_drm_private *priv = drm->dev_private;
>   
> @@ -137,13 +140,14 @@ int exynos_drm_register_dma(struct drm_device *drm, struct device *dev)
>   		priv->mapping = mapping;
>   	}
>   
> -	return drm_iommu_attach_device(drm, dev);
> +	return drm_iommu_attach_device(drm, dev, dma_priv);
>   }
>   
> -void exynos_drm_unregister_dma(struct drm_device *drm, struct device *dev)
> +void exynos_drm_unregister_dma(struct drm_device *drm, struct device *dev,
> +			       void **dma_priv)
>   {
>   	if (IS_ENABLED(CONFIG_EXYNOS_IOMMU))
> -		drm_iommu_detach_device(drm, dev);
> +		drm_iommu_detach_device(drm, dev, dma_priv);
>   }
>   
>   void exynos_drm_cleanup_dma(struct drm_device *drm)
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index d4d21d8cfb90..6ae9056e7a18 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -223,8 +223,10 @@ static inline bool is_drm_iommu_supported(struct drm_device *drm_dev)
>   	return priv->mapping ? true : false;
>   }
>   
> -int exynos_drm_register_dma(struct drm_device *drm, struct device *dev);
> -void exynos_drm_unregister_dma(struct drm_device *drm, struct device *dev);
> +int exynos_drm_register_dma(struct drm_device *drm, struct device *dev,
> +			    void **dma_priv);
> +void exynos_drm_unregister_dma(struct drm_device *drm, struct device *dev,
> +			       void **dma_priv);
>   void exynos_drm_cleanup_dma(struct drm_device *drm);
>   
>   #ifdef CONFIG_DRM_EXYNOS_DPI
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index 8ea2e1d77802..29ab8be8604c 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -97,6 +97,7 @@ struct fimc_scaler {
>   struct fimc_context {
>   	struct exynos_drm_ipp ipp;
>   	struct drm_device *drm_dev;
> +	void		*dma_priv;
>   	struct device	*dev;
>   	struct exynos_drm_ipp_task	*task;
>   	struct exynos_drm_ipp_formats	*formats;
> @@ -1133,7 +1134,7 @@ static int fimc_bind(struct device *dev, struct device *master, void *data)
>   
>   	ctx->drm_dev = drm_dev;
>   	ipp->drm_dev = drm_dev;
> -	exynos_drm_register_dma(drm_dev, dev);
> +	exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
>   
>   	exynos_drm_ipp_register(dev, ipp, &ipp_funcs,
>   			DRM_EXYNOS_IPP_CAP_CROP | DRM_EXYNOS_IPP_CAP_ROTATE |
> @@ -1153,7 +1154,7 @@ static void fimc_unbind(struct device *dev, struct device *master,
>   	struct exynos_drm_ipp *ipp = &ctx->ipp;
>   
>   	exynos_drm_ipp_unregister(dev, ipp);
> -	exynos_drm_unregister_dma(drm_dev, dev);
> +	exynos_drm_unregister_dma(drm_dev, dev, &ctx->dma_priv);
>   }
>   
>   static const struct component_ops fimc_component_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 21aec38702fc..bb67cad8371f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -167,6 +167,7 @@ static struct fimd_driver_data exynos5420_fimd_driver_data = {
>   struct fimd_context {
>   	struct device			*dev;
>   	struct drm_device		*drm_dev;
> +	void				*dma_priv;
>   	struct exynos_drm_crtc		*crtc;
>   	struct exynos_drm_plane		planes[WINDOWS_NR];
>   	struct exynos_drm_plane_config	configs[WINDOWS_NR];
> @@ -1090,7 +1091,7 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
>   	if (is_drm_iommu_supported(drm_dev))
>   		fimd_clear_channels(ctx->crtc);
>   
> -	return exynos_drm_register_dma(drm_dev, dev);
> +	return exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
>   }
>   
>   static void fimd_unbind(struct device *dev, struct device *master,
> @@ -1100,7 +1101,7 @@ static void fimd_unbind(struct device *dev, struct device *master,
>   
>   	fimd_atomic_disable(ctx->crtc);
>   
> -	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
> +	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
>   
>   	if (ctx->encoder)
>   		exynos_dpi_remove(ctx->encoder);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index 2a3382d43bc9..fcee33a43aca 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -232,6 +232,7 @@ struct g2d_runqueue_node {
>   
>   struct g2d_data {
>   	struct device			*dev;
> +	void				*dma_priv;
>   	struct clk			*gate_clk;
>   	void __iomem			*regs;
>   	int				irq;
> @@ -1409,7 +1410,7 @@ static int g2d_bind(struct device *dev, struct device *master, void *data)
>   		return ret;
>   	}
>   
> -	ret = exynos_drm_register_dma(drm_dev, dev);
> +	ret = exynos_drm_register_dma(drm_dev, dev, &g2d->dma_priv);
>   	if (ret < 0) {
>   		dev_err(dev, "failed to enable iommu.\n");
>   		g2d_fini_cmdlist(g2d);
> @@ -1434,7 +1435,7 @@ static void g2d_unbind(struct device *dev, struct device *master, void *data)
>   	priv->g2d_dev = NULL;
>   
>   	cancel_work_sync(&g2d->runqueue_work);
> -	exynos_drm_unregister_dma(g2d->drm_dev, dev);
> +	exynos_drm_unregister_dma(g2d->drm_dev, dev, &g2d->dma_priv);
>   }
>   
>   static const struct component_ops g2d_component_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> index 88b6fcaa20be..45e9aee8366a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> @@ -97,6 +97,7 @@ struct gsc_scaler {
>   struct gsc_context {
>   	struct exynos_drm_ipp ipp;
>   	struct drm_device *drm_dev;
> +	void		*dma_priv;
>   	struct device	*dev;
>   	struct exynos_drm_ipp_task	*task;
>   	struct exynos_drm_ipp_formats	*formats;
> @@ -1169,7 +1170,7 @@ static int gsc_bind(struct device *dev, struct device *master, void *data)
>   
>   	ctx->drm_dev = drm_dev;
>   	ctx->drm_dev = drm_dev;
> -	exynos_drm_register_dma(drm_dev, dev);
> +	exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
>   
>   	exynos_drm_ipp_register(dev, ipp, &ipp_funcs,
>   			DRM_EXYNOS_IPP_CAP_CROP | DRM_EXYNOS_IPP_CAP_ROTATE |
> @@ -1189,7 +1190,7 @@ static void gsc_unbind(struct device *dev, struct device *master,
>   	struct exynos_drm_ipp *ipp = &ctx->ipp;
>   
>   	exynos_drm_ipp_unregister(dev, ipp);
> -	exynos_drm_unregister_dma(drm_dev, dev);
> +	exynos_drm_unregister_dma(drm_dev, dev, &ctx->dma_priv);
>   }
>   
>   static const struct component_ops gsc_component_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
> index b98482990d1a..dafa87b82052 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
> @@ -56,6 +56,7 @@ struct rot_variant {
>   struct rot_context {
>   	struct exynos_drm_ipp ipp;
>   	struct drm_device *drm_dev;
> +	void		*dma_priv;
>   	struct device	*dev;
>   	void __iomem	*regs;
>   	struct clk	*clock;
> @@ -243,7 +244,7 @@ static int rotator_bind(struct device *dev, struct device *master, void *data)
>   
>   	rot->drm_dev = drm_dev;
>   	ipp->drm_dev = drm_dev;
> -	exynos_drm_register_dma(drm_dev, dev);
> +	exynos_drm_register_dma(drm_dev, dev, &rot->dma_priv);
>   
>   	exynos_drm_ipp_register(dev, ipp, &ipp_funcs,
>   			   DRM_EXYNOS_IPP_CAP_CROP | DRM_EXYNOS_IPP_CAP_ROTATE,
> @@ -261,7 +262,7 @@ static void rotator_unbind(struct device *dev, struct device *master,
>   	struct exynos_drm_ipp *ipp = &rot->ipp;
>   
>   	exynos_drm_ipp_unregister(dev, ipp);
> -	exynos_drm_unregister_dma(rot->drm_dev, rot->dev);
> +	exynos_drm_unregister_dma(rot->drm_dev, rot->dev, &rot->dma_priv);
>   }
>   
>   static const struct component_ops rotator_component_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_scaler.c b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
> index 497973e9b2c5..93c43c8d914e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_scaler.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
> @@ -39,6 +39,7 @@ struct scaler_data {
>   struct scaler_context {
>   	struct exynos_drm_ipp		ipp;
>   	struct drm_device		*drm_dev;
> +	void				*dma_priv;
>   	struct device			*dev;
>   	void __iomem			*regs;
>   	struct clk			*clock[SCALER_MAX_CLK];
> @@ -450,7 +451,7 @@ static int scaler_bind(struct device *dev, struct device *master, void *data)
>   
>   	scaler->drm_dev = drm_dev;
>   	ipp->drm_dev = drm_dev;
> -	exynos_drm_register_dma(drm_dev, dev);
> +	exynos_drm_register_dma(drm_dev, dev, &scaler->dma_priv);
>   
>   	exynos_drm_ipp_register(dev, ipp, &ipp_funcs,
>   			DRM_EXYNOS_IPP_CAP_CROP | DRM_EXYNOS_IPP_CAP_ROTATE |
> @@ -470,7 +471,8 @@ static void scaler_unbind(struct device *dev, struct device *master,
>   	struct exynos_drm_ipp *ipp = &scaler->ipp;
>   
>   	exynos_drm_ipp_unregister(dev, ipp);
> -	exynos_drm_unregister_dma(scaler->drm_dev, scaler->dev);
> +	exynos_drm_unregister_dma(scaler->drm_dev, scaler->dev,
> +				  &scaler->dma_priv);
>   }
>   
>   static const struct component_ops scaler_component_ops = {
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 38ae9c32feef..21b726baedea 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -94,6 +94,7 @@ struct mixer_context {
>   	struct platform_device *pdev;
>   	struct device		*dev;
>   	struct drm_device	*drm_dev;
> +	void			*dma_priv;
>   	struct exynos_drm_crtc	*crtc;
>   	struct exynos_drm_plane	planes[MIXER_WIN_NR];
>   	unsigned long		flags;
> @@ -894,12 +895,14 @@ static int mixer_initialize(struct mixer_context *mixer_ctx,
>   		}
>   	}
>   
> -	return exynos_drm_register_dma(drm_dev, mixer_ctx->dev);
> +	return exynos_drm_register_dma(drm_dev, mixer_ctx->dev,
> +				       &mixer_ctx->dma_priv);
>   }
>   
>   static void mixer_ctx_remove(struct mixer_context *mixer_ctx)
>   {
> -	exynos_drm_unregister_dma(mixer_ctx->drm_dev, mixer_ctx->dev);
> +	exynos_drm_unregister_dma(mixer_ctx->drm_dev, mixer_ctx->dev,
> +				  &mixer_ctx->dma_priv);
>   }
>   
>   static int mixer_enable_vblank(struct exynos_drm_crtc *crtc)
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/exynos: Fix cleanup of IOMMU related objects

[Inki Dae]

20. 3. 5. 오후 5:13에 Marek Szyprowski 이(가) 쓴 글:
> Hi Inki,
> 
> On 05.03.2020 09:01, Inki Dae wrote:
>> Hi Marek,
>>
>> 20. 3. 2. 오후 11:27에 Marek Szyprowski 이(가) 쓴 글:
>>> Store the IOMMU mapping created by device core of each Exynos DRM
>>> sub-device and restore it when Exynos DRM driver is unbound. This fixes
>>> IOMMU initialization failure for the second time when deferred probe is
>>> triggered from the bind() callback of master's compound DRM driver.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  5 +++--
>>>   drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  5 +++--
>>>   drivers/gpu/drm/exynos/exynos_drm_dma.c       | 22 +++++++++++--------
>>>   drivers/gpu/drm/exynos/exynos_drm_drv.h       |  6 +++--
>>>   drivers/gpu/drm/exynos/exynos_drm_fimc.c      |  5 +++--
>>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  5 +++--
>>>   drivers/gpu/drm/exynos/exynos_drm_g2d.c       |  5 +++--
>>>   drivers/gpu/drm/exynos/exynos_drm_gsc.c       |  5 +++--
>>>   drivers/gpu/drm/exynos/exynos_drm_rotator.c   |  5 +++--
>>>   drivers/gpu/drm/exynos/exynos_drm_scaler.c    |  6 +++--
>>>   drivers/gpu/drm/exynos/exynos_mixer.c         |  7 ++++--
>>>   11 files changed, 47 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>>> index 8428ae12dfa5..1f79bc2a881e 100644
>>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>>> @@ -55,6 +55,7 @@ static const char * const decon_clks_name[] = {
>>>   struct decon_context {
>>>   	struct device			*dev;
>>>   	struct drm_device		*drm_dev;
>>> +	void				*dma_priv;
>>>   	struct exynos_drm_crtc		*crtc;
>>>   	struct exynos_drm_plane		planes[WINDOWS_NR];
>>>   	struct exynos_drm_plane_config	configs[WINDOWS_NR];
>>> @@ -644,7 +645,7 @@ static int decon_bind(struct device *dev, struct device *master, void *data)
>>>   
>>>   	decon_clear_channels(ctx->crtc);
>>>   
>>> -	return exynos_drm_register_dma(drm_dev, dev);
>>> +	return exynos_drm_register_dma(drm_dev, dev, &ctx->dma_priv);
>>>   }
>>>   
>>>   static void decon_unbind(struct device *dev, struct device *master, void *data)
>>> @@ -654,7 +655,7 @@ static void decon_unbind(struct device *dev, struct device *master, void *data)
>>>   	decon_atomic_disable(ctx->crtc);
>>>   
>>>   	/* detach this sub driver from iommu mapping if supported. */
>>> -	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
>>> +	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
>>>   }
>>>   
>>>   static const struct component_ops decon_component_ops = {
>>> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>>> index ff59c641fa80..1eed3327999f 100644
>>> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>>> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>>> @@ -40,6 +40,7 @@
>>>   struct decon_context {
>>>   	struct device			*dev;
>>>   	struct drm_device		*drm_dev;
>>> +	void				*dma_priv;
>>>   	struct exynos_drm_crtc		*crtc;
>>>   	struct exynos_drm_plane		planes[WINDOWS_NR];
>>>   	struct exynos_drm_plane_config	configs[WINDOWS_NR];
>>> @@ -127,13 +128,13 @@ static int decon_ctx_initialize(struct decon_context *ctx,
>>>   
>>>   	decon_clear_channels(ctx->crtc);
>>>   
>>> -	return exynos_drm_register_dma(drm_dev, ctx->dev);
>>> +	return exynos_drm_register_dma(drm_dev, ctx->dev, &ctx->dma_priv);
>>>   }
>>>   
>>>   static void decon_ctx_remove(struct decon_context *ctx)
>>>   {
>>>   	/* detach this sub driver from iommu mapping if supported. */
>>> -	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev);
>>> +	exynos_drm_unregister_dma(ctx->drm_dev, ctx->dev, &ctx->dma_priv);
>>>   }
>>>   
>>>   static u32 decon_calc_clkdiv(struct decon_context *ctx,
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c
>>> index 9ebc02768847..482bec7756fa 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
>>> @@ -58,7 +58,7 @@ static inline void clear_dma_max_seg_size(struct device *dev)
>>>    * mapping.
>>>    */
>>>   static int drm_iommu_attach_device(struct drm_device *drm_dev,
>>> -				struct device *subdrv_dev)
>>> +				struct device *subdrv_dev, void **dma_priv)
>>>   {
>>>   	struct exynos_drm_private *priv = drm_dev->dev_private;
>>>   	int ret;
>>> @@ -74,7 +74,8 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>>>   		return ret;
>>>   
>>>   	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
>>> -		if (to_dma_iommu_mapping(subdrv_dev))
>>> +		*dma_priv = to_dma_iommu_mapping(subdrv_dev);
>>> +		if (*dma_priv)
>>>   			arm_iommu_detach_device(subdrv_dev);
>>>   
>>>   		ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
>>> @@ -98,19 +99,21 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>>>    * mapping
>>>    */
>>>   static void drm_iommu_detach_device(struct drm_device *drm_dev,
>>> -				struct device *subdrv_dev)
>>> +				    struct device *subdrv_dev, void **dma_priv)
>>>   {
>>>   	struct exynos_drm_private *priv = drm_dev->dev_private;
>>>   
>>> -	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU))
>>> +	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
>>>   		arm_iommu_detach_device(subdrv_dev);
>>> -	else if (IS_ENABLED(CONFIG_IOMMU_DMA))
>>> +		arm_iommu_attach_device(subdrv_dev, *dma_priv);
>> I don't see why arm_iommu_attach_device function should be called again at drm_iommu_detach_device function.
>> I think it should be no problem without arm_iommu_attach_device call.
> 
> If device is not attached again to the mapping created by the DMA 
> framework, it will be later considered as a device without IOMMU.
> 
>> If there is any problem without arm_iommu_attach_device function call then maybe getting wrong somewhere but not here.
> 
> The problem will be during the second initialization of Exynos DRM, 
> mainly if the first initialization is interrupted by deferred probe. 
> This issue would be also visible when Exynos DRM is compiled as module 
> and loaded, removed and loaded again. Sadly, due to some circular 
> dependencies, this is not yet possible without the hacks.

Could you add above your comments on the code so that other developers can understand why the hacks should be used here?

Thanks,
Inki Dae

> 
> Best regards
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel