On Mon, Jun 16, 2025 at 08:43:40PM +0800, Yongxing Mou wrote:
>
>
> On 2025/6/9 23:51, Dmitry Baryshkov wrote:
> > On Mon, Jun 09, 2025 at 08:21:48PM +0800, Yongxing Mou wrote:
> > > From: Abhinav Kumar <quic_abhin...@quicinc.com>
> > >
> > > Add connector abstraction for the DP MST. Each MST encoder
> > > is connected through a DRM bridge to a MST connector and each
> > > MST connector has a DP panel abstraction attached to it.
> > >
> > > Signed-off-by: Abhinav Kumar <quic_abhin...@quicinc.com>
> > > Signed-off-by: Yongxing Mou <quic_yong...@quicinc.com>
> > > ---
> > > drivers/gpu/drm/msm/dp/dp_mst_drm.c | 515
> > > ++++++++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/msm/dp/dp_mst_drm.h | 3 +
> > > 2 files changed, 518 insertions(+)
> >
> > > +
> > > +static enum drm_mode_status msm_dp_mst_connector_mode_valid(struct
> > > drm_connector *connector,
> > > + const struct
> > > drm_display_mode *mode)
> > > +{
> > > + struct msm_dp_mst_connector *mst_conn;
> > > + struct msm_dp *dp_display;
> > > + struct drm_dp_mst_port *mst_port;
> > > + struct msm_dp_panel *dp_panel;
> > > + struct msm_dp_mst *mst;
> > > + u16 full_pbn, required_pbn;
> > > + int available_slots, required_slots;
> > > + struct msm_dp_mst_bridge_state *dp_bridge_state;
> > > + int i, slots_in_use = 0, active_enc_cnt = 0;
> > > + const u32 tot_slots = 63;
> > > +
> > > + if (drm_connector_is_unregistered(connector))
> > > + return 0;
> > > +
> > > + mst_conn = to_msm_dp_mst_connector(connector);
> > > + dp_display = mst_conn->msm_dp;
> > > + mst = dp_display->msm_dp_mst;
> > > + mst_port = mst_conn->mst_port;
> > > + dp_panel = mst_conn->dp_panel;
> > > +
> > > + if (!dp_panel || !mst_port)
> > > + return MODE_ERROR;
> > > +
> > > + for (i = 0; i < mst->max_streams; i++) {
> > > + dp_bridge_state =
> > > to_msm_dp_mst_bridge_state(&mst->mst_bridge[i]);
> > > + if (dp_bridge_state->connector &&
> > > + dp_bridge_state->connector != connector) {
> > > + active_enc_cnt++;
> > > + slots_in_use += dp_bridge_state->num_slots;
> > > + }
> > > + }
> > > +
> > > + if (active_enc_cnt < DP_STREAM_MAX) {
> > > + full_pbn = mst_port->full_pbn;
> > > + available_slots = tot_slots - slots_in_use;
> > > + } else {
> > > + DRM_ERROR("all mst streams are active\n");
> > > + return MODE_BAD;
> > > + }
> > > +
> > > + required_pbn = drm_dp_calc_pbn_mode(mode->clock,
> > > (connector->display_info.bpc * 3) << 4);
> > > +
> > > + required_slots = msm_dp_mst_find_vcpi_slots(&mst->mst_mgr,
> > > required_pbn);
> > > +
> > > + if (required_pbn > full_pbn || required_slots > available_slots) {
> > > + drm_dbg_dp(dp_display->drm_dev,
> > > + "mode:%s not supported. pbn %d vs %d slots %d vs
> > > %d\n",
> > > + mode->name, required_pbn, full_pbn,
> > > + required_slots, available_slots);
> > > + return MODE_BAD;
> > > + }
> >
> > I almost missed this. Could you please point me, do other drivers
> > perform mode_valid() check based on the current slots available or not?
> > Could you please point me to the relevant code in other drivers? Because
> > it doesn't look correct to me. The mode on the screen remains valid no
> > matter if I plug or unplug other devices. The atomic_check() should fail
> > if we don't have enough resources (which includes slots).
> >
> Currently, I haven't found other drivers checking available slots during
> mode_valid(). Intel will check the PBN in here.
pointer? Also, what do AMD and nouveau do?
> This condition can help us
> in the following case:
>
> Assume two downstream devices both support 4K 60Hz 10-bit. In MST mode, when
> the first device occupies the 4Kx60Hzx10bit mode, the remaining bandwidth is
> insufficient to support the same mode for the second device.
>
> If we check the slots in mode_valid(), the second device will reject the
> 4Kx60Hzx10bit mode but accept 4Kx30Hzx10bit. However, if the check is done
> in atomic_check(), the second device will display a black screen (because
> 4Kx60Hzx10bit is considered valid in mode_valid() but failed in
> atomic_check()).
If we filter modes in mode_valid(), then consider the following
scenario: we plug monitor A, plug monitor B, then unplug monitor A. At
this point we only have monitor B, but it has all modes filtered when A
has been plugged. So, it is impossible to select 4k@60x10, even though
it is a perfectly valid mode now.
Also, with the check happening in the atomic_check() the user will not
get the black screen: the commit will get rejected, letting userspace to
lower the mode for the second monitor.
> > > +
> > > + return msm_dp_display_mode_valid(dp_display,
> > > &dp_display->connector->display_info, mode);
> > > +}
> > > +
> >
>
--
With best wishes
Dmitry
