Re: [PATCH 2/3] drm/exynos: remove unnecessary checking to support iommu

[Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>]

Hello,

On 2015-07-02 14:49, Joonyoung Shim wrote:
> Already drm_iommu_attach_device and drm_iommu_detach_device check
> whether support iommu internally, so we don't have to call
> is_drm_iommu_supported before call them.
>
> Signed-off-by: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>

Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>

> ---
>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  3 +--
>   drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  3 +--
>   drivers/gpu/drm/exynos/exynos_drm_fimd.c      |  9 +--------
>   drivers/gpu/drm/exynos/exynos_drm_g2d.c       |  6 ------
>   drivers/gpu/drm/exynos/exynos_drm_iommu.c     | 11 +++--------
>   drivers/gpu/drm/exynos/exynos_drm_ipp.c       | 16 ++++++----------
>   drivers/gpu/drm/exynos/exynos_mixer.c         |  3 +--
>   7 files changed, 13 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index 8b1225f..ba43437 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -514,8 +514,7 @@ static void decon_unbind(struct device *dev, struct device *master, void *data)
>   	decon_disable(ctx->crtc);
>   
>   	/* detach this sub driver from iommu mapping if supported. */
> -	if (is_drm_iommu_supported(ctx->drm_dev))
> -		drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
> +	drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
>   }
>   
>   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 362532a..2c29635 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -136,8 +136,7 @@ static int decon_ctx_initialize(struct decon_context *ctx,
>   static void decon_ctx_remove(struct decon_context *ctx)
>   {
>   	/* detach this sub driver from iommu mapping if supported. */
> -	if (is_drm_iommu_supported(ctx->drm_dev))
> -		drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
> +	drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
>   }
>   
>   static u32 decon_calc_clkdiv(struct decon_context *ctx,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 794e56c..300730c 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -348,13 +348,6 @@ static void fimd_clear_channels(struct exynos_drm_crtc *crtc)
>   	pm_runtime_put(ctx->dev);
>   }
>   
> -static void fimd_iommu_detach_devices(struct fimd_context *ctx)
> -{
> -	/* detach this sub driver from iommu mapping if supported. */
> -	if (is_drm_iommu_supported(ctx->drm_dev))
> -		drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
> -}
> -
>   static u32 fimd_calc_clkdiv(struct fimd_context *ctx,
>   		const struct drm_display_mode *mode)
>   {
> @@ -978,7 +971,7 @@ static void fimd_unbind(struct device *dev, struct device *master,
>   
>   	fimd_disable(ctx->crtc);
>   
> -	fimd_iommu_detach_devices(ctx);
> +	drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
>   
>   	if (ctx->display)
>   		exynos_dpi_remove(ctx->display);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index 7584834..7fb8d0b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -1288,9 +1288,6 @@ static int g2d_subdrv_probe(struct drm_device *drm_dev, struct device *dev)
>   		return ret;
>   	}
>   
> -	if (!is_drm_iommu_supported(drm_dev))
> -		return 0;
> -
>   	ret = drm_iommu_attach_device(drm_dev, dev);
>   	if (ret < 0) {
>   		dev_err(dev, "failed to enable iommu.\n");
> @@ -1303,9 +1300,6 @@ static int g2d_subdrv_probe(struct drm_device *drm_dev, struct device *dev)
>   
>   static void g2d_subdrv_remove(struct drm_device *drm_dev, struct device *dev)
>   {
> -	if (!is_drm_iommu_supported(drm_dev))
> -		return;
> -
>   	drm_iommu_detach_device(drm_dev, dev);
>   }
>   
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> index d4ec746..4c2ec1b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> @@ -87,10 +87,8 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
>   	struct device *dev = drm_dev->dev;
>   	int ret;
>   
> -	if (!dev->archdata.mapping) {
> -		DRM_ERROR("iommu_mapping is null.\n");
> -		return -EFAULT;
> -	}
> +	if (!dev->archdata.mapping)
> +		return 0;
>   
>   	subdrv_dev->dma_parms = devm_kzalloc(subdrv_dev,
>   					sizeof(*subdrv_dev->dma_parms),
> @@ -148,13 +146,10 @@ void drm_iommu_detach_device(struct drm_device *drm_dev,
>   int drm_iommu_attach_device_if_possible(struct exynos_drm_crtc *exynos_crtc,
>   			struct drm_device *drm_dev, struct device *subdrv_dev)
>   {
> -	int ret = 0;
> -
>   	if (is_drm_iommu_supported(drm_dev)) {
>   		if (exynos_crtc->ops->clear_channels)
>   			exynos_crtc->ops->clear_channels(exynos_crtc);
> -		return drm_iommu_attach_device(drm_dev, subdrv_dev);
>   	}
>   
> -	return ret;
> +	return drm_iommu_attach_device(drm_dev, subdrv_dev);
>   }
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index 67e5451..67d2423 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -1622,12 +1622,10 @@ static int ipp_subdrv_probe(struct drm_device *drm_dev, struct device *dev)
>   		INIT_LIST_HEAD(&ippdrv->cmd_list);
>   		mutex_init(&ippdrv->cmd_lock);
>   
> -		if (is_drm_iommu_supported(drm_dev)) {
> -			ret = drm_iommu_attach_device(drm_dev, ippdrv->dev);
> -			if (ret) {
> -				DRM_ERROR("failed to activate iommu\n");
> -				goto err;
> -			}
> +		ret = drm_iommu_attach_device(drm_dev, ippdrv->dev);
> +		if (ret) {
> +			DRM_ERROR("failed to activate iommu\n");
> +			goto err;
>   		}
>   	}
>   
> @@ -1637,8 +1635,7 @@ err:
>   	/* get ipp driver entry */
>   	list_for_each_entry_continue_reverse(ippdrv, &exynos_drm_ippdrv_list,
>   						drv_list) {
> -		if (is_drm_iommu_supported(drm_dev))
> -			drm_iommu_detach_device(drm_dev, ippdrv->dev);
> +		drm_iommu_detach_device(drm_dev, ippdrv->dev);
>   
>   		ipp_remove_id(&ctx->ipp_idr, &ctx->ipp_lock,
>   				ippdrv->prop_list.ipp_id);
> @@ -1654,8 +1651,7 @@ static void ipp_subdrv_remove(struct drm_device *drm_dev, struct device *dev)
>   
>   	/* get ipp driver entry */
>   	list_for_each_entry_safe(ippdrv, t, &exynos_drm_ippdrv_list, drv_list) {
> -		if (is_drm_iommu_supported(drm_dev))
> -			drm_iommu_detach_device(drm_dev, ippdrv->dev);
> +		drm_iommu_detach_device(drm_dev, ippdrv->dev);
>   
>   		ipp_remove_id(&ctx->ipp_idr, &ctx->ipp_lock,
>   				ippdrv->prop_list.ipp_id);
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 60538bf..1a01c48 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -891,8 +891,7 @@ static int mixer_initialize(struct mixer_context *mixer_ctx,
>   
>   static void mixer_ctx_remove(struct mixer_context *mixer_ctx)
>   {
> -	if (is_drm_iommu_supported(mixer_ctx->drm_dev))
> -		drm_iommu_detach_device(mixer_ctx->drm_dev, mixer_ctx->dev);
> +	drm_iommu_detach_device(mixer_ctx->drm_dev, mixer_ctx->dev);
>   }
>   
>   static int mixer_enable_vblank(struct exynos_drm_crtc *crtc)

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

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