[v2] drm/exynos: consider deferred probe case

Commit Message

Inki Dae May 29, 2014, 9:28 a.m. UTC

This patch makes sure that exynos drm framework handles deferred
probe case correctly.

Sub drivers could be probed before resources, clock, regulator,
phy or panel, are ready for them so we should make sure that exynos
drm core waits until all resources are ready and sub drivers are
probed correctly.

Chagelog v2:
- Make sure that exynos drm core tries to bind sub drivers only in case that
  they have a pair: crtc and encoder/connector components should be a pair.
- Remove unnecessary patch:
  drm/exynos: mipi-dsi: consider panel driver-deferred probe
- Return error type correctly.

Signed-off-by: Inki Dae <[e-mail]>
Acked-by: Kyungmin Park <[e-mail]>
---
 drivers/gpu/drm/exynos/exynos_dp_core.c  |   18 +++-
 drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   22 ++++-
 drivers/gpu/drm/exynos/exynos_drm_drv.c  |  139 +++++++++++++++++++++++++-----
 drivers/gpu/drm/exynos/exynos_drm_drv.h  |   13 ++-
 drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   41 ++++++---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c |   51 ++++++++---
 drivers/gpu/drm/exynos/exynos_hdmi.c     |   78 ++++++++++++-----
 drivers/gpu/drm/exynos/exynos_mixer.c    |   17 +++-
 8 files changed, 304 insertions(+), 75 deletions(-)

Comments

Andrzej Hajda June 4, 2014, 11:24 a.m. UTC | #1

On 05/29/2014 11:28 AM, Inki Dae wrote:
> This patch makes sure that exynos drm framework handles deferred
> probe case correctly.
> 
> Sub drivers could be probed before resources, clock, regulator,
> phy or panel, are ready for them so we should make sure that exynos
> drm core waits until all resources are ready and sub drivers are
> probed correctly.
> 
> Chagelog v2:
> - Make sure that exynos drm core tries to bind sub drivers only in case that
>   they have a pair: crtc and encoder/connector components should be a pair.

Do we really need it? It adds lot of code which in fact does not improve
exynos_drm - if some driver or device is missing drm will fail at load
or it will start with unusable pipeline anyway, the latter can be
changed to error as well.

> - Remove unnecessary patch:
>   drm/exynos: mipi-dsi: consider panel driver-deferred probe
> - Return error type correctly.
> 
> Signed-off-by: Inki Dae <[e-mail]>
> Acked-by: Kyungmin Park <[e-mail]>
> ---
>  drivers/gpu/drm/exynos/exynos_dp_core.c  |   18 +++-
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   22 ++++-
>  drivers/gpu/drm/exynos/exynos_drm_drv.c  |  139 +++++++++++++++++++++++++-----
>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   13 ++-
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   41 ++++++---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   51 ++++++++---
>  drivers/gpu/drm/exynos/exynos_hdmi.c     |   78 ++++++++++++-----
>  drivers/gpu/drm/exynos/exynos_mixer.c    |   17 +++-
>  8 files changed, 304 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index ff63901..a892586 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> @@ -1328,12 +1328,26 @@ static const struct component_ops exynos_dp_ops = {
>  
>  static int exynos_dp_probe(struct platform_device *pdev)
>  {
> -	return exynos_drm_component_add(&pdev->dev, &exynos_dp_ops);
> +	int ret;
> +
> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
> +					exynos_dp_display.type);
> +	if (ret)
> +		return ret;
> +
> +	ret = component_add(&pdev->dev, &exynos_dp_ops);
> +	if (ret)
> +		exynos_drm_component_del(&pdev->dev,
> +						EXYNOS_DEVICE_TYPE_CONNECTOR);
> +
> +	return ret;
>  }
>  
>  static int exynos_dp_remove(struct platform_device *pdev)
>  {
> -	exynos_drm_component_del(&pdev->dev, &exynos_dp_ops);
> +	component_del(&pdev->dev, &exynos_dp_ops);
> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);

As I wrote in the previous comment calling exynos_drm_component_del here
will cause exynos_drv to stop waiting for exynos_dp to bring up drm
again, which is wrong. The same comment is valid for all other calls of
exynos_drm_component_del in *_remove and *_probe.
Or maybe I miss something???

> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> index a832364..f1b8587 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
> @@ -295,9 +295,15 @@ struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
>  	struct exynos_dpi *ctx;
>  	int ret;
>  
> +	ret = exynos_drm_component_add(dev,
> +					EXYNOS_DEVICE_TYPE_CONNECTOR,
> +					exynos_dpi_display.type);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
>  	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>  	if (!ctx)
> -		return NULL;
> +		goto err_del_component;
>  
>  	ctx->dev = dev;
>  	exynos_dpi_display.ctx = ctx;
> @@ -306,16 +312,24 @@ struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
>  	ret = exynos_dpi_parse_dt(ctx);
>  	if (ret < 0) {
>  		devm_kfree(dev, ctx);
> -		return NULL;
> +		goto err_del_component;
>  	}
>  
>  	if (ctx->panel_node) {
>  		ctx->panel = of_drm_find_panel(ctx->panel_node);
> -		if (!ctx->panel)
> +		if (!ctx->panel) {
> +			exynos_drm_component_del(dev,
> +						EXYNOS_DEVICE_TYPE_CONNECTOR);
>  			return ERR_PTR(-EPROBE_DEFER);
> +		}
>  	}
>  
>  	return &exynos_dpi_display;
> +
> +err_del_component:
> +	exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
> +
> +	return NULL;
>  }
>  
>  int exynos_dpi_remove(struct device *dev)
> @@ -327,5 +341,7 @@ int exynos_dpi_remove(struct device *dev)
>  	encoder->funcs->destroy(encoder);
>  	drm_connector_cleanup(&ctx->connector);
>  
> +	exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
> +
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index c5a401ae..72a5de1 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -48,7 +48,10 @@ static LIST_HEAD(drm_component_list);
>  
>  struct component_dev {
>  	struct list_head list;
> -	struct device *dev;
> +	struct device *crtc_dev;
> +	struct device *conn_dev;
> +	enum exynos_drm_output_type out_type;
> +	unsigned int dev_type_flag;
>  };
>  
>  static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
> @@ -382,22 +385,65 @@ static const struct dev_pm_ops exynos_drm_pm_ops = {
>  };
>  
>  int exynos_drm_component_add(struct device *dev,
> -				const struct component_ops *ops)
> +				enum exynos_drm_device_type dev_type,
> +				enum exynos_drm_output_type out_type)
>  {
>  	struct component_dev *cdev;
> -	int ret;
> +
> +	if (dev_type != EXYNOS_DEVICE_TYPE_CRTC &&
> +			dev_type != EXYNOS_DEVICE_TYPE_CONNECTOR) {
> +		DRM_ERROR("invalid device type.\n");
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&drm_component_lock);
> +
> +	/*
> +	 * Make sure to check if there is a component which has two device
> +	 * objects, for connector and for encoder/connector.
> +	 * It should make sure that crtc and encoder/connector drivers are
> +	 * ready before exynos drm core binds them.
> +	 */
> +	list_for_each_entry(cdev, &drm_component_list, list) {
> +		if (cdev->out_type == out_type) {
> +			/*
> +			 * If crtc and encoder/connector device objects are
> +			 * added already just return.
> +			 */
> +			if (cdev->dev_type_flag == (EXYNOS_DEVICE_TYPE_CRTC |
> +						EXYNOS_DEVICE_TYPE_CONNECTOR)) {
> +				mutex_unlock(&drm_component_lock);
> +				return 0;
> +			}
> +
> +			if (dev_type == EXYNOS_DEVICE_TYPE_CRTC) {
> +				cdev->crtc_dev = dev;
> +				cdev->dev_type_flag |= dev_type;
> +			}
> +
> +			if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR) {
> +				cdev->conn_dev = dev;
> +				cdev->dev_type_flag |= dev_type;
> +			}
> +
> +			mutex_unlock(&drm_component_lock);
> +			return 0;
> +		}
> +	}
> +
> +	mutex_unlock(&drm_component_lock);
>  
>  	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
>  	if (!cdev)
>  		return -ENOMEM;
>  
> -	ret = component_add(dev, ops);
> -	if (ret) {
> -		kfree(cdev);
> -		return ret;
> -	}
> +	if (dev_type == EXYNOS_DEVICE_TYPE_CRTC)
> +		cdev->crtc_dev = dev;
> +	if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR)
> +		cdev->conn_dev = dev;
>  
> -	cdev->dev = dev;
> +	cdev->out_type = out_type;
> +	cdev->dev_type_flag = dev_type;
>  
>  	mutex_lock(&drm_component_lock);
>  	list_add_tail(&cdev->list, &drm_component_list);
> @@ -407,23 +453,40 @@ int exynos_drm_component_add(struct device *dev,
>  }
>  
>  void exynos_drm_component_del(struct device *dev,
> -				const struct component_ops *ops)
> +				enum exynos_drm_device_type dev_type)
>  {
>  	struct component_dev *cdev, *next;
>  
>  	mutex_lock(&drm_component_lock);
>  
>  	list_for_each_entry_safe(cdev, next, &drm_component_list, list) {
> -		if (dev == cdev->dev) {
> +		if (dev_type == EXYNOS_DEVICE_TYPE_CRTC) {
> +			if (cdev->crtc_dev == dev) {
> +				cdev->crtc_dev = NULL;
> +				cdev->dev_type_flag &= ~dev_type;
> +			}
> +		}
> +
> +		if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR) {
> +			if (cdev->conn_dev == dev) {
> +				cdev->conn_dev = NULL;
> +				cdev->dev_type_flag &= ~dev_type;
> +			}
> +		}
> +
> +		/*
> +		 * Release cdev object only in case that both of crtc and
> +		 * encoder/connector device objects are NULL.
> +		 */
> +		if (!cdev->crtc_dev && !cdev->conn_dev) {
>  			list_del(&cdev->list);
>  			kfree(cdev);
> -			break;
>  		}
> +
> +		break;
>  	}
>  
>  	mutex_unlock(&drm_component_lock);
> -
> -	component_del(dev, ops);
>  }
>  
>  static int compare_of(struct device *dev, void *data)
> @@ -433,29 +496,61 @@ static int compare_of(struct device *dev, void *data)
>  
>  static int exynos_drm_add_components(struct device *dev, struct master *m)
>  {
> -	unsigned int attached_cnt = 0;
>  	struct component_dev *cdev;
> +	unsigned int attach_cnt = 0;
>  
>  	mutex_lock(&drm_component_lock);
>  
>  	list_for_each_entry(cdev, &drm_component_list, list) {
>  		int ret;
>  
> +		/*
> +		 * Add components to master only in case that crtc and
> +		 * encoder/connector device objects exist.
> +		 */
> +		if (!cdev->crtc_dev || !cdev->conn_dev)
> +			continue;
> +
> +		attach_cnt++;
> +
>  		mutex_unlock(&drm_component_lock);
>  
> -		ret = component_master_add_child(m, compare_of, cdev->dev);
> -		if (!ret)
> -			attached_cnt++;
> +		/*
> +		 * fimd and dpi modules have same device object so add
> +		 * only crtc device object in this case.
> +		 *
> +		 * TODO. if dpi module follows driver-model driver then
> +		 * below codes can be removed.

It should not happen as DPI is not a separate ip block, it is just
integeral part of fimd.

> +		 */
> +		if (cdev->crtc_dev == cdev->conn_dev) {
> +			ret = component_master_add_child(m, compare_of,
> +					cdev->crtc_dev);
> +			if (ret < 0)
> +				return ret;
> +
> +			goto out_lock;
> +		}
> +
>  
> +		/*
> +		 * Do not chage below call order.
> +		 * crtc device first should be added to master because
> +		 * connector/encoder need pipe number of crtc when they
> +		 * are created.
> +		 */
> +		ret = component_master_add_child(m, compare_of, cdev->crtc_dev);
> +		ret |= component_master_add_child(m, compare_of,
> +							cdev->conn_dev);


The sequence above (up to my previous comment) could be simplified, to
sth like this:

		ret = component_master_add_child(m, compare_of,
						cdev->crtc_dev);
		if (!ret && cdev->crtc_dev != cdev->conn_dev)
			ret = component_master_add_child(m, compare_of,
						cdev->conn_dev);

Regards
Andrzej


> +		if (ret < 0)
> +			return ret;
> +
> +out_lock:
>  		mutex_lock(&drm_component_lock);
>  	}
>  
>  	mutex_unlock(&drm_component_lock);
>  
> -	if (!attached_cnt)
> -		return -ENXIO;
> -
> -	return 0;
> +	return attach_cnt ? 0 : -ENODEV;
>  }
>  
>  static int exynos_drm_bind(struct device *dev)
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index e82e620..36535f3 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -42,6 +42,13 @@ struct drm_connector;
>  
>  extern unsigned int drm_vblank_offdelay;
>  
> +/* This enumerates device type. */
> +enum exynos_drm_device_type {
> +	EXYNOS_DEVICE_TYPE_NONE,
> +	EXYNOS_DEVICE_TYPE_CRTC,
> +	EXYNOS_DEVICE_TYPE_CONNECTOR,
> +};
> +
>  /* this enumerates display type. */
>  enum exynos_drm_output_type {
>  	EXYNOS_DISPLAY_TYPE_NONE,
> @@ -354,12 +361,12 @@ void exynos_drm_remove_vidi(void);
>  int exynos_drm_create_enc_conn(struct drm_device *dev,
>  				struct exynos_drm_display *display);
>  
> -struct component_ops;
>  int exynos_drm_component_add(struct device *dev,
> -				const struct component_ops *ops);
> +				enum exynos_drm_device_type dev_type,
> +				enum exynos_drm_output_type out_type);
>  
>  void exynos_drm_component_del(struct device *dev,
> -				const struct component_ops *ops);
> +				enum exynos_drm_device_type dev_type);
>  
>  extern struct platform_driver fimd_driver;
>  extern struct platform_driver dp_driver;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 84661fe..6302aa6 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1423,10 +1423,16 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>  	struct exynos_dsi *dsi;
>  	int ret;
>  
> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
> +					exynos_dsi_display.type);
> +	if (ret)
> +		return ret;
> +
>  	dsi = devm_kzalloc(&pdev->dev, sizeof(*dsi), GFP_KERNEL);
>  	if (!dsi) {
>  		dev_err(&pdev->dev, "failed to allocate dsi object.\n");
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto err_del_component;
>  	}
>  
>  	init_completion(&dsi->completed);
> @@ -1440,7 +1446,7 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>  
>  	ret = exynos_dsi_parse_dt(dsi);
>  	if (ret)
> -		return ret;
> +		goto err_del_component;
>  
>  	dsi->supplies[0].supply = "vddcore";
>  	dsi->supplies[1].supply = "vddio";
> @@ -1454,32 +1460,37 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>  	dsi->pll_clk = devm_clk_get(&pdev->dev, "pll_clk");
>  	if (IS_ERR(dsi->pll_clk)) {
>  		dev_info(&pdev->dev, "failed to get dsi pll input clock\n");
> -		return -EPROBE_DEFER;
> +		ret = PTR_ERR(dsi->pll_clk);
> +		goto err_del_component;
>  	}
>  
>  	dsi->bus_clk = devm_clk_get(&pdev->dev, "bus_clk");
>  	if (IS_ERR(dsi->bus_clk)) {
>  		dev_info(&pdev->dev, "failed to get dsi bus clock\n");
> -		return -EPROBE_DEFER;
> +		ret = PTR_ERR(dsi->bus_clk);
> +		goto err_del_component;
>  	}
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	dsi->reg_base = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(dsi->reg_base)) {
>  		dev_err(&pdev->dev, "failed to remap io region\n");
> -		return PTR_ERR(dsi->reg_base);
> +		ret = PTR_ERR(dsi->reg_base);
> +		goto err_del_component;
>  	}
>  
>  	dsi->phy = devm_phy_get(&pdev->dev, "dsim");
>  	if (IS_ERR(dsi->phy)) {
>  		dev_info(&pdev->dev, "failed to get dsim phy\n");
> -		return -EPROBE_DEFER;
> +		ret = PTR_ERR(dsi->phy);
> +		goto err_del_component;
>  	}
>  
>  	dsi->irq = platform_get_irq(pdev, 0);
>  	if (dsi->irq < 0) {
>  		dev_err(&pdev->dev, "failed to request dsi irq resource\n");
> -		return dsi->irq;
> +		ret = dsi->irq;
> +		goto err_del_component;
>  	}
>  
>  	irq_set_status_flags(dsi->irq, IRQ_NOAUTOEN);
> @@ -1488,19 +1499,29 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>  					dev_name(&pdev->dev), dsi);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to request dsi irq\n");
> -		return ret;
> +		goto err_del_component;
>  	}
>  
>  	exynos_dsi_display.ctx = dsi;
>  
>  	platform_set_drvdata(pdev, &exynos_dsi_display);
>  
> -	return exynos_drm_component_add(&pdev->dev, &exynos_dsi_component_ops);
> +	ret = component_add(&pdev->dev, &exynos_dsi_component_ops);
> +	if (ret)
> +		goto err_del_component;
> +
> +	return ret;
> +
> +err_del_component:
> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
> +	return ret;
>  }
>  
>  static int exynos_dsi_remove(struct platform_device *pdev)
>  {
> -	exynos_drm_component_del(&pdev->dev, &exynos_dsi_component_ops);
> +	component_del(&pdev->dev, &exynos_dsi_component_ops);
> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index bd30d0c..47a772d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -941,12 +941,21 @@ static int fimd_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int ret = -EINVAL;
>  
> -	if (!dev->of_node)
> -		return -ENODEV;
> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
> +					fimd_manager.type);
> +	if (ret)
> +		return ret;
> +
> +	if (!dev->of_node) {
> +		ret = -ENODEV;
> +		goto err_del_component;
> +	}
>  
>  	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> -	if (!ctx)
> -		return -ENOMEM;
> +	if (!ctx) {
> +		ret = -ENOMEM;
> +		goto err_del_component;
> +	}
>  
>  	ctx->dev = dev;
>  	ctx->suspended = true;
> @@ -959,32 +968,37 @@ static int fimd_probe(struct platform_device *pdev)
>  	ctx->bus_clk = devm_clk_get(dev, "fimd");
>  	if (IS_ERR(ctx->bus_clk)) {
>  		dev_err(dev, "failed to get bus clock\n");
> -		return PTR_ERR(ctx->bus_clk);
> +		ret = PTR_ERR(ctx->bus_clk);
> +		goto err_del_component;
>  	}
>  
>  	ctx->lcd_clk = devm_clk_get(dev, "sclk_fimd");
>  	if (IS_ERR(ctx->lcd_clk)) {
>  		dev_err(dev, "failed to get lcd clock\n");
> -		return PTR_ERR(ctx->lcd_clk);
> +		ret = PTR_ERR(ctx->lcd_clk);
> +		goto err_del_component;
>  	}
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  
>  	ctx->regs = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(ctx->regs))
> -		return PTR_ERR(ctx->regs);
> +	if (IS_ERR(ctx->regs)) {
> +		ret = PTR_ERR(ctx->regs);
> +		goto err_del_component;
> +	}
>  
>  	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "vsync");
>  	if (!res) {
>  		dev_err(dev, "irq request failed.\n");
> -		return -ENXIO;
> +		ret = -ENXIO;
> +		goto err_del_component;
>  	}
>  
>  	ret = devm_request_irq(dev, res->start, fimd_irq_handler,
>  							0, "drm_fimd", ctx);
>  	if (ret) {
>  		dev_err(dev, "irq request failed.\n");
> -		return ret;
> +		goto err_del_component;
>  	}
>  
>  	ctx->driver_data = drm_fimd_get_driver_data(pdev);
> @@ -1001,14 +1015,27 @@ static int fimd_probe(struct platform_device *pdev)
>  
>  	pm_runtime_enable(&pdev->dev);
>  
> -	return exynos_drm_component_add(&pdev->dev, &fimd_component_ops);
> +	ret = component_add(&pdev->dev, &fimd_component_ops);
> +	if (ret)
> +		goto err_disable_pm_runtime;
> +
> +	return ret;
> +
> +err_disable_pm_runtime:
> +	pm_runtime_disable(&pdev->dev);
> +
> +err_del_component:
> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
> +	return ret;
>  }
>  
>  static int fimd_remove(struct platform_device *pdev)
>  {
>  	pm_runtime_disable(&pdev->dev);
>  
> -	exynos_drm_component_del(&pdev->dev, &fimd_component_ops);
> +	component_del(&pdev->dev, &fimd_component_ops);
> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index e05c86a..0c3bcee 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -2134,26 +2134,31 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>  	res->hdmi = devm_clk_get(dev, "hdmi");
>  	if (IS_ERR(res->hdmi)) {
>  		DRM_ERROR("failed to get clock 'hdmi'\n");
> +		ret = PTR_ERR(res->hdmi);
>  		goto fail;
>  	}
>  	res->sclk_hdmi = devm_clk_get(dev, "sclk_hdmi");
>  	if (IS_ERR(res->sclk_hdmi)) {
>  		DRM_ERROR("failed to get clock 'sclk_hdmi'\n");
> +		ret = PTR_ERR(res->sclk_hdmi);
>  		goto fail;
>  	}
>  	res->sclk_pixel = devm_clk_get(dev, "sclk_pixel");
>  	if (IS_ERR(res->sclk_pixel)) {
>  		DRM_ERROR("failed to get clock 'sclk_pixel'\n");
> +		ret = PTR_ERR(res->sclk_pixel);
>  		goto fail;
>  	}
>  	res->sclk_hdmiphy = devm_clk_get(dev, "sclk_hdmiphy");
>  	if (IS_ERR(res->sclk_hdmiphy)) {
>  		DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n");
> +		ret = PTR_ERR(res->sclk_hdmiphy);
>  		goto fail;
>  	}
>  	res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
>  	if (IS_ERR(res->mout_hdmi)) {
>  		DRM_ERROR("failed to get clock 'mout_hdmi'\n");
> +		ret = PTR_ERR(res->mout_hdmi);
>  		goto fail;
>  	}
>  
> @@ -2161,8 +2166,10 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>  
>  	res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
>  		sizeof(res->regul_bulk[0]), GFP_KERNEL);
> -	if (!res->regul_bulk)
> +	if (!res->regul_bulk) {
> +		ret = -ENOMEM;
>  		goto fail;
> +	}
>  	for (i = 0; i < ARRAY_SIZE(supply); ++i) {
>  		res->regul_bulk[i].supply = supply[i];
>  		res->regul_bulk[i].consumer = NULL;
> @@ -2170,14 +2177,14 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>  	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(supply), res->regul_bulk);
>  	if (ret) {
>  		DRM_ERROR("failed to get regulators\n");
> -		goto fail;
> +		return ret;
>  	}
>  	res->regul_count = ARRAY_SIZE(supply);
>  
> -	return 0;
> +	return ret;
>  fail:
>  	DRM_ERROR("HDMI resource init - failed\n");
> -	return -ENODEV;
> +	return ret;
>  }
>  
>  static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata
> @@ -2275,24 +2282,37 @@ static int hdmi_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int ret;
>  
> -	if (!dev->of_node)
> -		return -ENODEV;
> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
> +					hdmi_display.type);
> +	if (ret)
> +		return ret;
> +
> +	if (!dev->of_node) {
> +		ret = -ENODEV;
> +		goto err_del_component;
> +	}
>  
>  	pdata = drm_hdmi_dt_parse_pdata(dev);
> -	if (!pdata)
> -		return -EINVAL;
> +	if (!pdata) {
> +		ret = -EINVAL;
> +		goto err_del_component;
> +	}
>  
>  	hdata = devm_kzalloc(dev, sizeof(struct hdmi_context), GFP_KERNEL);
> -	if (!hdata)
> -		return -ENOMEM;
> +	if (!hdata) {
> +		ret = -ENOMEM;
> +		goto err_del_component;
> +	}
>  
>  	mutex_init(&hdata->hdmi_mutex);
>  
>  	platform_set_drvdata(pdev, &hdmi_display);
>  
>  	match = of_match_node(hdmi_match_types, dev->of_node);
> -	if (!match)
> -		return -ENODEV;
> +	if (!match) {
> +		ret = -ENODEV;
> +		goto err_del_component;
> +	}
>  
>  	drv_data = (struct hdmi_driver_data *)match->data;
>  	hdata->type = drv_data->type;
> @@ -2305,18 +2325,20 @@ static int hdmi_probe(struct platform_device *pdev)
>  	ret = hdmi_resources_init(hdata);
>  	if (ret) {
>  		DRM_ERROR("hdmi_resources_init failed\n");
> -		return -EINVAL;
> +		return ret;
>  	}
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	hdata->regs = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(hdata->regs))
> -		return PTR_ERR(hdata->regs);
> +	if (IS_ERR(hdata->regs)) {
> +		ret = PTR_ERR(hdata->regs);
> +		goto err_del_component;
> +	}
>  
>  	ret = devm_gpio_request(dev, hdata->hpd_gpio, "HPD");
>  	if (ret) {
>  		DRM_ERROR("failed to request HPD gpio\n");
> -		return ret;
> +		goto err_del_component;
>  	}
>  
>  	ddc_node = hdmi_legacy_ddc_dt_binding(dev);
> @@ -2327,14 +2349,15 @@ static int hdmi_probe(struct platform_device *pdev)
>  	ddc_node = of_parse_phandle(dev->of_node, "ddc", 0);
>  	if (!ddc_node) {
>  		DRM_ERROR("Failed to find ddc node in device tree\n");
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto err_del_component;
>  	}
>  
>  out_get_ddc_adpt:
>  	hdata->ddc_adpt = of_find_i2c_adapter_by_node(ddc_node);
>  	if (!hdata->ddc_adpt) {
>  		DRM_ERROR("Failed to get ddc i2c adapter by node\n");
> -		return -ENODEV;
> +		return -EPROBE_DEFER;
>  	}
>  
>  	phy_node = hdmi_legacy_phy_dt_binding(dev);
> @@ -2361,7 +2384,7 @@ out_get_phy_port:
>  		hdata->hdmiphy_port = of_find_i2c_device_by_node(phy_node);
>  		if (!hdata->hdmiphy_port) {
>  			DRM_ERROR("Failed to get hdmi phy i2c client\n");
> -			ret = -ENODEV;
> +			ret = -EPROBE_DEFER;
>  			goto err_ddc;
>  		}
>  	}
> @@ -2390,19 +2413,31 @@ out_get_phy_port:
>  			"samsung,syscon-phandle");
>  	if (IS_ERR(hdata->pmureg)) {
>  		DRM_ERROR("syscon regmap lookup failed.\n");
> +		ret = -EPROBE_DEFER;
>  		goto err_hdmiphy;
>  	}
>  
>  	pm_runtime_enable(dev);
>  	hdmi_display.ctx = hdata;
>  
> -	return exynos_drm_component_add(&pdev->dev, &hdmi_component_ops);
> +	ret = component_add(&pdev->dev, &hdmi_component_ops);
> +	if (ret)
> +		goto err_disable_pm_runtime;
> +
> +	return ret;
> +
> +err_disable_pm_runtime:
> +	pm_runtime_disable(dev);
>  
>  err_hdmiphy:
>  	if (hdata->hdmiphy_port)
>  		put_device(&hdata->hdmiphy_port->dev);
>  err_ddc:
>  	put_device(&hdata->ddc_adpt->dev);
> +
> +err_del_component:
> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
> +
>  	return ret;
>  }
>  
> @@ -2416,8 +2451,9 @@ static int hdmi_remove(struct platform_device *pdev)
>  	put_device(&hdata->ddc_adpt->dev);
>  
>  	pm_runtime_disable(&pdev->dev);
> +	component_del(&pdev->dev, &hdmi_component_ops);
>  
> -	exynos_drm_component_del(&pdev->dev, &hdmi_component_ops);
> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 483d7c0..4c5aed7 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -1273,12 +1273,25 @@ static const struct component_ops mixer_component_ops = {
>  
>  static int mixer_probe(struct platform_device *pdev)
>  {
> -	return exynos_drm_component_add(&pdev->dev, &mixer_component_ops);
> +	int ret;
> +
> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
> +					mixer_manager.type);
> +	if (ret)
> +		return ret;
> +
> +	ret = component_add(&pdev->dev, &mixer_component_ops);
> +	if (ret)
> +		exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
> +
> +	return ret;
>  }
>  
>  static int mixer_remove(struct platform_device *pdev)
>  {
> -	exynos_drm_component_del(&pdev->dev, &mixer_component_ops);
> +	component_del(&pdev->dev, &mixer_component_ops);
> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
> +
>  	return 0;
>  }
>  
>

Inki Dae June 5, 2014, 3:42 a.m. UTC | #2

On 2014? 06? 04? 20:24, Andrzej Hajda wrote:
> On 05/29/2014 11:28 AM, Inki Dae wrote:
>> This patch makes sure that exynos drm framework handles deferred
>> probe case correctly.
>>
>> Sub drivers could be probed before resources, clock, regulator,
>> phy or panel, are ready for them so we should make sure that exynos
>> drm core waits until all resources are ready and sub drivers are
>> probed correctly.
>>
>> Chagelog v2:
>> - Make sure that exynos drm core tries to bind sub drivers only in case that
>>   they have a pair: crtc and encoder/connector components should be a pair.
> 
> Do we really need it? It adds lot of code which in fact does not improve
> exynos_drm - if some driver or device is missing drm will fail at load
> or it will start with unusable pipeline anyway, the latter can be
> changed to error as well.

Previous version also incurred unusable pipeline: crtc driver is bound
even though probe of connector/encoder driver returned error. In this
case, the crtc driver would have nothing to do alone without
connector/encoder part.

> 
>> - Remove unnecessary patch:
>>   drm/exynos: mipi-dsi: consider panel driver-deferred probe
>> - Return error type correctly.
>>
>> Signed-off-by: Inki Dae <[e-mail]>
>> Acked-by: Kyungmin Park <[e-mail]>
>> ---
>>  drivers/gpu/drm/exynos/exynos_dp_core.c  |   18 +++-
>>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   22 ++++-
>>  drivers/gpu/drm/exynos/exynos_drm_drv.c  |  139 +++++++++++++++++++++++++-----
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   13 ++-
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   41 ++++++---
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   51 ++++++++---
>>  drivers/gpu/drm/exynos/exynos_hdmi.c     |   78 ++++++++++++-----
>>  drivers/gpu/drm/exynos/exynos_mixer.c    |   17 +++-
>>  8 files changed, 304 insertions(+), 75 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> index ff63901..a892586 100644
>> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> @@ -1328,12 +1328,26 @@ static const struct component_ops exynos_dp_ops = {
>>  
>>  static int exynos_dp_probe(struct platform_device *pdev)
>>  {
>> -	return exynos_drm_component_add(&pdev->dev, &exynos_dp_ops);
>> +	int ret;
>> +
>> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
>> +					exynos_dp_display.type);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = component_add(&pdev->dev, &exynos_dp_ops);
>> +	if (ret)
>> +		exynos_drm_component_del(&pdev->dev,
>> +						EXYNOS_DEVICE_TYPE_CONNECTOR);
>> +
>> +	return ret;
>>  }
>>  
>>  static int exynos_dp_remove(struct platform_device *pdev)
>>  {
>> -	exynos_drm_component_del(&pdev->dev, &exynos_dp_ops);
>> +	component_del(&pdev->dev, &exynos_dp_ops);
>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
> 
> As I wrote in the previous comment calling exynos_drm_component_del here
> will cause exynos_drv to stop waiting for exynos_dp to bring up drm
> again, which is wrong. The same comment is valid for all other calls of
> exynos_drm_component_del in *_remove and *_probe.
> Or maybe I miss something???
> 

If we remove exynos_drm_component_del calls, the deferred probe case
doesn't work correctly. Assume that connector/encoder driver returned
-EPROBE_DEFER but its corresponding crtc driver worked correctly. In
this case, if we don't call exynos_drm_component_del function at fail
case, exynos drm core will try to bind the sub driver which returned
-EPROBE_DEFER. However, the sub driver will be probed by deferred probe
again, and will call exynos_drm_component_add again. That would be your
missing point.


>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
>> index a832364..f1b8587 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
>> @@ -295,9 +295,15 @@ struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
>>  	struct exynos_dpi *ctx;
>>  	int ret;
>>  
>> +	ret = exynos_drm_component_add(dev,
>> +					EXYNOS_DEVICE_TYPE_CONNECTOR,
>> +					exynos_dpi_display.type);
>> +	if (ret)
>> +		return ERR_PTR(ret);
>> +
>>  	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>  	if (!ctx)
>> -		return NULL;
>> +		goto err_del_component;
>>  
>>  	ctx->dev = dev;
>>  	exynos_dpi_display.ctx = ctx;
>> @@ -306,16 +312,24 @@ struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
>>  	ret = exynos_dpi_parse_dt(ctx);
>>  	if (ret < 0) {
>>  		devm_kfree(dev, ctx);
>> -		return NULL;
>> +		goto err_del_component;
>>  	}
>>  
>>  	if (ctx->panel_node) {
>>  		ctx->panel = of_drm_find_panel(ctx->panel_node);
>> -		if (!ctx->panel)
>> +		if (!ctx->panel) {
>> +			exynos_drm_component_del(dev,
>> +						EXYNOS_DEVICE_TYPE_CONNECTOR);
>>  			return ERR_PTR(-EPROBE_DEFER);
>> +		}
>>  	}
>>  
>>  	return &exynos_dpi_display;
>> +
>> +err_del_component:
>> +	exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>> +
>> +	return NULL;
>>  }
>>  
>>  int exynos_dpi_remove(struct device *dev)
>> @@ -327,5 +341,7 @@ int exynos_dpi_remove(struct device *dev)
>>  	encoder->funcs->destroy(encoder);
>>  	drm_connector_cleanup(&ctx->connector);
>>  
>> +	exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>> +
>>  	return 0;
>>  }
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index c5a401ae..72a5de1 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> @@ -48,7 +48,10 @@ static LIST_HEAD(drm_component_list);
>>  
>>  struct component_dev {
>>  	struct list_head list;
>> -	struct device *dev;
>> +	struct device *crtc_dev;
>> +	struct device *conn_dev;
>> +	enum exynos_drm_output_type out_type;
>> +	unsigned int dev_type_flag;
>>  };
>>  
>>  static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
>> @@ -382,22 +385,65 @@ static const struct dev_pm_ops exynos_drm_pm_ops = {
>>  };
>>  
>>  int exynos_drm_component_add(struct device *dev,
>> -				const struct component_ops *ops)
>> +				enum exynos_drm_device_type dev_type,
>> +				enum exynos_drm_output_type out_type)
>>  {
>>  	struct component_dev *cdev;
>> -	int ret;
>> +
>> +	if (dev_type != EXYNOS_DEVICE_TYPE_CRTC &&
>> +			dev_type != EXYNOS_DEVICE_TYPE_CONNECTOR) {
>> +		DRM_ERROR("invalid device type.\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	mutex_lock(&drm_component_lock);
>> +
>> +	/*
>> +	 * Make sure to check if there is a component which has two device
>> +	 * objects, for connector and for encoder/connector.
>> +	 * It should make sure that crtc and encoder/connector drivers are
>> +	 * ready before exynos drm core binds them.
>> +	 */
>> +	list_for_each_entry(cdev, &drm_component_list, list) {
>> +		if (cdev->out_type == out_type) {
>> +			/*
>> +			 * If crtc and encoder/connector device objects are
>> +			 * added already just return.
>> +			 */
>> +			if (cdev->dev_type_flag == (EXYNOS_DEVICE_TYPE_CRTC |
>> +						EXYNOS_DEVICE_TYPE_CONNECTOR)) {
>> +				mutex_unlock(&drm_component_lock);
>> +				return 0;
>> +			}
>> +
>> +			if (dev_type == EXYNOS_DEVICE_TYPE_CRTC) {
>> +				cdev->crtc_dev = dev;
>> +				cdev->dev_type_flag |= dev_type;
>> +			}
>> +
>> +			if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR) {
>> +				cdev->conn_dev = dev;
>> +				cdev->dev_type_flag |= dev_type;
>> +			}
>> +
>> +			mutex_unlock(&drm_component_lock);
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	mutex_unlock(&drm_component_lock);
>>  
>>  	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
>>  	if (!cdev)
>>  		return -ENOMEM;
>>  
>> -	ret = component_add(dev, ops);
>> -	if (ret) {
>> -		kfree(cdev);
>> -		return ret;
>> -	}
>> +	if (dev_type == EXYNOS_DEVICE_TYPE_CRTC)
>> +		cdev->crtc_dev = dev;
>> +	if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR)
>> +		cdev->conn_dev = dev;
>>  
>> -	cdev->dev = dev;
>> +	cdev->out_type = out_type;
>> +	cdev->dev_type_flag = dev_type;
>>  
>>  	mutex_lock(&drm_component_lock);
>>  	list_add_tail(&cdev->list, &drm_component_list);
>> @@ -407,23 +453,40 @@ int exynos_drm_component_add(struct device *dev,
>>  }
>>  
>>  void exynos_drm_component_del(struct device *dev,
>> -				const struct component_ops *ops)
>> +				enum exynos_drm_device_type dev_type)
>>  {
>>  	struct component_dev *cdev, *next;
>>  
>>  	mutex_lock(&drm_component_lock);
>>  
>>  	list_for_each_entry_safe(cdev, next, &drm_component_list, list) {
>> -		if (dev == cdev->dev) {
>> +		if (dev_type == EXYNOS_DEVICE_TYPE_CRTC) {
>> +			if (cdev->crtc_dev == dev) {
>> +				cdev->crtc_dev = NULL;
>> +				cdev->dev_type_flag &= ~dev_type;
>> +			}
>> +		}
>> +
>> +		if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR) {
>> +			if (cdev->conn_dev == dev) {
>> +				cdev->conn_dev = NULL;
>> +				cdev->dev_type_flag &= ~dev_type;
>> +			}
>> +		}
>> +
>> +		/*
>> +		 * Release cdev object only in case that both of crtc and
>> +		 * encoder/connector device objects are NULL.
>> +		 */
>> +		if (!cdev->crtc_dev && !cdev->conn_dev) {
>>  			list_del(&cdev->list);
>>  			kfree(cdev);
>> -			break;
>>  		}
>> +
>> +		break;
>>  	}
>>  
>>  	mutex_unlock(&drm_component_lock);
>> -
>> -	component_del(dev, ops);
>>  }
>>  
>>  static int compare_of(struct device *dev, void *data)
>> @@ -433,29 +496,61 @@ static int compare_of(struct device *dev, void *data)
>>  
>>  static int exynos_drm_add_components(struct device *dev, struct master *m)
>>  {
>> -	unsigned int attached_cnt = 0;
>>  	struct component_dev *cdev;
>> +	unsigned int attach_cnt = 0;
>>  
>>  	mutex_lock(&drm_component_lock);
>>  
>>  	list_for_each_entry(cdev, &drm_component_list, list) {
>>  		int ret;
>>  
>> +		/*
>> +		 * Add components to master only in case that crtc and
>> +		 * encoder/connector device objects exist.
>> +		 */
>> +		if (!cdev->crtc_dev || !cdev->conn_dev)
>> +			continue;
>> +
>> +		attach_cnt++;
>> +
>>  		mutex_unlock(&drm_component_lock);
>>  
>> -		ret = component_master_add_child(m, compare_of, cdev->dev);
>> -		if (!ret)
>> -			attached_cnt++;
>> +		/*
>> +		 * fimd and dpi modules have same device object so add
>> +		 * only crtc device object in this case.
>> +		 *
>> +		 * TODO. if dpi module follows driver-model driver then
>> +		 * below codes can be removed.
> 
> It should not happen as DPI is not a separate ip block, it is just
> integeral part of fimd.
> 

Hm.. agree. it seems that we may need more clear way.

>> +		 */
>> +		if (cdev->crtc_dev == cdev->conn_dev) {
>> +			ret = component_master_add_child(m, compare_of,
>> +					cdev->crtc_dev);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			goto out_lock;
>> +		}
>> +
>>  
>> +		/*
>> +		 * Do not chage below call order.
>> +		 * crtc device first should be added to master because
>> +		 * connector/encoder need pipe number of crtc when they
>> +		 * are created.
>> +		 */
>> +		ret = component_master_add_child(m, compare_of, cdev->crtc_dev);
>> +		ret |= component_master_add_child(m, compare_of,
>> +							cdev->conn_dev);
> 
> 
> The sequence above (up to my previous comment) could be simplified, to
> sth like this:
> 
> 		ret = component_master_add_child(m, compare_of,
> 						cdev->crtc_dev);
> 		if (!ret && cdev->crtc_dev != cdev->conn_dev)
> 			ret = component_master_add_child(m, compare_of,
> 						cdev->conn_dev);
> 
> Regards
> Andrzej
> 
> 
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +out_lock:
>>  		mutex_lock(&drm_component_lock);
>>  	}
>>  
>>  	mutex_unlock(&drm_component_lock);
>>  
>> -	if (!attached_cnt)
>> -		return -ENXIO;
>> -
>> -	return 0;
>> +	return attach_cnt ? 0 : -ENODEV;
>>  }
>>  
>>  static int exynos_drm_bind(struct device *dev)
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index e82e620..36535f3 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -42,6 +42,13 @@ struct drm_connector;
>>  
>>  extern unsigned int drm_vblank_offdelay;
>>  
>> +/* This enumerates device type. */
>> +enum exynos_drm_device_type {
>> +	EXYNOS_DEVICE_TYPE_NONE,
>> +	EXYNOS_DEVICE_TYPE_CRTC,
>> +	EXYNOS_DEVICE_TYPE_CONNECTOR,
>> +};
>> +
>>  /* this enumerates display type. */
>>  enum exynos_drm_output_type {
>>  	EXYNOS_DISPLAY_TYPE_NONE,
>> @@ -354,12 +361,12 @@ void exynos_drm_remove_vidi(void);
>>  int exynos_drm_create_enc_conn(struct drm_device *dev,
>>  				struct exynos_drm_display *display);
>>  
>> -struct component_ops;
>>  int exynos_drm_component_add(struct device *dev,
>> -				const struct component_ops *ops);
>> +				enum exynos_drm_device_type dev_type,
>> +				enum exynos_drm_output_type out_type);
>>  
>>  void exynos_drm_component_del(struct device *dev,
>> -				const struct component_ops *ops);
>> +				enum exynos_drm_device_type dev_type);
>>  
>>  extern struct platform_driver fimd_driver;
>>  extern struct platform_driver dp_driver;
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index 84661fe..6302aa6 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -1423,10 +1423,16 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>  	struct exynos_dsi *dsi;
>>  	int ret;
>>  
>> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
>> +					exynos_dsi_display.type);
>> +	if (ret)
>> +		return ret;
>> +
>>  	dsi = devm_kzalloc(&pdev->dev, sizeof(*dsi), GFP_KERNEL);
>>  	if (!dsi) {
>>  		dev_err(&pdev->dev, "failed to allocate dsi object.\n");
>> -		return -ENOMEM;
>> +		ret = -ENOMEM;
>> +		goto err_del_component;
>>  	}
>>  
>>  	init_completion(&dsi->completed);
>> @@ -1440,7 +1446,7 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>  
>>  	ret = exynos_dsi_parse_dt(dsi);
>>  	if (ret)
>> -		return ret;
>> +		goto err_del_component;
>>  
>>  	dsi->supplies[0].supply = "vddcore";
>>  	dsi->supplies[1].supply = "vddio";
>> @@ -1454,32 +1460,37 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>  	dsi->pll_clk = devm_clk_get(&pdev->dev, "pll_clk");
>>  	if (IS_ERR(dsi->pll_clk)) {
>>  		dev_info(&pdev->dev, "failed to get dsi pll input clock\n");
>> -		return -EPROBE_DEFER;
>> +		ret = PTR_ERR(dsi->pll_clk);
>> +		goto err_del_component;
>>  	}
>>  
>>  	dsi->bus_clk = devm_clk_get(&pdev->dev, "bus_clk");
>>  	if (IS_ERR(dsi->bus_clk)) {
>>  		dev_info(&pdev->dev, "failed to get dsi bus clock\n");
>> -		return -EPROBE_DEFER;
>> +		ret = PTR_ERR(dsi->bus_clk);
>> +		goto err_del_component;
>>  	}
>>  
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>  	dsi->reg_base = devm_ioremap_resource(&pdev->dev, res);
>>  	if (IS_ERR(dsi->reg_base)) {
>>  		dev_err(&pdev->dev, "failed to remap io region\n");
>> -		return PTR_ERR(dsi->reg_base);
>> +		ret = PTR_ERR(dsi->reg_base);
>> +		goto err_del_component;
>>  	}
>>  
>>  	dsi->phy = devm_phy_get(&pdev->dev, "dsim");
>>  	if (IS_ERR(dsi->phy)) {
>>  		dev_info(&pdev->dev, "failed to get dsim phy\n");
>> -		return -EPROBE_DEFER;
>> +		ret = PTR_ERR(dsi->phy);
>> +		goto err_del_component;
>>  	}
>>  
>>  	dsi->irq = platform_get_irq(pdev, 0);
>>  	if (dsi->irq < 0) {
>>  		dev_err(&pdev->dev, "failed to request dsi irq resource\n");
>> -		return dsi->irq;
>> +		ret = dsi->irq;
>> +		goto err_del_component;
>>  	}
>>  
>>  	irq_set_status_flags(dsi->irq, IRQ_NOAUTOEN);
>> @@ -1488,19 +1499,29 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>  					dev_name(&pdev->dev), dsi);
>>  	if (ret) {
>>  		dev_err(&pdev->dev, "failed to request dsi irq\n");
>> -		return ret;
>> +		goto err_del_component;
>>  	}
>>  
>>  	exynos_dsi_display.ctx = dsi;
>>  
>>  	platform_set_drvdata(pdev, &exynos_dsi_display);
>>  
>> -	return exynos_drm_component_add(&pdev->dev, &exynos_dsi_component_ops);
>> +	ret = component_add(&pdev->dev, &exynos_dsi_component_ops);
>> +	if (ret)
>> +		goto err_del_component;
>> +
>> +	return ret;
>> +
>> +err_del_component:
>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>> +	return ret;
>>  }
>>  
>>  static int exynos_dsi_remove(struct platform_device *pdev)
>>  {
>> -	exynos_drm_component_del(&pdev->dev, &exynos_dsi_component_ops);
>> +	component_del(&pdev->dev, &exynos_dsi_component_ops);
>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index bd30d0c..47a772d 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -941,12 +941,21 @@ static int fimd_probe(struct platform_device *pdev)
>>  	struct resource *res;
>>  	int ret = -EINVAL;
>>  
>> -	if (!dev->of_node)
>> -		return -ENODEV;
>> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
>> +					fimd_manager.type);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (!dev->of_node) {
>> +		ret = -ENODEV;
>> +		goto err_del_component;
>> +	}
>>  
>>  	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>> -	if (!ctx)
>> -		return -ENOMEM;
>> +	if (!ctx) {
>> +		ret = -ENOMEM;
>> +		goto err_del_component;
>> +	}
>>  
>>  	ctx->dev = dev;
>>  	ctx->suspended = true;
>> @@ -959,32 +968,37 @@ static int fimd_probe(struct platform_device *pdev)
>>  	ctx->bus_clk = devm_clk_get(dev, "fimd");
>>  	if (IS_ERR(ctx->bus_clk)) {
>>  		dev_err(dev, "failed to get bus clock\n");
>> -		return PTR_ERR(ctx->bus_clk);
>> +		ret = PTR_ERR(ctx->bus_clk);
>> +		goto err_del_component;
>>  	}
>>  
>>  	ctx->lcd_clk = devm_clk_get(dev, "sclk_fimd");
>>  	if (IS_ERR(ctx->lcd_clk)) {
>>  		dev_err(dev, "failed to get lcd clock\n");
>> -		return PTR_ERR(ctx->lcd_clk);
>> +		ret = PTR_ERR(ctx->lcd_clk);
>> +		goto err_del_component;
>>  	}
>>  
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>  
>>  	ctx->regs = devm_ioremap_resource(dev, res);
>> -	if (IS_ERR(ctx->regs))
>> -		return PTR_ERR(ctx->regs);
>> +	if (IS_ERR(ctx->regs)) {
>> +		ret = PTR_ERR(ctx->regs);
>> +		goto err_del_component;
>> +	}
>>  
>>  	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "vsync");
>>  	if (!res) {
>>  		dev_err(dev, "irq request failed.\n");
>> -		return -ENXIO;
>> +		ret = -ENXIO;
>> +		goto err_del_component;
>>  	}
>>  
>>  	ret = devm_request_irq(dev, res->start, fimd_irq_handler,
>>  							0, "drm_fimd", ctx);
>>  	if (ret) {
>>  		dev_err(dev, "irq request failed.\n");
>> -		return ret;
>> +		goto err_del_component;
>>  	}
>>  
>>  	ctx->driver_data = drm_fimd_get_driver_data(pdev);
>> @@ -1001,14 +1015,27 @@ static int fimd_probe(struct platform_device *pdev)
>>  
>>  	pm_runtime_enable(&pdev->dev);
>>  
>> -	return exynos_drm_component_add(&pdev->dev, &fimd_component_ops);
>> +	ret = component_add(&pdev->dev, &fimd_component_ops);
>> +	if (ret)
>> +		goto err_disable_pm_runtime;
>> +
>> +	return ret;
>> +
>> +err_disable_pm_runtime:
>> +	pm_runtime_disable(&pdev->dev);
>> +
>> +err_del_component:
>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
>> +	return ret;
>>  }
>>  
>>  static int fimd_remove(struct platform_device *pdev)
>>  {
>>  	pm_runtime_disable(&pdev->dev);
>>  
>> -	exynos_drm_component_del(&pdev->dev, &fimd_component_ops);
>> +	component_del(&pdev->dev, &fimd_component_ops);
>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index e05c86a..0c3bcee 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -2134,26 +2134,31 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>  	res->hdmi = devm_clk_get(dev, "hdmi");
>>  	if (IS_ERR(res->hdmi)) {
>>  		DRM_ERROR("failed to get clock 'hdmi'\n");
>> +		ret = PTR_ERR(res->hdmi);
>>  		goto fail;
>>  	}
>>  	res->sclk_hdmi = devm_clk_get(dev, "sclk_hdmi");
>>  	if (IS_ERR(res->sclk_hdmi)) {
>>  		DRM_ERROR("failed to get clock 'sclk_hdmi'\n");
>> +		ret = PTR_ERR(res->sclk_hdmi);
>>  		goto fail;
>>  	}
>>  	res->sclk_pixel = devm_clk_get(dev, "sclk_pixel");
>>  	if (IS_ERR(res->sclk_pixel)) {
>>  		DRM_ERROR("failed to get clock 'sclk_pixel'\n");
>> +		ret = PTR_ERR(res->sclk_pixel);
>>  		goto fail;
>>  	}
>>  	res->sclk_hdmiphy = devm_clk_get(dev, "sclk_hdmiphy");
>>  	if (IS_ERR(res->sclk_hdmiphy)) {
>>  		DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n");
>> +		ret = PTR_ERR(res->sclk_hdmiphy);
>>  		goto fail;
>>  	}
>>  	res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
>>  	if (IS_ERR(res->mout_hdmi)) {
>>  		DRM_ERROR("failed to get clock 'mout_hdmi'\n");
>> +		ret = PTR_ERR(res->mout_hdmi);
>>  		goto fail;
>>  	}
>>  
>> @@ -2161,8 +2166,10 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>  
>>  	res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
>>  		sizeof(res->regul_bulk[0]), GFP_KERNEL);
>> -	if (!res->regul_bulk)
>> +	if (!res->regul_bulk) {
>> +		ret = -ENOMEM;
>>  		goto fail;
>> +	}
>>  	for (i = 0; i < ARRAY_SIZE(supply); ++i) {
>>  		res->regul_bulk[i].supply = supply[i];
>>  		res->regul_bulk[i].consumer = NULL;
>> @@ -2170,14 +2177,14 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>  	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(supply), res->regul_bulk);
>>  	if (ret) {
>>  		DRM_ERROR("failed to get regulators\n");
>> -		goto fail;
>> +		return ret;
>>  	}
>>  	res->regul_count = ARRAY_SIZE(supply);
>>  
>> -	return 0;
>> +	return ret;
>>  fail:
>>  	DRM_ERROR("HDMI resource init - failed\n");
>> -	return -ENODEV;
>> +	return ret;
>>  }
>>  
>>  static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata
>> @@ -2275,24 +2282,37 @@ static int hdmi_probe(struct platform_device *pdev)
>>  	struct resource *res;
>>  	int ret;
>>  
>> -	if (!dev->of_node)
>> -		return -ENODEV;
>> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
>> +					hdmi_display.type);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (!dev->of_node) {
>> +		ret = -ENODEV;
>> +		goto err_del_component;
>> +	}
>>  
>>  	pdata = drm_hdmi_dt_parse_pdata(dev);
>> -	if (!pdata)
>> -		return -EINVAL;
>> +	if (!pdata) {
>> +		ret = -EINVAL;
>> +		goto err_del_component;
>> +	}
>>  
>>  	hdata = devm_kzalloc(dev, sizeof(struct hdmi_context), GFP_KERNEL);
>> -	if (!hdata)
>> -		return -ENOMEM;
>> +	if (!hdata) {
>> +		ret = -ENOMEM;
>> +		goto err_del_component;
>> +	}
>>  
>>  	mutex_init(&hdata->hdmi_mutex);
>>  
>>  	platform_set_drvdata(pdev, &hdmi_display);
>>  
>>  	match = of_match_node(hdmi_match_types, dev->of_node);
>> -	if (!match)
>> -		return -ENODEV;
>> +	if (!match) {
>> +		ret = -ENODEV;
>> +		goto err_del_component;
>> +	}
>>  
>>  	drv_data = (struct hdmi_driver_data *)match->data;
>>  	hdata->type = drv_data->type;
>> @@ -2305,18 +2325,20 @@ static int hdmi_probe(struct platform_device *pdev)
>>  	ret = hdmi_resources_init(hdata);
>>  	if (ret) {
>>  		DRM_ERROR("hdmi_resources_init failed\n");
>> -		return -EINVAL;
>> +		return ret;
>>  	}
>>  
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>  	hdata->regs = devm_ioremap_resource(dev, res);
>> -	if (IS_ERR(hdata->regs))
>> -		return PTR_ERR(hdata->regs);
>> +	if (IS_ERR(hdata->regs)) {
>> +		ret = PTR_ERR(hdata->regs);
>> +		goto err_del_component;
>> +	}
>>  
>>  	ret = devm_gpio_request(dev, hdata->hpd_gpio, "HPD");
>>  	if (ret) {
>>  		DRM_ERROR("failed to request HPD gpio\n");
>> -		return ret;
>> +		goto err_del_component;
>>  	}
>>  
>>  	ddc_node = hdmi_legacy_ddc_dt_binding(dev);
>> @@ -2327,14 +2349,15 @@ static int hdmi_probe(struct platform_device *pdev)
>>  	ddc_node = of_parse_phandle(dev->of_node, "ddc", 0);
>>  	if (!ddc_node) {
>>  		DRM_ERROR("Failed to find ddc node in device tree\n");
>> -		return -ENODEV;
>> +		ret = -ENODEV;
>> +		goto err_del_component;
>>  	}
>>  
>>  out_get_ddc_adpt:
>>  	hdata->ddc_adpt = of_find_i2c_adapter_by_node(ddc_node);
>>  	if (!hdata->ddc_adpt) {
>>  		DRM_ERROR("Failed to get ddc i2c adapter by node\n");
>> -		return -ENODEV;
>> +		return -EPROBE_DEFER;
>>  	}
>>  
>>  	phy_node = hdmi_legacy_phy_dt_binding(dev);
>> @@ -2361,7 +2384,7 @@ out_get_phy_port:
>>  		hdata->hdmiphy_port = of_find_i2c_device_by_node(phy_node);
>>  		if (!hdata->hdmiphy_port) {
>>  			DRM_ERROR("Failed to get hdmi phy i2c client\n");
>> -			ret = -ENODEV;
>> +			ret = -EPROBE_DEFER;
>>  			goto err_ddc;
>>  		}
>>  	}
>> @@ -2390,19 +2413,31 @@ out_get_phy_port:
>>  			"samsung,syscon-phandle");
>>  	if (IS_ERR(hdata->pmureg)) {
>>  		DRM_ERROR("syscon regmap lookup failed.\n");
>> +		ret = -EPROBE_DEFER;
>>  		goto err_hdmiphy;
>>  	}
>>  
>>  	pm_runtime_enable(dev);
>>  	hdmi_display.ctx = hdata;
>>  
>> -	return exynos_drm_component_add(&pdev->dev, &hdmi_component_ops);
>> +	ret = component_add(&pdev->dev, &hdmi_component_ops);
>> +	if (ret)
>> +		goto err_disable_pm_runtime;
>> +
>> +	return ret;
>> +
>> +err_disable_pm_runtime:
>> +	pm_runtime_disable(dev);
>>  
>>  err_hdmiphy:
>>  	if (hdata->hdmiphy_port)
>>  		put_device(&hdata->hdmiphy_port->dev);
>>  err_ddc:
>>  	put_device(&hdata->ddc_adpt->dev);
>> +
>> +err_del_component:
>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>> +
>>  	return ret;
>>  }
>>  
>> @@ -2416,8 +2451,9 @@ static int hdmi_remove(struct platform_device *pdev)
>>  	put_device(&hdata->ddc_adpt->dev);
>>  
>>  	pm_runtime_disable(&pdev->dev);
>> +	component_del(&pdev->dev, &hdmi_component_ops);
>>  
>> -	exynos_drm_component_del(&pdev->dev, &hdmi_component_ops);
>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>>  	return 0;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>> index 483d7c0..4c5aed7 100644
>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>> @@ -1273,12 +1273,25 @@ static const struct component_ops mixer_component_ops = {
>>  
>>  static int mixer_probe(struct platform_device *pdev)
>>  {
>> -	return exynos_drm_component_add(&pdev->dev, &mixer_component_ops);
>> +	int ret;
>> +
>> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
>> +					mixer_manager.type);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = component_add(&pdev->dev, &mixer_component_ops);
>> +	if (ret)
>> +		exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
>> +
>> +	return ret;
>>  }
>>  
>>  static int mixer_remove(struct platform_device *pdev)
>>  {
>> -	exynos_drm_component_del(&pdev->dev, &mixer_component_ops);
>> +	component_del(&pdev->dev, &mixer_component_ops);
>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
>> +
>>  	return 0;
>>  }
>>  
>>
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Andrzej Hajda June 5, 2014, 7:36 a.m. UTC | #3

