| Message ID | 20190326103146.24795-12-tomi.valkeinen@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/bridge: tc358767: DP support | expand |
On 26.03.2019 11:31, Tomi Valkeinen wrote: > Link training will sometimes fail if the DP link is, for some whatever > reason, enabled when tc_main_link_enable() is called. Only tc_stream_enable enables it, does it mean that link training can happen after tc_stream_enable? It suggests that driver/device preforms strange things, is it true? Or just overprotection? Regards Andrzej > > Ensure that the DP link is disabled by setting DP0CTL to 0 as the first > thing. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/bridge/tc358767.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index fe5fd7db0247..f628575c9de9 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -839,6 +839,8 @@ static int tc_main_link_enable(struct tc_data *tc) > > dev_dbg(tc->dev, "link enable\n"); > > + tc_write(DP0CTL, 0); > + > tc_write(DP0_SRCCTRL, tc_srcctrl(tc)); > /* SSCG and BW27 on DP1 must be set to the same as on DP0 */ > tc_write(DP1_SRCCTRL,
On 15/04/2019 11:49, Andrzej Hajda wrote: > On 26.03.2019 11:31, Tomi Valkeinen wrote: >> Link training will sometimes fail if the DP link is, for some whatever >> reason, enabled when tc_main_link_enable() is called. > > > Only tc_stream_enable enables it, does it mean that link training can > happen after tc_stream_enable? > > It suggests that driver/device preforms strange things, is it true? Or > just overprotection? Just protection. I did try all kinds of things when trying to get the link setup stable and having DP0CTL enabled before link training was one of the problems I encountered. In theory DP0CTL should always be disabled when we call tc_main_link_enable, but I thought it best leave it there in case we accidentally leave DP0CTL enabled via some error path or such. Maybe we should have a WARN there if DP0CTL is enabled (and then clear it), so that we might find those error cases. Tomi
Hi Tomi, On Mon, Apr 15, 2019 at 02:26:20PM +0300, Tomi Valkeinen wrote: > On 15/04/2019 11:49, Andrzej Hajda wrote: > > On 26.03.2019 11:31, Tomi Valkeinen wrote: > >> Link training will sometimes fail if the DP link is, for some whatever > >> reason, enabled when tc_main_link_enable() is called. > > > > Only tc_stream_enable enables it, does it mean that link training can > > happen after tc_stream_enable? > > > > It suggests that driver/device preforms strange things, is it true? Or > > just overprotection? > > Just protection. I did try all kinds of things when trying to get the > link setup stable and having DP0CTL enabled before link training was one > of the problems I encountered. > > In theory DP0CTL should always be disabled when we call > tc_main_link_enable, but I thought it best leave it there in case we > accidentally leave DP0CTL enabled via some error path or such. > > Maybe we should have a WARN there if DP0CTL is enabled (and then clear > it), so that we might find those error cases. I'd prefer a warning, as incorrect error paths (or such) could also create lots of other issues. I don't think we should protect against one particular issue that is supposed never to happen and then consider that we're safe.
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index fe5fd7db0247..f628575c9de9 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -839,6 +839,8 @@ static int tc_main_link_enable(struct tc_data *tc) dev_dbg(tc->dev, "link enable\n"); + tc_write(DP0CTL, 0); + tc_write(DP0_SRCCTRL, tc_srcctrl(tc)); /* SSCG and BW27 on DP1 must be set to the same as on DP0 */ tc_write(DP1_SRCCTRL,
Link training will sometimes fail if the DP link is, for some whatever reason, enabled when tc_main_link_enable() is called. Ensure that the DP link is disabled by setting DP0CTL to 0 as the first thing. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/bridge/tc358767.c | 2 ++ 1 file changed, 2 insertions(+)