Skip to content

Commit 471bdd0

Browse files
committed
drm/i915/dp_mst: Work around out-of-spec adapters filtering short pulses
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> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Lyude Paul <lyude@redhat.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200604184500.23730-2-imre.deak@intel.com
1 parent 8b9f343 commit 471bdd0

File tree

5 files changed

+68
-3
lines changed

5 files changed

+68
-3
lines changed

drivers/gpu/drm/drm_dp_mst_topology.c

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,11 +1178,37 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
11781178
struct drm_dp_sideband_msg_tx *txmsg)
11791179
{
11801180
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
1181+
unsigned long wait_timeout = msecs_to_jiffies(4000);
1182+
unsigned long wait_expires = jiffies + wait_timeout;
11811183
int ret;
11821184

1183-
ret = wait_event_timeout(mgr->tx_waitq,
1184-
check_txmsg_state(mgr, txmsg),
1185-
(4 * HZ));
1185+
for (;;) {
1186+
/*
1187+
* If the driver provides a way for this, change to
1188+
* poll-waiting for the MST reply interrupt if we didn't receive
1189+
* it for 50 msec. This would cater for cases where the HPD
1190+
* pulse signal got lost somewhere, even though the sink raised
1191+
* the corresponding MST interrupt correctly. One example is the
1192+
* Club 3D CAC-1557 TypeC -> DP adapter which for some reason
1193+
* filters out short pulses with a duration less than ~540 usec.
1194+
*
1195+
* The poll period is 50 msec to avoid missing an interrupt
1196+
* after the sink has cleared it (after a 110msec timeout
1197+
* since it raised the interrupt).
1198+
*/
1199+
ret = wait_event_timeout(mgr->tx_waitq,
1200+
check_txmsg_state(mgr, txmsg),
1201+
mgr->cbs->poll_hpd_irq ?
1202+
msecs_to_jiffies(50) :
1203+
wait_timeout);
1204+
1205+
if (ret || !mgr->cbs->poll_hpd_irq ||
1206+
time_after(jiffies, wait_expires))
1207+
break;
1208+
1209+
mgr->cbs->poll_hpd_irq(mgr);
1210+
}
1211+
11861212
mutex_lock(&mgr->qlock);
11871213
if (ret > 0) {
11881214
if (txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT) {

drivers/gpu/drm/i915/display/intel_dp_mst.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "intel_connector.h"
3434
#include "intel_ddi.h"
3535
#include "intel_display_types.h"
36+
#include "intel_hotplug.h"
3637
#include "intel_dp.h"
3738
#include "intel_dp_mst.h"
3839
#include "intel_dpio_phy.h"
@@ -747,8 +748,17 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
747748
return NULL;
748749
}
749750

751+
static void
752+
intel_dp_mst_poll_hpd_irq(struct drm_dp_mst_topology_mgr *mgr)
753+
{
754+
struct intel_dp *intel_dp = container_of(mgr, struct intel_dp, mst_mgr);
755+
756+
intel_hpd_trigger_irq(dp_to_dig_port(intel_dp));
757+
}
758+
750759
static const struct drm_dp_mst_topology_cbs mst_cbs = {
751760
.add_connector = intel_dp_add_mst_connector,
761+
.poll_hpd_irq = intel_dp_mst_poll_hpd_irq,
752762
};
753763

754764
static struct intel_dp_mst_encoder *

drivers/gpu/drm/i915/display/intel_hotplug.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,24 @@ static void i915_digport_work_func(struct work_struct *work)
348348
}
349349
}
350350

351+
/**
352+
* intel_hpd_trigger_irq - trigger an hpd irq event for a port
353+
* @dig_port: digital port
354+
*
355+
* Trigger an HPD interrupt event for the given port, emulating a short pulse
356+
* generated by the sink, and schedule the dig port work to handle it.
357+
*/
358+
void intel_hpd_trigger_irq(struct intel_digital_port *dig_port)
359+
{
360+
struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
361+
362+
spin_lock_irq(&i915->irq_lock);
363+
i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
364+
spin_unlock_irq(&i915->irq_lock);
365+
366+
queue_work(i915->hotplug.dp_wq, &i915->hotplug.dig_port_work);
367+
}
368+
351369
/*
352370
* Handle hotplug events outside the interrupt handler proper.
353371
*/

drivers/gpu/drm/i915/display/intel_hotplug.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
struct drm_i915_private;
1212
struct intel_connector;
13+
struct intel_digital_port;
1314
struct intel_encoder;
1415
enum port;
1516

@@ -19,6 +20,7 @@ enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
1920
bool irq_received);
2021
void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
2122
u32 pin_mask, u32 long_mask);
23+
void intel_hpd_trigger_irq(struct intel_digital_port *dig_port);
2224
void intel_hpd_init(struct drm_i915_private *dev_priv);
2325
void intel_hpd_init_work(struct drm_i915_private *dev_priv);
2426
void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);

include/drm/drm_dp_mst_helper.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,15 @@ struct drm_dp_mst_topology_mgr;
475475
struct drm_dp_mst_topology_cbs {
476476
/* create a connector for a port */
477477
struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path);
478+
/*
479+
* Checks for any pending MST interrupts, passing them to MST core for
480+
* processing, the same way an HPD IRQ pulse handler would do this.
481+
* If provided MST core calls this callback from a poll-waiting loop
482+
* when waiting for MST down message replies. The driver is expected
483+
* to guard against a race between this callback and the driver's HPD
484+
* IRQ pulse handler.
485+
*/
486+
void (*poll_hpd_irq)(struct drm_dp_mst_topology_mgr *mgr);
478487
};
479488

480489
#define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8)

0 commit comments

Comments
 (0)