On 06/05/2014 05:42 AM, Inki Dae wrote:
> On 2014? 06? 04? 20:24, Andrzej Hajda wrote:
>> On 05/29/2014 11:28 AM, Inki Dae wrote:
>>> This patch makes sure that exynos drm framework handles deferred
>>> probe case correctly.
>>>
>>> Sub drivers could be probed before resources, clock, regulator,
>>> phy or panel, are ready for them so we should make sure that exynos
>>> drm core waits until all resources are ready and sub drivers are
>>> probed correctly.
>>>
>>> Chagelog v2:
>>> - Make sure that exynos drm core tries to bind sub drivers only in case that
>>>   they have a pair: crtc and encoder/connector components should be a pair.
>> Do we really need it? It adds lot of code which in fact does not improve
>> exynos_drm - if some driver or device is missing drm will fail at load
>> or it will start with unusable pipeline anyway, the latter can be
>> changed to error as well.
> Previous version also incurred unusable pipeline: crtc driver is bound
> even though probe of connector/encoder driver returned error. In this
> case, the crtc driver would have nothing to do alone without
> connector/encoder part.

That was because exynos_drm_component_del were used on probe fail and
remove callbacks.
I have commented it in [1], I forgot to add that exynos_drm_add_components
should return success only if all components from the list are present
and added successfully.

