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

[Yongxing Mou]

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@xxxxxxxxxxx>
> > > >
> > > > 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@xxxxxxxxxxx>
> > > > Signed-off-by: Yongxing Mou <quic_yongmou@xxxxxxxxxxx>
> > > > ---
> > > >    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;
> > > > +	}
> > > > +