Re: [PATCH i-g-t v2 05/15] tests/kms: Clean up more users of unassigned pipes.

Ander Conselvan De Oliveira
2016-07-20T12:56:16Z

On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
> Use for_each_pipe_with_valid_output instead.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  tests/kms_crtc_background_color.c |  3 +--
>  tests/kms_flip_tiling.c           | 50 +++++++++++++++++++++++---------------
> -
>  tests/kms_panel_fitting.c         |  5 ++--
>  tests/kms_plane_scaling.c         |  5 ++--
>  4 files changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/tests/kms_crtc_background_color.c
> b/tests/kms_crtc_background_color.c
> index b496625c1693..b97c1142df6e 100644
> --- a/tests/kms_crtc_background_color.c
> +++ b/tests/kms_crtc_background_color.c
> @@ -133,10 +133,9 @@ static void test_crtc_background(data_t *data)
>
>   igt_require(data->display.has_universal_planes);
>
> - for_each_connected_output(display, output) {
> + for_each_pipe_with_valid_output(display, pipe, output) {
>   igt_plane_t *plane;
>
> - pipe = output->config.pipe;
>   igt_output_set_pipe(output, pipe);

Won't this cause the test to be executed more times? I.e., every output paired
with every supported pipe vs. every output only with the pipe it is currently
configured to use.

None of the changed tests are part of BAT, so I'm not sure we care about a
potential increase in execution time, though. But maybe add a note in the commit
message for future reference?

Ander

>
>   plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
> index f58e65be61ee..cd2f510f1d45 100644
> --- a/tests/kms_flip_tiling.c
> +++ b/tests/kms_flip_tiling.c
> @@ -80,16 +80,15 @@ static void wait_for_pageflip(int fd)
>  }
>
>  static void
> -test_flip_tiling(data_t *data, igt_output_t *output, uint64_t tiling[2])
> +test_flip_tiling(data_t *data, enum pipe pipe, igt_output_t *output, uint64_t
> tiling[2])
>  {
>   drmModeModeInfo *mode;
>   igt_plane_t *primary;
>   struct igt_fb fb[2];
>   igt_pipe_crc_t *pipe_crc;
>   igt_crc_t reference_crc, crc;
> - int fb_id, pipe, ret, width;
> + int fb_id, ret, width;
>
> - pipe = output->config.pipe;
>   pipe_crc = pipe_crc_new(pipe);
>   igt_output_set_pipe(output, pipe);
>
> @@ -184,31 +183,34 @@ igt_main
>   igt_subtest_f("flip-changes-tiling") {
>   uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_X_TILED,
>          LOCAL_DRM_FORMAT_MOD_NONE };
> + enum pipe pipe;
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
>   }
>
>   igt_subtest_f("flip-changes-tiling-Y") {
>   uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Y_TILED,
>          LOCAL_DRM_FORMAT_MOD_NONE };
> + enum pipe pipe;
>
>   igt_require_fb_modifiers(data.drm_fd);
>   igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
>   }
>
>   igt_subtest_f("flip-changes-tiling-Yf") {
>   uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Yf_TILED,
>          LOCAL_DRM_FORMAT_MOD_NONE };
> + enum pipe pipe;
>
>   igt_require_fb_modifiers(data.drm_fd);
>   igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
>   }
>
>   /*
> @@ -222,31 +224,34 @@ igt_main
>   igt_subtest_f("flip-X-tiled") {
>   uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_X_TILED,
>          LOCAL_I915_FORMAT_MOD_X_TILED };
> + enum pipe pipe;
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
>   }
>
>   igt_subtest_f("flip-Y-tiled") {
>   uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Y_TILED,
>          LOCAL_I915_FORMAT_MOD_Y_TILED };
> + enum pipe pipe;
>
>   igt_require_fb_modifiers(data.drm_fd);
>   igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
>   }
>
>   igt_subtest_f("flip-Yf-tiled") {
>   uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Yf_TILED,
>          LOCAL_I915_FORMAT_MOD_Yf_TILED };
> + enum pipe pipe;
>
>   igt_require_fb_modifiers(data.drm_fd);
>   igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
>   }
>
>   /*
> @@ -260,31 +265,34 @@ igt_main
>   igt_subtest_f("flip-to-X-tiled") {
>   uint64_t tiling[2] = { LOCAL_DRM_FORMAT_MOD_NONE,
>          LOCAL_I915_FORMAT_MOD_X_TILED };
> + enum pipe pipe;
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
>   }
>
>   igt_subtest_f("flip-to-Y-tiled") {
>   uint64_t tiling[2] = { LOCAL_DRM_FORMAT_MOD_NONE,
>          LOCAL_I915_FORMAT_MOD_Y_TILED };
> + enum pipe pipe;
>
>   igt_require_fb_modifiers(data.drm_fd);
>   igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
>   }
>
>   igt_subtest_f("flip-to-Yf-tiled") {
>   uint64_t tiling[2] = { LOCAL_DRM_FORMAT_MOD_NONE,
>          LOCAL_I915_FORMAT_MOD_Yf_TILED };
> + enum pipe pipe;
>
>   igt_require_fb_modifiers(data.drm_fd);
>   igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
>   }
>
>   igt_fixture {
> diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
> index b796c6812679..7c501fcdd3fb 100644
> --- a/tests/kms_panel_fitting.c
> +++ b/tests/kms_panel_fitting.c
> @@ -87,7 +87,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output,
> enum pipe pipe,
>   if (s == COMMIT_LEGACY) {
>   int ret;
>   ret = drmModeSetCrtc(data->drm_fd,
> - output->config.crtc->crtc_id,
> + plane->pipe->crtc_id,
>   data->fb_id1,
>   plane->pan_x, plane->pan_y,
>   &output->id,
> @@ -137,7 +137,7 @@ static void test_panel_fitting(data_t *d)
>   enum pipe pipe;
>   int valid_tests = 0;
>
> - for_each_connected_output(display, output) {
> + for_each_pipe_with_valid_output(display, pipe, output) {
>   drmModeModeInfo *mode, native_mode;
>   bool scaling_mode_set;
>
> @@ -153,7 +153,6 @@ static void test_panel_fitting(data_t *d)
>   if (!scaling_mode_set)
>   continue;
>
> - pipe = output->config.pipe;
>   igt_output_set_pipe(output, pipe);
>
>   mode = igt_output_get_mode(output);
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index ad5404d90bfa..39bb5e113411 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -96,7 +96,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output,
> enum pipe pipe,
>   if (s == COMMIT_LEGACY) {
>   int ret;
>   ret = drmModeSetCrtc(data->drm_fd,
> - output->config.crtc->crtc_id,
> + plane->pipe->crtc_id,
>   data->fb_id1,
>   plane->pan_x, plane->pan_y,
>   &output->id,
> @@ -186,10 +186,9 @@ static void test_plane_scaling(data_t *d)
>   igt_require(d->display.has_universal_planes);
>   igt_require(d->num_scalers);
>
> - for_each_connected_output(display, output) {
> + for_each_pipe_with_valid_output(display, pipe, output) {
>   drmModeModeInfo *mode;
>
> - pipe = output->config.pipe;
>   igt_output_set_pipe(output, pipe);
>
>   mode = igt_output_get_mode(output);