[igt-dev] [PATCH v3 2/2] tests/kms_vrr: Add new subtest to validate Flipline mode

Kazlauskas, Nicholas nicholas.kazlauskas at amd.com
Wed Sep 16 19:44:17 UTC 2020


On 2020-08-20 12:42 p.m., Bhanuprakash Modem wrote:
> Check flipline mode by making sure that flips happen at flipline
> decision boundary.
> 
> Example: if monitor vrr range is 40 - 60Hz and
> 
> * flip at refresh_rate > 60Hz:
>          Flip should happen at the flipline boundary & returned refresh rate
>          would be 60Hz.
> * flip at refresh_rate == 50Hz:
>          Flip should happen right away so returned refresh rate is 50Hz.
> * flip at refresh_rate < 40Hz:
>          Flip should happen at the vmax so the returned refresh rate
>          would be 40Hz.

Apologies for the delay for getting around to looking at this.

The first two patches in this series LGTM and pass on our hardware, so 
with Manasi's comments addressed those two are:

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>

However, I do have some comments about this last patch. On AMDGPU the 
behavior for rates below the range differs.

If refresh_rate < 40Hz on a 40 - 60Hz display we will use vmax, 60Hz. 
IIRC this produces a result with noticeably less stuttering.

For refresh_rate < 40Hz on a display that has vmin < 2 * vmax we'll pick 
a multiple of refresh_rate within vmin to vmax.

The behavior is left to the driver from the DRM API so the approach 
suggested along with the one currently in our driver is allowed:

"
The minimum vertical front porch duration is defined as the vertical 
front porch duration for the current mode.

The maximum vertical front porch duration is greater than or equal to 
the minimum vertical front porch duration. The duration is derived from 
the minimum supported variable refresh rate for the connector.

The driver may place further restrictions within these minimum and 
maximum bounds.
"

Regards,
Nicholas Kazlauskas

> 
> Cc: Harry Wentland <harry.wentland at amd.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> Cc: Manasi Navare <manasi.d.navare at intel.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> ---
>   tests/kms_vrr.c | 101 ++++++++++++++++++++++++++++++++++++------------
>   1 file changed, 76 insertions(+), 25 deletions(-)
> 
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index 15db7695e8..2a11fe6afb 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -37,6 +37,7 @@ enum {
>   	TEST_NONE = 0,
>   	TEST_DPMS = 1 << 0,
>   	TEST_SUSPEND = 1 << 1,
> +	TEST_FLIPLINE = 1 << 2,
>   };
>   
>   typedef struct range {
> @@ -52,6 +53,12 @@ typedef struct data {
>   	igt_fb_t fb1;
>   } 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. */
> @@ -104,13 +111,18 @@ static uint64_t rate_from_refresh(uint64_t refresh)
>   	return NSECS_PER_SEC / refresh;
>   }
>   
> -/* Returns the min and max vrr range from the connector debugfs. */
> +/* Read min and max vrr range from the connector debugfs.
> + *  - min range should be less than the current mode vfreq
> + *  - if max range is grater than the current mode vfreq, consider
> + *       current mode vfreq as the max range.
> + */
>   static range_t get_vrr_range(data_t *data, igt_output_t *output)
>   {
>   	char buf[256];
>   	char *start_loc;
>   	int fd, res;
>   	range_t range;
> +	drmModeModeInfo *mode = igt_output_get_mode(output);
>   
>   	fd = igt_debugfs_connector_dir(data->drm_fd, output->name, O_RDONLY);
>   	igt_assert(fd >= 0);
> @@ -122,32 +134,28 @@ static range_t get_vrr_range(data_t *data, igt_output_t *output)
>   
>   	igt_assert(start_loc = strstr(buf, "Min: "));
>   	igt_assert_eq(sscanf(start_loc, "Min: %u", &range.min), 1);
> +	igt_require(mode->vrefresh > range.min);
>   
>   	igt_assert(start_loc = strstr(buf, "Max: "));
>   	igt_assert_eq(sscanf(start_loc, "Max: %u", &range.max), 1);
>   
> +	range.max = (mode->vrefresh < range.max) ? mode->vrefresh : range.max;
> +
>   	return range;
>   }
>   
> -/* Returns a suitable vrr test frequency. */
> -static uint64_t get_test_rate_ns(data_t *data, igt_output_t *output)
> +/* Returns vrr test frequency for min, mid & max range. */
> +static vtest_ns_t get_test_rate_ns(data_t *data, igt_output_t *output)
>   {
> -	drmModeModeInfo *mode = igt_output_get_mode(output);
>   	range_t range;
> -	uint64_t vtest;
> +	vtest_ns_t vtest_ns;
>   
> -	/*
> -	 * The frequency with the fastest convergence speed should be
> -	 * the midpoint between the current mode vfreq and the min
> -	 * supported vfreq.
> -	 */
>   	range = get_vrr_range(data, output);
> -	igt_require(mode->vrefresh > range.min);
> +	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);
>   
> -	vtest = (mode->vrefresh - range.min) / 2 + range.min;
> -	igt_require(vtest < mode->vrefresh);
> -
> -	return rate_from_refresh(vtest);
> +	return vtest_ns;
>   }
>   
>   /* Returns true if an output supports VRR. */
> @@ -250,6 +258,7 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>   	uint64_t start_ns, last_vblank_ns;
>   	uint32_t total_flip = 0, total_pass = 0;
>   	bool front = false;
> +	vtest_ns_t vtest_ns = get_test_rate_ns(data, output);
>   
>   	/* Align with the vblank region to speed up convergence. */
>   	last_vblank_ns = wait_for_vblank(data, pipe);
> @@ -268,10 +277,6 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>   		 * that same frame.
>   		 */
>   		vblank_ns = get_kernel_event_ns(data, DRM_EVENT_FLIP_COMPLETE);
> -		diff_ns = rate_ns - (vblank_ns - last_vblank_ns);
> -		last_vblank_ns = vblank_ns;
> -
> -		total_flip += 1;
>   
>   		/*
>   		 * Check if the difference between the two flip timestamps
> @@ -281,9 +286,19 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>   		 * difference between 144Hz and 143Hz which should give this
>   		 * enough accuracy for most use cases.
>   		 */
> +		if ((rate_ns <= vtest_ns.min) && (rate_ns >= vtest_ns.max))
> +			diff_ns = rate_ns - (vblank_ns - last_vblank_ns);
> +		else if (rate_ns > vtest_ns.min)
> +			diff_ns = vtest_ns.min - (vblank_ns - last_vblank_ns);
> +		else if (rate_ns < vtest_ns.max)
> +			diff_ns = vtest_ns.max - (vblank_ns - last_vblank_ns);
> +
>   		if (llabs(diff_ns) < 50000ll)
>   			total_pass += 1;
>   
> +		last_vblank_ns = vblank_ns;
> +		total_flip += 1;
> +
>   		now_ns = get_time_ns();
>   		if (now_ns - start_ns > duration_ns)
>   			break;
> @@ -310,10 +325,13 @@ flip_and_measure(data_t *data, igt_output_t *output, enum pipe pipe,
>   static void
>   test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   {
> -	uint64_t rate;
>   	uint32_t result;
> +	vtest_ns_t vtest_ns = get_test_rate_ns(data, output);
> +	range_t range = get_vrr_range(data, output);
> +	uint64_t rate = vtest_ns.mid;
>   
> -	rate = get_test_rate_ns(data, output);
> +	igt_info("VRR Test execution on %s, PIPE_%s\n",
> +		 output->name, kmstest_pipe_name(pipe));
>   
>   	prepare_test(data, output, pipe);
>   
> @@ -339,6 +357,35 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>   					      SUSPEND_TEST_NONE);
>   
> +	/*
> +	 * Check flipline mode by making sure that flips happen at flipline
> +	 * decision boundary.
> +	 *
> +	 * Example: if range is 40 - 60Hz and
> +	 * if refresh_rate > 60Hz:
> +	 *      Flip should happen at the flipline boundary & returned refresh rate
> +	 *      would be 60Hz.
> +	 * if refresh_rate is 50Hz:
> +	 *      Flip will happen right away so returned refresh rate is 50Hz.
> +	 * if refresh_rate < 40Hz:
> +	 *      Flip should happen at the vmax so the returned refresh rate
> +	 *      would be 40Hz.
> +	 */
> +	if (flags & TEST_FLIPLINE) {
> +		rate = rate_from_refresh(range.min - 5);
> +		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
> +		igt_assert_f(result > 75,
> +			     "Refresh rate %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
> +			     rate, result);
> +
> +		rate = rate_from_refresh(range.max + 5);
> +		result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
> +		igt_assert_f(result > 75,
> +			     "Refresh rate %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
> +			     rate, result);
> +	}
> +
> +	rate = vtest_ns.mid;
>   	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
>   
>   	set_vrr_on_pipe(data, pipe, 0);
> @@ -346,14 +393,14 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   	/* This check is delayed until after VRR is disabled so it isn't
>   	 * left enabled if the test fails. */
>   	igt_assert_f(result > 75,
> -		     "Target VRR on threshold not reached, result was %u%%\n",
> -		     result);
> +		     "Refresh rate %"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
> +		     rate, result);
>   
>   	result = flip_and_measure(data, output, pipe, rate, TEST_DURATION_NS);
>   
>   	igt_assert_f(result < 10,
> -		     "Target VRR off threshold exceeded, result was %u%%\n",
> -		     result);
> +		     "Refresh rate %"PRIu64"ns: Target VRR off threshold exceeded, result was %u%%\n",
> +		     rate, result);
>   
>   	/* Clean-up */
>   	igt_plane_set_fb(data->primary, NULL);
> @@ -413,6 +460,10 @@ igt_main
>   	igt_subtest("flip-suspend")
>   		run_vrr_test(&data, test_basic, TEST_SUSPEND);
>   
> +	igt_describe("Make sure that flips happen at flipline decision boundary.");
> +	igt_subtest("flipline")
> +		run_vrr_test(&data, test_basic, TEST_FLIPLINE);
> +
>   	igt_fixture {
>   		igt_display_fini(&data.display);
>   	}
> 



More information about the igt-dev mailing list