[1]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/32463

>
>>> - Remove unnecessary patch:
>>>   drm/exynos: mipi-dsi: consider panel driver-deferred probe
>>> - Return error type correctly.
>>>
>>> Signed-off-by: Inki Dae <[e-mail]>
>>> Acked-by: Kyungmin Park <[e-mail]>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_dp_core.c  |   18 +++-
>>>  drivers/gpu/drm/exynos/exynos_drm_dpi.c  |   22 ++++-
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.c  |  139 +++++++++++++++++++++++++-----
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |   13 ++-
>>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c  |   41 ++++++---
>>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   51 ++++++++---
>>>  drivers/gpu/drm/exynos/exynos_hdmi.c     |   78 ++++++++++++-----
>>>  drivers/gpu/drm/exynos/exynos_mixer.c    |   17 +++-
>>>  8 files changed, 304 insertions(+), 75 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
>>> index ff63901..a892586 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
>>> @@ -1328,12 +1328,26 @@ static const struct component_ops exynos_dp_ops = {
>>>  
>>>  static int exynos_dp_probe(struct platform_device *pdev)
>>>  {
>>> -	return exynos_drm_component_add(&pdev->dev, &exynos_dp_ops);
>>> +	int ret;
>>> +
>>> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
>>> +					exynos_dp_display.type);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = component_add(&pdev->dev, &exynos_dp_ops);
>>> +	if (ret)
>>> +		exynos_drm_component_del(&pdev->dev,
>>> +						EXYNOS_DEVICE_TYPE_CONNECTOR);
>>> +
>>> +	return ret;
>>>  }
>>>  
>>>  static int exynos_dp_remove(struct platform_device *pdev)
>>>  {
>>> -	exynos_drm_component_del(&pdev->dev, &exynos_dp_ops);
>>> +	component_del(&pdev->dev, &exynos_dp_ops);
>>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>> As I wrote in the previous comment calling exynos_drm_component_del here
>> will cause exynos_drv to stop waiting for exynos_dp to bring up drm
>> again, which is wrong. The same comment is valid for all other calls of
>> exynos_drm_component_del in *_remove and *_probe.
>> Or maybe I miss something???
>>
> If we remove exynos_drm_component_del calls, the deferred probe case
> doesn't work correctly. Assume that connector/encoder driver returned
> -EPROBE_DEFER but its corresponding crtc driver worked correctly. In
> this case, if we don't call exynos_drm_component_del function at fail
> case, exynos drm core will try to bind the sub driver which returned
> -EPROBE_DEFER. However, the sub driver will be probed by deferred probe
> again, and will call exynos_drm_component_add again. That would be your
> missing point.

Yes, I forgot again to examine exynos_drm_add_components.
I would change its behavior as described in my previous commit.

Regards
Andrzej

>
>
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
>>> index a832364..f1b8587 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
>>> @@ -295,9 +295,15 @@ struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
>>>  	struct exynos_dpi *ctx;
>>>  	int ret;
>>>  
>>> +	ret = exynos_drm_component_add(dev,
>>> +					EXYNOS_DEVICE_TYPE_CONNECTOR,
>>> +					exynos_dpi_display.type);
>>> +	if (ret)
>>> +		return ERR_PTR(ret);
>>> +
>>>  	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>>  	if (!ctx)
>>> -		return NULL;
>>> +		goto err_del_component;
>>>  
>>>  	ctx->dev = dev;
>>>  	exynos_dpi_display.ctx = ctx;
>>> @@ -306,16 +312,24 @@ struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
>>>  	ret = exynos_dpi_parse_dt(ctx);
>>>  	if (ret < 0) {
>>>  		devm_kfree(dev, ctx);
>>> -		return NULL;
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	if (ctx->panel_node) {
>>>  		ctx->panel = of_drm_find_panel(ctx->panel_node);
>>> -		if (!ctx->panel)
>>> +		if (!ctx->panel) {
>>> +			exynos_drm_component_del(dev,
>>> +						EXYNOS_DEVICE_TYPE_CONNECTOR);
>>>  			return ERR_PTR(-EPROBE_DEFER);
>>> +		}
>>>  	}
>>>  
>>>  	return &exynos_dpi_display;
>>> +
>>> +err_del_component:
>>> +	exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>>> +
>>> +	return NULL;
>>>  }
>>>  
>>>  int exynos_dpi_remove(struct device *dev)
>>> @@ -327,5 +341,7 @@ int exynos_dpi_remove(struct device *dev)
>>>  	encoder->funcs->destroy(encoder);
>>>  	drm_connector_cleanup(&ctx->connector);
>>>  
>>> +	exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>>> +
>>>  	return 0;
>>>  }
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> index c5a401ae..72a5de1 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> @@ -48,7 +48,10 @@ static LIST_HEAD(drm_component_list);
>>>  
>>>  struct component_dev {
>>>  	struct list_head list;
>>> -	struct device *dev;
>>> +	struct device *crtc_dev;
>>> +	struct device *conn_dev;
>>> +	enum exynos_drm_output_type out_type;
>>> +	unsigned int dev_type_flag;
>>>  };
>>>  
>>>  static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
>>> @@ -382,22 +385,65 @@ static const struct dev_pm_ops exynos_drm_pm_ops = {
>>>  };
>>>  
>>>  int exynos_drm_component_add(struct device *dev,
>>> -				const struct component_ops *ops)
>>> +				enum exynos_drm_device_type dev_type,
>>> +				enum exynos_drm_output_type out_type)
>>>  {
>>>  	struct component_dev *cdev;
>>> -	int ret;
>>> +
>>> +	if (dev_type != EXYNOS_DEVICE_TYPE_CRTC &&
>>> +			dev_type != EXYNOS_DEVICE_TYPE_CONNECTOR) {
>>> +		DRM_ERROR("invalid device type.\n");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	mutex_lock(&drm_component_lock);
>>> +
>>> +	/*
>>> +	 * Make sure to check if there is a component which has two device
>>> +	 * objects, for connector and for encoder/connector.
>>> +	 * It should make sure that crtc and encoder/connector drivers are
>>> +	 * ready before exynos drm core binds them.
>>> +	 */
>>> +	list_for_each_entry(cdev, &drm_component_list, list) {
>>> +		if (cdev->out_type == out_type) {
>>> +			/*
>>> +			 * If crtc and encoder/connector device objects are
>>> +			 * added already just return.
>>> +			 */
>>> +			if (cdev->dev_type_flag == (EXYNOS_DEVICE_TYPE_CRTC |
>>> +						EXYNOS_DEVICE_TYPE_CONNECTOR)) {
>>> +				mutex_unlock(&drm_component_lock);
>>> +				return 0;
>>> +			}
>>> +
>>> +			if (dev_type == EXYNOS_DEVICE_TYPE_CRTC) {
>>> +				cdev->crtc_dev = dev;
>>> +				cdev->dev_type_flag |= dev_type;
>>> +			}
>>> +
>>> +			if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR) {
>>> +				cdev->conn_dev = dev;
>>> +				cdev->dev_type_flag |= dev_type;
>>> +			}
>>> +
>>> +			mutex_unlock(&drm_component_lock);
>>> +			return 0;
>>> +		}
>>> +	}
>>> +
>>> +	mutex_unlock(&drm_component_lock);
>>>  
>>>  	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
>>>  	if (!cdev)
>>>  		return -ENOMEM;
>>>  
>>> -	ret = component_add(dev, ops);
>>> -	if (ret) {
>>> -		kfree(cdev);
>>> -		return ret;
>>> -	}
>>> +	if (dev_type == EXYNOS_DEVICE_TYPE_CRTC)
>>> +		cdev->crtc_dev = dev;
>>> +	if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR)
>>> +		cdev->conn_dev = dev;
>>>  
>>> -	cdev->dev = dev;
>>> +	cdev->out_type = out_type;
>>> +	cdev->dev_type_flag = dev_type;
>>>  
>>>  	mutex_lock(&drm_component_lock);
>>>  	list_add_tail(&cdev->list, &drm_component_list);
>>> @@ -407,23 +453,40 @@ int exynos_drm_component_add(struct device *dev,
>>>  }
>>>  
>>>  void exynos_drm_component_del(struct device *dev,
>>> -				const struct component_ops *ops)
>>> +				enum exynos_drm_device_type dev_type)
>>>  {
>>>  	struct component_dev *cdev, *next;
>>>  
>>>  	mutex_lock(&drm_component_lock);
>>>  
>>>  	list_for_each_entry_safe(cdev, next, &drm_component_list, list) {
>>> -		if (dev == cdev->dev) {
>>> +		if (dev_type == EXYNOS_DEVICE_TYPE_CRTC) {
>>> +			if (cdev->crtc_dev == dev) {
>>> +				cdev->crtc_dev = NULL;
>>> +				cdev->dev_type_flag &= ~dev_type;
>>> +			}
>>> +		}
>>> +
>>> +		if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR) {
>>> +			if (cdev->conn_dev == dev) {
>>> +				cdev->conn_dev = NULL;
>>> +				cdev->dev_type_flag &= ~dev_type;
>>> +			}
>>> +		}
>>> +
>>> +		/*
>>> +		 * Release cdev object only in case that both of crtc and
>>> +		 * encoder/connector device objects are NULL.
>>> +		 */
>>> +		if (!cdev->crtc_dev && !cdev->conn_dev) {
>>>  			list_del(&cdev->list);
>>>  			kfree(cdev);
>>> -			break;
>>>  		}
>>> +
>>> +		break;
>>>  	}
>>>  
>>>  	mutex_unlock(&drm_component_lock);
>>> -
>>> -	component_del(dev, ops);
>>>  }
>>>  
>>>  static int compare_of(struct device *dev, void *data)
>>> @@ -433,29 +496,61 @@ static int compare_of(struct device *dev, void *data)
>>>  
>>>  static int exynos_drm_add_components(struct device *dev, struct master *m)
>>>  {
>>> -	unsigned int attached_cnt = 0;
>>>  	struct component_dev *cdev;
>>> +	unsigned int attach_cnt = 0;
>>>  
>>>  	mutex_lock(&drm_component_lock);
>>>  
>>>  	list_for_each_entry(cdev, &drm_component_list, list) {
>>>  		int ret;
>>>  
>>> +		/*
>>> +		 * Add components to master only in case that crtc and
>>> +		 * encoder/connector device objects exist.
>>> +		 */
>>> +		if (!cdev->crtc_dev || !cdev->conn_dev)
>>> +			continue;
>>> +
>>> +		attach_cnt++;
>>> +
>>>  		mutex_unlock(&drm_component_lock);
>>>  
>>> -		ret = component_master_add_child(m, compare_of, cdev->dev);
>>> -		if (!ret)
>>> -			attached_cnt++;
>>> +		/*
>>> +		 * fimd and dpi modules have same device object so add
>>> +		 * only crtc device object in this case.
>>> +		 *
>>> +		 * TODO. if dpi module follows driver-model driver then
>>> +		 * below codes can be removed.
>> It should not happen as DPI is not a separate ip block, it is just
>> integeral part of fimd.
>>
> Hm.. agree. it seems that we may need more clear way.
>
>>> +		 */
>>> +		if (cdev->crtc_dev == cdev->conn_dev) {
>>> +			ret = component_master_add_child(m, compare_of,
>>> +					cdev->crtc_dev);
>>> +			if (ret < 0)
>>> +				return ret;
>>> +
>>> +			goto out_lock;
>>> +		}
>>> +
>>>  
>>> +		/*
>>> +		 * Do not chage below call order.
>>> +		 * crtc device first should be added to master because
>>> +		 * connector/encoder need pipe number of crtc when they
>>> +		 * are created.
>>> +		 */
>>> +		ret = component_master_add_child(m, compare_of, cdev->crtc_dev);
>>> +		ret |= component_master_add_child(m, compare_of,
>>> +							cdev->conn_dev);
>>
>> The sequence above (up to my previous comment) could be simplified, to
>> sth like this:
>>
>> 		ret = component_master_add_child(m, compare_of,
>> 						cdev->crtc_dev);
>> 		if (!ret && cdev->crtc_dev != cdev->conn_dev)
>> 			ret = component_master_add_child(m, compare_of,
>> 						cdev->conn_dev);
>>
>> Regards
>> Andrzej
>>
>>
>>> +		if (ret < 0)
>>> +			return ret;
>>> +
>>> +out_lock:
>>>  		mutex_lock(&drm_component_lock);
>>>  	}
>>>  
>>>  	mutex_unlock(&drm_component_lock);
>>>  
>>> -	if (!attached_cnt)
>>> -		return -ENXIO;
>>> -
>>> -	return 0;
>>> +	return attach_cnt ? 0 : -ENODEV;
>>>  }
>>>  
>>>  static int exynos_drm_bind(struct device *dev)
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>>> index e82e620..36535f3 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>>> @@ -42,6 +42,13 @@ struct drm_connector;
>>>  
>>>  extern unsigned int drm_vblank_offdelay;
>>>  
>>> +/* This enumerates device type. */
>>> +enum exynos_drm_device_type {
>>> +	EXYNOS_DEVICE_TYPE_NONE,
>>> +	EXYNOS_DEVICE_TYPE_CRTC,
>>> +	EXYNOS_DEVICE_TYPE_CONNECTOR,
>>> +};
>>> +
>>>  /* this enumerates display type. */
>>>  enum exynos_drm_output_type {
>>>  	EXYNOS_DISPLAY_TYPE_NONE,
>>> @@ -354,12 +361,12 @@ void exynos_drm_remove_vidi(void);
>>>  int exynos_drm_create_enc_conn(struct drm_device *dev,
>>>  				struct exynos_drm_display *display);
>>>  
>>> -struct component_ops;
>>>  int exynos_drm_component_add(struct device *dev,
>>> -				const struct component_ops *ops);
>>> +				enum exynos_drm_device_type dev_type,
>>> +				enum exynos_drm_output_type out_type);
>>>  
>>>  void exynos_drm_component_del(struct device *dev,
>>> -				const struct component_ops *ops);
>>> +				enum exynos_drm_device_type dev_type);
>>>  
>>>  extern struct platform_driver fimd_driver;
>>>  extern struct platform_driver dp_driver;
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> index 84661fe..6302aa6 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> @@ -1423,10 +1423,16 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>>  	struct exynos_dsi *dsi;
>>>  	int ret;
>>>  
>>> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
>>> +					exynos_dsi_display.type);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>>  	dsi = devm_kzalloc(&pdev->dev, sizeof(*dsi), GFP_KERNEL);
>>>  	if (!dsi) {
>>>  		dev_err(&pdev->dev, "failed to allocate dsi object.\n");
>>> -		return -ENOMEM;
>>> +		ret = -ENOMEM;
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	init_completion(&dsi->completed);
>>> @@ -1440,7 +1446,7 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>>  
>>>  	ret = exynos_dsi_parse_dt(dsi);
>>>  	if (ret)
>>> -		return ret;
>>> +		goto err_del_component;
>>>  
>>>  	dsi->supplies[0].supply = "vddcore";
>>>  	dsi->supplies[1].supply = "vddio";
>>> @@ -1454,32 +1460,37 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>>  	dsi->pll_clk = devm_clk_get(&pdev->dev, "pll_clk");
>>>  	if (IS_ERR(dsi->pll_clk)) {
>>>  		dev_info(&pdev->dev, "failed to get dsi pll input clock\n");
>>> -		return -EPROBE_DEFER;
>>> +		ret = PTR_ERR(dsi->pll_clk);
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	dsi->bus_clk = devm_clk_get(&pdev->dev, "bus_clk");
>>>  	if (IS_ERR(dsi->bus_clk)) {
>>>  		dev_info(&pdev->dev, "failed to get dsi bus clock\n");
>>> -		return -EPROBE_DEFER;
>>> +		ret = PTR_ERR(dsi->bus_clk);
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>  	dsi->reg_base = devm_ioremap_resource(&pdev->dev, res);
>>>  	if (IS_ERR(dsi->reg_base)) {
>>>  		dev_err(&pdev->dev, "failed to remap io region\n");
>>> -		return PTR_ERR(dsi->reg_base);
>>> +		ret = PTR_ERR(dsi->reg_base);
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	dsi->phy = devm_phy_get(&pdev->dev, "dsim");
>>>  	if (IS_ERR(dsi->phy)) {
>>>  		dev_info(&pdev->dev, "failed to get dsim phy\n");
>>> -		return -EPROBE_DEFER;
>>> +		ret = PTR_ERR(dsi->phy);
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	dsi->irq = platform_get_irq(pdev, 0);
>>>  	if (dsi->irq < 0) {
>>>  		dev_err(&pdev->dev, "failed to request dsi irq resource\n");
>>> -		return dsi->irq;
>>> +		ret = dsi->irq;
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	irq_set_status_flags(dsi->irq, IRQ_NOAUTOEN);
>>> @@ -1488,19 +1499,29 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>>>  					dev_name(&pdev->dev), dsi);
>>>  	if (ret) {
>>>  		dev_err(&pdev->dev, "failed to request dsi irq\n");
>>> -		return ret;
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	exynos_dsi_display.ctx = dsi;
>>>  
>>>  	platform_set_drvdata(pdev, &exynos_dsi_display);
>>>  
>>> -	return exynos_drm_component_add(&pdev->dev, &exynos_dsi_component_ops);
>>> +	ret = component_add(&pdev->dev, &exynos_dsi_component_ops);
>>> +	if (ret)
>>> +		goto err_del_component;
>>> +
>>> +	return ret;
>>> +
>>> +err_del_component:
>>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>>> +	return ret;
>>>  }
>>>  
>>>  static int exynos_dsi_remove(struct platform_device *pdev)
>>>  {
>>> -	exynos_drm_component_del(&pdev->dev, &exynos_dsi_component_ops);
>>> +	component_del(&pdev->dev, &exynos_dsi_component_ops);
>>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> index bd30d0c..47a772d 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> @@ -941,12 +941,21 @@ static int fimd_probe(struct platform_device *pdev)
>>>  	struct resource *res;
>>>  	int ret = -EINVAL;
>>>  
>>> -	if (!dev->of_node)
>>> -		return -ENODEV;
>>> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
>>> +					fimd_manager.type);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	if (!dev->of_node) {
>>> +		ret = -ENODEV;
>>> +		goto err_del_component;
>>> +	}
>>>  
>>>  	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>>> -	if (!ctx)
>>> -		return -ENOMEM;
>>> +	if (!ctx) {
>>> +		ret = -ENOMEM;
>>> +		goto err_del_component;
>>> +	}
>>>  
>>>  	ctx->dev = dev;
>>>  	ctx->suspended = true;
>>> @@ -959,32 +968,37 @@ static int fimd_probe(struct platform_device *pdev)
>>>  	ctx->bus_clk = devm_clk_get(dev, "fimd");
>>>  	if (IS_ERR(ctx->bus_clk)) {
>>>  		dev_err(dev, "failed to get bus clock\n");
>>> -		return PTR_ERR(ctx->bus_clk);
>>> +		ret = PTR_ERR(ctx->bus_clk);
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	ctx->lcd_clk = devm_clk_get(dev, "sclk_fimd");
>>>  	if (IS_ERR(ctx->lcd_clk)) {
>>>  		dev_err(dev, "failed to get lcd clock\n");
>>> -		return PTR_ERR(ctx->lcd_clk);
>>> +		ret = PTR_ERR(ctx->lcd_clk);
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>  
>>>  	ctx->regs = devm_ioremap_resource(dev, res);
>>> -	if (IS_ERR(ctx->regs))
>>> -		return PTR_ERR(ctx->regs);
>>> +	if (IS_ERR(ctx->regs)) {
>>> +		ret = PTR_ERR(ctx->regs);
>>> +		goto err_del_component;
>>> +	}
>>>  
>>>  	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "vsync");
>>>  	if (!res) {
>>>  		dev_err(dev, "irq request failed.\n");
>>> -		return -ENXIO;
>>> +		ret = -ENXIO;
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	ret = devm_request_irq(dev, res->start, fimd_irq_handler,
>>>  							0, "drm_fimd", ctx);
>>>  	if (ret) {
>>>  		dev_err(dev, "irq request failed.\n");
>>> -		return ret;
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	ctx->driver_data = drm_fimd_get_driver_data(pdev);
>>> @@ -1001,14 +1015,27 @@ static int fimd_probe(struct platform_device *pdev)
>>>  
>>>  	pm_runtime_enable(&pdev->dev);
>>>  
>>> -	return exynos_drm_component_add(&pdev->dev, &fimd_component_ops);
>>> +	ret = component_add(&pdev->dev, &fimd_component_ops);
>>> +	if (ret)
>>> +		goto err_disable_pm_runtime;
>>> +
>>> +	return ret;
>>> +
>>> +err_disable_pm_runtime:
>>> +	pm_runtime_disable(&pdev->dev);
>>> +
>>> +err_del_component:
>>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
>>> +	return ret;
>>>  }
>>>  
>>>  static int fimd_remove(struct platform_device *pdev)
>>>  {
>>>  	pm_runtime_disable(&pdev->dev);
>>>  
>>> -	exynos_drm_component_del(&pdev->dev, &fimd_component_ops);
>>> +	component_del(&pdev->dev, &fimd_component_ops);
>>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> index e05c86a..0c3bcee 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> @@ -2134,26 +2134,31 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>>  	res->hdmi = devm_clk_get(dev, "hdmi");
>>>  	if (IS_ERR(res->hdmi)) {
>>>  		DRM_ERROR("failed to get clock 'hdmi'\n");
>>> +		ret = PTR_ERR(res->hdmi);
>>>  		goto fail;
>>>  	}
>>>  	res->sclk_hdmi = devm_clk_get(dev, "sclk_hdmi");
>>>  	if (IS_ERR(res->sclk_hdmi)) {
>>>  		DRM_ERROR("failed to get clock 'sclk_hdmi'\n");
>>> +		ret = PTR_ERR(res->sclk_hdmi);
>>>  		goto fail;
>>>  	}
>>>  	res->sclk_pixel = devm_clk_get(dev, "sclk_pixel");
>>>  	if (IS_ERR(res->sclk_pixel)) {
>>>  		DRM_ERROR("failed to get clock 'sclk_pixel'\n");
>>> +		ret = PTR_ERR(res->sclk_pixel);
>>>  		goto fail;
>>>  	}
>>>  	res->sclk_hdmiphy = devm_clk_get(dev, "sclk_hdmiphy");
>>>  	if (IS_ERR(res->sclk_hdmiphy)) {
>>>  		DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n");
>>> +		ret = PTR_ERR(res->sclk_hdmiphy);
>>>  		goto fail;
>>>  	}
>>>  	res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
>>>  	if (IS_ERR(res->mout_hdmi)) {
>>>  		DRM_ERROR("failed to get clock 'mout_hdmi'\n");
>>> +		ret = PTR_ERR(res->mout_hdmi);
>>>  		goto fail;
>>>  	}
>>>  
>>> @@ -2161,8 +2166,10 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>>  
>>>  	res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
>>>  		sizeof(res->regul_bulk[0]), GFP_KERNEL);
>>> -	if (!res->regul_bulk)
>>> +	if (!res->regul_bulk) {
>>> +		ret = -ENOMEM;
>>>  		goto fail;
>>> +	}
>>>  	for (i = 0; i < ARRAY_SIZE(supply); ++i) {
>>>  		res->regul_bulk[i].supply = supply[i];
>>>  		res->regul_bulk[i].consumer = NULL;
>>> @@ -2170,14 +2177,14 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>>  	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(supply), res->regul_bulk);
>>>  	if (ret) {
>>>  		DRM_ERROR("failed to get regulators\n");
>>> -		goto fail;
>>> +		return ret;
>>>  	}
>>>  	res->regul_count = ARRAY_SIZE(supply);
>>>  
>>> -	return 0;
>>> +	return ret;
>>>  fail:
>>>  	DRM_ERROR("HDMI resource init - failed\n");
>>> -	return -ENODEV;
>>> +	return ret;
>>>  }
>>>  
>>>  static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata
>>> @@ -2275,24 +2282,37 @@ static int hdmi_probe(struct platform_device *pdev)
>>>  	struct resource *res;
>>>  	int ret;
>>>  
>>> -	if (!dev->of_node)
>>> -		return -ENODEV;
>>> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
>>> +					hdmi_display.type);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	if (!dev->of_node) {
>>> +		ret = -ENODEV;
>>> +		goto err_del_component;
>>> +	}
>>>  
>>>  	pdata = drm_hdmi_dt_parse_pdata(dev);
>>> -	if (!pdata)
>>> -		return -EINVAL;
>>> +	if (!pdata) {
>>> +		ret = -EINVAL;
>>> +		goto err_del_component;
>>> +	}
>>>  
>>>  	hdata = devm_kzalloc(dev, sizeof(struct hdmi_context), GFP_KERNEL);
>>> -	if (!hdata)
>>> -		return -ENOMEM;
>>> +	if (!hdata) {
>>> +		ret = -ENOMEM;
>>> +		goto err_del_component;
>>> +	}
>>>  
>>>  	mutex_init(&hdata->hdmi_mutex);
>>>  
>>>  	platform_set_drvdata(pdev, &hdmi_display);
>>>  
>>>  	match = of_match_node(hdmi_match_types, dev->of_node);
>>> -	if (!match)
>>> -		return -ENODEV;
>>> +	if (!match) {
>>> +		ret = -ENODEV;
>>> +		goto err_del_component;
>>> +	}
>>>  
>>>  	drv_data = (struct hdmi_driver_data *)match->data;
>>>  	hdata->type = drv_data->type;
>>> @@ -2305,18 +2325,20 @@ static int hdmi_probe(struct platform_device *pdev)
>>>  	ret = hdmi_resources_init(hdata);
>>>  	if (ret) {
>>>  		DRM_ERROR("hdmi_resources_init failed\n");
>>> -		return -EINVAL;
>>> +		return ret;
>>>  	}
>>>  
>>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>  	hdata->regs = devm_ioremap_resource(dev, res);
>>> -	if (IS_ERR(hdata->regs))
>>> -		return PTR_ERR(hdata->regs);
>>> +	if (IS_ERR(hdata->regs)) {
>>> +		ret = PTR_ERR(hdata->regs);
>>> +		goto err_del_component;
>>> +	}
>>>  
>>>  	ret = devm_gpio_request(dev, hdata->hpd_gpio, "HPD");
>>>  	if (ret) {
>>>  		DRM_ERROR("failed to request HPD gpio\n");
>>> -		return ret;
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  	ddc_node = hdmi_legacy_ddc_dt_binding(dev);
>>> @@ -2327,14 +2349,15 @@ static int hdmi_probe(struct platform_device *pdev)
>>>  	ddc_node = of_parse_phandle(dev->of_node, "ddc", 0);
>>>  	if (!ddc_node) {
>>>  		DRM_ERROR("Failed to find ddc node in device tree\n");
>>> -		return -ENODEV;
>>> +		ret = -ENODEV;
>>> +		goto err_del_component;
>>>  	}
>>>  
>>>  out_get_ddc_adpt:
>>>  	hdata->ddc_adpt = of_find_i2c_adapter_by_node(ddc_node);
>>>  	if (!hdata->ddc_adpt) {
>>>  		DRM_ERROR("Failed to get ddc i2c adapter by node\n");
>>> -		return -ENODEV;
>>> +		return -EPROBE_DEFER;
>>>  	}
>>>  
>>>  	phy_node = hdmi_legacy_phy_dt_binding(dev);
>>> @@ -2361,7 +2384,7 @@ out_get_phy_port:
>>>  		hdata->hdmiphy_port = of_find_i2c_device_by_node(phy_node);
>>>  		if (!hdata->hdmiphy_port) {
>>>  			DRM_ERROR("Failed to get hdmi phy i2c client\n");
>>> -			ret = -ENODEV;
>>> +			ret = -EPROBE_DEFER;
>>>  			goto err_ddc;
>>>  		}
>>>  	}
>>> @@ -2390,19 +2413,31 @@ out_get_phy_port:
>>>  			"samsung,syscon-phandle");
>>>  	if (IS_ERR(hdata->pmureg)) {
>>>  		DRM_ERROR("syscon regmap lookup failed.\n");
>>> +		ret = -EPROBE_DEFER;
>>>  		goto err_hdmiphy;
>>>  	}
>>>  
>>>  	pm_runtime_enable(dev);
>>>  	hdmi_display.ctx = hdata;
>>>  
>>> -	return exynos_drm_component_add(&pdev->dev, &hdmi_component_ops);
>>> +	ret = component_add(&pdev->dev, &hdmi_component_ops);
>>> +	if (ret)
>>> +		goto err_disable_pm_runtime;
>>> +
>>> +	return ret;
>>> +
>>> +err_disable_pm_runtime:
>>> +	pm_runtime_disable(dev);
>>>  
>>>  err_hdmiphy:
>>>  	if (hdata->hdmiphy_port)
>>>  		put_device(&hdata->hdmiphy_port->dev);
>>>  err_ddc:
>>>  	put_device(&hdata->ddc_adpt->dev);
>>> +
>>> +err_del_component:
>>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>>> +
>>>  	return ret;
>>>  }
>>>  
>>> @@ -2416,8 +2451,9 @@ static int hdmi_remove(struct platform_device *pdev)
>>>  	put_device(&hdata->ddc_adpt->dev);
>>>  
>>>  	pm_runtime_disable(&pdev->dev);
>>> +	component_del(&pdev->dev, &hdmi_component_ops);
>>>  
>>> -	exynos_drm_component_del(&pdev->dev, &hdmi_component_ops);
>>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> index 483d7c0..4c5aed7 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> @@ -1273,12 +1273,25 @@ static const struct component_ops mixer_component_ops = {
>>>  
>>>  static int mixer_probe(struct platform_device *pdev)
>>>  {
>>> -	return exynos_drm_component_add(&pdev->dev, &mixer_component_ops);
>>> +	int ret;
>>> +
>>> +	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
>>> +					mixer_manager.type);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = component_add(&pdev->dev, &mixer_component_ops);
>>> +	if (ret)
>>> +		exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
>>> +
>>> +	return ret;
>>>  }
>>>  
>>>  static int mixer_remove(struct platform_device *pdev)
>>>  {
>>> -	exynos_drm_component_del(&pdev->dev, &mixer_component_ops);
>>> +	component_del(&pdev->dev, &mixer_component_ops);
>>> +	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
>>> +
>>>  	return 0;
>>>  }
>>>  
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to [e-mail]
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index ff63901..a892586 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1328,12 +1328,26 @@  static const struct component_ops exynos_dp_ops = {
 
 static int exynos_dp_probe(struct platform_device *pdev)
 {
-	return exynos_drm_component_add(&pdev->dev, &exynos_dp_ops);
+	int ret;
+
+	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
+					exynos_dp_display.type);
+	if (ret)
+		return ret;
+
+	ret = component_add(&pdev->dev, &exynos_dp_ops);
+	if (ret)
+		exynos_drm_component_del(&pdev->dev,
+						EXYNOS_DEVICE_TYPE_CONNECTOR);
+
+	return ret;
 }
 
 static int exynos_dp_remove(struct platform_device *pdev)
 {
-	exynos_drm_component_del(&pdev->dev, &exynos_dp_ops);
+	component_del(&pdev->dev, &exynos_dp_ops);
+	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index a832364..f1b8587 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -295,9 +295,15 @@  struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
 	struct exynos_dpi *ctx;
 	int ret;
 
+	ret = exynos_drm_component_add(dev,
+					EXYNOS_DEVICE_TYPE_CONNECTOR,
+					exynos_dpi_display.type);
+	if (ret)
+		return ERR_PTR(ret);
+
 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
-		return NULL;
+		goto err_del_component;
 
 	ctx->dev = dev;
 	exynos_dpi_display.ctx = ctx;
@@ -306,16 +312,24 @@  struct exynos_drm_display *exynos_dpi_probe(struct device *dev)
 	ret = exynos_dpi_parse_dt(ctx);
 	if (ret < 0) {
 		devm_kfree(dev, ctx);
-		return NULL;
+		goto err_del_component;
 	}
 
 	if (ctx->panel_node) {
 		ctx->panel = of_drm_find_panel(ctx->panel_node);
-		if (!ctx->panel)
+		if (!ctx->panel) {
+			exynos_drm_component_del(dev,
+						EXYNOS_DEVICE_TYPE_CONNECTOR);
 			return ERR_PTR(-EPROBE_DEFER);
+		}
 	}
 
 	return &exynos_dpi_display;
+
+err_del_component:
+	exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
+
+	return NULL;
 }
 
 int exynos_dpi_remove(struct device *dev)
@@ -327,5 +341,7 @@  int exynos_dpi_remove(struct device *dev)
 	encoder->funcs->destroy(encoder);
 	drm_connector_cleanup(&ctx->connector);
 
+	exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index c5a401ae..72a5de1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -48,7 +48,10 @@  static LIST_HEAD(drm_component_list);
 
 struct component_dev {
 	struct list_head list;
-	struct device *dev;
+	struct device *crtc_dev;
+	struct device *conn_dev;
+	enum exynos_drm_output_type out_type;
+	unsigned int dev_type_flag;
 };
 
 static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
@@ -382,22 +385,65 @@  static const struct dev_pm_ops exynos_drm_pm_ops = {
 };
 
 int exynos_drm_component_add(struct device *dev,
-				const struct component_ops *ops)
+				enum exynos_drm_device_type dev_type,
+				enum exynos_drm_output_type out_type)
 {
 	struct component_dev *cdev;
-	int ret;
+
+	if (dev_type != EXYNOS_DEVICE_TYPE_CRTC &&
+			dev_type != EXYNOS_DEVICE_TYPE_CONNECTOR) {
+		DRM_ERROR("invalid device type.\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&drm_component_lock);
+
+	/*
+	 * Make sure to check if there is a component which has two device
+	 * objects, for connector and for encoder/connector.
+	 * It should make sure that crtc and encoder/connector drivers are
+	 * ready before exynos drm core binds them.
+	 */
+	list_for_each_entry(cdev, &drm_component_list, list) {
+		if (cdev->out_type == out_type) {
+			/*
+			 * If crtc and encoder/connector device objects are
+			 * added already just return.
+			 */
+			if (cdev->dev_type_flag == (EXYNOS_DEVICE_TYPE_CRTC |
+						EXYNOS_DEVICE_TYPE_CONNECTOR)) {
+				mutex_unlock(&drm_component_lock);
+				return 0;
+			}
+
+			if (dev_type == EXYNOS_DEVICE_TYPE_CRTC) {
+				cdev->crtc_dev = dev;
+				cdev->dev_type_flag |= dev_type;
+			}
+
+			if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR) {
+				cdev->conn_dev = dev;
+				cdev->dev_type_flag |= dev_type;
+			}
+
+			mutex_unlock(&drm_component_lock);
+			return 0;
+		}
+	}
+
+	mutex_unlock(&drm_component_lock);
 
 	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
 	if (!cdev)
 		return -ENOMEM;
 
-	ret = component_add(dev, ops);
-	if (ret) {
-		kfree(cdev);
-		return ret;
-	}
+	if (dev_type == EXYNOS_DEVICE_TYPE_CRTC)
+		cdev->crtc_dev = dev;
+	if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR)
+		cdev->conn_dev = dev;
 
-	cdev->dev = dev;
+	cdev->out_type = out_type;
+	cdev->dev_type_flag = dev_type;
 
 	mutex_lock(&drm_component_lock);
 	list_add_tail(&cdev->list, &drm_component_list);
@@ -407,23 +453,40 @@  int exynos_drm_component_add(struct device *dev,
 }
 
 void exynos_drm_component_del(struct device *dev,
-				const struct component_ops *ops)
+				enum exynos_drm_device_type dev_type)
 {
 	struct component_dev *cdev, *next;
 
 	mutex_lock(&drm_component_lock);
 
 	list_for_each_entry_safe(cdev, next, &drm_component_list, list) {
-		if (dev == cdev->dev) {
+		if (dev_type == EXYNOS_DEVICE_TYPE_CRTC) {
+			if (cdev->crtc_dev == dev) {
+				cdev->crtc_dev = NULL;
+				cdev->dev_type_flag &= ~dev_type;
+			}
+		}
+
+		if (dev_type == EXYNOS_DEVICE_TYPE_CONNECTOR) {
+			if (cdev->conn_dev == dev) {
+				cdev->conn_dev = NULL;
+				cdev->dev_type_flag &= ~dev_type;
+			}
+		}
+
+		/*
+		 * Release cdev object only in case that both of crtc and
+		 * encoder/connector device objects are NULL.
+		 */
+		if (!cdev->crtc_dev && !cdev->conn_dev) {
 			list_del(&cdev->list);
 			kfree(cdev);
-			break;
 		}
+
+		break;
 	}
 
 	mutex_unlock(&drm_component_lock);
-
-	component_del(dev, ops);
 }
 
 static int compare_of(struct device *dev, void *data)
@@ -433,29 +496,61 @@  static int compare_of(struct device *dev, void *data)
 
 static int exynos_drm_add_components(struct device *dev, struct master *m)
 {
-	unsigned int attached_cnt = 0;
 	struct component_dev *cdev;
+	unsigned int attach_cnt = 0;
 
 	mutex_lock(&drm_component_lock);
 
 	list_for_each_entry(cdev, &drm_component_list, list) {
 		int ret;
 
+		/*
+		 * Add components to master only in case that crtc and
+		 * encoder/connector device objects exist.
+		 */
+		if (!cdev->crtc_dev || !cdev->conn_dev)
+			continue;
+
+		attach_cnt++;
+
 		mutex_unlock(&drm_component_lock);
 
-		ret = component_master_add_child(m, compare_of, cdev->dev);
-		if (!ret)
-			attached_cnt++;
+		/*
+		 * fimd and dpi modules have same device object so add
+		 * only crtc device object in this case.
+		 *
+		 * TODO. if dpi module follows driver-model driver then
+		 * below codes can be removed.
+		 */
+		if (cdev->crtc_dev == cdev->conn_dev) {
+			ret = component_master_add_child(m, compare_of,
+					cdev->crtc_dev);
+			if (ret < 0)
+				return ret;
+
+			goto out_lock;
+		}
+
 
+		/*
+		 * Do not chage below call order.
+		 * crtc device first should be added to master because
+		 * connector/encoder need pipe number of crtc when they
+		 * are created.
+		 */
+		ret = component_master_add_child(m, compare_of, cdev->crtc_dev);
+		ret |= component_master_add_child(m, compare_of,
+							cdev->conn_dev);
+		if (ret < 0)
+			return ret;
+
+out_lock:
 		mutex_lock(&drm_component_lock);
 	}
 
 	mutex_unlock(&drm_component_lock);
 
-	if (!attached_cnt)
-		return -ENXIO;
-
-	return 0;
+	return attach_cnt ? 0 : -ENODEV;
 }
 
 static int exynos_drm_bind(struct device *dev)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index e82e620..36535f3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -42,6 +42,13 @@  struct drm_connector;
 
 extern unsigned int drm_vblank_offdelay;
 
+/* This enumerates device type. */
+enum exynos_drm_device_type {
+	EXYNOS_DEVICE_TYPE_NONE,
+	EXYNOS_DEVICE_TYPE_CRTC,
+	EXYNOS_DEVICE_TYPE_CONNECTOR,
+};
+
 /* this enumerates display type. */
 enum exynos_drm_output_type {
 	EXYNOS_DISPLAY_TYPE_NONE,
@@ -354,12 +361,12 @@  void exynos_drm_remove_vidi(void);
 int exynos_drm_create_enc_conn(struct drm_device *dev,
 				struct exynos_drm_display *display);
 
-struct component_ops;
 int exynos_drm_component_add(struct device *dev,
-				const struct component_ops *ops);
+				enum exynos_drm_device_type dev_type,
+				enum exynos_drm_output_type out_type);
 
 void exynos_drm_component_del(struct device *dev,
-				const struct component_ops *ops);
+				enum exynos_drm_device_type dev_type);
 
 extern struct platform_driver fimd_driver;
 extern struct platform_driver dp_driver;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 84661fe..6302aa6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1423,10 +1423,16 @@  static int exynos_dsi_probe(struct platform_device *pdev)
 	struct exynos_dsi *dsi;
 	int ret;
 
+	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
+					exynos_dsi_display.type);
+	if (ret)
+		return ret;
+
 	dsi = devm_kzalloc(&pdev->dev, sizeof(*dsi), GFP_KERNEL);
 	if (!dsi) {
 		dev_err(&pdev->dev, "failed to allocate dsi object.\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_del_component;
 	}
 
 	init_completion(&dsi->completed);
@@ -1440,7 +1446,7 @@  static int exynos_dsi_probe(struct platform_device *pdev)
 
 	ret = exynos_dsi_parse_dt(dsi);
 	if (ret)
-		return ret;
+		goto err_del_component;
 
 	dsi->supplies[0].supply = "vddcore";
 	dsi->supplies[1].supply = "vddio";
@@ -1454,32 +1460,37 @@  static int exynos_dsi_probe(struct platform_device *pdev)
 	dsi->pll_clk = devm_clk_get(&pdev->dev, "pll_clk");
 	if (IS_ERR(dsi->pll_clk)) {
 		dev_info(&pdev->dev, "failed to get dsi pll input clock\n");
-		return -EPROBE_DEFER;
+		ret = PTR_ERR(dsi->pll_clk);
+		goto err_del_component;
 	}
 
 	dsi->bus_clk = devm_clk_get(&pdev->dev, "bus_clk");
 	if (IS_ERR(dsi->bus_clk)) {
 		dev_info(&pdev->dev, "failed to get dsi bus clock\n");
-		return -EPROBE_DEFER;
+		ret = PTR_ERR(dsi->bus_clk);
+		goto err_del_component;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	dsi->reg_base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(dsi->reg_base)) {
 		dev_err(&pdev->dev, "failed to remap io region\n");
-		return PTR_ERR(dsi->reg_base);
+		ret = PTR_ERR(dsi->reg_base);
+		goto err_del_component;
 	}
 
 	dsi->phy = devm_phy_get(&pdev->dev, "dsim");
 	if (IS_ERR(dsi->phy)) {
 		dev_info(&pdev->dev, "failed to get dsim phy\n");
-		return -EPROBE_DEFER;
+		ret = PTR_ERR(dsi->phy);
+		goto err_del_component;
 	}
 
 	dsi->irq = platform_get_irq(pdev, 0);
 	if (dsi->irq < 0) {
 		dev_err(&pdev->dev, "failed to request dsi irq resource\n");
-		return dsi->irq;
+		ret = dsi->irq;
+		goto err_del_component;
 	}
 
 	irq_set_status_flags(dsi->irq, IRQ_NOAUTOEN);
@@ -1488,19 +1499,29 @@  static int exynos_dsi_probe(struct platform_device *pdev)
 					dev_name(&pdev->dev), dsi);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to request dsi irq\n");
-		return ret;
+		goto err_del_component;
 	}
 
 	exynos_dsi_display.ctx = dsi;
 
 	platform_set_drvdata(pdev, &exynos_dsi_display);
 
