[PATCH 3/3] drm/msm/hdmi: move resource allocation to probe function
Abhinav Kumar
quic_abhinavk at quicinc.com
Tue Aug 23 23:58:50 UTC 2022
On 6/16/2022 12:59 AM, Dmitry Baryshkov wrote:
> Rather than having all resource allocation happen in the _bind function
> (resulting in possible EPROBE_DEFER returns and component bind/unbind
> cycles) allocate and check all resources in _probe function. While we
> are at it, use platform_get_irq() to get the IRQ rather than going
> through the irq_of_parse_and_map().
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> ---
> drivers/gpu/drm/msm/hdmi/hdmi.c | 295 +++++++++++++++-----------------
> 1 file changed, 134 insertions(+), 161 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index 8dfe5690366b..429abd622c81 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -75,8 +75,6 @@ static void msm_hdmi_destroy(struct hdmi *hdmi)
>
> if (hdmi->i2c)
> msm_hdmi_i2c_destroy(hdmi->i2c);
> -
> - platform_set_drvdata(hdmi->pdev, NULL);
Do we still not need to do this in .remove?
> }
>
> static int msm_hdmi_get_phy(struct hdmi *hdmi)
> @@ -116,138 +114,10 @@ static int msm_hdmi_get_phy(struct hdmi *hdmi)
> * we are to EPROBE_DEFER we want to do it here, rather than later
> * at modeset_init() time
> */
> -static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
> +static int msm_hdmi_init(struct hdmi *hdmi)
> {
> - struct hdmi_platform_config *config = pdev->dev.platform_data;
> - struct hdmi *hdmi = NULL;
> - struct resource *res;
> - int i, ret;
> -
> - hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
> - if (!hdmi) {
> - ret = -ENOMEM;
> - goto fail;
> - }
> -
> - hdmi->pdev = pdev;
> - hdmi->config = config;
> - spin_lock_init(&hdmi->reg_lock);
> -
> - hdmi->mmio = msm_ioremap(pdev, "core_physical");
> - if (IS_ERR(hdmi->mmio)) {
> - ret = PTR_ERR(hdmi->mmio);
> - goto fail;
> - }
> -
> - /* HDCP needs physical address of hdmi register */
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> - "core_physical");
> - if (!res) {
> - ret = -EINVAL;
> - goto fail;
> - }
> - hdmi->mmio_phy_addr = res->start;
> -
> - hdmi->qfprom_mmio = msm_ioremap(pdev, "qfprom_physical");
> - if (IS_ERR(hdmi->qfprom_mmio)) {
> - DRM_DEV_INFO(&pdev->dev, "can't find qfprom resource\n");
> - hdmi->qfprom_mmio = NULL;
> - }
> -
> - hdmi->hpd_regs = devm_kcalloc(&pdev->dev,
> - config->hpd_reg_cnt,
> - sizeof(hdmi->hpd_regs[0]),
> - GFP_KERNEL);
> - if (!hdmi->hpd_regs) {
> - ret = -ENOMEM;
> - goto fail;
> - }
> - for (i = 0; i < config->hpd_reg_cnt; i++)
> - hdmi->hpd_regs[i].supply = config->hpd_reg_names[i];
> -
> - ret = devm_regulator_bulk_get(&pdev->dev, config->hpd_reg_cnt, hdmi->hpd_regs);
> - if (ret) {
> - DRM_DEV_ERROR(&pdev->dev, "failed to get hpd regulator: %d\n", ret);
> - goto fail;
> - }
> -
> - hdmi->pwr_regs = devm_kcalloc(&pdev->dev,
> - config->pwr_reg_cnt,
> - sizeof(hdmi->pwr_regs[0]),
> - GFP_KERNEL);
> - if (!hdmi->pwr_regs) {
> - ret = -ENOMEM;
> - goto fail;
> - }
> -
> - for (i = 0; i < config->pwr_reg_cnt; i++)
> - hdmi->pwr_regs[i].supply = config->pwr_reg_names[i];
> -
> - ret = devm_regulator_bulk_get(&pdev->dev, config->pwr_reg_cnt, hdmi->pwr_regs);
> - if (ret) {
> - DRM_DEV_ERROR(&pdev->dev, "failed to get pwr regulator: %d\n", ret);
> - goto fail;
> - }
> -
> - hdmi->hpd_clks = devm_kcalloc(&pdev->dev,
> - config->hpd_clk_cnt,
> - sizeof(hdmi->hpd_clks[0]),
> - GFP_KERNEL);
> - if (!hdmi->hpd_clks) {
> - ret = -ENOMEM;
> - goto fail;
> - }
> - for (i = 0; i < config->hpd_clk_cnt; i++) {
> - struct clk *clk;
> -
> - clk = msm_clk_get(pdev, config->hpd_clk_names[i]);
> - if (IS_ERR(clk)) {
> - ret = PTR_ERR(clk);
> - DRM_DEV_ERROR(&pdev->dev, "failed to get hpd clk: %s (%d)\n",
> - config->hpd_clk_names[i], ret);
> - goto fail;
> - }
> -
> - hdmi->hpd_clks[i] = clk;
> - }
> -
> - hdmi->pwr_clks = devm_kcalloc(&pdev->dev,
> - config->pwr_clk_cnt,
> - sizeof(hdmi->pwr_clks[0]),
> - GFP_KERNEL);
> - if (!hdmi->pwr_clks) {
> - ret = -ENOMEM;
> - goto fail;
> - }
> - for (i = 0; i < config->pwr_clk_cnt; i++) {
> - struct clk *clk;
> -
> - clk = msm_clk_get(pdev, config->pwr_clk_names[i]);
> - if (IS_ERR(clk)) {
> - ret = PTR_ERR(clk);
> - DRM_DEV_ERROR(&pdev->dev, "failed to get pwr clk: %s (%d)\n",
> - config->pwr_clk_names[i], ret);
> - goto fail;
> - }
> -
> - hdmi->pwr_clks[i] = clk;
> - }
> -
> - hdmi->hpd_gpiod = devm_gpiod_get_optional(&pdev->dev, "hpd", GPIOD_IN);
> - /* This will catch e.g. -EPROBE_DEFER */
> - if (IS_ERR(hdmi->hpd_gpiod)) {
> - ret = PTR_ERR(hdmi->hpd_gpiod);
> - DRM_DEV_ERROR(&pdev->dev, "failed to get hpd gpio: (%d)\n", ret);
> - goto fail;
> - }
> -
> - if (!hdmi->hpd_gpiod)
> - DBG("failed to get HPD gpio");
> -
> - if (hdmi->hpd_gpiod)
> - gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
> -
> - devm_pm_runtime_enable(&pdev->dev);
> + struct platform_device *pdev = hdmi->pdev;
> + int ret;
What about the rest of the msm_hdmi_init() function?
msm_hdmi_i2c_init, msm_hdmi_get_phy and msm_hdmi_hdcp_init have been
left behind. Any reason for that?
>
> hdmi->workq = alloc_ordered_workqueue("msm_hdmi", 0);
>
> @@ -271,13 +141,13 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
> hdmi->hdcp_ctrl = NULL;
> }
>
> - return hdmi;
> + return 0;
>
> fail:
> if (hdmi)
> msm_hdmi_destroy(hdmi);
>
> - return ERR_PTR(ret);
> + return ret;
> }
>
> /* Second part of initialization, the drm/kms level modeset_init,
> @@ -318,13 +188,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>
> drm_connector_attach_encoder(hdmi->connector, hdmi->encoder);
>
> - hdmi->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> - if (!hdmi->irq) {
> - ret = -EINVAL;
> - DRM_DEV_ERROR(dev->dev, "failed to get irq\n");
> - goto fail;
> - }
> -
> ret = devm_request_irq(&pdev->dev, hdmi->irq,
> msm_hdmi_irq, IRQF_TRIGGER_HIGH,
> "hdmi_isr", hdmi);
> @@ -344,8 +207,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>
> priv->bridges[priv->num_bridges++] = hdmi->bridge;
>
> - platform_set_drvdata(pdev, hdmi);
> -
> return 0;
>
> fail:
> @@ -373,7 +234,7 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
> static const char *hpd_reg_names_8960[] = {"core-vdda"};
> static const char *hpd_clk_names_8960[] = {"core", "master_iface", "slave_iface"};
>
> -static struct hdmi_platform_config hdmi_tx_8960_config = {
> +const static struct hdmi_platform_config hdmi_tx_8960_config = {
> HDMI_CFG(hpd_reg, 8960),
> HDMI_CFG(hpd_clk, 8960),
> };
> @@ -383,7 +244,7 @@ static const char *pwr_clk_names_8x74[] = {"extp", "alt_iface"};
> static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core"};
> static unsigned long hpd_clk_freq_8x74[] = {0, 19200000, 0};
>
> -static struct hdmi_platform_config hdmi_tx_8974_config = {
> +const static struct hdmi_platform_config hdmi_tx_8974_config = {
> HDMI_CFG(pwr_reg, 8x74),
> HDMI_CFG(pwr_clk, 8x74),
> HDMI_CFG(hpd_clk, 8x74),
> @@ -498,23 +359,12 @@ static int msm_hdmi_register_audio_driver(struct hdmi *hdmi, struct device *dev)
> static int msm_hdmi_bind(struct device *dev, struct device *master, void *data)
> {
> struct msm_drm_private *priv = dev_get_drvdata(master);
> - struct hdmi_platform_config *hdmi_cfg;
> - struct hdmi *hdmi;
> - struct device_node *of_node = dev->of_node;
> + struct hdmi *hdmi = dev_get_drvdata(dev);
> int err;
>
> - hdmi_cfg = (struct hdmi_platform_config *)
> - of_device_get_match_data(dev);
> - if (!hdmi_cfg) {
> - DRM_DEV_ERROR(dev, "unknown hdmi_cfg: %pOFn\n", of_node);
> - return -ENXIO;
> - }
> -
> - dev->platform_data = hdmi_cfg;
> -
> - hdmi = msm_hdmi_init(to_platform_device(dev));
> - if (IS_ERR(hdmi))
> - return PTR_ERR(hdmi);
> + err = msm_hdmi_init(hdmi);
> + if (err)
> + return err;
> priv->hdmi = hdmi;
>
> err = msm_hdmi_register_audio_driver(hdmi, dev);
> @@ -547,6 +397,129 @@ static const struct component_ops msm_hdmi_ops = {
>
> static int msm_hdmi_dev_probe(struct platform_device *pdev)
> {
> + const struct hdmi_platform_config *config;
> + struct device *dev = &pdev->dev;
> + struct hdmi *hdmi;
> + struct resource *res;
> + int i, ret;
> +
> + config = of_device_get_match_data(dev);
> + if (!config)
> + return -EINVAL;
> +
> + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
> + if (!hdmi)
> + return -ENOMEM;
> +
> + hdmi->pdev = pdev;
> + hdmi->config = config;
> + spin_lock_init(&hdmi->reg_lock);
> +
> + hdmi->mmio = msm_ioremap(pdev, "core_physical");
> + if (IS_ERR(hdmi->mmio))
> + return PTR_ERR(hdmi->mmio);
> +
> + /* HDCP needs physical address of hdmi register */
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "core_physical");
> + if (!res)
> + return -EINVAL;
> + hdmi->mmio_phy_addr = res->start;
> +
> + hdmi->qfprom_mmio = msm_ioremap(pdev, "qfprom_physical");
> + if (IS_ERR(hdmi->qfprom_mmio)) {
> + DRM_DEV_INFO(&pdev->dev, "can't find qfprom resource\n");
> + hdmi->qfprom_mmio = NULL;
> + }
> +
> + hdmi->irq = platform_get_irq(pdev, 0);
> + if (hdmi->irq < 0)
> + return hdmi->irq;
> +
> + hdmi->hpd_regs = devm_kcalloc(&pdev->dev,
> + config->hpd_reg_cnt,
> + sizeof(hdmi->hpd_regs[0]),
> + GFP_KERNEL);
> + if (!hdmi->hpd_regs)
> + return -ENOMEM;
> +
> + for (i = 0; i < config->hpd_reg_cnt; i++)
> + hdmi->hpd_regs[i].supply = config->hpd_reg_names[i];
> +
> + ret = devm_regulator_bulk_get(&pdev->dev, config->hpd_reg_cnt, hdmi->hpd_regs);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get hpd regulators\n");
> +
> + hdmi->pwr_regs = devm_kcalloc(&pdev->dev,
> + config->pwr_reg_cnt,
> + sizeof(hdmi->pwr_regs[0]),
> + GFP_KERNEL);
> + if (!hdmi->pwr_regs)
> + return -ENOMEM;
> +
> + for (i = 0; i < config->pwr_reg_cnt; i++)
> + hdmi->pwr_regs[i].supply = config->pwr_reg_names[i];
> +
> + ret = devm_regulator_bulk_get(&pdev->dev, config->pwr_reg_cnt, hdmi->pwr_regs);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get pwr regulators\n");
> +
> + hdmi->hpd_clks = devm_kcalloc(&pdev->dev,
> + config->hpd_clk_cnt,
> + sizeof(hdmi->hpd_clks[0]),
> + GFP_KERNEL);
> + if (!hdmi->hpd_clks)
> + return -ENOMEM;
> +
> + for (i = 0; i < config->hpd_clk_cnt; i++) {
> + struct clk *clk;
> +
> + clk = msm_clk_get(pdev, config->hpd_clk_names[i]);
> + if (IS_ERR(clk))
> + return dev_err_probe(dev, PTR_ERR(clk),
> + "failed to get hpd clk: %s\n",
> + config->hpd_clk_names[i]);
> +
> + hdmi->hpd_clks[i] = clk;
> + }
> +
> + hdmi->pwr_clks = devm_kcalloc(&pdev->dev,
> + config->pwr_clk_cnt,
> + sizeof(hdmi->pwr_clks[0]),
> + GFP_KERNEL);
> + if (!hdmi->pwr_clks)
> + return -ENOMEM;
> +
> + for (i = 0; i < config->pwr_clk_cnt; i++) {
> + struct clk *clk;
> +
> + clk = msm_clk_get(pdev, config->pwr_clk_names[i]);
> + if (IS_ERR(clk))
> + return dev_err_probe(dev, PTR_ERR(clk),
> + "failed to get pwr clk: %s\n",
> + config->pwr_clk_names[i]);
> +
> + hdmi->pwr_clks[i] = clk;
> + }
> +
> + hdmi->hpd_gpiod = devm_gpiod_get_optional(&pdev->dev, "hpd", GPIOD_IN);
> + /* This will catch e.g. -EPROBE_DEFER */
> + if (IS_ERR(hdmi->hpd_gpiod))
> + return dev_err_probe(dev, PTR_ERR(hdmi->hpd_gpiod),
> + "failed to get hpd gpio\n");
> +
> + if (!hdmi->hpd_gpiod)
> + DBG("failed to get HPD gpio");
> +
> + if (hdmi->hpd_gpiod)
> + gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
> +
> + ret = devm_pm_runtime_enable(&pdev->dev);
> + if (ret)
> + return ret;
> +
> + platform_set_drvdata(pdev, hdmi);
> +
> return component_add(&pdev->dev, &msm_hdmi_ops);
> }
>
More information about the dri-devel
mailing list