Re: [PATCH v3 6/7] drm/msm/dp: add pm_runtime_force_suspend()/resume()

[Stephen Boyd]

Quoting Kuogee Hsieh (2023-09-25 09:07:18)
>
> On 9/22/2023 6:35 PM, Abhinav Kumar wrote:
> >
> > Doing link training when we get hpd instead of atomic_enable() is a
> > design choice we have been following for a while because for the case
> > when link training fails in atomic_enable() and setting the link
> > status property as you mentioned, the compositor needs to be able to
> > handle that and also needs to try with a different resolution or take
> > some other corrective action. We have seen many compositors not able
> > to handle this complexity. So the design sends the hotplug to usermode
> > only after link training succeeds.
> >
> > I do not think we should change this design unless prototyped with an
> > existing compositor such as chrome or android at this point.
> >
> > Thanks
> >
> > Abhinav
>
>
> We did perform link training at atomic_enable() at eDP case since we can
> assume link training will always success without link rate or link lane
> being reduced.
>
> However for external DP case, link training can not be guarantee always
> success without link rate or lane being reduced as Abhinav mentioned.
>
> In addition,  CTS (compliance test) it required to complete link
> training within 10ms after hpd asserted.

Is it possible to change that timeout? I have to look around for the CTS
parameters because I'm pretty confused how it can work. What do we do if
DP wakes the system from suspend and asserts HPD? We need resume time to
be < 10ms?  That's not realistic.

>
> I am not sure do link training at atomic_enable() can meet this timing
> requirement.
>

At least in the DP spec itself it doesn't require the link to be trained
within 10ms of HPD being asserted. Instead it simply recommends that the
OS start configuring the display promptly after HPD is asserted, e.g.
within 100ms. There's some strict timing on IRQ_HPD, so the driver must
read DPCD registers within 100ms of IRQ_HPD rising edge; maybe that is
what CTS is checking for?

TL;DR: I don't see why CTS should stop us from link training in
atomic_enable(). It would be beneficial to do so to make eDP and DP the
same. It would also help to report a drm connector being connected
_before_ link training so that userspace knows the link itself is the
bad part of the equation (and not that the DP connector looks
disconnected to userspace when in fact it really is connected and the
monitor is asserting HPD, just the link training failed).