MST has been broken on amdgpu after a refactor in drm_dp_mst code that was aligning drm_dp_mst more closely with the atomic model.
The gitlab issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
This series fixes it.
It can be found at https://gitlab.freedesktop.org/hwentland/linux/-/tree/mst_regression
A stable-6.1.y rebase can be found at https://gitlab.freedesktop.org/hwentland/linux/-/tree/mst_regression_6.1
Lyude Paul (1): drm/amdgpu/display/mst: Fix mst_state->pbn_div and slot count assignments
Wayne Lin (6): drm/amdgpu/display/mst: limit payload to be updated one by one drm/amdgpu/display/mst: update mst_mgr relevant variable when long HPD drm/drm_print: correct format problem drm/display/dp_mst: Correct the kref of port. drm/amdgpu/display/mst: adjust the naming of mst_port and port of aconnector drm/amdgpu/display/mst: adjust the logic in 2nd phase of updating payload
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 4 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 48 +++++++++--- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 +- .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 2 +- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 16 ++-- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 76 +++++++++++++------ .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 53 ++++++------- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 14 +++- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +- include/drm/drm_print.h | 2 +- 10 files changed, 143 insertions(+), 80 deletions(-)
-- 2.39.0
From: Lyude Paul lyude@redhat.com
Looks like I made a pretty big mistake here without noticing: it seems when I moved the assignments of mst_state->pbn_div I completely missed the fact that the reason for us calling drm_dp_mst_update_slots() earlier was to account for the fact that we need to call this function using info from the root MST connector, instead of just trying to do this from each MST encoder's atomic check function. Otherwise, we end up filling out all of DC's link information with zeroes.
So, let's restore that and hopefully fix this DSC regression.
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/2171 Signed-off-by: Lyude Paul lyude@redhat.com Signed-off-by: Harry Wentland harry.wentland@amd.com Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the atomic state") Cc: stable@vger.kernel.org # 6.1 Acked-by: Harry Wentland harry.wentland@amd.com --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 +++++++++++++++++++ .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 ---- 2 files changed, 24 insertions(+), 5 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 22aadbad58d3..4c5b8793b8af 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -9643,6 +9643,8 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, struct drm_connector_state *old_con_state, *new_con_state; struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state, *new_crtc_state; + struct drm_dp_mst_topology_mgr *mgr; + struct drm_dp_mst_topology_state *mst_state; struct drm_plane *plane; struct drm_plane_state *old_plane_state, *new_plane_state; enum dc_status status; @@ -9898,6 +9900,28 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, lock_and_validation_needed = true; }
+#if defined(CONFIG_DRM_AMD_DC_DCN) + /* set the slot info for each mst_state based on the link encoding format */ + for_each_new_mst_mgr_in_state(state, mgr, mst_state, i) { + struct amdgpu_dm_connector *aconnector; + struct drm_connector *connector; + struct drm_connector_list_iter iter; + u8 link_coding_cap; + + drm_connector_list_iter_begin(dev, &iter); + drm_for_each_connector_iter(connector, &iter) { + if (connector->index == mst_state->mgr->conn_base_id) { + aconnector = to_amdgpu_dm_connector(connector); + link_coding_cap = dc_link_dp_mst_decide_link_encoding_format(aconnector->dc_link); + drm_dp_mst_update_slots(mst_state, link_coding_cap); + + break; + } + } + drm_connector_list_iter_end(&iter); + } +#endif + /** * Streams and planes are reset when there are changes that affect * bandwidth. Anything that affects bandwidth needs to go through 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 5fa9bab95038..e8d14ab0953a 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 @@ -927,11 +927,6 @@ static int compute_mst_dsc_configs_for_link(struct drm_atomic_state *state, if (IS_ERR(mst_state)) return PTR_ERR(mst_state);
- mst_state->pbn_div = dm_mst_get_pbn_divider(dc_link); -#if defined(CONFIG_DRM_AMD_DC_DCN) - drm_dp_mst_update_slots(mst_state, dc_link_dp_mst_decide_link_encoding_format(dc_link)); -#endif - /* Set up params */ for (i = 0; i < dc_state->stream_count; i++) { struct dc_dsc_policy dsc_policy = {0};
From: Wayne Lin Wayne.Lin@amd.com
[Why] amdgpu expects to update payload table for one stream one time by calling dm_helpers_dp_mst_write_payload_allocation_table(). Currently, it get modified to try to update HW payload table at once by referring mst_state.
[How] This is just a quick workaround. Should find way to remove the temporary struct dc_dp_mst_stream_allocation_table later if set struct link_mst_stream_allocatio directly is possible.
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/2171 Signed-off-by: Wayne Lin Wayne.Lin@amd.com Signed-off-by: Harry Wentland harry.wentland@amd.com Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the atomic state") Cc: stable@vger.kernel.org # 6.1 Acked-by: Harry Wentland harry.wentland@amd.com --- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 ++++++++++++++----- 1 file changed, 39 insertions(+), 12 deletions(-)
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 6994c9a1ed85..5cff56bb8f56 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 @@ -120,23 +120,50 @@ enum dc_edid_status dm_helpers_parse_edid_caps( }
static void -fill_dc_mst_payload_table_from_drm(struct drm_dp_mst_topology_state *mst_state, - struct amdgpu_dm_connector *aconnector, +fill_dc_mst_payload_table_from_drm(struct dc_link *link, + bool enable, + struct drm_dp_mst_atomic_payload *target_payload, struct dc_dp_mst_stream_allocation_table *table) { struct dc_dp_mst_stream_allocation_table new_table = { 0 }; struct dc_dp_mst_stream_allocation *sa; - struct drm_dp_mst_atomic_payload *payload; + struct link_mst_stream_allocation_table copy_of_link_table = + link->mst_stream_alloc_table; + + int i; + int current_hw_table_stream_cnt = copy_of_link_table.stream_count; + struct link_mst_stream_allocation *dc_alloc; + + /* TODO: refactor to set link->mst_stream_alloc_table directly if possible.*/ + if (enable) { + dc_alloc = + ©_of_link_table.stream_allocations[current_hw_table_stream_cnt]; + dc_alloc->vcp_id = target_payload->vcpi; + dc_alloc->slot_count = target_payload->time_slots; + } else { + for (i = 0; i < copy_of_link_table.stream_count; i++) { + dc_alloc = + ©_of_link_table.stream_allocations[i]; + + if (dc_alloc->vcp_id == target_payload->vcpi) { + dc_alloc->vcp_id = 0; + dc_alloc->slot_count = 0; + break; + } + } + ASSERT(i != copy_of_link_table.stream_count); + }
/* Fill payload info*/ - list_for_each_entry(payload, &mst_state->payloads, next) { - if (payload->delete) - continue; - - sa = &new_table.stream_allocations[new_table.stream_count]; - sa->slot_count = payload->time_slots; - sa->vcp_id = payload->vcpi; - new_table.stream_count++; + for (i = 0; i < MAX_CONTROLLER_NUM; i++) { + dc_alloc = + ©_of_link_table.stream_allocations[i]; + if (dc_alloc->vcp_id > 0 && dc_alloc->slot_count > 0) { + sa = &new_table.stream_allocations[new_table.stream_count]; + sa->slot_count = dc_alloc->slot_count; + sa->vcp_id = dc_alloc->vcp_id; + new_table.stream_count++; + } }
/* Overwrite the old table */ @@ -185,7 +212,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( * AUX message. The sequence is slot 1-63 allocated sequence for each * stream. AMD ASIC stream slot allocation should follow the same * sequence. copy DRM MST allocation to dc */ - fill_dc_mst_payload_table_from_drm(mst_state, aconnector, proposed_table); + fill_dc_mst_payload_table_from_drm(stream->link, enable, payload, proposed_table);
return true; }
From: Wayne Lin Wayne.Lin@amd.com
[Why & How] Now the vc_start_slot is controlled at drm side. When we service a long HPD, we still need to run dm_helpers_dp_mst_write_payload_allocation_table() to update drm mst_mgr's relevant variable. Otherwise, on the next plug-in, payload will get assigned with a wrong start slot.
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/2171 Signed-off-by: Wayne Lin Wayne.Lin@amd.com Signed-off-by: Harry Wentland harry.wentland@amd.com Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the atomic state") Cc: stable@vger.kernel.org # 6.1 Acked-by: Harry Wentland harry.wentland@amd.com --- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c index d9e490eca10f..bf5a31e2be8a 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c @@ -3999,10 +3999,13 @@ static enum dc_status deallocate_mst_payload(struct pipe_ctx *pipe_ctx) struct fixed31_32 avg_time_slots_per_mtp = dc_fixpt_from_int(0); int i; bool mst_mode = (link->type == dc_connection_mst_branch); + /* adjust for drm changes*/ + bool update_drm_mst_state = true; const struct link_hwss *link_hwss = get_link_hwss(link, &pipe_ctx->link_res); const struct dc_link_settings empty_link_settings = {0}; DC_LOGGER_INIT(link->ctx->logger);
+ /* deallocate_mst_payload is called before disable link. When mode or * disable/enable monitor, new stream is created which is not in link * stream[] yet. For this, payload is not allocated yet, so de-alloc @@ -4018,7 +4021,7 @@ static enum dc_status deallocate_mst_payload(struct pipe_ctx *pipe_ctx) &empty_link_settings, avg_time_slots_per_mtp);
- if (mst_mode) { + if (mst_mode || update_drm_mst_state) { /* when link is in mst mode, reply on mst manager to remove * payload */ @@ -4081,11 +4084,18 @@ static enum dc_status deallocate_mst_payload(struct pipe_ctx *pipe_ctx) stream->ctx, stream);
+ if (!update_drm_mst_state) + dm_helpers_dp_mst_send_payload_allocation( + stream->ctx, + stream, + false); + } + + if (update_drm_mst_state) dm_helpers_dp_mst_send_payload_allocation( stream->ctx, stream, false); - }
return DC_OK; }
From: Wayne Lin Wayne.Lin@amd.com
[why & how] __drm_dbg() parameter set format is wrong and not aligned with the format under CONFIG_DRM_USE_DYNAMIC_DEBUG is on. Fix it.
Signed-off-by: Wayne Lin Wayne.Lin@amd.com Signed-off-by: Harry Wentland harry.wentland@amd.com Acked-by: Harry Wentland harry.wentland@amd.com --- include/drm/drm_print.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index a44fb7ef257f..094ded23534c 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -521,7 +521,7 @@ __printf(1, 2) void __drm_err(const char *format, ...);
#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG) -#define __drm_dbg(fmt, ...) ___drm_dbg(NULL, fmt, ##__VA_ARGS__) +#define __drm_dbg(cat, fmt, ...) ___drm_dbg(NULL, cat, fmt, ##__VA_ARGS__) #else #define __drm_dbg(cat, fmt, ...) \ _dynamic_func_call_cls(cat, fmt, ___drm_dbg, \
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
Rule: 'Cc: stable@vger.kernel.org' or 'commit <sha1> upstream.' Subject: [PATCH 4/7] drm/drm_print: correct format problem Link: https://lore.kernel.org/stable/20230119235200.441386-5-harry.wentland%40amd....
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
From: Wayne Lin Wayne.Lin@amd.com
[why & how] We still need to refer to port while removing payload at commit_tail. we should keep the kref till then to release.
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/2171 Signed-off-by: Wayne Lin Wayne.Lin@amd.com Signed-off-by: Harry Wentland harry.wentland@amd.com Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the atomic state") Cc: stable@vger.kernel.org # 6.1 Acked-by: Harry Wentland harry.wentland@amd.com --- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index 51a46689cda7..4ca37261584a 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -3372,6 +3372,9 @@ void drm_dp_remove_payload(struct drm_dp_mst_topology_mgr *mgr,
mgr->payload_count--; mgr->next_start_slot -= payload->time_slots; + + if (payload->delete) + drm_dp_mst_put_port_malloc(payload->port); } EXPORT_SYMBOL(drm_dp_remove_payload);
@@ -4327,7 +4330,6 @@ int drm_dp_atomic_release_time_slots(struct drm_atomic_state *state,
drm_dbg_atomic(mgr->dev, "[MST PORT:%p] TU %d -> 0\n", port, payload->time_slots); if (!payload->delete) { - drm_dp_mst_put_port_malloc(port); payload->pbn = 0; payload->delete = true; topology_state->payload_mask &= ~BIT(payload->vcpi - 1);
From: Wayne Lin Wayne.Lin@amd.com
[why & how] The term (i.e. port & mst_port) that we used to use in amdgpu is a bit confusing. Rename them to mst_output_port & mst_root respectively.
Signed-off-by: Wayne Lin Wayne.Lin@amd.com Signed-off-by: Harry Wentland harry.wentland@amd.com Acked-by: Harry Wentland harry.wentland@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 4 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 +++++----- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 +- .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 2 +- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 16 +++---- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 19 ++++---- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 48 +++++++++---------- 7 files changed, 59 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index bf009de59710..c1ece3d2ea36 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -551,8 +551,8 @@ struct amdgpu_mst_connector {
struct drm_dp_mst_topology_mgr mst_mgr; struct amdgpu_dm_dp_aux dm_dp_aux; - struct drm_dp_mst_port *port; - struct amdgpu_connector *mst_port; + struct drm_dp_mst_port *mst_output_port; + struct amdgpu_connector *mst_root; bool is_mst_connector; struct amdgpu_encoder *mst_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 4c5b8793b8af..291dea18c658 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2197,7 +2197,7 @@ static void s3_handle_mst(struct drm_device *dev, bool suspend) drm_for_each_connector_iter(connector, &iter) { aconnector = to_amdgpu_dm_connector(connector); if (aconnector->dc_link->type != dc_connection_mst_branch || - aconnector->mst_port) + aconnector->mst_root) continue;
mgr = &aconnector->mst_mgr; @@ -6582,11 +6582,11 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, int clock, bpp = 0; bool is_y420 = false;
- if (!aconnector->port || !aconnector->dc_sink) + if (!aconnector->mst_output_port || !aconnector->dc_sink) return 0;
- mst_port = aconnector->port; - mst_mgr = &aconnector->mst_port->mst_mgr; + mst_port = aconnector->mst_output_port; + mst_mgr = &aconnector->mst_root->mst_mgr;
if (!crtc_state->connectors_changed && !crtc_state->mode_changed) return 0; @@ -6596,7 +6596,7 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, return PTR_ERR(mst_state);
if (!mst_state->pbn_div) - mst_state->pbn_div = dm_mst_get_pbn_divider(aconnector->mst_port->dc_link); + mst_state->pbn_div = dm_mst_get_pbn_divider(aconnector->mst_root->dc_link);
if (!state->duplicated) { int max_bpc = conn_state->max_requested_bpc; @@ -6642,7 +6642,7 @@ static int dm_update_mst_vcpi_slots_for_dsc(struct drm_atomic_state *state,
aconnector = to_amdgpu_dm_connector(connector);
- if (!aconnector->port) + if (!aconnector->mst_output_port) continue;
if (!new_con_state || !new_con_state->crtc) @@ -6682,7 +6682,7 @@ static int dm_update_mst_vcpi_slots_for_dsc(struct drm_atomic_state *state, dm_conn_state->pbn = pbn; dm_conn_state->vcpi_slots = slot_num;
- ret = drm_dp_mst_atomic_enable_dsc(state, aconnector->port, + ret = drm_dp_mst_atomic_enable_dsc(state, aconnector->mst_output_port, dm_conn_state->pbn, false); if (ret < 0) return ret; @@ -6690,7 +6690,7 @@ static int dm_update_mst_vcpi_slots_for_dsc(struct drm_atomic_state *state, continue; }
- vcpi = drm_dp_mst_atomic_enable_dsc(state, aconnector->port, pbn, true); + vcpi = drm_dp_mst_atomic_enable_dsc(state, aconnector->mst_output_port, pbn, true); if (vcpi < 0) return vcpi;
@@ -7104,7 +7104,7 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm, adev->mode_info.underscan_vborder_property, 0);
- if (!aconnector->mst_port) + if (!aconnector->mst_root) drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
/* This defaults to the max in the range, but we want 8bpc for non-edp. */ @@ -7122,7 +7122,7 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm, connector_type == DRM_MODE_CONNECTOR_eDP) { drm_connector_attach_hdr_output_metadata_property(&aconnector->base);
- if (!aconnector->mst_port) + if (!aconnector->mst_root) drm_connector_attach_vrr_capable_property(&aconnector->base);
#ifdef CONFIG_DRM_AMD_DC_HDCP @@ -9595,7 +9595,7 @@ static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct drm continue;
aconnector = to_amdgpu_dm_connector(connector); - if (!aconnector->port || !aconnector->mst_port) + if (!aconnector->mst_output_port || !aconnector->mst_root) aconnector = NULL; else break; @@ -9604,7 +9604,7 @@ static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct drm if (!aconnector) return 0;
- return drm_dp_mst_add_affected_dsc_crtcs(state, &aconnector->mst_port->mst_mgr); + return drm_dp_mst_add_affected_dsc_crtcs(state, &aconnector->mst_root->mst_mgr); } #endif
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 abbbb3813c1e..b440e2e0cfe0 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -604,8 +604,8 @@ struct amdgpu_dm_connector { /* DM only */ struct drm_dp_mst_topology_mgr mst_mgr; struct amdgpu_dm_dp_aux dm_dp_aux; - struct drm_dp_mst_port *port; - struct amdgpu_dm_connector *mst_port; + struct drm_dp_mst_port *mst_output_port; + struct amdgpu_dm_connector *mst_root; struct drm_dp_aux *dsc_aux; /* TODO see if we can merge with ddc_bus or make a dm_connector */ struct amdgpu_i2c_adapter *i2c; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c index 8873ecada27c..27711743c22c 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c @@ -344,7 +344,7 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) goto cleanup; }
- aux = (aconn->port) ? &aconn->port->aux : &aconn->dm_dp_aux.aux; + aux = (aconn->mst_output_port) ? &aconn->mst_output_port->aux : &aconn->dm_dp_aux.aux;
if (!aux) { DRM_DEBUG_DRIVER("No dp aux for amd connector\n"); diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c index 704860e6ba84..40a95fc0e4f8 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c @@ -1193,7 +1193,7 @@ static int dp_dsc_fec_support_show(struct seq_file *m, void *data) break; } dpcd_caps = aconnector->dc_link->dpcd_caps; - if (aconnector->port) { + if (aconnector->mst_output_port) { /* aconnector sets dsc_aux during get_modes call * if MST connector has it means it can either * enable DSC on the sink device or on MST branch @@ -1280,7 +1280,7 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf, mutex_lock(&aconnector->hpd_lock);
/* Don't support for mst end device*/ - if (aconnector->mst_port) { + if (aconnector->mst_root) { mutex_unlock(&aconnector->hpd_lock); return -EINVAL; } @@ -2539,13 +2539,13 @@ static int dp_is_mst_connector_show(struct seq_file *m, void *unused)
if (aconnector->mst_mgr.mst_state) { role = "root"; - } else if (aconnector->mst_port && - aconnector->mst_port->mst_mgr.mst_state) { + } else if (aconnector->mst_root && + aconnector->mst_root->mst_mgr.mst_state) {
role = "end";
- mgr = &aconnector->mst_port->mst_mgr; - port = aconnector->port; + mgr = &aconnector->mst_root->mst_mgr; + port = aconnector->mst_output_port;
drm_modeset_lock(&mgr->base.lock, NULL); if (port->pdt == DP_PEER_DEVICE_MST_BRANCHING && @@ -3392,12 +3392,12 @@ static int trigger_hpd_mst_set(void *data, u64 val) if (!aconnector->dc_link) continue;
- if (!aconnector->mst_port) + if (!aconnector->mst_root) continue;
link = aconnector->dc_link; dc_link_dp_receiver_power_ctrl(link, false); - drm_dp_mst_topology_mgr_set_mst(&aconnector->mst_port->mst_mgr, false); + drm_dp_mst_topology_mgr_set_mst(&aconnector->mst_root->mst_mgr, false); link->mst_stream_alloc_table.stream_count = 0; memset(link->mst_stream_alloc_table.stream_allocations, 0, sizeof(link->mst_stream_alloc_table.stream_allocations)); 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 5cff56bb8f56..7aff7eb13ea2 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 @@ -195,14 +195,14 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( * that blocks before commit guaranteeing that the state * is not gonna be swapped while still in use in commit tail */
- if (!aconnector || !aconnector->mst_port) + if (!aconnector || !aconnector->mst_root) return false;
- mst_mgr = &aconnector->mst_port->mst_mgr; + mst_mgr = &aconnector->mst_root->mst_mgr; mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
/* It's OK for this to fail */ - payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->port); + payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port); if (enable) drm_dp_add_payload_part1(mst_mgr, mst_state, payload); else @@ -247,10 +247,10 @@ enum act_return_status dm_helpers_dp_mst_poll_for_allocation_change_trigger(
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
- if (!aconnector || !aconnector->mst_port) + if (!aconnector || !aconnector->mst_root) return ACT_FAILED;
- mst_mgr = &aconnector->mst_port->mst_mgr; + mst_mgr = &aconnector->mst_root->mst_mgr;
if (!mst_mgr->mst_state) return ACT_FAILED; @@ -277,13 +277,14 @@ bool dm_helpers_dp_mst_send_payload_allocation(
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
- if (!aconnector || !aconnector->mst_port) + if (!aconnector || !aconnector->mst_root) return false;
- mst_mgr = &aconnector->mst_port->mst_mgr; + mst_mgr = &aconnector->mst_root->mst_mgr; mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
- payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->port); + payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port); + if (!enable) { set_flag = MST_CLEAR_ALLOCATED_PAYLOAD; clr_flag = MST_ALLOCATE_NEW_PAYLOAD; @@ -710,7 +711,7 @@ bool dm_helpers_dp_write_dsc_enable( aconnector->dsc_aux, stream, enable_dsc); #endif
- port = aconnector->port; + port = aconnector->mst_output_port;
if (enable) { if (port->passthrough_aux) { 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 e8d14ab0953a..bc94df02cb0b 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 @@ -134,7 +134,7 @@ dm_dp_mst_connector_destroy(struct drm_connector *connector) kfree(aconnector->edid);
drm_connector_cleanup(connector); - drm_dp_mst_put_port_malloc(aconnector->port); + drm_dp_mst_put_port_malloc(aconnector->mst_output_port); kfree(aconnector); }
@@ -146,7 +146,7 @@ amdgpu_dm_mst_connector_late_register(struct drm_connector *connector) int r;
r = drm_dp_mst_connector_late_register(connector, - amdgpu_dm_connector->port); + amdgpu_dm_connector->mst_output_port); if (r < 0) return r;
@@ -162,8 +162,8 @@ amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); - struct drm_dp_mst_port *port = aconnector->port; - struct amdgpu_dm_connector *root = aconnector->mst_port; + struct drm_dp_mst_port *port = aconnector->mst_output_port; + struct amdgpu_dm_connector *root = aconnector->mst_root; struct dc_link *dc_link = aconnector->dc_link; struct dc_sink *dc_sink = aconnector->dc_sink;
@@ -213,7 +213,7 @@ bool needs_dsc_aux_workaround(struct dc_link *link) static bool validate_dsc_caps_on_connector(struct amdgpu_dm_connector *aconnector) { struct dc_sink *dc_sink = aconnector->dc_sink; - struct drm_dp_mst_port *port = aconnector->port; + struct drm_dp_mst_port *port = aconnector->mst_output_port; u8 dsc_caps[16] = { 0 }; u8 dsc_branch_dec_caps_raw[3] = { 0 }; // DSC branch decoder caps 0xA0 ~ 0xA2 u8 *dsc_branch_dec_caps = NULL; @@ -231,7 +231,7 @@ static bool validate_dsc_caps_on_connector(struct amdgpu_dm_connector *aconnecto */ if (!aconnector->dsc_aux && !port->parent->port_parent && needs_dsc_aux_workaround(aconnector->dc_link)) - aconnector->dsc_aux = &aconnector->mst_port->dm_dp_aux.aux; + aconnector->dsc_aux = &aconnector->mst_root->dm_dp_aux.aux;
if (!aconnector->dsc_aux) return false; @@ -281,7 +281,7 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
if (!aconnector->edid) { struct edid *edid; - edid = drm_dp_mst_get_edid(connector, &aconnector->mst_port->mst_mgr, aconnector->port); + edid = drm_dp_mst_get_edid(connector, &aconnector->mst_root->mst_mgr, aconnector->mst_output_port);
if (!edid) { amdgpu_dm_set_mst_status(&aconnector->mst_status, @@ -410,15 +410,15 @@ dm_dp_mst_detect(struct drm_connector *connector, struct drm_modeset_acquire_ctx *ctx, bool force) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); - struct amdgpu_dm_connector *master = aconnector->mst_port; - struct drm_dp_mst_port *port = aconnector->port; + struct amdgpu_dm_connector *master = aconnector->mst_root; + struct drm_dp_mst_port *port = aconnector->mst_output_port; int connection_status;
if (drm_connector_is_unregistered(connector)) return connector_status_disconnected;
connection_status = drm_dp_mst_detect_port(connector, ctx, &master->mst_mgr, - aconnector->port); + aconnector->mst_output_port);
if (port->pdt != DP_PEER_DEVICE_NONE && !port->dpcd_rev) { uint8_t dpcd_rev; @@ -475,8 +475,8 @@ static int dm_dp_mst_atomic_check(struct drm_connector *connector, struct drm_atomic_state *state) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); - struct drm_dp_mst_topology_mgr *mst_mgr = &aconnector->mst_port->mst_mgr; - struct drm_dp_mst_port *mst_port = aconnector->port; + struct drm_dp_mst_topology_mgr *mst_mgr = &aconnector->mst_root->mst_mgr; + struct drm_dp_mst_port *mst_port = aconnector->mst_output_port;
return drm_dp_atomic_release_time_slots(state, mst_mgr, mst_port); } @@ -538,8 +538,8 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr, return NULL;
connector = &aconnector->base; - aconnector->port = port; - aconnector->mst_port = master; + aconnector->mst_output_port = port; + aconnector->mst_root = master; amdgpu_dm_set_mst_status(&aconnector->mst_status, MST_PROBE, true);
@@ -940,7 +940,7 @@ static int compute_mst_dsc_configs_for_link(struct drm_atomic_state *state, if (!aconnector) continue;
- if (!aconnector->port) + if (!aconnector->mst_output_port) continue;
stream->timing.flags.DSC = 0; @@ -948,7 +948,7 @@ static int compute_mst_dsc_configs_for_link(struct drm_atomic_state *state, params[count].timing = &stream->timing; params[count].sink = stream->sink; params[count].aconnector = aconnector; - params[count].port = aconnector->port; + params[count].port = aconnector->mst_output_port; params[count].clock_force_enable = aconnector->dsc_settings.dsc_force_enable; if (params[count].clock_force_enable == DSC_CLK_FORCE_ENABLE) debugfs_overwrite = true; @@ -1157,7 +1157,7 @@ int compute_mst_dsc_configs_for_state(struct drm_atomic_state *state,
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
- if (!aconnector || !aconnector->dc_sink || !aconnector->port) + if (!aconnector || !aconnector->dc_sink || !aconnector->mst_output_port) continue;
if (!aconnector->dc_sink->dsc_caps.dsc_dec_caps.is_dsc_supported) @@ -1172,7 +1172,7 @@ int compute_mst_dsc_configs_for_state(struct drm_atomic_state *state, if (!is_dsc_need_re_compute(state, dc_state, stream->link)) continue;
- mst_mgr = aconnector->port->mgr; + mst_mgr = aconnector->mst_output_port->mgr; ret = compute_mst_dsc_configs_for_link(state, dc_state, stream->link, vars, mst_mgr, &link_vars_start_index); if (ret != 0) @@ -1218,7 +1218,7 @@ static int pre_compute_mst_dsc_configs_for_state(struct drm_atomic_state *state,
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
- if (!aconnector || !aconnector->dc_sink || !aconnector->port) + if (!aconnector || !aconnector->dc_sink || !aconnector->mst_output_port) continue;
if (!aconnector->dc_sink->dsc_caps.dsc_dec_caps.is_dsc_supported) @@ -1230,7 +1230,7 @@ static int pre_compute_mst_dsc_configs_for_state(struct drm_atomic_state *state, if (!is_dsc_need_re_compute(state, dc_state, stream->link)) continue;
- mst_mgr = aconnector->port->mgr; + mst_mgr = aconnector->mst_output_port->mgr; ret = compute_mst_dsc_configs_for_link(state, dc_state, stream->link, vars, mst_mgr, &link_vars_start_index); if (ret != 0) @@ -1445,8 +1445,8 @@ enum dc_status dm_dp_mst_is_port_support_mode( * with DSC enabled. */ if (is_dsc_common_config_possible(stream, &bw_range) && - aconnector->port->passthrough_aux) { - mst_mgr = aconnector->port->mgr; + aconnector->mst_output_port->passthrough_aux) { + mst_mgr = aconnector->mst_output_port->mgr; mutex_lock(&mst_mgr->lock);
cur_link_settings = stream->link->verified_link_cap; @@ -1454,7 +1454,7 @@ enum dc_status dm_dp_mst_is_port_support_mode( upper_link_bw_in_kbps = dc_link_bandwidth_kbps(aconnector->dc_link, &cur_link_settings ); - down_link_bw_in_kbps = kbps_from_pbn(aconnector->port->full_pbn); + down_link_bw_in_kbps = kbps_from_pbn(aconnector->mst_output_port->full_pbn);
/* pick the bottleneck */ end_to_end_bw_in_kbps = min(upper_link_bw_in_kbps, @@ -1478,7 +1478,7 @@ enum dc_status dm_dp_mst_is_port_support_mode( bpp = convert_dc_color_depth_into_bpc(stream->timing.display_color_depth) * 3; pbn = drm_dp_calc_pbn_mode(stream->timing.pix_clk_100hz / 10, bpp, false);
- if (pbn > aconnector->port->full_pbn) + if (pbn > aconnector->mst_output_port->full_pbn) return DC_FAIL_BANDWIDTH_VALIDATE; #if defined(CONFIG_DRM_AMD_DC_DCN) }
From: Wayne Lin Wayne.Lin@amd.com
[why & how] adjust the coding in dm_helpers_dp_mst_send_payload_allocation() for reading easily.
Signed-off-by: Wayne Lin Wayne.Lin@amd.com Signed-off-by: Harry Wentland harry.wentland@amd.com Acked-by: Harry Wentland harry.wentland@amd.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
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 7aff7eb13ea2..fba6a57158cf 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 @@ -274,6 +274,7 @@ bool dm_helpers_dp_mst_send_payload_allocation( struct drm_dp_mst_atomic_payload *payload; enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD; enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD; + int ret = 0;
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
@@ -290,7 +291,10 @@ bool dm_helpers_dp_mst_send_payload_allocation( clr_flag = MST_ALLOCATE_NEW_PAYLOAD; }
- if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload)) { + if (enable) + ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload); + + if (ret) { amdgpu_dm_set_mst_status(&aconnector->mst_status, set_flag, false); } else {
For the whole series:
Reviewed-by: Lyude Paul lyude@redhat.com
So glad to have this fixed finally ♥
On Thu, 2023-01-19 at 18:51 -0500, Harry Wentland wrote:
MST has been broken on amdgpu after a refactor in drm_dp_mst code that was aligning drm_dp_mst more closely with the atomic model.
The gitlab issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
This series fixes it.
It can be found at https://gitlab.freedesktop.org/hwentland/linux/-/tree/mst_regression
A stable-6.1.y rebase can be found at https://gitlab.freedesktop.org/hwentland/linux/-/tree/mst_regression_6.1
Lyude Paul (1): drm/amdgpu/display/mst: Fix mst_state->pbn_div and slot count assignments
Wayne Lin (6): drm/amdgpu/display/mst: limit payload to be updated one by one drm/amdgpu/display/mst: update mst_mgr relevant variable when long HPD drm/drm_print: correct format problem drm/display/dp_mst: Correct the kref of port. drm/amdgpu/display/mst: adjust the naming of mst_port and port of aconnector drm/amdgpu/display/mst: adjust the logic in 2nd phase of updating payload
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 4 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 48 +++++++++--- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 +- .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 2 +- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 16 ++-- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 76 +++++++++++++------ .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 53 ++++++------- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 14 +++- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +- include/drm/drm_print.h | 2 +- 10 files changed, 143 insertions(+), 80 deletions(-)
-- 2.39.0
On 1/20/23 18:15, Lyude Paul wrote:
For the whole series:
Reviewed-by: Lyude Paul lyude@redhat.com
Thanks, series is merged to amd-staging-drm-next.
Harry
So glad to have this fixed finally ♥
On Thu, 2023-01-19 at 18:51 -0500, Harry Wentland wrote:
MST has been broken on amdgpu after a refactor in drm_dp_mst code that was aligning drm_dp_mst more closely with the atomic model.
The gitlab issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
This series fixes it.
It can be found at https://gitlab.freedesktop.org/hwentland/linux/-/tree/mst_regression
A stable-6.1.y rebase can be found at https://gitlab.freedesktop.org/hwentland/linux/-/tree/mst_regression_6.1
Lyude Paul (1): drm/amdgpu/display/mst: Fix mst_state->pbn_div and slot count assignments
Wayne Lin (6): drm/amdgpu/display/mst: limit payload to be updated one by one drm/amdgpu/display/mst: update mst_mgr relevant variable when long HPD drm/drm_print: correct format problem drm/display/dp_mst: Correct the kref of port. drm/amdgpu/display/mst: adjust the naming of mst_port and port of aconnector drm/amdgpu/display/mst: adjust the logic in 2nd phase of updating payload
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 4 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 48 +++++++++--- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 +- .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 2 +- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 16 ++-- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 76 +++++++++++++------ .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 53 ++++++------- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 14 +++- drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +- include/drm/drm_print.h | 2 +- 10 files changed, 143 insertions(+), 80 deletions(-)
-- 2.39.0
For the whole series, as rebased on v6.1.7. Tested on this Thinkpad X13 AMD Gen2:
Tested-By: Didier Raboud odyx@debian.org
Le vendredi, 20 janvier 2023, 00.51:53 h CET Harry Wentland a écrit :
MST has been broken on amdgpu after a refactor in drm_dp_mst code that was aligning drm_dp_mst more closely with the atomic model.
The gitlab issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
This series fixes it.
On 1/22/23 14:12, Didier 'OdyX' Raboud wrote:
For the whole series, as rebased on v6.1.7. Tested on this Thinkpad X13 AMD Gen2:
Tested-By: Didier Raboud odyx@debian.org
Thanks.
Harry
Le vendredi, 20 janvier 2023, 00.51:53 h CET Harry Wentland a écrit :
MST has been broken on amdgpu after a refactor in drm_dp_mst code that was aligning drm_dp_mst more closely with the atomic model.
The gitlab issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
This series fixes it.
linux-stable-mirror@lists.linaro.org
-
Didier 'OdyX' Raboud -
Harry Wentland -
kernel test robot -
Lyude Paul