Re: [PATCH i-g-t v5 2/3] lib/igt_vrr:Add VRR helper library for display refresh rate testing

["Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>]

On 8/26/2026 12:03 AM, Naladala Ramanaidu wrote:
> Introduce a new helper library for Variable Refresh Rate (VRR).
> 
> Add helpers to validate targeted refresh-rate testing.
> 
> v2: Modify debugfs with helpers.
> v3: Add helper to check cmrr support.
>      Address review comments. (Mitul)
> v4: Address below review comments.
>      - Add source and rationale for standard timing refresh
>        rates. (Chaitanya)
>      - Restrict target RR debugfs helper to Intel devices. (Chaitanya)
>      - Make Intel target RR debugfs helper return bool instead of
>        asserting in the library. (Mitul)
>      - Avoid assertions in library functions and return status
>        instead. (karthik)
>      - Reorder source list as per naming convention.
>      - Rename cmrr_supported() to igt_vrr_target_refresh_rate_supported()
>        for consistency. (Chaitanya)
>      - Move CMRR-specific macros and enums from the library to the test
>        file. (Chaitanya)
> 
> Assisted-by: GitHub Copilot:Claude Opus 4.6
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
> ---
>   lib/igt_vrr.c   | 149 ++++++++++++++++++++++++++++++++++++++++++++++++
>   lib/igt_vrr.h   |  29 ++++++++++
>   lib/meson.build |   1 +
>   3 files changed, 179 insertions(+)
>   create mode 100644 lib/igt_vrr.c
>   create mode 100644 lib/igt_vrr.h
> 
> diff --git a/lib/igt_vrr.c b/lib/igt_vrr.c
> new file mode 100644
> index 000000000..5b710b3cf
> --- /dev/null
> +++ b/lib/igt_vrr.c
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include <inttypes.h>
> +
> +#include "igt_vrr.h"
> +#include "igt_sysfs.h"
> +
> +/**
> + * Integer refresh rates that sinks advertise as standard video timings, and
> + * for which a fractional (rate * 1000/1001) counterpart is also defined:
> + *
> + *  - 24, 25, 30, 50 and 60 Hz are the film and broadcast (PAL/NTSC) cadences
> + *    carried over into the CTA-861 video formats.
> + *  - 48, 96, 100, 120, 200 and 240 Hz are the integer multiples of those
> + *    cadences, also listed as CTA-861 video formats.
> + *  - 75, 90, 144, 165 and 180 Hz are VESA DMT/CVT and adaptive-sync panel
> + *    rates in common use.
> + *
> + * The fractional variant of each entry (e.g. 60 -> 59.94) is what a target
> + * refresh rate in video mode programs.
> + */
> +const uint32_t igt_vrr_standard_video_timing_fps[] = {
> +	24, 25, 30, 48, 50, 60, 75, 90, 96, 100, 120, 144, 165, 180, 200, 240,
> +};
> +
> +const size_t igt_vrr_standard_video_timing_fps_count =
> +	ARRAY_SIZE(igt_vrr_standard_video_timing_fps);
> +
> +/**
> + * igt_intel_target_rr_debugfs_write:

Don't name these intel specific.

Otherwise these won't belong in lib, right?

For now just make the debugfs check intel specific.
If other drivers want to have similar implementation they can add their 
own debugfs.

Also keep the debugfs check in igt_intel_target_rr_debugfs_read and 
igt_vrr_intel_target_refresh_rate_supported

> + * @fd: DRM file descriptor.
> + * @crtc_index: Index of the CRTC.
> + * @numerator: Numerator of the target refresh rate fraction.
> + * @denominator: Denominator of the target refresh rate fraction.
> + *

Use actual arg names.

> + * Write the target refresh rate configuration to the per-CRTC
> + * VRR debugfs interface. Passing 0/0 clears the target refresh
> + * rate.
> + *
> + * Returns:
> + * true if the target refresh rate was read successfully, false otherwise.

written

> + */
> +bool
> +igt_intel_target_rr_debugfs_write(int fd, int crtc_index,
> +				  uint32_t rr_numerator,
> +				  uint32_t rr_denominator)
> +{
> +	char buf[32];
> +	int ret, dir, len;
> +
> +	if (!is_intel_device(fd)) {
> +		igt_info("Not an Intel device\n");
> +		return false;
> +	}
> +
> +	len = snprintf(buf, sizeof(buf), "%u/%u", rr_numerator, rr_denominator);
> +	if (len <= 0 || len >= (int)sizeof(buf))
> +		return false;
> +
> +	dir = igt_debugfs_crtc_dir(fd, crtc_index);
> +	if (dir < 0)
> +		return false;
> +
> +	ret = igt_sysfs_write(dir, "intel_vrr_target_refresh_rate", buf, len);
> +	close(dir);
> +
> +	return ret == len;
> +}
> +
> +/**
> + * igt_intel_target_rr_debugfs_read:
> + * @fd: DRM file descriptor.
> + * @crtc_index: Index of the CRTC.
> + * @buf: Buffer to store the target refresh rate string.
> + * @size: Size of @buf in bytes.
> + *
> + * Read the target refresh rate from the Intel per-CRTC VRR debugfs
> + * interface. The returned string is always NUL-terminated on success.
> + *
> + * Return:
> + * true if the target refresh rate was read successfully, false otherwise.
> + */
> +bool
> +igt_intel_target_rr_debugfs_read(int fd, int crtc_index, char *buf, size_t size)
> +{
> +	int ret, dir;
> +
> +	if (!buf || !size)
> +		return false;
> +
> +	dir = igt_debugfs_crtc_dir(fd, crtc_index);
> +	if (dir < 0)
> +		return false;
> +
> +	ret = igt_sysfs_read(dir, "intel_vrr_target_refresh_rate",
> +			     buf, size - 1);
> +	close(dir);
> +
> +	if (ret < 0)
> +		return false;
> +
> +	buf[ret] = '\0';
> +
> +	return true;
> +}
> +
> +/**
> + * igt_vrr_mode_line_refresh_hz:
> + * @mode: DRM display mode used for the calculation
> + *
> + * Compute the refresh rate directly from the mode timing parameters.
> + *
> + * Returns: Refresh rate in Hz as a floating-point value.
> + */
> +double igt_vrr_mode_line_refresh_hz(const drmModeModeInfo *mode)
> +{
> +	return (double)mode->clock * 1000.0 / ((double)mode->htotal * (double)mode->vtotal);
> +}
> +
> +/**
> + * igt_vrr_intel_target_refresh_rate_supported:
> + * @fd: DRM device file descriptor.
> + * @crtc_index: Index of the CRTC.
> + *
> + * Checks whether the intel_vrr_target_refresh_rate debugfs node is present
> + * for the specified CRTC, indicating CMRR support.
> + *
> + * Returns: true if CMRR is supported, false otherwise.
> + */
> +bool igt_vrr_intel_target_refresh_rate_supported(int fd, int crtc_index)
> +{
> +	int dir;
> +
> +	dir = igt_debugfs_crtc_dir(fd, crtc_index);
> +
> +	if (dir < 0)
> +		return false;
> +
> +	if (faccessat(dir, "intel_vrr_target_refresh_rate", F_OK, 0) == 0) {
> +		close(dir);
> +		return true;
> +	}
> +
> +	close(dir);
> +	return false;
> +}
> diff --git a/lib/igt_vrr.h b/lib/igt_vrr.h
> new file mode 100644
> index 000000000..d56639727
> --- /dev/null
> +++ b/lib/igt_vrr.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef IGT_VRR_H
> +#define IGT_VRR_H
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include "igt.h"
> +#include "igt_kms.h"
> +
> +extern const uint32_t igt_vrr_standard_video_timing_fps[];
> +extern const size_t igt_vrr_standard_video_timing_fps_count;
> +
> +bool
> +igt_intel_target_rr_debugfs_write(int fd, int crtc_index,
> +				  uint32_t rr_numerator,
> +				  uint32_t rr_denominator);
> +bool
> +igt_intel_target_rr_debugfs_read(int fd, int crtc_index,
> +				 char *buf, size_t size);
> +
> +double igt_vrr_mode_line_refresh_hz(const drmModeModeInfo *mode);
> +
> +bool igt_vrr_intel_target_refresh_rate_supported(int fd, int crtc_index);
> +
> +#endif
> diff --git a/lib/meson.build b/lib/meson.build
> index a7cde027e..b5aece55a 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -56,6 +56,7 @@ lib_sources = [
>   	'igt_vec.c',
>   	'igt_vgem.c',
>   	'igt_vkms.c',
> +        'igt_vrr.c',
>   	'igt_x86.c',
>   	'instdone.c',
>   	'intel_allocator.c',