[PATCH i-g-t 4/5] tests/intel/xe_pm: Only check the rpm resume after the first mmap operation
Nilawar, Badal
badal.nilawar at intel.com
Tue May 14 16:39:00 UTC 2024
On 14-05-2024 20:49, Nilawar, Badal wrote:
>
>
> On 14-05-2024 00:25, Rodrigo Vivi wrote:
>> The very first memory operation on a mmaped region after runtime suspend,
>> the device will be resumed if the memory used is vram.
>> However, after this first operation with page fault, the memory
>> can be migrated to the system memory.
This probably due to xe_bo_evict_all() in D3 cold.
>>
>> During this migration, Xe kernel will get a notification at
>> xe_bo_move_notify, and the bo will be removed from the
>> vram_userfault list. Then, on the next suspend, we won't mark
>> bo for page fault and resume won't happen and the active count
>> won't increase. This is okay, since the bo is now in the system
>> memory we don't need to wake up the device. But the current
>> test is wrong, because it assumes that the bo keeps forever
>> in vram, what is the issue here.
Not completely wrong in D3 Hot.
>>
>> Only checking the resume after the first operation is the
>> enough. But also, ensure that both read and write operations
>> are actually validated.
Should we keep "first operation" check only for D3 cold and previous
logic for D3 hot? May be combine patch 4 and 5 to handle this.
Regards,
Badal
>>
>> Cc: Badal Nilawar <badal.nilawar at intel.com>
>> Cc: Anshuman Gupta <anshuman.gupta at intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> ---
>> tests/intel/xe_pm.c | 36 +++++++++++++++++++++++++-----------
>> 1 file changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
>> index 72d484163..81226a910 100644
>> --- a/tests/intel/xe_pm.c
>> +++ b/tests/intel/xe_pm.c
>> @@ -37,6 +37,11 @@
>> #define PREFETCH (0x1 << 1)
>> #define UNBIND_ALL (0x1 << 2)
>> +enum mem_op {
>> + READ,
>> + WRITE,
>> +};
>> +
>> typedef struct {
>> int fd_xe;
>> struct pci_device *pci_xe;
>> @@ -525,7 +530,8 @@ static void test_vram_d3cold_threshold(device_t
>> device, int sysfs_fd)
>> *
>> * Functionality: pm-d3
>> */
>> -static void test_mmap(device_t device, uint32_t placement, uint32_t
>> flags)
>> +static void test_mmap(device_t device, uint32_t placement, uint32_t
>> flags,
>> + enum mem_op first_op)
>> {
>> size_t bo_size = 8192;
>> uint32_t *map = NULL;
>> @@ -563,8 +569,12 @@ static void test_mmap(device_t device, uint32_t
>> placement, uint32_t flags)
>>
>> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
>> active_time = igt_pm_get_runtime_active_time(device.pci_xe);
>> - for (i = 0; i < bo_size / sizeof(*map); i++)
>> - igt_assert(map[i] == MAGIC_1);
>> + for (i = 0; i < bo_size / sizeof(*map); i++) {
>> + if (first_op == READ)
>> + igt_assert(map[i] == MAGIC_1);
>> + else
>> + map[i] = MAGIC_2;
>> + }
>> /* dgfx page-fault on mmaping should wake the gpu */
>> if (xe_has_vram(device.fd_xe) && flags &
>> DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)
>> @@ -574,12 +584,12 @@ static void test_mmap(device_t device, uint32_t
>> placement, uint32_t flags)
>>
>> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
>> active_time = igt_pm_get_runtime_active_time(device.pci_xe);
> Above line can be removed.
>> - for (i = 0; i < bo_size / sizeof(*map); i++)
>> - map[i] = MAGIC_2;
>> -
>> - if (xe_has_vram(device.fd_xe) && flags &
>> DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)
>> - igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
>> - active_time);
>> + for (i = 0; i < bo_size / sizeof(*map); i++) {
>> + if (first_op == READ)
>> + map[i] = MAGIC_2;
>> + else
>> + igt_assert(map[i] == MAGIC_2);
>> + }
>>
>> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
>> @@ -781,7 +791,8 @@ igt_main
>> "when device along with parent bridge in d3");
>> igt_subtest("d3-mmap-system") {
>> dpms_on_off(device, DRM_MODE_DPMS_OFF);
>> - test_mmap(device, system_memory(device.fd_xe), 0);
>> + test_mmap(device, system_memory(device.fd_xe), 0, READ);
>> + test_mmap(device, system_memory(device.fd_xe), 0, WRITE);
>> dpms_on_off(device, DRM_MODE_DPMS_ON);
>> }
>> @@ -800,7 +811,10 @@ igt_main
>> /* Give some auto suspend delay to validate rpm active
>> during page fault */
>> igt_pm_set_autosuspend_delay(device.pci_xe, 1000);
>> dpms_on_off(device, DRM_MODE_DPMS_OFF);
>> - test_mmap(device, vram_memory(device.fd_xe, 0),
>> DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
>> + test_mmap(device, vram_memory(device.fd_xe, 0),
>> + DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM, READ);
>> + test_mmap(device, vram_memory(device.fd_xe, 0),
>> + DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM,
>> WRITE);With a comment this is
> Reviewed-by: Badal Nilawar <badal.nilawar at intel.com>
>
> Regards,
> Badal
>> dpms_on_off(device, DRM_MODE_DPMS_ON);
>> igt_pm_set_autosuspend_delay(device.pci_xe, delay_ms);
>> }
More information about the igt-dev
mailing list