drm/display/dp_mst: Add helpers for serializing SST <-> MST transitions
authorLyude Paul <lyude@redhat.com>
Wed, 17 Aug 2022 19:38:42 +0000 (15:38 -0400)
committerLyude Paul <lyude@redhat.com>
Tue, 23 Aug 2022 20:53:41 +0000 (16:53 -0400)
There's another kind of situation where we could potentially race with
nonblocking modesets and MST, especially if we were to only use the locking
provided by atomic modesetting:

* Display 1 begins as enabled on DP-1 in SST mode
* Display 1 switches to MST mode, exposes one sink in MST mode
* Userspace does non-blocking modeset to disable the SST display
* Userspace does non-blocking modeset to enable the MST display with a
  different CRTC, but the SST display hasn't been fully taken down yet
* Execution order between the last two commits isn't guaranteed since they
  share no drm resources

We can fix this however, by ensuring that we always pull in the atomic
topology state whenever a connector capable of driving an MST display
performs its atomic check - and then tracking CRTC commits happening on the
SST connector in the MST topology state. So, let's add some simple helpers
for doing that and hook them up in various drivers.

v2:
* Use intel_dp_mst_source_support() to check for MST support in i915, fixes
  CI failures

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Wayne Lin <Wayne.Lin@amd.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Fangzhi Zuo <Jerry.Zuo@amd.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sean Paul <sean@poorly.run>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220817193847.557945-14-lyude@redhat.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
drivers/gpu/drm/display/drm_dp_mst_topology.c
drivers/gpu/drm/i915/display/intel_dp.c
drivers/gpu/drm/nouveau/dispnv50/disp.c
drivers/gpu/drm/nouveau/dispnv50/disp.h
drivers/gpu/drm/nouveau/nouveau_connector.c
include/drm/display/drm_dp_mst_helper.h

index c97a4d759b94a11387f250183c67b56f9c79c680..789748739d79ac79d14583e287d1a6c10d62aa2d 100644 (file)
@@ -6291,10 +6291,17 @@ amdgpu_dm_connector_atomic_check(struct drm_connector *conn,
                drm_atomic_get_old_connector_state(state, conn);
        struct drm_crtc *crtc = new_con_state->crtc;
        struct drm_crtc_state *new_crtc_state;
+       struct amdgpu_dm_connector *aconn = to_amdgpu_dm_connector(conn);
        int ret;
 
        trace_amdgpu_dm_connector_atomic_check(new_con_state);
 
+       if (conn->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
+               ret = drm_dp_mst_root_conn_atomic_check(new_con_state, &aconn->mst_mgr);
+               if (ret < 0)
+                       return ret;
+       }
+
        if (!crtc)
                return 0;
 
index 2f7c43f88d74a0ba2d833b6f5377f2cf70e4dc06..97e8f8a83ed4d8eb28f5cb5a8ab703a6137c1e77 100644 (file)
@@ -4597,6 +4597,65 @@ void drm_dp_mst_atomic_wait_for_dependencies(struct drm_atomic_state *state)
 }
 EXPORT_SYMBOL(drm_dp_mst_atomic_wait_for_dependencies);
 
