On Thu, Nov 5, 2015 at 12:11 PM, Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
> 2015-11-05 16:53 GMT-02:00 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>:
>> With commit (drm/i915: Delay first PSR activation.) in kernel
>> PSR might take a bit longer to really activate after the modeset.
>
> Can you please expand this commit message a little bit for Future
> Paulo and Future Rodrigo? It would be nice to mention that the timeout
> is going to be 5 times the panel power cycle delay, which is XYZms for
> your machine, so the timeout needs to be a minimum of Xs, so 5s is the
> safe value (we print the panel power cycle delay time on dmesg).
Sure I will expand it.
>
> Also, maybe add a notice that since we do a lot of
> assert(psr_disabled), this commit is increasing the time it takes to
> run the whole set of PSR tests by a few minutes (check commit
> f4db3b18841263f8f617a9f7f0aaf14fab7196d1 for an explanation).
it shouldn't increase that much since igt_wait returns what ever
happens first: psr getting enabled or the time out
and the PSR delayed start is only after the full modeset when psr gets
enabled and these asserts happens many more times from what I saw.
>
> Maybe you could also provide patch 9/8 adding a DONT_ASSERT_PSR_STATUS
> flag and patch 10/8 that changes the callers of op_disables_psr(),
> making them use DONT_ASSERT_PSR_STATUS so we won't eat the full 5s
> timeout during every single GTT mmap test. This will make the time it
> takes to run the full set of PSR tests even smaller than what it is
> today (!!!).
oh I see... but I really like the assert enabled/disable even taking longer...
Maybe we should pick few tests that doesn't take so long and call them
basic_ to get included on BAT ;)
>
> With the improved commit message: Reviewed-by: Paulo Zanoni
> <paulo.r.zanoni@xxxxxxxxx>
Anyway I will double check with you again! ;)
Thanks for the review and comments!
>
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>> ---
>> tests/kms_frontbuffer_tracking.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
>> index 38ed662..cd2879d 100644
>> --- a/tests/kms_frontbuffer_tracking.c
>> +++ b/tests/kms_frontbuffer_tracking.c
>> @@ -848,7 +848,7 @@ static bool fbc_wait_until_enabled(void)
>>
>> static bool psr_wait_until_enabled(void)
>> {
>> - return igt_wait(psr_is_enabled(), 2000, 1);
>> + return igt_wait(psr_is_enabled(), 5000, 1);
>> }
>>
>> #define fbc_enable() igt_set_module_param_int("enable_fbc", 1)
>> --
>> 2.4.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
