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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Sep 04, 2017 at 03:36:56PM +0100, Tvrtko Ursulin wrote:
> 
> On 04/09/2017 15:27, Tvrtko Ursulin wrote:
> > On 07/08/2017 16:53, Daniel Vetter wrote:
> > > On Fri, Aug 04, 2017 at 09:43:41AM +0100, Tvrtko Ursulin wrote:
> > > > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > > > 
> > > > 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 do 60 flips against each pipe and each fb geometry.
> > > > 
> > > > Also, calling this a stress test is also not matching the
> > > > original idea of the test.
> > > > 
> > > > Several changes:
> > > > 
> > > > 1. Remove the stress from the name and reduce the number of
> > > > flips to one only.
> > > > 
> > > > 2. Move the page flip before CRC collection for a more useful
> > > > test.
> > > > 
> > > > 3. Add more flipping tests, for different rotation and sprite
> > > > planes.
> > > 
> > > I assume you didn't make the test overall slower with this?
> > > 
> > > > 4. Convert to table driven subtest generation.
> > > > 
> > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > 
> > > Didn't do a full review, but sounds all reasonable. And I assume you've
> > > tested this at least locally (the igt patchwork CI instance doesn't do
> > > full runs ... yet).
> > 
> > Yes I've ran it on SKL. It adds more subtests so the runtime is longer,
> > but it also finds new bugs.
> 
> Runtime is longer compared to v1 of this patch BTW, I wasn't clear. It is
> roughly on par with the situation without this patch.
> 
> But v2 adds a lot more subtests, three of which fail (flip on a rotated
> sprite plane). So there is value in it I think even like that.

Failing new subtests is ok imo, but pls do give a heads-up to CI folks
before merging (but I think as long as it's failing stable, it shouldn't
cause noise). Anything that takes out the machine or driver isn't ok, and
needs to be fixed first.

So sounds all good to me to go ahead.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux