dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port
@ 2020-06-03 21:10 Imre Deak
  2020-06-03 21:10 ` [PATCH 2/3] drm/dp_mst: Sanitize mgr->qlock locking in drm_dp_mst_wait_tx_reply() Imre Deak
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Imre Deak @ 2020-06-03 21:10 UTC (permalink / raw)
  To: intel-gfx, dri-devel

Currently MST on a port can get enabled/disabled from the hotplug work
and get disabled from the short pulse work in a racy way. Fix this by
relying on the MST state checking in the hotplug work and just schedule
a hotplug work from the short pulse handler if some problem happened
during the MST interrupt handling.

This removes the explicit MST disabling in case of an AUX failure, but
if AUX fails, then probably the detection will also fail during the
scheduled hotplug work and it's not guaranteed that we'll see
intermittent errors anyway.

While at it also simplify the error checking of the MST interrupt
handler.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 33 +++----------------------
 1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 55fda074c0ad..befbcacddaa1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5604,7 +5604,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 		}
 	}
 
-	return need_retrain;
+	return need_retrain ? -EINVAL : 0;
 }
 
 static bool
@@ -7255,35 +7255,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	}
 
 	if (intel_dp->is_mst) {
-		switch (intel_dp_check_mst_status(intel_dp)) {
-		case -EINVAL:
-			/*
-			 * If we were in MST mode, and device is not
-			 * there, get out of MST mode
-			 */
-			drm_dbg_kms(&i915->drm,
-				    "MST device may have disappeared %d vs %d\n",
-				    intel_dp->is_mst,
-				    intel_dp->mst_mgr.mst_state);
-			intel_dp->is_mst = false;
-			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
-							intel_dp->is_mst);
-
-			return IRQ_NONE;
-		case 1:
-			return IRQ_NONE;
-		default:
-			break;
-		}
-	}
-
-	if (!intel_dp->is_mst) {
-		bool handled;
-
-		handled = intel_dp_short_pulse(intel_dp);
-
-		if (!handled)
+		if (intel_dp_check_mst_status(intel_dp) < 0)
 			return IRQ_NONE;
+	} else if (!intel_dp_short_pulse(intel_dp)) {
+		return IRQ_NONE;
 	}
 
 	return IRQ_HANDLED;
-- 
2.23.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 2/3] drm/dp_mst: Sanitize mgr->qlock locking in drm_dp_mst_wait_tx_reply()
  2020-06-03 21:10 [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Imre Deak
@ 2020-06-03 21:10 ` Imre Deak
  2020-06-03 21:27   ` [Intel-gfx] " Souza, Jose
  2020-06-03 21:10 ` [PATCH 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses Imre Deak
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Imre Deak @ 2020-06-03 21:10 UTC (permalink / raw)
  To: intel-gfx, dri-devel

Make the locking look symmetric with the unlocking.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 1bdf3cfeeebb..5bc72e800b85 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1183,7 +1183,7 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
 	ret = wait_event_timeout(mgr->tx_waitq,
 				 check_txmsg_state(mgr, txmsg),
 				 (4 * HZ));
-	mutex_lock(&mstb->mgr->qlock);
+	mutex_lock(&mgr->qlock);
 	if (ret > 0) {
 		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
 			ret = -EIO;
-- 
2.23.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses
  2020-06-03 21:10 [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Imre Deak
  2020-06-03 21:10 ` [PATCH 2/3] drm/dp_mst: Sanitize mgr->qlock locking in drm_dp_mst_wait_tx_reply() Imre Deak
@ 2020-06-03 21:10 ` Imre Deak
  2020-06-03 22:18   ` [PATCH v2 " Imre Deak
  2020-06-04 18:45   ` [PATCH v3 " Imre Deak
  2020-06-03 21:27 ` [Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Souza, Jose
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Imre Deak @ 2020-06-03 21:10 UTC (permalink / raw)
  To: intel-gfx, dri-devel

Some TypeC -> native DP adapters, at least the Club CAC-1557 adapter,
incorrectly filter out HPD short pulses with a duration less than ~540
usec, leading to MST probe failures.

According to the DP alt mode specification adapters should forward short
pulses with a duration greater than 250 usec. According to the DP
specificatin DP sources should detect short pulses in the
500 usec -> 2 ms range. Based on this filtering out short pulses with a
duration less than 540 usec is incorrect.

To make such adapters work add support for a driver polling on MST
inerrupt flags, and wire this up in the i915 driver. The sink can clear
an interrupt it raised after 110 ms if the source doesn't respond, so
use a 50 ms poll period to avoid missing an interrupt. Polling of the
MST interrupt flags is explicitly allowed by the DP specification.

This fixes MST probe failures I saw using this adapter and a DELL U2515H
monitor.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c       | 18 +++++++++++++++---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 15 +++++++++++++++
 include/drm/drm_dp_mst_helper.h             |  1 +
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 5bc72e800b85..d1bf340a95a8 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1178,11 +1178,23 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
 				    struct drm_dp_sideband_msg_tx *txmsg)
 {
 	struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
+	unsigned long wait_expires = jiffies + msecs_to_jiffies(4000);
 	int ret;
 
-	ret = wait_event_timeout(mgr->tx_waitq,
-				 check_txmsg_state(mgr, txmsg),
-				 (4 * HZ));
+	for (;;) {
+		ret = wait_event_timeout(mgr->tx_waitq,
+					 check_txmsg_state(mgr, txmsg),
+					 mgr->cbs->update_hpd_irq_state ?
+						msecs_to_jiffies(50) :
+						wait_expires);
+
+		if (ret || !mgr->cbs->update_hpd_irq_state ||
+		    time_after(jiffies, wait_expires))
+			break;
+
+		mgr->cbs->update_hpd_irq_state(mgr);
+	}
+
 	mutex_lock(&mgr->qlock);
 	if (ret > 0) {
 		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index d18b406f2a7d..1ff7d0096262 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -765,8 +765,23 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 	return NULL;
 }
 
+static void
+intel_dp_mst_update_hpd_irq_state(struct drm_dp_mst_topology_mgr *mgr)
+{
+	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+
+	spin_lock_irq(&i915->irq_lock);
+	i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
+	spin_unlock_irq(&i915->irq_lock);
+
+	queue_work(i915->hotplug.dp_wq, &i915->hotplug.dig_port_work);
+}
+
 static const struct drm_dp_mst_topology_cbs mst_cbs = {
 	.add_connector = intel_dp_add_mst_connector,
+	.update_hpd_irq_state = intel_dp_mst_update_hpd_irq_state,
 };
 
 static struct intel_dp_mst_encoder *
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 9e1ffcd7cb68..c902f4380200 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -475,6 +475,7 @@ struct drm_dp_mst_topology_mgr;
 struct drm_dp_mst_topology_cbs {
 	/* create a connector for a port */
 	struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path);
+	void (*update_hpd_irq_state)(struct drm_dp_mst_topology_mgr *mgr);
 };
 
 #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)
-- 
2.23.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port
  2020-06-03 21:10 [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Imre Deak
  2020-06-03 21:10 ` [PATCH 2/3] drm/dp_mst: Sanitize mgr->qlock locking in drm_dp_mst_wait_tx_reply() Imre Deak
  2020-06-03 21:10 ` [PATCH 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses Imre Deak
@ 2020-06-03 21:27 ` Souza, Jose
  2020-06-04 14:55 ` Ville Syrjälä
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Souza, Jose @ 2020-06-03 21:27 UTC (permalink / raw)
  To: dri-devel, intel-gfx, Deak, Imre

On Thu, 2020-06-04 at 00:10 +0300, Imre Deak wrote:
> Currently MST on a port can get enabled/disabled from the hotplug work
> and get disabled from the short pulse work in a racy way. Fix this by
> relying on the MST state checking in the hotplug work and just schedule
> a hotplug work from the short pulse handler if some problem happened
> during the MST interrupt handling.

Nice

> 
> This removes the explicit MST disabling in case of an AUX failure, but
> if AUX fails, then probably the detection will also fail during the
> scheduled hotplug work and it's not guaranteed that we'll see
> intermittent errors anyway.
> 
> While at it also simplify the error checking of the MST interrupt
> handler.
> 

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 33 +++----------------------
>  1 file changed, 4 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 55fda074c0ad..befbcacddaa1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5604,7 +5604,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>  		}
>  	}
>  
> -	return need_retrain;
> +	return need_retrain ? -EINVAL : 0;
>  }
>  
>  static bool
> @@ -7255,35 +7255,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  	}
>  
>  	if (intel_dp->is_mst) {
> -		switch (intel_dp_check_mst_status(intel_dp)) {
> -		case -EINVAL:
> -			/*
> -			 * If we were in MST mode, and device is not
> -			 * there, get out of MST mode
> -			 */
> -			drm_dbg_kms(&i915->drm,
> -				    "MST device may have disappeared %d vs %d\n",
> -				    intel_dp->is_mst,
> -				    intel_dp->mst_mgr.mst_state);
> -			intel_dp->is_mst = false;
> -			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> -							intel_dp->is_mst);
> -
> -			return IRQ_NONE;
> -		case 1:
> -			return IRQ_NONE;
> -		default:
> -			break;
> -		}
> -	}
> -
> -	if (!intel_dp->is_mst) {
> -		bool handled;
> -
> -		handled = intel_dp_short_pulse(intel_dp);
> -
> -		if (!handled)
> +		if (intel_dp_check_mst_status(intel_dp) < 0)
>  			return IRQ_NONE;
> +	} else if (!intel_dp_short_pulse(intel_dp)) {
> +		return IRQ_NONE;
>  	}
>  
>  	return IRQ_HANDLED;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH 2/3] drm/dp_mst: Sanitize mgr->qlock locking in drm_dp_mst_wait_tx_reply()
  2020-06-03 21:10 ` [PATCH 2/3] drm/dp_mst: Sanitize mgr->qlock locking in drm_dp_mst_wait_tx_reply() Imre Deak
@ 2020-06-03 21:27   ` Souza, Jose
  0 siblings, 0 replies; 17+ messages in thread
From: Souza, Jose @ 2020-06-03 21:27 UTC (permalink / raw)
  To: dri-devel, intel-gfx, Deak, Imre

On Thu, 2020-06-04 at 00:10 +0300, Imre Deak wrote:
> Make the locking look symmetric with the unlocking.
> 

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1bdf3cfeeebb..5bc72e800b85 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1183,7 +1183,7 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
>  	ret = wait_event_timeout(mgr->tx_waitq,
>  				 check_txmsg_state(mgr, txmsg),
>  				 (4 * HZ));
> -	mutex_lock(&mstb->mgr->qlock);
> +	mutex_lock(&mgr->qlock);
>  	if (ret > 0) {
>  		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
>  			ret = -EIO;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses
  2020-06-03 21:10 ` [PATCH 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses Imre Deak
@ 2020-06-03 22:18   ` Imre Deak
  2020-06-04 15:12     ` [Intel-gfx] " Ville Syrjälä
  2020-06-04 18:45   ` [PATCH v3 " Imre Deak
  1 sibling, 1 reply; 17+ messages in thread
From: Imre Deak @ 2020-06-03 22:18 UTC (permalink / raw)
  To: intel-gfx, dri-devel

Some TypeC -> native DP adapters, at least the Club CAC-1557 adapter,
incorrectly filter out HPD short pulses with a duration less than ~540
usec, leading to MST probe failures.

According to the DP alt mode specification adapters should forward short
pulses with a duration greater than 250 usec. According to the DP
specificatin DP sources should detect short pulses in the
500 usec -> 2 ms range. Based on this filtering out short pulses with a
duration less than 540 usec is incorrect.

To make such adapters work add support for a driver polling on MST
inerrupt flags, and wire this up in the i915 driver. The sink can clear
an interrupt it raised after 110 ms if the source doesn't respond, so
use a 50 ms poll period to avoid missing an interrupt. Polling of the
MST interrupt flags is explicitly allowed by the DP specification.

This fixes MST probe failures I saw using this adapter and a DELL U2515H
monitor.

v2:
- Fix the wait event timeout for the no-poll case.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c       | 19 ++++++++++++++++---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 15 +++++++++++++++
 include/drm/drm_dp_mst_helper.h             |  1 +
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 5bc72e800b85..4e987a513df8 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1178,11 +1178,24 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
 				    struct drm_dp_sideband_msg_tx *txmsg)
 {
 	struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
+	unsigned long wait_timeout = msecs_to_jiffies(4000);
+	unsigned long wait_expires = jiffies + wait_timeout;
 	int ret;
 
-	ret = wait_event_timeout(mgr->tx_waitq,
-				 check_txmsg_state(mgr, txmsg),
-				 (4 * HZ));
+	for (;;) {
+		ret = wait_event_timeout(mgr->tx_waitq,
+					 check_txmsg_state(mgr, txmsg),
+					 mgr->cbs->update_hpd_irq_state ?
+						msecs_to_jiffies(50) :
+						wait_timeout);
+
+		if (ret || !mgr->cbs->update_hpd_irq_state ||
+		    time_after(jiffies, wait_expires))
+			break;
+
+		mgr->cbs->update_hpd_irq_state(mgr);
+	}
+
 	mutex_lock(&mgr->qlock);
 	if (ret > 0) {
 		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index d18b406f2a7d..1ff7d0096262 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -765,8 +765,23 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 	return NULL;
 }
 
+static void
+intel_dp_mst_update_hpd_irq_state(struct drm_dp_mst_topology_mgr *mgr)
+{
+	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+
+	spin_lock_irq(&i915->irq_lock);
+	i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
+	spin_unlock_irq(&i915->irq_lock);
+
+	queue_work(i915->hotplug.dp_wq, &i915->hotplug.dig_port_work);
+}
+
 static const struct drm_dp_mst_topology_cbs mst_cbs = {
 	.add_connector = intel_dp_add_mst_connector,
+	.update_hpd_irq_state = intel_dp_mst_update_hpd_irq_state,
 };
 
 static struct intel_dp_mst_encoder *
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 9e1ffcd7cb68..c902f4380200 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -475,6 +475,7 @@ struct drm_dp_mst_topology_mgr;
 struct drm_dp_mst_topology_cbs {
 	/* create a connector for a port */
 	struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path);
+	void (*update_hpd_irq_state)(struct drm_dp_mst_topology_mgr *mgr);
 };
 
 #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)
-- 
2.23.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port
  2020-06-03 21:10 [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Imre Deak
                   ` (2 preceding siblings ...)
  2020-06-03 21:27 ` [Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Souza, Jose
@ 2020-06-04 14:55 ` Ville Syrjälä
  2020-06-04 15:09   ` Imre Deak
  2020-06-04 18:44 ` [PATCH v2 " Imre Deak
       [not found] ` <159136523293.18507.17008252253062518394@emeril.freedesktop.org>
  5 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2020-06-04 14:55 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx, dri-devel

On Thu, Jun 04, 2020 at 12:10:38AM +0300, Imre Deak wrote:
> Currently MST on a port can get enabled/disabled from the hotplug work
> and get disabled from the short pulse work in a racy way. Fix this by
> relying on the MST state checking in the hotplug work and just schedule
> a hotplug work from the short pulse handler if some problem happened
> during the MST interrupt handling.
> 
> This removes the explicit MST disabling in case of an AUX failure, but
> if AUX fails, then probably the detection will also fail during the
> scheduled hotplug work and it's not guaranteed that we'll see
> intermittent errors anyway.
> 
> While at it also simplify the error checking of the MST interrupt
> handler.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 33 +++----------------------
>  1 file changed, 4 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 55fda074c0ad..befbcacddaa1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5604,7 +5604,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>  		}
>  	}
>  
> -	return need_retrain;
> +	return need_retrain ? -EINVAL : 0;
>  }
>  
>  static bool
> @@ -7255,35 +7255,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  	}
>  
>  	if (intel_dp->is_mst) {
> -		switch (intel_dp_check_mst_status(intel_dp)) {
> -		case -EINVAL:
> -			/*
> -			 * If we were in MST mode, and device is not
> -			 * there, get out of MST mode
> -			 */
> -			drm_dbg_kms(&i915->drm,
> -				    "MST device may have disappeared %d vs %d\n",
> -				    intel_dp->is_mst,
> -				    intel_dp->mst_mgr.mst_state);
> -			intel_dp->is_mst = false;
> -			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> -							intel_dp->is_mst);
> -
> -			return IRQ_NONE;
> -		case 1:
> -			return IRQ_NONE;
> -		default:
> -			break;
> -		}
> -	}
> -
> -	if (!intel_dp->is_mst) {
> -		bool handled;
> -
> -		handled = intel_dp_short_pulse(intel_dp);
> -
> -		if (!handled)
> +		if (intel_dp_check_mst_status(intel_dp) < 0)
>  			return IRQ_NONE;

Since we no longer need the tristate return, can you follow up
with a conversion to bool return? I'd vote to make it match the
semantics of intel_dp_short_pulse() so we get one step
closer to unifying the hpd_irq handling across the board.

> +	} else if (!intel_dp_short_pulse(intel_dp)) {
> +		return IRQ_NONE;
>  	}
>  
>  	return IRQ_HANDLED;
> -- 
> 2.23.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port
  2020-06-04 14:55 ` Ville Syrjälä
@ 2020-06-04 15:09   ` Imre Deak
  0 siblings, 0 replies; 17+ messages in thread
From: Imre Deak @ 2020-06-04 15:09 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel

On Thu, Jun 04, 2020 at 05:55:30PM +0300, Ville Syrjälä wrote:
> On Thu, Jun 04, 2020 at 12:10:38AM +0300, Imre Deak wrote:
> > Currently MST on a port can get enabled/disabled from the hotplug work
> > and get disabled from the short pulse work in a racy way. Fix this by
> > relying on the MST state checking in the hotplug work and just schedule
> > a hotplug work from the short pulse handler if some problem happened
> > during the MST interrupt handling.
> > 
> > This removes the explicit MST disabling in case of an AUX failure, but
> > if AUX fails, then probably the detection will also fail during the
> > scheduled hotplug work and it's not guaranteed that we'll see
> > intermittent errors anyway.
> > 
> > While at it also simplify the error checking of the MST interrupt
> > handler.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 33 +++----------------------
> >  1 file changed, 4 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 55fda074c0ad..befbcacddaa1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5604,7 +5604,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
> >  		}
> >  	}
> >  
> > -	return need_retrain;
> > +	return need_retrain ? -EINVAL : 0;
> >  }
> >  
> >  static bool
> > @@ -7255,35 +7255,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >  	}
> >  
> >  	if (intel_dp->is_mst) {
> > -		switch (intel_dp_check_mst_status(intel_dp)) {
> > -		case -EINVAL:
> > -			/*
> > -			 * If we were in MST mode, and device is not
> > -			 * there, get out of MST mode
> > -			 */
> > -			drm_dbg_kms(&i915->drm,
> > -				    "MST device may have disappeared %d vs %d\n",
> > -				    intel_dp->is_mst,
> > -				    intel_dp->mst_mgr.mst_state);
> > -			intel_dp->is_mst = false;
> > -			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> > -							intel_dp->is_mst);
> > -
> > -			return IRQ_NONE;
> > -		case 1:
> > -			return IRQ_NONE;
> > -		default:
> > -			break;
> > -		}
> > -	}
> > -
> > -	if (!intel_dp->is_mst) {
> > -		bool handled;
> > -
> > -		handled = intel_dp_short_pulse(intel_dp);
> > -
> > -		if (!handled)
> > +		if (intel_dp_check_mst_status(intel_dp) < 0)
> >  			return IRQ_NONE;
> 
> Since we no longer need the tristate return, can you follow up
> with a conversion to bool return? I'd vote to make it match the
> semantics of intel_dp_short_pulse() so we get one step
> closer to unifying the hpd_irq handling across the board.

Ok, makes sense.

> 
> > +	} else if (!intel_dp_short_pulse(intel_dp)) {
> > +		return IRQ_NONE;
> >  	}
> >  
> >  	return IRQ_HANDLED;
> > -- 
> > 2.23.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses
  2020-06-03 22:18   ` [PATCH v2 " Imre Deak
@ 2020-06-04 15:12     ` Ville Syrjälä
  2020-06-04 15:41       ` Imre Deak
  0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2020-06-04 15:12 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx, dri-devel

On Thu, Jun 04, 2020 at 01:18:59AM +0300, Imre Deak wrote:
> Some TypeC -> native DP adapters, at least the Club CAC-1557 adapter,
> incorrectly filter out HPD short pulses with a duration less than ~540
> usec, leading to MST probe failures.
> 
> According to the DP alt mode specification adapters should forward short
> pulses with a duration greater than 250 usec. According to the DP
> specificatin DP sources should detect short pulses in the
> 500 usec -> 2 ms range. 

IIRC it was 250 usec -> 2 ms as well in the DP spec.

500 usec -> 1 ms is the duration of the short hpd
the signalling side should use.

> Based on this filtering out short pulses with a
> duration less than 540 usec is incorrect.
> 
> To make such adapters work add support for a driver polling on MST
> inerrupt flags, and wire this up in the i915 driver. The sink can clear
> an interrupt it raised after 110 ms if the source doesn't respond, so
> use a 50 ms poll period to avoid missing an interrupt. Polling of the
> MST interrupt flags is explicitly allowed by the DP specification.
> 
> This fixes MST probe failures I saw using this adapter and a DELL U2515H
> monitor.
> 
> v2:
> - Fix the wait event timeout for the no-poll case.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c       | 19 ++++++++++++++++---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 15 +++++++++++++++
>  include/drm/drm_dp_mst_helper.h             |  1 +
>  3 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 5bc72e800b85..4e987a513df8 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1178,11 +1178,24 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
>  				    struct drm_dp_sideband_msg_tx *txmsg)
>  {
>  	struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
> +	unsigned long wait_timeout = msecs_to_jiffies(4000);
> +	unsigned long wait_expires = jiffies + wait_timeout;
>  	int ret;
>  
> -	ret = wait_event_timeout(mgr->tx_waitq,
> -				 check_txmsg_state(mgr, txmsg),
> -				 (4 * HZ));
> +	for (;;) {
> +		ret = wait_event_timeout(mgr->tx_waitq,
> +					 check_txmsg_state(mgr, txmsg),
> +					 mgr->cbs->update_hpd_irq_state ?
> +						msecs_to_jiffies(50) :
> +						wait_timeout);
> +
> +		if (ret || !mgr->cbs->update_hpd_irq_state ||
> +		    time_after(jiffies, wait_expires))
> +			break;

First I thought this was changing the behaviour when the callback
isn't provided, but then I noticed the ?: stuff for the timeout.

I think this stuff deserves a comment to explain why we would
ever do such a thing instead of simply waiting like we did before.

> +
> +		mgr->cbs->update_hpd_irq_state(mgr);
> +	}
> +
>  	mutex_lock(&mgr->qlock);
>  	if (ret > 0) {
>  		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index d18b406f2a7d..1ff7d0096262 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -765,8 +765,23 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
>  	return NULL;
>  }
>  
> +static void
> +intel_dp_mst_update_hpd_irq_state(struct drm_dp_mst_topology_mgr *mgr)
> +{
> +	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +
> +	spin_lock_irq(&i915->irq_lock);
> +	i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
> +	spin_unlock_irq(&i915->irq_lock);
> +
> +	queue_work(i915->hotplug.dp_wq, &i915->hotplug.dig_port_work);

I might suggest putting this code right next to intel_hpd_irq_handler()
so that people can actually see it when working on the hotplug code.

> +}
> +
>  static const struct drm_dp_mst_topology_cbs mst_cbs = {
>  	.add_connector = intel_dp_add_mst_connector,
> +	.update_hpd_irq_state = intel_dp_mst_update_hpd_irq_state,
>  };
>  
>  static struct intel_dp_mst_encoder *
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 9e1ffcd7cb68..c902f4380200 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -475,6 +475,7 @@ struct drm_dp_mst_topology_mgr;
>  struct drm_dp_mst_topology_cbs {
>  	/* create a connector for a port */
>  	struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path);
> +	void (*update_hpd_irq_state)(struct drm_dp_mst_topology_mgr *mgr);

I guess a bit of docs for this might be nice. Maybe s/update/poll/
might make the intention more clear? Not sure.

>  };
>  
>  #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)
> -- 
> 2.23.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses
  2020-06-04 15:12     ` [Intel-gfx] " Ville Syrjälä
@ 2020-06-04 15:41       ` Imre Deak
  0 siblings, 0 replies; 17+ messages in thread
From: Imre Deak @ 2020-06-04 15:41 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel

On Thu, Jun 04, 2020 at 06:12:27PM +0300, Ville Syrjälä wrote:
> On Thu, Jun 04, 2020 at 01:18:59AM +0300, Imre Deak wrote:
> > Some TypeC -> native DP adapters, at least the Club CAC-1557 adapter,
> > incorrectly filter out HPD short pulses with a duration less than ~540
> > usec, leading to MST probe failures.
> > 
> > According to the DP alt mode specification adapters should forward short
> > pulses with a duration greater than 250 usec. According to the DP
> > specificatin DP sources should detect short pulses in the
> > 500 usec -> 2 ms range. 
> 
> IIRC it was 250 usec -> 2 ms as well in the DP spec.
> 
> 500 usec -> 1 ms is the duration of the short hpd
> the signalling side should use.

Ah, correct (and this is what makes actually sense). For reference it's
described under "5.1.4 Source Device Behavior upon HPD Pulse Detection"

> > Based on this filtering out short pulses with a
> > duration less than 540 usec is incorrect.
> > 
> > To make such adapters work add support for a driver polling on MST
> > inerrupt flags, and wire this up in the i915 driver. The sink can clear
> > an interrupt it raised after 110 ms if the source doesn't respond, so
> > use a 50 ms poll period to avoid missing an interrupt. Polling of the
> > MST interrupt flags is explicitly allowed by the DP specification.
> > 
> > This fixes MST probe failures I saw using this adapter and a DELL U2515H
> > monitor.
> > 
> > v2:
> > - Fix the wait event timeout for the no-poll case.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c       | 19 ++++++++++++++++---
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 15 +++++++++++++++
> >  include/drm/drm_dp_mst_helper.h             |  1 +
> >  3 files changed, 32 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 5bc72e800b85..4e987a513df8 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1178,11 +1178,24 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
> >  				    struct drm_dp_sideband_msg_tx *txmsg)
> >  {
> >  	struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
> > +	unsigned long wait_timeout = msecs_to_jiffies(4000);
> > +	unsigned long wait_expires = jiffies + wait_timeout;
> >  	int ret;
> >  
> > -	ret = wait_event_timeout(mgr->tx_waitq,
> > -				 check_txmsg_state(mgr, txmsg),
> > -				 (4 * HZ));
> > +	for (;;) {
> > +		ret = wait_event_timeout(mgr->tx_waitq,
> > +					 check_txmsg_state(mgr, txmsg),
> > +					 mgr->cbs->update_hpd_irq_state ?
> > +						msecs_to_jiffies(50) :
> > +						wait_timeout);
> > +
> > +		if (ret || !mgr->cbs->update_hpd_irq_state ||
> > +		    time_after(jiffies, wait_expires))
> > +			break;
> 
> First I thought this was changing the behaviour when the callback
> isn't provided, but then I noticed the ?: stuff for the timeout.
>
> I think this stuff deserves a comment to explain why we would
> ever do such a thing instead of simply waiting like we did before.

Ok, will add a compact form of the commit log explanation.

> 
> > +
> > +		mgr->cbs->update_hpd_irq_state(mgr);
> > +	}
> > +
> >  	mutex_lock(&mgr->qlock);
> >  	if (ret > 0) {
> >  		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index d18b406f2a7d..1ff7d0096262 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -765,8 +765,23 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
> >  	return NULL;
> >  }
> >  
> > +static void
> > +intel_dp_mst_update_hpd_irq_state(struct drm_dp_mst_topology_mgr *mgr)
> > +{
> > +	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
> > +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > +
> > +	spin_lock_irq(&i915->irq_lock);
> > +	i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
> > +	spin_unlock_irq(&i915->irq_lock);
> > +
> > +	queue_work(i915->hotplug.dp_wq, &i915->hotplug.dig_port_work);
> 
> I might suggest putting this code right next to intel_hpd_irq_handler()
> so that people can actually see it when working on the hotplug code.

Ok.

> 
> > +}
> > +
> >  static const struct drm_dp_mst_topology_cbs mst_cbs = {
> >  	.add_connector = intel_dp_add_mst_connector,
> > +	.update_hpd_irq_state = intel_dp_mst_update_hpd_irq_state,
> >  };
> >  
> >  static struct intel_dp_mst_encoder *
> > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> > index 9e1ffcd7cb68..c902f4380200 100644
> > --- a/include/drm/drm_dp_mst_helper.h
> > +++ b/include/drm/drm_dp_mst_helper.h
> > @@ -475,6 +475,7 @@ struct drm_dp_mst_topology_mgr;
> >  struct drm_dp_mst_topology_cbs {
> >  	/* create a connector for a port */
> >  	struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path);
> > +	void (*update_hpd_irq_state)(struct drm_dp_mst_topology_mgr *mgr);
> 
> I guess a bit of docs for this might be nice. Maybe s/update/poll/
> might make the intention more clear? Not sure.

Ok.

> 
> >  };
> >  
> >  #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)
> > -- 
> > 2.23.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v2 1/3] drm/i915/dp_mst: Fix disabling MST on a port
  2020-06-03 21:10 [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Imre Deak
                   ` (3 preceding siblings ...)
  2020-06-04 14:55 ` Ville Syrjälä
@ 2020-06-04 18:44 ` Imre Deak
       [not found] ` <159136523293.18507.17008252253062518394@emeril.freedesktop.org>
  5 siblings, 0 replies; 17+ messages in thread
From: Imre Deak @ 2020-06-04 18:44 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: José Roberto de Souza

Currently MST on a port can get enabled/disabled from the hotplug work
and get disabled from the short pulse work in a racy way. Fix this by
relying on the MST state checking in the hotplug work and just schedule
a hotplug work from the short pulse handler if some problem happened
during the MST interrupt handling.

This removes the explicit MST disabling in case of an AUX failure, but
if AUX fails, then probably the detection will also fail during the
scheduled hotplug work and it's not guaranteed that we'll see
intermittent errors anyway.

While at it also simplify the error checking of the MST interrupt
handler.

v2:
- Convert intel_dp_check_mst_status() to return bool. (Ville)
- Change the intel_dp->is_mst check to an assert, since after this patch
  the condition can't change after we checked it previously.
- Document the return value from intel_dp_check_mst_status().

Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com> (v1)
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 67 ++++++++++---------------
 1 file changed, 27 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 55fda074c0ad..4b6e7cf577dd 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5556,35 +5556,47 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 			    "Could not write test response to sink\n");
 }
 
-static int
+/**
+ * intel_dp_check_mst_status - service any pending MST interrupts, check link status
+ * @intel_dp: Intel DP struct
+ *
+ * Read any pending MST interrupts, call MST core to handle these and ack the
+ * interrupts. Check if the main and AUX link state is ok.
+ *
+ * Returns:
+ * - %true if pending interrupts were serviced (or no interrupts were
+ *   pending) w/o detecting an error condition.
+ * - %false if an error condition - like AUX failure or a loss of link - is
+ *   detected, which needs servicing from the hotplug work.
+ */
+static bool
 intel_dp_check_mst_status(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
-	bool need_retrain = false;
-
-	if (!intel_dp->is_mst)
-		return -EINVAL;
+	bool link_ok = true;
 
+	drm_WARN_ON_ONCE(&i915->drm, !intel_dp->is_mst);
 	drm_WARN_ON_ONCE(&i915->drm, intel_dp->active_mst_links < 0);
 
 	for (;;) {
 		u8 esi[DP_DPRX_ESI_LEN] = {};
-		bool bret, handled;
+		bool handled;
 		int retry;
 
-		bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
-		if (!bret) {
+		if (!intel_dp_get_sink_irq_esi(intel_dp, esi)) {
 			drm_dbg_kms(&i915->drm,
 				    "failed to get ESI - device may have failed\n");
-			return -EINVAL;
+			link_ok = false;
+
+			break;
 		}
 
 		/* check link status - esi[10] = 0x200c */
-		if (intel_dp->active_mst_links > 0 && !need_retrain &&
+		if (intel_dp->active_mst_links > 0 && link_ok &&
 		    !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
 			drm_dbg_kms(&i915->drm,
 				    "channel EQ not ok, retraining\n");
-			need_retrain = true;
+			link_ok = false;
 		}
 
 		drm_dbg_kms(&i915->drm, "got esi %3ph\n", esi);
@@ -5604,7 +5616,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 		}
 	}
 
-	return need_retrain;
+	return link_ok;
 }
 
 static bool
@@ -7255,35 +7267,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	}
 
 	if (intel_dp->is_mst) {
-		switch (intel_dp_check_mst_status(intel_dp)) {
-		case -EINVAL:
-			/*
-			 * If we were in MST mode, and device is not
-			 * there, get out of MST mode
-			 */
-			drm_dbg_kms(&i915->drm,
-				    "MST device may have disappeared %d vs %d\n",
-				    intel_dp->is_mst,
-				    intel_dp->mst_mgr.mst_state);
-			intel_dp->is_mst = false;
-			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
-							intel_dp->is_mst);
-
-			return IRQ_NONE;
-		case 1:
-			return IRQ_NONE;
-		default:
-			break;
-		}
-	}
-
-	if (!intel_dp->is_mst) {
-		bool handled;
-
-		handled = intel_dp_short_pulse(intel_dp);
-
-		if (!handled)
+		if (!intel_dp_check_mst_status(intel_dp))
 			return IRQ_NONE;
+	} else if (!intel_dp_short_pulse(intel_dp)) {
+		return IRQ_NONE;
 	}
 
 	return IRQ_HANDLED;
-- 
2.23.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v3 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses
  2020-06-03 21:10 ` [PATCH 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses Imre Deak
  2020-06-03 22:18   ` [PATCH v2 " Imre Deak
@ 2020-06-04 18:45   ` Imre Deak
  2020-06-04 18:54     ` Ville Syrjälä
  2020-06-09 12:15     ` [Intel-gfx] " Imre Deak
  1 sibling, 2 replies; 17+ messages in thread
From: Imre Deak @ 2020-06-04 18:45 UTC (permalink / raw)
  To: intel-gfx, dri-devel

Some TypeC -> native DP adapters, at least the Club 3D CAC-1557 adapter,
incorrectly filter out HPD short pulses with a duration less than
~540 usec, leading to MST probe failures.

According to the DP Standard 2.0 section 5.1.4:
- DP sinks should generate short pulses in the 500 usec -> 1 msec range
- DP sources should detect short pulses in the 250 usec -> 2 msec range

According to the DP Alt Mode on TypeC Standard section 3.9.2, adapters
should detect and forward short pulses according to how sources should
detect them as specified in the DP Standard (250 usec -> 2 msec).

Based on the above filtering out short pulses with a duration less than
540 usec is incorrect.

To make such adapters work add support for a driver polling on MST
inerrupt flags, and wire this up in the i915 driver. The sink can clear
an interrupt it raised after 110 msec if the source doesn't respond, so
use a 50 msec poll period to avoid missing an interrupt. Polling of the
MST interrupt flags is explicitly allowed by the DP Standard.

This fixes MST probe failures I saw using this adapter and a DELL U2515H
monitor.

v2:
- Fix the wait event timeout for the no-poll case.
v3 (Ville):
- Fix the short pulse duration limits in the commit log prescribed by the
  DP Standard.
- Add code comment explaining why/how polling is used.
- Factor out a helper to schedule the port's hpd irq handler and move it
  to the rest of hotplug handlers.
- Document the new MST callback.
- s/update_hpd_irq_state/poll_hpd_irq/

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c        | 32 ++++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_dp_mst.c  | 10 ++++++
 drivers/gpu/drm/i915/display/intel_hotplug.c | 18 +++++++++++
 drivers/gpu/drm/i915/display/intel_hotplug.h |  2 ++
 include/drm/drm_dp_mst_helper.h              |  9 ++++++
 5 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 5bc72e800b85..2a309fb2c4cc 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1178,11 +1178,37 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
 				    struct drm_dp_sideband_msg_tx *txmsg)
 {
 	struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
+	unsigned long wait_timeout = msecs_to_jiffies(4000);
+	unsigned long wait_expires = jiffies + wait_timeout;
 	int ret;
 
-	ret = wait_event_timeout(mgr->tx_waitq,
-				 check_txmsg_state(mgr, txmsg),
-				 (4 * HZ));
+	for (;;) {
+		/*
+		 * If the driver provides a way for this, change to
+		 * poll-waiting for the MST reply interrupt if we didn't receive
+		 * it for 50 msec. This would cater for cases where the HPD
+		 * pulse signal got lost somewhere, even though the sink raised
+		 * the corresponding MST interrupt correctly. One example is the
+		 * Club 3D CAC-1557 TypeC -> DP adapter which for some reason
+		 * filters out short pulses with a duration less than ~540 usec.
+		 *
+		 * The poll period is 50 msec to avoid missing an interrupt
+		 * after the sink has cleared it (after a 110msec timeout
+		 * since it raised the interrupt).
+		 */
+		ret = wait_event_timeout(mgr->tx_waitq,
+					 check_txmsg_state(mgr, txmsg),
+					 mgr->cbs->poll_hpd_irq ?
+						msecs_to_jiffies(50) :
+						wait_timeout);
+
+		if (ret || !mgr->cbs->poll_hpd_irq ||
+		    time_after(jiffies, wait_expires))
+			break;
+
+		mgr->cbs->poll_hpd_irq(mgr);
+	}
+
 	mutex_lock(&mgr->qlock);
 	if (ret > 0) {
 		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index d18b406f2a7d..9be52643205d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -33,6 +33,7 @@
 #include "intel_connector.h"
 #include "intel_ddi.h"
 #include "intel_display_types.h"
+#include "intel_hotplug.h"
 #include "intel_dp.h"
 #include "intel_dp_mst.h"
 #include "intel_dpio_phy.h"
@@ -765,8 +766,17 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
 	return NULL;
 }
 
+static void
+intel_dp_mst_poll_hpd_irq(struct drm_dp_mst_topology_mgr *mgr)
+{
+	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
+
+	intel_hpd_trigger_irq(dp_to_dig_port(intel_dp));
+}
+
 static const struct drm_dp_mst_topology_cbs mst_cbs = {
 	.add_connector = intel_dp_add_mst_connector,
+	.poll_hpd_irq = intel_dp_mst_poll_hpd_irq,
 };
 
 static struct intel_dp_mst_encoder *
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 4f6f560e093e..664f88354101 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -347,6 +347,24 @@ static void i915_digport_work_func(struct work_struct *work)
 	}
 }
 
+/**
+ * intel_hpd_trigger_irq - trigger an hpd irq event for a port
+ * @dig_port: digital port
+ *
+ * Trigger an HPD interrupt event for the given port, emulating a short pulse
+ * generated by the sink, and schedule the dig port work to handle it.
+ */
+void intel_hpd_trigger_irq(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+
+	spin_lock_irq(&i915->irq_lock);
+	i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
+	spin_unlock_irq(&i915->irq_lock);
+
+	queue_work(i915->hotplug.dp_wq, &i915->hotplug.dig_port_work);
+}
+
 /*
  * Handle hotplug events outside the interrupt handler proper.
  */
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
index 777b0743257e..a704d7c94d16 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.h
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
@@ -10,6 +10,7 @@
 
 struct drm_i915_private;
 struct intel_connector;
+struct intel_digital_port;
 struct intel_encoder;
 enum port;
 
@@ -18,6 +19,7 @@ enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
 					       struct intel_connector *connector);
 void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 			   u32 pin_mask, u32 long_mask);
+void intel_hpd_trigger_irq(struct intel_digital_port *dig_port);
 void intel_hpd_init(struct drm_i915_private *dev_priv);
 void intel_hpd_init_work(struct drm_i915_private *dev_priv);
 void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 9e1ffcd7cb68..b230ff6f7081 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -475,6 +475,15 @@ struct drm_dp_mst_topology_mgr;
 struct drm_dp_mst_topology_cbs {
 	/* create a connector for a port */
 	struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path);
+	/*
+	 * Checks for any pending MST interrupts, passing them to MST core for
+	 * processing, the same way an HPD IRQ pulse handler would do this.
+	 * If provided MST core calls this callback from a poll-waiting loop
+	 * when waiting for MST down message replies. The driver is expected
+	 * to guard against a race between this callback and the driver's HPD
+	 * IRQ pulse handler.
+	 */
+	void (*poll_hpd_irq)(struct drm_dp_mst_topology_mgr *mgr);
 };
 
 #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)
-- 
2.23.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v3 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses
  2020-06-04 18:45   ` [PATCH v3 " Imre Deak
@ 2020-06-04 18:54     ` Ville Syrjälä
  2020-06-09 12:15     ` [Intel-gfx] " Imre Deak
  1 sibling, 0 replies; 17+ messages in thread
From: Ville Syrjälä @ 2020-06-04 18:54 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx, dri-devel

On Thu, Jun 04, 2020 at 09:45:00PM +0300, Imre Deak wrote:
> Some TypeC -> native DP adapters, at least the Club 3D CAC-1557 adapter,
> incorrectly filter out HPD short pulses with a duration less than
> ~540 usec, leading to MST probe failures.
> 
> According to the DP Standard 2.0 section 5.1.4:
> - DP sinks should generate short pulses in the 500 usec -> 1 msec range
> - DP sources should detect short pulses in the 250 usec -> 2 msec range
> 
> According to the DP Alt Mode on TypeC Standard section 3.9.2, adapters
> should detect and forward short pulses according to how sources should
> detect them as specified in the DP Standard (250 usec -> 2 msec).
> 
> Based on the above filtering out short pulses with a duration less than
> 540 usec is incorrect.
> 
> To make such adapters work add support for a driver polling on MST
> inerrupt flags, and wire this up in the i915 driver. The sink can clear
> an interrupt it raised after 110 msec if the source doesn't respond, so
> use a 50 msec poll period to avoid missing an interrupt. Polling of the
> MST interrupt flags is explicitly allowed by the DP Standard.
> 
> This fixes MST probe failures I saw using this adapter and a DELL U2515H
> monitor.
> 
> v2:
> - Fix the wait event timeout for the no-poll case.
> v3 (Ville):
> - Fix the short pulse duration limits in the commit log prescribed by the
>   DP Standard.
> - Add code comment explaining why/how polling is used.
> - Factor out a helper to schedule the port's hpd irq handler and move it
>   to the rest of hotplug handlers.
> - Document the new MST callback.
> - s/update_hpd_irq_state/poll_hpd_irq/
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c        | 32 ++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c  | 10 ++++++
>  drivers/gpu/drm/i915/display/intel_hotplug.c | 18 +++++++++++
>  drivers/gpu/drm/i915/display/intel_hotplug.h |  2 ++
>  include/drm/drm_dp_mst_helper.h              |  9 ++++++
>  5 files changed, 68 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 5bc72e800b85..2a309fb2c4cc 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1178,11 +1178,37 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
>  				    struct drm_dp_sideband_msg_tx *txmsg)
>  {
>  	struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
> +	unsigned long wait_timeout = msecs_to_jiffies(4000);
> +	unsigned long wait_expires = jiffies + wait_timeout;
>  	int ret;
>  
> -	ret = wait_event_timeout(mgr->tx_waitq,
> -				 check_txmsg_state(mgr, txmsg),
> -				 (4 * HZ));
> +	for (;;) {
> +		/*
> +		 * If the driver provides a way for this, change to
> +		 * poll-waiting for the MST reply interrupt if we didn't receive
> +		 * it for 50 msec. This would cater for cases where the HPD
> +		 * pulse signal got lost somewhere, even though the sink raised
> +		 * the corresponding MST interrupt correctly. One example is the
> +		 * Club 3D CAC-1557 TypeC -> DP adapter which for some reason
> +		 * filters out short pulses with a duration less than ~540 usec.
> +		 *
> +		 * The poll period is 50 msec to avoid missing an interrupt
> +		 * after the sink has cleared it (after a 110msec timeout
> +		 * since it raised the interrupt).
> +		 */
> +		ret = wait_event_timeout(mgr->tx_waitq,
> +					 check_txmsg_state(mgr, txmsg),
> +					 mgr->cbs->poll_hpd_irq ?
> +						msecs_to_jiffies(50) :
> +						wait_timeout);
> +
> +		if (ret || !mgr->cbs->poll_hpd_irq ||
> +		    time_after(jiffies, wait_expires))
> +			break;
> +
> +		mgr->cbs->poll_hpd_irq(mgr);
> +	}
> +
>  	mutex_lock(&mgr->qlock);
>  	if (ret > 0) {
>  		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index d18b406f2a7d..9be52643205d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -33,6 +33,7 @@
>  #include "intel_connector.h"
>  #include "intel_ddi.h"
>  #include "intel_display_types.h"
> +#include "intel_hotplug.h"
>  #include "intel_dp.h"
>  #include "intel_dp_mst.h"
>  #include "intel_dpio_phy.h"
> @@ -765,8 +766,17 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
>  	return NULL;
>  }
>  
> +static void
> +intel_dp_mst_poll_hpd_irq(struct drm_dp_mst_topology_mgr *mgr)
> +{
> +	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
> +
> +	intel_hpd_trigger_irq(dp_to_dig_port(intel_dp));
> +}
> +
>  static const struct drm_dp_mst_topology_cbs mst_cbs = {
>  	.add_connector = intel_dp_add_mst_connector,
> +	.poll_hpd_irq = intel_dp_mst_poll_hpd_irq,
>  };
>  
>  static struct intel_dp_mst_encoder *
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 4f6f560e093e..664f88354101 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -347,6 +347,24 @@ static void i915_digport_work_func(struct work_struct *work)
>  	}
>  }
>  
> +/**
> + * intel_hpd_trigger_irq - trigger an hpd irq event for a port
> + * @dig_port: digital port
> + *
> + * Trigger an HPD interrupt event for the given port, emulating a short pulse
> + * generated by the sink, and schedule the dig port work to handle it.
> + */
> +void intel_hpd_trigger_irq(struct intel_digital_port *dig_port)
> +{
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +
> +	spin_lock_irq(&i915->irq_lock);
> +	i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
> +	spin_unlock_irq(&i915->irq_lock);
> +
> +	queue_work(i915->hotplug.dp_wq, &i915->hotplug.dig_port_work);
> +}
> +
>  /*
>   * Handle hotplug events outside the interrupt handler proper.
>   */
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
> index 777b0743257e..a704d7c94d16 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> @@ -10,6 +10,7 @@
>  
>  struct drm_i915_private;
>  struct intel_connector;
> +struct intel_digital_port;
>  struct intel_encoder;
>  enum port;
>  
> @@ -18,6 +19,7 @@ enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
>  					       struct intel_connector *connector);
>  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  			   u32 pin_mask, u32 long_mask);
> +void intel_hpd_trigger_irq(struct intel_digital_port *dig_port);
>  void intel_hpd_init(struct drm_i915_private *dev_priv);
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv);
>  void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 9e1ffcd7cb68..b230ff6f7081 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -475,6 +475,15 @@ struct drm_dp_mst_topology_mgr;
>  struct drm_dp_mst_topology_cbs {
>  	/* create a connector for a port */
>  	struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path);
> +	/*
> +	 * Checks for any pending MST interrupts, passing them to MST core for
> +	 * processing, the same way an HPD IRQ pulse handler would do this.
> +	 * If provided MST core calls this callback from a poll-waiting loop
> +	 * when waiting for MST down message replies. The driver is expected
> +	 * to guard against a race between this callback and the driver's HPD
> +	 * IRQ pulse handler.
> +	 */
> +	void (*poll_hpd_irq)(struct drm_dp_mst_topology_mgr *mgr);
>  };
>  
>  #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)
> -- 
> 2.23.1

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

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

* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses
  2020-06-04 18:45   ` [PATCH v3 " Imre Deak
  2020-06-04 18:54     ` Ville Syrjälä
@ 2020-06-09 12:15     ` Imre Deak
  2020-06-09 15:58       ` Lyude Paul
  1 sibling, 1 reply; 17+ messages in thread
From: Imre Deak @ 2020-06-09 12:15 UTC (permalink / raw)
  To: Dave Airlie, Lyude Paul; +Cc: intel-gfx, dri-devel

Hi Dave, Lyude,

are you ok to merge this patchset via the drm-intel-next-queued tree?

--Imre

On Thu, Jun 04, 2020 at 09:45:00PM +0300, Imre Deak wrote:
> Some TypeC -> native DP adapters, at least the Club 3D CAC-1557 adapter,
> incorrectly filter out HPD short pulses with a duration less than
> ~540 usec, leading to MST probe failures.
> 
> According to the DP Standard 2.0 section 5.1.4:
> - DP sinks should generate short pulses in the 500 usec -> 1 msec range
> - DP sources should detect short pulses in the 250 usec -> 2 msec range
> 
> According to the DP Alt Mode on TypeC Standard section 3.9.2, adapters
> should detect and forward short pulses according to how sources should
> detect them as specified in the DP Standard (250 usec -> 2 msec).
> 
> Based on the above filtering out short pulses with a duration less than
> 540 usec is incorrect.
> 
> To make such adapters work add support for a driver polling on MST
> inerrupt flags, and wire this up in the i915 driver. The sink can clear
> an interrupt it raised after 110 msec if the source doesn't respond, so
> use a 50 msec poll period to avoid missing an interrupt. Polling of the
> MST interrupt flags is explicitly allowed by the DP Standard.
> 
> This fixes MST probe failures I saw using this adapter and a DELL U2515H
> monitor.
> 
> v2:
> - Fix the wait event timeout for the no-poll case.
> v3 (Ville):
> - Fix the short pulse duration limits in the commit log prescribed by the
>   DP Standard.
> - Add code comment explaining why/how polling is used.
> - Factor out a helper to schedule the port's hpd irq handler and move it
>   to the rest of hotplug handlers.
> - Document the new MST callback.
> - s/update_hpd_irq_state/poll_hpd_irq/
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c        | 32 ++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c  | 10 ++++++
>  drivers/gpu/drm/i915/display/intel_hotplug.c | 18 +++++++++++
>  drivers/gpu/drm/i915/display/intel_hotplug.h |  2 ++
>  include/drm/drm_dp_mst_helper.h              |  9 ++++++
>  5 files changed, 68 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 5bc72e800b85..2a309fb2c4cc 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1178,11 +1178,37 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
>  				    struct drm_dp_sideband_msg_tx *txmsg)
>  {
>  	struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
> +	unsigned long wait_timeout = msecs_to_jiffies(4000);
> +	unsigned long wait_expires = jiffies + wait_timeout;
>  	int ret;
>  
> -	ret = wait_event_timeout(mgr->tx_waitq,
> -				 check_txmsg_state(mgr, txmsg),
> -				 (4 * HZ));
> +	for (;;) {
> +		/*
> +		 * If the driver provides a way for this, change to
> +		 * poll-waiting for the MST reply interrupt if we didn't receive
> +		 * it for 50 msec. This would cater for cases where the HPD
> +		 * pulse signal got lost somewhere, even though the sink raised
> +		 * the corresponding MST interrupt correctly. One example is the
> +		 * Club 3D CAC-1557 TypeC -> DP adapter which for some reason
> +		 * filters out short pulses with a duration less than ~540 usec.
> +		 *
> +		 * The poll period is 50 msec to avoid missing an interrupt
> +		 * after the sink has cleared it (after a 110msec timeout
> +		 * since it raised the interrupt).
> +		 */
> +		ret = wait_event_timeout(mgr->tx_waitq,
> +					 check_txmsg_state(mgr, txmsg),
> +					 mgr->cbs->poll_hpd_irq ?
> +						msecs_to_jiffies(50) :
> +						wait_timeout);
> +
> +		if (ret || !mgr->cbs->poll_hpd_irq ||
> +		    time_after(jiffies, wait_expires))
> +			break;
> +
> +		mgr->cbs->poll_hpd_irq(mgr);
> +	}
> +
>  	mutex_lock(&mgr->qlock);
>  	if (ret > 0) {
>  		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index d18b406f2a7d..9be52643205d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -33,6 +33,7 @@
>  #include "intel_connector.h"
>  #include "intel_ddi.h"
>  #include "intel_display_types.h"
> +#include "intel_hotplug.h"
>  #include "intel_dp.h"
>  #include "intel_dp_mst.h"
>  #include "intel_dpio_phy.h"
> @@ -765,8 +766,17 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
>  	return NULL;
>  }
>  
> +static void
> +intel_dp_mst_poll_hpd_irq(struct drm_dp_mst_topology_mgr *mgr)
> +{
> +	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
> +
> +	intel_hpd_trigger_irq(dp_to_dig_port(intel_dp));
> +}
> +
>  static const struct drm_dp_mst_topology_cbs mst_cbs = {
>  	.add_connector = intel_dp_add_mst_connector,
> +	.poll_hpd_irq = intel_dp_mst_poll_hpd_irq,
>  };
>  
>  static struct intel_dp_mst_encoder *
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 4f6f560e093e..664f88354101 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -347,6 +347,24 @@ static void i915_digport_work_func(struct work_struct *work)
>  	}
>  }
>  
> +/**
> + * intel_hpd_trigger_irq - trigger an hpd irq event for a port
> + * @dig_port: digital port
> + *
> + * Trigger an HPD interrupt event for the given port, emulating a short pulse
> + * generated by the sink, and schedule the dig port work to handle it.
> + */
> +void intel_hpd_trigger_irq(struct intel_digital_port *dig_port)
> +{
> +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> +
> +	spin_lock_irq(&i915->irq_lock);
> +	i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
> +	spin_unlock_irq(&i915->irq_lock);
> +
> +	queue_work(i915->hotplug.dp_wq, &i915->hotplug.dig_port_work);
> +}
> +
>  /*
>   * Handle hotplug events outside the interrupt handler proper.
>   */
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
> index 777b0743257e..a704d7c94d16 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> @@ -10,6 +10,7 @@
>  
>  struct drm_i915_private;
>  struct intel_connector;
> +struct intel_digital_port;
>  struct intel_encoder;
>  enum port;
>  
> @@ -18,6 +19,7 @@ enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
>  					       struct intel_connector *connector);
>  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  			   u32 pin_mask, u32 long_mask);
> +void intel_hpd_trigger_irq(struct intel_digital_port *dig_port);
>  void intel_hpd_init(struct drm_i915_private *dev_priv);
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv);
>  void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 9e1ffcd7cb68..b230ff6f7081 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -475,6 +475,15 @@ struct drm_dp_mst_topology_mgr;
>  struct drm_dp_mst_topology_cbs {
>  	/* create a connector for a port */
>  	struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path);
> +	/*
> +	 * Checks for any pending MST interrupts, passing them to MST core for
> +	 * processing, the same way an HPD IRQ pulse handler would do this.
> +	 * If provided MST core calls this callback from a poll-waiting loop
> +	 * when waiting for MST down message replies. The driver is expected
> +	 * to guard against a race between this callback and the driver's HPD
> +	 * IRQ pulse handler.
> +	 */
> +	void (*poll_hpd_irq)(struct drm_dp_mst_topology_mgr *mgr);
>  };
>  
>  #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)
> -- 
> 2.23.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses
  2020-06-09 12:15     ` [Intel-gfx] " Imre Deak
@ 2020-06-09 15:58       ` Lyude Paul
  2020-06-09 18:03         ` Imre Deak
  0 siblings, 1 reply; 17+ messages in thread
From: Lyude Paul @ 2020-06-09 15:58 UTC (permalink / raw)
  To: imre.deak, Dave Airlie; +Cc: intel-gfx, dri-devel

Hi! Awesome patch series!

Reviewed-by: Lyude Paul <lyude@redhat.com>

Also re merging via drm-intel-next-queued - I think that should be fine, fwiw
merging via drm-misc-next might be another option (I've definitely done this in
the past for series that touched MST and drivers, but I don't have a hard
preference either way).

On Tue, 2020-06-09 at 15:15 +0300, Imre Deak wrote:
> Hi Dave, Lyude,
> 
> are you ok to merge this patchset via the drm-intel-next-queued tree?
> 
> --Imre
> 
> On Thu, Jun 04, 2020 at 09:45:00PM +0300, Imre Deak wrote:
> > Some TypeC -> native DP adapters, at least the Club 3D CAC-1557 adapter,
> > incorrectly filter out HPD short pulses with a duration less than
> > ~540 usec, leading to MST probe failures.
> > 
> > According to the DP Standard 2.0 section 5.1.4:
> > - DP sinks should generate short pulses in the 500 usec -> 1 msec range
> > - DP sources should detect short pulses in the 250 usec -> 2 msec range
> > 
> > According to the DP Alt Mode on TypeC Standard section 3.9.2, adapters
> > should detect and forward short pulses according to how sources should
> > detect them as specified in the DP Standard (250 usec -> 2 msec).
> > 
> > Based on the above filtering out short pulses with a duration less than
> > 540 usec is incorrect.
> > 
> > To make such adapters work add support for a driver polling on MST
> > inerrupt flags, and wire this up in the i915 driver. The sink can clear
> > an interrupt it raised after 110 msec if the source doesn't respond, so
> > use a 50 msec poll period to avoid missing an interrupt. Polling of the
> > MST interrupt flags is explicitly allowed by the DP Standard.
> > 
> > This fixes MST probe failures I saw using this adapter and a DELL U2515H
> > monitor.
> > 
> > v2:
> > - Fix the wait event timeout for the no-poll case.
> > v3 (Ville):
> > - Fix the short pulse duration limits in the commit log prescribed by the
> >   DP Standard.
> > - Add code comment explaining why/how polling is used.
> > - Factor out a helper to schedule the port's hpd irq handler and move it
> >   to the rest of hotplug handlers.
> > - Document the new MST callback.
> > - s/update_hpd_irq_state/poll_hpd_irq/
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c        | 32 ++++++++++++++++++--
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c  | 10 ++++++
> >  drivers/gpu/drm/i915/display/intel_hotplug.c | 18 +++++++++++
> >  drivers/gpu/drm/i915/display/intel_hotplug.h |  2 ++
> >  include/drm/drm_dp_mst_helper.h              |  9 ++++++
> >  5 files changed, 68 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 5bc72e800b85..2a309fb2c4cc 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1178,11 +1178,37 @@ static int drm_dp_mst_wait_tx_reply(struct
> > drm_dp_mst_branch *mstb,
> >  				    struct drm_dp_sideband_msg_tx *txmsg)
> >  {
> >  	struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
> > +	unsigned long wait_timeout = msecs_to_jiffies(4000);
> > +	unsigned long wait_expires = jiffies + wait_timeout;
> >  	int ret;
> >  
> > -	ret = wait_event_timeout(mgr->tx_waitq,
> > -				 check_txmsg_state(mgr, txmsg),
> > -				 (4 * HZ));
> > +	for (;;) {
> > +		/*
> > +		 * If the driver provides a way for this, change to
> > +		 * poll-waiting for the MST reply interrupt if we didn't receive
> > +		 * it for 50 msec. This would cater for cases where the HPD
> > +		 * pulse signal got lost somewhere, even though the sink raised
> > +		 * the corresponding MST interrupt correctly. One example is the
> > +		 * Club 3D CAC-1557 TypeC -> DP adapter which for some reason
> > +		 * filters out short pulses with a duration less than ~540 usec.
> > +		 *
> > +		 * The poll period is 50 msec to avoid missing an interrupt
> > +		 * after the sink has cleared it (after a 110msec timeout
> > +		 * since it raised the interrupt).
> > +		 */
> > +		ret = wait_event_timeout(mgr->tx_waitq,
> > +					 check_txmsg_state(mgr, txmsg),
> > +					 mgr->cbs->poll_hpd_irq ?
> > +						msecs_to_jiffies(50) :
> > +						wait_timeout);
> > +
> > +		if (ret || !mgr->cbs->poll_hpd_irq ||
> > +		    time_after(jiffies, wait_expires))
> > +			break;
> > +
> > +		mgr->cbs->poll_hpd_irq(mgr);
> > +	}
> > +
> >  	mutex_lock(&mgr->qlock);
> >  	if (ret > 0) {
> >  		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index d18b406f2a7d..9be52643205d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -33,6 +33,7 @@
> >  #include "intel_connector.h"
> >  #include "intel_ddi.h"
> >  #include "intel_display_types.h"
> > +#include "intel_hotplug.h"
> >  #include "intel_dp.h"
> >  #include "intel_dp_mst.h"
> >  #include "intel_dpio_phy.h"
> > @@ -765,8 +766,17 @@ static struct drm_connector
> > *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
> >  	return NULL;
> >  }
> >  
> > +static void
> > +intel_dp_mst_poll_hpd_irq(struct drm_dp_mst_topology_mgr *mgr)
> > +{
> > +	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
> > +
> > +	intel_hpd_trigger_irq(dp_to_dig_port(intel_dp));
> > +}
> > +
> >  static const struct drm_dp_mst_topology_cbs mst_cbs = {
> >  	.add_connector = intel_dp_add_mst_connector,
> > +	.poll_hpd_irq = intel_dp_mst_poll_hpd_irq,
> >  };
> >  
> >  static struct intel_dp_mst_encoder *
> > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > index 4f6f560e093e..664f88354101 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > @@ -347,6 +347,24 @@ static void i915_digport_work_func(struct work_struct
> > *work)
> >  	}
> >  }
> >  
> > +/**
> > + * intel_hpd_trigger_irq - trigger an hpd irq event for a port
> > + * @dig_port: digital port
> > + *
> > + * Trigger an HPD interrupt event for the given port, emulating a short
> > pulse
> > + * generated by the sink, and schedule the dig port work to handle it.
> > + */
> > +void intel_hpd_trigger_irq(struct intel_digital_port *dig_port)
> > +{
> > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > +
> > +	spin_lock_irq(&i915->irq_lock);
> > +	i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
> > +	spin_unlock_irq(&i915->irq_lock);
> > +
> > +	queue_work(i915->hotplug.dp_wq, &i915->hotplug.dig_port_work);
> > +}
> > +
> >  /*
> >   * Handle hotplug events outside the interrupt handler proper.
> >   */
> > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h
> > b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > index 777b0743257e..a704d7c94d16 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > @@ -10,6 +10,7 @@
> >  
> >  struct drm_i915_private;
> >  struct intel_connector;
> > +struct intel_digital_port;
> >  struct intel_encoder;
> >  enum port;
> >  
> > @@ -18,6 +19,7 @@ enum intel_hotplug_state intel_encoder_hotplug(struct
> > intel_encoder *encoder,
> >  					       struct intel_connector
> > *connector);
> >  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> >  			   u32 pin_mask, u32 long_mask);
> > +void intel_hpd_trigger_irq(struct intel_digital_port *dig_port);
> >  void intel_hpd_init(struct drm_i915_private *dev_priv);
> >  void intel_hpd_init_work(struct drm_i915_private *dev_priv);
> >  void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
> > diff --git a/include/drm/drm_dp_mst_helper.h
> > b/include/drm/drm_dp_mst_helper.h
> > index 9e1ffcd7cb68..b230ff6f7081 100644
> > --- a/include/drm/drm_dp_mst_helper.h
> > +++ b/include/drm/drm_dp_mst_helper.h
> > @@ -475,6 +475,15 @@ struct drm_dp_mst_topology_mgr;
> >  struct drm_dp_mst_topology_cbs {
> >  	/* create a connector for a port */
> >  	struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr
> > *mgr, struct drm_dp_mst_port *port, const char *path);
> > +	/*
> > +	 * Checks for any pending MST interrupts, passing them to MST core for
> > +	 * processing, the same way an HPD IRQ pulse handler would do this.
> > +	 * If provided MST core calls this callback from a poll-waiting loop
> > +	 * when waiting for MST down message replies. The driver is expected
> > +	 * to guard against a race between this callback and the driver's HPD
> > +	 * IRQ pulse handler.
> > +	 */
> > +	void (*poll_hpd_irq)(struct drm_dp_mst_topology_mgr *mgr);
> >  };
> >  
> >  #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)
> > -- 
> > 2.23.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses
  2020-06-09 15:58       ` Lyude Paul
