On Tue, Aug 18, 2026 at 11:48:58AM +0200, Konrad Dybcio wrote:
> On 8/8/26 7:13 PM, Jesse Casco wrote:
> > msm_dp_display_atomic_enable() returns early when link training fails,
> > leaving ->power_on false and the main link down.
> > msm_dp_display_atomic_disable() nevertheless writes DP_STATE_CTRL_PUSH_IDLE
> > and waits for an idle-pattern completion that cannot arrive, so every failed
> > enable is followed by "PUSH_IDLE pattern timedout".
> > 
> > Every other step of the teardown is already gated on that flag:
> > msm_dp_display_disable(), called from .atomic_post_disable(), returns early
> > on !power_on. The PUSH_IDLE write is the only one that is not, so the
> > controller's runtime-PM reference is then dropped without the link having
> > been taken down.
> > 
> > On glymur (Snapdragon X2 Elite) the consequence is not a warning. The SoC
> > does not survive it: TrustZone force-stops the SOCCP and ADSP remote
> > processors and the machine resets silently about 50 ms later, with no oops
> > and no panic. On an ASUS Zenbook A16 (UX3607OA), whose eDP panel does not
> > currently train, this reproduces without any compositor or GPU involvement:
> > 
> 
> > +   /*
> > +    * If .atomic_enable() bailed out - link training failure is the common
> > +    * case - the mainlink was never brought up and ->power_on stayed false.
> > +    * Driving the PUSH_IDLE pattern into a controller that was never
> > +    * enabled times out, and .atomic_post_disable() then drops the
> > +    * controller's runtime-PM reference without tearing the PHY back down,
> > +    * because msm_dp_display_disable() returns early on !power_on.  On
> > +    * glymur (Snapdragon X2 Elite) that combination is answered by a
> > +    * TrustZone-level SOCCP/ADSP force-stop and a silent SoC reset.
> > +    * There is nothing to push idle, so leave it alone.
> > +    */
> > +   if (!dp->power_on)
> > +           return;
> 
> Dmitry, should we even be reaching atomic_disable if lt fails before?

Unfortuately. The atomic_disable() always comes after atomic_enable(),
there is no way to fail the enable path (you can see the outcome in
other drivers where the _enable path ends up setting flags which
_disable path checks before shutting down corresponding hardware).

Reviewed-by: Dmitry Baryshkov <[email protected]>



-- 
With best wishes
Dmitry

Reply via email to