* Re: [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT
2016-10-17 14:46 ` Imre Deak
@ 2016-10-17 14:56 ` Jani Nikula
2016-10-17 15:01 ` Saarinen, Jani
2016-10-18 7:34 ` Daniel Vetter
2 siblings, 0 replies; 10+ messages in thread
From: Jani Nikula @ 2016-10-17 14:56 UTC (permalink / raw)
To: imre.deak, Daniel Vetter; +Cc: intel-gfx
On Mon, 17 Oct 2016, Imre Deak <imre.deak@intel.com> wrote:
> On ma, 2016-10-17 at 16:32 +0200, Daniel Vetter wrote:
>> On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
>> > Add gem_exec_suspend/basic-s3-devices and basic-s4-devices subtests
>> > to
>> > BAT. At the same time remove basic-s4 from the list, which is atm
>> > implicitly disabled via HIBERNATION=n in kconfig. We would need at
>> > least
>> > basic S4 coverage provided by basic-s4-devices, which requires
>> > HIBERNATION=y.
>> >
>> > Signed-off-by: Imre Deak <imre.deak@intel.com>
>>
>> What's the impact on BAT runtime with this?
>
> I measured 8 sec for S3-devices and 9 sec for S4-devices on my APL.
>
>> Afaik we're already over budget ... Where do you safe the time to
>> afford this?
>
> I didn't, but we don't have any S4 coverage in CI atm and it's the
> minimum that can be added. The S3-devices subtest is not critical,
> although it would be useful for cases where we wouldn't get any logs
> for the full S3 test.
If our CI was sufficiently clever, it could detect these patches to
change the test list, and let us all know what the impact is...
BR,
Jani.
>
> --Imre
>
>> -Daniel
>>
>> > ---
>> > tests/intel-ci/fast-feedback.testlist | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-
>> > ci/fast-feedback.testlist
>> > index f59ec98..efa7e86 100644
>> > --- a/tests/intel-ci/fast-feedback.testlist
>> > +++ b/tests/intel-ci/fast-feedback.testlist
>> > @@ -73,8 +73,9 @@ igt@gem_exec_store@basic-default
>> > igt@gem_exec_store@basic-render
>> > igt@gem_exec_store@basic-vebox
>> > igt@gem_exec_suspend@basic
>> > +igt@gem_exec_suspend@basic-s3-devices
>> > igt@gem_exec_suspend@basic-s3
>> > -igt@gem_exec_suspend@basic-s4
>> > +igt@gem_exec_suspend@basic-s4-devices
>> > igt@gem_flink_basic@bad-flink
>> > igt@gem_flink_basic@bad-open
>> > igt@gem_flink_basic@basic
>> > --
>> > 2.5.0
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT
2016-10-17 14:46 ` Imre Deak
2016-10-17 14:56 ` Jani Nikula
@ 2016-10-17 15:01 ` Saarinen, Jani
2016-10-17 15:02 ` Imre Deak
2016-10-18 7:34 ` Daniel Vetter
2 siblings, 1 reply; 10+ messages in thread
From: Saarinen, Jani @ 2016-10-17 15:01 UTC (permalink / raw)
To: Deak, Imre, Daniel Vetter; +Cc: intel-gfx@lists.freedesktop.org
HI,
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Imre Deak
> Sent: Monday, October 17, 2016 5:47 PM
> To: Daniel Vetter <daniel@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-
> S3/S4-devices to BAT
>
> On ma, 2016-10-17 at 16:32 +0200, Daniel Vetter wrote:
> > On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
> > > Add gem_exec_suspend/basic-s3-devices and basic-s4-devices subtests
> > > to BAT. At the same time remove basic-s4 from the list, which is atm
> > > implicitly disabled via HIBERNATION=n in kconfig. We would need at
> > > least basic S4 coverage provided by basic-s4-devices, which requires
> > > HIBERNATION=y.
> > >
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >
> > What's the impact on BAT runtime with this?
Are these also stable enough to be added so that we are not causing flip-flops?
>
> I measured 8 sec for S3-devices and 9 sec for S4-devices on my APL.
>
> > Afaik we're already over budget ... Where do you safe the time to
> > afford this?
>
> I didn't, but we don't have any S4 coverage in CI atm and it's the minimum
> that can be added. The S3-devices subtest is not critical, although it would be
> useful for cases where we wouldn't get any logs for the full S3 test.
>
> --Imre
>
> > -Daniel
> >
> > > ---
> > > tests/intel-ci/fast-feedback.testlist | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-
> > > ci/fast-feedback.testlist index f59ec98..efa7e86 100644
> > > --- a/tests/intel-ci/fast-feedback.testlist
> > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > @@ -73,8 +73,9 @@ igt@gem_exec_store@basic-default
> > > igt@gem_exec_store@basic-render
> > > igt@gem_exec_store@basic-vebox
> > > igt@gem_exec_suspend@basic
> > > +igt@gem_exec_suspend@basic-s3-devices
> > > igt@gem_exec_suspend@basic-s3
> > > -igt@gem_exec_suspend@basic-s4
> > > +igt@gem_exec_suspend@basic-s4-devices
> > > igt@gem_flink_basic@bad-flink
> > > igt@gem_flink_basic@bad-open
> > > igt@gem_flink_basic@basic
> > > --
> > > 2.5.0
> > >
Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT
2016-10-17 15:01 ` Saarinen, Jani
@ 2016-10-17 15:02 ` Imre Deak
0 siblings, 0 replies; 10+ messages in thread
From: Imre Deak @ 2016-10-17 15:02 UTC (permalink / raw)
To: Saarinen, Jani, Daniel Vetter; +Cc: intel-gfx@lists.freedesktop.org
On ma, 2016-10-17 at 18:01 +0300, Saarinen, Jani wrote:
> HI,
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On
> > Behalf
> > Of Imre Deak
> > Sent: Monday, October 17, 2016 5:47 PM
> > To: Daniel Vetter <daniel@ffwll.ch>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] intel-ci: Add
> > gem_exec_suspend/basic-
> > S3/S4-devices to BAT
> >
> > On ma, 2016-10-17 at 16:32 +0200, Daniel Vetter wrote:
> > > On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
> > > > Add gem_exec_suspend/basic-s3-devices and basic-s4-devices
> > > > subtests
> > > > to BAT. At the same time remove basic-s4 from the list, which
> > > > is atm
> > > > implicitly disabled via HIBERNATION=n in kconfig. We would need
> > > > at
> > > > least basic S4 coverage provided by basic-s4-devices, which
> > > > requires
> > > > HIBERNATION=y.
> > > >
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > >
> > > What's the impact on BAT runtime with this?
> Are these also stable enough to be added so that we are not causing
> flip-flops?
Based on my tests, they are.
> > I measured 8 sec for S3-devices and 9 sec for S4-devices on my APL.
> >
> > > Afaik we're already over budget ... Where do you safe the time to
> > > afford this?
> >
> > I didn't, but we don't have any S4 coverage in CI atm and it's the
> > minimum
> > that can be added. The S3-devices subtest is not critical, although
> > it would be
> > useful for cases where we wouldn't get any logs for the full S3
> > test.
> >
> > --Imre
> >
> > > -Daniel
> > >
> > > > ---
> > > > tests/intel-ci/fast-feedback.testlist | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tests/intel-ci/fast-feedback.testlist
> > > > b/tests/intel-
> > > > ci/fast-feedback.testlist index f59ec98..efa7e86 100644
> > > > --- a/tests/intel-ci/fast-feedback.testlist
> > > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > > @@ -73,8 +73,9 @@ igt@gem_exec_store@basic-default
> > > > igt@gem_exec_store@basic-render
> > > > igt@gem_exec_store@basic-vebox
> > > > igt@gem_exec_suspend@basic
> > > > +igt@gem_exec_suspend@basic-s3-devices
> > > > igt@gem_exec_suspend@basic-s3
> > > > -igt@gem_exec_suspend@basic-s4
> > > > +igt@gem_exec_suspend@basic-s4-devices
> > > > igt@gem_flink_basic@bad-flink
> > > > igt@gem_flink_basic@bad-open
> > > > igt@gem_flink_basic@basic
> > > > --
> > > > 2.5.0
> > > >
>
> Jani Saarinen
> Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT
2016-10-17 14:46 ` Imre Deak
2016-10-17 14:56 ` Jani Nikula
2016-10-17 15:01 ` Saarinen, Jani
@ 2016-10-18 7:34 ` Daniel Vetter
2016-10-18 9:03 ` Petri Latvala
2 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2016-10-18 7:34 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
On Mon, Oct 17, 2016 at 05:46:50PM +0300, Imre Deak wrote:
> On ma, 2016-10-17 at 16:32 +0200, Daniel Vetter wrote:
> > On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
> > > Add gem_exec_suspend/basic-s3-devices and basic-s4-devices subtests
> > > to
> > > BAT. At the same time remove basic-s4 from the list, which is atm
> > > implicitly disabled via HIBERNATION=n in kconfig. We would need at
> > > least
> > > basic S4 coverage provided by basic-s4-devices, which requires
> > > HIBERNATION=y.
> > >
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >
> > What's the impact on BAT runtime with this?
>
> I measured 8 sec for S3-devices and 9 sec for S4-devices on my APL.
>
> > Afaik we're already over budget ... Where do you safe the time to
> > afford this?
>
> I didn't, but we don't have any S4 coverage in CI atm and it's the
> minimum that can be added. The S3-devices subtest is not critical,
> although it would be useful for cases where we wouldn't get any logs
> for the full S3 test.
Yeah, I don't think a reduced S3 test in BAT is useful if we have the
full one already. It's a good test for debugging, but not within the very
tight constraints we have for BAT.
Re S4: If it indeed improves coverage (i.e. calls our shutdown hooks and
all these S4-only callbacks) then adding it to BAT sounds reasonable.
Still there's the issue of where to get the machine time from. I really do
think you need to first trade in some speed-up here (or throw out some
other tests) before you can add more tests.
And yes I know that's not fun, but until we have CI running a more
complete test set (after BAT has done the initial sanity checking, to
avoid wasting machine time on broken patches), that's the reality we have
:(
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT
2016-10-18 7:34 ` Daniel Vetter
@ 2016-10-18 9:03 ` Petri Latvala
2016-10-18 9:23 ` Ville Syrjälä
0 siblings, 1 reply; 10+ messages in thread
From: Petri Latvala @ 2016-10-18 9:03 UTC (permalink / raw)
To: intel-gfx
On Tue, Oct 18, 2016 at 09:34:49AM +0200, Daniel Vetter wrote:
> Re S4: If it indeed improves coverage (i.e. calls our shutdown hooks and
> all these S4-only callbacks) then adding it to BAT sounds reasonable.
> Still there's the issue of where to get the machine time from. I really do
> think you need to first trade in some speed-up here (or throw out some
> other tests) before you can add more tests.
For the record, the runtimes of all tests in CI can be found in the
result-runtimes.log file, within the test log directory.
Over 10s runtimes on a randomly selected machine are:
31.92 igt@gem_ctx_switch@basic-default-heavy
22.91 igt@gem_busy@basic-hang-default
21.03 igt@kms_flip@basic-flip-vs-dpms
20.96 igt@kms_flip@basic-flip-vs-modeset
20.39 igt@gem_ctx_switch@basic-default
20.30 igt@gem_exec_create@basic
20.11 igt@gem_ctx_create@basic-files
19.64 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
18.93 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
18.91 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
18.76 igt@gem_exec_suspend@basic-s3
17.98 igt@kms_pipe_crc_basic@hang-read-crc-pipe-c
16.70 igt@kms_flip@basic-flip-vs-wf_vblank
14.17 igt@kms_pipe_crc_basic@hang-read-crc-pipe-a
14.14 igt@gem_exec_nop@basic-parallel
14.12 igt@gem_exec_nop@basic-series
13.76 igt@kms_pipe_crc_basic@hang-read-crc-pipe-b
12.70 igt@gem_ringfill@basic-default-hang
11.34 igt@drv_hangman@error-state-basic
10.91 igt@gem_sync@basic-many-each
10.81 igt@gem_close_race@basic-threads
10.73 igt@kms_flip@basic-plain-flip
10.33 igt@gem_sync@basic-store-each
10.29 igt@gem_sync@basic-store-all
10.11 igt@gem_sync@basic-each
10.08 igt@gem_sync@basic-all
Does any of these strike as a low-hanging fruit?
--
Petri Latvala
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT
2016-10-18 9:03 ` Petri Latvala
@ 2016-10-18 9:23 ` Ville Syrjälä
0 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2016-10-18 9:23 UTC (permalink / raw)
To: Petri Latvala; +Cc: intel-gfx
On Tue, Oct 18, 2016 at 12:03:12PM +0300, Petri Latvala wrote:
> On Tue, Oct 18, 2016 at 09:34:49AM +0200, Daniel Vetter wrote:
> > Re S4: If it indeed improves coverage (i.e. calls our shutdown hooks and
> > all these S4-only callbacks) then adding it to BAT sounds reasonable.
> > Still there's the issue of where to get the machine time from. I really do
> > think you need to first trade in some speed-up here (or throw out some
> > other tests) before you can add more tests.
>
> For the record, the runtimes of all tests in CI can be found in the
> result-runtimes.log file, within the test log directory.
>
> Over 10s runtimes on a randomly selected machine are:
>
> 31.92 igt@gem_ctx_switch@basic-default-heavy
> 22.91 igt@gem_busy@basic-hang-default
> 21.03 igt@kms_flip@basic-flip-vs-dpms
> 20.96 igt@kms_flip@basic-flip-vs-modeset
> 20.39 igt@gem_ctx_switch@basic-default
> 20.30 igt@gem_exec_create@basic
> 20.11 igt@gem_ctx_create@basic-files
> 19.64 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
> 18.93 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
> 18.91 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
> 18.76 igt@gem_exec_suspend@basic-s3
> 17.98 igt@kms_pipe_crc_basic@hang-read-crc-pipe-c
> 16.70 igt@kms_flip@basic-flip-vs-wf_vblank
> 14.17 igt@kms_pipe_crc_basic@hang-read-crc-pipe-a
> 14.14 igt@gem_exec_nop@basic-parallel
> 14.12 igt@gem_exec_nop@basic-series
> 13.76 igt@kms_pipe_crc_basic@hang-read-crc-pipe-b
> 12.70 igt@gem_ringfill@basic-default-hang
> 11.34 igt@drv_hangman@error-state-basic
> 10.91 igt@gem_sync@basic-many-each
> 10.81 igt@gem_close_race@basic-threads
> 10.73 igt@kms_flip@basic-plain-flip
> 10.33 igt@gem_sync@basic-store-each
> 10.29 igt@gem_sync@basic-store-all
> 10.11 igt@gem_sync@basic-each
> 10.08 igt@gem_sync@basic-all
>
> Does any of these strike as a low-hanging fruit?
s/15/5/ for the S3 delay I'd say. If there's a machine that won't work
with 5 seconds, then we find out what's the lowest safe value and use
that instead. 15 seconds is excessive on all the machines I've seen.
And maybe make these delays configurabe via env variables so that I
don't have to keep patching igt when I want to test suspendy stuff ;)
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread