RE: [PATCH i-g-t v4 1/2] lib/igt_vrr:Add VRR helper library for display refresh rate testing

> -----Original Message-----
> From: Naladala, Ramanaidu <[email protected]>
> Sent: 12 August 2026 18:38
> To: [email protected]
> Cc: Borah, Chaitanya Kumar <[email protected]>; Golani,
> Mitulkumar Ajitkumar <[email protected]>; Naladala,
> Ramanaidu <[email protected]>
> Subject: [PATCH i-g-t v4 1/2] lib/igt_vrr:Add VRR helper library for display
> refresh rate testing
> 
> 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)
> 

Please update change log, this helps reader to track what new changes are coming as part of this revision.

> Signed-off-by: Naladala Ramanaidu <[email protected]>
> ---
>  lib/igt_vrr.c   | 154 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_vrr.h   |  44 ++++++++++++++
>  lib/meson.build |   1 +
>  3 files changed, 199 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..704eaa3a3
> --- /dev/null
> +++ b/lib/igt_vrr.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include <inttypes.h>
> +
> +#include "igt_vrr.h"
> +#include "igt_sysfs.h"
> +
> +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_target_rr_debugfs_write:
> + * @fd: DRM file descriptor.
> + * @crtc_index: Index of the CRTC.
> + * @vrefresh: Target refresh rate to program.
> + * @numerator: Numerator component of the target refresh rate fraction.
> + * @denominator: Denominator component of the target refresh rate
> fraction.
> + *
> + * Write the target refresh rate configuration to the per-CRTC
> + * VRR debugfs interface.
> + *
> + * Returns: None.
> + */
> +void
> +igt_target_rr_debugfs_write(int fd, int crtc_index,
> +			    uint32_t vrefresh,
> +			    uint32_t numerator,
> +			    uint32_t denominator)
> +{
> +	char buf[32];
> +	int ret, dir;
> +	uint64_t val;
> +
> +	val = vrefresh * numerator;

Use val = (uint64_t)vrefresh * numerator; for safety even though current ranges don't overflow.

> +
> +	snprintf(buf, sizeof(buf), "%" PRIu64 "/%u",
> +		 val, denominator);
> +
> +	dir = igt_debugfs_crtc_dir(fd, crtc_index);
> +	igt_require_fd(dir);
> +
> +	ret = igt_sysfs_write(dir, "intel_vrr_target_refresh_rate",
> +			      buf, sizeof(buf) - 1);

write the actual string length, not a fixed size. buf is uninitialized stack, so this sends garbage bytes after the terminating NUL
to the kernel. Please write strlen(buf) and assert against that.

"size_t n = strlen(buf);
ret = igt_sysfs_write(dir, "intel_vrr_target_refresh_rate", buf, n);
igt_assert_f(ret == n, "debugfs write failed\n");"

> +	 close(dir);
> +	 igt_assert_f(ret == (sizeof(buf) - 1), "debugfs_write failed"); }
> +
> +/**
> + * igt_cmrr_debugfs_read:
> + * @fd: DRM file descriptor.
> + * @crtc_index: Index of the CRTC.
> + *
> + * Read the configured refresh rate from the per-CRTC VRR debugfs node.
> + *
> + * Return: None.
> + */
> +void
> +igt_target_rr_debugfs_read(int fd, int crtc_index) {

The above doc block says igt_cmrr_debugfs_read but the function is igt_target_rr_debugfs_read.

> +	char buf[32];
> +	int ret, dir;
> +
> +	dir = igt_debugfs_crtc_dir(fd, crtc_index);
> +	igt_require_fd(dir);
> +
> +	ret = igt_sysfs_read(dir, "intel_vrr_target_refresh_rate",
> +			     buf,  sizeof(buf) - 1);
> +	close(dir);
> +	igt_assert_f(ret >= 0,
> +		     "Failed to read intel_vrr_target_refresh_rate.\n");

The close(dir) and igt_assert_f(...) lines in the write helper use spaces; IGT uses tabs.

> +
> +	buf[ret] = '\0';
> +
> +	igt_info("vrr target RR: %s\n", buf);
> +}
> +
> +/**
> + * 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_get_mode_with_video_timing:
> + * @output: Display output containing connector mode list
> + * @fps: Requested integer refresh rate in Hz
> + * @matched_mode: Returned mode that matches @fps
> + *
> + * Find and return a connector mode that matches the requested
> + * video timing refresh rate in Hz.
> + *
> + * Returns: true when a mode is found, false otherwise  */
> +
> +bool igt_vrr_get_mode_with_video_timing(igt_output_t *output,
> +					uint32_t fps,
> +					drmModeModeInfo
> *matched_mode)
> +{
> +	drmModeConnectorPtr connector;
> +
> +	connector = output->config.connector;
> +	if (!connector)
> +		return false;
> +
> +	for (int i = 0; i < connector->count_modes; i++) {
> +		if (connector->modes[i].vrefresh == fps) {
> +			*matched_mode = connector->modes[i];
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +/**
> + * cmrr_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 cmrr_supported(int fd, int crtc_index) {

add the igt_ namespace prefix. cmrr_supported in a shared lib is inconsistent with every other exported symbol.
Rename to igt_vrr_cmrr_supported() (update the header and callers).

> +	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..a317d4684
> --- /dev/null
> +++ b/lib/igt_vrr.h
> @@ -0,0 +1,44 @@
> +/* 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"
> +
> +#define CMRR_NUMERATOR 1000ULL
> +#define CMRR_DENOMINATOR 1000ULL
> +#define CMRR_VIDEO_MODE_DENOMINATOR 1001ULL #define
> +TARGET_RR_SAMP_COUNT 100
> +
> +enum {
> +	CMRR_VIDEO_MODE,
> +	CMRR_NON_VIDEO_MODE,
> +	CMRR_DISABLE,
> +};
> +
> +extern const uint32_t igt_vrr_standard_video_timing_fps[];
> +extern const size_t igt_vrr_standard_video_timing_fps_count;
> +
> +void
> +igt_target_rr_debugfs_write(int fd, int crtc_index,
> +			    uint32_t vrefresh,
> +			    uint32_t numerator,
> +			    uint32_t denominator);
> +void
> +igt_target_rr_debugfs_read(int fd, int crtc_index);
> +
> +double igt_vrr_mode_line_refresh_hz(const drmModeModeInfo *mode);
> +
> +bool igt_vrr_get_mode_with_video_timing(igt_output_t *output,
> +					uint32_t fps,
> +					drmModeModeInfo
> *matched_mode);
> +
> +bool cmrr_supported(int fd, int crtc_index);
> +
> +#endif
> diff --git a/lib/meson.build b/lib/meson.build index 3001b473e..8675bd4a6
> 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -22,6 +22,7 @@ lib_sources = [
>  	'igt_configfs.c',
>  	'igt_facts.c',
>  	'igt_crc.c',
> +        'igt_vrr.c',
>  	'igt_debugfs.c',
>  	'igt_device.c',
>  	'igt_device_scan.c',
> --
> 2.43.0