subject:"\[Intel\-gfx\] \[PATCH i\-g\-t\] kms_atomic_transition\: Add subtest time limit\/randomize plane, pipe combinations"

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

2017-11-02 Thread Imre Deak

On Wed, Nov 01, 2017 at 01:08:47PM +0200, Imre Deak wrote:
> On Wed, Nov 01, 2017 at 10:48:50AM +, Chris Wilson wrote:
> > Quoting Imre Deak (2017-11-01 09:56:22)
> > > On Tue, Oct 31, 2017 at 10:23:25PM +, Chris Wilson wrote:
> > > > Quoting Imre Deak (2017-10-31 13:44:47)
> > > > > Doing modeset on internal panels may have a considerable overhead due 
> > > > > to
> > > > > the panel specific power sequencing delays. To avoid long test 
> > > > > runtimes
> > > > > limit the runtime of each subtest. Randomize the plane/pipe 
> > > > > combinations
> > > > > to preserve the test coverage on such panels at least over multiple 
> > > > > test
> > > > > runs.
> > > > > 
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103334
> > > > > Cc: Maarten Lankhorst 
> > > > > Signed-off-by: Imre Deak 
> > > > > ---
> > > > >  tests/kms_atomic_transition.c | 175 
> > > > > --
> > > > >  1 file changed, 150 insertions(+), 25 deletions(-)
> > > > > 
> > > > > diff --git a/tests/kms_atomic_transition.c 
> > > > > b/tests/kms_atomic_transition.c
> > > > > index 4c295125..ac67fc3a 100644
> > > > > --- a/tests/kms_atomic_transition.c
> > > > > +++ b/tests/kms_atomic_transition.c
> > > > > @@ -39,6 +39,14 @@
> > > > >  #define DRM_CAP_CURSOR_HEIGHT 0x9
> > > > >  #endif
> > > > >  
> > > > > +#define MAX_SUBTEST_DURATION_NS (20ULL * NSEC_PER_SEC)
> > > > > +
> > > > > +struct test_config {
> > > > > +   igt_display_t *display;
> > > > > +   bool user_seed;
> > > > > +   int seed;
> > > > > +};
> > > > > +
> > > > >  struct plane_parms {
> > > > > struct igt_fb *fb;
> > > > > uint32_t width, height;
> > > > > @@ -401,6 +409,28 @@ static void wait_for_transition(igt_display_t 
> > > > > *display, enum pipe pipe, bool non
> > > > > }
> > > > >  }
> > > > >  
> > > > > +/* Copied from https://benpfaff.org/writings/clc/shuffle.html */
> > > > > +static void shuffle_array(uint32_t *array, int size, int seed)
> > > > > +{
> > > > > +   int i;
> > > > > +
> > > > > +   for (i = 0; i < size; i++) {
> > > > > +   int j = i + rand() / (RAND_MAX / (size - i) + 1);
> > > > > +
> > > > > +   igt_swap(array[i], array[j]);
> > > > > +   }
> > > > > +}
> > > > 
> > > > igt_permute_array()
> > > 
> > > Thanks, will use that instead.
> > > 
> > > > Not saying anything, but I was told using CI for stochastic coverage was
> > > > a flat no...
> > > 
> > > Ok, but it would only be the case for slow panels where the alternative
> > > is not to run the test at all. And is this a problem if we can replay a
> > > failing case with --seed?
> > 
> > If the purpose of CI is purely regression testing and not exploratory
> > debugging, each PW run must be with the same seed as the CI_DRM run.
> > 
> > The seed must be clearly displayed in the results so that when comparing
> > CI_DRM runs the flip-flop can be traced to the change in seed.
> 
> Adding Petri and Tomi. So I guess the question is if we want this in CI
> and if it's a reasonable effort to implement it.

Discussing more with Tomi and Martin, the randomization idea is not so
practical at least for now.

So another approach would be to split out the testing on internal panels
from plane-all-modeset-transition and
plane-all-modeset-transition-fencing. We could have fast and slow
version of these internal-panel tests where the fast one would test all
combinations and the slow one only combinations with either all planes
on/off or only a single plane on. We could then include only the fast
versions in the fast-feedback testlist.

Will follow up with a version doing the above.

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

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

2017-11-01 Thread Maarten Lankhorst

