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

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Mon, 2019-04-01 at 16:04 -0700, Souza, Jose wrote:
> On Mon, 2019-04-01 at 14:23 +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)
> 
> I can't see how parms[i].mask can be 0, it always be one of: 1 << (0,
> 1, 2 or 3).
> 

You are right, it can't be zero. What happens is that we get
a such mask parameter that parms[i].mask & mask expression happens to
be 0 for all planes and then wait_for_transition asserts onfd_completed. 
I also tried this manually, if I call wm_setup_plane with mask
parameter set as 0 and then wait_for_transition, I get this assert
constantly. So I guess checking that should not harm anyway.

> > 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);
> >  
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev