| Message ID | 1401355682-6939-1-git-send-email-inki.dae@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 <inki.dae@samsung.com> > Acked-by: Kyungmin Park <kyungmin.park@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??? > + > 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; > } > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <inki.dae@samsung.com> > Acked-by: Kyungmin Park <kyungmin.park@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??? > + > 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; > } > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@samsung.com> >> Acked-by: Kyungmin Park <kyungmin.park@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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <inki.dae@samsung.com> >>> Acked-by: Kyungmin Park <kyungmin.park@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. 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 majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }