Re: [PATCH v2 39/60] drm/omap: dss: dsi: Move initialization code from bind to probe
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
> ---
Reviewed-by: Sebastian Reichel
-- 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,
> +
[PATCH v2 39/60] drm/omap: dss: dsi: Move initialization code from bind to probe
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
---
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_r
