Re: [PATCH] drm/msm/dp: check core_initialized flag at both host_init() and host_deinit()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Stephen

On 2/27/2023 11:53 AM, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2023-02-24 10:29:58)
There is a reboot/suspend test case where system suspend is forced during
system booting up. Since host_init() of external DP is executed at hpd

dp_display_host_init()?

thread context, this test case may created a scenario that host_deinit()

dp_display_host_deinit()?

ack for both.
from pm_suspend() run before host_init() if hpd thread has no chance to
run during booting up while suspend request command was issued.
At this scenario system will crash at aux register access at host_deinit()
since aux clock had not yet been enabled by host_init().  Therefore we

The aux clk is enabled in dp_power_clk_enable() right? Can you clarify?


Yes, thats right. Its mapped to core_*** in the dts hence the name goes to the DP_CORE_PM case in the dp_power_clk_enable()

3092 				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
3093 					 <&dispcc DISP_CC_MDSS_DP_AUX_CLK>,
3094 					 <&dispcc DISP_CC_MDSS_DP_LINK_CLK>,
3095 					 <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>,
3096 					 <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>;
3097 				clock-names = "core_iface", "core_aux"

have to ensure aux clock enabled by checking core_initialized flag before
access aux registers at pm_suspend.

I'd much more like to get rid of 'core_initialized'. What is preventing
us from enabling the power (i.e. dp_power_init()), or at least enough
clks and pm runtime state, during probe? That would fix this problem and
also clean things up. As I understand, the device is half initialized in
probe and half initialized in the kthread. If we put all power
management into the runtime PM ops and synced that state during probe so
that runtime PM state matched device probe state we could make runtime
PM be the only suspend function and then push the power state tracking
into the device core.


You are correct. https://patchwork.freedesktop.org/patch/523879/ will be doing that. Its still in review.


Fixes: 989ebe7bc446 ("drm/msm/dp: do not initialize phy until plugin interrupt received")
Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>

The code looks OK to me, so

Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>

once the commit text is cleaned up to indicate the proper function
names.

Thanks , will fix all those.



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux