Re: [PATCH i-g-t] kms_rotation_crc: 90 degree flip test is not a stress test

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 03/08/2017 15:19, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-08-03 14:41:34)
>>
>> On 03/08/2017 14:27, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2017-08-03 14:09:59)
>>>>
>>>> On 03/08/2017 13:53, Chris Wilson wrote:
>>>>> Quoting Tvrtko Ursulin (2017-08-03 13:42:50)
>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> To the best of my recollection the page flipping test was added
>>>>>> simply to start exercising page flips with 90/270 rotation.
>>>>>>
>>>>>> There is no need to do 60 flips which can take quite some time
>>>>>> because we test each pipe and then each fb geometry. And
>>>>>> calling this a stress test is also not matching the original
>>>>>> idea of the test.
>>>>>>
>>>>>> So remove the stress from the name and reduce the number of
>>>>>> flips to three only.
>>>>>
>>>>> Considering this found a bug, do we have an explicit test that says a
>>>>> rotated page flip takes no longer than a vblank (given the right
>>>>> conditions, i.e subsequent flips)?
>>>>
>>>> That was just me misremembering how the test work, wasn't a bug. Once I
>>>> looked at the code in more detail I realized the test does much more
>>>> flipping than it initially seemed. Num_pipes * 4 fb geometries * 2
>>>> framebuffers * 60 flips. In total around 8 seconds of flipping per pipe.
>>>> So the 25 second runtime is in line with 3 pipes at 60Hz plus some test
>>>> setup time.
>>>
>>> Ah. But do we have a test that says we can hit vrefresh with rotated
>>> pipes? I know we check the timings for the ordinary case, but from a
>>> quick check of the likely suspects, I can't see such a test.
>>
>> Not in kms_rotation_crc, so one to pencil in on the TODO list.
>>
>>> PS please add the explanation for why it took longer than expected into
>>> the commit log.
>>
>> I mentioned in the commit why it takes long ("...60 flips which can take
>> quite some time because we test each pipe and then each fb
>> geometry..."), but I am not sure that is longer than expected.
> 
> Even on a second read, it is still 60 flips, not 60 flips per
> combination. :)

Ah ok, it isn't fully understandable. That's what happens with quick 
patches which only change one trivial thing.

>> better say I am not sure what is expected. One could even say, since the
>> test had stress in its name, that 25 seconds was not unexpected. :) Now
>> it is not a stress test any more and it finishes in five seconds so all
>> is good. I am not sure what exactly you think I should add? The formula
>> for exact number of flips test was doing?
> 
> Language. The test is a crc check, how many unique frames do we show?
> What changes between every flip that applies any stress to the driver,
> i.e. causes either the hw or driver to do something different? If we are
> looking for a race, can crc even spot it? Do we gain anything from this
> test by even displaying a second frame?

I don't think we gain much by doing more than one flip and there are no 
crc checks or anything. So from that pov it is not the best test. Should 
be probably changed so in flip mode it flips before the crc collection.

> Ultimately, is the path through the driver taken by each frame a good
> enough metric to decide if the test has achieved its maximal coverage?

There should be a ton more coverage that we should add before calling it 
maximal coverage. Like tiling changes between flips, which is currently 
tested only for unrotated fbs. some rotation changes might also be 
possible between flips?

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx