Skip to content

Commit 6f85f73

Browse files
committed
drm/dp_mst: Add basic topology reprobing when resuming
Finally! For a very long time, our MST helpers have had one very annoying issue: They don't know how to reprobe the topology state when coming out of suspend. This means that if a user has a machine connected to an MST topology and decides to suspend their machine, we lose all topology changes that happened during that period. That can be a big problem if the machine was connected to a different topology on the same port before resuming, as we won't bother reprobing any of the ports and likely cause the user's monitors not to come back up as expected. So, we start fixing this by teaching our MST helpers how to reprobe the link addresses of each connected topology when resuming. As it turns out, the behavior that we want here is identical to the behavior we want when initially probing a newly connected MST topology, with a couple of important differences: - We need to be more careful about handling the potential races between events from the MST hub that could change the topology state as we're performing the link address reprobe - We need to be more careful about handling unlikely state changes on ports - such as an input port turning into an output port, something that would be far more likely to happen in situations like the MST hub we're connected to being changed while we're suspend Both of which have been solved by previous commits. That leaves one requirement: - We need to prune any MST ports in our in-memory topology state that were present when suspending, but have not appeared in the post-resume link address response from their parent branch device Which we can now handle in this commit by modifying drm_dp_send_link_address(). We then introduce suspend/resume reprobing by introducing drm_dp_mst_topology_mgr_invalidate_mstb(), which we call in drm_dp_mst_topology_mgr_suspend() to traverse the in-memory topology state to indicate that each mstb needs it's link address resent and PBN resources reprobed. On resume, we start back up &mgr->work and have it reprobe the topology in the same way we would on a hotplug, removing any leftover ports that no longer appear in the topology state. Changes since v4: * Split indenting changes in drm_dp_mst_topology_mgr_resume() into a separate patch * Only fire hotplugs when something has actually changed after a link address probe * Don't try to change port->connector at all on ports, just throw out ports that need their connectors removed to make things easier. Cc: Juston Li <[email protected]> Cc: Imre Deak <[email protected]> Cc: Ville Syrjälä <[email protected]> Cc: Harry Wentland <[email protected]> Cc: Daniel Vetter <[email protected]> Reviewed-by: Sean Paul <[email protected]> Signed-off-by: Lyude Paul <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent d20ebea commit 6f85f73

File tree

5 files changed

+156
-40
lines changed

5 files changed

+156
-40
lines changed

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,7 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend)
974974
if (suspend) {
975975
drm_dp_mst_topology_mgr_suspend(mgr);
976976
} else {
977-
ret = drm_dp_mst_topology_mgr_resume(mgr);
977+
ret = drm_dp_mst_topology_mgr_resume(mgr, true);
978978
if (ret < 0) {
979979
drm_dp_mst_topology_mgr_set_mst(mgr, false);
980980
need_hotplug = true;

drivers/gpu/drm/drm_dp_mst_topology.c

Lines changed: 148 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
6767
struct drm_dp_mst_port *port,
6868
int offset, int size, u8 *bytes);
6969

70-
static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
71-
struct drm_dp_mst_branch *mstb);
70+
static int drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
71+
struct drm_dp_mst_branch *mstb);
7272
static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
7373
struct drm_dp_mst_branch *mstb,
7474
struct drm_dp_mst_port *port);
@@ -1977,7 +1977,7 @@ drm_dp_mst_add_port(struct drm_device *dev,
19771977
return port;
19781978
}
19791979

1980-
static void
1980+
static int
19811981
drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
19821982
struct drm_device *dev,
19831983
struct drm_dp_link_addr_reply_port *port_msg)
@@ -1986,33 +1986,45 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
19861986
struct drm_dp_mst_port *port;
19871987
int old_ddps = 0, ret;
19881988
u8 new_pdt = DP_PEER_DEVICE_NONE;
1989-
bool created = false, send_link_addr = false;
1989+
bool created = false, send_link_addr = false, changed = false;
19901990

