[PATCH] tests/kms_vrr: Update LOBF test to resuse the existing logic

Reddy Guddati, Santhosh santhosh.reddy.guddati at intel.com
Thu Sep 19 08:07:41 UTC 2024


1. Fix Typo in subject "reuse".
2. Remove FIXME: test_lobf comments

Rest LGTM.
Reviewed by : Santhosh Reddy Guddati <santhosh.reddy.guddati at intel.com>


On 13-09-2024 12:26, Jeevan B wrote:
> Removed the test_lobf function and used test_basic instead to simplify
> the code. Improved validation and error messages to make LOBF testing
> more reliable.
> 
> Signed-off-by: Jeevan B <jeevan.b at intel.com>
> ---
>   tests/kms_vrr.c | 45 ++++++++++++++++++---------------------------
>   1 file changed, 18 insertions(+), 27 deletions(-)
> 
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index 36a22eebe..5d9b321d3 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -640,7 +640,7 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   			     (range.max + 5), rate[0], result);
>   	}
>   
> -	if (flags & ~(TEST_NEGATIVE | TEST_MAXMIN)) {
> +	if (flags & ~(TEST_NEGATIVE | TEST_MAXMIN | TEST_LINK_OFF)) {
>   		rate[0] = vtest_ns.rate_ns;
>   		result = flip_and_measure(data, output, pipe, rate, 1, data->duration_ns);
>   		igt_assert_f(result > 75,
> @@ -656,16 +656,24 @@ test_basic(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   			     (range.min - 10), rate[0], result);
>   	}
>   
> -	if (flags & TEST_MAXMIN) {
> +	if (flags & (TEST_MAXMIN | TEST_LINK_OFF)) {
>   		unsigned int range_min =
>   			/* For Intel h/w tweak the min rate, as h/w will terminate the vblank at Vmax. */
>   			is_intel_device(data->drm_fd) ? (range.min + 2) : range.min;
>   		uint64_t maxmin_rates[] = {vtest_ns.max, rate_from_refresh(range_min)};
>   
>   		result = flip_and_measure(data, output, pipe, maxmin_rates, 2, data->duration_ns);
> -		igt_assert_f(result > 75,
> -			     "Refresh rates (%u/%u Hz) %"PRIu64"ns/%"PRIu64"ns: Target VRR on threshold not reached, result was %u%%\n",
> -			     range.max, range_min, maxmin_rates[0], maxmin_rates[1], result);
> +
> +		if (flags & TEST_LINK_OFF)
> +			igt_assert_f(igt_get_i915_edp_lobf_status(data->drm_fd, output->name),
> +				     "LOBF not enabled\n");
> +		else
> +			igt_assert_f(result > 75,
> +				     "Refresh rates (%u/%u Hz) %"PRIu64"ns/%"PRIu64"ns: Target "
> +				     "VRR on threshold not reached, result was %u%%\n",
> +				     range.max, range_min, maxmin_rates[0], maxmin_rates[1],
> +				     result);
> +
>   		return;
>   	}
>   
> @@ -796,27 +804,6 @@ test_seamless_virtual_rr_basic(data_t *data, enum pipe pipe, igt_output_t *outpu
>    * (Set Context Latency + Guardband) > (First SDP Position + Wake Time)
>    * one the depends patches are merged please fix this function.
>    */
> -  

  >>> Also Remove FIXME comments from here
  */
> -static void
> -test_lobf(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
> -{
> -	uint64_t rate[] = {0};
> -
> -	rate[0] = rate_from_refresh(data->switch_modes[HIGH_RR_MODE].vrefresh);
> -	prepare_test(data, output, pipe);
> -
> -	igt_info("LOBF test execution on %s, PIPE %s with VRR range: (%u-%u) Hz\n",
> -		 output->name, kmstest_pipe_name(pipe), data->range.min, data->range.max);
> -
> -	igt_output_override_mode(output, &data->switch_modes[HIGH_RR_MODE]);
> -	flip_and_measure(data, output, pipe, rate, 1, TEST_DURATION_NS);
> -	igt_output_override_mode(output, &data->switch_modes[LOW_RR_MODE]);
> -	rate[0] = rate_from_refresh(data->switch_modes[LOW_RR_MODE].vrefresh);
> -	flip_and_measure(data, output, pipe, rate, 1, NSECS_PER_SEC);
> -	igt_assert_f(igt_get_i915_edp_lobf_status(data->drm_fd, output->name),
> -		     "LOBF not enabled\n");
> -}
> -
>   static void
>   test_cmrr(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
>   {
> @@ -1098,12 +1085,16 @@ igt_main_args("drs:", long_opts, help_str, opt_handler, &data)
>   			run_vrr_test(&data, test_cmrr, TEST_CMRR);
>   		}
>   
> +		/* FIXME: test_lobf : Lobf can be enabled when
> +		 * (Set Context Latency + Guardband) > (First SDP Position + Wake Time)
> +		 * one the depends patches are merged please fix this function.
> +		 */
>   		igt_describe("Test to validate the link-off between active frames in "
>   			     "non-PSR operation.");
>   		igt_subtest_with_dynamic("lobf") {
>   			igt_require(intel_display_ver(intel_get_drm_devid(data.drm_fd)) >= 20);
>   
> -			run_vrr_test(&data, test_lobf, TEST_LINK_OFF);
> +			run_vrr_test(&data, test_basic, TEST_LINK_OFF);
>   		}
>   	}
>   


More information about the igt-dev mailing list