Re: [PATCH v3 05/19] drm/amd/display: Refactor PCON VRR compatibility check

[Harry Wentland]

On 2026-02-03 13:56, Tomasz Pakuła wrote:
> [Why]
> DP->HDMI PCONs prevously entered the DP path

Again, not a fan of changing behavior in a "Refactor" commit.
Maybe drop refactor and put the functional change in the title
instead? And/or disentangle the non-functional and functional
portions of the change into two separate commits. Though I'm not
set on separating this into two commits.

Otherwise the change looks good.

Harry

> 
> [How]
> Restructure amdgpu_dm_update_freesync_caps() and move
> dm_get_adaptive_sync_support_type() to dm_helpers_is_vrr_pcon_allowed()
> to better reflect what this function does. It never actually gave us any
> other info.
> 
> Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@xxxxxxxxx>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 +++++++-----
>  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 39 ++++++-------------
>  drivers/gpu/drm/amd/display/dc/dm_helpers.h   |  2 +-
>  3 files changed, 27 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 284f5b326c18..9346b62d981b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -13278,7 +13278,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  	struct dpcd_caps dpcd_caps = {0};
>  	const struct edid *edid;
>  	bool freesync_capable = false;
> -	enum adaptive_sync_type as_type = ADAPTIVE_SYNC_TYPE_NONE;
> +	bool pcon_allowed = false;
> +	bool is_pcon = false;
>  
>  	if (!connector->state) {
>  		drm_err(adev_to_drm(adev), "%s - Connector has no state", __func__);
> @@ -13306,18 +13307,23 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  	if (!adev->dm.freesync_module || !dc_supports_vrr(sink->ctx->dce_version))
>  		goto update;
>  
> +	/* Gather all data */
>  	edid = drm_edid_raw(drm_edid); // FIXME: Get rid of drm_edid_raw()
>  	parse_amd_vsdb_cea(amdgpu_dm_connector, edid, &vsdb_info);
>  
> -	if (amdgpu_dm_connector->dc_link)
> +	if (amdgpu_dm_connector->dc_link) {
>  		dpcd_caps = amdgpu_dm_connector->dc_link->dpcd_caps;
> +		is_pcon = dpcd_caps.dongle_type == DISPLAY_DONGLE_DP_HDMI_CONVERTER;
> +		pcon_allowed = dm_helpers_is_vrr_pcon_allowed(amdgpu_dm_connector->dc_link);
> +	}
>  
>  	/* Some eDP panels only have the refresh rate range info in DisplayID */
>  	if (is_monitor_range_invalid(connector))
>  		parse_edid_displayid_vrr(connector, edid);
>  
> -	if (sink->sink_signal == SIGNAL_TYPE_DISPLAY_PORT ||
> -	    sink->sink_signal == SIGNAL_TYPE_EDP) {
> +	/* DP & eDP excluding PCONs */
> +	if ((sink->sink_signal == SIGNAL_TYPE_EDP ||
> +	     sink->sink_signal == SIGNAL_TYPE_DISPLAY_PORT) && !is_pcon) {
>  		/*
>  		 * Many monitors expose AMD vsdb in CAE even for DP and their
>  		 * monitor ranges do not contain Range Limits Only flag
> @@ -13342,17 +13348,15 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  			amdgpu_dm_connector->as_type = ADAPTIVE_SYNC_TYPE_EDP;
>  		}
>  
> +	/* HDMI */
>  	} else if (sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A && vsdb_info.freesync_supported) {
>  		monitor_range_from_vsdb(&connector->display_info, &vsdb_info);
>  		freesync_capable = copy_range_to_amdgpu_connector(connector);
> -	}
>  
> -	if (amdgpu_dm_connector->dc_link)
> -		as_type = dm_get_adaptive_sync_support_type(amdgpu_dm_connector->dc_link);
> -
> -	if (as_type == FREESYNC_TYPE_PCON_IN_WHITELIST && vsdb_info.freesync_supported) {
> +	/* DP -> HDMI PCON */
> +	} else if (pcon_allowed && vsdb_info.freesync_supported) {
> +		amdgpu_dm_connector->as_type = FREESYNC_TYPE_PCON_IN_WHITELIST;
>  		amdgpu_dm_connector->pack_sdp_v1_3 = true;
> -		amdgpu_dm_connector->as_type = as_type;
>  		amdgpu_dm_connector->vsdb_info = vsdb_info;
>  
>  		monitor_range_from_vsdb(&connector->display_info, &vsdb_info);
> 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 1f41d6540b83..45a91df619d9 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
> @@ -1381,40 +1381,25 @@ void dm_helpers_dp_mst_update_branch_bandwidth(
>  	// TODO
>  }
>  
> -static bool dm_is_freesync_pcon_whitelist(const uint32_t branch_dev_id)
> +bool dm_helpers_is_vrr_pcon_allowed(const struct dc_link *link)
>  {
> -	bool ret_val = false;
> +	if (link->dpcd_caps.dongle_type != DISPLAY_DONGLE_DP_HDMI_CONVERTER)
> +		return false;
>  
> -	switch (branch_dev_id) {
> +	if (!link->dpcd_caps.allow_invalid_MSA_timing_param)
> +		return false;
> +
> +	if (!link->dpcd_caps.adaptive_sync_caps.dp_adap_sync_caps.bits.ADAPTIVE_SYNC_SDP_SUPPORT)
> +		return false;
> +
> +	switch (link->dpcd_caps.branch_dev_id) {
>  	case DP_BRANCH_DEVICE_ID_0060AD:
>  	case DP_BRANCH_DEVICE_ID_00E04C:
>  	case DP_BRANCH_DEVICE_ID_90CC24:
> -		ret_val = true;
> -		break;
> -	default:
> -		break;
> +		return true;
>  	}
>  
> -	return ret_val;
> -}
> -
> -enum adaptive_sync_type dm_get_adaptive_sync_support_type(struct dc_link *link)
> -{
> -	struct dpcd_caps *dpcd_caps = &link->dpcd_caps;
> -	enum adaptive_sync_type as_type = ADAPTIVE_SYNC_TYPE_NONE;
> -
> -	switch (dpcd_caps->dongle_type) {
> -	case DISPLAY_DONGLE_DP_HDMI_CONVERTER:
> -		if (dpcd_caps->adaptive_sync_caps.dp_adap_sync_caps.bits.ADAPTIVE_SYNC_SDP_SUPPORT == true &&
> -			dpcd_caps->allow_invalid_MSA_timing_param == true &&
> -			dm_is_freesync_pcon_whitelist(dpcd_caps->branch_dev_id))
> -			as_type = FREESYNC_TYPE_PCON_IN_WHITELIST;
> -		break;
> -	default:
> -		break;
> -	}
> -
> -	return as_type;
> +	return false;
>  }
>  
>  bool dm_helpers_is_fullscreen(struct dc_context *ctx, struct dc_stream_state *stream)
> diff --git a/drivers/gpu/drm/amd/display/dc/dm_helpers.h b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
> index 7014b8c2c956..e51f1e489129 100644
> --- a/drivers/gpu/drm/amd/display/dc/dm_helpers.h
> +++ b/drivers/gpu/drm/amd/display/dc/dm_helpers.h
> @@ -220,10 +220,10 @@ int dm_helpers_dmub_set_config_sync(struct dc_context *ctx,
>  		const struct dc_link *link,
>  		struct set_config_cmd_payload *payload,
>  		enum set_config_status *operation_result);
> -enum adaptive_sync_type dm_get_adaptive_sync_support_type(struct dc_link *link);
>  
>  enum dc_edid_status dm_helpers_get_sbios_edid(struct dc_link *link, struct dc_edid *edid);
>  
> +bool dm_helpers_is_vrr_pcon_allowed(const struct dc_link *link);
>  bool dm_helpers_is_fullscreen(struct dc_context *ctx, struct dc_stream_state *stream);
>  bool dm_helpers_is_hdr_on(struct dc_context *ctx, struct dc_stream_state *stream);
>