[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0d56fa79-94b3-414e-a3c9-5b142d116ec9@amd.com>
Date: Fri, 6 Feb 2026 13:39:04 -0500
From: Harry Wentland <harry.wentland@....com>
To: Tomasz Pakuła <tomasz.pakula.oficjalny@...il.com>,
alexander.deucher@....com, sunpeng.li@....com
Cc: maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
tzimmermann@...e.de, airlied@...il.com, simona@...ll.ch,
siqueira@...lia.com, dri-devel@...ts.freedesktop.org,
amd-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
bernhard.berger@...il.com, michel.daenzer@...lbox.org, daniel@...ishbar.org,
admin@...1337.dev
Subject: Re: [PATCH v3 05/19] drm/amd/display: Refactor PCON VRR compatibility
check
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@...il.com>
> ---
> .../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);
>
Powered by blists - more mailing lists