Re: [PATCH i-g-t v2] tests/intel/kms_pm_dc: Remove PKC10 dependecy to validate DC6

[Imre Deak <imre.deak@intel.com>]

On Thu, Mar 13, 2025 at 02:34:36AM +0530, Mohammed Thasleem wrote:
> DC6 counter was removed MTL onwards, so pkc10 counter is used
> to validated DC6 tests but with the new approach of kmd changes
> created DC6 debugfs entry counter to validated DC6 tests and
> removeing the existing pkc10 code.
> 
> v2: Update debugfs message.
> 
> Signed-off-by: Mohammed Thasleem <mohammed.thasleem@intel.com>
> ---
>  tests/intel/kms_pm_dc.c | 78 ++++++++++-------------------------------
>  1 file changed, 18 insertions(+), 60 deletions(-)
> 
> diff --git a/tests/intel/kms_pm_dc.c b/tests/intel/kms_pm_dc.c
> index 1b10c7959..658ba0564 100644
> --- a/tests/intel/kms_pm_dc.c
> +++ b/tests/intel/kms_pm_dc.c
> @@ -274,6 +274,7 @@ static uint32_t read_dc_counter(uint32_t debugfs_fd, int dc_flag)
>  {
>  	char buf[4096];
>  	char *str;
> +	data_t data = {};
>  
>  	igt_debugfs_simple_read(debugfs_fd, "i915_dmc_info", buf, sizeof(buf));
>  
> @@ -281,8 +282,14 @@ static uint32_t read_dc_counter(uint32_t debugfs_fd, int dc_flag)
>  		str = strstr(buf, "DC3 -> DC5 count");
>  		igt_assert_f(str, "DC5 counter is not available\n");
>  	} else if (dc_flag & CHECK_DC6) {
> -		str = strstr(buf, "DC5 -> DC6 count");
> -		igt_assert_f(str, "DC6 counter is not available\n");
> +		if (intel_display_ver(data.devid) >= 14) {
> +			str = strstr(buf, "DC5 -> DC6 allowed count");
> +			igt_assert_f(str, "DC5 -> DC6 allowed count "
> +					   "is not available\n");
> +		} else {
> +			str = strstr(buf, "DC5 -> DC6 count");
> +			igt_assert_f(str, "DC6 counter is not available\n");
> +		}

Could the above not depend on the display device version? It could look
first for the real DC6 counter, fallback to the DC6-allowed counter and
fail if neither is found.

>  	} else if (dc_flag & CHECK_DC3CO) {
>  		str = strstr(buf, "DC3CO count");
>  		igt_assert_f(str, "DC3CO counter is not available\n");
> @@ -374,6 +381,7 @@ static void check_dc3co_with_videoplayback_like_load(data_t *data)
>  static void require_dc_counter(int debugfs_fd, int dc_flag)
>  {
>  	char buf[4096];
> +	data_t data = {};
>  
>  	igt_debugfs_simple_read(debugfs_fd, "i915_dmc_info",
>  				buf, sizeof(buf));
> @@ -388,8 +396,12 @@ static void require_dc_counter(int debugfs_fd, int dc_flag)
>  			      "DC5 counter is not available\n");
>  		break;
>  	case CHECK_DC6:
> -		igt_skip_on_f(!strstr(buf, "DC5 -> DC6 count"),
> -			      "DC6 counter is not available\n");
> +		if (intel_display_ver(data.devid) >= 14)
> +			igt_skip_on_f(!strstr(buf, "DC5 -> DC6 allowed count"),
> +				      "DC5 -> DC6 allowed count is not available\n");
> +		else
> +			igt_skip_on_f(!strstr(buf, "DC5 -> DC6 count"),
> +				      "DC6 counter is not available\n");

The same as above.

>  		break;
>  	default:
>  		igt_assert_f(0, "Unknown DC counter %d\n", dc_flag);
> @@ -414,20 +426,6 @@ static void test_dc3co_vpb_simulation(data_t *data)
>  	cleanup_dc3co_fbs(data);
>  }
>  
> -static void psr_dpms(data_t *data, int mode)
> -{
> -	igt_output_t *output;
> -
> -	for_each_connected_output(&data->display, output) {
> -		drmModeConnectorPtr connector = output->config.connector;
> -
> -		if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> -			continue;
> -
> -		kmstest_set_connector_dpms(data->drm_fd, connector, mode);
> -	}
> -}
> -
>  static void test_dc5_retention_flops(data_t *data, int dc_flag)
>  {
>  	uint32_t dc_counter_before_psr;
> @@ -718,40 +716,6 @@ static void test_deep_pkgc_state(data_t *data)
>  	igt_assert_f(pkgc_flag, "PKGC10 is not achieved.\n");
>  }
>  
> -static void test_pkgc_state_dpms(data_t *data)
> -{
> -	unsigned int timeout_sec = 6;
> -	unsigned int prev_value = 0, cur_value = 0;
> -
> -	prev_value = read_pkgc_counter(data->debugfs_root_fd);
> -	setup_dc_dpms(data);
> -	dpms_off(data);
> -	igt_wait((cur_value = read_pkgc_counter(data->debugfs_root_fd)) > prev_value,
> -		  timeout_sec * 1000, 100);
> -	igt_assert_f(cur_value > prev_value, "PKGC10 is not achieved.\n");
> -	dpms_on(data);
> -	cleanup_dc_dpms(data);
> -}
> -
> -static void test_pkgc_state_psr(data_t *data)
> -{
> -	unsigned int timeout_sec = 6;
> -	unsigned int prev_value = 0, cur_value = 0;
> -
> -	prev_value = read_pkgc_counter(data->debugfs_root_fd);
> -	setup_output(data);
> -	setup_primary(data);
> -	igt_require(!psr_disabled_check(data->debugfs_fd));
> -	igt_assert(psr_wait_entry(data->debugfs_fd, data->op_psr_mode, NULL));
> -	psr_dpms(data, DRM_MODE_DPMS_OFF);
> -	igt_wait((cur_value = read_pkgc_counter(data->debugfs_root_fd)) > prev_value,
> -		  timeout_sec * 1000, 100);
> -	igt_assert_f(cur_value > prev_value, "PKGC10 is not achieved.\n");
> -	psr_sink_error_check(data->debugfs_fd, data->op_psr_mode, data->output);
> -	psr_dpms(data, DRM_MODE_DPMS_ON);
> -	cleanup_dc_psr(data);
> -}
> -
>  static void kms_poll_state_restore(int sig)
>  {
>  	int sysfs_fd;
> @@ -815,10 +779,7 @@ igt_main
>  		psr_enable(data.drm_fd, data.debugfs_fd, data.op_psr_mode, NULL);
>  		igt_require_f(igt_pm_pc8_plus_residencies_enabled(data.msr_fd),
>  			      "PC8+ residencies not supported\n");
> -		if (intel_display_ver(data.devid) >= 14)
> -			test_pkgc_state_psr(&data);
> -		else
> -			test_dc_state_psr(&data, CHECK_DC6);
> +		test_dc_state_psr(&data, CHECK_DC6);
>  	}
>  
>  	igt_describe("This test validates display engine entry to PKGC10 state "
> @@ -863,10 +824,7 @@ igt_main
>  	igt_subtest("dc6-dpms") {
>  		igt_require_f(igt_pm_pc8_plus_residencies_enabled(data.msr_fd),
>  			      "PC8+ residencies not supported\n");
> -		if (intel_display_ver(data.devid) >= 14)
> -			test_pkgc_state_dpms(&data);
> -		else
> -			test_dc_state_dpms(&data, CHECK_DC6);
> +		test_dc_state_dpms(&data, CHECK_DC6);
>  
>  	}
>  
> -- 
> 2.43.0
>