[PATCH] drm/display/dsc: Refactor MST DSC Determination Policy

Harry Wentland harry.wentland at amd.com
Fri Sep 20 21:22:26 UTC 2024



On 2024-09-20 01:56, Jani Nikula wrote:
> On Thu, 19 Sep 2024, Fangzhi Zuo <Jerry.Zuo at amd.com> wrote:
>> [why]
>> How we determine the dsc_aux used for dsc decompression in
>> drm_dp_mst_dsc_aux_for_port() today having some defects:
>>
>> 1. The method how we determine a connected peer device is virtual or not
>>    in drm_dp_mst_is_virtual_dpcd() is not always correct. There are DP1.4 products
>>    in the market which don't fully comply with DP spec to enumerate virtual peer device.
>>    That leads to existing logic defects. For example:
>>
>>    -  Some 1.4 mst hubs with hdmi output port don't enumerate virtual dpcd/peer device.
>>       When probing the hub, its topology is constructed with a branch device only, with
>>       peer device type set as DP-to-Legacy converter for its HDMI output port.
>>       Under this condition, drm_dp_mst_is_virtual_dpcd() will still determine it's connected
>>       with a virtual peer device with virtual dpcd. And results in the section for
>>       analyzing DP-to-DP peer device case of drm_dp_mst_dsc_aux_for_port(). That's logically
>>       incorrect.
>>
>> 2. Existing routine is designed based on analyzing different connected peer device types, such
>>    as dp-dp, dp-hdmi peer device, and virtual sink. Such categorization is redundant and unnecessary.
>>    The key info of determining where to do dsc decompression relies on the dsc capability from dpcd
>>    only. No matter the mst branch device enumerates virtual dpcd or not, if it's supporting dsc, it
>>    must declare it's dsc capability at somewhere within its responded topology.
>>
>> 3. Synaptics quirk analyzing today is unnecessary as long as we determine dsc aux generically
>>    by dpcd info.
>>
>> Based on above, we would like to refactor the logic how we determine the dsc aux today a bit.
>>
>> [how]
>> Ideally, dsc_aux should be determined by the topology connection status and
>> dpcd capability info only. In this way, dsc aux could be determined in a more generic
>> way, instead of enumerating and analyzing on different connected peer device types.
>> We construct the refactored algorithm into 2 parts:
>>
>> 1. Firstly, deal with all known quirks or w/a for branch devices in the market that
>>    don't comply with DP spec and can't determine its dsc aux in a generic way.
>>    e.g. Force to pick root aux as the dsc decompression point.
>>
>> 2. Refine the dsc determination policy by generalizing detection solely based on
>>    topology connection status and dpcd info, instead of analyzing it by enumerating
>>    different connection types of the peer device.
>>
>> In addition, we add parsing vendor data 0x50C ~ 0x50F into dpcd quirk routine for
>> expanding the ability to determine specific dock products that shall pick root aux as the
>> dsc decompression point.
> 
> Too many changes in one patch. Please split up.
> 

Thanks, Jani. Agreed.

Harry

