On Thu, Jan 15, 2026 at 08:57:24AM -0600, Bjorn Andersson wrote: > On Thu, Jan 15, 2026 at 09:29:08AM +0200, Dmitry Baryshkov wrote: > > From: Jessica Zhang <[email protected]> > > > > Instead of relying on the link_ready flag to specify if DP is connected, > > read the DPCD bits and get the sink count to accurately detect if DP is > > connected. > > This makes it sounds like the two options are equal, but they most > definitely aren't. > > I think this commit message should capture the fact that "link_ready" > not only says that the cable is connected, but that we've managed to > bring up the main link - which is a source of race conditions in the hot > plug detection logic, as well as making it impossible to move link > management to the enable/disable calls. > > > > > Signed-off-by: Jessica Zhang <[email protected]> > > Signed-off-by: Dmitry Baryshkov <[email protected]> > > --- > > drivers/gpu/drm/msm/dp/dp_display.c | 60 > > +++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/msm/dp/dp_drm.c | 20 ------------- > > drivers/gpu/drm/msm/dp/dp_drm.h | 2 ++ > > 3 files changed, 62 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > > b/drivers/gpu/drm/msm/dp/dp_display.c > > index 5997cd28ba11..a05144de3b93 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_display.c > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > > @@ -1151,6 +1151,66 @@ static int msm_dp_hpd_event_thread_start(struct > > msm_dp_display_private *msm_dp_p > > return 0; > > } > > > > +/** > > + * msm_dp_bridge_detect - callback to determine if connector is connected > > + * @bridge: Pointer to drm bridge structure > > + * @connector: Pointer to drm connector structure > > + * Returns: Bridge's 'is connected' status > > Could you please rewrite the return definition, to capture what the > value really refers to. > > > + */ > > +enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge, > > + struct drm_connector *connector) > > +{ > > + struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(bridge); > > + struct msm_dp *dp = msm_dp_bridge->msm_dp_display; > > + struct msm_dp_display_private *priv; > > + int ret = 0; > > First usage is an assignment, so no need for the zero-initialization. > > > + int status = connector_status_disconnected; > > + u8 dpcd[DP_RECEIVER_CAP_SIZE]; > > + struct drm_dp_desc desc; > > + > > + dp = to_dp_bridge(bridge)->msm_dp_display; > > + > > + priv = container_of(dp, struct msm_dp_display_private, msm_dp_display); > > + > > + if (!dp->link_ready) > > + return status; > > So despite the commit message, we're still relying on the link_ready > flag? (With the improvement that even if the code thinks we've trained > the link, we can still determine that we should report it as > disconnected) > > Perhaps I'm missing something here? Did we change the meaning of > "link_ready"?
Not yet. It changes in the next commit (and I should probably add a commit renaming it). Note, before the next commit (moving link training) we can't completely change detect() definition, but we also can't move link training if we don'g have a proper detect() at that time. I agree with Jessica's decision here to have two separate commits: this one adds (imperfect) detect(), the next one moves link training. > Other than this part, this looks quite familiar to my experiments. Very > happy to see you continue this work!!! It has been on my plate for quite a while. Let's finally get it done. -- With best wishes Dmitry
