Re: [PATCH i-g-t v2 2/2] tests/intel/xe_pm: Add mocs suspend resume test

["Nilawar, Badal" <badal.nilawar@intel.com>]

On 25-04-2024 01:38, janga.rahul.kumar@intel.com wrote:
> From: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
> 
> Add test to check mocs values are retained over suspend/resume.
> 
> v2: Extend test to check S3/S4 suspend states. (Matt Roper)
> 
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Janga Rahul Kumar <janga.rahul.kumar@intel.com>
> ---
>   tests/intel/xe_pm.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 56 insertions(+)
> 
> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
> index 51442537b..baec50d7f 100644
> --- a/tests/intel/xe_pm.c
> +++ b/tests/intel/xe_pm.c
> @@ -251,6 +251,11 @@ static void close_fw_handle(int sig)
>    * Description: suspend/autoresume on %arg[1] state and exec after RPM
>    * Functionality: pm - %arg[1]
>    *
> + * SUBTEST: %s-mocs
> + * Description: test checks for mocs state before and after %arg[1] state
> + * Functionality: pm - %arg[1]
> + * GPU requirements: D3 feature should be supported
> + *
>    * arg[1]:
>    *
>    * @s2idle:	s2idle
> @@ -609,6 +614,49 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags)
>   	close(fw_handle);
>   }
>   
> +/**
> + * SUBTEST: mocs_suspend_resume
> + * Description:
> + *     Validate mocs register contents over suspend resume
> + *
> + * Functionality: mocs
> + * Run type: FULL
> + */
> +static void test_mocs_suspend_resume(device_t device, bool runtime_sr, enum igt_suspend_state state)
> +{
> +	int gt;
> +
> +	xe_for_each_gt(device.fd_xe, gt) {
> +		char path[256];
> +
> +		// Mocs debugfs contents before and after suspend-resume
> +		char mocs_content_pre[4096], mocs_contents_post[4096];
> +
> +		sprintf(path, "gt%d/mocs", gt);
> +		igt_assert(igt_debugfs_exists(device.fd_xe, path, O_RDONLY));
> +		igt_debugfs_dump(device.fd_xe, path);
> +		igt_debugfs_read(device.fd_xe, path, mocs_content_pre);
> +
> +		fw_handle = igt_debugfs_open(device.fd_xe, "forcewake_all", O_RDONLY);
> +		igt_assert(fw_handle >= 0);
> +		igt_assert(igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_ACTIVE);
The code in above 3 lines is required for rpm use case only, so better 
to move it inside if statement below.
> +
> +		if (runtime_sr) {
> +			/* Runtime suspend  */
> +			close(fw_handle);
> +			igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> +		} else {
> +			igt_system_suspend_autoresume(state, SUSPEND_TEST_NONE);
> +		}
> +
> +		igt_assert(igt_debugfs_exists(device.fd_xe, path, O_RDONLY));
> +		igt_debugfs_dump(device.fd_xe, path);
> +		igt_debugfs_read(device.fd_xe, path, mocs_contents_post);
> +
> +		igt_assert(strcmp(mocs_content_pre, mocs_contents_post) == 0);
> +	}
> +}
> +
>   igt_main
>   {
>   	struct drm_xe_engine_class_instance *hwe;
> @@ -703,6 +751,10 @@ igt_main
>   				cleanup_d3(device);
>   			}
>   		}
> +
> +		igt_subtest_f("%s-mocs", s->name) {
No need of braces for single statement.
> +			test_mocs_suspend_resume(device, 0, s->state);
> +		}
>   	}
>   
>   	for (const struct d_state *d = d_states; d->name; d++) {
> @@ -767,6 +819,10 @@ igt_main
>   			dpms_on_off(device, DRM_MODE_DPMS_ON);
>   			igt_pm_set_autosuspend_delay(device.pci_xe, delay_ms);
>   		}
> +
> +		igt_subtest("mocs_suspend_resume")
> +			test_mocs_suspend_resume(device, 1, 0);
> +
Remove extra blank line.

Regards,
Badal
>   	}
>   
>   	igt_fixture {