Re: [Intel-gfx] [PATCH v2 08/12] drm/i915/dp: Convert link training error to debug message on disconnected sink

["Govindapillai, Vinod" <vinod.govindapillai@intel.com>]

On Thu, 2023-05-04 at 02:10 +0300, Imre Deak wrote:
> If a sink is disconnected it's expected that link training actions will
> fail on it, so downgrade the error messages about such actions to be a
> debug message. Such - expected - link training failures are more
> frequent after a follow up patch, after which an active TypeC link is
> reset after the sink is disconnected which also involves a link
> training.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c               |  2 +-
>  drivers/gpu/drm/i915/display/intel_dp.h               |  1 +
>  drivers/gpu/drm/i915/display/intel_dp_link_training.c | 11 +++++++++--
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4361c1ac65c3a..1942a05719776 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4179,7 +4179,7 @@ static int intel_dp_prep_link_retrain(struct intel_dp *intel_dp,
>         return ret;
>  }
>  
> -static bool intel_dp_is_connected(struct intel_dp *intel_dp)
> +bool intel_dp_is_connected(struct intel_dp *intel_dp)
>  {
>         struct intel_connector *connector = intel_dp->attached_connector;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index ef39e4f7a329e..488da392fafe5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -42,6 +42,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
>                               int link_rate, int lane_count);
>  int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>                                             int link_rate, u8 lane_count);
> +bool intel_dp_is_connected(struct intel_dp *intel_dp);
>  int intel_dp_retrain_link(struct intel_encoder *encoder,
>                           struct drm_modeset_acquire_ctx *ctx);
>  void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index f83944eaac128..2fe076e2d64b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -33,6 +33,7 @@ static void lt_msg(struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
>         struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>         struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>         struct intel_connector *connector = intel_dp->attached_connector;
> +       const char *discon_str = "";
>         struct va_format vaf;
>         va_list args;
>  
> @@ -43,6 +44,11 @@ static void lt_msg(struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
>         vaf.fmt = format;
>         vaf.va = &args;
>  
> +       if (is_error && !intel_dp_is_connected(intel_dp)) {
> +               discon_str = " (sink disconnected)";
> +               is_error = false;
> +       }
> +
Hi imre,

If the ignore long HPD env variable is set, in case of spurious HPDs we will ignore the long HPDs
and the connector status remains unchanged. So don't we need to downgrade the error messages to
debug messages if dev_priv->display.hotplug.ignore_long_hpd is set?

BR
vinod

>         if (is_error)
>                 drm_err(&i915->drm, "[CONNECTOR:%d:%s][ENCODER:%d:%s][%s] %pV\n",
>                         connector->base.base.id, connector->base.name,
> @@ -50,11 +56,12 @@ static void lt_msg(struct intel_dp *intel_dp, enum drm_dp_phy dp_phy,
>                         drm_dp_phy_name(dp_phy),
>                         &vaf);
>         else
> -               drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s][ENCODER:%d:%s][%s] %pV\n",
> +               drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s][ENCODER:%d:%s][%s] %pV%s\n",
>                             connector->base.base.id, connector->base.name,
>                             encoder->base.base.id, encoder->base.name,
>                             drm_dp_phy_name(dp_phy),
> -                           &vaf);
> +                           &vaf,
> +                           discon_str);
>  }
>  
>  #define lt_err(intel_dp, dp_phy, format, ...) \