+/**
+ * drm_dp_mst_root_conn_atomic_check() - Serialize CRTC commits on MST-capable connectors operating
+ * in SST mode
+ * @new_conn_state: The new connector state of the &drm_connector
+ * @mgr: The MST topology manager for the &drm_connector
+ *
+ * Since MST uses fake &drm_encoder structs, the generic atomic modesetting code isn't able to
+ * serialize non-blocking commits happening on the real DP connector of an MST topology switching
+ * into/away from MST mode - as the CRTC on the real DP connector and the CRTCs on the connector's
+ * MST topology will never share the same &drm_encoder.
+ *
+ * This function takes care of this serialization issue, by checking a root MST connector's atomic
+ * state to determine if it is about to have a modeset - and then pulling in the MST topology state
+ * if so, along with adding any relevant CRTCs to &drm_dp_mst_topology_state.pending_crtc_mask.
+ *
+ * Drivers implementing MST must call this function from the
+ * &drm_connector_helper_funcs.atomic_check hook of any physical DP &drm_connector capable of
+ * driving MST sinks.
+ *
+ * Returns:
+ * 0 on success, negative error code otherwise
+ */
+int drm_dp_mst_root_conn_atomic_check(struct drm_connector_state *new_conn_state,
+                                     struct drm_dp_mst_topology_mgr *mgr)
+{
+       struct drm_atomic_state *state = new_conn_state->state;
+       struct drm_connector_state *old_conn_state =
+               drm_atomic_get_old_connector_state(state, new_conn_state->connector);
+       struct drm_crtc_state *crtc_state;
+       struct drm_dp_mst_topology_state *mst_state = NULL;
+
+       if (new_conn_state->crtc) {
+               crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc);
+               if (crtc_state && drm_atomic_crtc_needs_modeset(crtc_state)) {
+                       mst_state = drm_atomic_get_mst_topology_state(state, mgr);
+                       if (IS_ERR(mst_state))
+                               return PTR_ERR(mst_state);
+
+                       mst_state->pending_crtc_mask |= drm_crtc_mask(new_conn_state->crtc);
+               }
+       }
+
+       if (old_conn_state->crtc) {
+               crtc_state = drm_atomic_get_new_crtc_state(state, old_conn_state->crtc);
+               if (crtc_state && drm_atomic_crtc_needs_modeset(crtc_state)) {
+                       if (!mst_state) {
+                               mst_state = drm_atomic_get_mst_topology_state(state, mgr);
+                               if (IS_ERR(mst_state))
+                                       return PTR_ERR(mst_state);
+                       }
+
+                       mst_state->pending_crtc_mask |= drm_crtc_mask(old_conn_state->crtc);
+               }
+       }
+
+       return 0;
+}
+EXPORT_SYMBOL(drm_dp_mst_root_conn_atomic_check);
+
 /**
  * drm_dp_mst_update_slots() - updates the slot info depending on the DP ecoding format
  * @mst_state: mst_state to update
index 32292c0be2bd07a6512aa039da65f0bb6a5471c8..a4e113253df3fe14148d49e107996003c2fd4595 100644 (file)
@@ -4992,12 +4992,21 @@ static int intel_dp_connector_atomic_check(struct drm_connector *conn,
 {
        struct drm_i915_private *dev_priv = to_i915(conn->dev);
        struct intel_atomic_state *state = to_intel_atomic_state(_state);
+       struct drm_connector_state *conn_state = drm_atomic_get_new_connector_state(_state, conn);
+       struct intel_connector *intel_conn = to_intel_connector(conn);
+       struct intel_dp *intel_dp = enc_to_intel_dp(intel_conn->encoder);
        int ret;
 
        ret = intel_digital_connector_atomic_check(conn, &state->base);
        if (ret)
                return ret;
 
+       if (intel_dp_mst_source_support(intel_dp)) {
+               ret = drm_dp_mst_root_conn_atomic_check(conn_state, &intel_dp->mst_mgr);
+               if (ret)
+                       return ret;
+       }
+
        /*
         * We don't enable port sync on BDW due to missing w/as and
         * due to not having adjusted the modeset sequence appropriately.
index 24807aa9da5f86278c8d3261705948b6cd3b1c9e..7e9a0b50bb42a9bbcec85b9d5dcf9ccab2a501e9 100644 (file)
@@ -1813,7 +1813,7 @@ nv50_sor_func = {
        .destroy = nv50_sor_destroy,
 };
 
-static bool nv50_has_mst(struct nouveau_drm *drm)
+bool nv50_has_mst(struct nouveau_drm *drm)
 {
        struct nvkm_bios *bios = nvxx_bios(&drm->client.device);
        u32 data;
index 38dec11e7dda5577fdd11263b8e9b8b8b2a44309..9d66c9c726c35ef315e34f39fea7ec8feca4eaf0 100644 (file)
@@ -106,6 +106,8 @@ void nv50_dmac_destroy(struct nv50_dmac *);
  */
 struct nouveau_encoder *nv50_real_outp(struct drm_encoder *encoder);
 
+bool nv50_has_mst(struct nouveau_drm *drm);
+
 u32 *evo_wait(struct nv50_dmac *, int nr);
 void evo_kick(u32 *, struct nv50_dmac *);
 
index b8ee2173ca8f4d5c5c3badf9a11e858be79b6f2f..1991bbb1d05c386b5b62d4f915becfe8c08586aa 100644 (file)
@@ -1106,11 +1106,25 @@ nouveau_connector_best_encoder(struct drm_connector *connector)
        return NULL;
 }
 
+static int
+nouveau_connector_atomic_check(struct drm_connector *connector, struct drm_atomic_state *state)
+{
+       struct nouveau_connector *nv_conn = nouveau_connector(connector);
+       struct drm_connector_state *conn_state =
+               drm_atomic_get_new_connector_state(state, connector);
+
+       if (!nv_conn->dp_encoder || !nv50_has_mst(nouveau_drm(connector->dev)))
+               return 0;
+
+       return drm_dp_mst_root_conn_atomic_check(conn_state, &nv_conn->dp_encoder->dp.mstm->mgr);
+}
+
 static const struct drm_connector_helper_funcs
 nouveau_connector_helper_funcs = {
        .get_modes = nouveau_connector_get_modes,
        .mode_valid = nouveau_connector_mode_valid,
        .best_encoder = nouveau_connector_best_encoder,
+       .atomic_check = nouveau_connector_atomic_check,
 };
 
 static const struct drm_connector_funcs
index 0ef7d0e6cf0c0f7c8e0b2da1ce229fed572f523a..b9c361b242ea482e0f5f406abbe4fedebd50b8e0 100644 (file)
@@ -911,6 +911,8 @@ int drm_dp_send_query_stream_enc_status(struct drm_dp_mst_topology_mgr *mgr,
                struct drm_dp_mst_port *port,
                struct drm_dp_query_stream_enc_status_ack_reply *status);
 int __must_check drm_dp_mst_atomic_check(struct drm_atomic_state *state);
+int __must_check drm_dp_mst_root_conn_atomic_check(struct drm_connector_state *new_conn_state,
+                                                  struct drm_dp_mst_topology_mgr *mgr);
 
 void drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port);
 void drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port);