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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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?

> 
> 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