intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
@ 2020-06-04  0:23 Manasi Navare
  2020-06-04  0:49 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Manasi Navare @ 2020-06-04  0:23 UTC (permalink / raw)
  To: intel-gfx

We have noticed that when link training fails the panel
sends a long pulse indicating connector disconnect. In this case
we need to reset the link parameters instead of continuing
to use the fallback parameters since else this long pulse
by the panel followed by a modeset request which was triggered by the userspace
before getting the connector status as disconnected, will
result into a modeset now using lower link rate/lane count values.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1385
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 28 +++++++++++++++++--------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 55fda074c0ad..f7af372647dd 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6111,6 +6111,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 	intel_dp->edid_quirks = 0;
 }
 
+static void
+intel_dp_reset_link_params(struct intel_dp *intel_dp)
+{
+	/* Initial max link lane count */
+	intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
+
+	/* Initial max link rate */
+	intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
+
+	intel_dp->reset_link_params = false;
+}
+
 static int
 intel_dp_detect(struct drm_connector *connector,
 		struct drm_modeset_acquire_ctx *ctx,
@@ -6139,6 +6151,11 @@ intel_dp_detect(struct drm_connector *connector,
 		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
 		memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
 
+		/*Reset the immutable VRR Capable property */
+		drm_connector_set_vrr_capable_property(connector,
+						       false);
+		intel_dp_reset_link_params(intel_dp);
+
 		if (intel_dp->is_mst) {
 			drm_dbg_kms(&dev_priv->drm,
 				    "MST device may have disappeared %d vs %d\n",
@@ -6152,15 +6169,8 @@ intel_dp_detect(struct drm_connector *connector,
 		goto out;
 	}
 
-	if (intel_dp->reset_link_params) {
-		/* Initial max link lane count */
-		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
-
-		/* Initial max link rate */
-		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
-
-		intel_dp->reset_link_params = false;
-	}
+	if (intel_dp->reset_link_params)
+		intel_dp_reset_link_params(intel_dp);
 
 	intel_dp_print_rates(intel_dp);
 
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dp: Reset link params on connector disconnect
  2020-06-04  0:23 [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect Manasi Navare
@ 2020-06-04  0:49 ` Patchwork
  2020-06-04  1:19 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  2020-06-04 15:25 ` [Intel-gfx] [PATCH] " Ville Syrjälä
  2 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2020-06-04  0:49 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dp: Reset link params on connector disconnect
URL   : https://patchwork.freedesktop.org/series/77974/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
01ef52e81850 drm/i915/dp: Reset link params on connector disconnect
-:13: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#13: 
by the panel followed by a modeset request which was triggered by the userspace

total: 0 errors, 1 warnings, 0 checks, 46 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dp: Reset link params on connector disconnect
  2020-06-04  0:23 [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect Manasi Navare
  2020-06-04  0:49 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2020-06-04  1:19 ` Patchwork
  2020-06-04 15:25 ` [Intel-gfx] [PATCH] " Ville Syrjälä
  2 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2020-06-04  1:19 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dp: Reset link params on connector disconnect
URL   : https://patchwork.freedesktop.org/series/77974/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8579 -> Patchwork_17862
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_17862 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_17862, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17862/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_17862:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-cml-u2:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-cml-u2/igt@kms_chamelium@dp-hpd-fast.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17862/fi-cml-u2/igt@kms_chamelium@dp-hpd-fast.html
    - fi-kbl-7500u:       [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-kbl-7500u/igt@kms_chamelium@dp-hpd-fast.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17862/fi-kbl-7500u/igt@kms_chamelium@dp-hpd-fast.html

  
Known issues
------------

  Here are the changes found in Patchwork_17862 that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * {igt@kms_flip@basic-flip-vs-wf_vblank@b-dvi-d1}:
    - fi-bwr-2160:        [FAIL][5] ([i915#1928]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8579/fi-bwr-2160/igt@kms_flip@basic-flip-vs-wf_vblank@b-dvi-d1.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17862/fi-bwr-2160/igt@kms_flip@basic-flip-vs-wf_vblank@b-dvi-d1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1928]: https://gitlab.freedesktop.org/drm/intel/issues/1928


Participating hosts (51 -> 9)
------------------------------

  ERROR: It appears as if the changes made in Patchwork_17862 prevented too many machines from booting.

  Missing    (42): fi-kbl-soraka fi-bdw-gvtdvm fi-icl-u2 fi-apl-guc fi-snb-2520m fi-icl-y fi-skl-lmem fi-byt-n2820 fi-icl-guc fi-icl-dsi fi-skl-6600u fi-snb-2600 fi-bxt-dsi fi-bdw-5557u fi-cml-s fi-tgl-u fi-bsw-n3050 fi-byt-j1900 fi-glk-dsi fi-hsw-4770 fi-ivb-3770 fi-elk-e7500 fi-kbl-7560u fi-tgl-y fi-bsw-nick fi-skl-6700k2 fi-kbl-r fi-ilk-m540 fi-ehl-1 fi-tgl-dsi fi-skl-guc fi-cfl-8700k fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-cfl-guc fi-kbl-guc fi-whl-u fi-kbl-x1275 fi-bsw-kefka fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_8579 -> Patchwork_17862

  CI-20190529: 20190529
  CI_DRM_8579: 289eb12c88c49a4ac8d325dc457d8878c7f5bdc0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5694: a9b6c4c74bfddf7d3d2da3be08804fe315945cea @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17862: 01ef52e818506813a16584526c1365017ca7aefa @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

01ef52e81850 drm/i915/dp: Reset link params on connector disconnect

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17862/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04  0:23 [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect Manasi Navare
  2020-06-04  0:49 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2020-06-04  1:19 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2020-06-04 15:25 ` Ville Syrjälä
  2020-06-04 18:35   ` Manasi Navare
  2 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2020-06-04 15:25 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

On Wed, Jun 03, 2020 at 05:23:59PM -0700, Manasi Navare wrote:
> We have noticed that when link training fails the panel
> sends a long pulse indicating connector disconnect. In this case
> we need to reset the link parameters instead of continuing
> to use the fallback parameters since else this long pulse
> by the panel followed by a modeset request which was triggered by the userspace
> before getting the connector status as disconnected, will
> result into a modeset now using lower link rate/lane count values.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1385
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 28 +++++++++++++++++--------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 55fda074c0ad..f7af372647dd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6111,6 +6111,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  	intel_dp->edid_quirks = 0;
>  }
>  
> +static void
> +intel_dp_reset_link_params(struct intel_dp *intel_dp)
> +{
> +	/* Initial max link lane count */
> +	intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> +
> +	/* Initial max link rate */
> +	intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> +
> +	intel_dp->reset_link_params = false;
> +}
> +
>  static int
>  intel_dp_detect(struct drm_connector *connector,
>  		struct drm_modeset_acquire_ctx *ctx,
> @@ -6139,6 +6151,11 @@ intel_dp_detect(struct drm_connector *connector,
>  		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
>  		memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
>  
> +		/*Reset the immutable VRR Capable property */
> +		drm_connector_set_vrr_capable_property(connector,
> +						       false);
> +		intel_dp_reset_link_params(intel_dp);
> +

Why would we care what those are when the sink is disconnected?

>  		if (intel_dp->is_mst) {
>  			drm_dbg_kms(&dev_priv->drm,
>  				    "MST device may have disappeared %d vs %d\n",
> @@ -6152,15 +6169,8 @@ intel_dp_detect(struct drm_connector *connector,
>  		goto out;
>  	}
>  
> -	if (intel_dp->reset_link_params) {
> -		/* Initial max link lane count */
> -		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> -
> -		/* Initial max link rate */
> -		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> -
> -		intel_dp->reset_link_params = false;
> -	}
> +	if (intel_dp->reset_link_params)
> +		intel_dp_reset_link_params(intel_dp);
>  
>  	intel_dp_print_rates(intel_dp);
>  
> -- 
> 2.19.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04 15:25 ` [Intel-gfx] [PATCH] " Ville Syrjälä
@ 2020-06-04 18:35   ` Manasi Navare
  2020-06-04 18:38     ` Ville Syrjälä
  0 siblings, 1 reply; 14+ messages in thread
From: Manasi Navare @ 2020-06-04 18:35 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Thu, Jun 04, 2020 at 06:25:43PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 03, 2020 at 05:23:59PM -0700, Manasi Navare wrote:
> > We have noticed that when link training fails the panel
> > sends a long pulse indicating connector disconnect. In this case
> > we need to reset the link parameters instead of continuing
> > to use the fallback parameters since else this long pulse
> > by the panel followed by a modeset request which was triggered by the userspace
> > before getting the connector status as disconnected, will
> > result into a modeset now using lower link rate/lane count values.
> > 
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1385
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 28 +++++++++++++++++--------
> >  1 file changed, 19 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 55fda074c0ad..f7af372647dd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -6111,6 +6111,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> >  	intel_dp->edid_quirks = 0;
> >  }
> >  
> > +static void
> > +intel_dp_reset_link_params(struct intel_dp *intel_dp)
> > +{
> > +	/* Initial max link lane count */
> > +	intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > +
> > +	/* Initial max link rate */
> > +	intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > +
> > +	intel_dp->reset_link_params = false;
> > +}
> > +
> >  static int
> >  intel_dp_detect(struct drm_connector *connector,
> >  		struct drm_modeset_acquire_ctx *ctx,
> > @@ -6139,6 +6151,11 @@ intel_dp_detect(struct drm_connector *connector,
> >  		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
> >  		memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> >  
> > +		/*Reset the immutable VRR Capable property */
> > +		drm_connector_set_vrr_capable_property(connector,
> > +						       false);
> > +		intel_dp_reset_link_params(intel_dp);
> > +
> 
> Why would we care what those are when the sink is disconnected?

We are noticing this happen in case the panel send this long pulse indicating
status change to disconnected, while the modeset was already triggered by userspace
in this case IGT, so the modeset continues right after i915_hotplug_work_fn
so we need to reset all params which fixes the bug mentioned.

Manasi

> 
> >  		if (intel_dp->is_mst) {
> >  			drm_dbg_kms(&dev_priv->drm,
> >  				    "MST device may have disappeared %d vs %d\n",
> > @@ -6152,15 +6169,8 @@ intel_dp_detect(struct drm_connector *connector,
> >  		goto out;
> >  	}
> >  
> > -	if (intel_dp->reset_link_params) {
> > -		/* Initial max link lane count */
> > -		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > -
> > -		/* Initial max link rate */
> > -		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > -
> > -		intel_dp->reset_link_params = false;
> > -	}
> > +	if (intel_dp->reset_link_params)
> > +		intel_dp_reset_link_params(intel_dp);
> >  
> >  	intel_dp_print_rates(intel_dp);
> >  
> > -- 
> > 2.19.1
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04 18:35   ` Manasi Navare
@ 2020-06-04 18:38     ` Ville Syrjälä
  2020-06-04 18:52       ` Manasi Navare
  0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2020-06-04 18:38 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

On Thu, Jun 04, 2020 at 11:35:30AM -0700, Manasi Navare wrote:
> On Thu, Jun 04, 2020 at 06:25:43PM +0300, Ville Syrjälä wrote:
> > On Wed, Jun 03, 2020 at 05:23:59PM -0700, Manasi Navare wrote:
> > > We have noticed that when link training fails the panel
> > > sends a long pulse indicating connector disconnect. In this case
> > > we need to reset the link parameters instead of continuing
> > > to use the fallback parameters since else this long pulse
> > > by the panel followed by a modeset request which was triggered by the userspace
> > > before getting the connector status as disconnected, will
> > > result into a modeset now using lower link rate/lane count values.
> > > 
> > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1385
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp.c | 28 +++++++++++++++++--------
> > >  1 file changed, 19 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 55fda074c0ad..f7af372647dd 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -6111,6 +6111,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > >  	intel_dp->edid_quirks = 0;
> > >  }
> > >  
> > > +static void
> > > +intel_dp_reset_link_params(struct intel_dp *intel_dp)
> > > +{
> > > +	/* Initial max link lane count */
> > > +	intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > > +
> > > +	/* Initial max link rate */
> > > +	intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > > +
> > > +	intel_dp->reset_link_params = false;
> > > +}
> > > +
> > >  static int
> > >  intel_dp_detect(struct drm_connector *connector,
> > >  		struct drm_modeset_acquire_ctx *ctx,
> > > @@ -6139,6 +6151,11 @@ intel_dp_detect(struct drm_connector *connector,
> > >  		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
> > >  		memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> > >  
> > > +		/*Reset the immutable VRR Capable property */
> > > +		drm_connector_set_vrr_capable_property(connector,
> > > +						       false);
> > > +		intel_dp_reset_link_params(intel_dp);
> > > +
> > 
> > Why would we care what those are when the sink is disconnected?
> 
> We are noticing this happen in case the panel send this long pulse indicating
> status change to disconnected, while the modeset was already triggered by userspace
> in this case IGT, so the modeset continues right after i915_hotplug_work_fn
> so we need to reset all params which fixes the bug mentioned.

Why did the link params get out of whack before hpd in the first place?

> 
> Manasi
> 
> > 
> > >  		if (intel_dp->is_mst) {
> > >  			drm_dbg_kms(&dev_priv->drm,
> > >  				    "MST device may have disappeared %d vs %d\n",
> > > @@ -6152,15 +6169,8 @@ intel_dp_detect(struct drm_connector *connector,
> > >  		goto out;
> > >  	}
> > >  
> > > -	if (intel_dp->reset_link_params) {
> > > -		/* Initial max link lane count */
> > > -		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > > -
> > > -		/* Initial max link rate */
> > > -		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > > -
> > > -		intel_dp->reset_link_params = false;
> > > -	}
> > > +	if (intel_dp->reset_link_params)
> > > +		intel_dp_reset_link_params(intel_dp);
> > >  
> > >  	intel_dp_print_rates(intel_dp);
> > >  
> > > -- 
> > > 2.19.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04 18:38     ` Ville Syrjälä
@ 2020-06-04 18:52       ` Manasi Navare
  2020-06-04 18:58         ` Ville Syrjälä
  0 siblings, 1 reply; 14+ messages in thread
From: Manasi Navare @ 2020-06-04 18:52 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Thu, Jun 04, 2020 at 09:38:19PM +0300, Ville Syrjälä wrote:
> On Thu, Jun 04, 2020 at 11:35:30AM -0700, Manasi Navare wrote:
> > On Thu, Jun 04, 2020 at 06:25:43PM +0300, Ville Syrjälä wrote:
> > > On Wed, Jun 03, 2020 at 05:23:59PM -0700, Manasi Navare wrote:
> > > > We have noticed that when link training fails the panel
> > > > sends a long pulse indicating connector disconnect. In this case
> > > > we need to reset the link parameters instead of continuing
> > > > to use the fallback parameters since else this long pulse
> > > > by the panel followed by a modeset request which was triggered by the userspace
> > > > before getting the connector status as disconnected, will
> > > > result into a modeset now using lower link rate/lane count values.
> > > > 
> > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1385
> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_dp.c | 28 +++++++++++++++++--------
> > > >  1 file changed, 19 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > index 55fda074c0ad..f7af372647dd 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > @@ -6111,6 +6111,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > > >  	intel_dp->edid_quirks = 0;
> > > >  }
> > > >  
> > > > +static void
> > > > +intel_dp_reset_link_params(struct intel_dp *intel_dp)
> > > > +{
> > > > +	/* Initial max link lane count */
> > > > +	intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > > > +
> > > > +	/* Initial max link rate */
> > > > +	intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > > > +
> > > > +	intel_dp->reset_link_params = false;
> > > > +}
> > > > +
> > > >  static int
> > > >  intel_dp_detect(struct drm_connector *connector,
> > > >  		struct drm_modeset_acquire_ctx *ctx,
> > > > @@ -6139,6 +6151,11 @@ intel_dp_detect(struct drm_connector *connector,
> > > >  		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
> > > >  		memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> > > >  
> > > > +		/*Reset the immutable VRR Capable property */
> > > > +		drm_connector_set_vrr_capable_property(connector,
> > > > +						       false);
> > > > +		intel_dp_reset_link_params(intel_dp);
> > > > +
> > > 
> > > Why would we care what those are when the sink is disconnected?
> > 
> > We are noticing this happen in case the panel send this long pulse indicating
> > status change to disconnected, while the modeset was already triggered by userspace
> > in this case IGT, so the modeset continues right after i915_hotplug_work_fn
> > so we need to reset all params which fixes the bug mentioned.
> 
> Why did the link params get out of whack before hpd in the first place?
>

Most of the failures, we see the link training fails due to AUX timeouts and then link params fallback to lower values
Then we get this hpd, in this case if we dont reset the param to max values, prev triggered modeset continues
with fallback values but since connector probe doesnt happen again through IGT, it tries the same mode
with fallback values and return encoder config failure.

So after reseting the params, the modeset happens with original values and that time link training passes.
This is seen in all kms_atomic_transitions IGT tests

Manasi
 
> > 
> > Manasi
> > 
> > > 
> > > >  		if (intel_dp->is_mst) {
> > > >  			drm_dbg_kms(&dev_priv->drm,
> > > >  				    "MST device may have disappeared %d vs %d\n",
> > > > @@ -6152,15 +6169,8 @@ intel_dp_detect(struct drm_connector *connector,
> > > >  		goto out;
> > > >  	}
> > > >  
> > > > -	if (intel_dp->reset_link_params) {
> > > > -		/* Initial max link lane count */
> > > > -		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > > > -
> > > > -		/* Initial max link rate */
> > > > -		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > > > -
> > > > -		intel_dp->reset_link_params = false;
> > > > -	}
> > > > +	if (intel_dp->reset_link_params)
> > > > +		intel_dp_reset_link_params(intel_dp);
> > > >  
> > > >  	intel_dp_print_rates(intel_dp);
> > > >  
> > > > -- 
> > > > 2.19.1
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04 18:52       ` Manasi Navare
@ 2020-06-04 18:58         ` Ville Syrjälä
  2020-06-04 19:01           ` Ville Syrjälä
  0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2020-06-04 18:58 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

On Thu, Jun 04, 2020 at 11:52:24AM -0700, Manasi Navare wrote:
> On Thu, Jun 04, 2020 at 09:38:19PM +0300, Ville Syrjälä wrote:
> > On Thu, Jun 04, 2020 at 11:35:30AM -0700, Manasi Navare wrote:
> > > On Thu, Jun 04, 2020 at 06:25:43PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Jun 03, 2020 at 05:23:59PM -0700, Manasi Navare wrote:
> > > > > We have noticed that when link training fails the panel
> > > > > sends a long pulse indicating connector disconnect. In this case
> > > > > we need to reset the link parameters instead of continuing
> > > > > to use the fallback parameters since else this long pulse
> > > > > by the panel followed by a modeset request which was triggered by the userspace
> > > > > before getting the connector status as disconnected, will
> > > > > result into a modeset now using lower link rate/lane count values.
> > > > > 
> > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1385
> > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_dp.c | 28 +++++++++++++++++--------
> > > > >  1 file changed, 19 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > index 55fda074c0ad..f7af372647dd 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > @@ -6111,6 +6111,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > > > >  	intel_dp->edid_quirks = 0;
> > > > >  }
> > > > >  
> > > > > +static void
> > > > > +intel_dp_reset_link_params(struct intel_dp *intel_dp)
> > > > > +{
> > > > > +	/* Initial max link lane count */
> > > > > +	intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > > > > +
> > > > > +	/* Initial max link rate */
> > > > > +	intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > > > > +
> > > > > +	intel_dp->reset_link_params = false;
> > > > > +}
> > > > > +
> > > > >  static int
> > > > >  intel_dp_detect(struct drm_connector *connector,
> > > > >  		struct drm_modeset_acquire_ctx *ctx,
> > > > > @@ -6139,6 +6151,11 @@ intel_dp_detect(struct drm_connector *connector,
> > > > >  		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
> > > > >  		memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> > > > >  
> > > > > +		/*Reset the immutable VRR Capable property */
> > > > > +		drm_connector_set_vrr_capable_property(connector,
> > > > > +						       false);
> > > > > +		intel_dp_reset_link_params(intel_dp);
> > > > > +
> > > > 
> > > > Why would we care what those are when the sink is disconnected?
> > > 
> > > We are noticing this happen in case the panel send this long pulse indicating
> > > status change to disconnected, while the modeset was already triggered by userspace
> > > in this case IGT, so the modeset continues right after i915_hotplug_work_fn
> > > so we need to reset all params which fixes the bug mentioned.
> > 
> > Why did the link params get out of whack before hpd in the first place?
> >
> 
> Most of the failures, we see the link training fails due to AUX timeouts and then link params fallback to lower values
> Then we get this hpd, in this case if we dont reset the param to max values, prev triggered modeset continues
> with fallback values but since connector probe doesnt happen again through IGT, it tries the same mode
> with fallback values and return encoder config failure.

If the link training failed then clearly the sink didn't like us anymore
anyway. So feels like resetting these here is just shifting some race
window around a bit, but it could still fail if the sink still doesn't
like us.

Would be good if someone was able to figure out why the sink goes bad in
the first place.

> 
> So after reseting the params, the modeset happens with original values and that time link training passes.
> This is seen in all kms_atomic_transitions IGT tests
> 
> Manasi
>  
> > > 
> > > Manasi
> > > 
> > > > 
> > > > >  		if (intel_dp->is_mst) {
> > > > >  			drm_dbg_kms(&dev_priv->drm,
> > > > >  				    "MST device may have disappeared %d vs %d\n",
> > > > > @@ -6152,15 +6169,8 @@ intel_dp_detect(struct drm_connector *connector,
> > > > >  		goto out;
> > > > >  	}
> > > > >  
> > > > > -	if (intel_dp->reset_link_params) {
> > > > > -		/* Initial max link lane count */
> > > > > -		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > > > > -
> > > > > -		/* Initial max link rate */
> > > > > -		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > > > > -
> > > > > -		intel_dp->reset_link_params = false;
> > > > > -	}
> > > > > +	if (intel_dp->reset_link_params)
> > > > > +		intel_dp_reset_link_params(intel_dp);
> > > > >  
> > > > >  	intel_dp_print_rates(intel_dp);
> > > > >  
> > > > > -- 
> > > > > 2.19.1
> > > > 
> > > > -- 
> > > > Ville Syrjälä
> > > > Intel
> > 
> > -- 
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04 18:58         ` Ville Syrjälä
@ 2020-06-04 19:01           ` Ville Syrjälä
  2020-06-04 19:08             ` Imre Deak
  0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2020-06-04 19:01 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

On Thu, Jun 04, 2020 at 09:58:24PM +0300, Ville Syrjälä wrote:
> On Thu, Jun 04, 2020 at 11:52:24AM -0700, Manasi Navare wrote:
> > On Thu, Jun 04, 2020 at 09:38:19PM +0300, Ville Syrjälä wrote:
> > > On Thu, Jun 04, 2020 at 11:35:30AM -0700, Manasi Navare wrote:
> > > > On Thu, Jun 04, 2020 at 06:25:43PM +0300, Ville Syrjälä wrote:
> > > > > On Wed, Jun 03, 2020 at 05:23:59PM -0700, Manasi Navare wrote:
> > > > > > We have noticed that when link training fails the panel
> > > > > > sends a long pulse indicating connector disconnect. In this case
> > > > > > we need to reset the link parameters instead of continuing
> > > > > > to use the fallback parameters since else this long pulse
> > > > > > by the panel followed by a modeset request which was triggered by the userspace
> > > > > > before getting the connector status as disconnected, will
> > > > > > result into a modeset now using lower link rate/lane count values.
> > > > > > 
> > > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1385
> > > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/display/intel_dp.c | 28 +++++++++++++++++--------
> > > > > >  1 file changed, 19 insertions(+), 9 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > index 55fda074c0ad..f7af372647dd 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > @@ -6111,6 +6111,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > > > > >  	intel_dp->edid_quirks = 0;
> > > > > >  }
> > > > > >  
> > > > > > +static void
> > > > > > +intel_dp_reset_link_params(struct intel_dp *intel_dp)
> > > > > > +{
> > > > > > +	/* Initial max link lane count */
> > > > > > +	intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > > > > > +
> > > > > > +	/* Initial max link rate */
> > > > > > +	intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > > > > > +
> > > > > > +	intel_dp->reset_link_params = false;
> > > > > > +}
> > > > > > +
> > > > > >  static int
> > > > > >  intel_dp_detect(struct drm_connector *connector,
> > > > > >  		struct drm_modeset_acquire_ctx *ctx,
> > > > > > @@ -6139,6 +6151,11 @@ intel_dp_detect(struct drm_connector *connector,
> > > > > >  		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
> > > > > >  		memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> > > > > >  
> > > > > > +		/*Reset the immutable VRR Capable property */
> > > > > > +		drm_connector_set_vrr_capable_property(connector,
> > > > > > +						       false);
> > > > > > +		intel_dp_reset_link_params(intel_dp);
> > > > > > +
> > > > > 
> > > > > Why would we care what those are when the sink is disconnected?
> > > > 
> > > > We are noticing this happen in case the panel send this long pulse indicating
> > > > status change to disconnected, while the modeset was already triggered by userspace
> > > > in this case IGT, so the modeset continues right after i915_hotplug_work_fn
> > > > so we need to reset all params which fixes the bug mentioned.
> > > 
> > > Why did the link params get out of whack before hpd in the first place?
> > >
> > 
> > Most of the failures, we see the link training fails due to AUX timeouts and then link params fallback to lower values
> > Then we get this hpd, in this case if we dont reset the param to max values, prev triggered modeset continues
> > with fallback values but since connector probe doesnt happen again through IGT, it tries the same mode
> > with fallback values and return encoder config failure.
> 
> If the link training failed then clearly the sink didn't like us anymore
> anyway. So feels like resetting these here is just shifting some race
> window around a bit, but it could still fail if the sink still doesn't
> like us.
> 
> Would be good if someone was able to figure out why the sink goes bad in
> the first place.

Oh, and don't we now have Imre's "weird hpd happened in the middle of
the test, don't trust the results" thing in igt?

> 
> > 
> > So after reseting the params, the modeset happens with original values and that time link training passes.
> > This is seen in all kms_atomic_transitions IGT tests
> > 
> > Manasi
> >  
> > > > 
> > > > Manasi
> > > > 
> > > > > 
> > > > > >  		if (intel_dp->is_mst) {
> > > > > >  			drm_dbg_kms(&dev_priv->drm,
> > > > > >  				    "MST device may have disappeared %d vs %d\n",
> > > > > > @@ -6152,15 +6169,8 @@ intel_dp_detect(struct drm_connector *connector,
> > > > > >  		goto out;
> > > > > >  	}
> > > > > >  
> > > > > > -	if (intel_dp->reset_link_params) {
> > > > > > -		/* Initial max link lane count */
> > > > > > -		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > > > > > -
> > > > > > -		/* Initial max link rate */
> > > > > > -		intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > > > > > -
> > > > > > -		intel_dp->reset_link_params = false;
> > > > > > -	}
> > > > > > +	if (intel_dp->reset_link_params)
> > > > > > +		intel_dp_reset_link_params(intel_dp);
> > > > > >  
> > > > > >  	intel_dp_print_rates(intel_dp);
> > > > > >  
> > > > > > -- 
> > > > > > 2.19.1
> > > > > 
> > > > > -- 
> > > > > Ville Syrjälä
> > > > > Intel
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04 19:01           ` Ville Syrjälä
@ 2020-06-04 19:08             ` Imre Deak
  2020-06-04 19:20               ` Manasi Navare
  2020-06-04 19:23               ` Imre Deak
  0 siblings, 2 replies; 14+ messages in thread
From: Imre Deak @ 2020-06-04 19:08 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Thu, Jun 04, 2020 at 10:01:40PM +0300, Ville Syrjälä wrote:
> [...]
> > > Then we get this hpd, in this case if we dont reset the param to max values, prev triggered modeset continues
> > > with fallback values but since connector probe doesnt happen again through IGT, it tries the same mode
> > > with fallback values and return encoder config failure.
> > 
> > If the link training failed then clearly the sink didn't like us anymore
> > anyway. So feels like resetting these here is just shifting some race
> > window around a bit, but it could still fail if the sink still doesn't
> > like us.
> > 
> > Would be good if someone was able to figure out why the sink goes bad in
> > the first place.
> 
> Oh, and don't we now have Imre's "weird hpd happened in the middle of
> the test, don't trust the results" thing in igt?

An LG and IIyama monitor this happens on disconnect and reconnect after
waking from an idle state when modesetting them, not sure if it's the
same case.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04 19:08             ` Imre Deak
@ 2020-06-04 19:20               ` Manasi Navare
  2020-06-04 19:23               ` Imre Deak
  1 sibling, 0 replies; 14+ messages in thread
From: Manasi Navare @ 2020-06-04 19:20 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Thu, Jun 04, 2020 at 10:08:58PM +0300, Imre Deak wrote:
> On Thu, Jun 04, 2020 at 10:01:40PM +0300, Ville Syrjälä wrote:
> > [...]
> > > > Then we get this hpd, in this case if we dont reset the param to max values, prev triggered modeset continues
> > > > with fallback values but since connector probe doesnt happen again through IGT, it tries the same mode
> > > > with fallback values and return encoder config failure.
> > > 
> > > If the link training failed then clearly the sink didn't like us anymore
> > > anyway. So feels like resetting these here is just shifting some race
> > > window around a bit, but it could still fail if the sink still doesn't
> > > like us.
> > > 
> > > Would be good if someone was able to figure out why the sink goes bad in
> > > the first place.
> > 
> > Oh, and don't we now have Imre's "weird hpd happened in the middle of
> > the test, don't trust the results" thing in igt?
> 
> An LG and IIyama monitor this happens on disconnect and reconnect after
> waking from an idle state when modesetting them, not sure if it's the
> same case.

Well in this case, it happens just after link training failure due to some AUX timeouts
then looks like the panel detects that the link was not enabled and sends this HPD
which puts us into connector status changing from connected to disconnected.

But in IGT, we dont get any uevent so we dont reprobe and continue with the next
igt_display_commit. 
So should we in IGT in kms_atomic_transitions, plane-all-modeset-transitions subtest,
should we check the connector status everytime before back to back commit calls?

Like I think in real use case, after a link failure the userspace would get a uevent and
respond to it by reprobing a connector, but we dont do that in IGT so these
random link failures cause issues like in here.

Manasi

> 
> --Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04 19:08             ` Imre Deak
  2020-06-04 19:20               ` Manasi Navare
@ 2020-06-04 19:23               ` Imre Deak
  2020-06-04 19:40                 ` Manasi Navare
  1 sibling, 1 reply; 14+ messages in thread
From: Imre Deak @ 2020-06-04 19:23 UTC (permalink / raw)
  To: Ville Syrjälä, Manasi D Navare; +Cc: intel-gfx

On Thu, Jun 04, 2020 at 10:08:58PM +0300, Imre Deak wrote:
> On Thu, Jun 04, 2020 at 10:01:40PM +0300, Ville Syrjälä wrote:
> > [...]
> > > > Then we get this hpd, in this case if we dont reset the param to max values, prev triggered modeset continues
> > > > with fallback values but since connector probe doesnt happen again through IGT, it tries the same mode
> > > > with fallback values and return encoder config failure.
> > > 
> > > If the link training failed then clearly the sink didn't like us anymore
> > > anyway. So feels like resetting these here is just shifting some race
> > > window around a bit, but it could still fail if the sink still doesn't
> > > like us.
> > > 
> > > Would be good if someone was able to figure out why the sink goes bad in
> > > the first place.
> > 
> > Oh, and don't we now have Imre's "weird hpd happened in the middle of
> > the test, don't trust the results" thing in igt?
> 
> An LG and IIyama monitor this happens on disconnect and reconnect after
> waking from an idle state when modesetting them, not sure if it's the
> same case.

Manasi, could you try if a modeset on the monitor after it has been
disabled for a while always results in a long HPD pulse a few seconds
after the modeset? If so does this also happen when you just modeset in
a sequence from one mode to the other not letting the monitor idle? The
same monitor should be also tested then with the above sequences on
older platforms if it behaves the same on those too.

> 
> --Imre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04 19:23               ` Imre Deak
@ 2020-06-04 19:40                 ` Manasi Navare
  2020-06-04 19:49                   ` Imre Deak
  0 siblings, 1 reply; 14+ messages in thread
From: Manasi Navare @ 2020-06-04 19:40 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Thu, Jun 04, 2020 at 10:23:40PM +0300, Imre Deak wrote:
> On Thu, Jun 04, 2020 at 10:08:58PM +0300, Imre Deak wrote:
> > On Thu, Jun 04, 2020 at 10:01:40PM +0300, Ville Syrjälä wrote:
> > > [...]
> > > > > Then we get this hpd, in this case if we dont reset the param to max values, prev triggered modeset continues
> > > > > with fallback values but since connector probe doesnt happen again through IGT, it tries the same mode
> > > > > with fallback values and return encoder config failure.
> > > > 
> > > > If the link training failed then clearly the sink didn't like us anymore
> > > > anyway. So feels like resetting these here is just shifting some race
> > > > window around a bit, but it could still fail if the sink still doesn't
> > > > like us.
> > > > 
> > > > Would be good if someone was able to figure out why the sink goes bad in
> > > > the first place.
> > > 
> > > Oh, and don't we now have Imre's "weird hpd happened in the middle of
> > > the test, don't trust the results" thing in igt?
> > 
> > An LG and IIyama monitor this happens on disconnect and reconnect after
> > waking from an idle state when modesetting them, not sure if it's the
> > same case.
> 
> Manasi, could you try if a modeset on the monitor after it has been
> disabled for a while always results in a long HPD pulse a few seconds
> after the modeset? If so does this also happen when you just modeset in
> a sequence from one mode to the other not letting the monitor idle? The
> same monitor should be also tested then with the above sequences on
> older platforms if it behaves the same on those too.
>

This test has been passing on older ICL platforms. But on TGL
we do see these AUX E timeouts once in a while which recover on their
own for the next modeset. Any idea why these spurious AUX timeouts and how I can possibly
rootcause why these timeouts are seen only with AUX E?

Manasi
 
> > 
> > --Imre
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect
  2020-06-04 19:40                 ` Manasi Navare
@ 2020-06-04 19:49                   ` Imre Deak
  0 siblings, 0 replies; 14+ messages in thread
From: Imre Deak @ 2020-06-04 19:49 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

On Thu, Jun 04, 2020 at 12:40:20PM -0700, Manasi Navare wrote:
> On Thu, Jun 04, 2020 at 10:23:40PM +0300, Imre Deak wrote:
> > On Thu, Jun 04, 2020 at 10:08:58PM +0300, Imre Deak wrote:
> > > On Thu, Jun 04, 2020 at 10:01:40PM +0300, Ville Syrjälä wrote:
> > > > [...]
> > > > > > Then we get this hpd, in this case if we dont reset the param to max values, prev triggered modeset continues
> > > > > > with fallback values but since connector probe doesnt happen again through IGT, it tries the same mode
> > > > > > with fallback values and return encoder config failure.
> > > > > 
> > > > > If the link training failed then clearly the sink didn't like us anymore
> > > > > anyway. So feels like resetting these here is just shifting some race
> > > > > window around a bit, but it could still fail if the sink still doesn't
> > > > > like us.
> > > > > 
> > > > > Would be good if someone was able to figure out why the sink goes bad in
> > > > > the first place.
> > > > 
> > > > Oh, and don't we now have Imre's "weird hpd happened in the middle of
> > > > the test, don't trust the results" thing in igt?
> > > 
> > > An LG and IIyama monitor this happens on disconnect and reconnect after
> > > waking from an idle state when modesetting them, not sure if it's the
> > > same case.
> > 
> > Manasi, could you try if a modeset on the monitor after it has been
> > disabled for a while always results in a long HPD pulse a few seconds
> > after the modeset? If so does this also happen when you just modeset in
> > a sequence from one mode to the other not letting the monitor idle? The
> > same monitor should be also tested then with the above sequences on
> > older platforms if it behaves the same on those too.
> >
> 
> This test has been passing on older ICL platforms. But on TGL we do
> see these AUX E timeouts once in a while which recover on their own
> for the next modeset. Any idea why these spurious AUX timeouts and how
> I can possibly rootcause why these timeouts are seen only with AUX E?

If the monitor is in a disconnected state as you described, then AUX
will fail too. So you need to root cause why the monitor gets
disconnected. One possibility for that is what I described above. You
can't really make a conclusion on a test passing on ICL and not on TGL,
the timing can be different. You'd need to check if a disconnect happens
due to long HPD pulse when using the same monitor with the sequences I
described above, both on TGL and then also on ICL.

> 
> Manasi
>  
> > > 
> > > --Imre
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-06-04 19:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04  0:23 [Intel-gfx] [PATCH] drm/i915/dp: Reset link params on connector disconnect Manasi Navare
2020-06-04  0:49 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2020-06-04  1:19 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-06-04 15:25 ` [Intel-gfx] [PATCH] " Ville Syrjälä
2020-06-04 18:35   ` Manasi Navare
2020-06-04 18:38     ` Ville Syrjälä
2020-06-04 18:52       ` Manasi Navare
2020-06-04 18:58         ` Ville Syrjälä
2020-06-04 19:01           ` Ville Syrjälä
2020-06-04 19:08             ` Imre Deak
2020-06-04 19:20               ` Manasi Navare
2020-06-04 19:23               ` Imre Deak
2020-06-04 19:40                 ` Manasi Navare
2020-06-04 19:49                   ` Imre Deak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).