[igt-dev] [PATCH v2 0/8] tests/kms_vrr: Modify kms_vrr to allow flicker profiling

[igt-dev] [PATCH v2 0/8] tests/kms_vrr: Modify kms_vrr to allow flicker profiling

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Second revision of the set. New version addresses the review comments
from Bhanu and adds a new maxmin subtest which alternates the rate
between max and min every frame.

Sean Paul (8):
  tests/kms_vrr: Move fb0 and fb1 to an array
  tests/kms_vrr: Move vtest_ns into data_t
  tests/kms_vrr: Allow test rate to be altered from the command line
  tests/kms_vrr: Allow test duration to be specified from the command
    line
  tests/kms_vrr: Change the pattern displayed in the test
  tests/kms_vrr: Add ability to flip static image for flicker profiling
  tests/kms_vrr: Allow for multiple rates in a test
  tests/kms_vrr: Add a max/min test to oscillate between rates

 tests/kms_vrr.c | 213 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 156 insertions(+), 57 deletions(-)

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[igt-dev] [PATCH v2 1/8] tests/kms_vrr: Move fb0 and fb1 to an array

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Consolidate the 2 fb variables into an array. No functional changes.

Cc: Mark Yacoub <markyacoub@chromium.org>
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>

Changes in v2:
- Clarified commit msg (Bhanu)
---
 tests/kms_vrr.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index e203fd4d5..4eeae4747 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -100,8 +100,7 @@ typedef struct data {
 	igt_display_t display;
 	int drm_fd;
 	igt_plane_t *primary;
-	igt_fb_t fb0;
-	igt_fb_t fb1;
+	igt_fb_t fb[2];
 	range_t range;
 } data_t;
 
@@ -268,13 +267,13 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 	/* Prepare resources */
 	igt_create_color_fb(data->drm_fd, mode.hdisplay, mode.vdisplay,
 			    DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
-			    0.50, 0.50, 0.50, &data->fb0);
+			    0.50, 0.50, 0.50, &data->fb[0]);
 
 	igt_create_color_fb(data->drm_fd, mode.hdisplay, mode.vdisplay,
 			    DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
-			    0.50, 0.50, 0.50, &data->fb1);
+			    0.50, 0.50, 0.50, &data->fb[1]);
 
-	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb0);
+	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
 
 	igt_paint_color(cr, 0, 0, mode.hdisplay / 10, mode.vdisplay / 10,
 			1.00, 0.00, 0.00);
@@ -283,7 +282,7 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 
 	/* Take care of any required modesetting before the test begins. */
 	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
-	igt_plane_set_fb(data->primary, &data->fb0);
+	igt_plane_set_fb(data->primary, &data->fb[0]);
 
 	/* Clear vrr_enabled state before enabling it, because
 	 * it might be left enabled if the previous test fails.
@@ -333,7 +332,7 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 	vtest_ns_t vtest_ns = get_test_rate_ns(data->range);
 
 	/* Align with the flip completion event to speed up convergence. */
-	do_flip(data, &data->fb0);
+	do_flip(data, &data->fb[0]);
 	start_ns = last_event_ns = target_ns = get_kernel_event_ns(data,
 							DRM_EVENT_FLIP_COMPLETE);
 
@@ -342,7 +341,7 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 		int64_t diff_ns;
 
 		front = !front;
-		do_flip(data, front ? &data->fb1 : &data->fb0);
+		do_flip(data, front ? &data->fb[1] : &data->fb[0]);
 
 		/* We need to cpture flip event instead of vblank event,
 		 * because vblank is triggered after each frame, but depending
@@ -492,8 +491,8 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 	igt_output_override_mode(output, NULL);
 	igt_display_commit2(&data->display, COMMIT_ATOMIC);
 
-	igt_remove_fb(data->drm_fd, &data->fb1);
-	igt_remove_fb(data->drm_fd, &data->fb0);
+	igt_remove_fb(data->drm_fd, &data->fb[1]);
+	igt_remove_fb(data->drm_fd, &data->fb[0]);
 }
 
 /* Runs tests on outputs that are VRR capable. */
-- 
Sean Paul, Software Engineer, Google / Chromium OS

[igt-dev] [PATCH v2 2/8] tests/kms_vrr: Move vtest_ns into data_t

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

vtest_ns is derived from data->range which is fixed. Move it into data_t
and generate it once in prepare_test().

Cc: Mark Yacoub <markyacoub@chromium.org>
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 tests/kms_vrr.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index 4eeae4747..36f5b100d 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -96,20 +96,21 @@ typedef struct range {
 	unsigned int max;
 } range_t;
 
+typedef struct vtest_ns {
+	uint64_t min;
+	uint64_t mid;
+	uint64_t max;
+} vtest_ns_t;
+
 typedef struct data {
 	igt_display_t display;
 	int drm_fd;
 	igt_plane_t *primary;
 	igt_fb_t fb[2];
 	range_t range;
+	vtest_ns_t vtest_ns;
 } data_t;
 
-typedef struct vtest_ns {
-	uint64_t min;
-	uint64_t mid;
-	uint64_t max;
-} vtest_ns_t;
-
 typedef void (*test_t)(data_t*, enum pipe, igt_output_t*, uint32_t);
 
 /* Converts a timespec structure to nanoseconds. */
@@ -210,18 +211,6 @@ get_vrr_range(data_t *data, igt_output_t *output)
 	return range;
 }
 