@ 2020-06-09 18:03         ` Imre Deak
  0 siblings, 0 replies; 17+ messages in thread
From: Imre Deak @ 2020-06-09 18:03 UTC (permalink / raw)
  To: Lyude Paul; +Cc: Dave Airlie, intel-gfx, dri-devel

On Tue, Jun 09, 2020 at 11:58:18AM -0400, Lyude Paul wrote:
> Hi! Awesome patch series!
> 
> Reviewed-by: Lyude Paul <lyude@redhat.com>

Thanks.

> Also re merging via drm-intel-next-queued - I think that should be fine, fwiw
> merging via drm-misc-next might be another option (I've definitely done this in
> the past for series that touched MST and drivers, but I don't have a hard
> preference either way).

Ok, if no objections I'll merge 2/3 via drm-misc-next, that seems to
make more sense.

Could you also take a look at
https://patchwork.freedesktop.org/series/78100/

I should've CC'd you.

> On Tue, 2020-06-09 at 15:15 +0300, Imre Deak wrote:
> > Hi Dave, Lyude,
> > 
> > are you ok to merge this patchset via the drm-intel-next-queued tree?
> > 
> > --Imre
> > 
> > On Thu, Jun 04, 2020 at 09:45:00PM +0300, Imre Deak wrote:
> > > Some TypeC -> native DP adapters, at least the Club 3D CAC-1557 adapter,
> > > incorrectly filter out HPD short pulses with a duration less than
> > > ~540 usec, leading to MST probe failures.
> > > 
> > > According to the DP Standard 2.0 section 5.1.4:
> > > - DP sinks should generate short pulses in the 500 usec -> 1 msec range
> > > - DP sources should detect short pulses in the 250 usec -> 2 msec range
> > > 
> > > According to the DP Alt Mode on TypeC Standard section 3.9.2, adapters
> > > should detect and forward short pulses according to how sources should
> > > detect them as specified in the DP Standard (250 usec -> 2 msec).
> > > 
> > > Based on the above filtering out short pulses with a duration less than
> > > 540 usec is incorrect.
> > > 
> > > To make such adapters work add support for a driver polling on MST
> > > inerrupt flags, and wire this up in the i915 driver. The sink can clear
> > > an interrupt it raised after 110 msec if the source doesn't respond, so
> > > use a 50 msec poll period to avoid missing an interrupt. Polling of the
> > > MST interrupt flags is explicitly allowed by the DP Standard.
> > > 
> > > This fixes MST probe failures I saw using this adapter and a DELL U2515H
> > > monitor.
> > > 
> > > v2:
> > > - Fix the wait event timeout for the no-poll case.
> > > v3 (Ville):
> > > - Fix the short pulse duration limits in the commit log prescribed by the
> > >   DP Standard.
> > > - Add code comment explaining why/how polling is used.
> > > - Factor out a helper to schedule the port's hpd irq handler and move it
> > >   to the rest of hotplug handlers.
> > > - Document the new MST callback.
> > > - s/update_hpd_irq_state/poll_hpd_irq/
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_mst_topology.c        | 32 ++++++++++++++++++--
> > >  drivers/gpu/drm/i915/display/intel_dp_mst.c  | 10 ++++++
> > >  drivers/gpu/drm/i915/display/intel_hotplug.c | 18 +++++++++++
> > >  drivers/gpu/drm/i915/display/intel_hotplug.h |  2 ++
> > >  include/drm/drm_dp_mst_helper.h              |  9 ++++++
> > >  5 files changed, 68 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index 5bc72e800b85..2a309fb2c4cc 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -1178,11 +1178,37 @@ static int drm_dp_mst_wait_tx_reply(struct
> > > drm_dp_mst_branch *mstb,
> > >  				    struct drm_dp_sideband_msg_tx *txmsg)
> > >  {
> > >  	struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
> > > +	unsigned long wait_timeout = msecs_to_jiffies(4000);
> > > +	unsigned long wait_expires = jiffies + wait_timeout;
> > >  	int ret;
> > >  
> > > -	ret = wait_event_timeout(mgr->tx_waitq,
> > > -				 check_txmsg_state(mgr, txmsg),
> > > -				 (4 * HZ));
> > > +	for (;;) {
> > > +		/*
> > > +		 * If the driver provides a way for this, change to
> > > +		 * poll-waiting for the MST reply interrupt if we didn't receive
> > > +		 * it for 50 msec. This would cater for cases where the HPD
> > > +		 * pulse signal got lost somewhere, even though the sink raised
> > > +		 * the corresponding MST interrupt correctly. One example is the
> > > +		 * Club 3D CAC-1557 TypeC -> DP adapter which for some reason
> > > +		 * filters out short pulses with a duration less than ~540 usec.
> > > +		 *
> > > +		 * The poll period is 50 msec to avoid missing an interrupt
> > > +		 * after the sink has cleared it (after a 110msec timeout
> > > +		 * since it raised the interrupt).
> > > +		 */
> > > +		ret = wait_event_timeout(mgr->tx_waitq,
> > > +					 check_txmsg_state(mgr, txmsg),
> > > +					 mgr->cbs->poll_hpd_irq ?
> > > +						msecs_to_jiffies(50) :
> > > +						wait_timeout);
> > > +
> > > +		if (ret || !mgr->cbs->poll_hpd_irq ||
> > > +		    time_after(jiffies, wait_expires))
> > > +			break;
> > > +
> > > +		mgr->cbs->poll_hpd_irq(mgr);
> > > +	}
> > > +
> > >  	mutex_lock(&mgr->qlock);
> > >  	if (ret > 0) {
> > >  		if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > index d18b406f2a7d..9be52643205d 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > @@ -33,6 +33,7 @@
> > >  #include "intel_connector.h"
> > >  #include "intel_ddi.h"
> > >  #include "intel_display_types.h"
> > > +#include "intel_hotplug.h"
> > >  #include "intel_dp.h"
> > >  #include "intel_dp_mst.h"
> > >  #include "intel_dpio_phy.h"
> > > @@ -765,8 +766,17 @@ static struct drm_connector
> > > *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
> > >  	return NULL;
> > >  }
> > >  
> > > +static void
> > > +intel_dp_mst_poll_hpd_irq(struct drm_dp_mst_topology_mgr *mgr)
> > > +{
> > > +	struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
> > > +
> > > +	intel_hpd_trigger_irq(dp_to_dig_port(intel_dp));
> > > +}
> > > +
> > >  static const struct drm_dp_mst_topology_cbs mst_cbs = {
> > >  	.add_connector = intel_dp_add_mst_connector,
> > > +	.poll_hpd_irq = intel_dp_mst_poll_hpd_irq,
> > >  };
> > >  
> > >  static struct intel_dp_mst_encoder *
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > index 4f6f560e093e..664f88354101 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > @@ -347,6 +347,24 @@ static void i915_digport_work_func(struct work_struct
> > > *work)
> > >  	}
> > >  }
> > >  
> > > +/**
> > > + * intel_hpd_trigger_irq - trigger an hpd irq event for a port
> > > + * @dig_port: digital port
> > > + *
> > > + * Trigger an HPD interrupt event for the given port, emulating a short
> > > pulse
> > > + * generated by the sink, and schedule the dig port work to handle it.
> > > + */
> > > +void intel_hpd_trigger_irq(struct intel_digital_port *dig_port)
> > > +{
> > > +	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
> > > +
> > > +	spin_lock_irq(&i915->irq_lock);
> > > +	i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
> > > +	spin_unlock_irq(&i915->irq_lock);
> > > +
> > > +	queue_work(i915->hotplug.dp_wq, &i915->hotplug.dig_port_work);
> > > +}
> > > +
> > >  /*
> > >   * Handle hotplug events outside the interrupt handler proper.
> > >   */
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h
> > > b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > > index 777b0743257e..a704d7c94d16 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > > @@ -10,6 +10,7 @@
> > >  
> > >  struct drm_i915_private;
> > >  struct intel_connector;
> > > +struct intel_digital_port;
> > >  struct intel_encoder;
> > >  enum port;
> > >  
> > > @@ -18,6 +19,7 @@ enum intel_hotplug_state intel_encoder_hotplug(struct
> > > intel_encoder *encoder,
> > >  					       struct intel_connector
> > > *connector);
> > >  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > >  			   u32 pin_mask, u32 long_mask);
> > > +void intel_hpd_trigger_irq(struct intel_digital_port *dig_port);
> > >  void intel_hpd_init(struct drm_i915_private *dev_priv);
> > >  void intel_hpd_init_work(struct drm_i915_private *dev_priv);
> > >  void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
> > > diff --git a/include/drm/drm_dp_mst_helper.h
> > > b/include/drm/drm_dp_mst_helper.h
> > > index 9e1ffcd7cb68..b230ff6f7081 100644
> > > --- a/include/drm/drm_dp_mst_helper.h
> > > +++ b/include/drm/drm_dp_mst_helper.h
> > > @@ -475,6 +475,15 @@ struct drm_dp_mst_topology_mgr;
> > >  struct drm_dp_mst_topology_cbs {
> > >  	/* create a connector for a port */
> > >  	struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr
> > > *mgr, struct drm_dp_mst_port *port, const char *path);
> > > +	/*
> > > +	 * Checks for any pending MST interrupts, passing them to MST core for
> > > +	 * processing, the same way an HPD IRQ pulse handler would do this.
> > > +	 * If provided MST core calls this callback from a poll-waiting loop
> > > +	 * when waiting for MST down message replies. The driver is expected
> > > +	 * to guard against a race between this callback and the driver's HPD
> > > +	 * IRQ pulse handler.
> > > +	 */
> > > +	void (*poll_hpd_irq)(struct drm_dp_mst_topology_mgr *mgr);
> > >  };
> > >  
> > >  #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)
> > > -- 
> > > 2.23.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: ✓ Fi.CI.IGT: success for series starting with [RESEND,v3,1/3] drm/i915/dp_mst: Fix disabling MST on a port (rev6)
       [not found] ` <159136523293.18507.17008252253062518394@emeril.freedesktop.org>
