linux-kernel - Re: [PATCH v2 07/14] drm/msm/hdmi: switch to pm_runtime_resume_and

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9ddb841c-382a-4684-92cb-3a07e72ce5da@quicinc.com>
Date: Tue, 18 Jun 2024 17:00:55 -0700
From: Jessica Zhang <quic_jesszhan@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Rob Clark
	<robdclark@...il.com>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        Sean Paul
	<sean@...rly.run>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        "David
 Airlie" <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>
CC: <linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
        <freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 07/14] drm/msm/hdmi: switch to
 pm_runtime_resume_and_get()



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@...cinc.com>

> 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@...aro.org>
> ---
>   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
>