[PATCH v2 14/14] drm/amd/display: Trigger modesets on MST DSC connectors

[Kazlauskas, Nicholas]

On 8/20/19 5:09 PM, Lyude Paul wrote:
> This should definitely be implemented as an atomic helper in
> drm_dp_mst_topology.c as well.

This is probably reasonable, most drivers would probably want this.

The issues with this in general are still:

(1) Knowing if you have a DSC in the MST network to decide when to do this

(2) Marking the CRTCs for the connectors in the MST chain as having 
their mode changed while iterating over the connectors

For amdgpu it's "acceptable" based on other design constraints to just 
lock and grab everything on a modeset but this may not be true for every 
driver.

Nicholas Kazlauskas

> 
> On Tue, 2019-08-20 at 15:12 -0400, 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
>>
>> v2: Do this check only on Navi and before adding connectors
>> and planes on modesetting crtcs
>>
>> Cc: Leo Li <sunpeng.li at amd.com>
>> Cc: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>> Signed-off-by: David Francis <David.Francis at amd.com>
>> ---
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 74 +++++++++++++++++++
>>   1 file changed, 74 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..e64f2a6eb71a 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,70 @@ 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;
>> +	struct drm_crtc *crtcs_affected[AMDGPU_MAX_CRTCS] = { 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;
>> +		else
>> +			break;
>> +	}
>> +
>> +	if (!aconnector)
>> +		return 0;
>> +
>> +	i = 0;
>> +	drm_connector_list_iter_begin(state->dev, &conn_iter);
>> +	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 >= AMDGPU_MAX_CRTCS)
>> +			continue;
>> +
>> +		crtcs_affected[i] = connector->state->crtc;
>> +		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;
>> +	}
>> +
>> +	return 0;
>> +
>> +}
>> +#endif
>>   static void get_freesync_config_for_crtc(
>>   	struct dm_crtc_state *new_crtc_state,
>>   	struct dm_connector_state *new_con_state)
>> @@ -7160,6 +7223,17 @@ static int amdgpu_dm_atomic_check(struct drm_device
>> *dev,
>>   	if (ret)
>>   		goto fail;
>>   
>> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
>> +	if (adev->asic_type >= CHIP_NAVI10) {
>> +		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
>>   	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
>> new_crtc_state, i) {
>>   		if (!drm_atomic_crtc_needs_modeset(new_crtc_state) &&
>>   		    !new_crtc_state->color_mgmt_changed &&