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

[Abhinav Kumar]

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 at quicinc.com>
> 
> The code looks OK to me, so
> 
> Reviewed-by: Stephen Boyd <swboyd at chromium.org>
> 
> once the commit text is cleaned up to indicate the proper function
> names.

Thanks , will fix all those.