On Sun, 9 Jul 2023 at 20:22, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > > > On 7/7/2023 5:06 PM, Dmitry Baryshkov wrote: > > On 08/07/2023 02:52, Kuogee Hsieh wrote: > >> Since both pm_runtime_resume() and pm_runtime_suspend() are not > >> populated at dp_pm_ops. Those pm_runtime_get/put() functions within > >> dp_power.c will not have any effects in addition to increase/decrease > >> power counter. > > > > Lie. > > > > Even if the commit text is incorrect, review comments like this are not > helping the patch nor the author and will just get ignored anyway. The review comment might be overreacting, excuse me. I was really impressed by the commit message, which contradicts the basic source code. pm_runtime_get() does a lot more than just increasing the power counter. > >> Also pm_runtime_xxx() should be executed at top > >> layer. > > > > Why? > > > > I guess he meant to centralize this around dp_display.c. Will elaborate > while posting the next rev. > > >> > >> Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx> > >> --- > >> drivers/gpu/drm/msm/dp/dp_power.c | 9 --------- > >> 1 file changed, 9 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c > >> b/drivers/gpu/drm/msm/dp/dp_power.c > >> index 5cb84ca..ed2f62a 100644 > >> --- a/drivers/gpu/drm/msm/dp/dp_power.c > >> +++ b/drivers/gpu/drm/msm/dp/dp_power.c > >> @@ -152,8 +152,6 @@ int dp_power_client_init(struct dp_power *dp_power) > >> power = container_of(dp_power, struct dp_power_private, dp_power); > >> - pm_runtime_enable(power->dev); > >> - > >> return dp_power_clk_init(power); > >> } > >> @@ -162,8 +160,6 @@ void dp_power_client_deinit(struct dp_power > >> *dp_power) > >> struct dp_power_private *power; > >> power = container_of(dp_power, struct dp_power_private, dp_power); > >> - > >> - pm_runtime_disable(power->dev); > >> } > >> int dp_power_init(struct dp_power *dp_power) > >> @@ -173,11 +169,7 @@ int dp_power_init(struct dp_power *dp_power) > >> power = container_of(dp_power, struct dp_power_private, dp_power); > >> - pm_runtime_get_sync(power->dev); > >> - > >> rc = dp_power_clk_enable(dp_power, DP_CORE_PM, true); > >> - if (rc) > >> - pm_runtime_put_sync(power->dev); > >> return rc; > >> } > >> @@ -189,7 +181,6 @@ int dp_power_deinit(struct dp_power *dp_power) > >> power = container_of(dp_power, struct dp_power_private, dp_power); > >> dp_power_clk_enable(dp_power, DP_CORE_PM, false); > >> - pm_runtime_put_sync(power->dev); > >> return 0; > >> } > > -- With best wishes Dmitry
