On 4/3/2024 1:04 PM, Stephen Boyd wrote:
Quoting Abhinav Kumar (2024-04-03 12:58:50)
On 4/3/2024 12:51 PM, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2024-03-29 12:50:35)
Currently qmp_combo_dp_power_on() always return 0 in regardless of
return value of cfg->configure_dp_phy(). This patch propagate
return value of cfg->configure_dp_phy() all the way back to caller.
Is this found via code inspection or because the phy is failing to power
on sometimes? I ask because I'm looking at a DP bug on Trogdor with
chromeos' v6.6 based kernel and wondering if this is related.
No, we actually hit an issue. This issue was originally reported as a
link training issue while bringing up DP on x1e80100.
While debugging that we noticed that we should not have even proceeded
to link training because the PLL was not getting locked and it was
failing silently since there are no other error prints (and hence the
second part of the patch to improve the error logs), and we do not
return any error code from this driver, we could not catch the PLL
unlocked issue.
Did link training succeed in that case and the screen was black? Also,
did you figure out why the PLL failed to lock? I sometimes see reports
now with an "Unexpected interrupt:" message from the DP driver and the
interrupt is the PLL unlocked one (DP_INTR_PLL_UNLOCKED).
No the link training had failed.
Yes, root-cause was that the PLL registers were misconfigured in the
x1e80100 DP PHY for HBR2. Once we programmed the correct values it
worked. This was specific to x1e80100.
Yes, Doug mentioned this to me on IRC that this issue is still there.
Surprising because I thought we had pushed
https://patchwork.freedesktop.org/patch/551847/ long ago and it was
fixed. It certainly did that time when I had tested this.
Also, is the call to phy_power_on() going to be checked in
the DP driver?
$ git grep -n phy_power_on -- drivers/gpu/drm/msm/dp/
drivers/gpu/drm/msm/dp/dp_ctrl.c:1453: phy_power_on(phy);
Yes, this is a good point. We should also post the patch to add the
error checking in DP driver to fail if phy_power_on fails.
Sounds great, thanks.
