| Message ID | 20181026203549.1796-5-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/dp_mst: Improve VCPI helpers, use in nouveau | expand |
On Fri, Oct 26, 2018 at 04:35:49PM -0400, Lyude Paul wrote: > Currently, nouveau uses the yolo method of setting up MST displays: it > uses the old VCPI helpers (drm_dp_find_vcpi_slots()) for computing the > display configuration. These helpers don't take care to make sure they > take a reference to the mstb port that they're checking, and > additionally don't actually check whether or not the topology still has > enough bandwidth to provide the VCPI tokens required. > > So, drop usage of the old helpers and move entirely over to the atomic > helpers. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 54 +++++++++++++++++++++---- > 1 file changed, 47 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c > index 6cbbae3f438b..37503319e5b1 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -747,16 +747,22 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, > struct drm_crtc_state *crtc_state, > struct drm_connector_state *conn_state) > { > - struct nv50_mstc *mstc = nv50_mstc(conn_state->connector); > + struct drm_atomic_state *state = crtc_state->state; > + struct drm_connector *connector = conn_state->connector; > + struct nv50_mstc *mstc = nv50_mstc(connector); > struct nv50_mstm *mstm = mstc->mstm; > - int bpp = conn_state->connector->display_info.bpc * 3; > + int bpp = connector->display_info.bpc * 3; > int slots; > > - mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, bpp); > - > - slots = drm_dp_find_vcpi_slots(&mstm->mgr, mstc->pbn); > - if (slots < 0) > - return slots; > + mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, > + bpp); > + /* Zombies don't need VCPI */ > + if (!drm_connector_is_unregistered(connector)) { > + slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr, > + mstc->port, mstc->pbn); > + if (slots < 0) > + return slots; > + } > > return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state, > mstc->native); > @@ -920,12 +926,38 @@ nv50_mstc_get_modes(struct drm_connector *connector) > return ret; > } > > +static int > +nv50_mstc_atomic_check(struct drm_connector *connector, > + struct drm_connector_state *new_conn_state) > +{ > + struct drm_atomic_state *state = new_conn_state->state; > + struct nv50_mstc *mstc = nv50_mstc(connector); > + struct drm_dp_mst_topology_mgr *mgr = &mstc->mstm->mgr; > + struct drm_connector_state *old_conn_state; > + struct drm_crtc *old_crtc; > + > + old_conn_state = drm_atomic_get_old_connector_state(state, connector); > + old_crtc = old_conn_state->crtc; > + > + /* We only need to release VCPI here if we're going from having a CRTC > + * on this connector, to not having one > + */ > + if (!old_crtc || new_conn_state->crtc) > + return 0; > + > + /* Release the previous VCPI allocation since the encoder's > + * atomic_check() won't be called > + */ > + return drm_dp_atomic_release_vcpi_slots(state, mgr, mstc->port); > +} > + > static const struct drm_connector_helper_funcs > nv50_mstc_help = { > .get_modes = nv50_mstc_get_modes, > .mode_valid = nv50_mstc_mode_valid, > .best_encoder = nv50_mstc_best_encoder, > .atomic_best_encoder = nv50_mstc_atomic_best_encoder, > + .atomic_check = nv50_mstc_atomic_check, > }; > > static enum drm_connector_status > @@ -2084,6 +2116,8 @@ nv50_disp_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) > struct drm_connector *connector; > struct drm_crtc_state *new_crtc_state; > struct drm_crtc *crtc; > + struct drm_dp_mst_topology_mgr *mgr; > + struct drm_dp_mst_topology_state *mst_state; > int ret, i; > > /* We need to handle colour management on a per-plane basis. */ > @@ -2109,6 +2143,12 @@ nv50_disp_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) > return ret; > } > > + for_each_new_mst_mgr_in_state(state, mgr, mst_state, i) { > + ret = drm_dp_mst_atomic_check(mst_state); > + if (ret) > + return ret; > + } For even less code I think you could also push this loop into the helpers. I can't come up with any scenario where a driver does not want to check all of them at the same time in the atomic_check sequence. Otherwise lgtm. -Daniel > + > return 0; > } > > -- > 2.17.2 >
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 6cbbae3f438b..37503319e5b1 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -747,16 +747,22 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { - struct nv50_mstc *mstc = nv50_mstc(conn_state->connector); + struct drm_atomic_state *state = crtc_state->state; + struct drm_connector *connector = conn_state->connector; + struct nv50_mstc *mstc = nv50_mstc(connector); struct nv50_mstm *mstm = mstc->mstm; - int bpp = conn_state->connector->display_info.bpc * 3; + int bpp = connector->display_info.bpc * 3; int slots; - mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, bpp); - - slots = drm_dp_find_vcpi_slots(&mstm->mgr, mstc->pbn); - if (slots < 0) - return slots; + mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, + bpp); + /* Zombies don't need VCPI */ + if (!drm_connector_is_unregistered(connector)) { + slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr, + mstc->port, mstc->pbn); + if (slots < 0) + return slots; + } return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state, mstc->native); @@ -920,12 +926,38 @@ nv50_mstc_get_modes(struct drm_connector *connector) return ret; } +static int +nv50_mstc_atomic_check(struct drm_connector *connector, + struct drm_connector_state *new_conn_state) +{ + struct drm_atomic_state *state = new_conn_state->state; + struct nv50_mstc *mstc = nv50_mstc(connector); + struct drm_dp_mst_topology_mgr *mgr = &mstc->mstm->mgr; + struct drm_connector_state *old_conn_state; + struct drm_crtc *old_crtc; + + old_conn_state = drm_atomic_get_old_connector_state(state, connector); + old_crtc = old_conn_state->crtc; + + /* We only need to release VCPI here if we're going from having a CRTC + * on this connector, to not having one + */ + if (!old_crtc || new_conn_state->crtc) + return 0; + + /* Release the previous VCPI allocation since the encoder's + * atomic_check() won't be called + */ + return drm_dp_atomic_release_vcpi_slots(state, mgr, mstc->port); +} + static const struct drm_connector_helper_funcs nv50_mstc_help = { .get_modes = nv50_mstc_get_modes, .mode_valid = nv50_mstc_mode_valid, .best_encoder = nv50_mstc_best_encoder, .atomic_best_encoder = nv50_mstc_atomic_best_encoder, + .atomic_check = nv50_mstc_atomic_check, }; static enum drm_connector_status @@ -2084,6 +2116,8 @@ nv50_disp_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) struct drm_connector *connector; struct drm_crtc_state *new_crtc_state; struct drm_crtc *crtc; + struct drm_dp_mst_topology_mgr *mgr; + struct drm_dp_mst_topology_state *mst_state; int ret, i; /* We need to handle colour management on a per-plane basis. */ @@ -2109,6 +2143,12 @@ nv50_disp_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) return ret; } + for_each_new_mst_mgr_in_state(state, mgr, mst_state, i) { + ret = drm_dp_mst_atomic_check(mst_state); + if (ret) + return ret; + } + return 0; }
Currently, nouveau uses the yolo method of setting up MST displays: it uses the old VCPI helpers (drm_dp_find_vcpi_slots()) for computing the display configuration. These helpers don't take care to make sure they take a reference to the mstb port that they're checking, and additionally don't actually check whether or not the topology still has enough bandwidth to provide the VCPI tokens required. So, drop usage of the old helpers and move entirely over to the atomic helpers. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 54 +++++++++++++++++++++---- 1 file changed, 47 insertions(+), 7 deletions(-)