| Message ID | 20180526172518.18710-41-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Sat, May 26, 2018 at 08:24:58PM +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.pinchart@ideasonboard.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> -- Sebastian > drivers/gpu/drm/omapdrm/dss/hdmi4.c | 218 +++++++++++++++++++----------------- > 1 file changed, 117 insertions(+), 101 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c > index 1d1f2e0b2b2a..89fdce02278c 100644 > --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c > +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c > @@ -553,53 +553,10 @@ static const struct omap_dss_device_ops hdmi_ops = { > }, > }; > > -static void hdmi_init_output(struct omap_hdmi *hdmi) > -{ > - struct omap_dss_device *out = &hdmi->output; > - > - out->dev = &hdmi->pdev->dev; > - out->id = OMAP_DSS_OUTPUT_HDMI; > - out->output_type = OMAP_DISPLAY_TYPE_HDMI; > - out->name = "hdmi.0"; > - out->dispc_channel = OMAP_DSS_CHANNEL_DIGIT; > - out->ops = &hdmi_ops; > - out->owner = THIS_MODULE; > - out->of_ports = BIT(0); > - > - omapdss_device_register(out); > -} > - > -static void hdmi_uninit_output(struct omap_hdmi *hdmi) > -{ > - struct omap_dss_device *out = &hdmi->output; > - > - omapdss_device_unregister(out); > -} > - > -static int hdmi_probe_of(struct omap_hdmi *hdmi) > -{ > - struct platform_device *pdev = hdmi->pdev; > - struct device_node *node = pdev->dev.of_node; > - struct device_node *ep; > - int r; > - > - ep = of_graph_get_endpoint_by_regs(node, 0, 0); > - if (!ep) > - return 0; > - > - r = hdmi_parse_lanes_of(pdev, ep, &hdmi->phy); > - if (r) > - goto err; > - > - of_node_put(ep); > - return 0; > - > -err: > - of_node_put(ep); > - return r; > -} > +/* ----------------------------------------------------------------------------- > + * Audio Callbacks > + */ > > -/* Audio callbacks */ > static int hdmi_audio_startup(struct device *dev, > void (*abort_cb)(struct device *dev)) > { > @@ -714,27 +671,123 @@ static int hdmi_audio_register(struct omap_hdmi *hdmi) > return 0; > } > > -/* HDMI HW IP initialisation */ > +/* ----------------------------------------------------------------------------- > + * Component Bind & Unbind > + */ > + > static int hdmi4_bind(struct device *dev, struct device *master, void *data) > { > - struct platform_device *pdev = to_platform_device(dev); > struct dss_device *dss = dss_get_device(master); > - struct omap_hdmi *hdmi; > + struct omap_hdmi *hdmi = dev_get_drvdata(dev); > int r; > + > + hdmi->dss = dss; > + > + r = hdmi_pll_init(dss, hdmi->pdev, &hdmi->pll, &hdmi->wp); > + if (r) > + return r; > + > + r = hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp); > + if (r) > + goto err_pll_uninit; > + > + r = hdmi_audio_register(hdmi); > + if (r) { > + DSSERR("Registering HDMI audio failed\n"); > + goto err_cec_uninit; > + } > + > + hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs, > + hdmi); > + > + return 0; > + > +err_cec_uninit: > + hdmi4_cec_uninit(&hdmi->core); > +err_pll_uninit: > + hdmi_pll_uninit(&hdmi->pll); > + return r; > +} > + > +static void hdmi4_unbind(struct device *dev, struct device *master, void *data) > +{ > + struct omap_hdmi *hdmi = dev_get_drvdata(dev); > + > + dss_debugfs_remove_file(hdmi->debugfs); > + > + if (hdmi->audio_pdev) > + platform_device_unregister(hdmi->audio_pdev); > + > + hdmi4_cec_uninit(&hdmi->core); > + hdmi_pll_uninit(&hdmi->pll); > +} > + > +static const struct component_ops hdmi4_component_ops = { > + .bind = hdmi4_bind, > + .unbind = hdmi4_unbind, > +}; > + > +/* ----------------------------------------------------------------------------- > + * Probe & Remove, Suspend & Resume > + */ > + > +static void hdmi4_init_output(struct omap_hdmi *hdmi) > +{ > + struct omap_dss_device *out = &hdmi->output; > + > + out->dev = &hdmi->pdev->dev; > + out->id = OMAP_DSS_OUTPUT_HDMI; > + out->output_type = OMAP_DISPLAY_TYPE_HDMI; > + out->name = "hdmi.0"; > + out->dispc_channel = OMAP_DSS_CHANNEL_DIGIT; > + out->ops = &hdmi_ops; > + out->owner = THIS_MODULE; > + out->of_ports = BIT(0); > + > + omapdss_device_register(out); > +} > + > +static void hdmi4_uninit_output(struct omap_hdmi *hdmi) > +{ > + struct omap_dss_device *out = &hdmi->output; > + > + omapdss_device_unregister(out); > +} > + > +static int hdmi4_probe_of(struct omap_hdmi *hdmi) > +{ > + struct platform_device *pdev = hdmi->pdev; > + struct device_node *node = pdev->dev.of_node; > + struct device_node *ep; > + int r; > + > + ep = of_graph_get_endpoint_by_regs(node, 0, 0); > + if (!ep) > + return 0; > + > + r = hdmi_parse_lanes_of(pdev, ep, &hdmi->phy); > + of_node_put(ep); > + return r; > +} > + > +static int hdmi4_probe(struct platform_device *pdev) > +{ > + struct omap_hdmi *hdmi; > int irq; > + int r; > > hdmi = kzalloc(sizeof(*hdmi), GFP_KERNEL); > if (!hdmi) > return -ENOMEM; > > hdmi->pdev = pdev; > - hdmi->dss = dss; > + > dev_set_drvdata(&pdev->dev, hdmi); > > mutex_init(&hdmi->lock); > spin_lock_init(&hdmi->audio_playing_lock); > > - r = hdmi_probe_of(hdmi); > + r = hdmi4_probe_of(hdmi); > if (r) > goto err_free; > > @@ -742,27 +795,19 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data) > if (r) > goto err_free; > > - r = hdmi_pll_init(dss, pdev, &hdmi->pll, &hdmi->wp); > - if (r) > - goto err_free; > - > r = hdmi_phy_init(pdev, &hdmi->phy, 4); > if (r) > - goto err_pll; > + goto err_free; > > r = hdmi4_core_init(pdev, &hdmi->core); > if (r) > - goto err_pll; > - > - r = hdmi4_cec_init(pdev, &hdmi->core, &hdmi->wp); > - if (r) > - goto err_pll; > + goto err_free; > > irq = platform_get_irq(pdev, 0); > if (irq < 0) { > DSSERR("platform_get_irq failed\n"); > r = -ENODEV; > - goto err_pll; > + goto err_free; > } > > r = devm_request_threaded_irq(&pdev->dev, irq, > @@ -770,67 +815,38 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data) > IRQF_ONESHOT, "OMAP HDMI", hdmi); > if (r) { > DSSERR("HDMI IRQ request failed\n"); > - goto err_pll; > + goto err_free; > } > > pm_runtime_enable(&pdev->dev); > > - hdmi_init_output(hdmi); > + hdmi4_init_output(hdmi); > > - r = hdmi_audio_register(hdmi); > - if (r) { > - DSSERR("Registering HDMI audio failed\n"); > + r = component_add(&pdev->dev, &hdmi4_component_ops); > + if (r) > goto err_uninit_output; > - } > - > - hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs, > - hdmi); > > return 0; > > err_uninit_output: > - hdmi_uninit_output(hdmi); > + hdmi4_uninit_output(hdmi); > pm_runtime_disable(&pdev->dev); > -err_pll: > - hdmi_pll_uninit(&hdmi->pll); > err_free: > kfree(hdmi); > return r; > } > > -static void hdmi4_unbind(struct device *dev, struct device *master, void *data) > +static int hdmi4_remove(struct platform_device *pdev) > { > - struct omap_hdmi *hdmi = dev_get_drvdata(dev); > + struct omap_hdmi *hdmi = platform_get_drvdata(pdev); > > - dss_debugfs_remove_file(hdmi->debugfs); > - > - if (hdmi->audio_pdev) > - platform_device_unregister(hdmi->audio_pdev); > - > - hdmi_uninit_output(hdmi); > - > - hdmi4_cec_uninit(&hdmi->core); > + component_del(&pdev->dev, &hdmi4_component_ops); > > - hdmi_pll_uninit(&hdmi->pll); > + hdmi4_uninit_output(hdmi); > > - pm_runtime_disable(dev); > + pm_runtime_disable(&pdev->dev); > > kfree(hdmi); > -} > - > -static const struct component_ops hdmi4_component_ops = { > - .bind = hdmi4_bind, > - .unbind = hdmi4_unbind, > -}; > - > -static int hdmi4_probe(struct platform_device *pdev) > -{ > - return component_add(&pdev->dev, &hdmi4_component_ops); > -} > - > -static int hdmi4_remove(struct platform_device *pdev) > -{ > - component_del(&pdev->dev, &hdmi4_component_ops); > return 0; > } > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c index 1d1f2e0b2b2a..89fdce02278c 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c @@ -553,53 +553,10 @@ static const struct omap_dss_device_ops hdmi_ops = { }, }; -static void hdmi_init_output(struct omap_hdmi *hdmi) -{ - struct omap_dss_device *out = &hdmi->output; - - out->dev = &hdmi->pdev->dev; - out->id = OMAP_DSS_OUTPUT_HDMI; - out->output_type = OMAP_DISPLAY_TYPE_HDMI; - out->name = "hdmi.0"; - out->dispc_channel = OMAP_DSS_CHANNEL_DIGIT; - out->ops = &hdmi_ops; - out->owner = THIS_MODULE; - out->of_ports = BIT(0); - - omapdss_device_register(out); -} - -static void hdmi_uninit_output(struct omap_hdmi *hdmi) -{ - struct omap_dss_device *out = &hdmi->output; - - omapdss_device_unregister(out); -} - -static int hdmi_probe_of(struct omap_hdmi *hdmi) -{ - struct platform_device *pdev = hdmi->pdev; - struct device_node *node = pdev->dev.of_node; - struct device_node *ep; - int r; - - ep = of_graph_get_endpoint_by_regs(node, 0, 0); - if (!ep) - return 0; - - r = hdmi_parse_lanes_of(pdev, ep, &hdmi->phy); - if (r) - goto err; - - of_node_put(ep); - return 0; - -err: - of_node_put(ep); - return r; -} +/* ----------------------------------------------------------------------------- + * Audio Callbacks + */ -/* Audio callbacks */ static int hdmi_audio_startup(struct device *dev, void (*abort_cb)(struct device *dev)) { @@ -714,27 +671,123 @@ static int hdmi_audio_register(struct omap_hdmi *hdmi) return 0; } -/* HDMI HW IP initialisation */ +/* ----------------------------------------------------------------------------- + * Component Bind & Unbind + */ + static int hdmi4_bind(struct device *dev, struct device *master, void *data) { - struct platform_device *pdev = to_platform_device(dev); struct dss_device *dss = dss_get_device(master); - struct omap_hdmi *hdmi; + struct omap_hdmi *hdmi = dev_get_drvdata(dev); int r; + + hdmi->dss = dss; + + r = hdmi_pll_init(dss, hdmi->pdev, &hdmi->pll, &hdmi->wp); + if (r) + return r; + + r = hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp); + if (r) + goto err_pll_uninit; + + r = hdmi_audio_register(hdmi); + if (r) { + DSSERR("Registering HDMI audio failed\n"); + goto err_cec_uninit; + } + + hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs, + hdmi); + + return 0; + +err_cec_uninit: + hdmi4_cec_uninit(&hdmi->core); +err_pll_uninit: + hdmi_pll_uninit(&hdmi->pll); + return r; +} + +static void hdmi4_unbind(struct device *dev, struct device *master, void *data) +{ + struct omap_hdmi *hdmi = dev_get_drvdata(dev); + + dss_debugfs_remove_file(hdmi->debugfs); + + if (hdmi->audio_pdev) + platform_device_unregister(hdmi->audio_pdev); + + hdmi4_cec_uninit(&hdmi->core); + hdmi_pll_uninit(&hdmi->pll); +} + +static const struct component_ops hdmi4_component_ops = { + .bind = hdmi4_bind, + .unbind = hdmi4_unbind, +}; + +/* ----------------------------------------------------------------------------- + * Probe & Remove, Suspend & Resume + */ + +static void hdmi4_init_output(struct omap_hdmi *hdmi) +{ + struct omap_dss_device *out = &hdmi->output; + + out->dev = &hdmi->pdev->dev; + out->id = OMAP_DSS_OUTPUT_HDMI; + out->output_type = OMAP_DISPLAY_TYPE_HDMI; + out->name = "hdmi.0"; + out->dispc_channel = OMAP_DSS_CHANNEL_DIGIT; + out->ops = &hdmi_ops; + out->owner = THIS_MODULE; + out->of_ports = BIT(0); + + omapdss_device_register(out); +} + +static void hdmi4_uninit_output(struct omap_hdmi *hdmi) +{ + struct omap_dss_device *out = &hdmi->output; + + omapdss_device_unregister(out); +} + +static int hdmi4_probe_of(struct omap_hdmi *hdmi) +{ + struct platform_device *pdev = hdmi->pdev; + struct device_node *node = pdev->dev.of_node; + struct device_node *ep; + int r; + + ep = of_graph_get_endpoint_by_regs(node, 0, 0); + if (!ep) + return 0; + + r = hdmi_parse_lanes_of(pdev, ep, &hdmi->phy); + of_node_put(ep); + return r; +} + +static int hdmi4_probe(struct platform_device *pdev) +{ + struct omap_hdmi *hdmi; int irq; + int r; hdmi = kzalloc(sizeof(*hdmi), GFP_KERNEL); if (!hdmi) return -ENOMEM; hdmi->pdev = pdev; - hdmi->dss = dss; + dev_set_drvdata(&pdev->dev, hdmi); mutex_init(&hdmi->lock); spin_lock_init(&hdmi->audio_playing_lock); - r = hdmi_probe_of(hdmi); + r = hdmi4_probe_of(hdmi); if (r) goto err_free; @@ -742,27 +795,19 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data) if (r) goto err_free; - r = hdmi_pll_init(dss, pdev, &hdmi->pll, &hdmi->wp); - if (r) - goto err_free; - r = hdmi_phy_init(pdev, &hdmi->phy, 4); if (r) - goto err_pll; + goto err_free; r = hdmi4_core_init(pdev, &hdmi->core); if (r) - goto err_pll; - - r = hdmi4_cec_init(pdev, &hdmi->core, &hdmi->wp); - if (r) - goto err_pll; + goto err_free; irq = platform_get_irq(pdev, 0); if (irq < 0) { DSSERR("platform_get_irq failed\n"); r = -ENODEV; - goto err_pll; + goto err_free; } r = devm_request_threaded_irq(&pdev->dev, irq, @@ -770,67 +815,38 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data) IRQF_ONESHOT, "OMAP HDMI", hdmi); if (r) { DSSERR("HDMI IRQ request failed\n"); - goto err_pll; + goto err_free; } pm_runtime_enable(&pdev->dev); - hdmi_init_output(hdmi); + hdmi4_init_output(hdmi); - r = hdmi_audio_register(hdmi); - if (r) { - DSSERR("Registering HDMI audio failed\n"); + r = component_add(&pdev->dev, &hdmi4_component_ops); + if (r) goto err_uninit_output; - } - - hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs, - hdmi); return 0; err_uninit_output: - hdmi_uninit_output(hdmi); + hdmi4_uninit_output(hdmi); pm_runtime_disable(&pdev->dev); -err_pll: - hdmi_pll_uninit(&hdmi->pll); err_free: kfree(hdmi); return r; } -static void hdmi4_unbind(struct device *dev, struct device *master, void *data) +static int hdmi4_remove(struct platform_device *pdev) { - struct omap_hdmi *hdmi = dev_get_drvdata(dev); + struct omap_hdmi *hdmi = platform_get_drvdata(pdev); - dss_debugfs_remove_file(hdmi->debugfs); - - if (hdmi->audio_pdev) - platform_device_unregister(hdmi->audio_pdev); - - hdmi_uninit_output(hdmi); - - hdmi4_cec_uninit(&hdmi->core); + component_del(&pdev->dev, &hdmi4_component_ops); - hdmi_pll_uninit(&hdmi->pll); + hdmi4_uninit_output(hdmi); - pm_runtime_disable(dev); + pm_runtime_disable(&pdev->dev); kfree(hdmi); -} - -static const struct component_ops hdmi4_component_ops = { - .bind = hdmi4_bind, - .unbind = hdmi4_unbind, -}; - -static int hdmi4_probe(struct platform_device *pdev) -{ - return component_add(&pdev->dev, &hdmi4_component_ops); -} - -static int hdmi4_remove(struct platform_device *pdev) -{ - component_del(&pdev->dev, &hdmi4_component_ops); return 0; }
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.pinchart@ideasonboard.com> --- drivers/gpu/drm/omapdrm/dss/hdmi4.c | 218 +++++++++++++++++++----------------- 1 file changed, 117 insertions(+), 101 deletions(-)