19911991
port = drm_dp_get_port(mstb, port_msg->port_number);
19921992
if (!port) {
19931993
port = drm_dp_mst_add_port(dev, mgr, mstb,
19941994
port_msg->port_number);
19951995
if (!port)
1996-
return;
1996+
return -ENOMEM;
19971997
created = true;
1998-
} else if (port_msg->input_port && !port->input && port->connector) {
1999-
/* Destroying the connector is impossible in this context, so
2000-
* replace the port with a new one
1998+
changed = true;
1999+
} else if (!port->input && port_msg->input_port && port->connector) {
2000+
/* Since port->connector can't be changed here, we create a
2001+
* new port if input_port changes from 0 to 1
20012002
*/
20022003
drm_dp_mst_topology_unlink_port(mgr, port);
20032004
drm_dp_mst_topology_put_port(port);
2004-
20052005
port = drm_dp_mst_add_port(dev, mgr, mstb,
20062006
port_msg->port_number);
20072007
if (!port)
2008-
return;
2008+
return -ENOMEM;
2009+
changed = true;
20092010
created = true;
2010-
} else {
2011-
/* Locking is only needed when the port has a connector
2012-
* exposed to userspace
2011+
} else if (port->input && !port_msg->input_port) {
2012+
changed = true;
2013+
} else if (port->connector) {
2014+
/* We're updating a port that's exposed to userspace, so do it
2015+
* under lock
20132016
*/
20142017
drm_modeset_lock(&mgr->base.lock, NULL);
2018+
20152019
old_ddps = port->ddps;
2020+
changed = port->ddps != port_msg->ddps ||
2021+
(port->ddps &&
2022+
(port->ldps != port_msg->legacy_device_plug_status ||
2023+
port->dpcd_rev != port_msg->dpcd_revision ||
2024+
port->mcs != port_msg->mcs ||
2025+
port->pdt != port_msg->peer_device_type ||
2026+
port->num_sdp_stream_sinks !=
2027+
port_msg->num_sdp_stream_sinks));
20162028
}
20172029

20182030
port->input = port_msg->input_port;
@@ -2054,23 +2066,38 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb,
20542066
goto fail;
20552067
}
20562068

2057-
if (!created)
2069+
/*
2070+
* If this port wasn't just created, then we're reprobing because
2071+
* we're coming out of suspend. In this case, always resend the link
2072+
* address if there's an MSTB on this port
2073+
*/
2074+
if (!created && port->pdt == DP_PEER_DEVICE_MST_BRANCHING)
2075+
send_link_addr = true;
2076+
2077+
if (port->connector)
20582078
drm_modeset_unlock(&mgr->base.lock);
2059-
else if (!port->connector && !port->input)
2079+
else if (!port->input)
20602080
drm_dp_mst_port_add_connector(mstb, port);
20612081

2062-
if (send_link_addr && port->mstb)
2063-
drm_dp_send_link_address(mgr, port->mstb);
2082+
if (send_link_addr && port->mstb) {
2083+
ret = drm_dp_send_link_address(mgr, port->mstb);
2084+
if (ret == 1) /* MSTB below us changed */
2085+
changed = true;
2086+
else if (ret < 0)
2087+
goto fail_put;
2088+
}
20642089

20652090
/* put reference to this port */
20662091
drm_dp_mst_topology_put_port(port);
2067-
return;
2092+
return changed;
20682093

20692094
fail:
20702095
drm_dp_mst_topology_unlink_port(mgr, port);
2071-
drm_dp_mst_topology_put_port(port);
2072-
if (!created)
2096+
if (port->connector)
20732097
drm_modeset_unlock(&mgr->base.lock);
2098+
fail_put:
2099+
drm_dp_mst_topology_put_port(port);
2100+
return ret;
20742101
}
20752102

20762103
static void
@@ -2228,13 +2255,20 @@ drm_dp_get_mst_branch_device_by_guid(struct drm_dp_mst_topology_mgr *mgr,
22282255
return mstb;
22292256
}
22302257

2231-
static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
2258+
static int drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
22322259
struct drm_dp_mst_branch *mstb)
22332260
{
22342261
struct drm_dp_mst_port *port;
2262+
int ret;
2263+
bool changed = false;
22352264

2236-
if (!mstb->link_address_sent)
2237-
drm_dp_send_link_address(mgr, mstb);
2265+
if (!mstb->link_address_sent) {
2266+
ret = drm_dp_send_link_address(mgr, mstb);
2267+
if (ret == 1)
2268+
changed = true;
2269+
else if (ret < 0)
2270+
return ret;
2271+
}
22382272

22392273
list_for_each_entry(port, &mstb->ports, next) {
22402274
struct drm_dp_mst_branch *mstb_child = NULL;
@@ -2246,17 +2280,25 @@ static void drm_dp_check_and_send_link_address(struct drm_dp_mst_topology_mgr *m
22462280
drm_modeset_lock(&mgr->base.lock, NULL);
22472281
drm_dp_send_enum_path_resources(mgr, mstb, port);
22482282
drm_modeset_unlock(&mgr->base.lock);
2283+
changed = true;
22492284
}
22502285

22512286
if (port->mstb)
22522287
mstb_child = drm_dp_mst_topology_get_mstb_validated(
22532288
mgr, port->mstb);
22542289

22552290
if (mstb_child) {
2256-
drm_dp_check_and_send_link_address(mgr, mstb_child);
2291+
ret = drm_dp_check_and_send_link_address(mgr,
2292+
mstb_child);
22572293
drm_dp_mst_topology_put_mstb(mstb_child);
2294+
if (ret == 1)
2295+
changed = true;
2296+
else if (ret < 0)
2297+
return ret;
22582298
}
22592299
}
2300+
2301+
return changed;
22602302
}
22612303

22622304
static void drm_dp_mst_link_probe_work(struct work_struct *work)
@@ -2282,11 +2324,12 @@ static void drm_dp_mst_link_probe_work(struct work_struct *work)
22822324
return;
22832325
}
22842326

2285-
drm_dp_check_and_send_link_address(mgr, mstb);
2327+
ret = drm_dp_check_and_send_link_address(mgr, mstb);
22862328
drm_dp_mst_topology_put_mstb(mstb);
22872329

22882330
mutex_unlock(&mgr->probe_lock);
2289-
drm_kms_helper_hotplug_event(dev);
2331+
if (ret)
2332+
drm_kms_helper_hotplug_event(dev);
22902333
}
22912334

22922335
static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
@@ -2532,16 +2575,18 @@ drm_dp_dump_link_address(struct drm_dp_link_address_ack_reply *reply)
25322575
}
25332576
}
25342577

2535-
static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
2578+
static int drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
25362579
struct drm_dp_mst_branch *mstb)
25372580
{
25382581
struct drm_dp_sideband_msg_tx *txmsg;
25392582
struct drm_dp_link_address_ack_reply *reply;
2540-
int i, len, ret;
2583+
struct drm_dp_mst_port *port, *tmp;
2584+
int i, len, ret, port_mask = 0;
2585+
bool changed = false;
25412586

25422587
txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
25432588
if (!txmsg)
2544-
return;
2589+
return -ENOMEM;
25452590

25462591
txmsg->dst = mstb;
25472592
len = build_link_address(txmsg);
@@ -2567,14 +2612,39 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
25672612

25682613
drm_dp_check_mstb_guid(mstb, reply->guid);
25692614

2570-
for (i = 0; i < reply->nports; i++)
2571-
drm_dp_mst_handle_link_address_port(mstb, mgr->dev,
2572-
&reply->ports[i]);
2615+
for (i = 0; i < reply->nports; i++) {
2616+
port_mask |= BIT(reply->ports[i].port_number);
2617+
ret = drm_dp_mst_handle_link_address_port(mstb, mgr->dev,
2618+
&reply->ports[i]);
2619+
if (ret == 1)
2620+
changed = true;
2621+
else if (ret < 0)
2622+
goto out;
2623+
}
2624+
2625+
/* Prune any ports that are currently a part of mstb in our in-memory
2626+
* topology, but were not seen in this link address. Usually this
2627+
* means that they were removed while the topology was out of sync,
2628+
* e.g. during suspend/resume
2629+
*/
2630+
mutex_lock(&mgr->lock);
2631+
list_for_each_entry_safe(port, tmp, &mstb->ports, next) {
2632+
if (port_mask & BIT(port->port_num))
2633+
continue;
2634+
2635+
DRM_DEBUG_KMS("port %d was not in link address, removing\n",
2636+
port->port_num);
2637+
list_del(&port->next);
2638+
drm_dp_mst_topology_put_port(port);
2639+
changed = true;
2640+
}
2641+
mutex_unlock(&mgr->lock);
25732642

25742643
out:
25752644
if (ret <= 0)
25762645
mstb->link_address_sent = false;
25772646
kfree(txmsg);
2647+
return ret < 0 ? ret : changed;
25782648
}
25792649

25802650
static int
@@ -3179,6 +3249,23 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
31793249
}
31803250
EXPORT_SYMBOL(drm_dp_mst_topology_mgr_set_mst);
31813251

3252+
static void
3253+
drm_dp_mst_topology_mgr_invalidate_mstb(struct drm_dp_mst_branch *mstb)
3254+
{
3255+
struct drm_dp_mst_port *port;
3256+
3257+
/* The link address will need to be re-sent on resume */
3258+
mstb->link_address_sent = false;
3259+
3260+
list_for_each_entry(port, &mstb->ports, next) {
3261+
/* The PBN for each port will also need to be re-probed */
3262+
port->available_pbn = 0;
3263+
3264+
if (port->mstb)
3265+
drm_dp_mst_topology_mgr_invalidate_mstb(port->mstb);
3266+
}
3267+
}
3268+
31823269
/**
31833270
* drm_dp_mst_topology_mgr_suspend() - suspend the MST manager
31843271
* @mgr: manager to suspend
@@ -3195,20 +3282,36 @@ void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr)
31953282
flush_work(&mgr->up_req_work);
31963283
flush_work(&mgr->work);
31973284
flush_work(&mgr->delayed_destroy_work);
3285+
3286+
mutex_lock(&mgr->lock);
3287+
if (mgr->mst_state && mgr->mst_primary)
3288+
drm_dp_mst_topology_mgr_invalidate_mstb(mgr->mst_primary);
3289+
mutex_unlock(&mgr->lock);
31983290
}
31993291
EXPORT_SYMBOL(drm_dp_mst_topology_mgr_suspend);
32003292

32013293
/**
32023294
* drm_dp_mst_topology_mgr_resume() - resume the MST manager
32033295
* @mgr: manager to resume
3296+
* @sync: whether or not to perform topology reprobing synchronously
32043297
*
32053298
* This will fetch DPCD and see if the device is still there,
32063299
* if it is, it will rewrite the MSTM control bits, and return.
32073300
*
3208-
* if the device fails this returns -1, and the driver should do
3301+
* If the device fails this returns -1, and the driver should do
32093302
* a full MST reprobe, in case we were undocked.
3303+
*
3304+
* During system resume (where it is assumed that the driver will be calling
3305+
* drm_atomic_helper_resume()) this function should be called beforehand with
3306+
* @sync set to true. In contexts like runtime resume where the driver is not
3307+
* expected to be calling drm_atomic_helper_resume(), this function should be
3308+
* called with @sync set to false in order to avoid deadlocking.
3309+
*
3310+
* Returns: -1 if the MST topology was removed while we were suspended, 0
3311+
* otherwise.
32103312
*/
3211-
int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr)
3313+
int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr,
3314+
bool sync)
32123315
{
32133316
int ret;
32143317
u8 guid[16];
@@ -3241,8 +3344,19 @@ int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr)
32413344
}
32423345
drm_dp_check_mstb_guid(mgr->mst_primary, guid);
32433346

3347+
/*
3348+
* For the final step of resuming the topology, we need to bring the
3349+
* state of our in-memory topology back into sync with reality. So,
3350+
* restart the probing process as if we're probing a new hub
3351+
*/
3352+
queue_work(system_long_wq, &mgr->work);
32443353
mutex_unlock(&mgr->lock);
32453354

3355+
if (sync) {
3356+
DRM_DEBUG_KMS("Waiting for link probe work to finish re-syncing topology...\n");
3357+
flush_work(&mgr->work);
3358+
}
3359+
32463360
return 0;
32473361

32483362
out_fail:

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7625,7 +7625,8 @@ void intel_dp_mst_resume(struct drm_i915_private *dev_priv)
76257625
if (!intel_dp->can_mst)
76267626
continue;
76277627

7628-
ret = drm_dp_mst_topology_mgr_resume(&intel_dp->mst_mgr);
7628+
ret = drm_dp_mst_topology_mgr_resume(&intel_dp->mst_mgr,
7629+
true);
76297630
if (ret) {
76307631
intel_dp->is_mst = false;
76317632
drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,

drivers/gpu/drm/nouveau/dispnv50/disp.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,14 +1309,14 @@ nv50_mstm_fini(struct nv50_mstm *mstm)
13091309
}
13101310

13111311
static void
1312-
nv50_mstm_init(struct nv50_mstm *mstm)
1312+
nv50_mstm_init(struct nv50_mstm *mstm, bool runtime)
13131313
{
13141314
int ret;
13151315

13161316
if (!mstm || !mstm->mgr.mst_state)
13171317
return;
13181318

1319-
ret = drm_dp_mst_topology_mgr_resume(&mstm->mgr);
1319+
ret = drm_dp_mst_topology_mgr_resume(&mstm->mgr, !runtime);
13201320
if (ret == -1) {
13211321
drm_dp_mst_topology_mgr_set_mst(&mstm->mgr, false);
13221322
drm_kms_helper_hotplug_event(mstm->mgr.dev);
@@ -2263,7 +2263,7 @@ nv50_display_init(struct drm_device *dev, bool resume, bool runtime)
22632263
if (encoder->encoder_type != DRM_MODE_ENCODER_DPMST) {
22642264
struct nouveau_encoder *nv_encoder =
22652265
nouveau_encoder(encoder);
2266-
nv50_mstm_init(nv_encoder->dp.mstm);
2266+
nv50_mstm_init(nv_encoder->dp.mstm, runtime);
22672267
}
22682268
}
22692269

include/drm/drm_dp_mst_helper.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,8 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
706706

707707
void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
708708
int __must_check
709-
drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr);
709+
drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr,
710+
bool sync);
710711

711712
ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
712713
unsigned int offset, void *buffer, size_t size);

0 commit comments

Comments
 (0)