-	return exynos_drm_component_add(&pdev->dev, &exynos_dsi_component_ops);
+	ret = component_add(&pdev->dev, &exynos_dsi_component_ops);
+	if (ret)
+		goto err_del_component;
+
+	return ret;
+
+err_del_component:
+	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
+	return ret;
 }
 
 static int exynos_dsi_remove(struct platform_device *pdev)
 {
-	exynos_drm_component_del(&pdev->dev, &exynos_dsi_component_ops);
+	component_del(&pdev->dev, &exynos_dsi_component_ops);
+	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index bd30d0c..47a772d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -941,12 +941,21 @@  static int fimd_probe(struct platform_device *pdev)
 	struct resource *res;
 	int ret = -EINVAL;
 
-	if (!dev->of_node)
-		return -ENODEV;
+	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
+					fimd_manager.type);
+	if (ret)
+		return ret;
+
+	if (!dev->of_node) {
+		ret = -ENODEV;
+		goto err_del_component;
+	}
 
 	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
-	if (!ctx)
-		return -ENOMEM;
+	if (!ctx) {
+		ret = -ENOMEM;
+		goto err_del_component;
+	}
 
 	ctx->dev = dev;
 	ctx->suspended = true;
@@ -959,32 +968,37 @@  static int fimd_probe(struct platform_device *pdev)
 	ctx->bus_clk = devm_clk_get(dev, "fimd");
 	if (IS_ERR(ctx->bus_clk)) {
 		dev_err(dev, "failed to get bus clock\n");
-		return PTR_ERR(ctx->bus_clk);
+		ret = PTR_ERR(ctx->bus_clk);
+		goto err_del_component;
 	}
 
 	ctx->lcd_clk = devm_clk_get(dev, "sclk_fimd");
 	if (IS_ERR(ctx->lcd_clk)) {
 		dev_err(dev, "failed to get lcd clock\n");
-		return PTR_ERR(ctx->lcd_clk);
+		ret = PTR_ERR(ctx->lcd_clk);
+		goto err_del_component;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
 	ctx->regs = devm_ioremap_resource(dev, res);
-	if (IS_ERR(ctx->regs))
-		return PTR_ERR(ctx->regs);
+	if (IS_ERR(ctx->regs)) {
+		ret = PTR_ERR(ctx->regs);
+		goto err_del_component;
+	}
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "vsync");
 	if (!res) {
 		dev_err(dev, "irq request failed.\n");
-		return -ENXIO;
+		ret = -ENXIO;
+		goto err_del_component;
 	}
 
 	ret = devm_request_irq(dev, res->start, fimd_irq_handler,
 							0, "drm_fimd", ctx);
 	if (ret) {
 		dev_err(dev, "irq request failed.\n");
-		return ret;
+		goto err_del_component;
 	}
 
 	ctx->driver_data = drm_fimd_get_driver_data(pdev);
