Re: [PATCH v2 07/14] drm/msm/hdmi: switch to pm_runtime_resume_and_get()

[Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>]

On 5/22/2024 3:50 AM, Dmitry Baryshkov wrote:
> The pm_runtime_get_sync() function is a bad choise for runtime power

[nit: s/choise/choice/]

Reviewed-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>

> management. Switch HDMI driver to pm_runtime_resume_and_get() and add
> proper error handling, while we are at it.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
>   drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
>   drivers/gpu/drm/msm/hdmi/hdmi_hpd.c    | 12 ++++++++++--
>   drivers/gpu/drm/msm/hdmi/hdmi_phy.c    |  6 +++++-
>   3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> index fb99328107dd..d1b35328b6e8 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> @@ -19,7 +19,7 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)
>   	const struct hdmi_platform_config *config = hdmi->config;
>   	int ret;
>   
> -	pm_runtime_get_sync(&hdmi->pdev->dev);
> +	pm_runtime_resume_and_get(&hdmi->pdev->dev);
>   
>   	ret = regulator_bulk_enable(config->pwr_reg_cnt, hdmi->pwr_regs);
>   	if (ret)
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> index 36266aa626dc..fc21ad3b01dc 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
> @@ -85,7 +85,12 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
>   	if (hdmi->hpd_gpiod)
>   		gpiod_set_value_cansleep(hdmi->hpd_gpiod, 1);
>   
> -	pm_runtime_get_sync(dev);
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "runtime resume failed: %d\n", ret);
> +		goto fail;
> +	}
> +
>   	ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
>   	if (ret)
>   		goto fail;
> @@ -178,7 +183,10 @@ static enum drm_connector_status detect_reg(struct hdmi *hdmi)
>   	uint32_t hpd_int_status = 0;
>   	int ret;
>   
> -	pm_runtime_get_sync(&hdmi->pdev->dev);
> +	ret = pm_runtime_resume_and_get(&hdmi->pdev->dev);
> +	if (ret)
> +		goto out;
> +
>   	ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
>   	if (ret)
>   		goto out;
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
> index 88a3423b7f24..d5acae752300 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
> @@ -58,7 +58,11 @@ int msm_hdmi_phy_resource_enable(struct hdmi_phy *phy)
>   	struct device *dev = &phy->pdev->dev;
>   	int i, ret = 0;
>   
> -	pm_runtime_get_sync(dev);
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "runtime resume failed: %d\n", ret);
> +		return ret;
> +	}
>   
>   	ret = regulator_bulk_enable(cfg->num_regs, phy->regs);
>   	if (ret) {
>
> --
> 2.39.2