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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 19 Sep 2024, Fangzhi Zuo <Jerry.Zuo@xxxxxxx> 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.

BR,
Jani.

>
> Signed-off-by: Fangzhi Zuo <Jerry.Zuo@xxxxxxx>
> Signed-off-by: Wayne Lin <wayne.lin@xxxxxxx>
> ---
>  .../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)

-- 
Jani Nikula, Intel



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux