On 2024-09-20 01:56, Jani Nikula wrote:
> On Thu, 19 Sep 2024, Fangzhi Zuo <jerry....@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....@amd.com>
>> Signed-off-by: Wayne Lin <wayne....@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)
>
