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.pinch...@ideasonboard.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reic...@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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to