@@ -1001,14 +1015,27 @@  static int fimd_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(&pdev->dev);
 
-	return exynos_drm_component_add(&pdev->dev, &fimd_component_ops);
+	ret = component_add(&pdev->dev, &fimd_component_ops);
+	if (ret)
+		goto err_disable_pm_runtime;
+
+	return ret;
+
+err_disable_pm_runtime:
+	pm_runtime_disable(&pdev->dev);
+
+err_del_component:
+	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
+	return ret;
 }
 
 static int fimd_remove(struct platform_device *pdev)
 {
 	pm_runtime_disable(&pdev->dev);
 
-	exynos_drm_component_del(&pdev->dev, &fimd_component_ops);
+	component_del(&pdev->dev, &fimd_component_ops);
+	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index e05c86a..0c3bcee 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -2134,26 +2134,31 @@  static int hdmi_resources_init(struct hdmi_context *hdata)
 	res->hdmi = devm_clk_get(dev, "hdmi");
 	if (IS_ERR(res->hdmi)) {
 		DRM_ERROR("failed to get clock 'hdmi'\n");
+		ret = PTR_ERR(res->hdmi);
 		goto fail;
 	}
 	res->sclk_hdmi = devm_clk_get(dev, "sclk_hdmi");
 	if (IS_ERR(res->sclk_hdmi)) {
 		DRM_ERROR("failed to get clock 'sclk_hdmi'\n");
+		ret = PTR_ERR(res->sclk_hdmi);
 		goto fail;
 	}
 	res->sclk_pixel = devm_clk_get(dev, "sclk_pixel");
 	if (IS_ERR(res->sclk_pixel)) {
 		DRM_ERROR("failed to get clock 'sclk_pixel'\n");
+		ret = PTR_ERR(res->sclk_pixel);
 		goto fail;
 	}
 	res->sclk_hdmiphy = devm_clk_get(dev, "sclk_hdmiphy");
 	if (IS_ERR(res->sclk_hdmiphy)) {
 		DRM_ERROR("failed to get clock 'sclk_hdmiphy'\n");
+		ret = PTR_ERR(res->sclk_hdmiphy);
 		goto fail;
 	}
 	res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
 	if (IS_ERR(res->mout_hdmi)) {
 		DRM_ERROR("failed to get clock 'mout_hdmi'\n");
+		ret = PTR_ERR(res->mout_hdmi);
 		goto fail;
 	}
 
@@ -2161,8 +2166,10 @@  static int hdmi_resources_init(struct hdmi_context *hdata)
 
 	res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
 		sizeof(res->regul_bulk[0]), GFP_KERNEL);
