Re: [PATCH i-g-t] kms_cursor_crc: Move igt_pipe_crc_{new, free} to init

[Matt Roper <matthew.d.roper@intel.com>]

On Thu, May 22, 2014 at 08:01:09PM +0300, Ville Syrjälä wrote:
> On Thu, May 22, 2014 at 09:47:39AM -0700, Matt Roper wrote:
> > If a subtest fails, cleanup_crtc() never gets called.  Currently that
> > also causes igt_pipe_crc_free() to never be called, leading all
> > subsequent subtests to also fail with -EBUSY at igt_pipe_crc_new().
> > Move the calls to igt_pipe_crc_{new,free} into igt_main so that
> > we don't need to worry about closing and reopening the CRC
> > filedescriptor for each subtest.
> 
> IIRC we can't call them at init because when you call igt_pipe_crc_new()
> the pipe->port mapping has to be properly set up so that the auto crc
> source will know what to do.
> 
> The subtest failure case is the reason why at the start of every subtest
> we call igt_pipe_crc_free() before calling igt_pipe_crc_new(). Are you
> saying that's not working as intended?

Right, you're working on a fresh test_data structure for each call of
run_test(), so you've already lost the test_data->pipe_crc pointer that
you're trying to check for NULL there.

I guess the right fix is to move the pipe_crc pointer up into data_t
rather than test_data_t?


Matt

> 
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  tests/kms_cursor_crc.c | 21 ++++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > index 06625ee..1b90baa 100644
> > --- a/tests/kms_cursor_crc.c
> > +++ b/tests/kms_cursor_crc.c
> > @@ -39,11 +39,14 @@
> >  #define DRM_CAP_CURSOR_HEIGHT 0x9
> >  #endif
> >  
> > +#define MAX_PIPES 3
> > +
> >  typedef struct {
> >  	int drm_fd;
> >  	igt_display_t display;
> >  	struct igt_fb primary_fb;
> >  	struct igt_fb fb;
> > +	igt_pipe_crc_t *pipe_crcs[MAX_PIPES];
> >  } data_t;
> >  
> >  typedef struct {
> > @@ -251,12 +254,6 @@ static bool prepare_crtc(test_data_t *test_data, igt_output_t *output,
> >  
> >  	igt_display_commit(display);
> >  
> > -	/* create the pipe_crc object for this pipe */
> > -	if (test_data->pipe_crc)
> > -		igt_pipe_crc_free(test_data->pipe_crc);
> > -
> > -	test_data->pipe_crc = igt_pipe_crc_new(test_data->pipe,
> > -					       INTEL_PIPE_CRC_SOURCE_AUTO);
> >  	if (!test_data->pipe_crc) {
> >  		igt_info("auto crc not supported on this connector with pipe %i\n",
> >  			 test_data->pipe);
> > @@ -289,7 +286,6 @@ static void cleanup_crtc(test_data_t *test_data, igt_output_t *output)
> >  	igt_display_t *display = &data->display;
> >  	igt_plane_t *primary;
> >  
> > -	igt_pipe_crc_free(test_data->pipe_crc);
> >  	test_data->pipe_crc = NULL;
> >  
> >  	igt_remove_fb(data->drm_fd, &data->primary_fb);
> > @@ -315,6 +311,7 @@ static void run_test(data_t *data, void (*testfunc)(test_data_t *), int cursor_w
> >  		test_data.output = output;
> >  		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
> >  			test_data.pipe = p;
> > +			test_data.pipe_crc = data->pipe_crcs[p];
> >  
> >  			if (!prepare_crtc(&test_data, output, cursor_w, cursor_h))
> >  				continue;
> > @@ -385,6 +382,7 @@ igt_main
> >  	uint64_t cursor_width = 64, cursor_height = 64;
> >  	data_t data = {};
> >  	int ret;
> > +	int p;
> >  
> >  	igt_skip_on_simulation();
> >  
> > @@ -405,11 +403,20 @@ igt_main
> >  		igt_require_pipe_crc();
> >  
> >  		igt_display_init(&data.display, data.drm_fd);
> > +
> > +		for (p = 0; p < igt_display_get_n_pipes(&data.display); p++)
> > +			data.pipe_crcs[p] = igt_pipe_crc_new(p,
> > +							     INTEL_PIPE_CRC_SOURCE_AUTO);
> >  	}
> >  
> >  	run_test_generic(&data, cursor_width);
> >  
> >  	igt_fixture {
> > +		for (p = 0; p < igt_display_get_n_pipes(&data.display); p++) {
> > +			if (data.pipe_crcs[p])
> > +				igt_pipe_crc_free(data.pipe_crcs[p]);
> > +		}
> > +
> >  		igt_display_fini(&data.display);
> >  	}
> >  }
> > -- 
> > 1.8.5.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795