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

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

 



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 <maarten.lankhorst@xxxxxxxxxxxxxxx>
>>> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
>>> ---
>>>  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 = &display->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(&start);
>>> +
>>>  	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(&start) >= 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(&start) >= MAX_SUBTEST_DURATION_NS)
>>> +					goto cleanup;
>>> +
>>> +				wm_setup_plane(display, pipe,
>>> +					       plane_combinations[j], parms,
>>> +					       fencing);
>>>  
>>>  				if (type == TRANSITION_MODESET)
>>>  					igt_output_override_mode(output, &override_mode);
>>>  
>>> -				atomic_commit(display, pipe, flags, (void *)(unsigned long) j, fencing);
>>> +				atomic_commit(display, pipe, flags,
>>> +					      (void *)(unsigned long)plane_combinations[j],
>>> +					      fencing);
>>>  				wait_for_transition(display, pipe, nonblocking, fencing);
>>>  
>>> -				wm_setup_plane(display, pipe, i, parms, fencing);
>>> +				wm_setup_plane(display, pipe,
>>> +					       plane_combinations[i], parms,
>>> +					       fencing);
>>>  				if (type == TRANSITION_MODESET)
>>>  					igt_output_override_mode(output, NULL);
>>>  
>>> -				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);
>>>  			}
>>>  		}
>>> @@ -579,6 +637,9 @@ cleanup:
>>>  	igt_remove_fb(display->drm_fd, &fb);
>>>  	igt_remove_fb(display->drm_fd, &argb_fb);
>>>  	igt_remove_fb(display->drm_fd, &sprite_fb);
>>> +
>>> +	free(plane_combinations);
>>> +
>>>  	if (skip_test)
>>>  		igt_skip("Atomic nonblocking modesets are not supported.\n");
>>>  }
>>> @@ -696,16 +757,24 @@ static void collect_crcs_mask(igt_pipe_crc_t **pipe_crcs, unsigned mask, igt_crc
>>>  	}
>>>  }
>>>  
>>> -static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblocking, bool fencing)
>>> +static void run_modeset_tests(struct test_config *test_config, int howmany,
>>> +			      bool nonblocking, bool fencing)
>>>  {
>>> +	igt_display_t *display = test_config->display;
>>>  	struct igt_fb fbs[2];
>>>  	int i, j;
>>>  	unsigned iter_max = 1 << display->n_pipes;
>>> +	uint32_t *pipe_combinations;
>>> +	struct timespec start = { };
>>>  	igt_pipe_crc_t *pipe_crcs[IGT_MAX_PIPES] = { 0 };
>>>  	igt_output_t *output;
>>>  	unsigned width = 0, height = 0;
>>>  	bool skip_test = false;
>>>  
>>> +	pipe_combinations = malloc(sizeof(*pipe_combinations) * iter_max);
>>> +	igt_assert(pipe_combinations);
>>> +	init_combination_array(pipe_combinations, iter_max, test_config->seed);
>> Kill all the changes to run_modeset_tests too please? The randomness
>> here goes unused, and I would rather have it run all the modesetting
>> combinations.  I can understand plane transitions taking too long, but
>> the modeset tests are typically very short.
> IIUC with 3 pipes it's 27 iterations, so with 2 full modesets per iteration it
> can take ~1 minute using slow panels. The same with 4 pipes would take ~4
> minutes.
I'm ok with that. :)
_______________________________________________
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