| Message ID | 20230818170156.2194015-2-gildekel@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Define a final failure state when link training fails | expand |
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@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@linux.intel.com> > Cc: Manasi Navare <navaremanasi@chromium.org> > Cc: Sean Paul <seanpaul@chromium.org> > Signed-off-by: Gil Dekel <gildekel@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
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;
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). 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@linux.intel.com> Cc: Manasi Navare <navaremanasi@chromium.org> Cc: Sean Paul <seanpaul@chromium.org> Signed-off-by: Gil Dekel <gildekel@chromium.org> --- drivers/gpu/drm/i915/display/intel_dp.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) -- Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics