[PATCH i-g-t 2/2] tests/intel/xe_pm: Extend mocs-rpm test to support D3Cold

[Kamil Konieczny]

Hi Riana,
On 2024-07-02 at 16:19:18 +0530, Riana Tauro wrote:
> Hi Badal
> 
> On 7/1/2024 8:24 PM, Badal Nilawar wrote:
> > Use existing d3 helpers to cover D3Hot and D3Cold.
> add description for the test being modified in commit message
> > 
> > Signed-off-by: Badal Nilawar <badal.nilawar at intel.com>
> > ---
> >   tests/intel/xe_pm.c | 49 +++++++++++++++++++++++++++++++--------------
> >   1 file changed, 34 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
> > index b30e94a2a..eac5ea8a7 100644
> > --- a/tests/intel/xe_pm.c
> > +++ b/tests/intel/xe_pm.c
> > @@ -611,10 +611,15 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags,
> >   }
> >   /**
> > - * SUBTEST: mocs-rpm
> > + * SUBTEST: %s-mocs-rpm
> %s already denotes the runtime pm state. 'rpm' can be dropped
> >    * Description:
> >    *     Validate mocs register contents over suspend resume
> %s/register/registers
> %s/suspend resume/runtime suspend,resume

or just:
 * Description: Validate mocs registers contents over runtime suspend and resume

> >    *
> > + * arg[2]:
> %s/arg[2]/arg[1]
> > + *
> > + * @d3hot:	d3hot
> > + * @d3cold:	d3cold
> > + *
> >    * Functionality: mocs registers
> Functionality for the other mocs test is pm - %arg[1]

Please do not use '%args[1]' in Functionality or other fields,
this is only for subtests names, also put all these before
describing args[] vars.

+Cc: Katarzyna Piecielska <katarzyna.piecielska at intel.com>

Regards,
Kamil

> Should be same
> >    * Run type: FULL
> >    */
> > @@ -629,9 +634,14 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags,
> >    * @s3:		s3
> >    * @s4:		s4
> >    */
> > -static void test_mocs_suspend_resume(device_t device, int s_state)
> > +static void test_mocs_suspend_resume(device_t device, enum igt_suspend_state s_state,
> > +				     enum igt_acpi_d_state d_state)
> >   {
> >   	int gt;
> > +	uint64_t active_time;
> > +	bool check_rpm = (d_state == IGT_ACPI_D3Hot ||
> > +			  d_state == IGT_ACPI_D3Cold);
> > +
> >   	xe_for_each_gt(device.fd_xe, gt) {
> It would be good to have a log to identify the gt running this test,
> else would be difficult to identify which GT moc registers failed
> >   		char path[256];
> > @@ -644,19 +654,27 @@ static void test_mocs_suspend_resume(device_t device, int s_state)
> >   		igt_debugfs_dump(device.fd_xe, path);
> >   		igt_debugfs_read(device.fd_xe, path, mocs_content_pre);
> > -		if (s_state == NO_SUSPEND) {
> > +		if (check_rpm) {
> > +			igt_assert(in_d3(device, d_state));
> > +			active_time = igt_pm_get_runtime_active_time(device.pci_xe);
> > +
> >   			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);
> > +			igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
> > +				   active_time);
> >   			/* Make sure runtime pm goes back to suspended status after closing forcewake_all */
> >   			close(fw_handle);
> > -			igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> > +			sleep(1);
> > +
> > +			igt_assert(in_d3(device, d_state));
> >   		} else {
> > -			enum igt_suspend_test test = s_state == SUSPEND_STATE_DISK ?
> > -				SUSPEND_TEST_DEVICES : SUSPEND_TEST_NONE;
> > +			if (s_state != NO_SUSPEND) {
> > +				enum igt_suspend_test test = s_state == SUSPEND_STATE_DISK ?
> > +					SUSPEND_TEST_DEVICES : SUSPEND_TEST_NONE;
> > -			igt_system_suspend_autoresume(s_state, test);
> > +				igt_system_suspend_autoresume(s_state, test);
> > +			}
> >   		}
> >   		igt_assert(igt_debugfs_exists(device.fd_xe, path, O_RDONLY));
> >   		igt_debugfs_dump(device.fd_xe, path);
> > @@ -766,7 +784,7 @@ igt_main
> >   		}
> >   		igt_subtest_f("%s-mocs", s->name)
> add igt_describe
> > -			test_mocs_suspend_resume(device, s->state);
> > +			test_mocs_suspend_resume(device, s->state, NO_RPM);
> >   	}
> >   	igt_fixture {
> > @@ -825,6 +843,13 @@ igt_main
> >   			igt_pm_set_autosuspend_delay(device.pci_xe, delay_ms);
> >   		}
> add igt_describe
> 
> Could you also add the tests temporarily to BAT in these series to check the
> CI of the new tests?
> 
> Thanks
> Riana Tauro
> > +
> > +		igt_subtest_f("%s-mocs-rpm", d->name) {
> > +			igt_assert(setup_d3(device, d->state));
> > +			test_mocs_suspend_resume(device, NO_SUSPEND, d->state);
> > +			cleanup_d3(device);
> > +		}
> > +
> >   	}
> >   	igt_describe("Validate whether card is limited to d3hot,"
> > @@ -835,12 +860,6 @@ igt_main
> >   		test_vram_d3cold_threshold(device, sysfs_fd);
> >   	}
> > -	igt_subtest("mocs-rpm") {
> > -		dpms_on_off(device, DRM_MODE_DPMS_OFF);
> > -		test_mocs_suspend_resume(device, NO_SUSPEND);
> > -		dpms_on_off(device, DRM_MODE_DPMS_ON);
> > -	}
> > -
> >   	igt_fixture {
> >   		close(sysfs_fd);
> >   		igt_pm_set_d3cold_allowed(device.pci_slot_name, d3cold_allowed);