-/* Returns vrr test frequency for min, mid & max range. */
-static vtest_ns_t get_test_rate_ns(range_t range)
-{
-	vtest_ns_t vtest_ns;
-
-	vtest_ns.min = rate_from_refresh(range.min);
-	vtest_ns.mid = rate_from_refresh(((range.max + range.min) / 2));
-	vtest_ns.max = rate_from_refresh(range.max);
-
-	return vtest_ns;
-}
-
 /* Returns true if driver supports VRR. */
 static bool has_vrr(igt_output_t *output)
 {
@@ -255,6 +244,11 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 	/* Capture VRR range */
 	data->range = get_vrr_range(data, output);
 
+	data->vtest_ns.min = rate_from_refresh(data->range.min);
+	data->vtest_ns.mid = rate_from_refresh(
+				(data->range.min + data->range.max) / 2);
+	data->vtest_ns.max = rate_from_refresh(data->range.max);
+
 	/* Override mode with max vrefresh.
 	 *   - vrr_min range should be less than the override mode vrefresh.
 	 *   - Limit the vrr_max range with the override mode vrefresh.
@@ -329,7 +323,7 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 	uint64_t start_ns, last_event_ns, target_ns;
 	uint32_t total_flip = 0, total_pass = 0;
 	bool front = false;
-	vtest_ns_t vtest_ns = get_test_rate_ns(data->range);
+	vtest_ns_t vtest_ns = data->vtest_ns;
 
 	/* Align with the flip completion event to speed up convergence. */
 	do_flip(data, &data->fb[0]);
@@ -406,7 +400,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 
 	prepare_test(data, output, pipe);
 	range = data->range;
-	vtest_ns = get_test_rate_ns(range);
+	vtest_ns = data->vtest_ns;
 	rate = vtest_ns.mid;
 
 	igt_info("VRR Test execution on %s, PIPE_%s with VRR range: (%u-%u) Hz\n",
-- 
Sean Paul, Software Engineer, Google / Chromium OS

[igt-dev] [PATCH v2 3/8] tests/kms_vrr: Allow test rate to be altered from the command line

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Instead of always using the midpoint, introduce optional argument
--refresh-rate to allow callers to specify the target refresh rate for
the test.

Cc: Mark Yacoub <markyacoub@chromium.org>
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>

Changes in v2:
- Add short opt to the main callsite (Bhanu)
---
 tests/kms_vrr.c | 44 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index 36f5b100d..62335e9bf 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -98,7 +98,7 @@ typedef struct range {
 
 typedef struct vtest_ns {
 	uint64_t min;
-	uint64_t mid;
+	uint64_t rate_ns;
 	uint64_t max;
 } vtest_ns_t;
 
@@ -245,10 +245,18 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 	data->range = get_vrr_range(data, output);
 
 	data->vtest_ns.min = rate_from_refresh(data->range.min);
-	data->vtest_ns.mid = rate_from_refresh(
-				(data->range.min + data->range.max) / 2);
 	data->vtest_ns.max = rate_from_refresh(data->range.max);
 
+	/* If unspecified on the command line, default rate to the midpoint */
+	if (data->vtest_ns.rate_ns == 0) {
+		range_t *range = &data->range;
+		data->vtest_ns.rate_ns = rate_from_refresh(
+						(range->min + range->max) / 2);
+	}
+	igt_assert_f(data->vtest_ns.rate_ns <= data->vtest_ns.min &&
+		     data->vtest_ns.rate_ns >= data->vtest_ns.max,
+		     "Invalid test rate specified!\n");
+
 	/* Override mode with max vrefresh.
 	 *   - vrr_min range should be less than the override mode vrefresh.
 	 *   - Limit the vrr_max range with the override mode vrefresh.
@@ -401,7 +409,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 	prepare_test(data, output, pipe);
 	range = data->range;
 	vtest_ns = data->vtest_ns;
-	rate = vtest_ns.mid;
+	rate = vtest_ns.rate_ns;
 
 	igt_info("VRR Test execution on %s, PIPE_%s with VRR range: (%u-%u) Hz\n",
 		 output->name, kmstest_pipe_name(pipe), range.min, range.max);
@@ -452,7 +460,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 	}
 
 	if (flags & ~TEST_NEGATIVE) {
-		rate = vtest_ns.mid;
+		rate = vtest_ns.rate_ns;
 		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
 		igt_assert_f(result > 75,
 			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
@@ -473,7 +481,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 	 * a VRR capable panel.
 	 */
 	set_vrr_on_pipe(data, pipe, (flags & TEST_NEGATIVE)? true : false);
-	rate = vtest_ns.mid;
+	rate = vtest_ns.rate_ns;
 	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
 	igt_assert_f(result < 10,
 		     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR %s threshold exceeded, result was %u%%\n",
@@ -525,10 +533,30 @@ run_vrr_test(data_t *data, test_t test, uint32_t flags)
 	}
 }
 
-igt_main
+static int opt_handler(int opt, int opt_index, void *_data)
 {
-	data_t data = {};
+	data_t *data = _data;
+
+	switch (opt) {
+	case 'r':
+		data->vtest_ns.rate_ns = rate_from_refresh(atoi(optarg));
+		break;
+	}
+	return IGT_OPT_HANDLER_SUCCESS;
+}
 
+static const struct option long_opts[] = {
+	{ .name = "refresh-rate", .has_arg = true, .val = 'r', },
+	{}
+};
+
+static const char help_str[] =
+	"  --refresh-rate <refresh-hz>\t\tThe refresh rate to flip at\n";
+
+static data_t data;
+
+igt_main_args("r:", long_opts, help_str, opt_handler, &data)
+{
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

[igt-dev] [PATCH v2 4/8] tests/kms_vrr: Allow test duration to be specified from the command line

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Using --duration argument, otherwise use default.

Cc: Mark Yacoub <markyacoub@chromium.org>
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>

Changes in v2:
- Add short opt to the main callsite (Bhanu)
---
 tests/kms_vrr.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index 62335e9bf..af61c312e 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -109,6 +109,7 @@ typedef struct data {
 	igt_fb_t fb[2];
 	range_t range;
 	vtest_ns_t vtest_ns;
+	uint64_t duration_ns;
 } data_t;
 
 typedef void (*test_t)(data_t*, enum pipe, igt_output_t*, uint32_t);
@@ -257,6 +258,9 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 		     data->vtest_ns.rate_ns >= data->vtest_ns.max,
 		     "Invalid test rate specified!\n");
 
+	if (data->duration_ns == 0)
+		data->duration_ns = TEST_DURATION_NS;
+
 	/* Override mode with max vrefresh.
 	 *   - vrr_min range should be less than the override mode vrefresh.
 	 *   - Limit the vrr_max range with the override mode vrefresh.
@@ -453,7 +457,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 	 */
 	if (flags & TEST_FLIPLINE) {
 		rate = rate_from_refresh(range.max + 5);
-		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
+		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
 		igt_assert_f(result > 75,
 			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
 			     (range.max + 5), rate, result);
@@ -461,7 +465,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 
 	if (flags & ~TEST_NEGATIVE) {
 		rate = vtest_ns.rate_ns;
-		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
+		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
 		igt_assert_f(result > 75,
 			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
 			     ((range.max + range.min) / 2), rate, result);
@@ -469,7 +473,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 
 	if (flags & TEST_FLIPLINE) {
 		rate = rate_from_refresh(range.min - 5);
-		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
+		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
 		igt_assert_f(result < 50,
 			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold exceeded, result was %u%%\n",
 			     (range.min - 5), rate, result);
@@ -482,7 +486,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 	 */
 	set_vrr_on_pipe(data, pipe, (flags & TEST_NEGATIVE)? true : false);
 	rate = vtest_ns.rate_ns;
-	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
+	result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
 	igt_assert_f(result < 10,
 		     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR %s threshold exceeded, result was %u%%\n",
 		     ((range.max + range.min) / 2), rate, (flags & TEST_NEGATIVE)? "on" : "off", result);
@@ -538,6 +542,9 @@ static int opt_handler(int opt, int opt_index, void *_data)
 	data_t *data = _data;
 
 	switch (opt) {
+	case 'd':
+		data->duration_ns = atoi(optarg) * NSECS_PER_SEC;
+		break;
 	case 'r':
 		data->vtest_ns.rate_ns = rate_from_refresh(atoi(optarg));
 		break;
@@ -546,16 +553,18 @@ static int opt_handler(int opt, int opt_index, void *_data)
 }
 
 static const struct option long_opts[] = {
+	{ .name = "duration", .has_arg = true, .val = 'd', },
 	{ .name = "refresh-rate", .has_arg = true, .val = 'r', },
 	{}
 };
 
 static const char help_str[] =
+	"  --duration <duration-seconds>\t\tHow long to run the test for\n"
 	"  --refresh-rate <refresh-hz>\t\tThe refresh rate to flip at\n";
 
 static data_t data;
 
-igt_main_args("r:", long_opts, help_str, opt_handler, &data)
+igt_main_args("d:r:", long_opts, help_str, opt_handler, &data)
 {
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [igt-dev] [PATCH v2 4/8] tests/kms_vrr: Allow test duration to be specified from the command line

[Modem, Bhanuprakash]

On Tue-24-10-2023 02:26 am, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Using --duration argument, otherwise use default.
> 
> Cc: Mark Yacoub <markyacoub@chromium.org>
> Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> 
> Changes in v2:
> - Add short opt to the main callsite (Bhanu)
> ---
>   tests/kms_vrr.c | 19 ++++++++++++++-----
>   1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index 62335e9bf..af61c312e 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -109,6 +109,7 @@ typedef struct data {
>   	igt_fb_t fb[2];
>   	range_t range;
>   	vtest_ns_t vtest_ns;
> +	uint64_t duration_ns;
>   } data_t;
>   
>   typedef void (*test_t)(data_t*, enum pipe, igt_output_t*, uint32_t);
> @@ -257,6 +258,9 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
>   		     data->vtest_ns.rate_ns >= data->vtest_ns.max,
>   		     "Invalid test rate specified!\n");
>   
> +	if (data->duration_ns == 0)
> +		data->duration_ns = TEST_DURATION_NS;
> +
>   	/* Override mode with max vrefresh.
>   	 *   - vrr_min range should be less than the override mode vrefresh.
>   	 *   - Limit the vrr_max range with the override mode vrefresh.
> @@ -453,7 +457,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   	 */
>   	if (flags & TEST_FLIPLINE) {
>   		rate = rate_from_refresh(range.max + 5);
> -		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
> +		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
>   		igt_assert_f(result > 75,
>   			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
>   			     (range.max + 5), rate, result);
> @@ -461,7 +465,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   
>   	if (flags & ~TEST_NEGATIVE) {
>   		rate = vtest_ns.rate_ns;
> -		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
> +		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
>   		igt_assert_f(result > 75,
>   			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
>   			     ((range.max + range.min) / 2), rate, result);
> @@ -469,7 +473,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   
>   	if (flags & TEST_FLIPLINE) {
>   		rate = rate_from_refresh(range.min - 5);
> -		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
> +		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
>   		igt_assert_f(result < 50,
>   			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold exceeded, result was %u%%\n",
>   			     (range.min - 5), rate, result);
> @@ -482,7 +486,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   	 */
>   	set_vrr_on_pipe(data, pipe, (flags & TEST_NEGATIVE)? true : false);
>   	rate = vtest_ns.rate_ns;
> -	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
> +	result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
>   	igt_assert_f(result < 10,
>   		     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR %s threshold exceeded, result was %u%%\n",
>   		     ((range.max + range.min) / 2), rate, (flags & TEST_NEGATIVE)? "on" : "off", result);
> @@ -538,6 +542,9 @@ static int opt_handler(int opt, int opt_index, void *_data)
>   	data_t *data = _data;
>   
>   	switch (opt) {
> +	case 'd':
> +		data->duration_ns = atoi(optarg) * NSECS_PER_SEC;
> +		break;
>   	case 'r':
>   		data->vtest_ns.rate_ns = rate_from_refresh(atoi(optarg));
>   		break;
> @@ -546,16 +553,18 @@ static int opt_handler(int opt, int opt_index, void *_data)
>   }
>   
>   static const struct option long_opts[] = {
> +	{ .name = "duration", .has_arg = true, .val = 'd', },
>   	{ .name = "refresh-rate", .has_arg = true, .val = 'r', },
>   	{}
>   };
>   
>   static const char help_str[] =
> +	"  --duration <duration-seconds>\t\tHow long to run the test for\n"
>   	"  --refresh-rate <refresh-hz>\t\tThe refresh rate to flip at\n";
>   
>   static data_t data;
>   
> -igt_main_args("r:", long_opts, help_str, opt_handler, &data)
> +igt_main_args("d:r:", long_opts, help_str, opt_handler, &data)
-------------------^
Nit: s/"d:r:"/"dr:"

- Bhanu

>   {
>   	igt_fixture {
>   		data.drm_fd = drm_open_driver_master(DRIVER_ANY);

[igt-dev] [PATCH v2 5/8] tests/kms_vrr: Change the pattern displayed in the test

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Upgrade the tiny box in the top left corner to some vertical color
bars with horizontal grey and white bars at the bottom.

Cc: Mark Yacoub <markyacoub@chromium.org>
Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>

Changes in v2 (Bhanu):
- Removed hardcoded bar heights
- Made number of vertical bars configurable
- Fill in the entire screen horizontally
---
 tests/kms_vrr.c | 45 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index af61c312e..34d75d648 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -232,11 +232,31 @@ static void set_vrr_on_pipe(data_t *data, enum pipe pipe, bool enabled)
 	igt_display_commit2(&data->display, COMMIT_ATOMIC);
 }
 
+static void paint_bar(cairo_t *cr, unsigned int x, unsigned int y,
+		      unsigned int w, unsigned int h,
+		      unsigned int bar, unsigned int num_bars,
+		      float start_r, float start_g, float start_b,
+		      float end_r, float end_g, float end_b)
+{
+	float progress = (float)bar / (float)num_bars;
+	float color[] = {
+		start_r + progress * (end_r - start_r),
+		start_g + progress * (end_g - start_g),
+		start_b + progress * (end_b - start_b)
+	};
+	igt_paint_color(cr, x, y, w, h,
+			color[0] > 0 ? color[0] : 0.0,
+			color[1] > 0 ? color[1] : 0.0,
+			color[2] > 0 ? color[2] : 0.0);
+}
+
 /* Prepare the display for testing on the given pipe. */
 static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 {
+	static const unsigned int num_bars = 256;
 	drmModeModeInfo mode;
 	cairo_t *cr;
+	int bar_width, bar_height, bar_remaining, horizontal_bar_height;
 
 	/* Reset output */
 	igt_display_reset(&data->display);
@@ -279,11 +299,28 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 			    DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
 			    0.50, 0.50, 0.50, &data->fb[1]);
 
+	bar_width = mode.hdisplay / num_bars;
+	horizontal_bar_height = mode.vdisplay / 8;
+	bar_height = mode.vdisplay - horizontal_bar_height * 2;
+	bar_remaining = mode.hdisplay % bar_width;
 	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
-
-	igt_paint_color(cr, 0, 0, mode.hdisplay / 10, mode.vdisplay / 10,
-			1.00, 0.00, 0.00);
-
+	for (int j = 0; j < num_bars; ++j) {
+		unsigned int width = bar_width;
+		if (j == num_bars - 1)
+			width += bar_remaining;
+
+		/* Red->Green->Blue gradient */
+		if (j < num_bars / 2)
+			paint_bar(cr, j * bar_width, 0, width, bar_height,
+				  j, num_bars / 2,
+				  1.0, 0.0, 0.0, 0.0, 1.0, 0.0);
+		else
+			paint_bar(cr, j * bar_width, 0, width, bar_height,
+				  j - num_bars / 2, num_bars / 2,
+				  0.0, 1.0, 0.0, 0.0, 0.0, 1.0);
+	}
+	igt_paint_color(cr, 0, mode.vdisplay - horizontal_bar_height,
+			mode.hdisplay, horizontal_bar_height, 1.00, 1.00, 1.00);
 	igt_put_cairo_ctx(cr);
 
 	/* Take care of any required modesetting before the test begins. */
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [igt-dev] [PATCH v2 5/8] tests/kms_vrr: Change the pattern displayed in the test

[Modem, Bhanuprakash]

On Tue-24-10-2023 02:26 am, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Upgrade the tiny box in the top left corner to some vertical color
> bars with horizontal grey and white bars at the bottom.
> 
> Cc: Mark Yacoub <markyacoub@chromium.org>
> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> 
> Changes in v2 (Bhanu):
> - Removed hardcoded bar heights
> - Made number of vertical bars configurable
> - Fill in the entire screen horizontally
> ---
>   tests/kms_vrr.c | 45 +++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index af61c312e..34d75d648 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -232,11 +232,31 @@ static void set_vrr_on_pipe(data_t *data, enum pipe pipe, bool enabled)
>   	igt_display_commit2(&data->display, COMMIT_ATOMIC);
>   }
>   
> +static void paint_bar(cairo_t *cr, unsigned int x, unsigned int y,
> +		      unsigned int w, unsigned int h,
> +		      unsigned int bar, unsigned int num_bars,
> +		      float start_r, float start_g, float start_b,
> +		      float end_r, float end_g, float end_b)
> +{
> +	float progress = (float)bar / (float)num_bars;
> +	float color[] = {
> +		start_r + progress * (end_r - start_r),
> +		start_g + progress * (end_g - start_g),
> +		start_b + progress * (end_b - start_b)
> +	};
> +	igt_paint_color(cr, x, y, w, h,
> +			color[0] > 0 ? color[0] : 0.0,
> +			color[1] > 0 ? color[1] : 0.0,
> +			color[2] > 0 ? color[2] : 0.0);
> +}
> +
>   /* Prepare the display for testing on the given pipe. */
>   static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
>   {
> +	static const unsigned int num_bars = 256;

Why static?

>   	drmModeModeInfo mode;
>   	cairo_t *cr;
> +	int bar_width, bar_height, bar_remaining, horizontal_bar_height;
>   
>   	/* Reset output */
>   	igt_display_reset(&data->display);
> @@ -279,11 +299,28 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
>   			    DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
>   			    0.50, 0.50, 0.50, &data->fb[1]);
>   
> +	bar_width = mode.hdisplay / num_bars;
> +	horizontal_bar_height = mode.vdisplay / 8 > +	bar_height = mode.vdisplay - horizontal_bar_height * 2;
> +	bar_remaining = mode.hdisplay % bar_width;
>   	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
> -
> -	igt_paint_color(cr, 0, 0, mode.hdisplay / 10, mode.vdisplay / 10,
> -			1.00, 0.00, 0.00);
> -
> +	for (int j = 0; j < num_bars; ++j) {
> +		unsigned int width = bar_width;
> +		if (j == num_bars - 1)
> +			width += bar_remaining;
> +
> +		/* Red->Green->Blue gradient */
> +		if (j < num_bars / 2)
> +			paint_bar(cr, j * bar_width, 0, width, bar_height,
> +				  j, num_bars / 2,
> +				  1.0, 0.0, 0.0, 0.0, 1.0, 0.0);
> +		else
> +			paint_bar(cr, j * bar_width, 0, width, bar_height,
> +				  j - num_bars / 2, num_bars / 2,
> +				  0.0, 1.0, 0.0, 0.0, 0.0, 1.0);
> +	}

As per the commit message, grey bar is missing here.

igt_paint_color(cr, 0, bar_height,
		mode.hdisplay, horizontal_bar_height, 0.5, 0.5, 0.5);

> +	igt_paint_color(cr, 0, mode.vdisplay - horizontal_bar_height,
-------------------------------^
Should be (bar_height + horizontal_bar_height), right?

- Bhanu

> +			mode.hdisplay, horizontal_bar_height, 1.00, 1.00, 1.00);
>   	igt_put_cairo_ctx(cr);
>   
>   	/* Take care of any required modesetting before the test begins. */

Re: [igt-dev] [PATCH v2 5/8] tests/kms_vrr: Change the pattern displayed in the test

[Modem, Bhanuprakash]

On 16-11-2023 01:55 pm, Modem, Bhanuprakash wrote:
> 
> On Tue-24-10-2023 02:26 am, Sean Paul wrote:
>> From: Sean Paul <seanpaul@chromium.org>
>>
>> Upgrade the tiny box in the top left corner to some vertical color
>> bars with horizontal grey and white bars at the bottom.
>>
>> Cc: Mark Yacoub <markyacoub@chromium.org>
>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>>
>> Changes in v2 (Bhanu):
>> - Removed hardcoded bar heights
>> - Made number of vertical bars configurable
>> - Fill in the entire screen horizontally
>> ---
>>   tests/kms_vrr.c | 45 +++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 41 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
>> index af61c312e..34d75d648 100644
>> --- a/tests/kms_vrr.c
>> +++ b/tests/kms_vrr.c
>> @@ -232,11 +232,31 @@ static void set_vrr_on_pipe(data_t *data, enum 
>> pipe pipe, bool enabled)
>>       igt_display_commit2(&data->display, COMMIT_ATOMIC);
>>   }
>> +static void paint_bar(cairo_t *cr, unsigned int x, unsigned int y,
>> +              unsigned int w, unsigned int h,
>> +              unsigned int bar, unsigned int num_bars,
>> +              float start_r, float start_g, float start_b,
>> +              float end_r, float end_g, float end_b)
>> +{
>> +    float progress = (float)bar / (float)num_bars;
>> +    float color[] = {
>> +        start_r + progress * (end_r - start_r),
>> +        start_g + progress * (end_g - start_g),
>> +        start_b + progress * (end_b - start_b)
>> +    };
>> +    igt_paint_color(cr, x, y, w, h,
>> +            color[0] > 0 ? color[0] : 0.0,
>> +            color[1] > 0 ? color[1] : 0.0,
>> +            color[2] > 0 ? color[2] : 0.0);
>> +}
>> +
>>   /* Prepare the display for testing on the given pipe. */
>>   static void prepare_test(data_t *data, igt_output_t *output, enum 
>> pipe pipe)
>>   {
>> +    static const unsigned int num_bars = 256;
> 
> Why static?
> 
>>       drmModeModeInfo mode;
>>       cairo_t *cr;
>> +    int bar_width, bar_height, bar_remaining, horizontal_bar_height;
>>       /* Reset output */
>>       igt_display_reset(&data->display);
>> @@ -279,11 +299,28 @@ static void prepare_test(data_t *data, 
>> igt_output_t *output, enum pipe pipe)
>>                   DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
>>                   0.50, 0.50, 0.50, &data->fb[1]);
>> +    bar_width = mode.hdisplay / num_bars;
>> +    horizontal_bar_height = mode.vdisplay / 8 > +    bar_height = 
>> mode.vdisplay - horizontal_bar_height * 2;
>> +    bar_remaining = mode.hdisplay % bar_width;
>>       cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
>> -
>> -    igt_paint_color(cr, 0, 0, mode.hdisplay / 10, mode.vdisplay / 10,
>> -            1.00, 0.00, 0.00);
>> -
>> +    for (int j = 0; j < num_bars; ++j) {
>> +        unsigned int width = bar_width;
>> +        if (j == num_bars - 1)
>> +            width += bar_remaining;
>> +
>> +        /* Red->Green->Blue gradient */
>> +        if (j < num_bars / 2)
>> +            paint_bar(cr, j * bar_width, 0, width, bar_height,
>> +                  j, num_bars / 2,
>> +                  1.0, 0.0, 0.0, 0.0, 1.0, 0.0);
>> +        else
>> +            paint_bar(cr, j * bar_width, 0, width, bar_height,
>> +                  j - num_bars / 2, num_bars / 2,
>> +                  0.0, 1.0, 0.0, 0.0, 0.0, 1.0);
>> +    }
> 
> As per the commit message, grey bar is missing here.
> 
> igt_paint_color(cr, 0, bar_height,
>          mode.hdisplay, horizontal_bar_height, 0.5, 0.5, 0.5);

Got it, As we already created a gray framebuffer & painting the patters 
on it, no need to paint the gray bar again.

Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

- Bhanu

> 
>> +    igt_paint_color(cr, 0, mode.vdisplay - horizontal_bar_height,
> -------------------------------^
> Should be (bar_height + horizontal_bar_height), right?
> 
> - Bhanu
> 
>> +            mode.hdisplay, horizontal_bar_height, 1.00, 1.00, 1.00);
>>       igt_put_cairo_ctx(cr);
>>       /* Take care of any required modesetting before the test begins. */

Re: [igt-dev] [PATCH v2 5/8] tests/kms_vrr: Change the pattern displayed in the test

[Modem, Bhanuprakash]

On 16-11-2023 01:55 pm, Modem, Bhanuprakash wrote:
> 
> On Tue-24-10-2023 02:26 am, Sean Paul wrote:
>> From: Sean Paul <seanpaul@chromium.org>
>>
>> Upgrade the tiny box in the top left corner to some vertical color
>> bars with horizontal grey and white bars at the bottom.
>>
>> Cc: Mark Yacoub <markyacoub@chromium.org>
>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>>
>> Changes in v2 (Bhanu):
>> - Removed hardcoded bar heights
>> - Made number of vertical bars configurable
>> - Fill in the entire screen horizontally
>> ---
>>   tests/kms_vrr.c | 45 +++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 41 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
>> index af61c312e..34d75d648 100644
>> --- a/tests/kms_vrr.c
>> +++ b/tests/kms_vrr.c
>> @@ -232,11 +232,31 @@ static void set_vrr_on_pipe(data_t *data, enum 
>> pipe pipe, bool enabled)
>>       igt_display_commit2(&data->display, COMMIT_ATOMIC);
>>   }
>> +static void paint_bar(cairo_t *cr, unsigned int x, unsigned int y,
>> +              unsigned int w, unsigned int h,
>> +              unsigned int bar, unsigned int num_bars,
>> +              float start_r, float start_g, float start_b,
>> +              float end_r, float end_g, float end_b)
>> +{
>> +    float progress = (float)bar / (float)num_bars;
>> +    float color[] = {
>> +        start_r + progress * (end_r - start_r),
>> +        start_g + progress * (end_g - start_g),
>> +        start_b + progress * (end_b - start_b)
>> +    };
>> +    igt_paint_color(cr, x, y, w, h,
>> +            color[0] > 0 ? color[0] : 0.0,
>> +            color[1] > 0 ? color[1] : 0.0,
>> +            color[2] > 0 ? color[2] : 0.0);
>> +}
>> +
>>   /* Prepare the display for testing on the given pipe. */
>>   static void prepare_test(data_t *data, igt_output_t *output, enum 
>> pipe pipe)
>>   {
>> +    static const unsigned int num_bars = 256;
> 
> Why static?
> 
>>       drmModeModeInfo mode;
>>       cairo_t *cr;
>> +    int bar_width, bar_height, bar_remaining, horizontal_bar_height;
>>       /* Reset output */
>>       igt_display_reset(&data->display);
>> @@ -279,11 +299,28 @@ static void prepare_test(data_t *data, 
>> igt_output_t *output, enum pipe pipe)
>>                   DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
>>                   0.50, 0.50, 0.50, &data->fb[1]);
>> +    bar_width = mode.hdisplay / num_bars;
>> +    horizontal_bar_height = mode.vdisplay / 8 > +    bar_height = 
>> mode.vdisplay - horizontal_bar_height * 2;
>> +    bar_remaining = mode.hdisplay % bar_width;
>>       cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
>> -
>> -    igt_paint_color(cr, 0, 0, mode.hdisplay / 10, mode.vdisplay / 10,
>> -            1.00, 0.00, 0.00);
>> -
>> +    for (int j = 0; j < num_bars; ++j) {
>> +        unsigned int width = bar_width;
>> +        if (j == num_bars - 1)
>> +            width += bar_remaining;
>> +
>> +        /* Red->Green->Blue gradient */
>> +        if (j < num_bars / 2)
>> +            paint_bar(cr, j * bar_width, 0, width, bar_height,
>> +                  j, num_bars / 2,
>> +                  1.0, 0.0, 0.0, 0.0, 1.0, 0.0);
>> +        else
>> +            paint_bar(cr, j * bar_width, 0, width, bar_height,
>> +                  j - num_bars / 2, num_bars / 2,
>> +                  0.0, 1.0, 0.0, 0.0, 0.0, 1.0);
>> +    }
> 
> As per the commit message, grey bar is missing here.
> 
> igt_paint_color(cr, 0, bar_height,
>          mode.hdisplay, horizontal_bar_height, 0.5, 0.5, 0.5);

Got it, As we already created a gray framebuffer & painting the patters 
on it, no need to paint the gray bar again.

Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

- Bhanu

> 
>> +    igt_paint_color(cr, 0, mode.vdisplay - horizontal_bar_height,
> -------------------------------^
> Should be (bar_height + horizontal_bar_height), right?
> 
> - Bhanu
> 
>> +            mode.hdisplay, horizontal_bar_height, 1.00, 1.00, 1.00);
>>       igt_put_cairo_ctx(cr);
>>       /* Take care of any required modesetting before the test begins. */

[igt-dev] [PATCH v2 6/8] tests/kms_vrr: Add ability to flip static image for flicker profiling

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Add a --static-image argument which will paint both framebuffers
identically. This allows us to profile VRR flicker on the panel.

Cc: Mark Yacoub <markyacoub@chromium.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>

Changes in v2:
- Add short opt to the main callsite (Bhanu)
---
 tests/kms_vrr.c | 53 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index 34d75d648..57bd87ddf 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -110,6 +110,7 @@ typedef struct data {
 	range_t range;
 	vtest_ns_t vtest_ns;
 	uint64_t duration_ns;
+	bool static_image;
 } data_t;
 
 typedef void (*test_t)(data_t*, enum pipe, igt_output_t*, uint32_t);
@@ -257,6 +258,7 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 	drmModeModeInfo mode;
 	cairo_t *cr;
 	int bar_width, bar_height, bar_remaining, horizontal_bar_height;
+	int num_painted_fbs;
 
 	/* Reset output */
 	igt_display_reset(&data->display);
@@ -303,25 +305,31 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
 	horizontal_bar_height = mode.vdisplay / 8;
 	bar_height = mode.vdisplay - horizontal_bar_height * 2;
 	bar_remaining = mode.hdisplay % bar_width;
-	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
-	for (int j = 0; j < num_bars; ++j) {
-		unsigned int width = bar_width;
-		if (j == num_bars - 1)
-			width += bar_remaining;
-
-		/* Red->Green->Blue gradient */
-		if (j < num_bars / 2)
-			paint_bar(cr, j * bar_width, 0, width, bar_height,
-				  j, num_bars / 2,
-				  1.0, 0.0, 0.0, 0.0, 1.0, 0.0);
-		else
-			paint_bar(cr, j * bar_width, 0, width, bar_height,
-				  j - num_bars / 2, num_bars / 2,
-				  0.0, 1.0, 0.0, 0.0, 0.0, 1.0);
+	num_painted_fbs = data->static_image ? 2 : 1;
+	for (int i = 0; i < num_painted_fbs; ++i) {
+		cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[i]);
+		for (int j = 0; j < num_bars; ++j) {
+			unsigned int width = bar_width;
+			if (j == num_bars - 1)
+				width += bar_remaining;
+
+			/* Red->Green->Blue gradient */
+			if (j < num_bars / 2)
+				paint_bar(cr, j * bar_width, 0, width,
+					  bar_height,
+					  j, num_bars / 2,
+					  1.0, 0.0, 0.0, 0.0, 1.0, 0.0);
+			else
+				paint_bar(cr, j * bar_width, 0, width,
+					  bar_height,
+					  j - num_bars / 2, num_bars / 2,
+					  0.0, 1.0, 0.0, 0.0, 0.0, 1.0);
+		}
+		igt_paint_color(cr, 0, mode.vdisplay - horizontal_bar_height,
+				mode.hdisplay, horizontal_bar_height,
+				1.00, 1.00, 1.00);
+		igt_put_cairo_ctx(cr);
 	}
-	igt_paint_color(cr, 0, mode.vdisplay - horizontal_bar_height,
-			mode.hdisplay, horizontal_bar_height, 1.00, 1.00, 1.00);
-	igt_put_cairo_ctx(cr);
 
 	/* Take care of any required modesetting before the test begins. */
 	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
@@ -585,6 +593,9 @@ static int opt_handler(int opt, int opt_index, void *_data)
 	case 'r':
 		data->vtest_ns.rate_ns = rate_from_refresh(atoi(optarg));
 		break;
+	case 's':
+		data->static_image = true;
+		break;
 	}
 	return IGT_OPT_HANDLER_SUCCESS;
 }
@@ -592,16 +603,18 @@ static int opt_handler(int opt, int opt_index, void *_data)
 static const struct option long_opts[] = {
 	{ .name = "duration", .has_arg = true, .val = 'd', },
 	{ .name = "refresh-rate", .has_arg = true, .val = 'r', },
+	{ .name = "static-image", .has_arg = false, .val = 's', },
 	{}
 };
 
 static const char help_str[] =
 	"  --duration <duration-seconds>\t\tHow long to run the test for\n"
-	"  --refresh-rate <refresh-hz>\t\tThe refresh rate to flip at\n";
+	"  --refresh-rate <refresh-hz>\t\tThe refresh rate to flip at\n"
+	"  --static-image\t\tFlip a static image for flicker profiling\n";
 
 static data_t data;
 
-igt_main_args("d:r:", long_opts, help_str, opt_handler, &data)
+igt_main_args("d:r:s", long_opts, help_str, opt_handler, &data)
 {
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [igt-dev] [PATCH v2 6/8] tests/kms_vrr: Add ability to flip static image for flicker profiling

[Modem, Bhanuprakash]

On Tue-24-10-2023 02:26 am, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Add a --static-image argument which will paint both framebuffers
> identically. This allows us to profile VRR flicker on the panel.
> 
> Cc: Mark Yacoub <markyacoub@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> 
> Changes in v2:
> - Add short opt to the main callsite (Bhanu)
> ---
>   tests/kms_vrr.c | 53 ++++++++++++++++++++++++++++++-------------------
>   1 file changed, 33 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index 34d75d648..57bd87ddf 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -110,6 +110,7 @@ typedef struct data {
>   	range_t range;
>   	vtest_ns_t vtest_ns;
>   	uint64_t duration_ns;
> +	bool static_image;
>   } data_t;
>   
>   typedef void (*test_t)(data_t*, enum pipe, igt_output_t*, uint32_t);
> @@ -257,6 +258,7 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
>   	drmModeModeInfo mode;
>   	cairo_t *cr;
>   	int bar_width, bar_height, bar_remaining, horizontal_bar_height;
> +	int num_painted_fbs;
>   
>   	/* Reset output */
>   	igt_display_reset(&data->display);
> @@ -303,25 +305,31 @@ static void prepare_test(data_t *data, igt_output_t *output, enum pipe pipe)
>   	horizontal_bar_height = mode.vdisplay / 8;
>   	bar_height = mode.vdisplay - horizontal_bar_height * 2;
>   	bar_remaining = mode.hdisplay % bar_width;
> -	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
> -	for (int j = 0; j < num_bars; ++j) {
> -		unsigned int width = bar_width;
> -		if (j == num_bars - 1)
> -			width += bar_remaining;
> -
> -		/* Red->Green->Blue gradient */
> -		if (j < num_bars / 2)
> -			paint_bar(cr, j * bar_width, 0, width, bar_height,
> -				  j, num_bars / 2,
> -				  1.0, 0.0, 0.0, 0.0, 1.0, 0.0);
> -		else
> -			paint_bar(cr, j * bar_width, 0, width, bar_height,
> -				  j - num_bars / 2, num_bars / 2,
> -				  0.0, 1.0, 0.0, 0.0, 0.0, 1.0);
> +	num_painted_fbs = data->static_image ? 2 : 1;
> +	for (int i = 0; i < num_painted_fbs; ++i) {
> +		cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[i]);
> +		for (int j = 0; j < num_bars; ++j) {
> +			unsigned int width = bar_width;
> +			if (j == num_bars - 1)
> +				width += bar_remaining;
> +
> +			/* Red->Green->Blue gradient */
> +			if (j < num_bars / 2)
> +				paint_bar(cr, j * bar_width, 0, width,
> +					  bar_height,
> +					  j, num_bars / 2,
> +					  1.0, 0.0, 0.0, 0.0, 1.0, 0.0);
> +			else
> +				paint_bar(cr, j * bar_width, 0, width,
> +					  bar_height,
> +					  j - num_bars / 2, num_bars / 2,
> +					  0.0, 1.0, 0.0, 0.0, 0.0, 1.0);
> +		}
> +		igt_paint_color(cr, 0, mode.vdisplay - horizontal_bar_height,
> +				mode.hdisplay, horizontal_bar_height,
> +				1.00, 1.00, 1.00);
> +		igt_put_cairo_ctx(cr);
>   	}
> -	igt_paint_color(cr, 0, mode.vdisplay - horizontal_bar_height,
> -			mode.hdisplay, horizontal_bar_height, 1.00, 1.00, 1.00);
> -	igt_put_cairo_ctx(cr);
>   
>   	/* Take care of any required modesetting before the test begins. */
>   	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> @@ -585,6 +593,9 @@ static int opt_handler(int opt, int opt_index, void *_data)
>   	case 'r':
>   		data->vtest_ns.rate_ns = rate_from_refresh(atoi(optarg));
>   		break;
> +	case 's':
> +		data->static_image = true;
> +		break;
>   	}
>   	return IGT_OPT_HANDLER_SUCCESS;
>   }
> @@ -592,16 +603,18 @@ static int opt_handler(int opt, int opt_index, void *_data)
>   static const struct option long_opts[] = {
>   	{ .name = "duration", .has_arg = true, .val = 'd', },
>   	{ .name = "refresh-rate", .has_arg = true, .val = 'r', },
> +	{ .name = "static-image", .has_arg = false, .val = 's', },
>   	{}
>   };
>   
>   static const char help_str[] =
>   	"  --duration <duration-seconds>\t\tHow long to run the test for\n"
> -	"  --refresh-rate <refresh-hz>\t\tThe refresh rate to flip at\n";
> +	"  --refresh-rate <refresh-hz>\t\tThe refresh rate to flip at\n"
> +	"  --static-image\t\tFlip a static image for flicker profiling\n";
>   
>   static data_t data;
>   
> -igt_main_args("d:r:", long_opts, help_str, opt_handler, &data)
> +igt_main_args("d:r:s", long_opts, help_str, opt_handler, &data)
------------------^
Nit: s/"d:r:s"/"drs:"/

With this change, please add my
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

- Bhanu

>   {
>   	igt_fixture {
>   		data.drm_fd = drm_open_driver_master(DRIVER_ANY);

[igt-dev] [PATCH v2 7/8] tests/kms_vrr: Allow for multiple rates in a test

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Instead of passing just one rate into flip_and_measure, pass in an array
to allow for tests which vary the refresh rate from one frame to the
next. Currently the test will just cycle through the rates in order per
frame.

Just framework, no changes to any tests.

Signed-off-by: Sean Paul <seanpaul@chromium.org>

Changes in v2:
- Added to the set
---
 tests/kms_vrr.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index 57bd87ddf..b986087f0 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -375,7 +375,7 @@ do_flip(data_t *data, igt_fb_t *fb)
  */
 static uint32_t
 flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
-		 uint64_t rate_ns, uint64_t duration_ns)
+		 uint64_t *rates_ns, int num_rates, uint64_t duration_ns)
 {
 	uint64_t start_ns, last_event_ns, target_ns;
 	uint32_t total_flip = 0, total_pass = 0;
@@ -387,9 +387,10 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 	start_ns = last_event_ns = target_ns = get_kernel_event_ns(data,
 							DRM_EVENT_FLIP_COMPLETE);
 
-	for (;;) {
+	for (int i = 0;;++i) {
 		uint64_t event_ns, wait_ns;
 		int64_t diff_ns;
+		uint64_t rate_ns = rates_ns[i % num_rates];
 
 		front = !front;
 		do_flip(data, front ? &data->fb[1] : &data->fb[0]);
@@ -440,8 +441,12 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
 		while (get_time_ns() < target_ns - 10);
 	}
 
-	igt_info("Completed %u flips, %u were in threshold for (%llu Hz) %"PRIu64"ns.\n",
-		 total_flip, total_pass, (NSECS_PER_SEC/rate_ns), rate_ns);
+	igt_info("Completed %u flips, %u were in threshold for [", total_flip, total_pass);
+	for (int i = 0; i < num_rates; ++i) {
+		igt_info("(%llu Hz) %"PRIu64"ns%s", (NSECS_PER_SEC/rates_ns[i]), rates_ns[i],
+			 i < num_rates - 1 ? "," : "");
+	}
+	igt_info("]\n");
 
 	return total_flip ? ((total_pass * 100) / total_flip) : 0;
 }
@@ -453,12 +458,12 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 	uint32_t result;
 	vtest_ns_t vtest_ns;
 	range_t range;
-	uint64_t rate;
+	uint64_t rate[] = {0};
 
 	prepare_test(data, output, pipe);
 	range = data->range;
 	vtest_ns = data->vtest_ns;
-	rate = vtest_ns.rate_ns;
+	rate[0] = vtest_ns.rate_ns;
 
 	igt_info("VRR Test execution on %s, PIPE_%s with VRR range: (%u-%u) Hz\n",
 		 output->name, kmstest_pipe_name(pipe), range.min, range.max);
@@ -470,7 +475,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 	 * This is to make sure we were actually in the middle of
 	 * active flipping before doing the DPMS/suspend steps.
 	 */
-	flip_and_measure(data, output, pipe, rate, 250000000ull);
+	flip_and_measure(data, output, pipe, rate, 1, 250000000ull);
 
 	if (flags & TEST_DPMS) {
 		kmstest_set_connector_dpms(output->display->drm_fd,
@@ -501,27 +506,27 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 	 *      next Vmin.
 	 */
 	if (flags & TEST_FLIPLINE) {
-		rate = rate_from_refresh(range.max + 5);
-		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
+		rate[0] = rate_from_refresh(range.max + 5);
+		result = flip_and_measure(data, output, pipe, rate, 1, data->duration_ns);
 		igt_assert_f(result > 75,
 			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
-			     (range.max + 5), rate, result);
+			     (range.max + 5), rate[0], result);
 	}
 
 	if (flags & ~TEST_NEGATIVE) {
-		rate = vtest_ns.rate_ns;
-		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
+		rate[0] = vtest_ns.rate_ns;
+		result = flip_and_measure(data, output, pipe, rate, 1, data->duration_ns);
 		igt_assert_f(result > 75,
 			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
-			     ((range.max + range.min) / 2), rate, result);
+			     ((range.max + range.min) / 2), rate[0], result);
 	}
 
 	if (flags & TEST_FLIPLINE) {
-		rate = rate_from_refresh(range.min - 5);
-		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
+		rate[0] = rate_from_refresh(range.min - 5);
+		result = flip_and_measure(data, output, pipe, rate, 1, data->duration_ns);
 		igt_assert_f(result < 50,
 			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold exceeded, result was %u%%\n",
-			     (range.min - 5), rate, result);
+			     (range.min - 5), rate[0], result);
 	}
 
 	/*
@@ -530,11 +535,11 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 	 * a VRR capable panel.
 	 */
 	set_vrr_on_pipe(data, pipe, (flags & TEST_NEGATIVE)? true : false);
-	rate = vtest_ns.rate_ns;
-	result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
+	rate[0] = vtest_ns.rate_ns;
+	result = flip_and_measure(data, output, pipe, rate, 1, data->duration_ns);
 	igt_assert_f(result < 10,
 		     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR %s threshold exceeded, result was %u%%\n",
-		     ((range.max + range.min) / 2), rate, (flags & TEST_NEGATIVE)? "on" : "off", result);
+		     ((range.max + range.min) / 2), rate[0], (flags & TEST_NEGATIVE)? "on" : "off", result);
 
 	/* Clean-up */
 	igt_plane_set_fb(data->primary, NULL);
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [igt-dev] [PATCH v2 7/8] tests/kms_vrr: Allow for multiple rates in a test

[Modem, Bhanuprakash]

On Tue-24-10-2023 02:26 am, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Instead of passing just one rate into flip_and_measure, pass in an array
> to allow for tests which vary the refresh rate from one frame to the
> next. Currently the test will just cycle through the rates in order per
> frame.
> 
> Just framework, no changes to any tests.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

LGTM:
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

> 
> Changes in v2:
> - Added to the set
> ---
>   tests/kms_vrr.c | 43 ++++++++++++++++++++++++-------------------
>   1 file changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index 57bd87ddf..b986087f0 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -375,7 +375,7 @@ do_flip(data_t *data, igt_fb_t *fb)
>    */
>   static uint32_t
>   flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
> -		 uint64_t rate_ns, uint64_t duration_ns)
> +		 uint64_t *rates_ns, int num_rates, uint64_t duration_ns)
>   {
>   	uint64_t start_ns, last_event_ns, target_ns;
>   	uint32_t total_flip = 0, total_pass = 0;
> @@ -387,9 +387,10 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>   	start_ns = last_event_ns = target_ns = get_kernel_event_ns(data,
>   							DRM_EVENT_FLIP_COMPLETE);
>   
> -	for (;;) {
> +	for (int i = 0;;++i) {
>   		uint64_t event_ns, wait_ns;
>   		int64_t diff_ns;
> +		uint64_t rate_ns = rates_ns[i % num_rates];
>   
>   		front = !front;
>   		do_flip(data, front ? &data->fb[1] : &data->fb[0]);
> @@ -440,8 +441,12 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>   		while (get_time_ns() < target_ns - 10);
>   	}
>   
> -	igt_info("Completed %u flips, %u were in threshold for (%llu Hz) %"PRIu64"ns.\n",
> -		 total_flip, total_pass, (NSECS_PER_SEC/rate_ns), rate_ns);
> +	igt_info("Completed %u flips, %u were in threshold for [", total_flip, total_pass);
> +	for (int i = 0; i < num_rates; ++i) {
> +		igt_info("(%llu Hz) %"PRIu64"ns%s", (NSECS_PER_SEC/rates_ns[i]), rates_ns[i],
> +			 i < num_rates - 1 ? "," : "");
> +	}
> +	igt_info("]\n");
>   
>   	return total_flip ? ((total_pass * 100) / total_flip) : 0;
>   }
> @@ -453,12 +458,12 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   	uint32_t result;
>   	vtest_ns_t vtest_ns;
>   	range_t range;
> -	uint64_t rate;
> +	uint64_t rate[] = {0};
>   
>   	prepare_test(data, output, pipe);
>   	range = data->range;
>   	vtest_ns = data->vtest_ns;
> -	rate = vtest_ns.rate_ns;
> +	rate[0] = vtest_ns.rate_ns;
>   
>   	igt_info("VRR Test execution on %s, PIPE_%s with VRR range: (%u-%u) Hz\n",
>   		 output->name, kmstest_pipe_name(pipe), range.min, range.max);
> @@ -470,7 +475,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   	 * This is to make sure we were actually in the middle of
>   	 * active flipping before doing the DPMS/suspend steps.
>   	 */
> -	flip_and_measure(data, output, pipe, rate, 250000000ull);
> +	flip_and_measure(data, output, pipe, rate, 1, 250000000ull);
>   
>   	if (flags & TEST_DPMS) {
>   		kmstest_set_connector_dpms(output->display->drm_fd,
> @@ -501,27 +506,27 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   	 *      next Vmin.
>   	 */
>   	if (flags & TEST_FLIPLINE) {
> -		rate = rate_from_refresh(range.max + 5);
> -		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
> +		rate[0] = rate_from_refresh(range.max + 5);
> +		result = flip_and_measure(data, output, pipe, rate, 1, data->duration_ns);
>   		igt_assert_f(result > 75,
>   			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
> -			     (range.max + 5), rate, result);
> +			     (range.max + 5), rate[0], result);
>   	}
>   
>   	if (flags & ~TEST_NEGATIVE) {
> -		rate = vtest_ns.rate_ns;
> -		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
> +		rate[0] = vtest_ns.rate_ns;
> +		result = flip_and_measure(data, output, pipe, rate, 1, data->duration_ns);
>   		igt_assert_f(result > 75,
>   			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
> -			     ((range.max + range.min) / 2), rate, result);
> +			     ((range.max + range.min) / 2), rate[0], result);
>   	}
>   
>   	if (flags & TEST_FLIPLINE) {
> -		rate = rate_from_refresh(range.min - 5);
> -		result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
> +		rate[0] = rate_from_refresh(range.min - 5);
> +		result = flip_and_measure(data, output, pipe, rate, 1, data->duration_ns);
>   		igt_assert_f(result < 50,
>   			     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR on threshold exceeded, result was %u%%\n",
> -			     (range.min - 5), rate, result);
> +			     (range.min - 5), rate[0], result);
>   	}
>   
>   	/*
> @@ -530,11 +535,11 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   	 * a VRR capable panel.
>   	 */
>   	set_vrr_on_pipe(data, pipe, (flags & TEST_NEGATIVE)? true : false);
> -	rate = vtest_ns.rate_ns;
> -	result = flip_and_measure(data, output, pipe, rate, data->duration_ns);
> +	rate[0] = vtest_ns.rate_ns;
> +	result = flip_and_measure(data, output, pipe, rate, 1, data->duration_ns);
>   	igt_assert_f(result < 10,
>   		     "Refresh rate (%u Hz) %"PRIu64"ns: Target VRR %s threshold exceeded, result was %u%%\n",
> -		     ((range.max + range.min) / 2), rate, (flags & TEST_NEGATIVE)? "on" : "off", result);
> +		     ((range.max + range.min) / 2), rate[0], (flags & TEST_NEGATIVE)? "on" : "off", result);
>   
>   	/* Clean-up */
>   	igt_plane_set_fb(data->primary, NULL);

