diff mbox series

[35/45] drm/msm/dp: add a mst session mutex to protect bridge ops

Message ID 20241205-dp_mst-v1-35-f8618d42a99a@quicinc.com (mailing list archive)
State New, archived
Headers show
Series drm/msm/dp: Add MST support for MSM chipsets | expand

Commit Message

Abhinav Kumar Dec. 6, 2024, 4:32 a.m. UTC 
To protect against concurrent access of the dp mst bridges
introduce a session mutex.

Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_mst_drm.c | 20 ++++++++++++++++++++
 drivers/gpu/drm/msm/dp/dp_mst_drm.h |  1 +
 2 files changed, 21 insertions(+)

Comments

Dmitry Baryshkov Dec. 8, 2024, 8:54 a.m. UTC | #1 
On Thu, Dec 05, 2024 at 08:32:06PM -0800, Abhinav Kumar wrote:
> To protect against concurrent access of the dp mst bridges
> introduce a session mutex.

Why is it necessary? What kind of concurrent access is expected?

Why wasn't it introduced together with struct msm_dp_mst?

> 
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_mst_drm.c | 20 ++++++++++++++++++++
>  drivers/gpu/drm/msm/dp/dp_mst_drm.h |  1 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_mst_drm.c b/drivers/gpu/drm/msm/dp/dp_mst_drm.c
> index 15c61fd37c418889074222c0f576778adadf51c9..313eb63b9a35cbbb36db2d7d8f0a85e4441f2998 100644
> --- a/drivers/gpu/drm/msm/dp/dp_mst_drm.c
> +++ b/drivers/gpu/drm/msm/dp/dp_mst_drm.c
> @@ -344,6 +344,7 @@ static void msm_dp_mst_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
>  	struct msm_dp_mst_bridge *bridge;
>  	struct msm_dp *dp;
>  	struct msm_dp_mst_bridge_state *msm_dp_bridge_state;
> +	struct msm_dp_mst *dp_mst;
>  
>  	if (!drm_bridge) {
>  		DRM_ERROR("Invalid params\n");
> @@ -353,6 +354,7 @@ static void msm_dp_mst_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
>  	bridge = to_msm_dp_mst_bridge(drm_bridge);
>  	msm_dp_bridge_state = to_msm_dp_mst_bridge_state(bridge);
>  	dp = bridge->display;
> +	dp_mst = dp->msm_dp_mst;
>  
>  	/* to cover cases of bridge_disable/bridge_enable without modeset */
>  	bridge->connector = msm_dp_bridge_state->connector;
> @@ -363,12 +365,14 @@ static void msm_dp_mst_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
>  		return;
>  	}
>  
> +	mutex_lock(&dp_mst->mst_lock);
>  	msm_dp_display_atomic_prepare(dp);
>  
>  	rc = _msm_dp_mst_bridge_pre_enable_part1(bridge, old_bridge_state);
>  	if (rc) {
>  		DRM_ERROR("[%d] DP display pre-enable failed, rc=%d\n", bridge->id, rc);
>  		msm_dp_display_unprepare(dp);
> +		mutex_unlock(&dp_mst->mst_lock);
>  		return;
>  	}
>  
> @@ -381,6 +385,8 @@ static void msm_dp_mst_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
>  		   drm_mode_vrefresh(&bridge->drm_mode),
>  		   bridge->vcpi, bridge->start_slot,
>  		   bridge->start_slot + bridge->num_slots);
> +
> +	mutex_unlock(&dp_mst->mst_lock);
>  }
>  
>  static void msm_dp_mst_bridge_atomic_disable(struct drm_bridge *drm_bridge,
> @@ -388,6 +394,7 @@ static void msm_dp_mst_bridge_atomic_disable(struct drm_bridge *drm_bridge,
>  {
>  	struct msm_dp_mst_bridge *bridge;
>  	struct msm_dp *dp;
> +	struct msm_dp_mst *mst;
>  
>  	if (!drm_bridge) {
>  		DRM_ERROR("Invalid params\n");
> @@ -401,6 +408,9 @@ static void msm_dp_mst_bridge_atomic_disable(struct drm_bridge *drm_bridge,
>  	}
>  
>  	dp = bridge->display;
> +	mst = dp->msm_dp_mst;
> +
> +	mutex_lock(&mst->mst_lock);
>  
>  	_msm_dp_mst_bridge_pre_disable_part1(bridge, old_bridge_state);
>  
> @@ -410,6 +420,8 @@ static void msm_dp_mst_bridge_atomic_disable(struct drm_bridge *drm_bridge,
>  
>  	drm_dbg_dp(dp->drm_dev, "mst bridge:%d conn:%d disable complete\n", bridge->id,
>  		   DP_MST_CONN_ID(bridge));
> +
> +	mutex_unlock(&mst->mst_lock);
>  }
>  
>  static void msm_dp_mst_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
> @@ -418,6 +430,7 @@ static void msm_dp_mst_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
>  	int conn = 0;
>  	struct msm_dp_mst_bridge *bridge;
>  	struct msm_dp *dp;
> +	struct msm_dp_mst *mst;
>  
>  	if (!drm_bridge) {
>  		DRM_ERROR("Invalid params\n");
> @@ -433,6 +446,9 @@ static void msm_dp_mst_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
>  	conn = DP_MST_CONN_ID(bridge);
>  
>  	dp = bridge->display;
> +	mst = dp->msm_dp_mst;
> +
> +	mutex_lock(&mst->mst_lock);
>  
>  	msm_dp_display_atomic_post_disable_helper(dp, bridge->msm_dp_panel);
>  
> @@ -444,6 +460,8 @@ static void msm_dp_mst_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
>  
>  	drm_dbg_dp(dp->drm_dev, "mst bridge:%d conn:%d post disable complete\n",
>  		   bridge->id, conn);
> +
> +	mutex_unlock(&mst->mst_lock);
>  }
>  
>  static void msm_dp_mst_bridge_mode_set(struct drm_bridge *drm_bridge,
> @@ -1072,6 +1090,8 @@ int msm_dp_mst_init(struct msm_dp *dp_display, u32 max_streams, u32 max_dpcd_tra
>  
>  	dp_display->msm_dp_mst = msm_dp_mst;
>  
> +	mutex_init(&msm_dp_mst->mst_lock);
> +
>  	msm_dp_mst->mst_initialized = true;
>  
>  	drm_dbg_dp(dp_display->drm_dev, "dp drm mst topology manager init completed\n");
> diff --git a/drivers/gpu/drm/msm/dp/dp_mst_drm.h b/drivers/gpu/drm/msm/dp/dp_mst_drm.h
> index 5fe5dc7596086467e9a3b3d7d04a665853fbb3d7..b1adb8a61115d4809107553809206bb2ed3c6c3d 100644
> --- a/drivers/gpu/drm/msm/dp/dp_mst_drm.h
> +++ b/drivers/gpu/drm/msm/dp/dp_mst_drm.h
> @@ -80,6 +80,7 @@ struct msm_dp_mst {
>  	struct drm_dp_aux *dp_aux;
>  	bool mst_session_hpd_state;
>  	u32 max_streams;
> +	struct mutex mst_lock;
>  };
>  
>  struct msm_dp_mst_connector {
> 
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/dp/dp_mst_drm.c b/drivers/gpu/drm/msm/dp/dp_mst_drm.c
index 15c61fd37c418889074222c0f576778adadf51c9..313eb63b9a35cbbb36db2d7d8f0a85e4441f2998 100644
--- a/drivers/gpu/drm/msm/dp/dp_mst_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_mst_drm.c
@@ -344,6 +344,7 @@  static void msm_dp_mst_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
 	struct msm_dp_mst_bridge *bridge;
 	struct msm_dp *dp;
 	struct msm_dp_mst_bridge_state *msm_dp_bridge_state;
+	struct msm_dp_mst *dp_mst;
 
 	if (!drm_bridge) {
 		DRM_ERROR("Invalid params\n");
@@ -353,6 +354,7 @@  static void msm_dp_mst_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
 	bridge = to_msm_dp_mst_bridge(drm_bridge);
 	msm_dp_bridge_state = to_msm_dp_mst_bridge_state(bridge);
 	dp = bridge->display;
+	dp_mst = dp->msm_dp_mst;
 
 	/* to cover cases of bridge_disable/bridge_enable without modeset */
 	bridge->connector = msm_dp_bridge_state->connector;
@@ -363,12 +365,14 @@  static void msm_dp_mst_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
 		return;
 	}
 
+	mutex_lock(&dp_mst->mst_lock);
 	msm_dp_display_atomic_prepare(dp);
 
 	rc = _msm_dp_mst_bridge_pre_enable_part1(bridge, old_bridge_state);
 	if (rc) {
 		DRM_ERROR("[%d] DP display pre-enable failed, rc=%d\n", bridge->id, rc);
 		msm_dp_display_unprepare(dp);
+		mutex_unlock(&dp_mst->mst_lock);
 		return;
 	}
 
@@ -381,6 +385,8 @@  static void msm_dp_mst_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
 		   drm_mode_vrefresh(&bridge->drm_mode),
 		   bridge->vcpi, bridge->start_slot,
 		   bridge->start_slot + bridge->num_slots);
+
+	mutex_unlock(&dp_mst->mst_lock);
 }
 
 static void msm_dp_mst_bridge_atomic_disable(struct drm_bridge *drm_bridge,
@@ -388,6 +394,7 @@  static void msm_dp_mst_bridge_atomic_disable(struct drm_bridge *drm_bridge,
 {
 	struct msm_dp_mst_bridge *bridge;
 	struct msm_dp *dp;
+	struct msm_dp_mst *mst;
 
 	if (!drm_bridge) {
 		DRM_ERROR("Invalid params\n");
@@ -401,6 +408,9 @@  static void msm_dp_mst_bridge_atomic_disable(struct drm_bridge *drm_bridge,
 	}
 
 	dp = bridge->display;
+	mst = dp->msm_dp_mst;
+
+	mutex_lock(&mst->mst_lock);
 
 	_msm_dp_mst_bridge_pre_disable_part1(bridge, old_bridge_state);
 
@@ -410,6 +420,8 @@  static void msm_dp_mst_bridge_atomic_disable(struct drm_bridge *drm_bridge,
 
 	drm_dbg_dp(dp->drm_dev, "mst bridge:%d conn:%d disable complete\n", bridge->id,
 		   DP_MST_CONN_ID(bridge));
+
+	mutex_unlock(&mst->mst_lock);
 }
 
 static void msm_dp_mst_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
@@ -418,6 +430,7 @@  static void msm_dp_mst_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
 	int conn = 0;
 	struct msm_dp_mst_bridge *bridge;
 	struct msm_dp *dp;
+	struct msm_dp_mst *mst;
 
 	if (!drm_bridge) {
 		DRM_ERROR("Invalid params\n");
@@ -433,6 +446,9 @@  static void msm_dp_mst_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
 	conn = DP_MST_CONN_ID(bridge);
 
 	dp = bridge->display;
+	mst = dp->msm_dp_mst;
+
+	mutex_lock(&mst->mst_lock);
 
 	msm_dp_display_atomic_post_disable_helper(dp, bridge->msm_dp_panel);
 
@@ -444,6 +460,8 @@  static void msm_dp_mst_bridge_atomic_post_disable(struct drm_bridge *drm_bridge,
 
 	drm_dbg_dp(dp->drm_dev, "mst bridge:%d conn:%d post disable complete\n",
 		   bridge->id, conn);
+
+	mutex_unlock(&mst->mst_lock);
 }
 
 static void msm_dp_mst_bridge_mode_set(struct drm_bridge *drm_bridge,
@@ -1072,6 +1090,8 @@  int msm_dp_mst_init(struct msm_dp *dp_display, u32 max_streams, u32 max_dpcd_tra
 
 	dp_display->msm_dp_mst = msm_dp_mst;
 
+	mutex_init(&msm_dp_mst->mst_lock);
+
 	msm_dp_mst->mst_initialized = true;
 
 	drm_dbg_dp(dp_display->drm_dev, "dp drm mst topology manager init completed\n");
diff --git a/drivers/gpu/drm/msm/dp/dp_mst_drm.h b/drivers/gpu/drm/msm/dp/dp_mst_drm.h
index 5fe5dc7596086467e9a3b3d7d04a665853fbb3d7..b1adb8a61115d4809107553809206bb2ed3c6c3d 100644
--- a/drivers/gpu/drm/msm/dp/dp_mst_drm.h
+++ b/drivers/gpu/drm/msm/dp/dp_mst_drm.h
@@ -80,6 +80,7 @@  struct msm_dp_mst {
 	struct drm_dp_aux *dp_aux;
 	bool mst_session_hpd_state;
 	u32 max_streams;
+	struct mutex mst_lock;
 };
 
 struct msm_dp_mst_connector {