Re: [PATCH v2 03/38] drm/msm/dp: break up dp_display_enable into two parts

[Yongxing Mou <quic_yongmou@quicinc.com>]

On 2025/8/13 20:59, Dmitry Baryshkov wrote:
> On Wed, Aug 13, 2025 at 05:36:10PM +0800, Yongxing Mou wrote:
>>
>>
>> On 2025/6/9 20:59, Dmitry Baryshkov wrote:
>>> On Mon, Jun 09, 2025 at 08:21:22PM +0800, Yongxing Mou wrote:
>>>> From: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>>>
>>>> dp_display_enable() currently re-trains the link if needed
>>>> and then enables the pixel clock, programs the controller to
>>>> start sending the pixel stream. Splite these two parts into
>>>> prepare/enable APIs, to support MST bridges_enable inserte
>>>
>>> typos
>>>
>>>> the MST payloads funcs between enable stream_clks and programe
>>>> register.
>>>>
>>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>>> Signed-off-by: Yongxing Mou <quic_yongmou@quicinc.com>
>>>> ---
>>>>    drivers/gpu/drm/msm/dp/dp_ctrl.c    | 57 +++++++++++++--------
>>>>    drivers/gpu/drm/msm/dp/dp_ctrl.h    |  3 +-
>>>>    drivers/gpu/drm/msm/dp/dp_display.c | 99 +++++++++++++++++++++++++++----------
>>>>    drivers/gpu/drm/msm/dp/dp_display.h |  1 +
>>>>    4 files changed, 111 insertions(+), 49 deletions(-)
>>>>
> 
>>>> @@ -831,7 +831,37 @@ static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display,
>>>>    	return 0;
>>>>    }
>>>> -static int msm_dp_display_enable(struct msm_dp_display_private *dp, bool force_link_train)
>>>> +static int msm_dp_display_prepare(struct msm_dp_display_private *dp)
>>>> +{
>>>> +	int rc = 0;
>>>> +	struct msm_dp *msm_dp_display = &dp->msm_dp_display;
>>>> +	bool force_link_train = false;
>>>> +
>>>> +	drm_dbg_dp(dp->drm_dev, "sink_count=%d\n", dp->link->sink_count);
>>>> +	if (msm_dp_display->prepared) {
>>>> +		drm_dbg_dp(dp->drm_dev, "Link already setup, return\n");
>>>> +		return 0;
>>>> +	}
>>>
>>> How can it be prepared here? It is called at the beginning of the
>>> .atomic_enable() only, so there is no way this can be true.
>>>
>> Emm, sorry for forget this case.. Whern MST enabled,
>> msm_dp_display_prepare() will be called from mst_bridge_atomic_pre_enable,
>> that means, when second stream called this func, it already prepared, so we
>> should skip here. so this condition will really hit in MST case..
> 
> Then it should be refcounted. And, ideally, this should come later as a
> part of one of MST-enablement patches, when it actually makes sense
> 
Okay. I will move this part to appropriate patch.
>>>> +
>>>> +	rc = pm_runtime_resume_and_get(&msm_dp_display->pdev->dev);
>>>> +	if (rc) {
>>>> +		DRM_ERROR("failed to pm_runtime_resume\n");
>>>> +		return rc;
>>>> +	}
>>>> +
>>>> +	if (dp->hpd_state == ST_CONNECTED && !msm_dp_display->power_on) {
>>>> +		msm_dp_display_host_phy_init(dp);
>>>> +		force_link_train = true;
>>>> +	}
>>>> +
>