Re: [igt-dev] [PATCH i-g-t 2/2] tests/kms_frontbuffer_tracking: Fix skips when PSR is not available.

Maarten Lankhorst
2019-02-01T09:52:55Z

Op 31-01-2019 om 21:58 schreef Souza, Jose:
> On Thu, 2019-01-31 at 15:18 +0100, Maarten Lankhorst wrote:
>> Use psr.can_test to determine we can disable PSR, and not skip
>> because
>> PSR is unavailable.
>>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: José Roberto de Souza <jose.souza@intel.com>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> lib/igt_psr.c | 2 +-
>> tests/kms_frontbuffer_tracking.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
>> index 5eb6d2c6ace5..6ad2c522f9a4 100644
>> --- a/lib/igt_psr.c
>> +++ b/lib/igt_psr.c
>> @@ -106,7 +106,7 @@ static bool psr_set(int debugfs_fd, int mode)
>>
>> ret = has_psr_debugfs(debugfs_fd);
>> if (ret == -ENODEV) {
>> - igt_skip_on_f(mode >= PSR_MODE_1, "PSR not
>> available\n");
>> + igt_skip("PSR not available\n");
>> return false;
>> }
>>
>> diff --git a/tests/kms_frontbuffer_tracking.c
>> b/tests/kms_frontbuffer_tracking.c
>> index 609f7b41a527..4d15ce1cf1d6 100644
>> --- a/tests/kms_frontbuffer_tracking.c
>> +++ b/tests/kms_frontbuffer_tracking.c
>> @@ -1134,7 +1134,7 @@ static bool disable_features(const struct
>> test_mode *t)
>>
>> fbc_disable();
>> drrs_disable();
>> - return psr_disable(drm.debugfs);
> Although the return true of this functions will cause a unecessary
> igt_display_commit() this changes looks good. In other patch we could
> just drop the return and remove the commit in prepare_subtest_data().
>
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

Thanks, pushed.

This should have been documented, but the function must return true if and only if a modeset is required.

Specifically, It should return false for the debugfs case so we don't do unnecessary modesets,

but true for the modparam case.

I think the rework borked it.

~Maarten