* [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
@ 2025-09-12 7:33 Chuanyu Tseng
2025-09-12 13:45 ` Derek Foreman
0 siblings, 1 reply; 22+ messages in thread
From: Chuanyu Tseng @ 2025-09-12 7:33 UTC (permalink / raw)
To: dri-devel
Cc: harry.wentland, Mario.Limonciello, xaver.hugl, mdaenzer, victoria,
seanpaul, Sunpeng.Li, Chuanyu Tseng
Introduce a DRM interface for DRM clients to further restrict the
VRR Range within the panel supported VRR range on a per-commit
basis.
The goal is to give DRM client the ability to do frame-doubling/
ramping themselves, or to set lower static refresh rates for power
savings.
This is done through 2 new CRTC properties, along with a client
cap. See the docstrings in patch for details.
This RFC doesn't include a driver-side implementation yet; that is
coming soon. Currently, looking to get some comments on whether this
interface makes sense for both compositor and drivers
---
drivers/gpu/drm/drm_atomic_uapi.c | 30 +++++++++++++++++++++---
drivers/gpu/drm/drm_connector.c | 25 ++++++++++++++++++++
drivers/gpu/drm/drm_file.c | 2 ++
drivers/gpu/drm/drm_ioctl.c | 7 ++++++
drivers/gpu/drm/drm_mode_config.c | 14 ++++++++++++
include/drm/drm_connector.h | 1 +
include/drm/drm_crtc.h | 38 +++++++++++++++++++++++++++++++
include/drm/drm_file.h | 10 ++++++++
include/drm/drm_mode_config.h | 18 +++++++++++++++
include/uapi/drm/drm.h | 17 ++++++++++++++
10 files changed, 159 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 85dbdaa4a2e2..73f929cff4e1 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -365,8 +365,8 @@ static s32 __user *get_out_fence_for_connector(struct drm_atomic_state *state,
}
static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
- struct drm_crtc_state *state, struct drm_property *property,
- uint64_t val)
+ struct drm_crtc_state *state, struct drm_file *file_priv,
+ struct drm_property *property, uint64_t val)
{
struct drm_device *dev = crtc->dev;
struct drm_mode_config *config = &dev->mode_config;
@@ -421,6 +421,26 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
state->scaling_filter = val;
} else if (crtc->funcs->atomic_set_property) {
return crtc->funcs->atomic_set_property(crtc, state, property, val);
+ } else if (property == config->prop_vrr_range_control_min) {
+ if (file_priv->vrr_range_control) {
+ drm_dbg_atomic(dev, "Setting vrr_range_min crtc property not"
+ "permitted with DRM_CLIENT_CAP_VRR_RANGE_CONTROL"
+ "client cap\n");
+ return -EINVAL;
+ }
+ if (!val)
+ return -EINVAL;
+ state->vrr_range_min = val;
+ } else if (property == config->prop_vrr_range_control_max) {
+ if (file_priv->vrr_range_control) {
+ drm_dbg_atomic(dev,"Setting vrr_range_max crtc property not"
+ "permitted with DRM_CLIENT_CAP_VRR_RANGE_CONTROL"
+ "client cap\n");
+ return -EINVAL;
+ }
+ if (!val)
+ return -EINVAL;
+ state->vrr_range_max = val;
} else {
drm_dbg_atomic(crtc->dev,
"[CRTC:%d:%s] unknown property [PROP:%d:%s]\n",
@@ -446,6 +466,10 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
else if (property == config->prop_vrr_enabled)
*val = state->vrr_enabled;
+ else if (property == config->prop_vrr_range_control_min)
+ *val = state->vrr_range_min;
+ else if (property == config->prop_vrr_range_control_max)
+ *val = state->vrr_range_max;
else if (property == config->degamma_lut_property)
*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
else if (property == config->ctm_property)
@@ -1062,7 +1086,7 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
}
ret = drm_atomic_crtc_set_property(crtc,
- crtc_state, prop, prop_value);
+ crtc_state, file_priv, prop, prop_value);
break;
}
case DRM_MODE_OBJECT_PLANE: {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 272d6254ea47..dc4b50ff5fe0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2866,6 +2866,31 @@ int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
}
EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+/**
+ * drm_connector_attach_vrr_range_control_property - attach "VRR_RANGE_CONTROL_MIN" and
+ * "VRR_RANGE_CONTROL_MAX" property
+ *
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to send VRR range control min and max value to the
+ * driver.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_vrr_range_control_property(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_property *prop_min = dev->mode_config.prop_vrr_range_control_min;
+ struct drm_property *prop_max = dev->mode_config.prop_vrr_range_control_max;
+
+ drm_object_attach_property(&connector->base, prop_min, 0);
+ drm_object_attach_property(&connector->base, prop_max, 0);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_vrr_range_control_property);
+
/**
* drm_connector_attach_broadcast_rgb_property - attach "Broadcast RGB" property
* @connector: connector to attach the property on.
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index eebd1a05ee97..7ed28e94544a 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -157,6 +157,8 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
init_waitqueue_head(&file->event_wait);
file->event_space = 4096; /* set aside 4k for event buffer */
+ file->vrr_range_control = false; /* set as false for init */
+
spin_lock_init(&file->master_lookup_lock);
mutex_init(&file->event_read_lock);
mutex_init(&file->client_name_lock);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index d8a24875a7ba..273139688ba1 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -373,6 +373,13 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
return -EINVAL;
file_priv->supports_virtualized_cursor_plane = req->value;
break;
+ case DRM_CLIENT_CAP_VRR_RANGE_CONTROL:
+ if (!file_priv->atomic)
+ return -EINVAL;
+ if (req->value == 0)
+ return -EINVAL;
+ file_priv->vrr_range_control = req->value;
+ break;
default:
return -EINVAL;
}
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 25f376869b3a..1f74284208c6 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -340,6 +340,20 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_vrr_enabled = prop;
+ prop = drm_property_create_range(dev,
+ DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_RANGE,
+ "VRR-RANGE-CONTROL-MIN", 0, UINT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.prop_vrr_range_control_min = prop;
+
+ prop = drm_property_create_range(dev,
+ DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_RANGE,
+ "VRR-RANGE-CONTROL-MAX", 0, UINT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.prop_vrr_range_control_max = prop;
+
prop = drm_property_create(dev,
DRM_MODE_PROP_BLOB,
"DEGAMMA_LUT", 0);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8f34f4b8183d..dd2c3337235a 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2451,6 +2451,7 @@ int drm_connector_attach_vrr_capable_property(
int drm_connector_attach_broadcast_rgb_property(struct drm_connector *connector);
int drm_connector_attach_colorspace_property(struct drm_connector *connector);
int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *connector);
+int drm_connector_attach_vrr_range_control_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_mode_create_aspect_ratio_property(struct drm_device *dev);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index caa56e039da2..39d1bf66f713 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -299,6 +299,44 @@ struct drm_crtc_state {
*/
bool vrr_enabled;
+ /** @vrr_range_min:
+ *
+ * This is desired minimal FPS number.
+ *
+ * Default state is 'vrr_range_min = 0', (and 'vrr_range_max = 0'),
+ * indicating legacy VRR_ENABLED behavior. if both are set to a non-zeo
+ * value, the new VRR range control behavior will be active. See
+ * &DRM_CLIENT_CAP_VRR_RANGE_CONTROL.
+ *
+ * If setting a non-zero value, the driver should check that:
+ *
+ * 1. Both vrr_range_min and vrr_range_max are set to a non-zero value.
+ * This indicates the driver to switch the new VRR range control
+ * behavior.
+ * 2. Both vrr_rage_min and vrr_range_max are within the panel's supported
+ * FPS range.
+ * 3. vrr_range_min is less-than-or-equal-to vrr_range_max.
+ */
+ uint16_t vrr_range_min;
+
+ /** @vrr_range_max:
+ *
+ * Default state is 'vrr_range_max = 0', (and 'vrr_range_min = 0'),
+ * indicating legacy VRR_ENABLED behavior. if both are set to a non-zeo
+ * value, the new VRR range control behavior will be active. See
+ * &DRM_CLIENT_CAP_VRR_RANGE_CONTROL.
+ *
+ * If setting a non-zero value, the driver should check that:
+ *
+ * 1. Both vrr_range_min and vrr_range_max are set to a non-zero value.
+ * This indicates the driver to switch the new VRR range control
+ * behavior.
+ * 2. Both vrr_rage_min and vrr_range_max are within the panel's supported
+ * FPS range.
+ * 3. vrr_range_min is less-than-or-equal-to vrr_range_max.
+ */
+ uint16_t vrr_range_max;
+
/**
* @self_refresh_active:
*
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 115763799625..4cb57a503a02 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -240,6 +240,16 @@ struct drm_file {
*/
bool supports_virtualized_cursor_plane;
+ /**
+ * @vrr_range_control:
+ *
+ * If set to true, the DRM driver will allow setting of the
+ * &drm_mode_config.prop_vrr_range_control_min_property and
+ * &drm_mode_config.prop_vrr_range_control_max_property CRTC
+ * properties, if the properties are supported by the driver.
+ */
+ bool vrr_range_control;
+
/**
* @master:
*
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 2e848b816218..e02dd46ca5c2 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -680,6 +680,24 @@ struct drm_mode_config {
*/
struct drm_property *prop_vrr_enabled;
+ /**
+ * @prop_vrr_range_control_min_property: Optional CRTC properties to
+ * further limit the minimum allowed refresh rate within the panel's
+ * supported refresh rate range. It's invalid to set unless the
+ * client advertises &DRM_CLIENT_CAP_VRR_RANGE_CONTROL.
+ * See also &drm_ctrc_state.vrr_range_min.
+ */
+ struct drm_property *prop_vrr_range_control_min;
+
+ /**
+ * @prop_vrr_range_control_max_property: Optional CRTC properties to
+ * further limit the maximum allowed refresh rate within the panel's
+ * supported refresh rate range. It's invalid to set unless the
+ * client advertises &DRM_CLIENT_CAP_VRR_RANGE_CONTROL.
+ * See also &drm_ctrc_state.vrr_range_max.
+ */
+ struct drm_property *prop_vrr_range_control_max;
+
/**
* @dvi_i_subconnector_property: Optional DVI-I property to
* differentiate between analog or digital mode.
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 3cd5cf15e3c9..e4d918342e67 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -906,6 +906,23 @@ struct drm_get_cap {
*/
#define DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT 6
+/**
+ * DRM_CLIENT_CAP_VRR_RANGE_CONTROL
+ *
+ * The driver shall not program a refresh rate that is:
+ * - Below the &drm_crtc_state.vrr_range_min, nor
+ * - Above the &drm_crtc_state.vrr_range_max
+ * Even if the panel supports a wider range than the range requested.
+ *
+ * Once set, the driver will allow setting of the
+ *
+ * - &drm_mode_config.prop_vrr_range_control_min and
+ * - &drm_mode_config.prop_vrr_range_control_max properties.
+ *
+ * Otherwise, setting them will be invalid.
+ */
+#define DRM_CLIENT_CAP_VRR_RANGE_CONTROL 7
+
/* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
struct drm_set_client_cap {
__u64 capability;
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-12 7:33 [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface Chuanyu Tseng
@ 2025-09-12 13:45 ` Derek Foreman
2025-09-15 10:01 ` Michel Dänzer
0 siblings, 1 reply; 22+ messages in thread
From: Derek Foreman @ 2025-09-12 13:45 UTC (permalink / raw)
To: Chuanyu Tseng, dri-devel
Cc: harry.wentland, Mario.Limonciello, xaver.hugl, mdaenzer, victoria,
seanpaul, Sunpeng.Li
On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
> Introduce a DRM interface for DRM clients to further restrict the
> VRR Range within the panel supported VRR range on a per-commit
> basis.
>
> The goal is to give DRM client the ability to do frame-doubling/
> ramping themselves, or to set lower static refresh rates for power
> savings.
I'm interested in limiting the range of VRR to enable HDMI's
QMS/CinemaVRR features - ie: switching to a fixed rate for media
playback without incurring screen blackouts/resyncs/"bonks" during the
switch.
I could see using an interface such as this to do the frame rate
limiting, by setting the lower and upper bounds both to a media file's
framerate. However for that use case it's not precise enough, as video
may have a rate like 23.9760239... FPS.
Would it be better to expose the limits as a numerator/denominator pair
so a rate can be something like 24000/1001fps?
Thanks,
Derek
> This is done through 2 new CRTC properties, along with a client
> cap. See the docstrings in patch for details.
>
> This RFC doesn't include a driver-side implementation yet; that is
> coming soon. Currently, looking to get some comments on whether this
> interface makes sense for both compositor and drivers
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 30 +++++++++++++++++++++---
> drivers/gpu/drm/drm_connector.c | 25 ++++++++++++++++++++
> drivers/gpu/drm/drm_file.c | 2 ++
> drivers/gpu/drm/drm_ioctl.c | 7 ++++++
> drivers/gpu/drm/drm_mode_config.c | 14 ++++++++++++
> include/drm/drm_connector.h | 1 +
> include/drm/drm_crtc.h | 38 +++++++++++++++++++++++++++++++
> include/drm/drm_file.h | 10 ++++++++
> include/drm/drm_mode_config.h | 18 +++++++++++++++
> include/uapi/drm/drm.h | 17 ++++++++++++++
> 10 files changed, 159 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 85dbdaa4a2e2..73f929cff4e1 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -365,8 +365,8 @@ static s32 __user *get_out_fence_for_connector(struct drm_atomic_state *state,
> }
>
> static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> - struct drm_crtc_state *state, struct drm_property *property,
> - uint64_t val)
> + struct drm_crtc_state *state, struct drm_file *file_priv,
> + struct drm_property *property, uint64_t val)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_mode_config *config = &dev->mode_config;
> @@ -421,6 +421,26 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> state->scaling_filter = val;
> } else if (crtc->funcs->atomic_set_property) {
> return crtc->funcs->atomic_set_property(crtc, state, property, val);
> + } else if (property == config->prop_vrr_range_control_min) {
> + if (file_priv->vrr_range_control) {
> + drm_dbg_atomic(dev, "Setting vrr_range_min crtc property not"
> + "permitted with DRM_CLIENT_CAP_VRR_RANGE_CONTROL"
> + "client cap\n");
> + return -EINVAL;
> + }
> + if (!val)
> + return -EINVAL;
> + state->vrr_range_min = val;
> + } else if (property == config->prop_vrr_range_control_max) {
> + if (file_priv->vrr_range_control) {
> + drm_dbg_atomic(dev,"Setting vrr_range_max crtc property not"
> + "permitted with DRM_CLIENT_CAP_VRR_RANGE_CONTROL"
> + "client cap\n");
> + return -EINVAL;
> + }
> + if (!val)
> + return -EINVAL;
> + state->vrr_range_max = val;
> } else {
> drm_dbg_atomic(crtc->dev,
> "[CRTC:%d:%s] unknown property [PROP:%d:%s]\n",
> @@ -446,6 +466,10 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> *val = (state->mode_blob) ? state->mode_blob->base.id : 0;
> else if (property == config->prop_vrr_enabled)
> *val = state->vrr_enabled;
> + else if (property == config->prop_vrr_range_control_min)
> + *val = state->vrr_range_min;
> + else if (property == config->prop_vrr_range_control_max)
> + *val = state->vrr_range_max;
> else if (property == config->degamma_lut_property)
> *val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
> else if (property == config->ctm_property)
> @@ -1062,7 +1086,7 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
> }
>
> ret = drm_atomic_crtc_set_property(crtc,
> - crtc_state, prop, prop_value);
> + crtc_state, file_priv, prop, prop_value);
> break;
> }
> case DRM_MODE_OBJECT_PLANE: {
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 272d6254ea47..dc4b50ff5fe0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2866,6 +2866,31 @@ int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
> }
> EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
>
> +/**
> + * drm_connector_attach_vrr_range_control_property - attach "VRR_RANGE_CONTROL_MIN" and
> + * "VRR_RANGE_CONTROL_MAX" property
> + *
> + * @connector: connector to attach the property on.
> + *
> + * This is used to allow the userspace to send VRR range control min and max value to the
> + * driver.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_attach_vrr_range_control_property(struct drm_connector *connector)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_property *prop_min = dev->mode_config.prop_vrr_range_control_min;
> + struct drm_property *prop_max = dev->mode_config.prop_vrr_range_control_max;
> +
> + drm_object_attach_property(&connector->base, prop_min, 0);
> + drm_object_attach_property(&connector->base, prop_max, 0);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_vrr_range_control_property);
> +
> /**
> * drm_connector_attach_broadcast_rgb_property - attach "Broadcast RGB" property
> * @connector: connector to attach the property on.
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index eebd1a05ee97..7ed28e94544a 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -157,6 +157,8 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
> init_waitqueue_head(&file->event_wait);
> file->event_space = 4096; /* set aside 4k for event buffer */
>
> + file->vrr_range_control = false; /* set as false for init */
> +
> spin_lock_init(&file->master_lookup_lock);
> mutex_init(&file->event_read_lock);
> mutex_init(&file->client_name_lock);
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index d8a24875a7ba..273139688ba1 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -373,6 +373,13 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
> return -EINVAL;
> file_priv->supports_virtualized_cursor_plane = req->value;
> break;
> + case DRM_CLIENT_CAP_VRR_RANGE_CONTROL:
> + if (!file_priv->atomic)
> + return -EINVAL;
> + if (req->value == 0)
> + return -EINVAL;
> + file_priv->vrr_range_control = req->value;
> + break;
> default:
> return -EINVAL;
> }
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 25f376869b3a..1f74284208c6 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -340,6 +340,20 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> return -ENOMEM;
> dev->mode_config.prop_vrr_enabled = prop;
>
> + prop = drm_property_create_range(dev,
> + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_RANGE,
> + "VRR-RANGE-CONTROL-MIN", 0, UINT_MAX);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.prop_vrr_range_control_min = prop;
> +
> + prop = drm_property_create_range(dev,
> + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_RANGE,
> + "VRR-RANGE-CONTROL-MAX", 0, UINT_MAX);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.prop_vrr_range_control_max = prop;
> +
> prop = drm_property_create(dev,
> DRM_MODE_PROP_BLOB,
> "DEGAMMA_LUT", 0);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 8f34f4b8183d..dd2c3337235a 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -2451,6 +2451,7 @@ int drm_connector_attach_vrr_capable_property(
> int drm_connector_attach_broadcast_rgb_property(struct drm_connector *connector);
> int drm_connector_attach_colorspace_property(struct drm_connector *connector);
> int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *connector);
> +int drm_connector_attach_vrr_range_control_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_mode_create_aspect_ratio_property(struct drm_device *dev);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index caa56e039da2..39d1bf66f713 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -299,6 +299,44 @@ struct drm_crtc_state {
> */
> bool vrr_enabled;
>
> + /** @vrr_range_min:
> + *
> + * This is desired minimal FPS number.
> + *
> + * Default state is 'vrr_range_min = 0', (and 'vrr_range_max = 0'),
> + * indicating legacy VRR_ENABLED behavior. if both are set to a non-zeo
> + * value, the new VRR range control behavior will be active. See
> + * &DRM_CLIENT_CAP_VRR_RANGE_CONTROL.
> + *
> + * If setting a non-zero value, the driver should check that:
> + *
> + * 1. Both vrr_range_min and vrr_range_max are set to a non-zero value.
> + * This indicates the driver to switch the new VRR range control
> + * behavior.
> + * 2. Both vrr_rage_min and vrr_range_max are within the panel's supported
> + * FPS range.
> + * 3. vrr_range_min is less-than-or-equal-to vrr_range_max.
> + */
> + uint16_t vrr_range_min;
> +
> + /** @vrr_range_max:
> + *
> + * Default state is 'vrr_range_max = 0', (and 'vrr_range_min = 0'),
> + * indicating legacy VRR_ENABLED behavior. if both are set to a non-zeo
> + * value, the new VRR range control behavior will be active. See
> + * &DRM_CLIENT_CAP_VRR_RANGE_CONTROL.
> + *
> + * If setting a non-zero value, the driver should check that:
> + *
> + * 1. Both vrr_range_min and vrr_range_max are set to a non-zero value.
> + * This indicates the driver to switch the new VRR range control
> + * behavior.
> + * 2. Both vrr_rage_min and vrr_range_max are within the panel's supported
> + * FPS range.
> + * 3. vrr_range_min is less-than-or-equal-to vrr_range_max.
> + */
> + uint16_t vrr_range_max;
> +
> /**
> * @self_refresh_active:
> *
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 115763799625..4cb57a503a02 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -240,6 +240,16 @@ struct drm_file {
> */
> bool supports_virtualized_cursor_plane;
>
> + /**
> + * @vrr_range_control:
> + *
> + * If set to true, the DRM driver will allow setting of the
> + * &drm_mode_config.prop_vrr_range_control_min_property and
> + * &drm_mode_config.prop_vrr_range_control_max_property CRTC
> + * properties, if the properties are supported by the driver.
> + */
> + bool vrr_range_control;
> +
> /**
> * @master:
> *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 2e848b816218..e02dd46ca5c2 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -680,6 +680,24 @@ struct drm_mode_config {
> */
> struct drm_property *prop_vrr_enabled;
>
> + /**
> + * @prop_vrr_range_control_min_property: Optional CRTC properties to
> + * further limit the minimum allowed refresh rate within the panel's
> + * supported refresh rate range. It's invalid to set unless the
> + * client advertises &DRM_CLIENT_CAP_VRR_RANGE_CONTROL.
> + * See also &drm_ctrc_state.vrr_range_min.
> + */
> + struct drm_property *prop_vrr_range_control_min;
> +
> + /**
> + * @prop_vrr_range_control_max_property: Optional CRTC properties to
> + * further limit the maximum allowed refresh rate within the panel's
> + * supported refresh rate range. It's invalid to set unless the
> + * client advertises &DRM_CLIENT_CAP_VRR_RANGE_CONTROL.
> + * See also &drm_ctrc_state.vrr_range_max.
> + */
> + struct drm_property *prop_vrr_range_control_max;
> +
> /**
> * @dvi_i_subconnector_property: Optional DVI-I property to
> * differentiate between analog or digital mode.
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 3cd5cf15e3c9..e4d918342e67 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -906,6 +906,23 @@ struct drm_get_cap {
> */
> #define DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT 6
>
> +/**
> + * DRM_CLIENT_CAP_VRR_RANGE_CONTROL
> + *
> + * The driver shall not program a refresh rate that is:
> + * - Below the &drm_crtc_state.vrr_range_min, nor
> + * - Above the &drm_crtc_state.vrr_range_max
> + * Even if the panel supports a wider range than the range requested.
> + *
> + * Once set, the driver will allow setting of the
> + *
> + * - &drm_mode_config.prop_vrr_range_control_min and
> + * - &drm_mode_config.prop_vrr_range_control_max properties.
> + *
> + * Otherwise, setting them will be invalid.
> + */
> +#define DRM_CLIENT_CAP_VRR_RANGE_CONTROL 7
> +
> /* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
> struct drm_set_client_cap {
> __u64 capability;
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-12 13:45 ` Derek Foreman
@ 2025-09-15 10:01 ` Michel Dänzer
2025-09-15 15:37 ` Derek Foreman
2025-09-24 7:50 ` Michel Dänzer
0 siblings, 2 replies; 22+ messages in thread
From: Michel Dänzer @ 2025-09-15 10:01 UTC (permalink / raw)
To: Derek Foreman, Chuanyu Tseng
Cc: harry.wentland, Mario.Limonciello, xaver.hugl, victoria, seanpaul,
Sunpeng.Li, dri-devel
On 12.09.25 15:45, Derek Foreman wrote:
> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>> Introduce a DRM interface for DRM clients to further restrict the
>> VRR Range within the panel supported VRR range on a per-commit
>> basis.
>>
>> The goal is to give DRM client the ability to do frame-doubling/
>> ramping themselves, or to set lower static refresh rates for power
>> savings.
>
> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>
> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>
> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>> This is done through 2 new CRTC properties, along with a client
>> cap. See the docstrings in patch for details.
Not sure why a client cap would be needed for this. Not sure even a DRM_CAP is needed, the properties could simply be added only if the driver supports them.
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index 272d6254ea47..dc4b50ff5fe0 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -2866,6 +2866,31 @@ int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
>> }
>> EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
>> +/**
>> + * drm_connector_attach_vrr_range_control_property - attach "VRR_RANGE_CONTROL_MIN" and
>> + * "VRR_RANGE_CONTROL_MAX" property
>> + *
>> + * @connector: connector to attach the property on.
>> + *
>> + * This is used to allow the userspace to send VRR range control min and max value to the
>> + * driver.
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int drm_connector_attach_vrr_range_control_property(struct drm_connector *connector)
>> +{
>> + struct drm_device *dev = connector->dev;
>> + struct drm_property *prop_min = dev->mode_config.prop_vrr_range_control_min;
>> + struct drm_property *prop_max = dev->mode_config.prop_vrr_range_control_max;
>> +
>> + drm_object_attach_property(&connector->base, prop_min, 0);
>> + drm_object_attach_property(&connector->base, prop_max, 0);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(drm_connector_attach_vrr_range_control_property);
To me it would make more sense for these to be CRTC properties, same as VRR_ENABLED.
>> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>> index 25f376869b3a..1f74284208c6 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -340,6 +340,20 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>> return -ENOMEM;
>> dev->mode_config.prop_vrr_enabled = prop;
>> + prop = drm_property_create_range(dev,
>> + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_RANGE,
>> + "VRR-RANGE-CONTROL-MIN", 0, UINT_MAX);
>> + if (!prop)
>> + return -ENOMEM;
>> + dev->mode_config.prop_vrr_range_control_min = prop;
>> +
>> + prop = drm_property_create_range(dev,
>> + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_RANGE,
>> + "VRR-RANGE-CONTROL-MAX", 0, UINT_MAX);
>> + if (!prop)
>> + return -ENOMEM;
>> + dev->mode_config.prop_vrr_range_control_max = prop;
Property names use underscores, not dashes.
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-15 10:01 ` Michel Dänzer
@ 2025-09-15 15:37 ` Derek Foreman
2025-09-15 15:49 ` Michel Dänzer
2025-09-24 8:34 ` Ville Syrjälä
2025-09-24 7:50 ` Michel Dänzer
1 sibling, 2 replies; 22+ messages in thread
From: Derek Foreman @ 2025-09-15 15:37 UTC (permalink / raw)
To: Michel Dänzer, Chuanyu Tseng
Cc: harry.wentland, Mario.Limonciello, xaver.hugl, victoria, seanpaul,
Sunpeng.Li, dri-devel
On 9/15/25 5:01 AM, Michel Dänzer wrote:
> On 12.09.25 15:45, Derek Foreman wrote:
>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>> Introduce a DRM interface for DRM clients to further restrict the
>>> VRR Range within the panel supported VRR range on a per-commit
>>> basis.
>>>
>>> The goal is to give DRM client the ability to do frame-doubling/
>>> ramping themselves, or to set lower static refresh rates for power
>>> savings.
>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>>
>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>>
>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
Surprisingly, this would also not be precise enough for exact media
playback, as the exact intended framerate might not result in an integer
number of scan lines. When that happens a QMS/CinemaVRR capable HDMI
source is expected to periodically post a frame with a single extra scan
line to minimize the error.
But I don't really know if these new properties are intended to express
such a faux-constant rate (where the short term average rate is constant
but frame to frame may be +1) , or if another new not-quite-orthogonal
property would be required instead. Would I need a separate property for
that?
>
>>> This is done through 2 new CRTC properties, along with a client
>>> cap. See the docstrings in patch for details.
> Not sure why a client cap would be needed for this. Not sure even a DRM_CAP is needed, the properties could simply be added only if the driver supports them.
>
>
>>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>>> index 272d6254ea47..dc4b50ff5fe0 100644
>>> --- a/drivers/gpu/drm/drm_connector.c
>>> +++ b/drivers/gpu/drm/drm_connector.c
>>> @@ -2866,6 +2866,31 @@ int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
>>> }
>>> EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
>>> +/**
>>> + * drm_connector_attach_vrr_range_control_property - attach "VRR_RANGE_CONTROL_MIN" and
>>> + * "VRR_RANGE_CONTROL_MAX" property
>>> + *
>>> + * @connector: connector to attach the property on.
>>> + *
>>> + * This is used to allow the userspace to send VRR range control min and max value to the
>>> + * driver.
>>> + *
>>> + * Returns:
>>> + * Zero on success, negative errno on failure.
>>> + */
>>> +int drm_connector_attach_vrr_range_control_property(struct drm_connector *connector)
>>> +{
>>> + struct drm_device *dev = connector->dev;
>>> + struct drm_property *prop_min = dev->mode_config.prop_vrr_range_control_min;
>>> + struct drm_property *prop_max = dev->mode_config.prop_vrr_range_control_max;
>>> +
>>> + drm_object_attach_property(&connector->base, prop_min, 0);
>>> + drm_object_attach_property(&connector->base, prop_max, 0);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL(drm_connector_attach_vrr_range_control_property);
> To me it would make more sense for these to be CRTC properties, same as VRR_ENABLED.
>
>
>>> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>>> index 25f376869b3a..1f74284208c6 100644
>>> --- a/drivers/gpu/drm/drm_mode_config.c
>>> +++ b/drivers/gpu/drm/drm_mode_config.c
>>> @@ -340,6 +340,20 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>>> return -ENOMEM;
>>> dev->mode_config.prop_vrr_enabled = prop;
>>> + prop = drm_property_create_range(dev,
>>> + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_RANGE,
>>> + "VRR-RANGE-CONTROL-MIN", 0, UINT_MAX);
>>> + if (!prop)
>>> + return -ENOMEM;
>>> + dev->mode_config.prop_vrr_range_control_min = prop;
>>> +
>>> + prop = drm_property_create_range(dev,
>>> + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_RANGE,
>>> + "VRR-RANGE-CONTROL-MAX", 0, UINT_MAX);
>>> + if (!prop)
>>> + return -ENOMEM;
>>> + dev->mode_config.prop_vrr_range_control_max = prop;
> Property names use underscores, not dashes.
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-15 15:37 ` Derek Foreman
@ 2025-09-15 15:49 ` Michel Dänzer
2025-09-16 21:56 ` Xaver Hugl
2025-09-24 8:34 ` Ville Syrjälä
1 sibling, 1 reply; 22+ messages in thread
From: Michel Dänzer @ 2025-09-15 15:49 UTC (permalink / raw)
To: Derek Foreman, Chuanyu Tseng
Cc: harry.wentland, Mario.Limonciello, xaver.hugl, victoria, seanpaul,
Sunpeng.Li, dri-devel
On 15.09.25 17:37, Derek Foreman wrote:
> On 9/15/25 5:01 AM, Michel Dänzer wrote:
>> On 12.09.25 15:45, Derek Foreman wrote:
>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>> Introduce a DRM interface for DRM clients to further restrict the
>>>> VRR Range within the panel supported VRR range on a per-commit
>>>> basis.
>>>>
>>>> The goal is to give DRM client the ability to do frame-doubling/
>>>> ramping themselves, or to set lower static refresh rates for power
>>>> savings.
>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>>>
>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>>>
>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>
> Surprisingly, this would also not be precise enough for exact media playback, as the exact intended framerate might not result in an integer number of scan lines. When that happens a QMS/CinemaVRR capable HDMI source is expected to periodically post a frame with a single extra scan line to minimize the error.
Interesting, maybe your suggestion of numerator / denominator properties is better then.
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-15 15:49 ` Michel Dänzer
@ 2025-09-16 21:56 ` Xaver Hugl
2025-09-17 15:12 ` Derek Foreman
0 siblings, 1 reply; 22+ messages in thread
From: Xaver Hugl @ 2025-09-16 21:56 UTC (permalink / raw)
To: Michel Dänzer
Cc: Derek Foreman, Chuanyu Tseng, harry.wentland, Mario.Limonciello,
victoria, seanpaul, Sunpeng.Li, dri-devel
Am Mo., 15. Sept. 2025 um 17:49 Uhr schrieb Michel Dänzer
<michel.daenzer@mailbox.org>:
>
> On 15.09.25 17:37, Derek Foreman wrote:
> > On 9/15/25 5:01 AM, Michel Dänzer wrote:
> >> On 12.09.25 15:45, Derek Foreman wrote:
> >>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
> >>>> Introduce a DRM interface for DRM clients to further restrict the
> >>>> VRR Range within the panel supported VRR range on a per-commit
> >>>> basis.
> >>>>
> >>>> The goal is to give DRM client the ability to do frame-doubling/
> >>>> ramping themselves, or to set lower static refresh rates for power
> >>>> savings.
> >>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
> >>>
> >>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
> >>>
> >>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
> >> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
> >
> > Surprisingly, this would also not be precise enough for exact media playback, as the exact intended framerate might not result in an integer number of scan lines. When that happens a QMS/CinemaVRR capable HDMI source is expected to periodically post a frame with a single extra scan line to minimize the error.
>
> Interesting, maybe your suggestion of numerator / denominator properties is better then.
API wise, I'd much prefer just using nanoseconds instead of two
properties that compositors will in practice just use the same way.
> --
> Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
> https://redhat.com \ Libre software enthusiast
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-16 21:56 ` Xaver Hugl
@ 2025-09-17 15:12 ` Derek Foreman
2025-09-18 8:33 ` Tseng, Chuan Yu (Max)
0 siblings, 1 reply; 22+ messages in thread
From: Derek Foreman @ 2025-09-17 15:12 UTC (permalink / raw)
To: Xaver Hugl, Michel Dänzer
Cc: Chuanyu Tseng, harry.wentland, Mario.Limonciello, victoria,
seanpaul, Sunpeng.Li, dri-devel
On 9/16/25 4:56 PM, Xaver Hugl wrote:
> Am Mo., 15. Sept. 2025 um 17:49 Uhr schrieb Michel Dänzer
> <michel.daenzer@mailbox.org>:
>> On 15.09.25 17:37, Derek Foreman wrote:
>>> On 9/15/25 5:01 AM, Michel Dänzer wrote:
>>>> On 12.09.25 15:45, Derek Foreman wrote:
>>>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>>>> Introduce a DRM interface for DRM clients to further restrict the
>>>>>> VRR Range within the panel supported VRR range on a per-commit
>>>>>> basis.
>>>>>>
>>>>>> The goal is to give DRM client the ability to do frame-doubling/
>>>>>> ramping themselves, or to set lower static refresh rates for power
>>>>>> savings.
>>>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>>>>>
>>>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>>>>>
>>>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
>>>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>>> Surprisingly, this would also not be precise enough for exact media playback, as the exact intended framerate might not result in an integer number of scan lines. When that happens a QMS/CinemaVRR capable HDMI source is expected to periodically post a frame with a single extra scan line to minimize the error.
>> Interesting, maybe your suggestion of numerator / denominator properties is better then.
> API wise, I'd much prefer just using nanoseconds instead of two
> properties that compositors will in practice just use the same way.
Yeah, I hear you. Period is generally much nicer than frequency, and
every other time I'd unconditionally agree, but QMS is awkward in this
regard.
The media file I start with will have a fraction specified in integers
for the rate, eg: something like 24000/1001 fps. That will map to an
index in an array of QMS blessed target framerates (24000/1001, 24, 25,
48/1001, 48...) and the index ends up in a bitfield in the HDMI QMS
infoframe. That infoframe also has a bit to indicate that the framerate
is currently constant, with constant defined as "constant number of
scanlines but may be exactly 1 scanline longer occasionally".
In the constant state we'd need to maintain that fixed rate within that
constraint, and the integer math to do that needs to start from 24000/1001.
So if we used a nanosecond period for the interface, we'd need to take
the media file's values and convert them to nanoseconds, then in the
kernel convert back to something like milliframes per second (so we
could get something near 23976), then look that up in the QMS accepted
rates array, have some manner of epsilon to decide if we're close enough
to one of them to use it, and then use the integer representation (back
to 24000/1001) to setup the scanline temporal dithering algorithm to do
the +1 extra line every few frames to hit the exact rate.
In effect we'd throw away the precise values we started with and try to
reconstruct them later.
QMS also has the added strange feature of being able to set a fixed rate
below the display's normal VRR minimum, so I'm undecided as to whether
this range control interface is an ideal match for setting up QMS
anyway, or whether I should propose a separate fixed rate property
later. I just don't want to ignore this discussion and show up proposing
another non-orthogonal property later.
Sorry to be speaking in hypotheticals, I do have a working QMS
implementation to share soonish, it's just not quite ready for review yet...
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-17 15:12 ` Derek Foreman
@ 2025-09-18 8:33 ` Tseng, Chuan Yu (Max)
2025-09-22 19:06 ` Leo Li
0 siblings, 1 reply; 22+ messages in thread
From: Tseng, Chuan Yu (Max) @ 2025-09-18 8:33 UTC (permalink / raw)
To: Derek Foreman, Xaver Hugl, Michel Dänzer
Cc: Wentland, Harry, Limonciello, Mario, victoria@system76.com,
seanpaul@google.com, Li, Sun peng (Leo),
dri-devel@lists.freedesktop.org
[AMD Official Use Only - AMD Internal Distribution Only]
On 9/16/25 4:56 PM, Xaver Hugl wrote:
> Am Mo., 15. Sept. 2025 um 17:49 Uhr schrieb Michel Dänzer
> <michel.daenzer@mailbox.org>:
>> On 15.09.25 17:37, Derek Foreman wrote:
>>> On 9/15/25 5:01 AM, Michel Dänzer wrote:
>>>> On 12.09.25 15:45, Derek Foreman wrote:
>>>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>>>> Introduce a DRM interface for DRM clients to further restrict the
>>>>>> VRR Range within the panel supported VRR range on a per-commit
>>>>>> basis.
>>>>>>
>>>>>> The goal is to give DRM client the ability to do frame-doubling/
>>>>>> ramping themselves, or to set lower static refresh rates for
>>>>>> power savings.
>>>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>>>>>
>>>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>>>>>
>>>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
>>>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>>> Surprisingly, this would also not be precise enough for exact media playback, as the exact intended framerate might not result in an integer number of scan lines. When that happens a QMS/CinemaVRR capable HDMI source is expected to periodically post a frame with a single extra scan line to minimize the error.
>> Interesting, maybe your suggestion of numerator / denominator properties is better then.
> API wise, I'd much prefer just using nanoseconds instead of two
> properties that compositors will in practice just use the same way.
>Yeah, I hear you. Period is generally much nicer than frequency, and every other time I'd unconditionally agree, but QMS is awkward in this regard.
>
>The media file I start with will have a fraction specified in integers for the rate, eg: something like 24000/1001 fps. That will map to an index in an array of QMS blessed target framerates (24000/1001, 24, 25, 48/1001, 48...) and the index ends up in a bitfield in the HDMI QMS infoframe. That infoframe also has a bit to indicate that the framerate is currently constant, with constant defined as "constant number of scanlines but may be exactly 1 scanline longer occasionally".
>
>In the constant state we'd need to maintain that fixed rate within that constraint, and the integer math to do that needs to start from 24000/1001.
>
>So if we used a nanosecond period for the interface, we'd need to take the media file's values and convert them to nanoseconds, then in the kernel convert back to something like milliframes per second (so we could get something near 23976), then look that up in the QMS accepted rates array, have some manner of epsilon to decide if we're close enough to one of them to use it, and then use the integer representation (back to 24000/1001) to setup the scanline temporal dithering algorithm to do the +1 extra line every few frames to hit the exact rate.
>
>In effect we'd throw away the precise values we started with and try to reconstruct them later.
>
>QMS also has the added strange feature of being able to set a fixed rate below the display's normal VRR minimum, so I'm undecided as to whether this range control interface is an ideal match for setting up QMS anyway, or whether I should propose a separate fixed rate property later. I just don't want to ignore this discussion and show up proposing another non-orthogonal property later.
>
>Sorry to be speaking in hypotheticals, I do have a working QMS implementation to share soonish, it's just not quite ready for review yet...
Thanks for the input. For the QMS support, it's related to HDMI 2.1 spec. From my knowledge, it's not open to everyone.
We might sperate this QMS support into different discussions.
To support FPS/1.001, I think we can augment 1 more property in this patch, once it's set, we can divide this FPS with 1.001, vise versa.
Thanks,
--
Chuanyu.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-18 8:33 ` Tseng, Chuan Yu (Max)
@ 2025-09-22 19:06 ` Leo Li
2025-09-23 8:37 ` Michel Dänzer
0 siblings, 1 reply; 22+ messages in thread
From: Leo Li @ 2025-09-22 19:06 UTC (permalink / raw)
To: Tseng, Chuan Yu (Max), Derek Foreman, Xaver Hugl,
Michel Dänzer
Cc: Wentland, Harry, Limonciello, Mario, victoria@system76.com,
seanpaul@google.com, dri-devel@lists.freedesktop.org
On 2025-09-18 04:33, Tseng, Chuan Yu (Max) wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> On 9/16/25 4:56 PM, Xaver Hugl wrote:
>> Am Mo., 15. Sept. 2025 um 17:49 Uhr schrieb Michel Dänzer
>> <michel.daenzer@mailbox.org>:
>>> On 15.09.25 17:37, Derek Foreman wrote:
>>>> On 9/15/25 5:01 AM, Michel Dänzer wrote:
>>>>> On 12.09.25 15:45, Derek Foreman wrote:
>>>>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>>>>> Introduce a DRM interface for DRM clients to further restrict the
>>>>>>> VRR Range within the panel supported VRR range on a per-commit
>>>>>>> basis.
>>>>>>>
>>>>>>> The goal is to give DRM client the ability to do frame-doubling/
>>>>>>> ramping themselves, or to set lower static refresh rates for
>>>>>>> power savings.
>>>>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>>>>>>
>>>>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>>>>>>
>>>>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
>>>>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>>>> Surprisingly, this would also not be precise enough for exact media playback, as the exact intended framerate might not result in an integer number of scan lines. When that happens a QMS/CinemaVRR capable HDMI source is expected to periodically post a frame with a single extra scan line to minimize the error.
>>> Interesting, maybe your suggestion of numerator / denominator properties is better then.
>> API wise, I'd much prefer just using nanoseconds instead of two
>> properties that compositors will in practice just use the same way.
>
>> Yeah, I hear you. Period is generally much nicer than frequency, and every other time I'd unconditionally agree, but QMS is awkward in this regard.
>>
>> The media file I start with will have a fraction specified in integers for the rate, eg: something like 24000/1001 fps. That will map to an index in an array of QMS blessed target framerates (24000/1001, 24, 25, 48/1001, 48...) and the index ends up in a bitfield in the HDMI QMS infoframe. That infoframe also has a bit to indicate that the framerate is currently constant, with constant defined as "constant number of scanlines but may be exactly 1 scanline longer occasionally".
>>
>> In the constant state we'd need to maintain that fixed rate within that constraint, and the integer math to do that needs to start from 24000/1001.
>>
>> So if we used a nanosecond period for the interface, we'd need to take the media file's values and convert them to nanoseconds, then in the kernel convert back to something like milliframes per second (so we could get something near 23976), then look that up in the QMS accepted rates array, have some manner of epsilon to decide if we're close enough to one of them to use it, and then use the integer representation (back to 24000/1001) to setup the scanline temporal dithering algorithm to do the +1 extra line every few frames to hit the exact rate.
>>
>> In effect we'd throw away the precise values we started with and try to reconstruct them later.
>>
>> QMS also has the added strange feature of being able to set a fixed rate below the display's normal VRR minimum, so I'm undecided as to whether this range control interface is an ideal match for setting up QMS anyway, or whether I should propose a separate fixed rate property later. I just don't want to ignore this discussion and show up proposing another non-orthogonal property later.
Static video/desktop frame rates was indeed one of the motivations for proposing this API, so it is worth discussing.
For amdgpu (and I think most HW are like this), hardware VRR granularity is at # of total vertical scanlines (vtotal). So if that isn't precise enough, then the driver will have to do record-keeping to alternate between some vtotal and vtotal+1 to avoid drift.
It's not impossible to do, though I'm not sure at what point the driver is considered to be doing "unexpected adjustments of refresh rate", which was something we were also trying to address with this new API. Today, drivers are free to do unexpected things with the vtotal, such as frame-doubling to handle rates below the supported vrr min, and frame-ramping to prevent panel flicker. We discussed at the display hackfest that this was not something compositors liked, and that compositors would like to handle that themselves.
Now, memory fails me, and I don't remember the exact motivation for why compositors want transparent vrr control. Was it because of unexpected driver-reported vblank timestamps messing with compositor internal record keeping? Or something else entirely?
Another way of putting it: Would the compositor rather:
1. Specify a min_vtotal + 1 == max_vtotal so driver doesn't do any unexpected adjustments out of the specified range, or
2. Specify a min_frame_ns == max_frame_ns (or some other highly-precise unit), and have driver correct for drift by alternating between two vtotals, and hence adjust refresh rate beyond the specified range?
>>
>> Sorry to be speaking in hypotheticals, I do have a working QMS implementation to share soonish, it's just not quite ready for review yet...
>
> Thanks for the input. For the QMS support, it's related to HDMI 2.1 spec. From my knowledge, it's not open to everyone.
> We might sperate this QMS support into different discussions.
>
> To support FPS/1.001, I think we can augment 1 more property in this patch, once it's set, we can divide this FPS with 1.001, vise versa.
I think whether we want a separate thing for QMS also depends on what unit we use for specifying the min/max.
Thanks,
Leo
>
> Thanks,
> --
> Chuanyu.
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-22 19:06 ` Leo Li
@ 2025-09-23 8:37 ` Michel Dänzer
2025-09-23 14:35 ` Leo Li
0 siblings, 1 reply; 22+ messages in thread
From: Michel Dänzer @ 2025-09-23 8:37 UTC (permalink / raw)
To: Leo Li, Tseng, Chuan Yu (Max), Derek Foreman, Xaver Hugl
Cc: Wentland, Harry, Limonciello, Mario, victoria@system76.com,
seanpaul@google.com, dri-devel@lists.freedesktop.org
On 22.09.25 21:06, Leo Li wrote:
> On 2025-09-18 04:33, Tseng, Chuan Yu (Max) wrote:
>> On 9/16/25 4:56 PM, Xaver Hugl wrote:
>>> Am Mo., 15. Sept. 2025 um 17:49 Uhr schrieb Michel Dänzer
>>> <michel.daenzer@mailbox.org>:
>>>> On 15.09.25 17:37, Derek Foreman wrote:
>>>>> On 9/15/25 5:01 AM, Michel Dänzer wrote:
>>>>>> On 12.09.25 15:45, Derek Foreman wrote:
>>>>>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>>>>>> Introduce a DRM interface for DRM clients to further restrict the
>>>>>>>> VRR Range within the panel supported VRR range on a per-commit
>>>>>>>> basis.
>>>>>>>>
>>>>>>>> The goal is to give DRM client the ability to do frame-doubling/
>>>>>>>> ramping themselves, or to set lower static refresh rates for
>>>>>>>> power savings.
>>>>>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>>>>>>>
>>>>>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>>>>>>>
>>>>>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
>>>>>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>>>>> Surprisingly, this would also not be precise enough for exact media playback, as the exact intended framerate might not result in an integer number of scan lines. When that happens a QMS/CinemaVRR capable HDMI source is expected to periodically post a frame with a single extra scan line to minimize the error.
>>>> Interesting, maybe your suggestion of numerator / denominator properties is better then.
>>> API wise, I'd much prefer just using nanoseconds instead of two
>>> properties that compositors will in practice just use the same way.
>>
>>> Yeah, I hear you. Period is generally much nicer than frequency, and every other time I'd unconditionally agree, but QMS is awkward in this regard.
>>>
>>> The media file I start with will have a fraction specified in integers for the rate, eg: something like 24000/1001 fps. That will map to an index in an array of QMS blessed target framerates (24000/1001, 24, 25, 48/1001, 48...) and the index ends up in a bitfield in the HDMI QMS infoframe. That infoframe also has a bit to indicate that the framerate is currently constant, with constant defined as "constant number of scanlines but may be exactly 1 scanline longer occasionally".
>>>
>>> In the constant state we'd need to maintain that fixed rate within that constraint, and the integer math to do that needs to start from 24000/1001.
>>>
>>> So if we used a nanosecond period for the interface, we'd need to take the media file's values and convert them to nanoseconds, then in the kernel convert back to something like milliframes per second (so we could get something near 23976), then look that up in the QMS accepted rates array, have some manner of epsilon to decide if we're close enough to one of them to use it, and then use the integer representation (back to 24000/1001) to setup the scanline temporal dithering algorithm to do the +1 extra line every few frames to hit the exact rate.
>>>
>>> In effect we'd throw away the precise values we started with and try to reconstruct them later.
>>>
>>> QMS also has the added strange feature of being able to set a fixed rate below the display's normal VRR minimum, so I'm undecided as to whether this range control interface is an ideal match for setting up QMS anyway, or whether I should propose a separate fixed rate property later. I just don't want to ignore this discussion and show up proposing another non-orthogonal property later.
>
> Static video/desktop frame rates was indeed one of the motivations for proposing this API, so it is worth discussing.
>
> For amdgpu (and I think most HW are like this), hardware VRR granularity is at # of total vertical scanlines (vtotal). So if that isn't precise enough, then the driver will have to do record-keeping to alternate between some vtotal and vtotal+1 to avoid drift.
>
> It's not impossible to do, though I'm not sure at what point the driver is considered to be doing "unexpected adjustments of refresh rate", which was something we were also trying to address with this new API. Today, drivers are free to do unexpected things with the vtotal, such as frame-doubling to handle rates below the supported vrr min, and frame-ramping to prevent panel flicker. We discussed at the display hackfest that this was not something compositors liked, and that compositors would like to handle that themselves.
>
> Now, memory fails me, and I don't remember the exact motivation for why compositors want transparent vrr control. Was it because of unexpected driver-reported vblank timestamps messing with compositor internal record keeping? Or something else entirely?
AFAIR it's mostly about the compositor being able to control the refresh rate in general (e.g. keeping it low to save power) and allowing it to handle LFC & ramping without interference by the kernel's corresponding handling.
> Another way of putting it: Would the compositor rather:
>
> 1. Specify a min_vtotal + 1 == max_vtotal so driver doesn't do any unexpected adjustments out of the specified range, or
> 2. Specify a min_frame_ns == max_frame_ns (or some other highly-precise unit), and have driver correct for drift by alternating between two vtotals, and hence adjust refresh rate beyond the specified range?
FWIW, I'd be fine with option 2.
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-23 8:37 ` Michel Dänzer
@ 2025-09-23 14:35 ` Leo Li
2025-09-24 11:44 ` Michel Dänzer
0 siblings, 1 reply; 22+ messages in thread
From: Leo Li @ 2025-09-23 14:35 UTC (permalink / raw)
To: Michel Dänzer, Tseng, Chuan Yu (Max), Derek Foreman,
Xaver Hugl
Cc: Wentland, Harry, Limonciello, Mario, victoria@system76.com,
seanpaul@google.com, dri-devel@lists.freedesktop.org
On 2025-09-23 04:37, Michel Dänzer wrote:
> On 22.09.25 21:06, Leo Li wrote:
>> On 2025-09-18 04:33, Tseng, Chuan Yu (Max) wrote:
>>> On 9/16/25 4:56 PM, Xaver Hugl wrote:
>>>> Am Mo., 15. Sept. 2025 um 17:49 Uhr schrieb Michel Dänzer
>>>> <michel.daenzer@mailbox.org>:
>>>>> On 15.09.25 17:37, Derek Foreman wrote:
>>>>>> On 9/15/25 5:01 AM, Michel Dänzer wrote:
>>>>>>> On 12.09.25 15:45, Derek Foreman wrote:
>>>>>>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>>>>>>> Introduce a DRM interface for DRM clients to further restrict the
>>>>>>>>> VRR Range within the panel supported VRR range on a per-commit
>>>>>>>>> basis.
>>>>>>>>>
>>>>>>>>> The goal is to give DRM client the ability to do frame-doubling/
>>>>>>>>> ramping themselves, or to set lower static refresh rates for
>>>>>>>>> power savings.
>>>>>>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>>>>>>>>
>>>>>>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>>>>>>>>
>>>>>>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
>>>>>>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>>>>>> Surprisingly, this would also not be precise enough for exact media playback, as the exact intended framerate might not result in an integer number of scan lines. When that happens a QMS/CinemaVRR capable HDMI source is expected to periodically post a frame with a single extra scan line to minimize the error.
>>>>> Interesting, maybe your suggestion of numerator / denominator properties is better then.
>>>> API wise, I'd much prefer just using nanoseconds instead of two
>>>> properties that compositors will in practice just use the same way.
>>>
>>>> Yeah, I hear you. Period is generally much nicer than frequency, and every other time I'd unconditionally agree, but QMS is awkward in this regard.
>>>>
>>>> The media file I start with will have a fraction specified in integers for the rate, eg: something like 24000/1001 fps. That will map to an index in an array of QMS blessed target framerates (24000/1001, 24, 25, 48/1001, 48...) and the index ends up in a bitfield in the HDMI QMS infoframe. That infoframe also has a bit to indicate that the framerate is currently constant, with constant defined as "constant number of scanlines but may be exactly 1 scanline longer occasionally".
>>>>
>>>> In the constant state we'd need to maintain that fixed rate within that constraint, and the integer math to do that needs to start from 24000/1001.
>>>>
>>>> So if we used a nanosecond period for the interface, we'd need to take the media file's values and convert them to nanoseconds, then in the kernel convert back to something like milliframes per second (so we could get something near 23976), then look that up in the QMS accepted rates array, have some manner of epsilon to decide if we're close enough to one of them to use it, and then use the integer representation (back to 24000/1001) to setup the scanline temporal dithering algorithm to do the +1 extra line every few frames to hit the exact rate.
>>>>
>>>> In effect we'd throw away the precise values we started with and try to reconstruct them later.
>>>>
>>>> QMS also has the added strange feature of being able to set a fixed rate below the display's normal VRR minimum, so I'm undecided as to whether this range control interface is an ideal match for setting up QMS anyway, or whether I should propose a separate fixed rate property later. I just don't want to ignore this discussion and show up proposing another non-orthogonal property later.
>>
>> Static video/desktop frame rates was indeed one of the motivations for proposing this API, so it is worth discussing.
>>
>> For amdgpu (and I think most HW are like this), hardware VRR granularity is at # of total vertical scanlines (vtotal). So if that isn't precise enough, then the driver will have to do record-keeping to alternate between some vtotal and vtotal+1 to avoid drift.
>>
>> It's not impossible to do, though I'm not sure at what point the driver is considered to be doing "unexpected adjustments of refresh rate", which was something we were also trying to address with this new API. Today, drivers are free to do unexpected things with the vtotal, such as frame-doubling to handle rates below the supported vrr min, and frame-ramping to prevent panel flicker. We discussed at the display hackfest that this was not something compositors liked, and that compositors would like to handle that themselves.
>>
>> Now, memory fails me, and I don't remember the exact motivation for why compositors want transparent vrr control. Was it because of unexpected driver-reported vblank timestamps messing with compositor internal record keeping? Or something else entirely?
>
> AFAIR it's mostly about the compositor being able to control the refresh rate in general (e.g. keeping it low to save power) and allowing it to handle LFC & ramping without interference by the kernel's corresponding handling.
I seem to recall a good reason mentioned for why compositors would like to handle LFC & ramping, but I don't recall the details. Otherwise, would it not be simpler for the compositor to leave that to the kernel, and have one additional "static_refresh" property to handle low hz desktop and static hz content scenarios?
Thanks,
Leo
>
>
>> Another way of putting it: Would the compositor rather:
>>
>> 1. Specify a min_vtotal + 1 == max_vtotal so driver doesn't do any unexpected adjustments out of the specified range, or
>> 2. Specify a min_frame_ns == max_frame_ns (or some other highly-precise unit), and have driver correct for drift by alternating between two vtotals, and hence adjust refresh rate beyond the specified range?
>
> FWIW, I'd be fine with option 2.
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-23 14:35 ` Leo Li
@ 2025-09-24 11:44 ` Michel Dänzer
0 siblings, 0 replies; 22+ messages in thread
From: Michel Dänzer @ 2025-09-24 11:44 UTC (permalink / raw)
To: Leo Li, Tseng, Chuan Yu (Max), Derek Foreman, Xaver Hugl
Cc: Wentland, Harry, Limonciello, Mario, victoria@system76.com,
seanpaul@google.com, dri-devel@lists.freedesktop.org
On 23.09.25 16:35, Leo Li wrote:
>
>
> On 2025-09-23 04:37, Michel Dänzer wrote:
>> On 22.09.25 21:06, Leo Li wrote:
>>> On 2025-09-18 04:33, Tseng, Chuan Yu (Max) wrote:
>>>> On 9/16/25 4:56 PM, Xaver Hugl wrote:
>>>>> Am Mo., 15. Sept. 2025 um 17:49 Uhr schrieb Michel Dänzer
>>>>> <michel.daenzer@mailbox.org>:
>>>>>> On 15.09.25 17:37, Derek Foreman wrote:
>>>>>>> On 9/15/25 5:01 AM, Michel Dänzer wrote:
>>>>>>>> On 12.09.25 15:45, Derek Foreman wrote:
>>>>>>>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>>>>>>>> Introduce a DRM interface for DRM clients to further restrict the
>>>>>>>>>> VRR Range within the panel supported VRR range on a per-commit
>>>>>>>>>> basis.
>>>>>>>>>>
>>>>>>>>>> The goal is to give DRM client the ability to do frame-doubling/
>>>>>>>>>> ramping themselves, or to set lower static refresh rates for
>>>>>>>>>> power savings.
>>>>>>>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>>>>>>>>>
>>>>>>>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>>>>>>>>>
>>>>>>>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
>>>>>>>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>>>>>>> Surprisingly, this would also not be precise enough for exact media playback, as the exact intended framerate might not result in an integer number of scan lines. When that happens a QMS/CinemaVRR capable HDMI source is expected to periodically post a frame with a single extra scan line to minimize the error.
>>>>>> Interesting, maybe your suggestion of numerator / denominator properties is better then.
>>>>> API wise, I'd much prefer just using nanoseconds instead of two
>>>>> properties that compositors will in practice just use the same way.
>>>>
>>>>> Yeah, I hear you. Period is generally much nicer than frequency, and every other time I'd unconditionally agree, but QMS is awkward in this regard.
>>>>>
>>>>> The media file I start with will have a fraction specified in integers for the rate, eg: something like 24000/1001 fps. That will map to an index in an array of QMS blessed target framerates (24000/1001, 24, 25, 48/1001, 48...) and the index ends up in a bitfield in the HDMI QMS infoframe. That infoframe also has a bit to indicate that the framerate is currently constant, with constant defined as "constant number of scanlines but may be exactly 1 scanline longer occasionally".
>>>>>
>>>>> In the constant state we'd need to maintain that fixed rate within that constraint, and the integer math to do that needs to start from 24000/1001.
>>>>>
>>>>> So if we used a nanosecond period for the interface, we'd need to take the media file's values and convert them to nanoseconds, then in the kernel convert back to something like milliframes per second (so we could get something near 23976), then look that up in the QMS accepted rates array, have some manner of epsilon to decide if we're close enough to one of them to use it, and then use the integer representation (back to 24000/1001) to setup the scanline temporal dithering algorithm to do the +1 extra line every few frames to hit the exact rate.
>>>>>
>>>>> In effect we'd throw away the precise values we started with and try to reconstruct them later.
>>>>>
>>>>> QMS also has the added strange feature of being able to set a fixed rate below the display's normal VRR minimum, so I'm undecided as to whether this range control interface is an ideal match for setting up QMS anyway, or whether I should propose a separate fixed rate property later. I just don't want to ignore this discussion and show up proposing another non-orthogonal property later.
>>>
>>> Static video/desktop frame rates was indeed one of the motivations for proposing this API, so it is worth discussing.
>>>
>>> For amdgpu (and I think most HW are like this), hardware VRR granularity is at # of total vertical scanlines (vtotal). So if that isn't precise enough, then the driver will have to do record-keeping to alternate between some vtotal and vtotal+1 to avoid drift.
>>>
>>> It's not impossible to do, though I'm not sure at what point the driver is considered to be doing "unexpected adjustments of refresh rate", which was something we were also trying to address with this new API. Today, drivers are free to do unexpected things with the vtotal, such as frame-doubling to handle rates below the supported vrr min, and frame-ramping to prevent panel flicker. We discussed at the display hackfest that this was not something compositors liked, and that compositors would like to handle that themselves.
>>>
>>> Now, memory fails me, and I don't remember the exact motivation for why compositors want transparent vrr control. Was it because of unexpected driver-reported vblank timestamps messing with compositor internal record keeping? Or something else entirely?
>>
>> AFAIR it's mostly about the compositor being able to control the refresh rate in general (e.g. keeping it low to save power) and allowing it to handle LFC & ramping without interference by the kernel's corresponding handling.
>
> I seem to recall a good reason mentioned for why compositors would like to handle LFC & ramping, but I don't recall the details.
E.g. the current UAPI doesn't allow the compositor to take advantage of refresh cycles inserted by the kernel for LFC, e.g. for moving the cursor plane:
The compositor has to guess if and when the kernel inserts refresh cycles for LFC, there's no way to know.
If the compositor tries anyway, but its atomic commit is too late for the LFC refresh cycle, it results in bad latency and possibly an application frame drop. To avoid this, the compositor would need to aim early, which isn't great for latency.
Either way, the additional compositor commits will interfere with the kernel's LFC / ramping calculations.
To avoid such issues, the idea is to give the compositor full control over the effective refresh rate range, allowing it to perform LFC & ramping itself, among other things.
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-15 15:37 ` Derek Foreman
2025-09-15 15:49 ` Michel Dänzer
@ 2025-09-24 8:34 ` Ville Syrjälä
2025-09-24 8:39 ` Michel Dänzer
1 sibling, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2025-09-24 8:34 UTC (permalink / raw)
To: Derek Foreman
Cc: Michel Dänzer, Chuanyu Tseng, harry.wentland,
Mario.Limonciello, xaver.hugl, victoria, seanpaul, Sunpeng.Li,
dri-devel
On Mon, Sep 15, 2025 at 03:37:07PM +0000, Derek Foreman wrote:
> On 9/15/25 5:01 AM, Michel Dänzer wrote:
> > On 12.09.25 15:45, Derek Foreman wrote:
> >> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
> >>> Introduce a DRM interface for DRM clients to further restrict the
> >>> VRR Range within the panel supported VRR range on a per-commit
> >>> basis.
> >>>
> >>> The goal is to give DRM client the ability to do frame-doubling/
> >>> ramping themselves, or to set lower static refresh rates for power
> >>> savings.
> >> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
> >>
> >> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
> >>
> >> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
> > I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>
> Surprisingly, this would also not be precise enough for exact media
> playback, as the exact intended framerate might not result in an integer
> number of scan lines. When that happens a QMS/CinemaVRR capable HDMI
> source is expected to periodically post a frame with a single extra scan
> line to minimize the error.
Intel VRR hardware has a "CMRR" feature where it can automagically
tweak the vtotal between frames to maintain a non integer average.
As for knobs to limit the min/max refresh rates, technically you
wouldn't need the max knob because that is ultimately defined by
the vtotal of the supplied timings. But I guess if you have a
knob to limit the min then a max knob might be convenient as well.
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-24 8:34 ` Ville Syrjälä
@ 2025-09-24 8:39 ` Michel Dänzer
2025-09-24 9:05 ` Ville Syrjälä
0 siblings, 1 reply; 22+ messages in thread
From: Michel Dänzer @ 2025-09-24 8:39 UTC (permalink / raw)
To: Ville Syrjälä, Derek Foreman
Cc: Chuanyu Tseng, harry.wentland, Mario.Limonciello, xaver.hugl,
victoria, seanpaul, Sunpeng.Li, dri-devel
On 24.09.25 10:34, Ville Syrjälä wrote:
> On Mon, Sep 15, 2025 at 03:37:07PM +0000, Derek Foreman wrote:
>> On 9/15/25 5:01 AM, Michel Dänzer wrote:
>>> On 12.09.25 15:45, Derek Foreman wrote:
>>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>>> Introduce a DRM interface for DRM clients to further restrict the
>>>>> VRR Range within the panel supported VRR range on a per-commit
>>>>> basis.
>>>>>
>>>>> The goal is to give DRM client the ability to do frame-doubling/
>>>>> ramping themselves, or to set lower static refresh rates for power
>>>>> savings.
>>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>>>>
>>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>>>>
>>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
>>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>>
>> Surprisingly, this would also not be precise enough for exact media
>> playback, as the exact intended framerate might not result in an integer
>> number of scan lines. When that happens a QMS/CinemaVRR capable HDMI
>> source is expected to periodically post a frame with a single extra scan
>> line to minimize the error.
>
> Intel VRR hardware has a "CMRR" feature where it can automagically
> tweak the vtotal between frames to maintain a non integer average.
Neat.
> As for knobs to limit the min/max refresh rates, technically you
> wouldn't need the max knob because that is ultimately defined by
> the vtotal of the supplied timings. But I guess if you have a
> knob to limit the min then a max knob might be convenient as well.
It allows the compositor to limit the maximum refresh rate without changing the mode (which can involve visual glitches).
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-24 8:39 ` Michel Dänzer
@ 2025-09-24 9:05 ` Ville Syrjälä
2025-09-24 9:32 ` Tseng, Chuan Yu (Max)
0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2025-09-24 9:05 UTC (permalink / raw)
To: Michel Dänzer
Cc: Derek Foreman, Chuanyu Tseng, harry.wentland, Mario.Limonciello,
xaver.hugl, victoria, seanpaul, Sunpeng.Li, dri-devel
On Wed, Sep 24, 2025 at 10:39:16AM +0200, Michel Dänzer wrote:
> On 24.09.25 10:34, Ville Syrjälä wrote:
> > On Mon, Sep 15, 2025 at 03:37:07PM +0000, Derek Foreman wrote:
> >> On 9/15/25 5:01 AM, Michel Dänzer wrote:
> >>> On 12.09.25 15:45, Derek Foreman wrote:
> >>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
> >>>>> Introduce a DRM interface for DRM clients to further restrict the
> >>>>> VRR Range within the panel supported VRR range on a per-commit
> >>>>> basis.
> >>>>>
> >>>>> The goal is to give DRM client the ability to do frame-doubling/
> >>>>> ramping themselves, or to set lower static refresh rates for power
> >>>>> savings.
> >>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
> >>>>
> >>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
> >>>>
> >>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
> >>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
> >>
> >> Surprisingly, this would also not be precise enough for exact media
> >> playback, as the exact intended framerate might not result in an integer
> >> number of scan lines. When that happens a QMS/CinemaVRR capable HDMI
> >> source is expected to periodically post a frame with a single extra scan
> >> line to minimize the error.
> >
> > Intel VRR hardware has a "CMRR" feature where it can automagically
> > tweak the vtotal between frames to maintain a non integer average.
>
> Neat.
>
>
> > As for knobs to limit the min/max refresh rates, technically you
> > wouldn't need the max knob because that is ultimately defined by
> > the vtotal of the supplied timings. But I guess if you have a
> > knob to limit the min then a max knob might be convenient as well.
>
> It allows the compositor to limit the maximum refresh rate without changing the mode (which can involve visual glitches).
I think the driver should be able to handle the two cases in exactly
the same way (assuming nothing else in the mode changes). But I'm not
opposed to having a max alongside the min.
IMO the min and max could be straight integers, if specified as
vtotals (that's what the hardware takes for us at least). And
for the CMRR thing we'd need another property to indicate the
target refresh rate somehow.
I suppose another option would be to have non-integer min/max,
and then enable CMRR if min==max==<non-integer value>. Not sure
I quite like that idea though.
Also not sure what the rules for such properties should be.
Should they be allowed to be specified outside the legal range
and the driver just clamps them, or should that be an error?
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-24 9:05 ` Ville Syrjälä
@ 2025-09-24 9:32 ` Tseng, Chuan Yu (Max)
2025-09-24 11:56 ` Michel Dänzer
0 siblings, 1 reply; 22+ messages in thread
From: Tseng, Chuan Yu (Max) @ 2025-09-24 9:32 UTC (permalink / raw)
To: Ville Syrjälä, Michel Dänzer
Cc: Derek Foreman, Wentland, Harry, Limonciello, Mario,
xaver.hugl@gmail.com, victoria@system76.com, seanpaul@google.com,
Li, Sun peng (Leo), dri-devel@lists.freedesktop.org
[AMD Official Use Only - AMD Internal Distribution Only]
n Wed, Sep 24, 2025 at 10:39:16AM +0200, Michel Dänzer wrote:
>> On 24.09.25 10:34, Ville Syrjälä wrote:
>> > On Mon, Sep 15, 2025 at 03:37:07PM +0000, Derek Foreman wrote:
>> >> On 9/15/25 5:01 AM, Michel Dänzer wrote:
>> >>> On 12.09.25 15:45, Derek Foreman wrote:
>> >>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>> >>>>> Introduce a DRM interface for DRM clients to further restrict
>> >>>>> the VRR Range within the panel supported VRR range on a
>> >>>>> per-commit basis.
>> >>>>>
>> >>>>> The goal is to give DRM client the ability to do frame-doubling/
>> >>>>> ramping themselves, or to set lower static refresh rates for
>> >>>>> power savings.
>> >>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>> >>>>
>> >>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>> >>>>
>> >>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
>> >>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>> >>
>> >> Surprisingly, this would also not be precise enough for exact media
>> >> playback, as the exact intended framerate might not result in an
>> >> integer number of scan lines. When that happens a QMS/CinemaVRR
>> >> capable HDMI source is expected to periodically post a frame with a
>> >> single extra scan line to minimize the error.
>> >
>> > Intel VRR hardware has a "CMRR" feature where it can automagically
>> > tweak the vtotal between frames to maintain a non integer average.
>>
>> Neat.
>>
>>
>> > As for knobs to limit the min/max refresh rates, technically you
>> > wouldn't need the max knob because that is ultimately defined by the
>> > vtotal of the supplied timings. But I guess if you have a knob to
>> > limit the min then a max knob might be convenient as well.
>>
>> It allows the compositor to limit the maximum refresh rate without changing the mode (which can involve visual glitches).
>I think the driver should be able to handle the two cases in exactly the same way (assuming nothing else in the mode changes). But I'm not opposed to having a max alongside the min.
>IMO the min and max could be straight integers, if specified as vtotals (that's what the hardware takes for us at least). And for the CMRR thing we'd need another property to indicate the target refresh rate somehow.
>I suppose another option would be to have non-integer min/max, and then enable CMRR if min==max==<non-integer value>. Not sure I quite like that idea though.
>Also not sure what the rules for such properties should be.
>Should they be allowed to be specified outside the legal range and the driver just clamps them, or should that be an error?
It's possible that compositor set the value which is not acceptable to sink vrr range.
It would be better to clamp the false value.
--
Chuanyu(Max)
AMD
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-24 9:32 ` Tseng, Chuan Yu (Max)
@ 2025-09-24 11:56 ` Michel Dänzer
2025-09-29 8:01 ` Ville Syrjälä
0 siblings, 1 reply; 22+ messages in thread
From: Michel Dänzer @ 2025-09-24 11:56 UTC (permalink / raw)
To: Tseng, Chuan Yu (Max), Ville Syrjälä
Cc: Derek Foreman, Wentland, Harry, Limonciello, Mario,
xaver.hugl@gmail.com, victoria@system76.com, seanpaul@google.com,
Li, Sun peng (Leo), dri-devel@lists.freedesktop.org
On 24.09.25 11:32, Tseng, Chuan Yu (Max) wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> n Wed, Sep 24, 2025 at 10:39:16AM +0200, Michel Dänzer wrote:
>>> On 24.09.25 10:34, Ville Syrjälä wrote:
>>>> On Mon, Sep 15, 2025 at 03:37:07PM +0000, Derek Foreman wrote:
>>>>> On 9/15/25 5:01 AM, Michel Dänzer wrote:
>>>>>> On 12.09.25 15:45, Derek Foreman wrote:
>>>>>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>>>>>> Introduce a DRM interface for DRM clients to further restrict
>>>>>>>> the VRR Range within the panel supported VRR range on a
>>>>>>>> per-commit basis.
>>>>>>>>
>>>>>>>> The goal is to give DRM client the ability to do frame-doubling/
>>>>>>>> ramping themselves, or to set lower static refresh rates for
>>>>>>>> power savings.
>>>>>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
>>>>>>>
>>>>>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
>>>>>>>
>>>>>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
>>>>>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
>>>>>
>>>>> Surprisingly, this would also not be precise enough for exact media
>>>>> playback, as the exact intended framerate might not result in an
>>>>> integer number of scan lines. When that happens a QMS/CinemaVRR
>>>>> capable HDMI source is expected to periodically post a frame with a
>>>>> single extra scan line to minimize the error.
>>>>
>>>> Intel VRR hardware has a "CMRR" feature where it can automagically
>>>> tweak the vtotal between frames to maintain a non integer average.
>>>
>>> Neat.
>>>
>>>
>>>> As for knobs to limit the min/max refresh rates, technically you
>>>> wouldn't need the max knob because that is ultimately defined by the
>>>> vtotal of the supplied timings. But I guess if you have a knob to
>>>> limit the min then a max knob might be convenient as well.
>>>
>>> It allows the compositor to limit the maximum refresh rate without changing the mode (which can involve visual glitches).
>
>> I think the driver should be able to handle the two cases in exactly the same way (assuming nothing else in the mode changes). But I'm not opposed to having a max alongside the min.
Compositors want to avoid setting the DRM_MODE_ATOMIC_ALLOW_MODESET flag under normal circumstances, because it may result in visual glitches.
>> IMO the min and max could be straight integers, if specified as vtotals (that's what the hardware takes for us at least). And for the CMRR thing we'd need another property to indicate the target refresh rate somehow.
>
>> I suppose another option would be to have non-integer min/max, and then enable CMRR if min==max==<non-integer value>. Not sure I quite like that idea though.
Do you see any issue with proposal 2 by Leo Li, specifying the limits as the total duration of a refresh cycle in nanoseconds? If the limits don't correspond to an integer number of scanlines, the driver should alternate between the nearest integer numbers of scanlines to approximate the requested duration on average.
>> Also not sure what the rules for such properties should be.
>> Should they be allowed to be specified outside the legal range and the driver just clamps them, or should that be an error?
>
> It's possible that compositor set the value which is not acceptable to sink vrr range.
> It would be better to clamp the false value.
One downside of implicit clamping is that the resulting effective limits may not be what the compositor expects.
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-24 11:56 ` Michel Dänzer
@ 2025-09-29 8:01 ` Ville Syrjälä
0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2025-09-29 8:01 UTC (permalink / raw)
To: Michel Dänzer
Cc: Tseng, Chuan Yu (Max), Derek Foreman, Wentland, Harry,
Limonciello, Mario, xaver.hugl@gmail.com, victoria@system76.com,
seanpaul@google.com, Li, Sun peng (Leo),
dri-devel@lists.freedesktop.org
On Wed, Sep 24, 2025 at 01:56:30PM +0200, Michel Dänzer wrote:
> On 24.09.25 11:32, Tseng, Chuan Yu (Max) wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> > n Wed, Sep 24, 2025 at 10:39:16AM +0200, Michel Dänzer wrote:
> >>> On 24.09.25 10:34, Ville Syrjälä wrote:
> >>>> On Mon, Sep 15, 2025 at 03:37:07PM +0000, Derek Foreman wrote:
> >>>>> On 9/15/25 5:01 AM, Michel Dänzer wrote:
> >>>>>> On 12.09.25 15:45, Derek Foreman wrote:
> >>>>>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
> >>>>>>>> Introduce a DRM interface for DRM clients to further restrict
> >>>>>>>> the VRR Range within the panel supported VRR range on a
> >>>>>>>> per-commit basis.
> >>>>>>>>
> >>>>>>>> The goal is to give DRM client the ability to do frame-doubling/
> >>>>>>>> ramping themselves, or to set lower static refresh rates for
> >>>>>>>> power savings.
> >>>>>>> I'm interested in limiting the range of VRR to enable HDMI's QMS/CinemaVRR features - ie: switching to a fixed rate for media playback without incurring screen blackouts/resyncs/"bonks" during the switch.
> >>>>>>>
> >>>>>>> I could see using an interface such as this to do the frame rate limiting, by setting the lower and upper bounds both to a media file's framerate. However for that use case it's not precise enough, as video may have a rate like 23.9760239... FPS.
> >>>>>>>
> >>>>>>> Would it be better to expose the limits as a numerator/denominator pair so a rate can be something like 24000/1001fps?
> >>>>>> I was thinking the properties could allow directly specifying the minimum and maximum number of total scanlines per refresh cycle, based on the assumption the driver needs to program something along those lines.
> >>>>>
> >>>>> Surprisingly, this would also not be precise enough for exact media
> >>>>> playback, as the exact intended framerate might not result in an
> >>>>> integer number of scan lines. When that happens a QMS/CinemaVRR
> >>>>> capable HDMI source is expected to periodically post a frame with a
> >>>>> single extra scan line to minimize the error.
> >>>>
> >>>> Intel VRR hardware has a "CMRR" feature where it can automagically
> >>>> tweak the vtotal between frames to maintain a non integer average.
> >>>
> >>> Neat.
> >>>
> >>>
> >>>> As for knobs to limit the min/max refresh rates, technically you
> >>>> wouldn't need the max knob because that is ultimately defined by the
> >>>> vtotal of the supplied timings. But I guess if you have a knob to
> >>>> limit the min then a max knob might be convenient as well.
> >>>
> >>> It allows the compositor to limit the maximum refresh rate without changing the mode (which can involve visual glitches).
> >
> >> I think the driver should be able to handle the two cases in exactly the same way (assuming nothing else in the mode changes). But I'm not opposed to having a max alongside the min.
>
> Compositors want to avoid setting the DRM_MODE_ATOMIC_ALLOW_MODESET flag under normal circumstances, because it may result in visual glitches.
If the driver can do the vtotal adjustment without a modeset
(which it should if it can do the same thing for the max
refresh rate property) then you don't need that flag. And for
i915 we'd probably handle the property change just like a
modeset, and after all the calculation are done the driver
will just notice that a full modeset isn't required to get to
the new state.
>
>
> >> IMO the min and max could be straight integers, if specified as vtotals (that's what the hardware takes for us at least). And for the CMRR thing we'd need another property to indicate the target refresh rate somehow.
> >
> >> I suppose another option would be to have non-integer min/max, and then enable CMRR if min==max==<non-integer value>. Not sure I quite like that idea though.
>
> Do you see any issue with proposal 2 by Leo Li, specifying the limits as the total duration of a refresh cycle in nanoseconds? If the limits don't correspond to an integer number of scanlines, the driver should alternate between the nearest integer numbers of scanlines to approximate the requested duration on average.
We can't do any kind of alternation for the min and max independently.
those will always just be integer number of scanlines. With CMRR will
automagically alternate between two vtotals to maintain the average.
I suppose a time based property would work for us, we'd just have to
convert it to scanlines anyway.
> >> Also not sure what the rules for such properties should be.
> >> Should they be allowed to be specified outside the legal range and the driver just clamps them, or should that be an error?
> >
> > It's possible that compositor set the value which is not acceptable to sink vrr range.
> > It would be better to clamp the false value.
>
> One downside of implicit clamping is that the resulting effective limits may not be what the compositor expects.
I suppose.
Though if/when we get the DC balance stuff actually working the min/max
vtotal will be changing dynamically all the time to maintain the balance.
So in that case the compositor can't accurately know the exact vtotal
for future frames anyway. But the absolute limits will be based on what
the user specified.
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-15 10:01 ` Michel Dänzer
2025-09-15 15:37 ` Derek Foreman
@ 2025-09-24 7:50 ` Michel Dänzer
2025-09-24 15:01 ` Leo Li
1 sibling, 1 reply; 22+ messages in thread
From: Michel Dänzer @ 2025-09-24 7:50 UTC (permalink / raw)
To: Derek Foreman, Chuanyu Tseng
Cc: harry.wentland, Mario.Limonciello, xaver.hugl, victoria, seanpaul,
Sunpeng.Li, dri-devel
On 15.09.25 12:01, Michel Dänzer wrote:
> On 12.09.25 15:45, Derek Foreman wrote:
>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>
>>> This is done through 2 new CRTC properties, along with a client
>>> cap. See the docstrings in patch for details.
>
> Not sure why a client cap would be needed for this.
According to https://hwentland.github.io/work/2025hackfest-notes.html#vrr-for-desktop-use-cases the client cap was intended for disabling LFC & ramping in the kernel driver. It's not really needed for that though:
If the min/max property values differ, the kernel driver can vary the effective refresh range between the limits and perform ramping, otherwise it can't.
If the effective maximum value is at least twice the effective minimum value, the kernel driver can perform LFC, otherwise it can't.
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-24 7:50 ` Michel Dänzer
@ 2025-09-24 15:01 ` Leo Li
2025-09-26 14:19 ` Xaver Hugl
0 siblings, 1 reply; 22+ messages in thread
From: Leo Li @ 2025-09-24 15:01 UTC (permalink / raw)
To: Michel Dänzer, Derek Foreman, Chuanyu Tseng
Cc: harry.wentland, Mario.Limonciello, xaver.hugl, victoria, seanpaul,
dri-devel
On 2025-09-24 03:50, Michel Dänzer wrote:
> On 15.09.25 12:01, Michel Dänzer wrote:
>> On 12.09.25 15:45, Derek Foreman wrote:
>>> On 9/12/25 2:33 AM, Chuanyu Tseng wrote:
>>>>
>>>> This is done through 2 new CRTC properties, along with a client
>>>> cap. See the docstrings in patch for details.
>>
>> Not sure why a client cap would be needed for this.
>
> According to https://hwentland.github.io/work/2025hackfest-notes.html#vrr-for-desktop-use-cases the client cap was intended for disabling LFC & ramping in the kernel driver. It's not really needed for that though:
>
> If the min/max property values differ, the kernel driver can vary the effective refresh range between the limits and perform ramping, otherwise it can't.
>
> If the effective maximum value is at least twice the effective minimum value, the kernel driver can perform LFC, otherwise it can't.
In this case, would the kernel be handling LFC and ramping still? Or would it be a team effort between compositor and kernel?
For example, say the compositor wishes to adjust (min, max) from (48, 48) to (60, 120), and we're using FPS as the unit. I could imagine the following scenarios:
1) Compositor does LFC/ramping
------------------------------
The compositor can make a sequence of atomic commits where (min, max) is set to (50, 60), (60, 75), ..., (100, 120). The difference between (min_1, max_1) itself and subsequent (min_2, max_2) will have to be small enough to avoid panel flicker. IOW, setting (60, 120) is bad because the range is too wide and may cause flicker, and going from (48, 48) to (120, 120) is also bad for the same reason. The compositor has to be fully aware of ramping here.
For LFC, compositor has to be fully aware too, and ramp to the calculated LFC fps. IOW if going from 24fps to 30fps and the panel supports a min of 48hz, the compositor has to ramp from (48, 48) to (60, 60). Setting a (min, max) outside of the panel supported range would be rejected.
2) Kernel does LFC/ramping
--------------------------
The compositor can request a change from (48, 48) to (60, 120) directly. There will be a period of a few frames where FPS would ramp up to the requested range. IOW, the requested (min, max) is not guaranteed to be applied immediately. Since the kernel handles ramping, we would have the same challenges as prior to this API if the (min, max) range is large. Though that might be ok, as long as the compositor is aware.
If the requested min happens to be below the panel supported rate, then kernel can enable LFC, but only if the calculated LFC fps is within the requested (min, max) range. I guess if it happens that the requested min is not possible, i.e.:
* min or max is < min_panel_supported, and
* max is < an int multiple of min,
Then the kernel would have to reject that commit?
3) Team effort
--------------
The compositor can make a sequence of atomic commits where (min, max) is set to (50, 60), (55, 75), ... (60, 120). The kernel will apply the new range immediately, but still apply ramping within the requested (min, max) values. Also, the same challenge mentioned in 2) applies here.
For LFC, I guess it would be the same as 2). The compositor has to know when the min < min_panel_supported, and make sure max is set to be at least double that, and within the panel supported range. The compositor would have to ramp to that target though.
--------
I suppose either 2) or 3) aligns with your thought Michel, but I'm undecided on which is best. Or maybe there's another method of operation that I haven't thought of.
Thanks,
Leo
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-24 15:01 ` Leo Li
@ 2025-09-26 14:19 ` Xaver Hugl
2025-09-27 8:51 ` Michel Dänzer
0 siblings, 1 reply; 22+ messages in thread
From: Xaver Hugl @ 2025-09-26 14:19 UTC (permalink / raw)
To: Leo Li
Cc: Michel Dänzer, Derek Foreman, Chuanyu Tseng, harry.wentland,
Mario.Limonciello, victoria, seanpaul, dri-devel
> 2) Kernel does LFC/ramping
I don't think that would be a good idea. The kernel doing ramping
would mean the user can't (easily) configure it, and it would
complicate the compositor doing ramping with a different strategy
(like reducing the allowed change at lower refresh rates).
Min and max refresh rate / duration is enough for compositors to
implement the features we need, let's not make it more complicated
than necessary please.
> It's possible that compositor set the value which is not acceptable to sink vrr range. It would be better to clamp the false value.
From time to time a user asks about how to override the EDID-provided
VRR range, without having to resort to manually patching an EDID and
overriding it. I think it would be best if the kernel just applies the
value the compositor sets, so that we can allow the user to configure
the minimum refresh rate without having to jump through so many hoops.
If a compositor wants to make sure it adheres to the limits, it can
clamp the value itself very easily.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface
2025-09-26 14:19 ` Xaver Hugl
@ 2025-09-27 8:51 ` Michel Dänzer
0 siblings, 0 replies; 22+ messages in thread
From: Michel Dänzer @ 2025-09-27 8:51 UTC (permalink / raw)
To: Xaver Hugl, Leo Li
Cc: Derek Foreman, Chuanyu Tseng, harry.wentland, Mario.Limonciello,
victoria, seanpaul, dri-devel
On 26.09.25 16:19, Xaver Hugl wrote:
>> 2) Kernel does LFC/ramping
>
> I don't think that would be a good idea. The kernel doing ramping
> would mean the user can't (easily) configure it, and it would
> complicate the compositor doing ramping with a different strategy
> (like reducing the allowed change at lower refresh rates).
What exactly would that mean when the min & max values differ though? If the kernel wants to change the refresh rate, it must not do any ramping but must switch to the new refresh rate immediately? That seems a bit weird as well.
If the compositor doesn't want the kernel to do any ramping, it can set min == max always.
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2025-09-29 8:01 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-12 7:33 [RFC PATCH] drm/uapi: Indroduce a VRR Range Control Interface Chuanyu Tseng
2025-09-12 13:45 ` Derek Foreman
2025-09-15 10:01 ` Michel Dänzer
2025-09-15 15:37 ` Derek Foreman
2025-09-15 15:49 ` Michel Dänzer
2025-09-16 21:56 ` Xaver Hugl
2025-09-17 15:12 ` Derek Foreman
2025-09-18 8:33 ` Tseng, Chuan Yu (Max)
2025-09-22 19:06 ` Leo Li
2025-09-23 8:37 ` Michel Dänzer
2025-09-23 14:35 ` Leo Li
2025-09-24 11:44 ` Michel Dänzer
2025-09-24 8:34 ` Ville Syrjälä
2025-09-24 8:39 ` Michel Dänzer
2025-09-24 9:05 ` Ville Syrjälä
2025-09-24 9:32 ` Tseng, Chuan Yu (Max)
2025-09-24 11:56 ` Michel Dänzer
2025-09-29 8:01 ` Ville Syrjälä
2025-09-24 7:50 ` Michel Dänzer
2025-09-24 15:01 ` Leo Li
2025-09-26 14:19 ` Xaver Hugl
2025-09-27 8:51 ` Michel Dänzer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.