| Message ID | 20241205-dp_mst-v1-35-f8618d42a99a@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm/dp: Add MST support for MSM chipsets | expand |
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 --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 {
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(+)