Op 01-11-17 om 13:55 schreef Imre Deak:
> On Wed, Nov 01, 2017 at 12:32:37PM +0100, Maarten Lankhorst wrote:
>> Op 31-10-17 om 14:44 schreef Imre Deak:
>>> Doing modeset on internal panels may have a considerable overhead due to
>>> the panel specific power sequencing delays. To avoid long test runtimes
>>> limit the runtime of each subtest. Randomize the plane/pipe combinations
>>> to preserve the test coverage on such panels at least over multiple test
>>> runs.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103334
>>> Cc: Maarten Lankhorst 
>>> Signed-off-by: Imre Deak 
>>> ---
>>>  tests/kms_atomic_transition.c | 175 
>>> --
>>>  1 file changed, 150 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
>>> index 4c295125..ac67fc3a 100644
>>> --- a/tests/kms_atomic_transition.c
>>> +++ b/tests/kms_atomic_transition.c
>>> @@ -39,6 +39,14 @@
>>>  #define DRM_CAP_CURSOR_HEIGHT 0x9
>>>  #endif
>>>  
>>> +#define MAX_SUBTEST_DURATION_NS (20ULL * NSEC_PER_SEC)
>>> +
>>> +struct test_config {
>>> +   igt_display_t *display;
>>> +   bool user_seed;
>>> +   int seed;
>>> +};
>>> +
>>>  struct plane_parms {
>>> struct igt_fb *fb;
>>> uint32_t width, height;
>>> @@ -401,6 +409,28 @@ static void wait_for_transition(igt_display_t 
>>> *display, enum pipe pipe, bool non
>>> }
>>>  }
>>>  
>>> +/* Copied from https://benpfaff.org/writings/clc/shuffle.html */
>>> +static void shuffle_array(uint32_t *array, int size, int seed)
>>> +{
>>> +   int i;
>>> +
>>> +   for (i = 0; i < size; i++) {
>>> +   int j = i + rand() / (RAND_MAX / (size - i) + 1);
>>> +
>>> +   igt_swap(array[i], array[j]);
>>> +   }
>>> +}
>> I wouldn't worry about predictibility of the random number generator..
>>
>> int j = i + rand() % (size - i) is good enough and easier to read. :)
> Chris already suggested igt_permute_array(), will use that.
>
>> I think the struct test_config can be killed too, since it goes unused
>> in shuffle_array, nothing in the test uses it...
> Oops, some kind of left-over from an earlier version. Thanks for spotting
> it.
>
>>> +static void init_combination_array(uint32_t *array, int size, int seed)
>>> +{
>>> +   int i;
>>> +
>>> +   for (i = 0; i < size; i++)
>>> +   array[i] = i;
>>> +
>>> +   shuffle_array(array, size, seed);
>>> +}
>>> +
>>>  /*
>>>   * 1. Set primary plane to a known fb.
>>>   * 2. Make sure getcrtc returns the correct fb id.
>>> @@ -411,19 +441,27 @@ static void wait_for_transition(igt_display_t 
>>> *display, enum pipe pipe, bool non
>>>   * so test this and make sure it works.
>>>   */
>>>  static void
>>> -run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t 
>>> *output,
>>> -   enum transition_type type, bool nonblocking, bool fencing)
>>> +run_transition_test(struct test_config *test_config, enum pipe pipe,
>>> +   igt_output_t *output, enum transition_type type,
>>> +   bool nonblocking, bool fencing)
>>>  {
>>> struct igt_fb fb, argb_fb, sprite_fb;
>>> drmModeModeInfo *mode, override_mode;
>>> +   igt_display_t *display = test_config->display;
>>> igt_plane_t *plane;
>>> igt_pipe_t *pipe_obj = >pipes[pipe];
>>> uint32_t iter_max = 1 << pipe_obj->n_planes, i;
>>> +   uint32_t *plane_combinations;
>>> +   struct timespec start = { };
>>> struct plane_parms parms[pipe_obj->n_planes];
>>> bool skip_test = false;
>>> unsigned flags = 0;
>>> int ret;
>>>  
>>> +   plane_combinations = malloc(sizeof(*plane_combinations) * iter_max);
>>> +   igt_assert(plane_combinations);
>>> +   init_combination_array(plane_combinations, iter_max, test_config->seed);
>> It would be cleaner to have a separate test for transition_modeset.
>> The rest should be run to completion and don't need shuffling, since
>> in normal cases, they'll never hit a timeout.
> Do you mean type == TRANSITION_MODESET? There are already separate
> subtests for that. Yes, can disable the timeout and shuffling for the
> rest.
>
>> So make a separate test for that, and perhaps also add a flag for
>> disabling the timeout.
> Ok.
>
>>> if (fencing)
>>> prepare_fencing(display, pipe);
>>> else
>>> @@ -527,39 +565,59 @@ run_transition_test(igt_display_t *display, enum pipe 
>>> pipe, igt_output_t *output
>>> goto cleanup;
>>> }
>>>  
>>> +   igt_nsec_elapsed();
>>> +
>>> for (i = 0; i < iter_max; i++) {
>>> igt_output_set_pipe(output, pipe);
>>>  
>>> -   wm_setup_plane(display, pipe, i, parms, fencing);
>>> +   wm_setup_plane(display, pipe, plane_combinations[i], parms,
>>> +  fencing);
>>>  
>>> -   atomic_commit(display, pipe, flags, (void *)(unsigned long)i, 
>>> fencing);
>>> +   atomic_commit(display, pipe, flags,
>>> +

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

2017-11-01 Thread Imre Deak

On Wed, Nov 01, 2017 at 12:32:37PM +0100, Maarten Lankhorst wrote:
> Op 31-10-17 om 14:44 schreef Imre Deak:
> > Doing modeset on internal panels may have a considerable overhead due to
> > the panel specific power sequencing delays. To avoid long test runtimes
> > limit the runtime of each subtest. Randomize the plane/pipe combinations
> > to preserve the test coverage on such panels at least over multiple test
> > runs.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103334
> > Cc: Maarten Lankhorst 
> > Signed-off-by: Imre Deak 
> > ---
> >  tests/kms_atomic_transition.c | 175 
> > --
> >  1 file changed, 150 insertions(+), 25 deletions(-)
> >
> > diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> > index 4c295125..ac67fc3a 100644
> > --- a/tests/kms_atomic_transition.c
> > +++ b/tests/kms_atomic_transition.c
> > @@ -39,6 +39,14 @@
> >  #define DRM_CAP_CURSOR_HEIGHT 0x9
> >  #endif
> >  
> > +#define MAX_SUBTEST_DURATION_NS (20ULL * NSEC_PER_SEC)
> > +
> > +struct test_config {
> > +   igt_display_t *display;
> > +   bool user_seed;
> > +   int seed;
> > +};
> > +
> >  struct plane_parms {
> > struct igt_fb *fb;
> > uint32_t width, height;
> > @@ -401,6 +409,28 @@ static void wait_for_transition(igt_display_t 
> > *display, enum pipe pipe, bool non
> > }
> >  }
> >  
> > +/* Copied from https://benpfaff.org/writings/clc/shuffle.html */
> > +static void shuffle_array(uint32_t *array, int size, int seed)
> > +{
> > +   int i;
> > +
> > +   for (i = 0; i < size; i++) {
> > +   int j = i + rand() / (RAND_MAX / (size - i) + 1);
> > +
> > +   igt_swap(array[i], array[j]);
> > +   }
> > +}
> I wouldn't worry about predictibility of the random number generator..
> 
> int j = i + rand() % (size - i) is good enough and easier to read. :)

Chris already suggested igt_permute_array(), will use that.

> 
> I think the struct test_config can be killed too, since it goes unused
> in shuffle_array, nothing in the test uses it...

Oops, some kind of left-over from an earlier version. Thanks for spotting
it.

> > +static void init_combination_array(uint32_t *array, int size, int seed)
> > +{
> > +   int i;
> > +
> > +   for (i = 0; i < size; i++)
> > +   array[i] = i;
> > +
> > +   shuffle_array(array, size, seed);
> > +}
> > +
> >  /*
> >   * 1. Set primary plane to a known fb.
> >   * 2. Make sure getcrtc returns the correct fb id.
> > @@ -411,19 +441,27 @@ static void wait_for_transition(igt_display_t 
> > *display, enum pipe pipe, bool non
> >   * so test this and make sure it works.
> >   */
> >  static void
> > -run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t 
> > *output,
> > -   enum transition_type type, bool nonblocking, bool fencing)
> > +run_transition_test(struct test_config *test_config, enum pipe pipe,
> > +   igt_output_t *output, enum transition_type type,
> > +   bool nonblocking, bool fencing)
> >  {
> > struct igt_fb fb, argb_fb, sprite_fb;
> > drmModeModeInfo *mode, override_mode;
> > +   igt_display_t *display = test_config->display;
> > igt_plane_t *plane;
> > igt_pipe_t *pipe_obj = >pipes[pipe];
> > uint32_t iter_max = 1 << pipe_obj->n_planes, i;
> > +   uint32_t *plane_combinations;
> > +   struct timespec start = { };
> > struct plane_parms parms[pipe_obj->n_planes];
> > bool skip_test = false;
> > unsigned flags = 0;
> > int ret;
> >  
> > +   plane_combinations = malloc(sizeof(*plane_combinations) * iter_max);
> > +   igt_assert(plane_combinations);
> > +   init_combination_array(plane_combinations, iter_max, test_config->seed);
> It would be cleaner to have a separate test for transition_modeset.
> The rest should be run to completion and don't need shuffling, since
> in normal cases, they'll never hit a timeout.

Do you mean type == TRANSITION_MODESET? There are already separate
subtests for that. Yes, can disable the timeout and shuffling for the
rest.

> So make a separate test for that, and perhaps also add a flag for
> disabling the timeout.

Ok.

> 
> > if (fencing)
> > prepare_fencing(display, pipe);
> > else
> > @@ -527,39 +565,59 @@ run_transition_test(igt_display_t *display, enum pipe 
> > pipe, igt_output_t *output
> > goto cleanup;
> > }
> >  
> > +   igt_nsec_elapsed();
> > +
> > for (i = 0; i < iter_max; i++) {
> > igt_output_set_pipe(output, pipe);
> >  
> > -   wm_setup_plane(display, pipe, i, parms, fencing);
> > +   wm_setup_plane(display, pipe, plane_combinations[i], parms,
> > +  fencing);
> >  
> > -   atomic_commit(display, pipe, flags, (void *)(unsigned long)i, 
> > fencing);
> > +   atomic_commit(display, pipe, flags,
> > + (void *)(unsigned long)plane_combinations[i],
> > +

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

2017-11-01 Thread Maarten Lankhorst

Op 31-10-17 om 14:44 schreef Imre Deak:
> Doing modeset on internal panels may have a considerable overhead due to
> the panel specific power sequencing delays. To avoid long test runtimes
> limit the runtime of each subtest. Randomize the plane/pipe combinations
> to preserve the test coverage on such panels at least over multiple test
> runs.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103334
> Cc: Maarten Lankhorst 
> Signed-off-by: Imre Deak 
> ---
>  tests/kms_atomic_transition.c | 175 
> --
>  1 file changed, 150 insertions(+), 25 deletions(-)
>
> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> index 4c295125..ac67fc3a 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -39,6 +39,14 @@
>  #define DRM_CAP_CURSOR_HEIGHT 0x9
>  #endif
>  
> +#define MAX_SUBTEST_DURATION_NS (20ULL * NSEC_PER_SEC)
> +
> +struct test_config {
> + igt_display_t *display;
> + bool user_seed;
> + int seed;
> +};
> +
>  struct plane_parms {
>   struct igt_fb *fb;
>   uint32_t width, height;
> @@ -401,6 +409,28 @@ static void wait_for_transition(igt_display_t *display, 
> enum pipe pipe, bool non
>   }
>  }
>  
> +/* Copied from https://benpfaff.org/writings/clc/shuffle.html */
> +static void shuffle_array(uint32_t *array, int size, int seed)
> +{
> + int i;
> +
> + for (i = 0; i < size; i++) {
> + int j = i + rand() / (RAND_MAX / (size - i) + 1);
> +
> + igt_swap(array[i], array[j]);
> + }
> +}
I wouldn't worry about predictibility of the random number generator..

int j = i + rand() % (size - i) is good enough and easier to read. :)

I think the struct test_config can be killed too, since it goes unused in 
shuffle_array, nothing in the test uses it...
> +static void init_combination_array(uint32_t *array, int size, int seed)
> +{
> + int i;
> +
> + for (i = 0; i < size; i++)
> + array[i] = i;
> +
> + shuffle_array(array, size, seed);
> +}
> +
>  /*
>   * 1. Set primary plane to a known fb.
>   * 2. Make sure getcrtc returns the correct fb id.
> @@ -411,19 +441,27 @@ static void wait_for_transition(igt_display_t *display, 
> enum pipe pipe, bool non
>   * so test this and make sure it works.
>   */
>  static void
> -run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t 
> *output,
> - enum transition_type type, bool nonblocking, bool fencing)
> +run_transition_test(struct test_config *test_config, enum pipe pipe,
> + igt_output_t *output, enum transition_type type,
> + bool nonblocking, bool fencing)
>  {
>   struct igt_fb fb, argb_fb, sprite_fb;
>   drmModeModeInfo *mode, override_mode;
> + igt_display_t *display = test_config->display;
>   igt_plane_t *plane;
>   igt_pipe_t *pipe_obj = >pipes[pipe];
>   uint32_t iter_max = 1 << pipe_obj->n_planes, i;
> + uint32_t *plane_combinations;
> + struct timespec start = { };
>   struct plane_parms parms[pipe_obj->n_planes];
>   bool skip_test = false;
>   unsigned flags = 0;
>   int ret;
>  
> + plane_combinations = malloc(sizeof(*plane_combinations) * iter_max);
> + igt_assert(plane_combinations);
> + init_combination_array(plane_combinations, iter_max, test_config->seed);
It would be cleaner to have a separate test for transition_modeset. The rest 
should be run to completion
and don't need shuffling, since in normal cases, they'll never hit a timeout.

So make a separate test for that, and perhaps also add a flag for disabling the 
timeout.

>   if (fencing)
>   prepare_fencing(display, pipe);
>   else
> @@ -527,39 +565,59 @@ run_transition_test(igt_display_t *display, enum pipe 
> pipe, igt_output_t *output
>   goto cleanup;
>   }
>  
> + igt_nsec_elapsed();
> +
>   for (i = 0; i < iter_max; i++) {
>   igt_output_set_pipe(output, pipe);
>  
> - wm_setup_plane(display, pipe, i, parms, fencing);
> + wm_setup_plane(display, pipe, plane_combinations[i], parms,
> +fencing);
>  
> - atomic_commit(display, pipe, flags, (void *)(unsigned long)i, 
> fencing);
> + atomic_commit(display, pipe, flags,
> +   (void *)(unsigned long)plane_combinations[i],
> +   fencing);
>   wait_for_transition(display, pipe, nonblocking, fencing);
>  
>   if (type == TRANSITION_MODESET_DISABLE) {
> + if (igt_nsec_elapsed() >= MAX_SUBTEST_DURATION_NS)
> + goto cleanup;
> +
>   igt_output_set_pipe(output, PIPE_NONE);
>  
>   wm_setup_plane(display, pipe, 0, parms, fencing);
>  
>   atomic_commit(display, pipe, flags, (void *)

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

2017-11-01 Thread Imre Deak

On Wed, Nov 01, 2017 at 10:48:50AM +, Chris Wilson wrote:
> Quoting Imre Deak (2017-11-01 09:56:22)
> > On Tue, Oct 31, 2017 at 10:23:25PM +, Chris Wilson wrote:
> > > Quoting Imre Deak (2017-10-31 13:44:47)
> > > > Doing modeset on internal panels may have a considerable overhead due to
> > > > the panel specific power sequencing delays. To avoid long test runtimes
> > > > limit the runtime of each subtest. Randomize the plane/pipe combinations
> > > > to preserve the test coverage on such panels at least over multiple test
> > > > runs.
> > > > 
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103334
> > > > Cc: Maarten Lankhorst 
> > > > Signed-off-by: Imre Deak 
> > > > ---
> > > >  tests/kms_atomic_transition.c | 175 
> > > > --
> > > >  1 file changed, 150 insertions(+), 25 deletions(-)
> > > > 
> > > > diff --git a/tests/kms_atomic_transition.c 
> > > > b/tests/kms_atomic_transition.c
> > > > index 4c295125..ac67fc3a 100644
> > > > --- a/tests/kms_atomic_transition.c
> > > > +++ b/tests/kms_atomic_transition.c
> > > > @@ -39,6 +39,14 @@
> > > >  #define DRM_CAP_CURSOR_HEIGHT 0x9
> > > >  #endif
> > > >  
> > > > +#define MAX_SUBTEST_DURATION_NS (20ULL * NSEC_PER_SEC)
> > > > +
> > > > +struct test_config {
> > > > +   igt_display_t *display;
> > > > +   bool user_seed;
> > > > +   int seed;
> > > > +};
> > > > +
> > > >  struct plane_parms {
> > > > struct igt_fb *fb;
> > > > uint32_t width, height;
> > > > @@ -401,6 +409,28 @@ static void wait_for_transition(igt_display_t 
> > > > *display, enum pipe pipe, bool non
> > > > }
> > > >  }
> > > >  
> > > > +/* Copied from https://benpfaff.org/writings/clc/shuffle.html */
> > > > +static void shuffle_array(uint32_t *array, int size, int seed)
> > > > +{
> > > > +   int i;
> > > > +
> > > > +   for (i = 0; i < size; i++) {
> > > > +   int j = i + rand() / (RAND_MAX / (size - i) + 1);
> > > > +
> > > > +   igt_swap(array[i], array[j]);
> > > > +   }
> > > > +}
> > > 
> > > igt_permute_array()
> > 
> > Thanks, will use that instead.
> > 
> > > Not saying anything, but I was told using CI for stochastic coverage was
> > > a flat no...
> > 
> > Ok, but it would only be the case for slow panels where the alternative
> > is not to run the test at all. And is this a problem if we can replay a
> > failing case with --seed?
> 
> If the purpose of CI is purely regression testing and not exploratory
> debugging, each PW run must be with the same seed as the CI_DRM run.
> 
> The seed must be clearly displayed in the results so that when comparing
> CI_DRM runs the flip-flop can be traced to the change in seed.

Adding Petri and Tomi. So I guess the question is if we want this in CI
and if it's a reasonable effort to implement it.

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

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

2017-11-01 Thread Chris Wilson

Quoting Imre Deak (2017-11-01 09:56:22)
> On Tue, Oct 31, 2017 at 10:23:25PM +, Chris Wilson wrote:
> > Quoting Imre Deak (2017-10-31 13:44:47)
> > > Doing modeset on internal panels may have a considerable overhead due to
> > > the panel specific power sequencing delays. To avoid long test runtimes
> > > limit the runtime of each subtest. Randomize the plane/pipe combinations
> > > to preserve the test coverage on such panels at least over multiple test
> > > runs.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103334
> > > Cc: Maarten Lankhorst 
> > > Signed-off-by: Imre Deak 
> > > ---
> > >  tests/kms_atomic_transition.c | 175 
> > > --
> > >  1 file changed, 150 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> > > index 4c295125..ac67fc3a 100644
> > > --- a/tests/kms_atomic_transition.c
> > > +++ b/tests/kms_atomic_transition.c
> > > @@ -39,6 +39,14 @@
> > >  #define DRM_CAP_CURSOR_HEIGHT 0x9
> > >  #endif
> > >  
> > > +#define MAX_SUBTEST_DURATION_NS (20ULL * NSEC_PER_SEC)
> > > +
> > > +struct test_config {
> > > +   igt_display_t *display;
> > > +   bool user_seed;
> > > +   int seed;
> > > +};
> > > +
> > >  struct plane_parms {
> > > struct igt_fb *fb;
> > > uint32_t width, height;
> > > @@ -401,6 +409,28 @@ static void wait_for_transition(igt_display_t 
> > > *display, enum pipe pipe, bool non
> > > }
> > >  }
> > >  
> > > +/* Copied from https://benpfaff.org/writings/clc/shuffle.html */
> > > +static void shuffle_array(uint32_t *array, int size, int seed)
> > > +{
> > > +   int i;
> > > +
> > > +   for (i = 0; i < size; i++) {
> > > +   int j = i + rand() / (RAND_MAX / (size - i) + 1);
> > > +
> > > +   igt_swap(array[i], array[j]);
> > > +   }
> > > +}
> > 
> > igt_permute_array()
> 
> Thanks, will use that instead.
> 
> > Not saying anything, but I was told using CI for stochastic coverage was
> > a flat no...
> 
> Ok, but it would only be the case for slow panels where the alternative
> is not to run the test at all. And is this a problem if we can replay a
> failing case with --seed?

If the purpose of CI is purely regression testing and not exploratory
debugging, each PW run must be with the same seed as the CI_DRM run.

The seed must be clearly displayed in the results so that when comparing
CI_DRM runs the flip-flop can be traced to the change in seed.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

2017-11-01 Thread Imre Deak

On Tue, Oct 31, 2017 at 10:23:25PM +, Chris Wilson wrote:
> Quoting Imre Deak (2017-10-31 13:44:47)
> > Doing modeset on internal panels may have a considerable overhead due to
> > the panel specific power sequencing delays. To avoid long test runtimes
> > limit the runtime of each subtest. Randomize the plane/pipe combinations
> > to preserve the test coverage on such panels at least over multiple test
> > runs.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103334
> > Cc: Maarten Lankhorst 
> > Signed-off-by: Imre Deak 
> > ---
> >  tests/kms_atomic_transition.c | 175 
> > --
> >  1 file changed, 150 insertions(+), 25 deletions(-)
> > 
> > diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> > index 4c295125..ac67fc3a 100644
> > --- a/tests/kms_atomic_transition.c
> > +++ b/tests/kms_atomic_transition.c
> > @@ -39,6 +39,14 @@
> >  #define DRM_CAP_CURSOR_HEIGHT 0x9
> >  #endif
> >  
> > +#define MAX_SUBTEST_DURATION_NS (20ULL * NSEC_PER_SEC)
> > +
> > +struct test_config {
> > +   igt_display_t *display;
> > +   bool user_seed;
> > +   int seed;
> > +};
> > +
> >  struct plane_parms {
> > struct igt_fb *fb;
> > uint32_t width, height;
> > @@ -401,6 +409,28 @@ static void wait_for_transition(igt_display_t 
> > *display, enum pipe pipe, bool non
> > }
> >  }
> >  
> > +/* Copied from https://benpfaff.org/writings/clc/shuffle.html */
> > +static void shuffle_array(uint32_t *array, int size, int seed)
> > +{
> > +   int i;
> > +
> > +   for (i = 0; i < size; i++) {
> > +   int j = i + rand() / (RAND_MAX / (size - i) + 1);
> > +
> > +   igt_swap(array[i], array[j]);
> > +   }
> > +}
> 
> igt_permute_array()

Thanks, will use that instead.

> Not saying anything, but I was told using CI for stochastic coverage was
> a flat no...

Ok, but it would only be the case for slow panels where the alternative
is not to run the test at all. And is this a problem if we can replay a
failing case with --seed?

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

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

2017-10-31 Thread Chris Wilson

Quoting Imre Deak (2017-10-31 13:44:47)
> Doing modeset on internal panels may have a considerable overhead due to
> the panel specific power sequencing delays. To avoid long test runtimes
> limit the runtime of each subtest. Randomize the plane/pipe combinations
> to preserve the test coverage on such panels at least over multiple test
> runs.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103334
> Cc: Maarten Lankhorst 
> Signed-off-by: Imre Deak 
> ---
>  tests/kms_atomic_transition.c | 175 
> --
>  1 file changed, 150 insertions(+), 25 deletions(-)
> 
> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
> index 4c295125..ac67fc3a 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -39,6 +39,14 @@
>  #define DRM_CAP_CURSOR_HEIGHT 0x9
>  #endif
>  
> +#define MAX_SUBTEST_DURATION_NS (20ULL * NSEC_PER_SEC)
> +
> +struct test_config {
> +   igt_display_t *display;
> +   bool user_seed;
> +   int seed;
> +};
> +
>  struct plane_parms {
> struct igt_fb *fb;
> uint32_t width, height;
> @@ -401,6 +409,28 @@ static void wait_for_transition(igt_display_t *display, 
> enum pipe pipe, bool non
> }
>  }
>  
> +/* Copied from https://benpfaff.org/writings/clc/shuffle.html */
> +static void shuffle_array(uint32_t *array, int size, int seed)
> +{
> +   int i;
> +
> +   for (i = 0; i < size; i++) {
> +   int j = i + rand() / (RAND_MAX / (size - i) + 1);
> +
> +   igt_swap(array[i], array[j]);
> +   }
> +}

igt_permute_array()

Not saying anything, but I was told using CI for stochastic coverage was
a flat no...
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

2017-10-31 Thread Imre Deak

Doing modeset on internal panels may have a considerable overhead due to
the panel specific power sequencing delays. To avoid long test runtimes
limit the runtime of each subtest. Randomize the plane/pipe combinations
to preserve the test coverage on such panels at least over multiple test
runs.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103334
Cc: Maarten Lankhorst 
Signed-off-by: Imre Deak 
---
 tests/kms_atomic_transition.c | 175 --
 1 file changed, 150 insertions(+), 25 deletions(-)

diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index 4c295125..ac67fc3a 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -39,6 +39,14 @@
 #define DRM_CAP_CURSOR_HEIGHT 0x9
 #endif
 
+#define MAX_SUBTEST_DURATION_NS (20ULL * NSEC_PER_SEC)
+
+struct test_config {
+   igt_display_t *display;
+   bool user_seed;
+   int seed;
+};
+
 struct plane_parms {
struct igt_fb *fb;
uint32_t width, height;
@@ -401,6 +409,28 @@ static void wait_for_transition(igt_display_t *display, 
enum pipe pipe, bool non
}
 }
 
+/* Copied from https://benpfaff.org/writings/clc/shuffle.html */
+static void shuffle_array(uint32_t *array, int size, int seed)
+{
+   int i;
+
+   for (i = 0; i < size; i++) {
+   int j = i + rand() / (RAND_MAX / (size - i) + 1);
+
+   igt_swap(array[i], array[j]);
+   }
+}
+
+static void init_combination_array(uint32_t *array, int size, int seed)
+{
+   int i;
+
+   for (i = 0; i < size; i++)
+   array[i] = i;
+
+   shuffle_array(array, size, seed);
+}
+
 /*
  * 1. Set primary plane to a known fb.
  * 2. Make sure getcrtc returns the correct fb id.
@@ -411,19 +441,27 @@ static void wait_for_transition(igt_display_t *display, 
enum pipe pipe, bool non
  * so test this and make sure it works.
  */
 static void
-run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t 
*output,
-   enum transition_type type, bool nonblocking, bool fencing)
+run_transition_test(struct test_config *test_config, enum pipe pipe,
+   igt_output_t *output, enum transition_type type,
+   bool nonblocking, bool fencing)
 {
struct igt_fb fb, argb_fb, sprite_fb;
drmModeModeInfo *mode, override_mode;
+   igt_display_t *display = test_config->display;
igt_plane_t *plane;
igt_pipe_t *pipe_obj = >pipes[pipe];
uint32_t iter_max = 1 << pipe_obj->n_planes, i;
+   uint32_t *plane_combinations;
+   struct timespec start = { };
struct plane_parms parms[pipe_obj->n_planes];
bool skip_test = false;
unsigned flags = 0;
int ret;
 
+   plane_combinations = malloc(sizeof(*plane_combinations) * iter_max);
+   igt_assert(plane_combinations);
+   init_combination_array(plane_combinations, iter_max, test_config->seed);
+
if (fencing)
prepare_fencing(display, pipe);
else
@@ -527,39 +565,59 @@ run_transition_test(igt_display_t *display, enum pipe 
pipe, igt_output_t *output
goto cleanup;
}
 
+   igt_nsec_elapsed();
+
for (i = 0; i < iter_max; i++) {
igt_output_set_pipe(output, pipe);
 
-   wm_setup_plane(display, pipe, i, parms, fencing);
+   wm_setup_plane(display, pipe, plane_combinations[i], parms,
+  fencing);
 
-   atomic_commit(display, pipe, flags, (void *)(unsigned long)i, 
fencing);
+   atomic_commit(display, pipe, flags,
+ (void *)(unsigned long)plane_combinations[i],
+ fencing);
wait_for_transition(display, pipe, nonblocking, fencing);
 
if (type == TRANSITION_MODESET_DISABLE) {
+   if (igt_nsec_elapsed() >= MAX_SUBTEST_DURATION_NS)
+   goto cleanup;
+
igt_output_set_pipe(output, PIPE_NONE);
 
wm_setup_plane(display, pipe, 0, parms, fencing);
 
atomic_commit(display, pipe, flags, (void *) 0UL, 
fencing);
wait_for_transition(display, pipe, nonblocking, 
fencing);
+
} else {
uint32_t j;
 
/* i -> i+1 will be done when i increases, can be 
skipped here */
for (j = iter_max - 1; j > i + 1; j--) {
-   wm_setup_plane(display, pipe, j, parms, 
fencing);
+   if (igt_nsec_elapsed() >= 
MAX_SUBTEST_DURATION_NS)
+   goto cleanup;
+
+   wm_setup_plane(display, pipe,
+  plane_combinations[j], parms,
+

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

Re: [Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

[Intel-gfx] [PATCH i-g-t] kms_atomic_transition: Add subtest time limit/randomize plane, pipe combinations

9 matches

Site Navigation

Mail list logo

Footer information