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 <inki.dae at samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park at samsung.com>
>> ---
>> 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 majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
