linux-kernel - Re: [PATCH v3 4/8] drm/msm/dp: Move link training to atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5u3th3xw5rbaokoky3zhqhirbmfjljd32oqvyprabkx3xy4uo@tmc2l5domvpk>
Date: Thu, 15 Jan 2026 09:39:29 -0600
From: Bjorn Andersson <andersson@...nel.org>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>, 
	Rob Clark <robin.clark@....qualcomm.com>, Dmitry Baryshkov <lumag@...nel.org>, 
	Abhinav Kumar <abhinav.kumar@...ux.dev>, Jessica Zhang <jesszhan0024@...il.com>, 
	Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Kuogee Hsieh <quic_khsieh@...cinc.com>, linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	Jessica Zhang <jessica.zhang@....qualcomm.com>
Subject: Re: [PATCH v3 4/8] drm/msm/dp: Move link training to atomic_enable()

On Thu, Jan 15, 2026 at 10:30:49AM +0100, Konrad Dybcio wrote:
> On 1/15/26 8:29 AM, Dmitry Baryshkov wrote:
> > From: Jessica Zhang <jessica.zhang@....qualcomm.com>
> > 
> > Currently, the DP link training is being done during HPD. Move
> > link training to atomic_enable() in accordance with the atomic_enable()
> > documentation.
> > 
> > Link disabling is already done in atomic_post_disable() (as part of the
> > dp_ctrl_off_link_stream() helper).
> > 
> > Finally, call the plug/unplug handlers directly in hpd_notify() instead
> > of queueing them in the event thread so that they aren't preempted by
> > other events.
> 
> I think this part of the commit message no longer applies 
> 
> > 
> > Signed-off-by: Jessica Zhang <jessica.zhang@....qualcomm.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
> > ---
> >  drivers/gpu/drm/msm/dp/dp_display.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> > index a05144de3b93..3184066adb15 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > @@ -436,11 +436,6 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
> >  	msm_dp_link_psm_config(dp->link, &dp->panel->link_info, false);
> >  
> >  	msm_dp_link_reset_phy_params_vx_px(dp->link);
> > -	rc = msm_dp_ctrl_on_link(dp->ctrl);
> > -	if (rc) {
> > -		DRM_ERROR("failed to complete DP link training\n");
> > -		goto end;
> > -	}
> >  
> >  	msm_dp_add_event(dp, EV_USER_NOTIFICATION, true, 0);
> >  
> > @@ -1695,6 +1690,12 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge,
> >  		force_link_train = true;
> >  	}
> >  
> > +	rc = msm_dp_ctrl_on_link(msm_dp_display->ctrl);
> > +	if (rc) {
> > +		DRM_ERROR("Failed link training (rc=%d)\n", rc);
> > +		drm_connector_set_link_status_property(dp->connector, DRM_LINK_STATUS_BAD);
> > +	}
> 
> Previously failure in this call would have propagated an error
> 
> msm_dp_ctrl_on_link() enables mainlink clocks, so 
> 

Missed that one. I guess in practice it's not much difference from the
previous behavior - where the display just wouldn't work either, except
for the crash you point out below.

It seems to be that it would make sense to return an error to userspace
when this happens, but the kernel-doc says:

"""
The bridge can assume that the display pipe (i.e. clocks and timing
signals) feeding it is running when this callback is called.
"""

What's the expected error path here?

Regards,
Bjorn

> > +
> >  	msm_dp_display_enable(msm_dp_display, force_link_train);
> 
> IIUC this will crash the machine if the above fails
> 
> Konrad
>