> BR,
> Jani.
> 
>>
>> Signed-off-by: Fangzhi Zuo <Jerry.Zuo at amd.com>
>> Signed-off-by: Wayne Lin <wayne.lin at amd.com>
>> ---
>>  .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c |   2 +-
>>  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  20 +-
>>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  59 +---
>>  drivers/gpu/drm/display/drm_dp_helper.c       |  30 +-
>>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 259 ++++++++----------
>>  drivers/gpu/drm/i915/display/intel_dp.c       |   2 +-
>>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |   3 +-
>>  include/drm/display/drm_dp.h                  |   1 +
>>  include/drm/display/drm_dp_helper.h           |   8 +
>>  include/drm/display/drm_dp_mst_helper.h       |   5 +-
>>  10 files changed, 181 insertions(+), 208 deletions(-)
>>
>> 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 db56b0aa5454..0da703f4ccac 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
>> @@ -1370,7 +1370,7 @@ static int dp_dsc_fec_support_show(struct seq_file *m, void *data)
>>  			 * enable DSC on the sink device or on MST branch
>>  			 * its connected to.
>>  			 */
>> -			if (aconnector->dsc_aux) {
>> +			if (aconnector->mst_output_port->dsc_aux) {
>>  				is_fec_supported = true;
>>  				is_dsc_supported = true;
>>  			}
>> 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 50109d13d967..f3613f38b9b8 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
>> @@ -806,20 +806,20 @@ bool dm_helpers_dp_write_dsc_enable(
>>  	uint8_t ret = 0;
>>  
>>  	if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST) {
>> -		if (!aconnector->dsc_aux)
>> +		if (!aconnector->mst_output_port->dsc_aux)
>>  			return false;
>>  
>>  		// apply w/a to synaptics
>>  		if (needs_dsc_aux_workaround(aconnector->dc_link) &&
>>  		    (aconnector->mst_downstream_port_present.byte & 0x7) != 0x3)
>>  			return write_dsc_enable_synaptics_non_virtual_dpcd_mst(
>> -				aconnector->dsc_aux, stream, enable_dsc);
>> +				aconnector->mst_output_port->dsc_aux, stream, enable_dsc);
>>  
>>  		port = aconnector->mst_output_port;
>>  
>>  		if (enable) {
>> -			if (port->passthrough_aux) {
>> -				ret = drm_dp_dpcd_write(port->passthrough_aux,
>> +			if (port->dsc_passthrough_aux) {
>> +				ret = drm_dp_dpcd_write(port->dsc_passthrough_aux,
>>  							DP_DSC_ENABLE,
>>  							&enable_passthrough, 1);
>>  				drm_dbg_dp(dev,
>> @@ -827,24 +827,24 @@ bool dm_helpers_dp_write_dsc_enable(
>>  					   ret);
>>  			}
>>  
>> -			ret = drm_dp_dpcd_write(aconnector->dsc_aux,
>> +			ret = drm_dp_dpcd_write(aconnector->mst_output_port->dsc_aux,
>>  						DP_DSC_ENABLE, &enable_dsc, 1);
>>  			drm_dbg_dp(dev,
>>  				   "MST_DSC Sent DSC decoding enable to %s port, ret = %u\n",
>> -				   (port->passthrough_aux) ? "remote RX" :
>> +				   (port->dsc_passthrough_aux) ? "remote RX" :
>>  				   "virtual dpcd",
>>  				   ret);
>>  		} else {
>> -			ret = drm_dp_dpcd_write(aconnector->dsc_aux,
>> +			ret = drm_dp_dpcd_write(aconnector->mst_output_port->dsc_aux,
>>  						DP_DSC_ENABLE, &enable_dsc, 1);
>>  			drm_dbg_dp(dev,
>>  				   "MST_DSC Sent DSC decoding disable to %s port, ret = %u\n",
>> -				   (port->passthrough_aux) ? "remote RX" :
>> +				   (port->dsc_passthrough_aux) ? "remote RX" :
>>  				   "virtual dpcd",
>>  				   ret);
>>  
>> -			if (port->passthrough_aux) {
>> -				ret = drm_dp_dpcd_write(port->passthrough_aux,
>> +			if (port->dsc_passthrough_aux) {
>> +				ret = drm_dp_dpcd_write(port->dsc_passthrough_aux,
>>  							DP_DSC_ENABLE,
>>  							&enable_passthrough, 1);
>>  				drm_dbg_dp(dev,
>> 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 6b5eed37532b..61ee3c38a29a 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
>> @@ -183,8 +183,8 @@ amdgpu_dm_mst_connector_early_unregister(struct drm_connector *connector)
>>  		dc_sink_release(dc_sink);
>>  		aconnector->dc_sink = NULL;
>>  		aconnector->edid = NULL;
>> -		aconnector->dsc_aux = NULL;
>> -		port->passthrough_aux = NULL;
>> +		aconnector->mst_output_port->dsc_aux = NULL;
>> +		aconnector->mst_output_port->dsc_passthrough_aux = NULL;
>>  	}
>>  
>>  	aconnector->mst_status = MST_STATUS_DEFAULT;
>> @@ -214,21 +214,6 @@ bool needs_dsc_aux_workaround(struct dc_link *link)
>>  }
>>  
>>  #if defined(CONFIG_DRM_AMD_DC_FP)
>> -static bool is_synaptics_cascaded_panamera(struct dc_link *link, struct drm_dp_mst_port *port)
>> -{
>> -	u8 branch_vendor_data[4] = { 0 }; // Vendor data 0x50C ~ 0x50F
>> -
>> -	if (drm_dp_dpcd_read(port->mgr->aux, DP_BRANCH_VENDOR_SPECIFIC_START, &branch_vendor_data, 4) == 4) {
>> -		if (link->dpcd_caps.branch_dev_id == DP_BRANCH_DEVICE_ID_90CC24 &&
>> -				IS_SYNAPTICS_CASCADED_PANAMERA(link->dpcd_caps.branch_dev_name, branch_vendor_data)) {
>> -			DRM_INFO("Synaptics Cascaded MST hub\n");
>> -			return true;
>> -		}
>> -	}
>> -
>> -	return false;
>> -}
>> -
>>  static bool validate_dsc_caps_on_connector(struct amdgpu_dm_connector *aconnector)
>>  {
>>  	struct dc_sink *dc_sink = aconnector->dc_sink;
>> @@ -237,32 +222,14 @@ static bool validate_dsc_caps_on_connector(struct amdgpu_dm_connector *aconnecto
>>  	u8 dsc_branch_dec_caps_raw[3] = { 0 };	// DSC branch decoder caps 0xA0 ~ 0xA2
>>  	u8 *dsc_branch_dec_caps = NULL;
>>  
>> -	aconnector->dsc_aux = drm_dp_mst_dsc_aux_for_port(port);
>> -
>> -	/*
>> -	 * drm_dp_mst_dsc_aux_for_port() will return NULL for certain configs
>> -	 * because it only check the dsc/fec caps of the "port variable" and not the dock
>> -	 *
>> -	 * This case will return NULL: DSC capabe MST dock connected to a non fec/dsc capable display
>> -	 *
>> -	 * Workaround: explicitly check the use case above and use the mst dock's aux as dsc_aux
>> -	 *
>> -	 */
>> -	if (!aconnector->dsc_aux && !port->parent->port_parent &&
>> -	    needs_dsc_aux_workaround(aconnector->dc_link))
>> -		aconnector->dsc_aux = &aconnector->mst_root->dm_dp_aux.aux;
>> -
>> -	/* synaptics cascaded MST hub case */
>> -	if (is_synaptics_cascaded_panamera(aconnector->dc_link, port))
>> -		aconnector->dsc_aux = port->mgr->aux;
>> -
>> -	if (!aconnector->dsc_aux)
>> +	drm_dp_mst_dsc_aux_for_port(port);
>> +	if (!aconnector->mst_output_port->dsc_aux)
>>  		return false;
>>  
>> -	if (drm_dp_dpcd_read(aconnector->dsc_aux, DP_DSC_SUPPORT, dsc_caps, 16) < 0)
>> +	if (drm_dp_dpcd_read(aconnector->mst_output_port->dsc_aux, DP_DSC_SUPPORT, dsc_caps, 16) < 0)
>>  		return false;
>>  
>> -	if (drm_dp_dpcd_read(aconnector->dsc_aux,
>> +	if (drm_dp_dpcd_read(aconnector->mst_output_port->dsc_aux,
>>  			DP_DSC_BRANCH_OVERALL_THROUGHPUT_0, dsc_branch_dec_caps_raw, 3) == 3)
>>  		dsc_branch_dec_caps = dsc_branch_dec_caps_raw;
>>  
>> @@ -279,10 +246,10 @@ static bool retrieve_downstream_port_device(struct amdgpu_dm_connector *aconnect
>>  {
>>  	union dp_downstream_port_present ds_port_present;
>>  
>> -	if (!aconnector->dsc_aux)
>> +	if (!aconnector->mst_output_port->dsc_aux)
>>  		return false;
>>  
>> -	if (drm_dp_dpcd_read(aconnector->dsc_aux, DP_DOWNSTREAMPORT_PRESENT, &ds_port_present, 1) < 0) {
>> +	if (drm_dp_dpcd_read(aconnector->mst_output_port->dsc_aux, DP_DOWNSTREAMPORT_PRESENT, &ds_port_present, 1) < 0) {
>>  		DRM_INFO("Failed to read downstream_port_present 0x05 from DFP of branch device\n");
>>  		return false;
>>  	}
>> @@ -501,8 +468,8 @@ dm_dp_mst_detect(struct drm_connector *connector,
>>  		dc_sink_release(aconnector->dc_sink);
>>  		aconnector->dc_sink = NULL;
>>  		aconnector->edid = NULL;
>> -		aconnector->dsc_aux = NULL;
>> -		port->passthrough_aux = NULL;
>> +		aconnector->mst_output_port->dsc_aux = NULL;
>> +		aconnector->mst_output_port->dsc_passthrough_aux = NULL;
>>  
>>  		amdgpu_dm_set_mst_status(&aconnector->mst_status,
>>  			MST_REMOTE_EDID | MST_ALLOCATE_NEW_PAYLOAD | MST_CLEAR_ALLOCATED_PAYLOAD,
>> @@ -1293,7 +1260,7 @@ static bool is_dsc_need_re_compute(
>>  			continue;
>>  
>>  		aconnector = (struct amdgpu_dm_connector *) stream->dm_stream_context;
>> -		if (!aconnector || !aconnector->dsc_aux)
>> +		if (!aconnector || !aconnector->mst_output_port->dsc_aux)
>>  			continue;
>>  
>>  		stream_on_link[new_stream_on_link_num] = aconnector;
>> @@ -1778,13 +1745,13 @@ enum dc_status dm_dp_mst_is_port_support_mode(
>>  	}
>>  
>>  	/*DSC necessary case*/
>> -	if (!aconnector->dsc_aux)
>> +	if (!aconnector->mst_output_port->dsc_aux)
>>  		return DC_FAIL_BANDWIDTH_VALIDATE;
>>  
>>  	if (is_dsc_common_config_possible(stream, &bw_range)) {
>>  
>>  		/*capable of dsc passthough. dsc bitstream along the entire path*/
>> -		if (aconnector->mst_output_port->passthrough_aux) {
>> +		if (aconnector->mst_output_port->dsc_passthrough_aux) {
>>  			if (bw_range.min_kbps > end_to_end_bw_in_kbps) {
>>  				DRM_DEBUG_DRIVER("MST_DSC dsc passthrough and decode at endpoint"
>>  						 "Max dsc compression bw can't fit into end-to-end bw\n");
>> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
>> index 6ee51003de3c..fedeec8f8bea 100644
>> --- a/drivers/gpu/drm/display/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
>> @@ -2259,6 +2259,7 @@ EXPORT_SYMBOL(drm_dp_stop_crc);
>>  struct dpcd_quirk {
>>  	u8 oui[3];
>>  	u8 device_id[6];
>> +	u8 vendor_data[4];
>>  	bool is_branch;
>>  	u32 quirks;
>>  };
>> @@ -2266,26 +2267,31 @@ struct dpcd_quirk {
>>  #define OUI(first, second, third) { (first), (second), (third) }
>>  #define DEVICE_ID(first, second, third, fourth, fifth, sixth) \
>>  	{ (first), (second), (third), (fourth), (fifth), (sixth) }
>> +#define VENDOR_DATA(first, second, third, fourth) \
>> +	{ (first), (second), (third), (fourth) }
>>  
>>  #define DEVICE_ID_ANY	DEVICE_ID(0, 0, 0, 0, 0, 0)
>> +#define VENDOR_DATA_ANY	VENDOR_DATA(0, 0, 0, 0)
>>  
>>  static const struct dpcd_quirk dpcd_quirk_list[] = {
>>  	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
>> -	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
>> +	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, VENDOR_DATA_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
>>  	/* LG LP140WF6-SPM1 eDP panel */
>> -	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
>> +	{ OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), VENDOR_DATA_ANY, false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
>>  	/* Apple panels need some additional handling to support PSR */
>> -	{ OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) },
>> +	{ OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, VENDOR_DATA_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) },
>>  	/* CH7511 seems to leave SINK_COUNT zeroed */
>> -	{ OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
>> +	{ OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), VENDOR_DATA_ANY, false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
>>  	/* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */
>> -	{ OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) },
>> +	{ OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, VENDOR_DATA_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) },
>>  	/* Synaptics DP1.4 MST hubs require DSC for some modes on which it applies HBLANK expansion. */
>> -	{ OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC) },
>> +	{ OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, VENDOR_DATA_ANY, true, BIT(DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC) },
>>  	/* MediaTek panels (at least in U3224KBA) require DSC for modes with a short HBLANK on UHBR links. */
>> -	{ OUI(0x00, 0x0C, 0xE7), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC) },
>> +	{ OUI(0x00, 0x0C, 0xE7), DEVICE_ID_ANY, VENDOR_DATA_ANY, false, BIT(DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC) },
>>  	/* Apple MacBookPro 2017 15 inch eDP Retina panel reports too low DP_MAX_LINK_RATE */
>> -	{ OUI(0x00, 0x10, 0xfa), DEVICE_ID(101, 68, 21, 101, 98, 97), false, BIT(DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS) },
>> +	{ OUI(0x00, 0x10, 0xfa), DEVICE_ID(101, 68, 21, 101, 98, 97), VENDOR_DATA_ANY, false, BIT(DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS) },
>> +	/* Lenovo dock that needs root branch for dsc */
>> +	{ OUI(0x90, 0xCC, 0x24), DEVICE_ID(83, 89, 78, 65, 83, 50), VENDOR_DATA(4, 6, 90, 0), true, BIT(DP_DPCD_QUIRK_DSC_MUST_ROOT_DECOMPRESSION) },
>>  };
>>  
>>  #undef OUI
>> @@ -2305,6 +2311,7 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
>>  	u32 quirks = 0;
>>  	int i;
>>  	u8 any_device[] = DEVICE_ID_ANY;
>> +	u8 any_vendor[] = VENDOR_DATA_ANY;
>>  
>>  	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
>>  		quirk = &dpcd_quirk_list[i];
>> @@ -2319,6 +2326,10 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
>>  		    memcmp(quirk->device_id, ident->device_id, sizeof(ident->device_id)) != 0)
>>  			continue;
>>  
>> +		if (memcmp(quirk->vendor_data, any_vendor, sizeof(any_vendor)) != 0 &&
>> +		    memcmp(quirk->vendor_data, ident->vendor_data, sizeof(ident->vendor_data)) != 0)
>> +			continue;
>> +
>>  		quirks |= quirk->quirks;
>>  	}
>>  
>> @@ -2344,10 +2355,11 @@ static void drm_dp_dump_desc(struct drm_dp_aux *aux,
>>  	const struct drm_dp_dpcd_ident *ident = &desc->ident;
>>  
>>  	drm_dbg_kms(aux->drm_dev,
>> -		    "%s: %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d quirks 0x%04x\n",
>> +		    "%s: %s: OUI %*phD dev-ID %*pE vendor-DATA %*pE HW-rev %d.%d SW-rev %d.%d quirks 0x%04x\n",
>>  		    aux->name, device_name,
>>  		    (int)sizeof(ident->oui), ident->oui,
>>  		    (int)strnlen(ident->device_id, sizeof(ident->device_id)), ident->device_id,
>> +		    (int)strnlen(ident->vendor_data, sizeof(ident->vendor_data)), ident->vendor_data,
>>  		    ident->hw_rev >> 4, ident->hw_rev & 0xf,
>>  		    ident->sw_major_rev, ident->sw_minor_rev,
>>  		    desc->quirks);
>> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> index a040d7dfced1..85fa9234b46d 100644
>> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> @@ -2258,6 +2258,8 @@ void drm_dp_mst_connector_early_unregister(struct drm_connector *connector,
>>  	drm_dbg_kms(port->mgr->dev, "unregistering %s remote bus for %s\n",
>>  		    port->aux.name, connector->kdev->kobj.name);
>>  	drm_dp_aux_unregister_devnode(&port->aux);
>> +	port->dsc_aux = NULL;
>> +	port->dsc_passthrough_aux = NULL;
>>  }
>>  EXPORT_SYMBOL(drm_dp_mst_connector_early_unregister);
>>  
>> @@ -5445,7 +5447,8 @@ int drm_dp_mst_add_affected_dsc_crtcs(struct drm_atomic_state *state, struct drm
>>  		if (!crtc)
>>  			continue;
>>  
>> -		if (!drm_dp_mst_dsc_aux_for_port(pos->port))
>> +		drm_dp_mst_dsc_aux_for_port(pos->port);
>> +		if (!pos->port->dsc_aux)
>>  			continue;
>>  
>>  		crtc_state = drm_atomic_get_crtc_state(mst_state->base.state, crtc);
>> @@ -5994,57 +5997,6 @@ static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port)
>>  	i2c_del_adapter(&port->aux.ddc);
>>  }
>>  
>> -/**
>> - * drm_dp_mst_is_virtual_dpcd() - Is the given port a virtual DP Peer Device
>> - * @port: The port to check
>> - *
>> - * A single physical MST hub object can be represented in the topology
>> - * by multiple branches, with virtual ports between those branches.
>> - *
>> - * As of DP1.4, An MST hub with internal (virtual) ports must expose
>> - * certain DPCD registers over those ports. See sections 2.6.1.1.1
>> - * and 2.6.1.1.2 of Display Port specification v1.4 for details.
>> - *
>> - * May acquire mgr->lock
>> - *
>> - * Returns:
>> - * true if the port is a virtual DP peer device, false otherwise
>> - */
>> -static bool drm_dp_mst_is_virtual_dpcd(struct drm_dp_mst_port *port)
>> -{
>> -	struct drm_dp_mst_port *downstream_port;
>> -
>> -	if (!port || port->dpcd_rev < DP_DPCD_REV_14)
>> -		return false;
>> -
>> -	/* Virtual DP Sink (Internal Display Panel) */
>> -	if (drm_dp_mst_port_is_logical(port))
>> -		return true;
>> -
>> -	/* DP-to-HDMI Protocol Converter */
>> -	if (port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV &&
>> -	    !port->mcs &&
>> -	    port->ldps)
>> -		return true;
>> -
>> -	/* DP-to-DP */
>> -	mutex_lock(&port->mgr->lock);
>> -	if (port->pdt == DP_PEER_DEVICE_MST_BRANCHING &&
>> -	    port->mstb &&
>> -	    port->mstb->num_ports == 2) {
>> -		list_for_each_entry(downstream_port, &port->mstb->ports, next) {
>> -			if (downstream_port->pdt == DP_PEER_DEVICE_SST_SINK &&
>> -			    !downstream_port->input) {
>> -				mutex_unlock(&port->mgr->lock);
>> -				return true;
>> -			}
>> -		}
>> -	}
>> -	mutex_unlock(&port->mgr->lock);
>> -
>> -	return false;
>> -}
>> -
>>  /**
>>   * drm_dp_mst_aux_for_parent() - Get the AUX device for an MST port's parent
>>   * @port: MST port whose parent's AUX device is returned
>> @@ -6074,114 +6026,145 @@ EXPORT_SYMBOL(drm_dp_mst_aux_for_parent);
>>   * This operation can be expensive (up to four aux reads), so
>>   * the caller should cache the return.
>>   *
>> - * Returns:
>> - * NULL if DSC cannot be enabled on this port, otherwise the aux device
>> + * port->dsc_aux - point for dsc decompression
>> + * port->dsc_passthrough_aux - point for dsc passthrough
>> + *
>>   */
>> -struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
>> +void drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
>>  {
>> -	struct drm_dp_mst_port *immediate_upstream_port;
>> -	struct drm_dp_aux *immediate_upstream_aux;
>> -	struct drm_dp_mst_port *fec_port;
>> +	struct drm_dp_mst_topology_mgr *mgr = port->mgr;
>> +	struct drm_dp_mst_port *immediate_upstream_port = NULL;
>> +	struct drm_dp_mst_port *fec_port = NULL;
>> +	struct drm_dp_mst_port *dsc_port = NULL;
>> +	struct drm_dp_aux *upstream_aux;
>>  	struct drm_dp_desc desc = {};
>> -	u8 endpoint_fec;
>> -	u8 endpoint_dsc;
>> +	bool end_has_dpcd = (port->dpcd_rev > 0);
>> +	u8 endpoint_dsc = 0;
>> +	u8 upstream_dsc;
>> +	u8 fec_cap;
>>  
>>  	if (!port)
>> -		return NULL;
>> +		return;
>> +
>> +	port->dsc_aux = NULL;
>> +	port->dsc_passthrough_aux = NULL;
>> +
>> +	/* Check special case first */
>> +	if (drm_dp_read_desc(mgr->aux, &desc, true) == 0) {
>> +		if (drm_dp_has_quirk(&desc, DP_DPCD_QUIRK_DSC_MUST_ROOT_DECOMPRESSION)) {
>> +			port->dsc_aux = mgr->aux;
>> +			drm_info(mgr->dev, "MST_DSC DSC_MUST_ROOT_DECODING w/a\n");
>> +			return;
>> +		}
>> +	}
>> +
>> +	/* Policy start */
>> +	if (!drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
>> +		drm_err(mgr->dev,
>> +			"MST_DSC Can't determine dsc aux for port %p which is not connected to end device\n",
>> +			port);
>> +		return;
>> +	}
>>  
>>  	if (port->parent->port_parent)
>>  		immediate_upstream_port = port->parent->port_parent;
>> -	else
>> -		immediate_upstream_port = NULL;
>>  
>> -	fec_port = immediate_upstream_port;
>> -	while (fec_port) {
>> -		/*
>> -		 * Each physical link (i.e. not a virtual port) between the
>> -		 * output and the primary device must support FEC
>> -		 */
>> -		if (!drm_dp_mst_is_virtual_dpcd(fec_port) &&
>> -		    !fec_port->fec_capable)
>> -			return NULL;
>> +	if (end_has_dpcd) {
>> +		drm_info(mgr->dev, "MST_DSC check port->aux for dsc decompression capability\n");
>> +		if (drm_dp_dpcd_read(&port->aux, DP_DSC_SUPPORT, &endpoint_dsc, 1) != 1) {
>> +			drm_err(mgr->dev, "MST_DSC Can't retrieve dsc caps from endpoint port\n");
>> +			goto out_dsc_fail;
>> +		}
>> +	}
>>  
>> -		fec_port = fec_port->parent->port_parent;
>> +	if (immediate_upstream_port) {
>> +		upstream_aux = &immediate_upstream_port->aux;
>> +		drm_info(mgr->dev, "MST_DSC check immediate_upstream_port->aux for dsc passthrough capability\n");
>> +	} else {
>> +		upstream_aux = mgr->aux;
>> +		drm_info(mgr->dev, "MST_DSC check mgr->aux for dsc passthrough capability\n");
>>  	}
>>  
>> -	/* DP-to-DP peer device */
>> -	if (drm_dp_mst_is_virtual_dpcd(immediate_upstream_port)) {
>> -		u8 upstream_dsc;
>> -
>> -		if (drm_dp_dpcd_read(&port->aux,
>> -				     DP_DSC_SUPPORT, &endpoint_dsc, 1) != 1)
>> -			return NULL;
>> -		if (drm_dp_dpcd_read(&port->aux,
>> -				     DP_FEC_CAPABILITY, &endpoint_fec, 1) != 1)
>> -			return NULL;
>> -		if (drm_dp_dpcd_read(&immediate_upstream_port->aux,
>> -				     DP_DSC_SUPPORT, &upstream_dsc, 1) != 1)
>> -			return NULL;
>> -
>> -		/* Enpoint decompression with DP-to-DP peer device */
>> -		if ((endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED) &&
>> -		    (endpoint_fec & DP_FEC_CAPABLE) &&
>> -		    (upstream_dsc & DP_DSC_PASSTHROUGH_IS_SUPPORTED)) {
>> -			port->passthrough_aux = &immediate_upstream_port->aux;
>> -			return &port->aux;
>> -		}
>> +	if (drm_dp_dpcd_read(upstream_aux, DP_DSC_SUPPORT, &upstream_dsc, 1) != 1) {
>> +		drm_err(mgr->dev, "MST_DSC Can't retrieve dsc caps from upstream port\n");
>> +		goto out_dsc_fail;
>> +	}
>>  
>> -		/* Virtual DPCD decompression with DP-to-DP peer device */
>> -		return &immediate_upstream_port->aux;
>> +	/* Consider passthrough as the first option for dsc_aux/dsc_passthrough_aux */
>> +	if (endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED &&
>> +			upstream_dsc & DP_DSC_PASSTHROUGH_IS_SUPPORTED) {
>> +		dsc_port = port;
>> +		port->dsc_aux = &port->aux;
>> +		port->dsc_passthrough_aux = upstream_aux;
>> +		drm_info(mgr->dev, "MST_DSC dsc passthrough virtual peer device and decompression at endpoint\n");
>>  	}
>>  
>> -	/* Virtual DPCD decompression with DP-to-HDMI or Virtual DP Sink */
>> -	if (drm_dp_mst_is_virtual_dpcd(port))
>> -		return &port->aux;
>> +	if (!dsc_port) {
>> +		if (!immediate_upstream_port) {
>> +			/* Topology with 1 mstb only
>> +			 * directly connected dp2.0 case which is enumerated as 1-to-1
>> +			 * sst branch device that output pdt is stream sink
>> +			 */
>> +			if (upstream_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED)
>> +				port->dsc_aux = mgr->aux;
>>  
>> -	/*
>> -	 * Synaptics quirk
>> -	 * Applies to ports for which:
>> -	 * - Physical aux has Synaptics OUI
>> -	 * - DPv1.4 or higher
>> -	 * - Port is on primary branch device
>> -	 * - Not a VGA adapter (DP_DWN_STRM_PORT_TYPE_ANALOG)
>> -	 */
>> -	if (immediate_upstream_port)
>> -		immediate_upstream_aux = &immediate_upstream_port->aux;
>> -	else
>> -		immediate_upstream_aux = port->mgr->aux;
>> +			if (!port->dsc_aux) {
>> +				drm_err(mgr->dev, "MST_DSC dsc decompression not support at root branch\n");
>> +				goto out_dsc_fail;
>> +			}
>>  
>> -	if (drm_dp_read_desc(immediate_upstream_aux, &desc, true))
>> -		return NULL;
>> +			drm_info(mgr->dev, "MST_DSC topology with 1 mstb only, dsc decompression at root branch\n");
>> +		} else {
>> +			/* Topology with multiple mstbs */
>> +			dsc_port = immediate_upstream_port;
>> +			endpoint_dsc = upstream_dsc;
>> +
>> +			if (endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED)
>> +				port->dsc_aux = &dsc_port->aux;
>> +			else {
>> +				drm_err(mgr->dev,
>> +					"MST_DSC dsc decompression not support at immediate_upstream_port %p\n",
>> +					dsc_port);
>> +				goto out_dsc_fail;
>> +			}
>>  
>> -	if (drm_dp_has_quirk(&desc, DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD)) {
>> -		u8 dpcd_ext[DP_RECEIVER_CAP_SIZE];
>> +			drm_info(mgr->dev, "MST_DSC topology with multiple mstbs, dsc decompression at virtual peer device\n");
>> +		}
>> +	}
>>  
>> -		if (drm_dp_read_dpcd_caps(immediate_upstream_aux, dpcd_ext) < 0)
>> -			return NULL;
>> +	/* Check the virtual channel from source till dsc port link support FEC */
>> +	fec_port = dsc_port;
>> +	while (fec_port) {
>> +		/*
>> +		 * Each link between the output and the source
>> +		 * must support FEC. Note that virtual dpcd fec is identical
>> +		 * to the fec capability of it's MST BU's DPRx
>> +		 */
>> +		if (!fec_port->fec_capable) {
>> +			/* enum path result of virtual peer device will return fec_capable as false */
>> +			if ((drm_dp_dpcd_read(&fec_port->aux, DP_FEC_CAPABILITY, &fec_cap, 1) != 1) ||
>> +					!(fec_cap & DP_FEC_CAPABLE)) {
>> +				drm_err(mgr->dev, "MST_DSC Failed to retrieve fec caps at port %p\n", fec_port);
>> +				goto out_dsc_fail;
>> +			}
>> +			fec_port->fec_capable = true;
>> +		}
>>  
>> -		if (dpcd_ext[DP_DPCD_REV] >= DP_DPCD_REV_14 &&
>> -		    ((dpcd_ext[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) &&
>> -		    ((dpcd_ext[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_TYPE_MASK)
>> -		     != DP_DWN_STRM_PORT_TYPE_ANALOG)))
>> -			return immediate_upstream_aux;
>> +		fec_port = fec_port->parent->port_parent;
>>  	}
>>  
>> -	/*
>> -	 * The check below verifies if the MST sink
>> -	 * connected to the GPU is capable of DSC -
>> -	 * therefore the endpoint needs to be
>> -	 * both DSC and FEC capable.
>> -	 */
>> -	if (drm_dp_dpcd_read(&port->aux,
>> -	   DP_DSC_SUPPORT, &endpoint_dsc, 1) != 1)
>> -		return NULL;
>> -	if (drm_dp_dpcd_read(&port->aux,
>> -	   DP_FEC_CAPABILITY, &endpoint_fec, 1) != 1)
>> -		return NULL;
>> -	if ((endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED) &&
>> -	   (endpoint_fec & DP_FEC_CAPABLE))
>> -		return &port->aux;
>> +	/* Ensure fec between source and the connected DPRx */
>> +	if ((drm_dp_dpcd_read(mgr->aux, DP_FEC_CAPABILITY, &fec_cap, 1) != 1) ||
>> +			!(fec_cap & DP_FEC_CAPABLE)) {
>> +		drm_err(mgr->dev, "MST_DSC fec not supported between source and the connected DPRx\n");
>> +		goto out_dsc_fail;
>> +	}
>>  
>> -	return NULL;
>> +	return;
>> +
>> +out_dsc_fail:
>> +	port->dsc_aux = NULL;
>> +	port->dsc_passthrough_aux = NULL;
>> +	return;
>>  }
>>  EXPORT_SYMBOL(drm_dp_mst_dsc_aux_for_port);
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index a1fcedfd404b..d39a7c6f5bfa 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -3219,7 +3219,7 @@ intel_dp_sink_set_dsc_passthrough(const struct intel_connector *connector,
>>  {
>>  	struct drm_i915_private *i915 = to_i915(connector->base.dev);
>>  	struct drm_dp_aux *aux = connector->port ?
>> -				 connector->port->passthrough_aux : NULL;
>> +				 connector->port->dsc_passthrough_aux : NULL;
>>  
>>  	if (!aux)
>>  		return;
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> index 15541932b809..73cb1c673525 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> @@ -1699,7 +1699,8 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo
>>  
>>  	intel_dp_init_modeset_retry_work(intel_connector);
>>  
>> -	intel_connector->dp.dsc_decompression_aux = drm_dp_mst_dsc_aux_for_port(port);
>> +	drm_dp_mst_dsc_aux_for_port(port);
>> +	intel_connector->dp.dsc_decompression_aux = port->dsc_aux;
>>  	intel_dp_mst_read_decompression_port_dsc_caps(intel_dp, intel_connector);
>>  	intel_connector->dp.dsc_hblank_expansion_quirk =
>>  		detect_dsc_hblank_expansion_quirk(intel_connector);
>> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
>> index a6f8b098c56f..fa454506866b 100644
>> --- a/include/drm/display/drm_dp.h
>> +++ b/include/drm/display/drm_dp.h
>> @@ -980,6 +980,7 @@
>>  #define DP_BRANCH_REVISION_START            0x509
>>  #define DP_BRANCH_HW_REV                    0x509
>>  #define DP_BRANCH_SW_REV                    0x50A
>> +#define DP_BRANCH_VENDOR_SPECIFIC_START     0x50C
>>  
>>  /* Link/Sink Device Power Control */
>>  #define DP_SET_POWER                        0x600
>> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
>> index 279624833ea9..5eb583004d00 100644
>> --- a/include/drm/display/drm_dp_helper.h
>> +++ b/include/drm/display/drm_dp_helper.h
>> @@ -643,6 +643,7 @@ struct drm_dp_dpcd_ident {
>>  	u8 hw_rev;
>>  	u8 sw_major_rev;
>>  	u8 sw_minor_rev;
>> +	u8 vendor_data[4];
>>  } __packed;
>>  
>>  /**
>> @@ -711,6 +712,13 @@ enum drm_dp_quirk {
>>  	 * requires enabling DSC.
>>  	 */
>>  	DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC,
>> +	/**
>> +	 * @DP_DPCD_QUIRK_DSC_MUST_ROOT_DECOMPRESSION:
>> +	 *
>> +	 * The device has internal virutual dpcd with dsc decoding cap,
>> +	 * but dsc decoding must be done at root mstb.
>> +	 */
>> +	DP_DPCD_QUIRK_DSC_MUST_ROOT_DECOMPRESSION,
>>  };
>>  
>>  /**
>> diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
>> index f6a1cbb0f600..b04ca4a97af2 100644
>> --- a/include/drm/display/drm_dp_mst_helper.h
>> +++ b/include/drm/display/drm_dp_mst_helper.h
>> @@ -135,7 +135,8 @@ struct drm_dp_mst_port {
>>  	 */
>>  	struct drm_dp_mst_branch *mstb;
>>  	struct drm_dp_aux aux; /* i2c bus for this port? */
>> -	struct drm_dp_aux *passthrough_aux;
>> +	struct drm_dp_aux *dsc_aux;
>> +	struct drm_dp_aux *dsc_passthrough_aux;
>>  	struct drm_dp_mst_branch *parent;
>>  
>>  	struct drm_connector *connector;
>> @@ -956,7 +957,7 @@ bool drm_dp_mst_port_is_logical(struct drm_dp_mst_port *port)
>>  }
>>  
>>  struct drm_dp_aux *drm_dp_mst_aux_for_parent(struct drm_dp_mst_port *port);
>> -struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port);
>> +void drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port);
>>  
>>  static inline struct drm_dp_mst_topology_state *
>>  to_drm_dp_mst_topology_state(struct drm_private_state *state)
> 



More information about the amd-gfx mailing list