Hi, On Sat, May 26, 2018 at 08:24:57PM +0300, Laurent Pinchart wrote: > There's no reason to delay initialization of most of the driver (such as > mapping memory I/O or enabling runtime PM) to the component bind > handler. Perform as much of the initialization as possible at probe > time, initializing at bind time only the parts that depends on the DSS. > The cleanup code is moved from unbind to remove in a similar way. > > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > ---
Reviewed-by: Sebastian Reichel <sebastian.reic...@collabora.co.uk>
-- Sebastian
> drivers/gpu/drm/omapdrm/dss/dsi.c | 301
> ++++++++++++++++++++------------------
> 1 file changed, 161 insertions(+), 140 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c
> b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 278094f29255..79312e53bfd9 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -4981,85 +4981,9 @@ static const struct omap_dss_device_ops dsi_ops = {
> },
> };
>
> -static void dsi_init_output(struct dsi_data *dsi)
> -{
> - struct omap_dss_device *out = &dsi->output;
> -
> - out->dev = dsi->dev;
> - out->id = dsi->module_id == 0 ?
> - OMAP_DSS_OUTPUT_DSI1 : OMAP_DSS_OUTPUT_DSI2;
> -
> - out->output_type = OMAP_DISPLAY_TYPE_DSI;
> - out->name = dsi->module_id == 0 ? "dsi.0" : "dsi.1";
> - out->dispc_channel = dsi_get_channel(dsi);
> - out->ops = &dsi_ops;
> - out->owner = THIS_MODULE;
> - out->of_ports = BIT(0);
> -
> - omapdss_device_register(out);
> -}
> -
> -static void dsi_uninit_output(struct dsi_data *dsi)
> -{
> - struct omap_dss_device *out = &dsi->output;
> -
> - omapdss_device_unregister(out);
> -}
> -
> -static int dsi_probe_of(struct dsi_data *dsi)
> -{
> - struct device_node *node = dsi->dev->of_node;
> - struct property *prop;
> - u32 lane_arr[10];
> - int len, num_pins;
> - int r, i;
> - struct device_node *ep;
> - struct omap_dsi_pin_config pin_cfg;
> -
> - ep = of_graph_get_endpoint_by_regs(node, 0, 0);
> - if (!ep)
> - return 0;
> -
> - prop = of_find_property(ep, "lanes", &len);
> - if (prop == NULL) {
> - dev_err(dsi->dev, "failed to find lane data\n");
> - r = -EINVAL;
> - goto err;
> - }
> -
> - num_pins = len / sizeof(u32);
> -
> - if (num_pins < 4 || num_pins % 2 != 0 ||
> - num_pins > dsi->num_lanes_supported * 2) {
> - dev_err(dsi->dev, "bad number of lanes\n");
> - r = -EINVAL;
> - goto err;
> - }
> -
> - r = of_property_read_u32_array(ep, "lanes", lane_arr, num_pins);
> - if (r) {
> - dev_err(dsi->dev, "failed to read lane data\n");
> - goto err;
> - }
> -
> - pin_cfg.num_pins = num_pins;
> - for (i = 0; i < num_pins; ++i)
> - pin_cfg.pins[i] = (int)lane_arr[i];
> -
> - r = dsi_configure_pins(&dsi->output, &pin_cfg);
> - if (r) {
> - dev_err(dsi->dev, "failed to configure pins");
> - goto err;
> - }
> -
> - of_node_put(ep);
> -
> - return 0;
> -
> -err:
> - of_node_put(ep);
> - return r;
> -}
> +/*
> -----------------------------------------------------------------------------
> + * PLL
> + */
>
> static const struct dss_pll_ops dsi_pll_ops = {
> .enable = dsi_pll_enable,
> @@ -5174,7 +5098,153 @@ static int dsi_init_pll_data(struct dss_device *dss,
> struct dsi_data *dsi)
> return 0;
> }
>
> -/* DSI1 HW IP initialisation */
> +/*
> -----------------------------------------------------------------------------
> + * Component Bind & Unbind
> + */
> +
> +static int dsi_bind(struct device *dev, struct device *master, void *data)
> +{
> + struct dss_device *dss = dss_get_device(master);
> + struct dsi_data *dsi = dev_get_drvdata(dev);
> + char name[10];
> + u32 rev;
> + int r;
> +
> + dsi->dss = dss;
> +
> + dsi_init_pll_data(dss, dsi);
> +
> + r = dsi_runtime_get(dsi);
> + if (r)
> + return r;
> +
> + rev = dsi_read_reg(dsi, DSI_REVISION);
> + dev_dbg(dev, "OMAP DSI rev %d.%d\n",
> + FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
> +
> + dsi->line_buffer_size = dsi_get_line_buf_size(dsi);
> +
> + dsi_runtime_put(dsi);
> +
> + snprintf(name, sizeof(name), "dsi%u_regs", dsi->module_id + 1);
> + dsi->debugfs.regs = dss_debugfs_create_file(dss, name,
> + dsi_dump_dsi_regs, &dsi);
> +#ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
> + snprintf(name, sizeof(name), "dsi%u_irqs", dsi->module_id + 1);
> + dsi->debugfs.irqs = dss_debugfs_create_file(dss, name,
> + dsi_dump_dsi_irqs, &dsi);
> +#endif
> + snprintf(name, sizeof(name), "dsi%u_clks", dsi->module_id + 1);
> + dsi->debugfs.clks = dss_debugfs_create_file(dss, name,
> + dsi_dump_dsi_clocks, &dsi);
> +
> + return 0;
> +}
> +
> +static void dsi_unbind(struct device *dev, struct device *master, void *data)
> +{
> + struct dsi_data *dsi = dev_get_drvdata(dev);
> +
> + dss_debugfs_remove_file(dsi->debugfs.clks);
> + dss_debugfs_remove_file(dsi->debugfs.irqs);
> + dss_debugfs_remove_file(dsi->debugfs.regs);
> +
> + of_platform_depopulate(dev);
> +
> + WARN_ON(dsi->scp_clk_refcount > 0);
> +
> + dss_pll_unregister(&dsi->pll);
> +}
> +
> +static const struct component_ops dsi_component_ops = {
> + .bind = dsi_bind,
> + .unbind = dsi_unbind,
> +};
> +
> +/*
> -----------------------------------------------------------------------------
> + * Probe & Remove, Suspend & Resume
> + */
> +
> +static void dsi_init_output(struct dsi_data *dsi)
> +{
> + struct omap_dss_device *out = &dsi->output;
> +
> + out->dev = dsi->dev;
> + out->id = dsi->module_id == 0 ?
> + OMAP_DSS_OUTPUT_DSI1 : OMAP_DSS_OUTPUT_DSI2;
> +
> + out->output_type = OMAP_DISPLAY_TYPE_DSI;
> + out->name = dsi->module_id == 0 ? "dsi.0" : "dsi.1";
> + out->dispc_channel = dsi_get_channel(dsi);
> + out->ops = &dsi_ops;
> + out->owner = THIS_MODULE;
> + out->of_ports = BIT(0);
> +
> + omapdss_device_register(out);
> +}
> +
> +static void dsi_uninit_output(struct dsi_data *dsi)
> +{
> + struct omap_dss_device *out = &dsi->output;
> +
> + omapdss_device_unregister(out);
> +}
> +
> +static int dsi_probe_of(struct dsi_data *dsi)
> +{
> + struct device_node *node = dsi->dev->of_node;
> + struct property *prop;
> + u32 lane_arr[10];
> + int len, num_pins;
> + int r, i;
> + struct device_node *ep;
> + struct omap_dsi_pin_config pin_cfg;
> +
> + ep = of_graph_get_endpoint_by_regs(node, 0, 0);
> + if (!ep)
> + return 0;
> +
> + prop = of_find_property(ep, "lanes", &len);
> + if (prop == NULL) {
> + dev_err(dsi->dev, "failed to find lane data\n");
> + r = -EINVAL;
> + goto err;
> + }
> +
> + num_pins = len / sizeof(u32);
> +
> + if (num_pins < 4 || num_pins % 2 != 0 ||
> + num_pins > dsi->num_lanes_supported * 2) {
> + dev_err(dsi->dev, "bad number of lanes\n");
> + r = -EINVAL;
> + goto err;
> + }
> +
> + r = of_property_read_u32_array(ep, "lanes", lane_arr, num_pins);
> + if (r) {
> + dev_err(dsi->dev, "failed to read lane data\n");
> + goto err;
> + }
> +
> + pin_cfg.num_pins = num_pins;
> + for (i = 0; i < num_pins; ++i)
> + pin_cfg.pins[i] = (int)lane_arr[i];
> +
> + r = dsi_configure_pins(&dsi->output, &pin_cfg);
> + if (r) {
> + dev_err(dsi->dev, "failed to configure pins");
> + goto err;
> + }
> +
> + of_node_put(ep);
> +
> + return 0;
> +
> +err:
> + of_node_put(ep);
> + return r;
> +}
> +
> static const struct dsi_of_data dsi_of_data_omap34xx = {
> .model = DSI_MODEL_OMAP3,
> .pll_hw = &dss_omap3_dsi_pll_hw,
> @@ -5240,24 +5310,21 @@ static const struct soc_device_attribute
> dsi_soc_devices[] = {
> { /* sentinel */ }
> };
>
> -static int dsi_bind(struct device *dev, struct device *master, void *data)
> +static int dsi_probe(struct platform_device *pdev)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct dss_device *dss = dss_get_device(master);
> const struct soc_device_attribute *soc;
> const struct dsi_module_id_data *d;
> - u32 rev;
> - int r, i;
> + struct device *dev = &pdev->dev;
> struct dsi_data *dsi;
> struct resource *dsi_mem;
> struct resource *res;
> - char name[10];
> + unsigned int i;
> + int r;
>
> dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> if (!dsi)
> return -ENOMEM;
>
> - dsi->dss = dss;
> dsi->dev = dev;
> dev_set_drvdata(dev, dsi);
>
> @@ -5354,18 +5421,8 @@ static int dsi_bind(struct device *dev, struct device
> *master, void *data)
> if (r)
> return r;
>
> - dsi_init_pll_data(dss, dsi);
> -
> pm_runtime_enable(dev);
>
> - r = dsi_runtime_get(dsi);
> - if (r)
> - goto err_pm_disable;
> -
> - rev = dsi_read_reg(dsi, DSI_REVISION);
> - dev_dbg(dev, "OMAP DSI rev %d.%d\n",
> - FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
> -
> /* DSI on OMAP3 doesn't have register DSI_GNQ, set number
> * of data to 3 by default */
> if (dsi->data->quirks & DSI_QUIRK_GNQ)
> @@ -5374,8 +5431,6 @@ static int dsi_bind(struct device *dev, struct device
> *master, void *data)
> else
> dsi->num_lanes_supported = 3;
>
> - dsi->line_buffer_size = dsi_get_line_buf_size(dsi);
> -
> dsi_init_output(dsi);
>
> r = dsi_probe_of(dsi);
> @@ -5388,67 +5443,33 @@ static int dsi_bind(struct device *dev, struct device
> *master, void *data)
> if (r)
> DSSERR("Failed to populate DSI child devices: %d\n", r);
>
> - dsi_runtime_put(dsi);
> -
> - snprintf(name, sizeof(name), "dsi%u_regs", dsi->module_id + 1);
> - dsi->debugfs.regs = dss_debugfs_create_file(dss, name,
> - dsi_dump_dsi_regs, &dsi);
> -#ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
> - snprintf(name, sizeof(name), "dsi%u_irqs", dsi->module_id + 1);
> - dsi->debugfs.irqs = dss_debugfs_create_file(dss, name,
> - dsi_dump_dsi_irqs, &dsi);
> -#endif
> - snprintf(name, sizeof(name), "dsi%u_clks", dsi->module_id + 1);
> - dsi->debugfs.clks = dss_debugfs_create_file(dss, name,
> - dsi_dump_dsi_clocks, &dsi);
> + r = component_add(&pdev->dev, &dsi_component_ops);
> + if (r)
> + goto err_uninit_output;
>
> return 0;
>
> err_uninit_output:
> dsi_uninit_output(dsi);
> - dsi_runtime_put(dsi);
> -err_pm_disable:
> pm_runtime_disable(dev);
> return r;
> }
>
> -static void dsi_unbind(struct device *dev, struct device *master, void *data)
> +static int dsi_remove(struct platform_device *pdev)
> {
> - struct dsi_data *dsi = dev_get_drvdata(dev);
> + struct dsi_data *dsi = platform_get_drvdata(pdev);
>
> - dss_debugfs_remove_file(dsi->debugfs.clks);
> - dss_debugfs_remove_file(dsi->debugfs.irqs);
> - dss_debugfs_remove_file(dsi->debugfs.regs);
> -
> - of_platform_depopulate(dev);
> -
> - WARN_ON(dsi->scp_clk_refcount > 0);
> -
> - dss_pll_unregister(&dsi->pll);
> + component_del(&pdev->dev, &dsi_component_ops);
>
> dsi_uninit_output(dsi);
>
> - pm_runtime_disable(dev);
> + pm_runtime_disable(&pdev->dev);
>
> if (dsi->vdds_dsi_reg != NULL && dsi->vdds_dsi_enabled) {
> regulator_disable(dsi->vdds_dsi_reg);
> dsi->vdds_dsi_enabled = false;
> }
> -}
>
> -static const struct component_ops dsi_component_ops = {
> - .bind = dsi_bind,
> - .unbind = dsi_unbind,
> -};
> -
> -static int dsi_probe(struct platform_device *pdev)
> -{
> - return component_add(&pdev->dev, &dsi_component_ops);
> -}
> -
> -static int dsi_remove(struct platform_device *pdev)
> -{
> - component_del(&pdev->dev, &dsi_component_ops);
> return 0;
> }
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
