[PATCH 1/3] amdgpu_dm: also send FreeSync packets on DisplayPort connectors

[PATCH 1/3] amdgpu_dm: also send FreeSync packets on DisplayPort connectors

[Xaver Hugl]

Afaict there's no reason why it wasn't done before, and this is required
for later FreeSync 2 HDR enablement.

Signed-off-by: Xaver Hugl <xaver.hugl@kde.org>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 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 163780030eb1..1660169ae5aa 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9048,7 +9048,10 @@ static void update_freesync_state_on_stream(
 
 	aconn = (struct amdgpu_dm_connector *)new_stream->dm_stream_context;
 
-	if (aconn && (aconn->as_type == FREESYNC_TYPE_PCON_IN_WHITELIST || aconn->vsdb_info.replay_mode)) {
+	if (aconn && (aconn->as_type == FREESYNC_TYPE_PCON_IN_WHITELIST ||
+		     (aconn->as_type == ADAPTIVE_SYNC_TYPE_DP &&
+			aconn->vsdb_info.amd_vsdb_version != 0) ||
+		      aconn->vsdb_info.replay_mode)) {
 		pack_sdp_v1_3 = aconn->pack_sdp_v1_3;
 
 		if (aconn->vsdb_info.amd_vsdb_version == 1)
@@ -9058,8 +9061,9 @@ static void update_freesync_state_on_stream(
 		else if (aconn->vsdb_info.amd_vsdb_version == 3)
 			packet_type = PACKET_TYPE_FS_V3;
 
-		mod_build_adaptive_sync_infopacket(new_stream, aconn->as_type, NULL,
-					&new_stream->adaptive_sync_infopacket);
+		if (aconn->as_type != ADAPTIVE_SYNC_TYPE_DP)
+			mod_build_adaptive_sync_infopacket(new_stream, aconn->as_type, NULL,
+							   &new_stream->adaptive_sync_infopacket);
 	}
 
 	mod_freesync_build_vrr_infopacket(
-- 
2.51.1

[PATCH 2/3] amdgpu_dm: also look for the AMD VSDB in CEA extensions

[Xaver Hugl]

This makes FreeSync 2 work on some DisplayPort monitors that are lacking the
vsdb in DisplayID extensions.

Signed-off-by: Xaver Hugl <xaver.hugl@kde.org>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 ++++++++++++-------
 1 file changed, 15 insertions(+), 9 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 1660169ae5aa..6597fe07e984 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -12557,9 +12557,9 @@ static int parse_amd_vsdb(struct amdgpu_dm_connector *aconnector,
 	return false;
 }
 
-static int parse_hdmi_amd_vsdb(struct amdgpu_dm_connector *aconnector,
-			       const struct edid *edid,
-			       struct amdgpu_hdmi_vsdb_info *vsdb_info)
+static int parse_cea_amd_vsdb(struct amdgpu_dm_connector *aconnector,
+			      const struct edid *edid,
+			      struct amdgpu_hdmi_vsdb_info *vsdb_info)
 {
 	u8 *edid_ext = NULL;
 	int i;
@@ -12657,16 +12657,22 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
 				freesync_capable = true;
 		}
 
-		parse_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
-
-		if (vsdb_info.replay_mode) {
+		i = parse_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
+		if (i <= 0) {
+			/* Some DP monitors have the AMD VSDB in the CEA-861 block instead */
+			i = parse_cea_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
+		}
+		if (i >= 0) {
 			amdgpu_dm_connector->vsdb_info.replay_mode = vsdb_info.replay_mode;
 			amdgpu_dm_connector->vsdb_info.amd_vsdb_version = vsdb_info.amd_vsdb_version;
-			amdgpu_dm_connector->as_type = ADAPTIVE_SYNC_TYPE_EDP;
+			if (sink->sink_signal == SIGNAL_TYPE_EDP)
+			    amdgpu_dm_connector->as_type = ADAPTIVE_SYNC_TYPE_EDP;
+			else
+			    amdgpu_dm_connector->as_type = ADAPTIVE_SYNC_TYPE_DP;
 		}
 
 	} else if (drm_edid && sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A) {
-		i = parse_hdmi_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
+		i = parse_cea_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
 		if (i >= 0 && vsdb_info.freesync_supported) {
 			amdgpu_dm_connector->min_vfreq = vsdb_info.min_refresh_rate_hz;
 			amdgpu_dm_connector->max_vfreq = vsdb_info.max_refresh_rate_hz;
@@ -12682,7 +12688,7 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
 		as_type = dm_get_adaptive_sync_support_type(amdgpu_dm_connector->dc_link);
 
 	if (as_type == FREESYNC_TYPE_PCON_IN_WHITELIST) {
-		i = parse_hdmi_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
+		i = parse_cea_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
 		if (i >= 0 && vsdb_info.freesync_supported && vsdb_info.amd_vsdb_version > 0) {
 
 			amdgpu_dm_connector->pack_sdp_v1_3 = true;
-- 
2.51.1

[PATCH 3/3] drm, amdgpu: add the "FreeSync HDR Mode" connector property

[Xaver Hugl]

This property allows userspace to make the driver signal the display that it
should switch to FreeSync 2 HDR mode, which uses the native primaries and a
gamma 2.2 transfer function, instead of BT2020 + PQ in the more common HDR
mode.
FreeSync HDR provides the big benefit that display behavior is more
predictable, and the "native" signal doesn't waste any color resolution on
out of bounds colors or luminance values.

The property has two values right now, "Disabled" and "FreeSync 2 Native".
If any other FreeSync HDR modes exist or will be added at some point, they
can be added as new enum values as well.

Signed-off-by: Xaver Hugl <xaver.hugl@kde.org>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +++++-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  1 +
 drivers/gpu/drm/drm_atomic_uapi.c             |  4 ++
 drivers/gpu/drm/drm_connector.c               | 45 +++++++++++++++++++
 include/drm/drm_connector.h                   | 18 ++++++++
 5 files changed, 81 insertions(+), 1 deletion(-)

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 6597fe07e984..82951c53562b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8494,6 +8494,10 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 		if (adev->dm.hdcp_workqueue)
 			drm_connector_attach_content_protection_property(&aconnector->base, true);
 	}
+
+	if (connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
+		drm_connector_attach_freesync_hdr_property(&aconnector->base);
+	}
 }
 
 static int amdgpu_dm_i2c_xfer(struct i2c_adapter *i2c_adap,
@@ -9011,6 +9015,7 @@ static void update_freesync_state_on_stream(
 	bool pack_sdp_v1_3 = false;
 	struct amdgpu_dm_connector *aconn;
 	enum vrr_packet_type packet_type = PACKET_TYPE_VRR;
+	enum color_transfer_func tf = TRANSFER_FUNC_UNKNOWN;
 
 	if (!new_stream)
 		return;
@@ -9066,12 +9071,16 @@ static void update_freesync_state_on_stream(
 							   &new_stream->adaptive_sync_infopacket);
 	}
 
+	if (aconn && aconn->freesync_hdr_mode == FREESYNC_2_HDR_NATIVE) {
+		tf = TRANSFER_FUNC_GAMMA_22;
+	}
+
 	mod_freesync_build_vrr_infopacket(
 		dm->freesync_module,
 		new_stream,
 		&vrr_params,
 		packet_type,
-		TRANSFER_FUNC_UNKNOWN,
+		tf,
 		&vrr_infopacket,
 		pack_sdp_v1_3);
 
@@ -10291,6 +10300,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 		struct dc_stream_update stream_update;
 		struct dc_info_packet hdr_packet;
 		struct dc_stream_status *status = NULL;
+		struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);
 		bool abm_changed, hdr_changed, scaling_changed, output_color_space_changed = false;
 
 		memset(&stream_update, 0, sizeof(stream_update));
@@ -10300,6 +10310,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
 			old_crtc_state = drm_atomic_get_old_crtc_state(state, &acrtc->base);
 		}
 
+		aconnector->freesync_hdr_mode = dm_new_con_state->base.freesync_hdr_mode;
+
 		/* Skip any modesets/resets */
 		if (!acrtc || drm_atomic_crtc_needs_modeset(new_crtc_state))
 			continue;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index b937da0a4e4a..b9b669efc075 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -795,6 +795,7 @@ struct amdgpu_dm_connector {
 	bool pack_sdp_v1_3;
 	enum adaptive_sync_type as_type;
 	struct amdgpu_hdmi_vsdb_info vsdb_info;
+	enum freesync_hdr_mode freesync_hdr_mode;
 };
 
 static inline void amdgpu_dm_set_mst_status(uint8_t *status,
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 85dbdaa4a2e2..c56ca11322c1 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -780,6 +780,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 		state->privacy_screen_sw_state = val;
 	} else if (property == connector->broadcast_rgb_property) {
 		state->hdmi.broadcast_rgb = val;
+	} else if (property == connector->freesync_hdr_property) {
+		state->freesync_hdr_mode = val;
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
@@ -865,6 +867,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = state->privacy_screen_sw_state;
 	} else if (property == connector->broadcast_rgb_property) {
 		*val = state->hdmi.broadcast_rgb;
+	} else if (property == connector->freesync_hdr_property) {
+		*val = state->freesync_hdr_mode;
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 272d6254ea47..93727992f757 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1802,6 +1802,15 @@ EXPORT_SYMBOL(drm_hdmi_connector_get_output_format_name);
  *
  *	Drivers can set up these properties by calling
  *	drm_mode_create_tv_margin_properties().
+ *
+ * FreeSync HDR Mode:
+ * 	This optional property allows userspace to signal the display to switch
+ * 	into FreeSync 2 HDR mode, which assumes a colorspace with native
+ * 	primaries and a gamma 2.2 transfer function with min and max luminance
+ * 	matching the display.
+ * 	Like with HDR_OUTPUT_METADATA, it is up to userspace to find out which
+ * 	mode the display supports, and which primaries and luminance levels it
+ * 	has to use.
  */
 
 int drm_connector_create_standard_properties(struct drm_device *dev)
@@ -2947,6 +2956,42 @@ bool drm_connector_atomic_hdr_metadata_equal(struct drm_connector_state *old_sta
 }
 EXPORT_SYMBOL(drm_connector_atomic_hdr_metadata_equal);
 
+static const struct drm_prop_enum_list freesync_hdr_mode_names[] = {
+	{ FREESYNC_HDR_DISABLED, "Disabled" },
+	{ FREESYNC_2_HDR_NATIVE, "FreeSync 2 Native" },
+};
+
+/**
+ * drm_connector_attach_freesync_hdr_property - attach "FreeSync HDR Mode"property
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to enable or disable FreeSync HDR.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_freesync_hdr_property(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop = connector->freesync_hdr_property;
+	if (!prop) {
+		prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
+						"FreeSync HDR Mode",
+						freesync_hdr_mode_names,
+						ARRAY_SIZE(freesync_hdr_mode_names));
+		if (!prop)
+			return -EINVAL;
+
+		connector->freesync_hdr_property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop,
+				   FREESYNC_HDR_DISABLED);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_freesync_hdr_property);
+
 /**
  * drm_connector_set_vrr_capable_property - sets the variable refresh rate
  * capable property for a connector
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8f34f4b8183d..33e557a2d985 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -462,6 +462,11 @@ enum drm_privacy_screen_status {
 	PRIVACY_SCREEN_ENABLED_LOCKED,
 };
 
+enum freesync_hdr_mode {
+    FREESYNC_HDR_DISABLED = 0,
+    FREESYNC_2_HDR_NATIVE = 1,
+};
+
 /**
  * enum drm_colorspace - color space
  *
@@ -1149,6 +1154,12 @@ struct drm_connector_state {
 	 * @drm_atomic_helper_connector_hdmi_check().
 	 */
 	struct drm_connector_hdmi_state hdmi;
+
+	/**
+	* @freesync_hdr_mode: Connector property to enable
+	* or disable FreeSync HDR
+	*/
+	enum freesync_hdr_mode freesync_hdr_mode;
 };
 
 struct drm_connector_hdmi_audio_funcs {
@@ -2103,6 +2114,12 @@ struct drm_connector {
 	 */
 	struct drm_property *broadcast_rgb_property;
 
+	/**
+	* @freesync_hdr_property: Connector property to enable
+	* or disable FreeSync HDR
+	*/
+	struct drm_property *freesync_hdr_property;
+
 #define DRM_CONNECTOR_POLL_HPD (1 << 0)
 #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -2453,6 +2470,7 @@ int drm_connector_attach_colorspace_property(struct drm_connector *connector);
 int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *connector);
 bool drm_connector_atomic_hdr_metadata_equal(struct drm_connector_state *old_state,
 					     struct drm_connector_state *new_state);
+int drm_connector_attach_freesync_hdr_property(struct drm_connector *connector);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
 int drm_mode_create_hdmi_colorspace_property(struct drm_connector *connector,
 					     u32 supported_colorspaces);
-- 
2.51.1

Re: [PATCH 3/3] drm, amdgpu: add the "FreeSync HDR Mode" connector property

[Xaver Hugl]

Note that I've only tested this on a single monitor so far, a Samsung
C49RG9, which is the only one I have that supports FreeSync 2 HDR. It
would be nice if more people could test it; the compositor side of
this is at https://invent.kde.org/plasma/kwin/-/merge_requests/8423

Also worth mentioning is
https://gitlab.freedesktop.org/emersion/libdisplay-info/-/issues/51,
if anyone wants to help out with figuring out the userspace side of
detecting whether or not a given display is actually FreeSync HDR
capable.

- Xaver

Re: [PATCH 3/3] drm,amdgpu: add the "FreeSync HDR Mode" connector property

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 6117 bytes --]

On Wed, 12 Nov 2025 16:18:32 +0100
Xaver Hugl <xaver.hugl@kde.org> wrote:

> This property allows userspace to make the driver signal the display that it
> should switch to FreeSync 2 HDR mode, which uses the native primaries and a
> gamma 2.2 transfer function, instead of BT2020 + PQ in the more common HDR
> mode.

Hi Xaver,

that's awesome!

> FreeSync HDR provides the big benefit that display behavior is more
> predictable, and the "native" signal doesn't waste any color resolution on
> out of bounds colors or luminance values.
> 
> The property has two values right now, "Disabled" and "FreeSync 2 Native".
> If any other FreeSync HDR modes exist or will be added at some point, they
> can be added as new enum values as well.

How should this interact with the connector properties "Colorspace" and
"HDR_OUTPUT_METADATA"?

Does one override the other when they disagree? Is userspace
expected to program all of them into agreement? Should the atomic
commit fail if they disagree?

What about instead of a new property, make a new value called "Native"
for "Colorspace", and require userspace to set HDR_OUTPUT_METADATA eotf
field to "traditional gamma HDR"? This might be a silly idea, but I'd
like to hear why. Alternatively, HDR_OUTPUT_METADATA could use a new
'metadata_type' value for the eotf.


Thanks,
pq

> Signed-off-by: Xaver Hugl <xaver.hugl@kde.org>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +++++-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  1 +
>  drivers/gpu/drm/drm_atomic_uapi.c             |  4 ++
>  drivers/gpu/drm/drm_connector.c               | 45 +++++++++++++++++++
>  include/drm/drm_connector.h                   | 18 ++++++++
>  5 files changed, 81 insertions(+), 1 deletion(-)

...

> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 272d6254ea47..93727992f757 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1802,6 +1802,15 @@ EXPORT_SYMBOL(drm_hdmi_connector_get_output_format_name);
>   *
>   *	Drivers can set up these properties by calling
>   *	drm_mode_create_tv_margin_properties().
> + *
> + * FreeSync HDR Mode:
> + * 	This optional property allows userspace to signal the display to switch
> + * 	into FreeSync 2 HDR mode, which assumes a colorspace with native
> + * 	primaries and a gamma 2.2 transfer function with min and max luminance
> + * 	matching the display.
> + * 	Like with HDR_OUTPUT_METADATA, it is up to userspace to find out which
> + * 	mode the display supports, and which primaries and luminance levels it
> + * 	has to use.
>   */
>  
>  int drm_connector_create_standard_properties(struct drm_device *dev)
> @@ -2947,6 +2956,42 @@ bool drm_connector_atomic_hdr_metadata_equal(struct drm_connector_state *old_sta
>  }
>  EXPORT_SYMBOL(drm_connector_atomic_hdr_metadata_equal);
>  
> +static const struct drm_prop_enum_list freesync_hdr_mode_names[] = {
> +	{ FREESYNC_HDR_DISABLED, "Disabled" },
> +	{ FREESYNC_2_HDR_NATIVE, "FreeSync 2 Native" },
> +};
> +
> +/**
> + * drm_connector_attach_freesync_hdr_property - attach "FreeSync HDR Mode"property
> + * @connector: connector to attach the property on.
> + *
> + * This is used to allow the userspace to enable or disable FreeSync HDR.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_attach_freesync_hdr_property(struct drm_connector *connector)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop = connector->freesync_hdr_property;
> +	if (!prop) {
> +		prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
> +						"FreeSync HDR Mode",
> +						freesync_hdr_mode_names,
> +						ARRAY_SIZE(freesync_hdr_mode_names));
> +		if (!prop)
> +			return -EINVAL;
> +
> +		connector->freesync_hdr_property = prop;
> +	}
> +
> +	drm_object_attach_property(&connector->base, prop,
> +				   FREESYNC_HDR_DISABLED);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_freesync_hdr_property);
> +
>  /**
>   * drm_connector_set_vrr_capable_property - sets the variable refresh rate
>   * capable property for a connector
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 8f34f4b8183d..33e557a2d985 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -462,6 +462,11 @@ enum drm_privacy_screen_status {
>  	PRIVACY_SCREEN_ENABLED_LOCKED,
>  };
>  
> +enum freesync_hdr_mode {
> +    FREESYNC_HDR_DISABLED = 0,
> +    FREESYNC_2_HDR_NATIVE = 1,
> +};
> +
>  /**
>   * enum drm_colorspace - color space
>   *
> @@ -1149,6 +1154,12 @@ struct drm_connector_state {
>  	 * @drm_atomic_helper_connector_hdmi_check().
>  	 */
>  	struct drm_connector_hdmi_state hdmi;
> +
> +	/**
> +	* @freesync_hdr_mode: Connector property to enable
> +	* or disable FreeSync HDR
> +	*/
> +	enum freesync_hdr_mode freesync_hdr_mode;
>  };
>  
>  struct drm_connector_hdmi_audio_funcs {
> @@ -2103,6 +2114,12 @@ struct drm_connector {
>  	 */
>  	struct drm_property *broadcast_rgb_property;
>  
> +	/**
> +	* @freesync_hdr_property: Connector property to enable
> +	* or disable FreeSync HDR
> +	*/
> +	struct drm_property *freesync_hdr_property;
> +
>  #define DRM_CONNECTOR_POLL_HPD (1 << 0)
>  #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
>  #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
> @@ -2453,6 +2470,7 @@ int drm_connector_attach_colorspace_property(struct drm_connector *connector);
>  int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *connector);
>  bool drm_connector_atomic_hdr_metadata_equal(struct drm_connector_state *old_state,
>  					     struct drm_connector_state *new_state);
> +int drm_connector_attach_freesync_hdr_property(struct drm_connector *connector);
>  int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
>  int drm_mode_create_hdmi_colorspace_property(struct drm_connector *connector,
>  					     u32 supported_colorspaces);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/3] drm, amdgpu: add the "FreeSync HDR Mode" connector property

[Xaver Hugl]

> > FreeSync HDR provides the big benefit that display behavior is more
> > predictable, and the "native" signal doesn't waste any color resolution on
> > out of bounds colors or luminance values.
> >
> > The property has two values right now, "Disabled" and "FreeSync 2 Native".
> > If any other FreeSync HDR modes exist or will be added at some point, they
> > can be added as new enum values as well.
>
> How should this interact with the connector properties "Colorspace" and
> "HDR_OUTPUT_METADATA"?
From experimentation, my display seems to ignore the colorspace and
"traditional gamma SDR" in HDR_OUTPUT_METADATA while FreeSync HDR is
active.
Setting the eotf to PQ however turns FreeSync HDR off and switches the
display into conventional HDR mode.

Making any definite statements about this will require someone from
AMD to provide documentation, which is why I didn't put anything
explicit about it into the patch.

> Does one override the other when they disagree? Is userspace
> expected to program all of them into agreement? Should the atomic
> commit fail if they disagree?
Just like with HDR_OUTPUT_METADATA, it should just be up to userspace
to set things up in a way that works with the display.

> What about instead of a new property, make a new value called "Native"
> for "Colorspace", and require userspace to set HDR_OUTPUT_METADATA eotf
> field to "traditional gamma HDR"? This might be a silly idea, but I'd
> like to hear why. Alternatively, HDR_OUTPUT_METADATA could use a new
> 'metadata_type' value for the eotf.
I thought (and hoped) it would end up being just like that, but at
least my display doesn't support the "traditional gamma HDR" at all.
If you set it anyways, it seems to just fall back to "traditional
gamma SDR".

As HDR output metadata and colorspace have been more or less just
passed through to the display without modifications so far, I'd rather
not have the kernel step in between and try to translate things -
especially not if there's actually displays out there that do support
the "traditional gamma HDR", in which case that value may become
ambiguous.

- Xaver