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

[Dmitry Baryshkov]

On Thu, 28 Sept 2023 at 01:01, Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
>
> 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).

Also this will move us closer to i915 user experience: failing link
training in the display enablement part. So that a part of xrandr
calls can retry link training.

-- 
With best wishes
Dmitry