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