* [PATCH 1/3] drm/msm/dpu: Drop sneaky dev_pm_opp_set_rate(0)
2026-07-28 13:20 [PATCH 0/3] Remove brownout risks from DPU Konrad Dybcio
@ 2026-07-28 13:20 ` Konrad Dybcio
2026-07-28 13:57 ` Dmitry Baryshkov
2026-07-28 13:20 ` [PATCH 2/3] drm/msm/dp: Drop dev_pm_opp_set_rate(0) Konrad Dybcio
2026-07-28 13:20 ` [PATCH 3/3] drm/msm/dsi: " Konrad Dybcio
2 siblings, 1 reply; 10+ messages in thread
From: Konrad Dybcio @ 2026-07-28 13:20 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Rajendra Nayak, Matthias Kaehlcke, Sravanthi Kollukuduru,
Rajesh Yadav, Chandan Uddaraju, Jeykumar Sankaran, Vara Reddy,
Stephen Boyd, Guenter Roeck, Kuogee Hsieh, Archit Taneja
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Tanmay Shah,
Konrad Dybcio
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
dev_pm_opp_set_rate(0) removes the vote specified in required-opps but
does not actually park the clock, making it run without the necessary
power backing. Prevent that from happening when
_dpu_core_perf_get_core_clk_rate() returns 0.
Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 2ff255d7795e..fea173e37464 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -394,6 +394,10 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
trace_dpu_core_perf_update_clk(kms->dev, !crtc->enabled, clk_rate);
+ /* If we're going offline, PM callbacks will disable the clocks instead */
+ if (!clk_rate)
+ return 0;
+
clk_rate = min(clk_rate, kms->perf.max_core_clk_rate);
ret = dev_pm_opp_set_rate(&kms->pdev->dev, clk_rate);
if (ret) {
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] drm/msm/dpu: Drop sneaky dev_pm_opp_set_rate(0)
2026-07-28 13:20 ` [PATCH 1/3] drm/msm/dpu: Drop sneaky dev_pm_opp_set_rate(0) Konrad Dybcio
@ 2026-07-28 13:57 ` Dmitry Baryshkov
2026-07-28 16:40 ` Konrad Dybcio
0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Baryshkov @ 2026-07-28 13:57 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Rajendra Nayak, Matthias Kaehlcke, Sravanthi Kollukuduru,
Rajesh Yadav, Chandan Uddaraju, Jeykumar Sankaran, Vara Reddy,
Stephen Boyd, Guenter Roeck, Kuogee Hsieh, Archit Taneja,
linux-arm-msm, dri-devel, freedreno, linux-kernel, Tanmay Shah,
Konrad Dybcio
On Tue, Jul 28, 2026 at 03:20:45PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> dev_pm_opp_set_rate(0) removes the vote specified in required-opps but
> does not actually park the clock, making it run without the necessary
> power backing. Prevent that from happening when
> _dpu_core_perf_get_core_clk_rate() returns 0.
>
> Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index 2ff255d7795e..fea173e37464 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -394,6 +394,10 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
>
> trace_dpu_core_perf_update_clk(kms->dev, !crtc->enabled, clk_rate);
>
> + /* If we're going offline, PM callbacks will disable the clocks instead */
> + if (!clk_rate)
> + return 0;
Is 0 here an actual case? If it is coming from the core_perf, then the
next line will handle it.
> +
> clk_rate = min(clk_rate, kms->perf.max_core_clk_rate);
> ret = dev_pm_opp_set_rate(&kms->pdev->dev, clk_rate);
> if (ret) {
>
> --
> 2.55.0
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] drm/msm/dpu: Drop sneaky dev_pm_opp_set_rate(0)
2026-07-28 13:57 ` Dmitry Baryshkov
@ 2026-07-28 16:40 ` Konrad Dybcio
0 siblings, 0 replies; 10+ messages in thread
From: Konrad Dybcio @ 2026-07-28 16:40 UTC (permalink / raw)
To: Dmitry Baryshkov, Konrad Dybcio
Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Rajendra Nayak, Matthias Kaehlcke, Sravanthi Kollukuduru,
Rajesh Yadav, Chandan Uddaraju, Jeykumar Sankaran, Vara Reddy,
Stephen Boyd, Guenter Roeck, Kuogee Hsieh, Archit Taneja,
linux-arm-msm, dri-devel, freedreno, linux-kernel, Tanmay Shah
On 7/28/26 3:57 PM, Dmitry Baryshkov wrote:
> On Tue, Jul 28, 2026 at 03:20:45PM +0200, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> dev_pm_opp_set_rate(0) removes the vote specified in required-opps but
>> does not actually park the clock, making it run without the necessary
>> power backing. Prevent that from happening when
>> _dpu_core_perf_get_core_clk_rate() returns 0.
>>
>> Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
>> index 2ff255d7795e..fea173e37464 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
>> @@ -394,6 +394,10 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
>>
>> trace_dpu_core_perf_update_clk(kms->dev, !crtc->enabled, clk_rate);
>>
>> + /* If we're going offline, PM callbacks will disable the clocks instead */
>> + if (!clk_rate)
>> + return 0;
>
> Is 0 here an actual case? If it is coming from the core_perf, then the
> next line will handle it.
Yes it is (upon suspend) and it will not be handled because it's a min()
and not a max() below
Konrad
>
>> +
>> clk_rate = min(clk_rate, kms->perf.max_core_clk_rate);
>> ret = dev_pm_opp_set_rate(&kms->pdev->dev, clk_rate);
>> if (ret) {
>>
>> --
>> 2.55.0
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] drm/msm/dp: Drop dev_pm_opp_set_rate(0)
2026-07-28 13:20 [PATCH 0/3] Remove brownout risks from DPU Konrad Dybcio
2026-07-28 13:20 ` [PATCH 1/3] drm/msm/dpu: Drop sneaky dev_pm_opp_set_rate(0) Konrad Dybcio
@ 2026-07-28 13:20 ` Konrad Dybcio
2026-07-28 13:34 ` sashiko-bot
2026-07-28 13:56 ` Dmitry Baryshkov
2026-07-28 13:20 ` [PATCH 3/3] drm/msm/dsi: " Konrad Dybcio
2 siblings, 2 replies; 10+ messages in thread
From: Konrad Dybcio @ 2026-07-28 13:20 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Rajendra Nayak, Matthias Kaehlcke, Sravanthi Kollukuduru,
Rajesh Yadav, Chandan Uddaraju, Jeykumar Sankaran, Vara Reddy,
Stephen Boyd, Guenter Roeck, Kuogee Hsieh, Archit Taneja
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Tanmay Shah,
Konrad Dybcio
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
dev_pm_opp_set_rate(0) removes the vote specified in required-opps but
does not actually park the clock, making it run without the necessary
power backing. Drop the explicit calls to it.
Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 86ef8c89ad44..e09924eae5a7 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1950,13 +1950,12 @@ static int msm_dp_ctrl_reinitialize_mainlink(struct msm_dp_ctrl_private *ctrl)
msm_dp_ctrl_mainlink_disable(ctrl);
ctrl->phy_opts.dp.lanes = ctrl->link->link_params.num_lanes;
phy_configure(phy, &ctrl->phy_opts);
+
/*
* Disable and re-enable the mainlink clock since the
* link clock might have been adjusted as part of the
* link maintenance.
*/
- dev_pm_opp_set_rate(ctrl->dev, 0);
-
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
phy_power_off(phy);
@@ -1982,7 +1981,6 @@ static int msm_dp_ctrl_deinitialize_mainlink(struct msm_dp_ctrl_private *ctrl)
msm_dp_ctrl_reset(&ctrl->msm_dp_ctrl);
- dev_pm_opp_set_rate(ctrl->dev, 0);
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
phy_power_off(phy);
@@ -2573,7 +2571,6 @@ void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl)
ctrl->stream_clks_on = false;
}
- dev_pm_opp_set_rate(ctrl->dev, 0);
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
phy_power_off(phy);
@@ -2602,7 +2599,6 @@ void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl)
ctrl->stream_clks_on = false;
}
- dev_pm_opp_set_rate(ctrl->dev, 0);
msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
phy_power_off(phy);
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] drm/msm/dp: Drop dev_pm_opp_set_rate(0)
2026-07-28 13:20 ` [PATCH 2/3] drm/msm/dp: Drop dev_pm_opp_set_rate(0) Konrad Dybcio
@ 2026-07-28 13:34 ` sashiko-bot
2026-07-28 13:56 ` Dmitry Baryshkov
1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-28 13:34 UTC (permalink / raw)
To: Konrad Dybcio; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removing `dev_pm_opp_set_rate(0)` entirely introduces a permanent performance state (voltage) vote leak when the display is disabled but the monitor remains plugged in.
--
--- Patch [2]: [PATCH 2/3] drm/msm/dp: Drop dev_pm_opp_set_rate(0) ---
commit bc14685e819772f265842e7901b3f8c5d0779863
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
drm/msm/dp: Drop dev_pm_opp_set_rate(0)
This commit removes explicit calls to dev_pm_opp_set_rate(0) in the DP link
disable and deinitialization paths. The commit message notes that it does
not actually park the clock and makes it run without the necessary power
backing.
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 86ef8c89ad443..e09924eae5a7a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1950,13 +1950,12 @@ static int msm_dp_ctrl_reinitialize_mainlink(struct msm_dp_ctrl_private *ctrl)
> msm_dp_ctrl_mainlink_disable(ctrl);
> ctrl->phy_opts.dp.lanes = ctrl->link->link_params.num_lanes;
> phy_configure(phy, &ctrl->phy_opts);
> +
> /*
> * Disable and re-enable the mainlink clock since the
> * link clock might have been adjusted as part of the
> * link maintenance.
> */
> - dev_pm_opp_set_rate(ctrl->dev, 0);
> -
> msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
>
> phy_power_off(phy);
> @@ -1982,7 +1981,6 @@ static int msm_dp_ctrl_deinitialize_mainlink(struct msm_dp_ctrl_private *ctrl)
>
> msm_dp_ctrl_reset(&ctrl->msm_dp_ctrl);
>
> - dev_pm_opp_set_rate(ctrl->dev, 0);
> msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
>
> phy_power_off(phy);
[Severity: Medium]
Does removing this call introduce a permanent performance state (voltage)
vote leak when the display is disabled but the monitor remains plugged in?
When a user connects a DP monitor, msm_dp_hpd_plug_handle() holds a
runtime PM reference for the duration of the plug. If the link is enabled
for display, the clocks are enabled and an OPP rate is set.
Later, if the display is turned off via DRM DPMS or screen timeout, this
deinitialization path disables the clocks. However, because the monitor is
still physically connected, the runtime PM reference is held. This means
the device never runtime suspends and genpd will not automatically drop
the performance vote.
Without explicitly dropping the OPP vote here, the SoC power domain might
stay in a high performance state indefinitely, causing a continuous
power regression.
> @@ -2573,7 +2571,6 @@ void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl)
> ctrl->stream_clks_on = false;
> }
>
> - dev_pm_opp_set_rate(ctrl->dev, 0);
> msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
>
> phy_power_off(phy);
[Severity: Medium]
Similarly, could dropping the OPP vote here in msm_dp_ctrl_off_link_stream()
leave the voltage vote active indefinitely while the link stream is off?
> @@ -2602,7 +2599,6 @@ void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl)
> ctrl->stream_clks_on = false;
> }
>
> - dev_pm_opp_set_rate(ctrl->dev, 0);
> msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl);
>
> phy_power_off(phy);
[Severity: Medium]
This removal in msm_dp_ctrl_off() appears to cause the same voltage vote
leak when the display is turned off but remains plugged in.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-topic-dpu_power-v1-0-e7783b859a70@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] drm/msm/dp: Drop dev_pm_opp_set_rate(0)
2026-07-28 13:20 ` [PATCH 2/3] drm/msm/dp: Drop dev_pm_opp_set_rate(0) Konrad Dybcio
2026-07-28 13:34 ` sashiko-bot
@ 2026-07-28 13:56 ` Dmitry Baryshkov
1 sibling, 0 replies; 10+ messages in thread
From: Dmitry Baryshkov @ 2026-07-28 13:56 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Rajendra Nayak, Matthias Kaehlcke, Sravanthi Kollukuduru,
Rajesh Yadav, Chandan Uddaraju, Jeykumar Sankaran, Vara Reddy,
Stephen Boyd, Guenter Roeck, Kuogee Hsieh, Archit Taneja,
linux-arm-msm, dri-devel, freedreno, linux-kernel, Tanmay Shah,
Konrad Dybcio
On Tue, Jul 28, 2026 at 03:20:46PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> dev_pm_opp_set_rate(0) removes the vote specified in required-opps but
> does not actually park the clock, making it run without the necessary
> power backing. Drop the explicit calls to it.
>
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/dp/dp_ctrl.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] drm/msm/dsi: Drop dev_pm_opp_set_rate(0)
2026-07-28 13:20 [PATCH 0/3] Remove brownout risks from DPU Konrad Dybcio
2026-07-28 13:20 ` [PATCH 1/3] drm/msm/dpu: Drop sneaky dev_pm_opp_set_rate(0) Konrad Dybcio
2026-07-28 13:20 ` [PATCH 2/3] drm/msm/dp: Drop dev_pm_opp_set_rate(0) Konrad Dybcio
@ 2026-07-28 13:20 ` Konrad Dybcio
2026-07-28 13:34 ` sashiko-bot
2026-07-28 13:55 ` Dmitry Baryshkov
2 siblings, 2 replies; 10+ messages in thread
From: Konrad Dybcio @ 2026-07-28 13:20 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Rajendra Nayak, Matthias Kaehlcke, Sravanthi Kollukuduru,
Rajesh Yadav, Chandan Uddaraju, Jeykumar Sankaran, Vara Reddy,
Stephen Boyd, Guenter Roeck, Kuogee Hsieh, Archit Taneja
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Tanmay Shah,
Konrad Dybcio
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
dev_pm_opp_set_rate(0) removes the vote specified in required-opps but
does not actually park the clock, making it run without the necessary
power backing. Drop the explicit call to it.
Every call site of ops->link_clk_disable() is followed by
pm_runtime_put(), so the power vote will be rescinded if deemed safe.
Fixes: 32d3e0feccfe ("drm/msm: dsi: Use OPP API to set clk/perf state")
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 59125d342fcb..7e4e3718b536 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -550,8 +550,6 @@ int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host)
void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host)
{
- /* Drop the performance state vote */
- dev_pm_opp_set_rate(&msm_host->pdev->dev, 0);
clk_disable_unprepare(msm_host->esc_clk);
clk_disable_unprepare(msm_host->pixel_clk);
clk_disable_unprepare(msm_host->byte_intf_clk);
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] drm/msm/dsi: Drop dev_pm_opp_set_rate(0)
2026-07-28 13:20 ` [PATCH 3/3] drm/msm/dsi: " Konrad Dybcio
@ 2026-07-28 13:34 ` sashiko-bot
2026-07-28 13:55 ` Dmitry Baryshkov
1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-28 13:34 UTC (permalink / raw)
To: Konrad Dybcio; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Asynchronous pm_runtime_put() in power-off path races with device teardown, leaking bus clocks.
- [High] Ignored return values for clock enablement in msm_dsi_host_xfer_prepare() leading to unclocked register access.
- [High] Unclocked register access crash due to missing cancel_work_sync() for err_work.
- [High] Double put_device() on the PHY device when dsi_get_phy() defers probe.
- [High] Asynchronous pm_runtime_put() causes hardware sequencing violation by disabling regulators while bus clocks are still running.
--
--- Patch [3]: [PATCH 3/3] drm/msm/dsi: Drop dev_pm_opp_set_rate(0) ---
commit 9d791038937d59c654ef91edf07d2b7ff4432725
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
drm/msm/dsi: Drop dev_pm_opp_set_rate(0)
This commit removes the dev_pm_opp_set_rate(0) call from
dsi_link_clk_disable_6g() because it removes the power vote without
parking the clock. It relies on the subsequent pm_runtime_put() to rescind
the power vote.
> Every call site of ops->link_clk_disable() is followed by
> pm_runtime_put(), so the power vote will be rescinded if deemed safe.
[Severity: High]
This isn't a bug introduced by this patch, but does relying on the
asynchronous pm_runtime_put() cause a resource leak during teardown?
In msm_dsi_host_power_off(), pm_runtime_put() is called to queue an async
suspend. However, if the device is immediately torn down,
msm_dsi_host_destroy() cancels any pending async work:
drivers/gpu/drm/msm/dsi/dsi_host.c:msm_dsi_host_destroy() {
...
pm_runtime_disable(&msm_host->pdev->dev);
...
}
Because the async suspend is canceled, does this leave the device in the
RPM_ACTIVE state and permanently leak the bus clocks? Should
pm_runtime_put_sync() be used instead?
[Severity: High]
This is a pre-existing issue, but does this asynchronous pm_runtime_put() also
cause a hardware power-sequencing violation?
In msm_dsi_host_power_off(), the regulators are disabled immediately after
the async put:
drivers/gpu/drm/msm/dsi/dsi_host.c:msm_dsi_host_power_off() {
...
pm_runtime_put(&msm_host->pdev->dev);
regulator_bulk_disable(msm_host->cfg_hnd->cfg->num_regulators,
msm_host->supplies);
...
}
Since pm_runtime_put() returns immediately while bus clocks are still running,
does this remove power before the clocks are actually disabled by the
asynchronous suspend?
[Severity: High]
This is a pre-existing issue, but is there a risk of an unclocked register
access crash during power off?
When the display is turned off in msm_dsi_host_power_off(), hardware clocks
are disabled but there is no call to cancel_work_sync() for err_work:
drivers/gpu/drm/msm/dsi/dsi_host.c:msm_dsi_host_power_off() {
...
cfg_hnd->ops->link_clk_disable(msm_host);
pm_runtime_put(&msm_host->pdev->dev);
...
}
If err_work executes after clocks are disabled or is flushed during driver
teardown:
drivers/gpu/drm/msm/dsi/dsi_host.c:dsi_err_worker() {
...
dsi_sw_reset(msm_host);
...
}
Will this unconditional hardware access cause a synchronous external abort?
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 59125d342fcb6..7e4e3718b536c 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -550,8 +550,6 @@ int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host)
>
> void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host)
> {
> - /* Drop the performance state vote */
> - dev_pm_opp_set_rate(&msm_host->pdev->dev, 0);
[Severity: High]
This is a pre-existing issue, but is it possible for a similar sequence to
cause a kernel panic due to ignored return values when enabling clocks?
In msm_dsi_host_xfer_prepare(), the return values for enablement are ignored:
drivers/gpu/drm/msm/dsi/dsi_host.c:msm_dsi_host_xfer_prepare() {
...
pm_runtime_get_sync(&msm_host->pdev->dev);
cfg_hnd->ops->link_clk_set_rate(msm_host);
cfg_hnd->ops->link_clk_enable(msm_host);
...
msm_host->dma_cmd_ctrl_restore = dsi_read(msm_host, REG_DSI_CTRL);
...
}
If clock enablement fails, will the unconditional dsi_read() to an unclocked
register cause a synchronous external abort?
[Severity: High]
This isn't a bug introduced by this patch, but does dsi_get_phy() double free
the PHY device on probe deferral?
If the PHY driver is not ready, dsi_get_phy() drops the reference but leaves a
dangling pointer in msm_dsi->phy_dev:
drivers/gpu/drm/msm/dsi/dsi.c:dsi_get_phy() {
...
put_device(&phy_pdev->dev);
DRM_DEV_ERROR(&pdev->dev, "%s: phy driver is not ready\n", __func__);
return -EPROBE_DEFER;
...
}
When the error is handled during dsi_init(), it calls dsi_destroy() which
drops the reference a second time:
drivers/gpu/drm/msm/dsi/dsi.c:dsi_destroy() {
...
if (msm_dsi->phy_dev) {
put_device(msm_dsi->phy_dev);
...
}
Could this lead to a reference count underflow?
> clk_disable_unprepare(msm_host->esc_clk);
> clk_disable_unprepare(msm_host->pixel_clk);
> clk_disable_unprepare(msm_host->byte_intf_clk);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-topic-dpu_power-v1-0-e7783b859a70@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] drm/msm/dsi: Drop dev_pm_opp_set_rate(0)
2026-07-28 13:20 ` [PATCH 3/3] drm/msm/dsi: " Konrad Dybcio
2026-07-28 13:34 ` sashiko-bot
@ 2026-07-28 13:55 ` Dmitry Baryshkov
1 sibling, 0 replies; 10+ messages in thread
From: Dmitry Baryshkov @ 2026-07-28 13:55 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Rajendra Nayak, Matthias Kaehlcke, Sravanthi Kollukuduru,
Rajesh Yadav, Chandan Uddaraju, Jeykumar Sankaran, Vara Reddy,
Stephen Boyd, Guenter Roeck, Kuogee Hsieh, Archit Taneja,
linux-arm-msm, dri-devel, freedreno, linux-kernel, Tanmay Shah,
Konrad Dybcio
On Tue, Jul 28, 2026 at 03:20:47PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> dev_pm_opp_set_rate(0) removes the vote specified in required-opps but
> does not actually park the clock, making it run without the necessary
> power backing. Drop the explicit call to it.
>
> Every call site of ops->link_clk_disable() is followed by
> pm_runtime_put(), so the power vote will be rescinded if deemed safe.
>
> Fixes: 32d3e0feccfe ("drm/msm: dsi: Use OPP API to set clk/perf state")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 --
> 1 file changed, 2 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread