| Message ID | 20191030192431.5798-2-mikita.lipski@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DSC MST support for AMDGPU | expand |
On 2019-10-30 3:24 p.m., mikita.lipski@amd.com wrote: > From: Mikita Lipski <mikita.lipski@amd.com> > > - Adding encoder atomic check to find vcpi slots for a connector > - Using DRM helper functions to calculate PBN > - Adding connector atomic check to release vcpi slots if connector > loses CRTC > - Calculate PBN and VCPI slots only once during atomic > check and store them on crtc_state to eliminate > redundant calculation > - Call drm_dp_mst_atomic_check to verify validity of MST topology > during state atomic check > > v2: squashed previous 3 separate patches, removed DSC PBN calculation, > and added PBN and VCPI slots properties to amdgpu connector > > v3: > - moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state > - updates stream's vcpi_slots and pbn on commit > - separated patch from the DSC MST series > > v4: > - set vcpi_slots and pbn properties to dm_connector_state > - copy porperties from connector state on to crtc state > > v5: > - keep the pbn and vcpi values only on connnector state > - added a void pointer to the stream state instead on two ints, > because dc_stream_state is OS agnostic. Pointer points to the > current dm_connector_state. > > v6: > - Remove new param from stream > > v7: > - Fix error with using max capable bpc > > Cc: Jun Lei <Jun.Lei@amd.com> > Cc: Jerry Zuo <Jerry.Zuo@amd.com> > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> > Cc: Lyude Paul <lyude@redhat.com> > Signed-off-by: Mikita Lipski <mikita.lipski@amd.com> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> You might want to verify that this still works as you expect when changing "max bpc" on an MST display. My understanding is that it'd still force a new modeset before encoder atomic check is called so you *should* have the correct bpc value during MST calculations. > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 65 ++++++++++++++++++- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 + > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 ++++----------- > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 32 +++++++++ > 4 files changed, 109 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 48f5b43e2698..d75726013436 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector) > state->underscan_hborder = 0; > state->underscan_vborder = 0; > state->base.max_requested_bpc = 8; > - > + state->vcpi_slots = 0; > + state->pbn = 0; > if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) > state->abm_level = amdgpu_dm_abm_level; > > @@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct drm_connector *connector) > new_state->underscan_enable = state->underscan_enable; > new_state->underscan_hborder = state->underscan_hborder; > new_state->underscan_vborder = state->underscan_vborder; > - > + new_state->vcpi_slots = state->vcpi_slots; > + new_state->pbn = state->pbn; > return &new_state->base; > } > > @@ -4606,10 +4608,64 @@ static void dm_encoder_helper_disable(struct drm_encoder *encoder) > > } > > +static int convert_dc_color_depth_into_bpc (enum dc_color_depth display_color_depth) > +{ > + switch (display_color_depth) { > + case COLOR_DEPTH_666: > + return 6; > + case COLOR_DEPTH_888: > + return 8; > + case COLOR_DEPTH_101010: > + return 10; > + case COLOR_DEPTH_121212: > + return 12; > + case COLOR_DEPTH_141414: > + return 14; > + case COLOR_DEPTH_161616: > + return 16; > + default: > + break; > + } > + return 0; > +} > + > static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, > struct drm_crtc_state *crtc_state, > struct drm_connector_state *conn_state) > { > + struct drm_atomic_state *state = crtc_state->state; > + struct drm_connector *connector = conn_state->connector; > + struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); > + struct dm_connector_state *dm_new_connector_state = to_dm_connector_state(conn_state); > + const struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; > + struct drm_dp_mst_topology_mgr *mst_mgr; > + struct drm_dp_mst_port *mst_port; > + enum dc_color_depth color_depth; > + int clock, bpp = 0; > + > + if (!aconnector->port || !aconnector->dc_sink) > + return 0; > + > + mst_port = aconnector->port; > + mst_mgr = &aconnector->mst_port->mst_mgr; > + > + if (!crtc_state->connectors_changed && !crtc_state->mode_changed) > + return 0; > + > + if (!state->duplicated) { > + color_depth = convert_color_depth_from_display_info(connector, conn_state); > + bpp = convert_dc_color_depth_into_bpc(color_depth) * 3; Minor nitpick here - this is converting from a numerical bpc to a DC enum and then from the DC enum back to a numerical bpc. Almost seems like we should just have a helper to get the numerical bpc. Logic seems fine though. > + clock = adjusted_mode->clock; > + dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp); > + } > + dm_new_connector_state->vcpi_slots = drm_dp_atomic_find_vcpi_slots(state, > + mst_mgr, > + mst_port, > + dm_new_connector_state->pbn); > + if (dm_new_connector_state->vcpi_slots < 0) { > + DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n", (int)dm_new_connector_state->vcpi_slots); > + return dm_new_connector_state->vcpi_slots; > + } > return 0; > } > > @@ -7651,6 +7707,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, > if (ret) > goto fail; > > + /* Perform validation of MST topology in the state*/ > + ret = drm_dp_mst_atomic_check(state); > + if (ret) > + goto fail; > + > if (state->legacy_cursor_update) { > /* > * This is a fast cursor update coming from the plane update > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > index c6fdebee7189..910c8598faf9 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > @@ -360,6 +360,8 @@ struct dm_connector_state { > bool freesync_enable; > bool freesync_capable; > uint8_t abm_level; > + uint64_t vcpi_slots; > + uint64_t pbn; > }; > > #define to_dm_connector_state(x)\ > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > index 11e5784aa62a..1b2cc85b4815 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > @@ -182,15 +182,20 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( > bool enable) > { > struct amdgpu_dm_connector *aconnector; > + struct drm_connector *connector; > + struct dm_connector_state *dm_conn_state; > struct drm_dp_mst_topology_mgr *mst_mgr; > struct drm_dp_mst_port *mst_port; > - int slots = 0; > bool ret; > - int clock; > - int bpp = 0; > - int pbn = 0; > > aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; > + /* Accessing the connector state is required for vcpi_slots allocation > + * and directly relies on behaviour in commit check > + * that blocks before commit guaranteeing that the state > + * is not gonna be swapped while still in use in commit tail */ > + > + dm_conn_state = to_dm_connector_state(aconnector->base.state); > + > > if (!aconnector || !aconnector->mst_port) > return false; > @@ -203,42 +208,10 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( > mst_port = aconnector->port; > > if (enable) { > - clock = stream->timing.pix_clk_100hz / 10; > - > - switch (stream->timing.display_color_depth) { > - > - case COLOR_DEPTH_666: > - bpp = 6; > - break; > - case COLOR_DEPTH_888: > - bpp = 8; > - break; > - case COLOR_DEPTH_101010: > - bpp = 10; > - break; > - case COLOR_DEPTH_121212: > - bpp = 12; > - break; > - case COLOR_DEPTH_141414: > - bpp = 14; > - break; > - case COLOR_DEPTH_161616: > - bpp = 16; > - break; > - default: > - ASSERT(bpp != 0); > - break; > - } > - > - bpp = bpp * 3; > - > - /* TODO need to know link rate */ > - > - pbn = drm_dp_calc_pbn_mode(clock, bpp); > - > - slots = drm_dp_find_vcpi_slots(mst_mgr, pbn); > - ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, slots); > > + ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, > + dm_conn_state->pbn, > + dm_conn_state->vcpi_slots); > if (!ret) > return false; > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index 779d0b60cac9..1a17ea1b42e0 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -251,10 +251,42 @@ dm_mst_atomic_best_encoder(struct drm_connector *connector, > return &to_amdgpu_dm_connector(connector)->mst_encoder->base; > } > > +static int dm_dp_mst_atomic_check(struct drm_connector *connector, > + struct drm_atomic_state *state) > +{ > + struct drm_connector_state *new_conn_state = > + drm_atomic_get_new_connector_state(state, connector); > + struct drm_connector_state *old_conn_state = > + drm_atomic_get_old_connector_state(state, connector); > + struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); > + struct drm_crtc_state *new_crtc_state; > + struct drm_dp_mst_topology_mgr *mst_mgr; > + struct drm_dp_mst_port *mst_port; > + > + mst_port = aconnector->port; > + mst_mgr = &aconnector->mst_port->mst_mgr; > + > + if (!old_conn_state->crtc) > + return 0; > + > + if (new_conn_state->crtc) { > + new_crtc_state = drm_atomic_get_old_crtc_state(state, new_conn_state->crtc); > + if (!new_crtc_state || > + !drm_atomic_crtc_needs_modeset(new_crtc_state) || > + new_crtc_state->enable) > + return 0; > + } > + > + return drm_dp_atomic_release_vcpi_slots(state, > + mst_mgr, > + mst_port); > +} > + > static const struct drm_connector_helper_funcs dm_dp_mst_connector_helper_funcs = { > .get_modes = dm_dp_mst_get_modes, > .mode_valid = amdgpu_dm_connector_mode_valid, > .atomic_best_encoder = dm_mst_atomic_best_encoder, > + .atomic_check = dm_dp_mst_atomic_check, > }; > > static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder) >
On 31.10.2019 9:16, Kazlauskas, Nicholas wrote: > On 2019-10-30 3:24 p.m., mikita.lipski@amd.com wrote: >> From: Mikita Lipski <mikita.lipski@amd.com> >> >> - Adding encoder atomic check to find vcpi slots for a connector >> - Using DRM helper functions to calculate PBN >> - Adding connector atomic check to release vcpi slots if connector >> loses CRTC >> - Calculate PBN and VCPI slots only once during atomic >> check and store them on crtc_state to eliminate >> redundant calculation >> - Call drm_dp_mst_atomic_check to verify validity of MST topology >> during state atomic check >> >> v2: squashed previous 3 separate patches, removed DSC PBN calculation, >> and added PBN and VCPI slots properties to amdgpu connector >> >> v3: >> - moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state >> - updates stream's vcpi_slots and pbn on commit >> - separated patch from the DSC MST series >> >> v4: >> - set vcpi_slots and pbn properties to dm_connector_state >> - copy porperties from connector state on to crtc state >> >> v5: >> - keep the pbn and vcpi values only on connnector state >> - added a void pointer to the stream state instead on two ints, >> because dc_stream_state is OS agnostic. Pointer points to the >> current dm_connector_state. >> >> v6: >> - Remove new param from stream >> >> v7: >> - Fix error with using max capable bpc >> >> Cc: Jun Lei <Jun.Lei@amd.com> >> Cc: Jerry Zuo <Jerry.Zuo@amd.com> >> Cc: Harry Wentland <harry.wentland@amd.com> >> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> >> Cc: Lyude Paul <lyude@redhat.com> >> Signed-off-by: Mikita Lipski <mikita.lipski@amd.com> > Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> > > You might want to verify that this still works as you expect when > changing "max bpc" on an MST display. > > My understanding is that it'd still force a new modeset before encoder > atomic check is called so you *should* have the correct bpc value during > MST calculations. Thanks. It does still works with MST even if you change the mode to the lower resolution. The encoder atomic check is called during drm_atomic_helper_check_modeset so new modeset is already forced then. >> --- >> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 65 ++++++++++++++++++- >> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 + >> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 ++++----------- >> .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 32 +++++++++ >> 4 files changed, 109 insertions(+), 41 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> index 48f5b43e2698..d75726013436 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector) >> state->underscan_hborder = 0; >> state->underscan_vborder = 0; >> state->base.max_requested_bpc = 8; >> - >> + state->vcpi_slots = 0; >> + state->pbn = 0; >> if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) >> state->abm_level = amdgpu_dm_abm_level; >> >> @@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct drm_connector *connector) >> new_state->underscan_enable = state->underscan_enable; >> new_state->underscan_hborder = state->underscan_hborder; >> new_state->underscan_vborder = state->underscan_vborder; >> - >> + new_state->vcpi_slots = state->vcpi_slots; >> + new_state->pbn = state->pbn; >> return &new_state->base; >> } >> >> @@ -4606,10 +4608,64 @@ static void dm_encoder_helper_disable(struct drm_encoder *encoder) >> >> } >> >> +static int convert_dc_color_depth_into_bpc (enum dc_color_depth display_color_depth) >> +{ >> + switch (display_color_depth) { >> + case COLOR_DEPTH_666: >> + return 6; >> + case COLOR_DEPTH_888: >> + return 8; >> + case COLOR_DEPTH_101010: >> + return 10; >> + case COLOR_DEPTH_121212: >> + return 12; >> + case COLOR_DEPTH_141414: >> + return 14; >> + case COLOR_DEPTH_161616: >> + return 16; >> + default: >> + break; >> + } >> + return 0; >> +} >> + >> static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, >> struct drm_crtc_state *crtc_state, >> struct drm_connector_state *conn_state) >> { >> + struct drm_atomic_state *state = crtc_state->state; >> + struct drm_connector *connector = conn_state->connector; >> + struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); >> + struct dm_connector_state *dm_new_connector_state = to_dm_connector_state(conn_state); >> + const struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; >> + struct drm_dp_mst_topology_mgr *mst_mgr; >> + struct drm_dp_mst_port *mst_port; >> + enum dc_color_depth color_depth; >> + int clock, bpp = 0; >> + >> + if (!aconnector->port || !aconnector->dc_sink) >> + return 0; >> + >> + mst_port = aconnector->port; >> + mst_mgr = &aconnector->mst_port->mst_mgr; >> + >> + if (!crtc_state->connectors_changed && !crtc_state->mode_changed) >> + return 0; >> + >> + if (!state->duplicated) { >> + color_depth = convert_color_depth_from_display_info(connector, conn_state); >> + bpp = convert_dc_color_depth_into_bpc(color_depth) * 3; > Minor nitpick here - this is converting from a numerical bpc to a DC > enum and then from the DC enum back to a numerical bpc. Almost seems > like we should just have a helper to get the numerical bpc. > > Logic seems fine though. It does seem redundant, but by calling convert_color_depth_from_display_info we are ensuring to choose the min between max display bpc and max requested bpc. Plus it does some other checks to get the right bpc. > >> + clock = adjusted_mode->clock; >> + dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp); >> + } >> + dm_new_connector_state->vcpi_slots = drm_dp_atomic_find_vcpi_slots(state, >> + mst_mgr, >> + mst_port, >> + dm_new_connector_state->pbn); >> + if (dm_new_connector_state->vcpi_slots < 0) { >> + DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n", (int)dm_new_connector_state->vcpi_slots); >> + return dm_new_connector_state->vcpi_slots; >> + } >> return 0; >> } >> >> @@ -7651,6 +7707,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, >> if (ret) >> goto fail; >> >> + /* Perform validation of MST topology in the state*/ >> + ret = drm_dp_mst_atomic_check(state); >> + if (ret) >> + goto fail; >> + >> if (state->legacy_cursor_update) { >> /* >> * This is a fast cursor update coming from the plane update >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h >> index c6fdebee7189..910c8598faf9 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h >> @@ -360,6 +360,8 @@ struct dm_connector_state { >> bool freesync_enable; >> bool freesync_capable; >> uint8_t abm_level; >> + uint64_t vcpi_slots; >> + uint64_t pbn; >> }; >> >> #define to_dm_connector_state(x)\ >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c >> index 11e5784aa62a..1b2cc85b4815 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c >> @@ -182,15 +182,20 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( >> bool enable) >> { >> struct amdgpu_dm_connector *aconnector; >> + struct drm_connector *connector; >> + struct dm_connector_state *dm_conn_state; >> struct drm_dp_mst_topology_mgr *mst_mgr; >> struct drm_dp_mst_port *mst_port; >> - int slots = 0; >> bool ret; >> - int clock; >> - int bpp = 0; >> - int pbn = 0; >> >> aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; >> + /* Accessing the connector state is required for vcpi_slots allocation >> + * and directly relies on behaviour in commit check >> + * that blocks before commit guaranteeing that the state >> + * is not gonna be swapped while still in use in commit tail */ >> + >> + dm_conn_state = to_dm_connector_state(aconnector->base.state); >> + >> >> if (!aconnector || !aconnector->mst_port) >> return false; >> @@ -203,42 +208,10 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( >> mst_port = aconnector->port; >> >> if (enable) { >> - clock = stream->timing.pix_clk_100hz / 10; >> - >> - switch (stream->timing.display_color_depth) { >> - >> - case COLOR_DEPTH_666: >> - bpp = 6; >> - break; >> - case COLOR_DEPTH_888: >> - bpp = 8; >> - break; >> - case COLOR_DEPTH_101010: >> - bpp = 10; >> - break; >> - case COLOR_DEPTH_121212: >> - bpp = 12; >> - break; >> - case COLOR_DEPTH_141414: >> - bpp = 14; >> - break; >> - case COLOR_DEPTH_161616: >> - bpp = 16; >> - break; >> - default: >> - ASSERT(bpp != 0); >> - break; >> - } >> - >> - bpp = bpp * 3; >> - >> - /* TODO need to know link rate */ >> - >> - pbn = drm_dp_calc_pbn_mode(clock, bpp); >> - >> - slots = drm_dp_find_vcpi_slots(mst_mgr, pbn); >> - ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, slots); >> >> + ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, >> + dm_conn_state->pbn, >> + dm_conn_state->vcpi_slots); >> if (!ret) >> return false; >> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c >> index 779d0b60cac9..1a17ea1b42e0 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c >> @@ -251,10 +251,42 @@ dm_mst_atomic_best_encoder(struct drm_connector *connector, >> return &to_amdgpu_dm_connector(connector)->mst_encoder->base; >> } >> >> +static int dm_dp_mst_atomic_check(struct drm_connector *connector, >> + struct drm_atomic_state *state) >> +{ >> + struct drm_connector_state *new_conn_state = >> + drm_atomic_get_new_connector_state(state, connector); >> + struct drm_connector_state *old_conn_state = >> + drm_atomic_get_old_connector_state(state, connector); >> + struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); >> + struct drm_crtc_state *new_crtc_state; >> + struct drm_dp_mst_topology_mgr *mst_mgr; >> + struct drm_dp_mst_port *mst_port; >> + >> + mst_port = aconnector->port; >> + mst_mgr = &aconnector->mst_port->mst_mgr; >> + >> + if (!old_conn_state->crtc) >> + return 0; >> + >> + if (new_conn_state->crtc) { >> + new_crtc_state = drm_atomic_get_old_crtc_state(state, new_conn_state->crtc); >> + if (!new_crtc_state || >> + !drm_atomic_crtc_needs_modeset(new_crtc_state) || >> + new_crtc_state->enable) >> + return 0; >> + } >> + >> + return drm_dp_atomic_release_vcpi_slots(state, >> + mst_mgr, >> + mst_port); >> +} >> + >> static const struct drm_connector_helper_funcs dm_dp_mst_connector_helper_funcs = { >> .get_modes = dm_dp_mst_get_modes, >> .mode_valid = amdgpu_dm_connector_mode_valid, >> .atomic_best_encoder = dm_mst_atomic_best_encoder, >> + .atomic_check = dm_dp_mst_atomic_check, >> }; >> >> static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder) >>
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 48f5b43e2698..d75726013436 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector) state->underscan_hborder = 0; state->underscan_vborder = 0; state->base.max_requested_bpc = 8; - + state->vcpi_slots = 0; + state->pbn = 0; if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) state->abm_level = amdgpu_dm_abm_level; @@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct drm_connector *connector) new_state->underscan_enable = state->underscan_enable; new_state->underscan_hborder = state->underscan_hborder; new_state->underscan_vborder = state->underscan_vborder; - + new_state->vcpi_slots = state->vcpi_slots; + new_state->pbn = state->pbn; return &new_state->base; } @@ -4606,10 +4608,64 @@ static void dm_encoder_helper_disable(struct drm_encoder *encoder) } +static int convert_dc_color_depth_into_bpc (enum dc_color_depth display_color_depth) +{ + switch (display_color_depth) { + case COLOR_DEPTH_666: + return 6; + case COLOR_DEPTH_888: + return 8; + case COLOR_DEPTH_101010: + return 10; + case COLOR_DEPTH_121212: + return 12; + case COLOR_DEPTH_141414: + return 14; + case COLOR_DEPTH_161616: + return 16; + default: + break; + } + return 0; +} + static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { + struct drm_atomic_state *state = crtc_state->state; + struct drm_connector *connector = conn_state->connector; + struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); + struct dm_connector_state *dm_new_connector_state = to_dm_connector_state(conn_state); + const struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; + struct drm_dp_mst_topology_mgr *mst_mgr; + struct drm_dp_mst_port *mst_port; + enum dc_color_depth color_depth; + int clock, bpp = 0; + + if (!aconnector->port || !aconnector->dc_sink) + return 0; + + mst_port = aconnector->port; + mst_mgr = &aconnector->mst_port->mst_mgr; + + if (!crtc_state->connectors_changed && !crtc_state->mode_changed) + return 0; + + if (!state->duplicated) { + color_depth = convert_color_depth_from_display_info(connector, conn_state); + bpp = convert_dc_color_depth_into_bpc(color_depth) * 3; + clock = adjusted_mode->clock; + dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp); + } + dm_new_connector_state->vcpi_slots = drm_dp_atomic_find_vcpi_slots(state, + mst_mgr, + mst_port, + dm_new_connector_state->pbn); + if (dm_new_connector_state->vcpi_slots < 0) { + DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n", (int)dm_new_connector_state->vcpi_slots); + return dm_new_connector_state->vcpi_slots; + } return 0; } @@ -7651,6 +7707,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, if (ret) goto fail; + /* Perform validation of MST topology in the state*/ + ret = drm_dp_mst_atomic_check(state); + if (ret) + goto fail; + if (state->legacy_cursor_update) { /* * This is a fast cursor update coming from the plane update diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index c6fdebee7189..910c8598faf9 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -360,6 +360,8 @@ struct dm_connector_state { bool freesync_enable; bool freesync_capable; uint8_t abm_level; + uint64_t vcpi_slots; + uint64_t pbn; }; #define to_dm_connector_state(x)\ diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 11e5784aa62a..1b2cc85b4815 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -182,15 +182,20 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( bool enable) { struct amdgpu_dm_connector *aconnector; + struct drm_connector *connector; + struct dm_connector_state *dm_conn_state; struct drm_dp_mst_topology_mgr *mst_mgr; struct drm_dp_mst_port *mst_port; - int slots = 0; bool ret; - int clock; - int bpp = 0; - int pbn = 0; aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; + /* Accessing the connector state is required for vcpi_slots allocation + * and directly relies on behaviour in commit check + * that blocks before commit guaranteeing that the state + * is not gonna be swapped while still in use in commit tail */ + + dm_conn_state = to_dm_connector_state(aconnector->base.state); + if (!aconnector || !aconnector->mst_port) return false; @@ -203,42 +208,10 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( mst_port = aconnector->port; if (enable) { - clock = stream->timing.pix_clk_100hz / 10; - - switch (stream->timing.display_color_depth) { - - case COLOR_DEPTH_666: - bpp = 6; - break; - case COLOR_DEPTH_888: - bpp = 8; - break; - case COLOR_DEPTH_101010: - bpp = 10; - break; - case COLOR_DEPTH_121212: - bpp = 12; - break; - case COLOR_DEPTH_141414: - bpp = 14; - break; - case COLOR_DEPTH_161616: - bpp = 16; - break; - default: - ASSERT(bpp != 0); - break; - } - - bpp = bpp * 3; - - /* TODO need to know link rate */ - - pbn = drm_dp_calc_pbn_mode(clock, bpp); - - slots = drm_dp_find_vcpi_slots(mst_mgr, pbn); - ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, slots); + ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, + dm_conn_state->pbn, + dm_conn_state->vcpi_slots); if (!ret) return false; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 779d0b60cac9..1a17ea1b42e0 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -251,10 +251,42 @@ dm_mst_atomic_best_encoder(struct drm_connector *connector, return &to_amdgpu_dm_connector(connector)->mst_encoder->base; } +static int dm_dp_mst_atomic_check(struct drm_connector *connector, + struct drm_atomic_state *state) +{ + struct drm_connector_state *new_conn_state = + drm_atomic_get_new_connector_state(state, connector); + struct drm_connector_state *old_conn_state = + drm_atomic_get_old_connector_state(state, connector); + struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); + struct drm_crtc_state *new_crtc_state; + struct drm_dp_mst_topology_mgr *mst_mgr; + struct drm_dp_mst_port *mst_port; + + mst_port = aconnector->port; + mst_mgr = &aconnector->mst_port->mst_mgr; + + if (!old_conn_state->crtc) + return 0; + + if (new_conn_state->crtc) { + new_crtc_state = drm_atomic_get_old_crtc_state(state, new_conn_state->crtc); + if (!new_crtc_state || + !drm_atomic_crtc_needs_modeset(new_crtc_state) || + new_crtc_state->enable) + return 0; + } + + return drm_dp_atomic_release_vcpi_slots(state, + mst_mgr, + mst_port); +} + static const struct drm_connector_helper_funcs dm_dp_mst_connector_helper_funcs = { .get_modes = dm_dp_mst_get_modes, .mode_valid = amdgpu_dm_connector_mode_valid, .atomic_best_encoder = dm_mst_atomic_best_encoder, + .atomic_check = dm_dp_mst_atomic_check, }; static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder)