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

["Naladala, Ramanaidu" <Ramanaidu.naladala@intel.com>]

Hi Chaitanya,

On 8/27/2026 12:24 PM, Borah, Chaitanya Kumar wrote:
>
>
> 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?
Sure. I will rename the debugfs name.
>
> 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
Sure. I will rename the debugfs names and i will add intel checks inside 
the debugfs.
>
>> + * @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.
Agree. I will update in the next revision.
>
>> + * 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
I will address in the next revision.
>
>> + */
>> +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',
>