@ 2020-06-11 12:47   ` Imre Deak
  0 siblings, 0 replies; 17+ messages in thread
From: Imre Deak @ 2020-06-11 12:47 UTC (permalink / raw)
  To: Jose Souza, Ville Syrjälä, Lyude Paul; +Cc: intel-gfx, dri-devel

On Fri, Jun 05, 2020 at 01:53:52PM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [RESEND,v3,1/3] drm/i915/dp_mst: Fix disabling MST on a port (rev6)
> URL   : https://patchwork.freedesktop.org/series/77969/
> State : success

Thanks for the reviews, pushed patch 1 to -dinq and patches 2,3 to
drm-misc-next.

> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_8590_full -> Patchwork_17882_full
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.
> 
>   
> 
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_17882_full that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_exec_whisper@basic-forked-all:
>     - shard-glk:          [PASS][1] -> [DMESG-WARN][2] ([i915#118] / [i915#95])
>    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-glk7/igt@gem_exec_whisper@basic-forked-all.html
>    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-glk2/igt@gem_exec_whisper@basic-forked-all.html
> 
>   * igt@gem_mmap_gtt@cpuset-big-copy-odd:
>     - shard-iclb:         [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
>    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-iclb6/igt@gem_mmap_gtt@cpuset-big-copy-odd.html
>    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-iclb2/igt@gem_mmap_gtt@cpuset-big-copy-odd.html
> 
>   * igt@gem_workarounds@suspend-resume:
>     - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([i915#180])
>    [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-apl2/igt@gem_workarounds@suspend-resume.html
>    [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-apl1/igt@gem_workarounds@suspend-resume.html
> 
>   * igt@kms_big_fb@linear-32bpp-rotate-180:
>     - shard-skl:          [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +9 similar issues
>    [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-skl6/igt@kms_big_fb@linear-32bpp-rotate-180.html
>    [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-skl5/igt@kms_big_fb@linear-32bpp-rotate-180.html
> 
>   * igt@kms_big_fb@linear-64bpp-rotate-180:
>     - shard-glk:          [PASS][9] -> [DMESG-FAIL][10] ([i915#118] / [i915#95]) +1 similar issue
>    [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-glk2/igt@kms_big_fb@linear-64bpp-rotate-180.html
>    [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-glk8/igt@kms_big_fb@linear-64bpp-rotate-180.html
> 
>   * igt@kms_big_fb@x-tiled-16bpp-rotate-0:
>     - shard-glk:          [PASS][11] -> [DMESG-WARN][12] ([i915#1982])
>    [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-glk5/igt@kms_big_fb@x-tiled-16bpp-rotate-0.html
>    [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-glk4/igt@kms_big_fb@x-tiled-16bpp-rotate-0.html
> 
>   * igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen:
>     - shard-apl:          [PASS][13] -> [DMESG-WARN][14] ([i915#95]) +17 similar issues
>    [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-apl4/igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen.html
>    [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-apl8/igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen.html
> 
>   * igt@kms_cursor_crc@pipe-c-cursor-suspend:
>     - shard-kbl:          [PASS][15] -> [DMESG-WARN][16] ([i915#180]) +4 similar issues
>    [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl3/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
>    [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
> 
>   * igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
>     - shard-snb:          [PASS][17] -> [TIMEOUT][18] ([i915#1958])
>    [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-snb6/igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled.html
>    [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-snb1/igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled.html
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
>     - shard-kbl:          [PASS][19] -> [DMESG-WARN][20] ([i915#93] / [i915#95])
>    [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move.html
>    [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move.html
> 
>   * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-cpu:
>     - shard-skl:          [PASS][21] -> [FAIL][22] ([i915#49])
>    [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-skl9/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-cpu.html
>    [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-skl9/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-cpu.html
> 
>   * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
>     - shard-skl:          [PASS][23] -> [FAIL][24] ([fdo#108145] / [i915#265]) +1 similar issue
>    [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
>    [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
> 
>   * igt@kms_plane_lowres@pipe-a-tiling-x:
>     - shard-snb:          [PASS][25] -> [SKIP][26] ([fdo#109271]) +1 similar issue
>    [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-snb2/igt@kms_plane_lowres@pipe-a-tiling-x.html
>    [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-snb1/igt@kms_plane_lowres@pipe-a-tiling-x.html
> 
>   * igt@kms_psr@psr2_cursor_blt:
>     - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109441]) +3 similar issues
>    [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html
>    [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-iclb4/igt@kms_psr@psr2_cursor_blt.html
> 
>   * igt@kms_universal_plane@universal-plane-gen9-features-pipe-c:
>     - shard-kbl:          [PASS][29] -> [DMESG-WARN][30] ([i915#1982])
>    [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl3/igt@kms_universal_plane@universal-plane-gen9-features-pipe-c.html
>    [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl4/igt@kms_universal_plane@universal-plane-gen9-features-pipe-c.html
> 
>   * igt@syncobj_wait@invalid-multi-wait-unsubmitted-submitted:
>     - shard-tglb:         [PASS][31] -> [DMESG-WARN][32] ([i915#402])
>    [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-tglb6/igt@syncobj_wait@invalid-multi-wait-unsubmitted-submitted.html
>    [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-tglb7/igt@syncobj_wait@invalid-multi-wait-unsubmitted-submitted.html
> 
>   
> #### Possible fixes ####
> 
>   * {igt@gem_ctx_isolation@preservation-s3@rcs0}:
>     - shard-apl:          [DMESG-WARN][33] ([i915#180]) -> [PASS][34] +2 similar issues
>    [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-apl2/igt@gem_ctx_isolation@preservation-s3@rcs0.html
>    [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-apl1/igt@gem_ctx_isolation@preservation-s3@rcs0.html
> 
>   * igt@gem_exec_whisper@basic-queues-forked-all:
>     - shard-glk:          [DMESG-WARN][35] ([i915#118] / [i915#95]) -> [PASS][36] +1 similar issue
>    [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-glk9/igt@gem_exec_whisper@basic-queues-forked-all.html
>    [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-glk9/igt@gem_exec_whisper@basic-queues-forked-all.html
> 
>   * igt@gen9_exec_parse@allowed-all:
>     - shard-kbl:          [DMESG-WARN][37] ([i915#1436] / [i915#716]) -> [PASS][38]
>    [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl1/igt@gen9_exec_parse@allowed-all.html
>    [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl3/igt@gen9_exec_parse@allowed-all.html
> 
>   * igt@i915_suspend@debugfs-reader:
>     - shard-kbl:          [INCOMPLETE][39] ([i915#155]) -> [PASS][40]
>    [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl2/igt@i915_suspend@debugfs-reader.html
>    [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl6/igt@i915_suspend@debugfs-reader.html
> 
>   * igt@kms_color@pipe-c-ctm-red-to-blue:
>     - shard-kbl:          [DMESG-WARN][41] ([i915#93] / [i915#95]) -> [PASS][42]
>    [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl7/igt@kms_color@pipe-c-ctm-red-to-blue.html
>    [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl7/igt@kms_color@pipe-c-ctm-red-to-blue.html
> 
>   * igt@kms_color@pipe-d-ctm-0-5:
>     - shard-tglb:         [DMESG-WARN][43] ([i915#1149] / [i915#402]) -> [PASS][44]
>    [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-tglb2/igt@kms_color@pipe-d-ctm-0-5.html
>    [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-tglb1/igt@kms_color@pipe-d-ctm-0-5.html
> 
>   * igt@kms_cursor_legacy@all-pipes-torture-move:
>     - shard-skl:          [DMESG-WARN][45] ([i915#128]) -> [PASS][46]
>    [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-skl10/igt@kms_cursor_legacy@all-pipes-torture-move.html
>    [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-skl3/igt@kms_cursor_legacy@all-pipes-torture-move.html
> 
>   * {igt@kms_flip@2x-flip-vs-suspend@ab-hdmi-a1-hdmi-a2}:
>     - shard-glk:          [DMESG-WARN][47] ([i915#1982]) -> [PASS][48]
>    [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-glk6/igt@kms_flip@2x-flip-vs-suspend@ab-hdmi-a1-hdmi-a2.html
>    [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-glk8/igt@kms_flip@2x-flip-vs-suspend@ab-hdmi-a1-hdmi-a2.html
> 
>   * igt@kms_flip_tiling@flip-x-tiled:
>     - shard-apl:          [DMESG-WARN][49] ([i915#95]) -> [PASS][50] +26 similar issues
>    [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-apl7/igt@kms_flip_tiling@flip-x-tiled.html
>    [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-apl3/igt@kms_flip_tiling@flip-x-tiled.html
> 
>   * igt@kms_hdr@bpc-switch-suspend:
>     - shard-skl:          [FAIL][51] ([i915#1188]) -> [PASS][52]
>    [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-skl10/igt@kms_hdr@bpc-switch-suspend.html
>    [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-skl3/igt@kms_hdr@bpc-switch-suspend.html
> 
>   * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
>     - shard-kbl:          [DMESG-WARN][53] ([i915#180]) -> [PASS][54] +3 similar issues
>    [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
>    [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl3/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
> 
>   * igt@kms_plane@plane-panning-bottom-right-pipe-c-planes:
>     - shard-skl:          [DMESG-WARN][55] ([i915#1982]) -> [PASS][56] +2 similar issues
>    [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-skl6/igt@kms_plane@plane-panning-bottom-right-pipe-c-planes.html
>    [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-skl5/igt@kms_plane@plane-panning-bottom-right-pipe-c-planes.html
> 
>   * igt@kms_psr@psr2_primary_mmap_cpu:
>     - shard-iclb:         [SKIP][57] ([fdo#109441]) -> [PASS][58] +2 similar issues
>    [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-iclb6/igt@kms_psr@psr2_primary_mmap_cpu.html
>    [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
> 
>   * igt@kms_setmode@basic:
>     - shard-apl:          [FAIL][59] ([i915#31]) -> [PASS][60]
>    [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-apl6/igt@kms_setmode@basic.html
>    [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-apl3/igt@kms_setmode@basic.html
>     - shard-kbl:          [FAIL][61] ([i915#31]) -> [PASS][62]
>    [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl6/igt@kms_setmode@basic.html
>    [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl3/igt@kms_setmode@basic.html
> 
>   * {igt@perf@polling-parameterized}:
>     - shard-hsw:          [FAIL][63] ([i915#1542]) -> [PASS][64]
>    [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-hsw6/igt@perf@polling-parameterized.html
>    [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-hsw5/igt@perf@polling-parameterized.html
> 
>   
> #### Warnings ####
> 
>   * igt@i915_pm_dc@dc3co-vpb-simulation:
>     - shard-snb:          [SKIP][65] ([fdo#109271]) -> [INCOMPLETE][66] ([i915#82]) +1 similar issue
>    [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-snb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
>    [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-snb5/igt@i915_pm_dc@dc3co-vpb-simulation.html
>     - shard-iclb:         [SKIP][67] ([i915#658]) -> [SKIP][68] ([i915#588])
>    [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-iclb6/igt@i915_pm_dc@dc3co-vpb-simulation.html
>    [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
> 
>   * igt@kms_content_protection@atomic:
>     - shard-apl:          [FAIL][69] ([fdo#110321] / [fdo#110336]) -> [TIMEOUT][70] ([i915#1319] / [i915#1635])
>    [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-apl6/igt@kms_content_protection@atomic.html
>    [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-apl3/igt@kms_content_protection@atomic.html
> 
>   * igt@kms_content_protection@legacy:
>     - shard-kbl:          [DMESG-FAIL][71] ([fdo#110321]) -> [TIMEOUT][72] ([i915#1319] / [i915#1958])
>    [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl3/igt@kms_content_protection@legacy.html
>    [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl4/igt@kms_content_protection@legacy.html
> 
>   * igt@kms_content_protection@lic:
>     - shard-apl:          [DMESG-FAIL][73] ([fdo#110321] / [i915#95]) -> [TIMEOUT][74] ([i915#1319] / [i915#1635]) +1 similar issue
>    [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-apl2/igt@kms_content_protection@lic.html
>    [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-apl1/igt@kms_content_protection@lic.html
> 
>   * igt@kms_content_protection@srm:
>     - shard-kbl:          [DMESG-FAIL][75] ([fdo#110321] / [i915#95]) -> [TIMEOUT][76] ([i915#1319])
>    [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl2/igt@kms_content_protection@srm.html
>    [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl3/igt@kms_content_protection@srm.html
> 
>   * igt@kms_frontbuffer_tracking@fbc-suspend:
>     - shard-kbl:          [DMESG-WARN][77] ([i915#180] / [i915#93] / [i915#95]) -> [DMESG-WARN][78] ([i915#93] / [i915#95])
>    [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
>    [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
>     - shard-apl:          [DMESG-WARN][79] ([i915#180] / [i915#95]) -> [DMESG-WARN][80] ([i915#95])
>    [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-apl4/igt@kms_frontbuffer_tracking@fbc-suspend.html
>    [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-apl8/igt@kms_frontbuffer_tracking@fbc-suspend.html
> 
>   * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
>     - shard-skl:          [DMESG-WARN][81] ([i915#1982]) -> [DMESG-FAIL][82] ([fdo#108145] / [i915#1982])
>    [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
>    [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
> 
>   * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
>     - shard-snb:          [SKIP][83] ([fdo#109271]) -> [TIMEOUT][84] ([i915#1958]) +1 similar issue
>    [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8590/shard-snb6/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html
>    [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/shard-snb1/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html
> 
>   
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
>   [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
>   [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
>   [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
>   [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
>   [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
>   [i915#1149]: https://gitlab.freedesktop.org/drm/intel/issues/1149
>   [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
>   [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
>   [i915#128]: https://gitlab.freedesktop.org/drm/intel/issues/128
>   [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
>   [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
>   [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
>   [i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
>   [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
>   [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
>   [i915#1928]: https://gitlab.freedesktop.org/drm/intel/issues/1928
>   [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
>   [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
>   [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
>   [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
>   [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
>   [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
>   [i915#46]: https://gitlab.freedesktop.org/drm/intel/issues/46
>   [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
>   [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
>   [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
>   [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
>   [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
>   [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
>   [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
>   [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
> 
> 
> Participating hosts (11 -> 11)
> ------------------------------
> 
>   No changes in participating hosts
> 
> 
> Build changes
> -------------
> 
>   * Linux: CI_DRM_8590 -> Patchwork_17882
> 
>   CI-20190529: 20190529
>   CI_DRM_8590: 91c6f0274b54c89679cd23f6fc65e9fe5922971f @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_5695: 53e8c878a6fb5708e63c99403691e8960b86ea9c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_17882: 23fe5e3ae83585e3d4ad9ecdfea368dd42ff6dfb @ git://anongit.freedesktop.org/gfx-ci/linux
>   piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17882/index.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2020-06-11 12:47 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 21:10 [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Imre Deak
2020-06-03 21:10 ` [PATCH 2/3] drm/dp_mst: Sanitize mgr->qlock locking in drm_dp_mst_wait_tx_reply() Imre Deak
2020-06-03 21:27   ` [Intel-gfx] " Souza, Jose
2020-06-03 21:10 ` [PATCH 3/3] drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses Imre Deak
2020-06-03 22:18   ` [PATCH v2 " Imre Deak
2020-06-04 15:12     ` [Intel-gfx] " Ville Syrjälä
2020-06-04 15:41       ` Imre Deak
2020-06-04 18:45   ` [PATCH v3 " Imre Deak
2020-06-04 18:54     ` Ville Syrjälä
2020-06-09 12:15     ` [Intel-gfx] " Imre Deak
2020-06-09 15:58       ` Lyude Paul
2020-06-09 18:03         ` Imre Deak
2020-06-03 21:27 ` [Intel-gfx] [PATCH 1/3] drm/i915/dp_mst: Fix disabling MST on a port Souza, Jose
2020-06-04 14:55 ` Ville Syrjälä
2020-06-04 15:09   ` Imre Deak
2020-06-04 18:44 ` [PATCH v2 " Imre Deak
     [not found] ` <159136523293.18507.17008252253062518394@emeril.freedesktop.org>
2020-06-11 12:47   ` ✓ Fi.CI.IGT: success for series starting with [RESEND,v3,1/3] drm/i915/dp_mst: Fix disabling MST on a port (rev6) 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).