-	if (!res->regul_bulk)
+	if (!res->regul_bulk) {
+		ret = -ENOMEM;
 		goto fail;
+	}
 	for (i = 0; i < ARRAY_SIZE(supply); ++i) {
 		res->regul_bulk[i].supply = supply[i];
 		res->regul_bulk[i].consumer = NULL;
@@ -2170,14 +2177,14 @@  static int hdmi_resources_init(struct hdmi_context *hdata)
 	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(supply), res->regul_bulk);
 	if (ret) {
 		DRM_ERROR("failed to get regulators\n");
-		goto fail;
+		return ret;
 	}
 	res->regul_count = ARRAY_SIZE(supply);
 
-	return 0;
+	return ret;
 fail:
 	DRM_ERROR("HDMI resource init - failed\n");
-	return -ENODEV;
+	return ret;
 }
 
 static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata
@@ -2275,24 +2282,37 @@  static int hdmi_probe(struct platform_device *pdev)
 	struct resource *res;
 	int ret;
 
-	if (!dev->of_node)
-		return -ENODEV;
+	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR,
+					hdmi_display.type);
+	if (ret)
+		return ret;
+
+	if (!dev->of_node) {
+		ret = -ENODEV;
+		goto err_del_component;
+	}
 
 	pdata = drm_hdmi_dt_parse_pdata(dev);
-	if (!pdata)
-		return -EINVAL;
+	if (!pdata) {
+		ret = -EINVAL;
+		goto err_del_component;
+	}
 
 	hdata = devm_kzalloc(dev, sizeof(struct hdmi_context), GFP_KERNEL);
-	if (!hdata)
-		return -ENOMEM;
+	if (!hdata) {
+		ret = -ENOMEM;
+		goto err_del_component;
+	}
 
 	mutex_init(&hdata->hdmi_mutex);
 
 	platform_set_drvdata(pdev, &hdmi_display);
 
 	match = of_match_node(hdmi_match_types, dev->of_node);
-	if (!match)
-		return -ENODEV;
+	if (!match) {
+		ret = -ENODEV;
+		goto err_del_component;
+	}
 
 	drv_data = (struct hdmi_driver_data *)match->data;
 	hdata->type = drv_data->type;
@@ -2305,18 +2325,20 @@  static int hdmi_probe(struct platform_device *pdev)
 	ret = hdmi_resources_init(hdata);
 	if (ret) {
 		DRM_ERROR("hdmi_resources_init failed\n");
-		return -EINVAL;
+		return ret;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hdata->regs = devm_ioremap_resource(dev, res);
-	if (IS_ERR(hdata->regs))
-		return PTR_ERR(hdata->regs);
+	if (IS_ERR(hdata->regs)) {
+		ret = PTR_ERR(hdata->regs);
+		goto err_del_component;
+	}
 
 	ret = devm_gpio_request(dev, hdata->hpd_gpio, "HPD");
 	if (ret) {
 		DRM_ERROR("failed to request HPD gpio\n");
-		return ret;
+		goto err_del_component;
 	}
 
 	ddc_node = hdmi_legacy_ddc_dt_binding(dev);
@@ -2327,14 +2349,15 @@  static int hdmi_probe(struct platform_device *pdev)
 	ddc_node = of_parse_phandle(dev->of_node, "ddc", 0);
 	if (!ddc_node) {
 		DRM_ERROR("Failed to find ddc node in device tree\n");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto err_del_component;
 	}
 
 out_get_ddc_adpt:
 	hdata->ddc_adpt = of_find_i2c_adapter_by_node(ddc_node);
 	if (!hdata->ddc_adpt) {
 		DRM_ERROR("Failed to get ddc i2c adapter by node\n");
-		return -ENODEV;
+		return -EPROBE_DEFER;
 	}
 
 	phy_node = hdmi_legacy_phy_dt_binding(dev);
@@ -2361,7 +2384,7 @@  out_get_phy_port:
 		hdata->hdmiphy_port = of_find_i2c_device_by_node(phy_node);
 		if (!hdata->hdmiphy_port) {
 			DRM_ERROR("Failed to get hdmi phy i2c client\n");
-			ret = -ENODEV;
+			ret = -EPROBE_DEFER;
 			goto err_ddc;
 		}
 	}
@@ -2390,19 +2413,31 @@  out_get_phy_port:
 			"samsung,syscon-phandle");
 	if (IS_ERR(hdata->pmureg)) {
 		DRM_ERROR("syscon regmap lookup failed.\n");
+		ret = -EPROBE_DEFER;
 		goto err_hdmiphy;
 	}
 
 	pm_runtime_enable(dev);
 	hdmi_display.ctx = hdata;
 
-	return exynos_drm_component_add(&pdev->dev, &hdmi_component_ops);
+	ret = component_add(&pdev->dev, &hdmi_component_ops);
+	if (ret)
+		goto err_disable_pm_runtime;
+
+	return ret;
+
+err_disable_pm_runtime:
+	pm_runtime_disable(dev);
 
 err_hdmiphy:
 	if (hdata->hdmiphy_port)
 		put_device(&hdata->hdmiphy_port->dev);
 err_ddc:
 	put_device(&hdata->ddc_adpt->dev);
+
+err_del_component:
+	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
+
 	return ret;
 }
 
@@ -2416,8 +2451,9 @@  static int hdmi_remove(struct platform_device *pdev)
 	put_device(&hdata->ddc_adpt->dev);
 
 	pm_runtime_disable(&pdev->dev);
+	component_del(&pdev->dev, &hdmi_component_ops);
 
-	exynos_drm_component_del(&pdev->dev, &hdmi_component_ops);
+	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 483d7c0..4c5aed7 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1273,12 +1273,25 @@  static const struct component_ops mixer_component_ops = {
 
 static int mixer_probe(struct platform_device *pdev)
 {
-	return exynos_drm_component_add(&pdev->dev, &mixer_component_ops);
+	int ret;
+
+	ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
+					mixer_manager.type);
+	if (ret)
+		return ret;
+
+	ret = component_add(&pdev->dev, &mixer_component_ops);
+	if (ret)
+		exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
+
+	return ret;
 }
 
 static int mixer_remove(struct platform_device *pdev)
 {
-	exynos_drm_component_del(&pdev->dev, &mixer_component_ops);
+	component_del(&pdev->dev, &mixer_component_ops);
+	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC);
+
 	return 0;
 }