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

Inki Dae inki.dae at samsung.com
Thu Mar 5 08:01:06 UTC 2020


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 at 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);
>  }


More information about the dri-devel mailing list