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

[Pekka Paalanen <pekka.paalanen@collabora.com>]

[-- 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 --]