Re: [igt-dev] [PATCH i-g-t v5 1/2] igt/tests/kms_atomic_transition: Skip transition, if no changes done

["Souza, Jose" <jose.souza@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4160 bytes --]

On Thu, 2019-04-04 at 15:13 +0300, Stanislav Lisovskiy wrote:
> While fixing used amount of planes, discovered that if
> wm_setup_plane is called with 0 planes(might happen during
> main testing cycle, as parms[i].mask can be 0 due to randomization)

As already said and you agreed this 'as parms[i].mask can be 0 due to
randomization' statement is not true, even if this patch is right the
commit message needs to be true.

For me this patch is only hidden the real bug.

> then subsequent wait_transition fails in assertion on fd_completed.
> So added return value to wm_setup_plane, which would allow to
> determine, if we need to skip this step.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  tests/kms_atomic_transition.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/kms_atomic_transition.c
> b/tests/kms_atomic_transition.c
> index 18f73317..638fe17e 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -118,11 +118,12 @@ static void configure_fencing(igt_plane_t
> *plane)
>  	igt_assert_eq(ret, 0);
>  }
>  
> -static void
> +static int
>  wm_setup_plane(igt_display_t *display, enum pipe pipe,
>  	       uint32_t mask, struct plane_parms *parms, bool fencing)
>  {
>  	igt_plane_t *plane;
> +	int planes_set_up = 0;
>  
>  	/*
>  	* Make sure these buffers are suited for display use
> @@ -133,8 +134,10 @@ wm_setup_plane(igt_display_t *display, enum pipe
> pipe,
>  		int i = plane->index;
>  
>  		if (!mask || !(parms[i].mask & mask)) {
> -			if (plane->values[IGT_PLANE_FB_ID])
> +			if (plane->values[IGT_PLANE_FB_ID]) {
>  				igt_plane_set_fb(plane, NULL);
> +				planes_set_up++;
> +			}
>  			continue;
>  		}
>  
> @@ -144,7 +147,10 @@ wm_setup_plane(igt_display_t *display, enum pipe
> pipe,
>  		igt_plane_set_fb(plane, parms[i].fb);
>  		igt_fb_set_size(parms[i].fb, plane, parms[i].width,
> parms[i].height);
>  		igt_plane_set_size(plane, parms[i].width,
> parms[i].height);
> +
> +		planes_set_up++;
>  	}
> +	return planes_set_up;
>  }
>  
>  static void ev_page_flip(int fd, unsigned seq, unsigned tv_sec,
> unsigned tv_usec, void *user_data)
> @@ -544,7 +550,8 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
>  
>  		igt_output_set_pipe(output, pipe);
>  
> -		wm_setup_plane(display, pipe, i, parms, fencing);
> +		if (!wm_setup_plane(display, pipe, i, parms, fencing))
> +			continue;
>  
>  		atomic_commit(display, pipe, flags, (void *)(unsigned
> long)i, fencing);
>  		wait_for_transition(display, pipe, nonblocking,
> fencing);
> @@ -552,7 +559,8 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
>  		if (type == TRANSITION_MODESET_DISABLE) {
>  			igt_output_set_pipe(output, PIPE_NONE);
>  
> -			wm_setup_plane(display, pipe, 0, parms,
> fencing);
> +			if (!wm_setup_plane(display, pipe, 0, parms,
> fencing))
> +				continue;
>  
>  			atomic_commit(display, pipe, flags, (void *)
> 0UL, fencing);
>  			wait_for_transition(display, pipe, nonblocking,
> fencing);
> @@ -568,7 +576,8 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
>  				    n_enable_planes < pipe_obj-
> >n_planes)
>  					continue;
>  
> -				wm_setup_plane(display, pipe, j, parms,
> fencing);
> +				if (!wm_setup_plane(display, pipe, j,
> parms, fencing))
> +					continue;
>  
>  				if (type >= TRANSITION_MODESET)
>  					igt_output_override_mode(output
> , &override_mode);
> @@ -576,7 +585,9 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
>  				atomic_commit(display, pipe, flags,
> (void *)(unsigned long) j, fencing);
>  				wait_for_transition(display, pipe,
> nonblocking, fencing);
>  
> -				wm_setup_plane(display, pipe, i, parms,
> fencing);
> +				if (!wm_setup_plane(display, pipe, i,
> parms, fencing))
> +					continue;
> +
>  				if (type >= TRANSITION_MODESET)
>  					igt_output_override_mode(output
> , NULL);
>  

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev