[Intel-gfx] [PATCH 1/3] drm/i915/dp_link_training: Add a final failing state to link training fallback

[Manasi Navare]

Thank you for the patch and all your work to improve the link training
fallback logic and
to correctly reflect the link status to the userspace.

Chiming in some of the findings and this logic justification below so
it will help
the reviewers further.


On Fri, Aug 18, 2023 at 10:02 AM Gil Dekel <gildekel at chromium.org> wrote:
>
> Instead of silently giving up when all link-training fallback values are
> exhausted, this patch modifies the fallback's failure branch to reduces
> both max_link_lane_count and max_link_rate to zero (0) and continues to
> emit uevents until userspace stops attempting to modeset.
>
> By doing so, we ensure the failing connector, which is in
> link-status=Bad, has all its modes pruned (due to effectively having a
> bandwidth of 0Gbps).
>

This is critical to correctly propagate the final link training
failure to the userspace instead
of just failing with an error message in the kernel.
This definitely completes the link training fallback logic by making
sure that if we have exhausted
all the link rate/lane count combinations and the physical link is
still failing to link train,
then the effective available link BW is marked as 0 so that all the
modes get pruned.

This correctly reflects the state of a connector which is connected
with essentially a bad link
and cannot display any mode and that display remains dark.

Regards
Manasi


> It is then the userspace's responsibility to ignore connectors with no
> modes, even if they are marked as connected.
>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Manasi Navare <navaremanasi at chromium.org>
> Cc: Sean Paul <seanpaul at chromium.org>
> Signed-off-by: Gil Dekel <gildekel at chromium.org>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 964bf0551bdc..1e4dae8aad90 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -276,8 +276,12 @@ static int intel_dp_common_len_rate_limit(const struct intel_dp *intel_dp,
>
>  static int intel_dp_common_rate(struct intel_dp *intel_dp, int index)
>  {
> +       /* This occurs when max link rate drops to 0 via link training fallback*/
> +       if (index < 0)
> +               return 0;
> +
>         if (drm_WARN_ON(&dp_to_i915(intel_dp)->drm,
> -                       index < 0 || index >= intel_dp->num_common_rates))
> +                       index >= intel_dp->num_common_rates))
>                 return 162000;
>
>         return intel_dp->common_rates[index];
> @@ -318,6 +322,9 @@ static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
>  int intel_dp_max_lane_count(struct intel_dp *intel_dp)
>  {
>         switch (intel_dp->max_link_lane_count) {
> +       /* This occurs when max link lane count drops to 0 via link training fallback*/
> +       case 0:
> +               return 0;
>         case 1:
>         case 2:
>         case 4:
> @@ -672,7 +679,14 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>                 intel_dp->max_link_lane_count = lane_count >> 1;
>         } else {
>                 drm_err(&i915->drm, "Link Training Unsuccessful\n");
> -               return -1;
> +               /*
> +                * Ensure all of the connector's modes are pruned in the next
> +                * probe by effectively reducing its bandwidth to 0 so userspace
> +                * can ignore it within the next modeset attempt.
> +                */
> +               intel_dp->max_link_rate = 0;
> +               intel_dp->max_link_lane_count = 0;
> +               return 0;
>         }
>
>         return 0;
> --
> Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics