summaryrefslogtreecommitdiff
path: root/include/drm
diff options
context:
space:
mode:
authorLyude Paul <lyude@redhat.com>2019-10-16 23:02:59 +0300
committerLyude Paul <lyude@redhat.com>2019-10-24 21:24:40 +0300
commit14692a3637d4f1cd8ccdb8c605222037b3ac3494 (patch)
tree02a8b470dc37b3fd382efcd84d55664d01b5c9e9 /include/drm
parent9408cc94eb041d0c2f9f00189a613b94c0449450 (diff)
downloadlinux-14692a3637d4f1cd8ccdb8c605222037b3ac3494.tar.xz
drm/dp_mst: Add probe_lock
Currently, MST lacks locking in a lot of places that really should have some sort of locking. Hotplugging and link address code paths are some of the offenders here, as there is actually nothing preventing us from running a link address probe while at the same time handling a connection status update request - something that's likely always been possible but never seen in the wild because hotplugging has been broken for ages now (with the exception of amdgpu, for reasons I don't think are worth digging into very far). Note: I'm going to start using the term "in-memory topology layout" here to refer to drm_dp_mst_port->mstb and drm_dp_mst_branch->ports. Locking in these places is a little tougher then it looks though. Generally we protect anything having to do with the in-memory topology layout under &mgr->lock. But this becomes nearly impossible to do from the context of link address probes due to the fact that &mgr->lock is usually grabbed under random various modesetting locks, meaning that there's no way we can just invert the &mgr->lock order and keep it locked throughout the whole process of updating the topology. Luckily there are only two workers which can modify the in-memory topology layout: drm_dp_mst_up_req_work() and drm_dp_mst_link_probe_work(), meaning as long as we prevent these two workers from traveling the topology layout in parallel with the intent of updating it we don't need to worry about grabbing &mgr->lock in these workers for reads. We only need to grab &mgr->lock in these workers for writes, so that readers outside these two workers are still protected from the topology layout changing beneath them. So, add the new &mgr->probe_lock and use it in both drm_dp_mst_link_probe_work() and drm_dp_mst_up_req_work(). Additionally, add some more detailed explanations for how this locking is intended to work to drm_dp_mst_port->mstb and drm_dp_mst_branch->ports. Signed-off-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Sean Paul <sean@poorly.run> Cc: Juston Li <juston.li@intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Harry Wentland <hwentlan@amd.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20191022023641.8026-6-lyude@redhat.com
Diffstat (limited to 'include/drm')
-rw-r--r--include/drm/drm_dp_mst_helper.h32
1 files changed, 28 insertions, 4 deletions
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 7d80c38ee00e..bccb5514e0ef 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -55,8 +55,6 @@ struct drm_dp_vcpi {
* @num_sdp_stream_sinks: Number of stream sinks
* @available_pbn: Available bandwidth for this port.
* @next: link to next port on this branch device
- * @mstb: branch device on this port, protected by
- * &drm_dp_mst_topology_mgr.lock
* @aux: i2c aux transport to talk to device connected to this port, protected
* by &drm_dp_mst_topology_mgr.lock
* @parent: branch device parent of this port
@@ -92,7 +90,17 @@ struct drm_dp_mst_port {
u8 num_sdp_stream_sinks;
uint16_t available_pbn;
struct list_head next;
- struct drm_dp_mst_branch *mstb; /* pointer to an mstb if this port has one */
+ /**
+ * @mstb: the branch device connected to this port, if there is one.
+ * This should be considered protected for reading by
+ * &drm_dp_mst_topology_mgr.lock. There are two exceptions to this:
+ * &drm_dp_mst_topology_mgr.up_req_work and
+ * &drm_dp_mst_topology_mgr.work, which do not grab
+ * &drm_dp_mst_topology_mgr.lock during reads but are the only
+ * updaters of this list and are protected from writing concurrently
+ * by &drm_dp_mst_topology_mgr.probe_lock.
+ */
+ struct drm_dp_mst_branch *mstb;
struct drm_dp_aux aux; /* i2c bus for this port? */
struct drm_dp_mst_branch *parent;
@@ -118,7 +126,6 @@ struct drm_dp_mst_port {
* @lct: Link count total to talk to this branch device.
* @num_ports: number of ports on the branch.
* @msg_slots: one bit per transmitted msg slot.
- * @ports: linked list of ports on this branch.
* @port_parent: pointer to the port parent, NULL if toplevel.
* @mgr: topology manager for this branch device.
* @tx_slots: transmission slots for this device.
@@ -156,6 +163,16 @@ struct drm_dp_mst_branch {
int num_ports;
int msg_slots;
+ /**
+ * @ports: the list of ports on this branch device. This should be
+ * considered protected for reading by &drm_dp_mst_topology_mgr.lock.
+ * There are two exceptions to this:
+ * &drm_dp_mst_topology_mgr.up_req_work and
+ * &drm_dp_mst_topology_mgr.work, which do not grab
+ * &drm_dp_mst_topology_mgr.lock during reads but are the only
+ * updaters of this list and are protected from updating the list
+ * concurrently by @drm_dp_mst_topology_mgr.probe_lock
+ */
struct list_head ports;
/* list of tx ops queue for this port */
@@ -503,6 +520,13 @@ struct drm_dp_mst_topology_mgr {
struct mutex lock;
/**
+ * @probe_lock: Prevents @work and @up_req_work, the only writers of
+ * &drm_dp_mst_port.mstb and &drm_dp_mst_branch.ports, from racing
+ * while they update the topology.
+ */
+ struct mutex probe_lock;
+
+ /**
* @mst_state: If this manager is enabled for an MST capable port. False
* if no MST sink/branch devices is connected.
*/