RE: [RFC 4/4] drm/i915/display/dp: On LT failure retry LT

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 14 Feb 2024, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Tuesday, February 13, 2024 11:45 PM
>> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-gfx@lists.freedesktop.org
>> Cc: Deak, Imre <imre.deak@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
>> Shankar, Uma <uma.shankar@intel.com>; Murthy, Arun R
>> <arun.r.murthy@intel.com>
>> Subject: Re: [RFC 4/4] drm/i915/display/dp: On LT failure retry LT
>>
>> On Tue, 06 Feb 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
>> > On link training failure retry link training with a lesser link
>> > rate/lane count as specified in the DP spec.
>> >
>> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_ddi.c | 10 +++++++++-
>> >  1 file changed, 9 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>> > b/drivers/gpu/drm/i915/display/intel_ddi.c
>> > index ed7620e7f763..29d785a4b904 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> > @@ -2502,6 +2502,7 @@ static void mtl_ddi_pre_enable_dp(struct
>> intel_atomic_state *state,
>> >                              crtc_state->port_clock,
>> >                              crtc_state->lane_count);
>> >
>> > +retry:
>> >     /*
>> >      * We only configure what the register value will be here.  Actual
>> >      * enabling happens during link training farther down.
>> > @@ -2586,7 +2587,14 @@ static void mtl_ddi_pre_enable_dp(struct
>> intel_atomic_state *state,
>> >      *     Pattern, wait for 5 idle patterns (DP_TP_STATUS Min_Idles_Sent)
>> >      *     (timeout after 800 us)
>> >      */
>> > -   intel_dp_start_link_train(intel_dp, crtc_state);
>> > +   if (!intel_dp_start_link_train(intel_dp, crtc_state)) {
>> > +           /* Link Training failed, retain */
>> > +           intel_dp->link_trained = false;
>> > +           intel_dp_stop_link_train(intel_dp, crtc_state);
>> > +           encoder->post_disable(state, encoder,
>> > +                              crtc_state, conn_state);
>> > +           goto retry;
>> > +   }
>>
>> As said, the retry needs to go via userspace.
>
> If within the supported mode range then also do we need to send uevent to user and should it come via userspace?
> The fallback mandates in DP2.1 spec does this fallback in a loop.
>
> The present fallback structure
> Struct dp_fallback {
>         U32 link rate;
>         U8 lane_count;
>         U32 resolution;
> }
>
> In the same fallback code, the present mode will be verified to see if its less than or equal to the resolution in dp_fallback. If so proceed within the fallback loop else set the max link_rate/lane count values and sent uevent.

I think I'll want *all* the link training fallbacks to go via userspace.

Trying to sometimes do it in kernel is a premature optimization for a
rare case, and it just complicates matters. We'll need the path via
uevent and userspace retry anyway, for when the mode doesn't fit, so use
it always. Let's not add multiple ways to do things, everything around
this is already quite complicated.

And as said, the uevent does give userspace some inkling that something
fishy is going on, and could use that info to inform the user that a
degraded experience may be expected. Again, adding a new stream to MST
at a later time might not fit because of the reduced parameters, and
it'll be surprising to the user if it used to work in the past (when
full param link training succeeded).

BR,
Jani.

>
> Thanks and Regards,
> Arun R Murthy
> --------------------
>>
>> BR,
>> Jani.
>>
>>
>> >
>> >     /* 6.n Set DP_TP_CTL link training to Normal */
>> >     if (!is_trans_port_sync_mode(crtc_state))
>>
>> --
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel