Re: [i-g-t,2/2] tests/kms_vrr: Add new test to validate LOBF

["Joshi, Kunal1" <kunal1.joshi@intel.com>]

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

Hello Jeevan,

Please find my comment inline

On 7/22/2024 1:20 PM, Jeevan B wrote:
> Validate pseudo static refresh rate changes in the VRR with
> fixed refresh rate framework for non-psr scenarios.
>
> Signed-off-by: Jeevan B<jeevan.b@intel.com>
> ---
>   tests/kms_vrr.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_vrr.c b/tests/kms_vrr.c
> index 7e8885f16..a79819303 100644
> --- a/tests/kms_vrr.c
> +++ b/tests/kms_vrr.c
> @@ -31,12 +31,18 @@
>    */
>   
>   #include "igt.h"
> +#include "igt_psr.h"
>   #include "i915/intel_drrs.h"
>   #include "sw_sync.h"
>   #include <fcntl.h>
>   #include <signal.h>
>   
>   /**
> + * SUBTEST: lobf
> + * Description: Test to validate link-off between active frames in non-psr
> + *              operation
> + * Functionality: LOBF
> + *
>    * SUBTEST: cmrr
>    * Description: Test to validate the content rate to exactly match with the
>    * 		requested rate without any frame drops.
> @@ -106,7 +112,8 @@ enum {
>   	TEST_FASTSET = 1 << 7,
>   	TEST_MAXMIN = 1 << 8,
>   	TEST_CMRR = 1 << 9,
> -	TEST_NEGATIVE = 1 << 10,
> +	TEST_LINK_OFF = 1 << 10,
> +	TEST_NEGATIVE = 1 << 11,
>   };
>   
>   enum {
> @@ -129,6 +136,7 @@ typedef struct vtest_ns {
>   typedef struct data {
>   	igt_display_t display;
>   	int drm_fd;
> +	int debugfs_fd;
>   	igt_plane_t *primary;
>   	igt_fb_t fb[2];
>   	range_t range;
> @@ -784,6 +792,43 @@ test_seamless_virtual_rr_basic(data_t *data, enum pipe pipe, igt_output_t *outpu
>   	}
>   }
>   
> +static void
> +test_lobf(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t flags)
> +{
> +	bool change_mode = false;
> +	time_t startTime = time(NULL);
> +	time_t secs = 4, change_time = 3;
> +	igt_plane_t *primary;
> +	drmModeModeInfo mode = *igt_output_get_mode(output);
> +
> +	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_require(output->config.connector->connector_type == DRM_MODE_CONNECTOR_eDP);
> +	igt_require(!igt_get_i915_edp_lobf_status(data->drm_fd, output->name));
> +	if (psr_sink_support(data->drm_fd, data->debugfs_fd, PSR_MODE_1, NULL))
How about PSR2, PR ...?

> +		psr_disable(data->drm_fd, data->debugfs_fd, NULL);
> +
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +	igt_create_color_fb(data->drm_fd, mode.hdisplay, mode.vdisplay,
> +			    DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> +			    1, 1, 1, &data->fb[0]);
> +
> +	igt_output_override_mode(output, &data->switch_modes[HIGH_RR_MODE]);
> +
> +	while (time(NULL) - startTime < secs) {
> +		igt_plane_set_fb(primary, &data->fb[0]);
> +		igt_display_commit(&data->display);
> +		if (!change_mode && time(NULL) - startTime >= change_time) {
> +			igt_output_override_mode(output, &data->switch_modes[LOW_RR_MODE]);
> +			change_time = true;
> +		}
> +	}
Can we add some kind of check that will tell if link off is possible if 
we switch from HIGH_RR_MODE to LOW_RR_MODE ?
I mean if we can somehow know what extended vblank duration is needed in 
order to off the link and does the two modes
will be able to have such extended vblank?
> +
> +	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)
>   {
> @@ -973,6 +1018,7 @@ igt_main_args("drs:", long_opts, help_str, opt_handler, &data)
>   {
>   	igt_fixture {
>   		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
> +		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
>   
>   		kmstest_set_vt_graphics_mode();
>   
> @@ -1030,13 +1076,20 @@ igt_main_args("drs:", long_opts, help_str, opt_handler, &data)
>   		igt_subtest_with_dynamic("seamless-rr-switch-virtual")
>   			run_vrr_test(&data, test_seamless_virtual_rr_basic, TEST_SEAMLESS_VIRTUAL_RR);
>   
> -		igt_describe("Test to validate the the content rate exactly match with the "
> +		igt_describe("Test to validate the content rate exactly match with the "
>   			     "requested rate without any frame drops.");
>   		igt_subtest_with_dynamic("cmrr") {
>   			igt_require(intel_display_ver(intel_get_drm_devid(data.drm_fd)) >= 20);
>   
>   			run_vrr_test(&data, test_cmrr, TEST_CMRR);
>   		}
> +		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);
> +		}
>   	}
>   
>   	igt_fixture {

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