Hi,

On Sat, May 26, 2018 at 08:24:59PM +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/hdmi5.c | 205 +++++++++++++++++++-----------------
>  1 file changed, 110 insertions(+), 95 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> index 92ae561bf974..64b45a612439 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> @@ -544,53 +544,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))
>  {
> @@ -711,27 +668,116 @@ static int hdmi_audio_register(struct omap_hdmi *hdmi)
>  	return 0;
>  }
>  
> -/* HDMI HW IP initialisation */
> +/* -----------------------------------------------------------------------------
> + * Component Bind & Unbind
> + */
> +
>  static int hdmi5_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 = hdmi_audio_register(hdmi);
> +	if (r) {
> +		DSSERR("Registering HDMI audio failed %d\n", r);
> +		goto err_pll_uninit;
> +	}
> +
> +	hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs,
> +						hdmi);
> +
> +	return 0;
> +
> +err_pll_uninit:
> +	hdmi_pll_uninit(&hdmi->pll);
> +	return r;
> +}
> +
> +static void hdmi5_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);
> +
> +	hdmi_pll_uninit(&hdmi->pll);
> +}
> +
> +static const struct component_ops hdmi5_component_ops = {
> +	.bind	= hdmi5_bind,
> +	.unbind	= hdmi5_unbind,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Probe & Remove, Suspend & Resume
> + */
> +
> +static void hdmi5_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 hdmi5_uninit_output(struct omap_hdmi *hdmi)
> +{
> +	struct omap_dss_device *out = &hdmi->output;
> +
> +	omapdss_device_unregister(out);
> +}
> +
> +static int hdmi5_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 hdmi5_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 = hdmi5_probe_of(hdmi);
>  	if (r)
>  		goto err_free;
>  
> @@ -739,23 +785,19 @@ static int hdmi5_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, 5);
>  	if (r)
> -		goto err_pll;
> +		goto err_free;
>  
>  	r = hdmi5_core_init(pdev, &hdmi->core);
>  	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,
> @@ -763,65 +805,38 @@ static int hdmi5_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);
> +	hdmi5_init_output(hdmi);
>  
> -	r = hdmi_audio_register(hdmi);
> -	if (r) {
> -		DSSERR("Registering HDMI audio failed %d\n", r);
> +	r = component_add(&pdev->dev, &hdmi5_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);
> +	hdmi5_uninit_output(hdmi);
>  	pm_runtime_disable(&pdev->dev);
> -err_pll:
> -	hdmi_pll_uninit(&hdmi->pll);
>  err_free:
>  	kfree(hdmi);
>  	return r;
>  }
>  
> -static void hdmi5_unbind(struct device *dev, struct device *master, void *data)
> +static int hdmi5_remove(struct platform_device *pdev)
>  {
> -	struct omap_hdmi *hdmi = dev_get_drvdata(dev);
> -
> -	dss_debugfs_remove_file(hdmi->debugfs);
> +	struct omap_hdmi *hdmi = platform_get_drvdata(pdev);
>  
> -	if (hdmi->audio_pdev)
> -		platform_device_unregister(hdmi->audio_pdev);
> -
> -	hdmi_uninit_output(hdmi);
> +	component_del(&pdev->dev, &hdmi5_component_ops);
>  
> -	hdmi_pll_uninit(&hdmi->pll);
> +	hdmi5_uninit_output(hdmi);
>  
> -	pm_runtime_disable(dev);
> +	pm_runtime_disable(&pdev->dev);
>  
>  	kfree(hdmi);
> -}
> -
> -static const struct component_ops hdmi5_component_ops = {
> -	.bind	= hdmi5_bind,
> -	.unbind	= hdmi5_unbind,
> -};
> -
> -static int hdmi5_probe(struct platform_device *pdev)
> -{
> -	return component_add(&pdev->dev, &hdmi5_component_ops);
> -}
> -
> -static int hdmi5_remove(struct platform_device *pdev)
> -{
> -	component_del(&pdev->dev, &hdmi5_component_ops);
>  	return 0;
>  }
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel