[PATCH i-g-t v1 1/2] lib/igt_psr: Skip the test when PSR is disabled

[Grzelak, Michal]

Hi Jeevan,

> -----Original Message-----
> From: B, Jeevan <jeevan.b at intel.com> 
> Sent: Monday, May 26, 2025 7:38 AM
> To: Grzelak, Michal <michal.grzelak at intel.com>; igt-dev at lists.freedesktop.org
> Cc: Grzelak, Michal <michal.grzelak at intel.com>
> Subject: RE: [PATCH i-g-t v1 1/2] lib/igt_psr: Skip the test when PSR is disabled

>> -----Original Message-----
>> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of 
>> Michal Grzelak
>> Sent: Tuesday, May 13, 2025 5:49 PM
>> To: igt-dev at lists.freedesktop.org
>> Cc: Grzelak, Michal <michal.grzelak at intel.com>
>> Subject: [PATCH i-g-t v1 1/2] lib/igt_psr: Skip the test when PSR is 
>> disabled
>> 
>> If PSR setup timing is not met, then PSR will stay disabled and test 
>> will fail anyway. Skip the test upon finding that setup timing was not met.
>> 
>> Signed-off-by: Michał Grzelak <michal.grzelak at intel.com>
>> ---
>>  lib/igt_psr.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/lib/igt_psr.c b/lib/igt_psr.c index 3d8f1949b..f35051ebc 
>> 100644
>> --- a/lib/igt_psr.c
>> +++ b/lib/igt_psr.c
>> @@ -38,7 +38,7 @@ bool psr_disabled_check(int debugfs_fd)
>>  	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
>>  				sizeof(buf));
>> 
>> -	return strstr(buf, "PSR mode: disabled\n");
>Lets not change the format here. We can still keep it as PSR mode : disabled/enabled
>> +	return strstr(buf, "PSR disabled:");
>>  }
>> 
>>  bool selective_fetch_check(int debugfs_fd, igt_output_t *output) @@ 
>> -122,6
>> +122,8 @@ static bool psr_active_check(int debugfs_fd, enum psr_mode 
>> +mode,
>> igt_output_t *o
>> 
>>  	igt_skip_on(strstr(buf, "PSR sink not reliable: yes"));
>> 
>> +	igt_skip_on(strstr(buf, "PSR setup timing not met"));
>> +
> I think instead of printing like this. It can be:  PSR sink not reliable: yes (setup timing not met)

My current understanding is that kernel on JSL reports that sink is reliable. But it disables PSR basing on the setup timing. Thus, questioned lines in i915_edp_psr_status with this change look like this:

PSR mode: disabled
PSR sink not reliable: no (PSR setup timing not met)

I thought it looks a bit misleading, because it seems like we are giving the reason why the sink is reliable, and not why PSR is disabled. That's why I also changed the format, to include this case. Also, because of that, in the IGT test we would still need two different checks for skip, since "sink not reliable:" is independent of "PSR setup timing not met".

What are your thoughts on this? Should we keep the reason after "sink not reliable:", change the format, or something else?

Best regards,
Michał