[igt-dev] [PATCH v2 8/8] tests/kms_vrr: Add a max/min test to oscillate between rates

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

The new MAXMIN test will oscillate between the max and min
refresh rates every frame to stress test the panel. This is
useful for manual profiling of flicker.

Signed-off-by: Sean Paul <seanpaul@chromium.org>

Changes in v2:
- Added to the set
---
 tests/kms_vrr.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
index b986087f0..58bed4f20 100644
--- a/tests/kms_vrr.c
+++ b/tests/kms_vrr.c
@@ -89,6 +89,7 @@ enum {
 	TEST_SUSPEND = 1 << 2,
 	TEST_FLIPLINE = 1 << 3,
 	TEST_NEGATIVE = 1 << 4,
+	TEST_MAXMIN = 1 << 6,
 };
 
 typedef struct range {
@@ -529,6 +530,14 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
 			     (range.min - 5), rate[0], result);
 	}
 
+	if (flags & TEST_MAXMIN) {
+		uint64_t maxmin_rates[] = {vtest_ns.max, vtest_ns.min};
+		result = flip_and_measure(data, output, pipe, maxmin_rates, 2, data->duration_ns);
+		igt_assert_f(result < 50,
+			     "Refresh rate (%u Hz) %"PRIu64"ns/%"PRIu64"ns: Target VRR on threshold exceeded, result was %u%%\n",
+			     (range.min - 5), maxmin_rates[0], maxmin_rates[1], result);
+	}
+
 	/*
 	 * If we request VRR on a non-VRR panel, it is unlikely to reject the
 	 * modeset. And the expected behavior is the same as disabling VRR on
@@ -654,6 +663,11 @@ igt_main_args("d:r:s", long_opts, help_str, opt_handler, &data)
 	igt_subtest_with_dynamic("negative-basic")
 		run_vrr_test(&data, test_basic, TEST_NEGATIVE);
 
+	igt_describe("Oscillates between highest and lowest refresh each frame for manual "
+		     "flicker profiling");
+	igt_subtest_with_dynamic("max-min")
+		run_vrr_test(&data, test_basic, TEST_MAXMIN);
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 		drm_close_driver(data.drm_fd);
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [igt-dev] [PATCH v2 8/8] tests/kms_vrr: Add a max/min test to oscillate between rates

[Modem, Bhanuprakash]

On Tue-24-10-2023 02:26 am, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> The new MAXMIN test will oscillate between the max and min
> refresh rates every frame to stress test the panel. This is
> useful for manual profiling of flicker.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> 
> Changes in v2:
> - Added to the set
> ---
>   tests/kms_vrr.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index b986087f0..58bed4f20 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -89,6 +89,7 @@ enum {
>   	TEST_SUSPEND = 1 << 2,
>   	TEST_FLIPLINE = 1 << 3,
>   	TEST_NEGATIVE = 1 << 4,
> +	TEST_MAXMIN = 1 << 6,
---------------------------^
Can't we use 5 here?

Also, I'll recommend always keep the TEST_NEGATIVE at the end.

>   };
>   
>   typedef struct range {
> @@ -529,6 +530,14 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   			     (range.min - 5), rate[0], result);
>   	}
>   
> +	if (flags & TEST_MAXMIN) {
> +		uint64_t maxmin_rates[] = {vtest_ns.max, vtest_ns.min};
> +		result = flip_and_measure(data, output, pipe, maxmin_rates, 2, data->duration_ns);
> +		igt_assert_f(result < 50,
> +			     "Refresh rate (%u Hz) %"PRIu64"ns/%"PRIu64"ns: Target VRR on threshold exceeded, result was %u%%\n",
> +			     (range.min - 5), maxmin_rates[0], maxmin_rates[1], result);
> +	}
> +
>   	/*
>   	 * If we request VRR on a non-VRR panel, it is unlikely to reject the
>   	 * modeset. And the expected behavior is the same as disabling VRR on
> @@ -654,6 +663,11 @@ igt_main_args("d:r:s", long_opts, help_str, opt_handler, &data)
>   	igt_subtest_with_dynamic("negative-basic")
>   		run_vrr_test(&data, test_basic, TEST_NEGATIVE);
>   
> +	igt_describe("Oscillates between highest and lowest refresh each frame for manual "
> +		     "flicker profiling");
> +	igt_subtest_with_dynamic("max-min")
> +		run_vrr_test(&data, test_basic, TEST_MAXMIN);

Testplan documentation is missing for this new test. Please check 
docs/test_documentation.md[1] for more details.

[1]: 
https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/docs/test_documentation.md 


Probably, we need a change as below:

+ * SUBTEST: max-min
+ * Description: Oscillates between highest and lowest refresh each 
frame for manual flicker profiling.
+ * Driver requirement: i915, xe
+ * Functionality: adaptive_sync
+ * Mega feature: VRR
+ * Test category: functionality test


- Bhanu

> +
>   	igt_fixture {
>   		igt_display_fini(&data.display);
>   		drm_close_driver(data.drm_fd);

[igt-dev] ✗ Fi.CI.BUILD: failure for tests/kms_vrr: Modify kms_vrr to allow flicker profiling

[Patchwork]

== Series Details ==

Series: tests/kms_vrr: Modify kms_vrr to allow flicker profiling
URL   : https://patchwork.freedesktop.org/series/125479/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
557856802dfee103802f1157f97c65bb476d5468 lib/igt_kmod: Remove wrong note regarding dependencies

Tail of build.log:
[1599/1645] Linking target tools/intel_infoframes.
[1600/1645] Linking target tools/intel_reg.
[1601/1645] Linking target tools/intel_dp_compliance.
[1602/1645] Linking target tools/intel_residency.
[1603/1645] Linking target tools/intel_gvtg_test.
[1604/1645] Linking target runner/testdata/successtest.
[1605/1645] Linking target runner/testdata/no-subtests.
[1606/1645] Linking target runner/testdata/abort-dynamic.
[1607/1645] Linking target tools/msm_dp_compliance.
[1608/1645] Linking target tools/amd_hdmi_compliance.
[1609/1645] Linking target runner/testdata/abort-fixture.
[1610/1645] Generating gem_stress.testlist with a meson_exe.py custom command.
[1611/1645] Linking target tools/lsgpu.
[1612/1645] Linking target runner/testdata/dynamic.
[1613/1645] Linking target tools/intel_l3_parity.
[1614/1645] Linking target runner/testdata/abort-simple.
[1615/1645] Linking target runner/igt_runner.
[1616/1645] Linking target runner/igt_comms_decoder.
[1617/1645] Linking target runner/testdata/skippers.
[1618/1645] Linking target runner/testdata/abort.
[1619/1645] Linking target runner/runner_json_test.
[1620/1645] Linking target runner/igt_resume.
[1621/1645] Linking target runner/igt_results.
[1622/1645] Compiling C object 'lib/76b5a35@@i915_perf@sha/meson-generated_.._i915_perf_metrics_mtlgt3.c.o'.
[1623/1645] Compiling C object 'lib/76b5a35@@i915_perf@sha/meson-generated_.._i915_perf_metrics_acmgt1.c.o'.
[1624/1645] Compiling C object 'runner/527aa9f@@runner_test@exe/runner_tests.c.o'.
[1625/1645] Linking target runner/runner_test.
[1626/1645] Compiling C object 'lib/76b5a35@@i915_perf@sha/meson-generated_.._i915_perf_metrics_acmgt2.c.o'.
[1627/1645] Compiling C object 'lib/76b5a35@@i915_perf@sha/meson-generated_.._i915_perf_metrics_acmgt3.c.o'.
[1628/1645] Linking target lib/libi915_perf.so.1.5.
[1629/1645] Generating symbol file 'lib/76b5a35@@i915_perf@sha/libi915_perf.so.1.5.symbols'.
[1630/1645] Linking target tools/i915-perf/i915-perf-configs.
[1631/1645] Linking target tools/i915-perf/i915-perf-recorder.
[1632/1645] Linking target tools/i915-perf/i915-perf-reader.
[1633/1645] Linking target tests/core_hotunplug.
[1634/1645] Linking target tests/gem_barrier_race.
[1635/1645] Linking target tests/perf.
[1636/1645] Generating gem_barrier_race.testlist with a meson_exe.py custom command.
[1637/1645] Generating perf.testlist with a meson_exe.py custom command.
[1638/1645] Generating core_hotunplug.testlist with a meson_exe.py custom command.
[1639/1645] Generating xe_tests.rst with a custom command.
[1640/1645] Generating i915_tests.rst with a custom command.
[1641/1645] Generating kms_tests.rst with a custom command.
FAILED: docs/testplan/kms_tests.rst 
/usr/src/igt-gpu-tools/scripts/igt_doc.py --config /usr/src/igt-gpu-tools/tests/intel/kms_test_config.json --rest docs/testplan/kms_tests.rst --check-testlist --igt-build-path /opt/igt/build
Warning: Missing documentation for igt@kms_vrr@max-min
Please refer: docs/test_documentation.md for more details
[1642/1645] Generating xe_tests.html with a custom command.
[1643/1645] Generating i915_tests.html with a custom command.
ninja: build stopped: subcommand failed.

[igt-dev] ✗ GitLab.Pipeline: warning for tests/kms_vrr: Modify kms_vrr to allow flicker profiling

[Patchwork]

== Series Details ==

Series: tests/kms_vrr: Modify kms_vrr to allow flicker profiling
URL   : https://patchwork.freedesktop.org/series/125479/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/1017374 for the overview.

build-containers:build-debian-mips has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/50673661):
          "intel-IGTPW_3412",
          "intel-IGTPW_3413"
      ],
      "Created": "2021-01-22T08:26:53.390348642Z",
      "DockerVersion": "",
      "Labels": {},
      "Architecture": "amd64",
      "Os": "linux",
      "Layers": [
          "sha256:04854e5e3796b945522a085073b5a49cadad51bd1c5414c91654fa3d63d8db4d"
      ]
  }
  Skipping, already built
  time="2023-10-24T07:44:36Z" level=fatal msg="Error initializing source docker://registry.freedesktop.org/gfx-ci/igt-ci-tags/build-debian-mips:dockerfile-aae5ff3c8b65e55cbc8e8a0c565007f61db3126d: unexpected http code: 500 (Internal Server Error), URL: https://gitlab.freedesktop.org/jwt/auth?account=gitlab-ci-token&scope=repository%3Agfx-ci%2Figt-ci-tags%2Fbuild-debian-mips%3Apull&service=container_registry" 
  section_end:1698133477:step_script
  section_start:1698133477:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1698133478:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/1017374

Re: [igt-dev] [PATCH v2 0/8] tests/kms_vrr: Modify kms_vrr to allow flicker profiling

[Sean Paul]

[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]

Friendly review ping.

On Mon, Oct 23, 2023 at 4:59 PM Sean Paul <sean@poorly.run> wrote:

> From: Sean Paul <seanpaul@chromium.org>
>
> Second revision of the set. New version addresses the review comments
> from Bhanu and adds a new maxmin subtest which alternates the rate
> between max and min every frame.
>
> Sean Paul (8):
>   tests/kms_vrr: Move fb0 and fb1 to an array
>   tests/kms_vrr: Move vtest_ns into data_t
>   tests/kms_vrr: Allow test rate to be altered from the command line
>   tests/kms_vrr: Allow test duration to be specified from the command
>     line
>   tests/kms_vrr: Change the pattern displayed in the test
>   tests/kms_vrr: Add ability to flip static image for flicker profiling
>   tests/kms_vrr: Allow for multiple rates in a test
>   tests/kms_vrr: Add a max/min test to oscillate between rates
>
>  tests/kms_vrr.c | 213 +++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 156 insertions(+), 57 deletions(-)
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
>
>

[-- Attachment #2: Type: text/html, Size: 1414 bytes --]

Re: [PATCH v2 0/8] tests/kms_vrr: Modify kms_vrr to allow flicker profiling

[Modem, Bhanuprakash]

Hello Sean Paul,

Are you planning to send a new rev, as we have few review comments?

FYI, I have merged few changes recently [*], you need to rebase this series.

[*]: https://patchwork.freedesktop.org/series/127047/

- Bhanu

On 14-11-2023 09:49 pm, Sean Paul wrote:
> Friendly review ping.
> 
> On Mon, Oct 23, 2023 at 4:59 PM Sean Paul <sean@poorly.run> wrote:
> 
>     From: Sean Paul <seanpaul@chromium.org <mailto:seanpaul@chromium.org>>
> 
>     Second revision of the set. New version addresses the review comments
>     from Bhanu and adds a new maxmin subtest which alternates the rate
>     between max and min every frame.
> 
>     Sean Paul (8):
>        tests/kms_vrr: Move fb0 and fb1 to an array
>        tests/kms_vrr: Move vtest_ns into data_t
>        tests/kms_vrr: Allow test rate to be altered from the command line
>        tests/kms_vrr: Allow test duration to be specified from the command
>          line
>        tests/kms_vrr: Change the pattern displayed in the test
>        tests/kms_vrr: Add ability to flip static image for flicker profiling
>        tests/kms_vrr: Allow for multiple rates in a test
>        tests/kms_vrr: Add a max/min test to oscillate between rates
> 
>       tests/kms_vrr.c | 213 +++++++++++++++++++++++++++++++++++-------------
>       1 file changed, 156 insertions(+), 57 deletions(-)
> 
>     -- 
>     Sean Paul, Software Engineer, Google / Chromium OS
>