drm/exynos: simplify DMA mapping

Submitted by Andrzej Hajda on Sept. 21, 2018, 8:58 a.m.

Details

Message ID 20180921085858eucas1p25807116d04d37d11a5296f44131c23f4~WXpntr8zL1932219322eucas1p2T@eucas1p2.samsung.com
State New
Headers show
Series "drm/exynos: simplify DMA mapping" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Andrzej Hajda Sept. 21, 2018, 8:58 a.m.
Moving DMA mapping creation to drm_iommu_attach_device allows to avoid
looping through all components and maintaining DMA device flags.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c   | 50 +++--------------------
 drivers/gpu/drm/exynos/exynos_drm_iommu.c | 13 +++++-
 2 files changed, 17 insertions(+), 46 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index b599f74692e5..c83437d8a595 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -197,8 +197,7 @@  struct exynos_drm_driver_info {
 
 #define DRM_COMPONENT_DRIVER	BIT(0)	/* supports component framework */
 #define DRM_VIRTUAL_DEVICE	BIT(1)	/* create virtual platform device */
-#define DRM_DMA_DEVICE		BIT(2)	/* can be used for dma allocations */
-#define DRM_FIMC_DEVICE		BIT(3)	/* devices shared with V4L2 subsystem */
+#define DRM_FIMC_DEVICE		BIT(2)	/* devices shared with V4L2 subsystem */
 
 #define DRV_PTR(drv, cond) (IS_ENABLED(cond) ? &drv : NULL)
 
@@ -209,16 +208,16 @@  struct exynos_drm_driver_info {
 static struct exynos_drm_driver_info exynos_drm_drivers[] = {
 	{
 		DRV_PTR(fimd_driver, CONFIG_DRM_EXYNOS_FIMD),
-		DRM_COMPONENT_DRIVER | DRM_DMA_DEVICE
+		DRM_COMPONENT_DRIVER
 	}, {
 		DRV_PTR(exynos5433_decon_driver, CONFIG_DRM_EXYNOS5433_DECON),
-		DRM_COMPONENT_DRIVER | DRM_DMA_DEVICE
+		DRM_COMPONENT_DRIVER
 	}, {
 		DRV_PTR(decon_driver, CONFIG_DRM_EXYNOS7_DECON),
-		DRM_COMPONENT_DRIVER | DRM_DMA_DEVICE
+		DRM_COMPONENT_DRIVER
 	}, {
 		DRV_PTR(mixer_driver, CONFIG_DRM_EXYNOS_MIXER),
-		DRM_COMPONENT_DRIVER | DRM_DMA_DEVICE
+		DRM_COMPONENT_DRIVER
 	}, {
 		DRV_PTR(mic_driver, CONFIG_DRM_EXYNOS_MIC),
 		DRM_COMPONENT_DRIVER
@@ -289,27 +288,6 @@  static struct component_match *exynos_drm_match_add(struct device *dev)
 	return match ?: ERR_PTR(-ENODEV);
 }
 
-static struct device *exynos_drm_get_dma_device(void)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(exynos_drm_drivers); ++i) {
-		struct exynos_drm_driver_info *info = &exynos_drm_drivers[i];
-		struct device *dev;
-
-		if (!info->driver || !(info->flags & DRM_DMA_DEVICE))
-			continue;
-
-		while ((dev = bus_find_device(&platform_bus_type, NULL,
-					    &info->driver->driver,
-					    (void *)platform_bus_type.match))) {
-			put_device(dev);
-			return dev;
-		}
-	}
-	return NULL;
-}
-
 static int exynos_drm_bind(struct device *dev)
 {
 	struct exynos_drm_private *private;
@@ -334,23 +312,6 @@  static int exynos_drm_bind(struct device *dev)
 	dev_set_drvdata(dev, drm);
 	drm->dev_private = (void *)private;
 
-	/* the first real CRTC device is used for all dma mapping operations */
-	private->dma_dev = exynos_drm_get_dma_device();
-	if (!private->dma_dev) {
-		DRM_ERROR("no device found for DMA mapping operations.\n");
-		ret = -ENODEV;
-		goto err_free_private;
-	}
-	DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
-		 dev_name(private->dma_dev));
-
-	/* create common IOMMU mapping for all devices attached to Exynos DRM */
-	ret = drm_create_iommu_mapping(drm);
-	if (ret < 0) {
-		DRM_ERROR("failed to create iommu mapping.\n");
-		goto err_free_private;
-	}
-
 	drm_mode_config_init(drm);
 
 	exynos_drm_mode_config_init(drm);
@@ -408,7 +369,6 @@  static int exynos_drm_bind(struct device *dev)
 err_mode_config_cleanup:
 	drm_mode_config_cleanup(drm);
 	drm_release_iommu_mapping(drm);
-err_free_private:
 	kfree(private);
 err_free_drm:
 	drm_dev_put(drm);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
index 0f373702414e..bb8b800a9fba 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
@@ -75,7 +75,18 @@  int drm_iommu_attach_device(struct drm_device *drm_dev,
 	struct exynos_drm_private *priv = drm_dev->dev_private;
 	int ret;
 
-	if (get_dma_ops(priv->dma_dev) != get_dma_ops(subdrv_dev)) {
+	if (!priv->dma_dev) {
+		priv->dma_dev = subdrv_dev;
+		DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
+			 dev_name(subdrv_dev));
+		/* create common IOMMU mapping for all Exynos DRM devices */
+		ret = drm_create_iommu_mapping(drm_dev);
+		if (ret < 0) {
+			priv->dma_dev = NULL;
+			DRM_ERROR("failed to create iommu mapping.\n");
+			return -EINVAL;
+		}
+	} else if (get_dma_ops(priv->dma_dev) != get_dma_ops(subdrv_dev)) {
 		DRM_ERROR("Device %s lacks support for IOMMU\n",
 			  dev_name(subdrv_dev));
 		return -EINVAL;

Comments

Hi Andrzej,

On 2018-09-21 10:58, Andrzej Hajda wrote:
> Moving DMA mapping creation to drm_iommu_attach_device allows to avoid
> looping through all components and maintaining DMA device flags.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>   drivers/gpu/drm/exynos/exynos_drm_drv.c   | 50 +++--------------------
>   drivers/gpu/drm/exynos/exynos_drm_iommu.c | 13 +++++-

I like simplification, but this one is a bit too invasive. It breaks
Exynos DRM when IOMMU support is disabled (dma_dev won't be set in such
case).

>   2 files changed, 17 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index b599f74692e5..c83437d8a595 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -197,8 +197,7 @@ struct exynos_drm_driver_info {
>   
>   #define DRM_COMPONENT_DRIVER	BIT(0)	/* supports component framework */
>   #define DRM_VIRTUAL_DEVICE	BIT(1)	/* create virtual platform device */
> -#define DRM_DMA_DEVICE		BIT(2)	/* can be used for dma allocations */
> -#define DRM_FIMC_DEVICE		BIT(3)	/* devices shared with V4L2 subsystem */
> +#define DRM_FIMC_DEVICE		BIT(2)	/* devices shared with V4L2 subsystem */
>   
>   #define DRV_PTR(drv, cond) (IS_ENABLED(cond) ? &drv : NULL)
>   
> @@ -209,16 +208,16 @@ struct exynos_drm_driver_info {
>   static struct exynos_drm_driver_info exynos_drm_drivers[] = {
>   	{
>   		DRV_PTR(fimd_driver, CONFIG_DRM_EXYNOS_FIMD),
> -		DRM_COMPONENT_DRIVER | DRM_DMA_DEVICE
> +		DRM_COMPONENT_DRIVER
>   	}, {
>   		DRV_PTR(exynos5433_decon_driver, CONFIG_DRM_EXYNOS5433_DECON),
> -		DRM_COMPONENT_DRIVER | DRM_DMA_DEVICE
> +		DRM_COMPONENT_DRIVER
>   	}, {
>   		DRV_PTR(decon_driver, CONFIG_DRM_EXYNOS7_DECON),
> -		DRM_COMPONENT_DRIVER | DRM_DMA_DEVICE
> +		DRM_COMPONENT_DRIVER
>   	}, {
>   		DRV_PTR(mixer_driver, CONFIG_DRM_EXYNOS_MIXER),
> -		DRM_COMPONENT_DRIVER | DRM_DMA_DEVICE
> +		DRM_COMPONENT_DRIVER
>   	}, {
>   		DRV_PTR(mic_driver, CONFIG_DRM_EXYNOS_MIC),
>   		DRM_COMPONENT_DRIVER
> @@ -289,27 +288,6 @@ static struct component_match *exynos_drm_match_add(struct device *dev)
>   	return match ?: ERR_PTR(-ENODEV);
>   }
>   
> -static struct device *exynos_drm_get_dma_device(void)
> -{
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(exynos_drm_drivers); ++i) {
> -		struct exynos_drm_driver_info *info = &exynos_drm_drivers[i];
> -		struct device *dev;
> -
> -		if (!info->driver || !(info->flags & DRM_DMA_DEVICE))
> -			continue;
> -
> -		while ((dev = bus_find_device(&platform_bus_type, NULL,
> -					    &info->driver->driver,
> -					    (void *)platform_bus_type.match))) {
> -			put_device(dev);
> -			return dev;
> -		}
> -	}
> -	return NULL;
> -}
> -
>   static int exynos_drm_bind(struct device *dev)
>   {
>   	struct exynos_drm_private *private;
> @@ -334,23 +312,6 @@ static int exynos_drm_bind(struct device *dev)
>   	dev_set_drvdata(dev, drm);
>   	drm->dev_private = (void *)private;
>   
> -	/* the first real CRTC device is used for all dma mapping operations */
> -	private->dma_dev = exynos_drm_get_dma_device();
> -	if (!private->dma_dev) {
> -		DRM_ERROR("no device found for DMA mapping operations.\n");
> -		ret = -ENODEV;
> -		goto err_free_private;
> -	}
> -	DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
> -		 dev_name(private->dma_dev));
> -
> -	/* create common IOMMU mapping for all devices attached to Exynos DRM */
> -	ret = drm_create_iommu_mapping(drm);
> -	if (ret < 0) {
> -		DRM_ERROR("failed to create iommu mapping.\n");
> -		goto err_free_private;
> -	}
> -
>   	drm_mode_config_init(drm);
>   
>   	exynos_drm_mode_config_init(drm);
> @@ -408,7 +369,6 @@ static int exynos_drm_bind(struct device *dev)
>   err_mode_config_cleanup:
>   	drm_mode_config_cleanup(drm);
>   	drm_release_iommu_mapping(drm);
> -err_free_private:
>   	kfree(private);
>   err_free_drm:
>   	drm_dev_put(drm);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> index 0f373702414e..bb8b800a9fba 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> @@ -75,7 +75,18 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
>   	struct exynos_drm_private *priv = drm_dev->dev_private;
>   	int ret;
>   
> -	if (get_dma_ops(priv->dma_dev) != get_dma_ops(subdrv_dev)) {
> +	if (!priv->dma_dev) {
> +		priv->dma_dev = subdrv_dev;
> +		DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
> +			 dev_name(subdrv_dev));
> +		/* create common IOMMU mapping for all Exynos DRM devices */
> +		ret = drm_create_iommu_mapping(drm_dev);
> +		if (ret < 0) {
> +			priv->dma_dev = NULL;
> +			DRM_ERROR("failed to create iommu mapping.\n");
> +			return -EINVAL;
> +		}
> +	} else if (get_dma_ops(priv->dma_dev) != get_dma_ops(subdrv_dev)) {
>   		DRM_ERROR("Device %s lacks support for IOMMU\n",
>   			  dev_name(subdrv_dev));
>   		return -EINVAL;

Best regards