Whenever a connector on an MST network is attached, detached, or undergoes a modeset, the DSC configs for each stream on that topology will be recalculated. This can change their required bandwidth, requiring a full reprogramming, as though a modeset was performed, even if that stream did not change timing.
Therefore, whenever a crtc has drm_atomic_crtc_needs_modeset, for each crtc that shares a MST topology with that stream and supports DSC, add that crtc (and all affected connectors and planes) to the atomic state and set mode_changed on its state
Cc: Leo Li sunpeng.li@amd.com Cc: Nicholas Kazlauskas nicholas.kazlauskas@amd.com Signed-off-by: David Francis David.Francis@amd.com
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+)
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 145fd73025dc..8d5357aec5e8 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6475,7 +6475,78 @@ static int do_aquire_global_lock(struct drm_device *dev,
return ret < 0 ? ret : 0;} +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT +static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct drm_crtc *crtc) +{
struct drm_connector *connector;struct drm_connector_state *conn_state;struct drm_connector_list_iter conn_iter;struct drm_crtc_state *new_crtc_state;struct amdgpu_dm_connector *aconnector = NULL, *aconnector_to_add;int i, j, ret;struct drm_crtc *crtcs_affected[MAX_PIPES] = { 0 }; > +for_each_new_connector_in_state(state, connector, conn_state, i) {if (conn_state->crtc != crtc)continue;aconnector = to_amdgpu_dm_connector(connector);if (!aconnector->port)aconnector = NULL;elsebreak;}if (!aconnector)return 0;i = 0;drm_connector_list_iter_begin(state->dev, &conn_iter);I don't like that we're grabbing the global connector lock every single time any CRTC undergoes a modeset even for ASICs that don't support DSC.
We do lock everything below in atomic check anyway for FULL updates but I'd like to avoid adding more code that does this if possible. Maybe a check at the start that only does this if the ASIC has DSC support would be OK.
Will do
drm_for_each_connector_iter(connector, &conn_iter) {if (!connector->state || !connector->state->crtc)continue;aconnector_to_add = to_amdgpu_dm_connector(connector);if (!aconnector_to_add->port)continue;if (aconnector_to_add->port->mgr != aconnector->port->mgr)continue;if (!aconnector_to_add->dc_sink)continue;if (!aconnector_to_add->dc_sink->sink_dsc_caps.dsc_dec_caps.is_dsc_supported)continue;if (i >= MAX_PIPES)continue;crtcs_affected[i] = connector->state->crtc;Drop this crtcs_affected array and just perform the logic below right here. We don't really need two loops here, redundant calls to drm_atomic_get_crtc_state and the other helpers are fine.
Unfortunately, calling drm_atomic_get_crtc_state inside drm_for_each_connector_iter causes a lockdep warning
i++;}drm_connector_list_iter_end(&conn_iter);for (j = 0; j < i; j++) {new_crtc_state = drm_atomic_get_crtc_state(state, crtcs_affected[j]);if (IS_ERR(new_crtc_state))return PTR_ERR(new_crtc_state);new_crtc_state->mode_changed = true;ret = drm_atomic_add_affected_connectors(state, crtcs_affected[j]);if (ret)return ret;ret = drm_atomic_add_affected_planes(state, crtcs_affected[j]);if (ret)return ret;}return 0;+} +#endif static void get_freesync_config_for_crtc( struct dm_crtc_state *new_crtc_state, struct dm_connector_state *new_con_state) @@ -7178,6 +7249,15 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, goto fail; }
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {ret = add_affected_mst_dsc_crtcs(state, crtc);if (ret)goto fail;}}+#endif /* * Add all primary and overlay planes on the CRTC to the state * whenever a plane is enabled to maintain correct z-ordering
________________________________________ From: Kazlauskas, Nicholas Nicholas.Kazlauskas@amd.com Sent: August 19, 2019 3:34 PM To: Francis, David; dri-devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org Cc: Li, Sun peng (Leo) Subject: Re: [PATCH 14/14] drm/amd/display: Trigger modesets on MST DSC connectors
On 8/19/19 11:50 AM, David Francis wrote:
Whenever a connector on an MST network is attached, detached, or undergoes a modeset, the DSC configs for each stream on that topology will be recalculated. This can change their required bandwidth, requiring a full reprogramming, as though a modeset was performed, even if that stream did not change timing.
Therefore, whenever a crtc has drm_atomic_crtc_needs_modeset, for each crtc that shares a MST topology with that stream and supports DSC, add that crtc (and all affected connectors and planes) to the atomic state and set mode_changed on its state
Cc: Leo Li sunpeng.li@amd.com Cc: Nicholas Kazlauskas nicholas.kazlauskas@amd.com Signed-off-by: David Francis David.Francis@amd.com
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+)
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 145fd73025dc..8d5357aec5e8 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6475,7 +6475,78 @@ static int do_aquire_global_lock(struct drm_device *dev,
return ret < 0 ? ret : 0;} +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT +static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct drm_crtc *crtc) +{
struct drm_connector *connector;struct drm_connector_state *conn_state;struct drm_connector_list_iter conn_iter;struct drm_crtc_state *new_crtc_state;struct amdgpu_dm_connector *aconnector = NULL, *aconnector_to_add;int i, j, ret;struct drm_crtc *crtcs_affected[MAX_PIPES] = { 0 }; > +for_each_new_connector_in_state(state, connector, conn_state, i) {if (conn_state->crtc != crtc)continue;aconnector = to_amdgpu_dm_connector(connector);if (!aconnector->port)aconnector = NULL;elsebreak;}if (!aconnector)return 0;i = 0;drm_connector_list_iter_begin(state->dev, &conn_iter);
I don't like that we're grabbing the global connector lock every single time any CRTC undergoes a modeset even for ASICs that don't support DSC.
We do lock everything below in atomic check anyway for FULL updates but I'd like to avoid adding more code that does this if possible. Maybe a check at the start that only does this if the ASIC has DSC support would be OK.
drm_for_each_connector_iter(connector, &conn_iter) {if (!connector->state || !connector->state->crtc)continue;aconnector_to_add = to_amdgpu_dm_connector(connector);if (!aconnector_to_add->port)continue;if (aconnector_to_add->port->mgr != aconnector->port->mgr)continue;if (!aconnector_to_add->dc_sink)continue;if (!aconnector_to_add->dc_sink->sink_dsc_caps.dsc_dec_caps.is_dsc_supported)continue;if (i >= MAX_PIPES)continue;crtcs_affected[i] = connector->state->crtc;
Drop this crtcs_affected array and just perform the logic below right here. We don't really need two loops here, redundant calls to drm_atomic_get_crtc_state and the other helpers are fine.
i++;}drm_connector_list_iter_end(&conn_iter);for (j = 0; j < i; j++) {new_crtc_state = drm_atomic_get_crtc_state(state, crtcs_affected[j]);if (IS_ERR(new_crtc_state))return PTR_ERR(new_crtc_state);new_crtc_state->mode_changed = true;ret = drm_atomic_add_affected_connectors(state, crtcs_affected[j]);if (ret)return ret;ret = drm_atomic_add_affected_planes(state, crtcs_affected[j]);if (ret)return ret;}return 0;+} +#endif static void get_freesync_config_for_crtc( struct dm_crtc_state *new_crtc_state, struct dm_connector_state *new_con_state) @@ -7178,6 +7249,15 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, goto fail; }
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {ret = add_affected_mst_dsc_crtcs(state, crtc);if (ret)goto fail;}}+#endif /* * Add all primary and overlay planes on the CRTC to the state * whenever a plane is enabled to maintain correct z-ordering