[igt-dev] [PATCH i-g-t 0/2] test/kms: Add support for display with non-contiguous pipes

[igt-dev] [PATCH i-g-t 0/2] test/kms: Add support for display with non-contiguous pipes

[Mohammed Khajapasha]

add support for display with non-contiguous pipes.

Mohammed Khajapasha (2):
  lib/igt_kms: Set pipe enum name to a pipe from drm pipe
  test/kms: In static iteration skip the testcases for disable pipes

 lib/igt_core.h              | 22 ++++++++++++++++++++++
 lib/igt_kms.c               | 13 +++++++++----
 tests/kms_color.c           |  2 +-
 tests/kms_color_chamelium.c |  2 +-
 tests/kms_concurrent.c      |  2 +-
 tests/kms_cursor_legacy.c   |  2 +-
 tests/kms_pipe_crc_basic.c  |  2 +-
 tests/kms_plane.c           |  2 +-
 tests/kms_plane_cursor.c    |  3 +++
 tests/kms_plane_lowres.c    |  2 +-
 tests/kms_plane_multiple.c  |  2 +-
 tests/kms_universal_plane.c |  2 +-
 12 files changed, 43 insertions(+), 13 deletions(-)

-- 
2.24.1

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

[igt-dev] [PATCH i-g-t 1/2] lib/igt_kms: Set pipe enum name to a pipe from drm pipe

[Mohammed Khajapasha]

set the pipe enum name to igt pipe from drm pipe,
in case of non-contiguous pipes in display, the igt pipes cann't
be same as enabled pipes in kernel.

Signed-off-by: Mohammed Khajapasha <mohammed.khajapasha@intel.com>
---
 lib/igt_kms.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index afef5939..a42a44c5 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1958,10 +1958,16 @@ void igt_display_require(igt_display_t *display, int drm_fd)
 		int p = 1;
 		int j, type;
 		uint8_t last_plane = 0, n_planes = 0;
+		struct drm_i915_get_pipe_from_crtc_id get_pipe;
 
 		pipe->crtc_id = resources->crtcs[i];
 		pipe->display = display;
-		pipe->pipe = i;
+		/* Get right pipe enum from kernel for a pipe */
+		get_pipe.pipe = 0;
+		get_pipe.crtc_id =  pipe->crtc_id;
+		do_ioctl(display->drm_fd,
+				DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, &get_pipe);
+		pipe->pipe = get_pipe.pipe;
 		pipe->plane_cursor = -1;
 		pipe->plane_primary = -1;
 		pipe->planes = NULL;
-- 
2.24.1

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

[igt-dev] [PATCH i-g-t 2/2] test/kms: In static iteration skip the testcases for disable pipes

[Mohammed Khajapasha]

In non-contiguous display pipes, the static iteration of pipes
in the testcases can cause iteration over disabled pipes and
segmentation fault, hence skip the testcase for disable pipes.

Signed-off-by: Mohammed Khajapasha <mohammed.khajapasha@intel.com>
---
 lib/igt_core.h              | 22 ++++++++++++++++++++++
 lib/igt_kms.c               |  5 ++---
 tests/kms_color.c           |  2 +-
 tests/kms_color_chamelium.c |  2 +-
 tests/kms_concurrent.c      |  2 +-
 tests/kms_cursor_legacy.c   |  2 +-
 tests/kms_pipe_crc_basic.c  |  2 +-
 tests/kms_plane.c           |  2 +-
 tests/kms_plane_cursor.c    |  3 +++
 tests/kms_plane_lowres.c    |  2 +-
 tests/kms_plane_multiple.c  |  2 +-
 tests/kms_universal_plane.c |  2 +-
 12 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/lib/igt_core.h b/lib/igt_core.h
index c5871520..2cd483e8 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -987,6 +987,28 @@ void igt_describe_f(const char *fmt, ...);
 	else igt_debug("Test requirement passed: %s\n", #expr); \
 } while (0)
 
+/**
+* igt_require_pipe:
+* @display: pointer to igt_display_t
+* @pipe: pipe which need to check
+*
+* Skip a (sub-)test if the pipe not enabled.
+*
+* Should be used everywhere where a test checks pipe and skip
+* test when pipe is not enabled.
+*/
+#define igt_require_pipe(display, p)	\
+	do { \
+		bool expr = 0; \
+		for (int n = 0; n < igt_display_get_n_pipes(display); n++) { \
+			if (((display)->pipes[(n)].pipe) == p) { \
+				expr = true; \
+				break;	\
+			} \
+		}	\
+		igt_skip_on_f(!expr, "Pipe %s does not exist.\n", kmstest_pipe_name(p)); \
+	} while (0)
+
 /**
  * igt_skip_on:
  * @expr: condition to test
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index a42a44c5..397127e0 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2136,8 +2136,7 @@ void igt_display_require_output_on_pipe(igt_display_t *display, enum pipe pipe)
 {
 	igt_output_t *output;
 
-	igt_skip_on_f(pipe >= igt_display_get_n_pipes(display),
-		      "Pipe %s does not exist.\n", kmstest_pipe_name(pipe));
+	igt_require_pipe(display, pipe);
 
 	for_each_valid_output_on_pipe(display, pipe, output)
 		return;
@@ -2486,7 +2485,7 @@ igt_output_t *igt_get_single_output_for_pipe(igt_display_t *display, enum pipe p
 	igt_output_t *chosen_outputs[display->n_pipes];
 
 	igt_assert(pipe != PIPE_NONE);
-	igt_require(pipe < display->n_pipes);
+	igt_require_pipe(display, pipe);
 
 	__igt_pipe_populate_outputs(display, chosen_outputs);
 
diff --git a/tests/kms_color.c b/tests/kms_color.c
index 7f2fbd4a..8c50ee66 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -628,7 +628,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 	igt_fixture {
 		igt_require_pipe_crc(data->drm_fd);
 
-		igt_require(p < data->display.n_pipes);
+		igt_require_pipe(&data->display, p);
 
 		pipe = &data->display.pipes[p];
 		igt_require(pipe->n_planes >= 0);
diff --git a/tests/kms_color_chamelium.c b/tests/kms_color_chamelium.c
index 7f5a911c..310c021a 100644
--- a/tests/kms_color_chamelium.c
+++ b/tests/kms_color_chamelium.c
@@ -519,7 +519,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 
 	igt_fixture {
 
-		igt_require(p < data->display.n_pipes);
+		igt_require_pipe(&data->display, p);
 
 		pipe = &data->display.pipes[p];
 		igt_require(pipe->n_planes >= 0);
diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
index 89016563..f332d332 100644
--- a/tests/kms_concurrent.c
+++ b/tests/kms_concurrent.c
@@ -320,7 +320,7 @@ run_tests_for_pipe(data_t *data, enum pipe pipe)
 	igt_fixture {
 		int valid_tests = 0;
 
-		igt_skip_on(pipe >= data->display.n_pipes);
+		igt_require_pipe(&data->display, pipe);
 		igt_require(data->display.pipes[pipe].n_planes > 0);
 
 		for_each_valid_output_on_pipe(&data->display, pipe, output)
diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
index 9306e54b..f544a6c9 100644
--- a/tests/kms_cursor_legacy.c
+++ b/tests/kms_cursor_legacy.c
@@ -1389,7 +1389,7 @@ igt_main
 			errno = 0;
 
 			igt_fixture {
-				igt_skip_on(n >= display.n_pipes);
+				igt_require_pipe(&display, n);
 			}
 
 			igt_subtest_f("pipe-%s-single-bo", kmstest_pipe_name(n))
diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index d169b7bd..ae6cb3e9 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -71,7 +71,7 @@ static void test_read_crc(data_t *data, enum pipe pipe, unsigned flags)
 	igt_crc_t *crcs = NULL;
 	int c, j;
 
-	igt_skip_on(pipe >= data->display.n_pipes);
+	igt_require_pipe(display, pipe);
 	igt_require_f(output, "No connector found for pipe %s\n",
 		      kmstest_pipe_name(pipe));
 
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index c6ead813..7d3f95d1 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -940,7 +940,7 @@ static void
 run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
 {
 	igt_fixture {
-		igt_skip_on(pipe >= data->display.n_pipes);
+		igt_require_pipe(&data->display, pipe);
 		igt_require(data->display.pipes[pipe].n_planes > 0);
 	}
 
diff --git a/tests/kms_plane_cursor.c b/tests/kms_plane_cursor.c
index adcdf5e8..4ab10204 100644
--- a/tests/kms_plane_cursor.c
+++ b/tests/kms_plane_cursor.c
@@ -317,6 +317,9 @@ igt_main
 	}
 
 	for_each_pipe_static(pipe)
+		igt_fixture {
+			igt_require_pipe(&data.display, pipe);
+		}
 		for (i = 0; i < ARRAY_SIZE(cursor_sizes); ++i) {
 			int size = cursor_sizes[i];
 
diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
index 012b25e3..a1f1fade 100644
--- a/tests/kms_plane_lowres.c
+++ b/tests/kms_plane_lowres.c
@@ -259,7 +259,7 @@ test_planes_on_pipe(data_t *data, uint64_t modifier)
 	igt_plane_t *plane;
 	unsigned tested = 0;
 
-	igt_skip_on(data->pipe >= data->display.n_pipes);
+	igt_require_pipe(&data->display, data->pipe);
 	igt_display_require_output_on_pipe(&data->display, data->pipe);
 	igt_skip_on(!igt_display_has_format_mod(&data->display,
 						DRM_FORMAT_XRGB8888, modifier));
diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
index 6cf060b3..520ec1fe 100644
--- a/tests/kms_plane_multiple.c
+++ b/tests/kms_plane_multiple.c
@@ -378,7 +378,7 @@ static void
 run_tests_for_pipe(data_t *data, enum pipe pipe)
 {
 	igt_fixture {
-		igt_skip_on(pipe >= data->display.n_pipes);
+		igt_require_pipe(&data->display, pipe);
 		igt_require(data->display.pipes[pipe].n_planes > 0);
 	}
 
diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c
index 676be633..cba54c4c 100644
--- a/tests/kms_universal_plane.c
+++ b/tests/kms_universal_plane.c
@@ -750,7 +750,7 @@ run_tests_for_pipe(data_t *data, enum pipe pipe)
 	igt_fixture {
 		int valid_tests = 0;
 
-		igt_skip_on(pipe >= data->display.n_pipes);
+		igt_require_pipe(&data->display, pipe);
 
 		for_each_valid_output_on_pipe(&data->display, pipe, output)
 			valid_tests++;
-- 
2.24.1

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

[igt-dev] [PATCH i-g-t 1/2] lib/igt_kms: Set pipe enum name to a pipe from drm pipe

[Mohammed Khajapasha]

set the pipe enum name to igt pipe from drm pipe,
in case of non-contiguous pipes in display, the igt pipes cann't
be same as enabled pipes in kernel.

Signed-off-by: Mohammed Khajapasha <mohammed.khajapasha@intel.com>
---
 lib/igt_kms.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index afef5939..a42a44c5 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1958,10 +1958,16 @@ void igt_display_require(igt_display_t *display, int drm_fd)
 		int p = 1;
 		int j, type;
 		uint8_t last_plane = 0, n_planes = 0;
+		struct drm_i915_get_pipe_from_crtc_id get_pipe;
 
 		pipe->crtc_id = resources->crtcs[i];
 		pipe->display = display;
-		pipe->pipe = i;
+		/* Get right pipe enum from kernel for a pipe */
+		get_pipe.pipe = 0;
+		get_pipe.crtc_id =  pipe->crtc_id;
+		do_ioctl(display->drm_fd,
+				DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, &get_pipe);
+		pipe->pipe = get_pipe.pipe;
 		pipe->plane_cursor = -1;
 		pipe->plane_primary = -1;
 		pipe->planes = NULL;
-- 
2.24.1

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

Re: [igt-dev] [PATCH i-g-t 1/2] lib/igt_kms: Set pipe enum name to a pipe from drm pipe

[Ville Syrjälä]

On Thu, Jun 04, 2020 at 02:35:05AM +0530, Mohammed Khajapasha wrote:
> set the pipe enum name to igt pipe from drm pipe,
> in case of non-contiguous pipes in display, the igt pipes cann't
> be same as enabled pipes in kernel.
> 
> Signed-off-by: Mohammed Khajapasha <mohammed.khajapasha@intel.com>
> ---
>  lib/igt_kms.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index afef5939..a42a44c5 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1958,10 +1958,16 @@ void igt_display_require(igt_display_t *display, int drm_fd)
>  		int p = 1;
>  		int j, type;
>  		uint8_t last_plane = 0, n_planes = 0;
> +		struct drm_i915_get_pipe_from_crtc_id get_pipe;
>  
>  		pipe->crtc_id = resources->crtcs[i];
>  		pipe->display = display;
> -		pipe->pipe = i;
> +		/* Get right pipe enum from kernel for a pipe */
> +		get_pipe.pipe = 0;
> +		get_pipe.crtc_id =  pipe->crtc_id;
> +		do_ioctl(display->drm_fd,
> +				DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, &get_pipe);
> +		pipe->pipe = get_pipe.pipe;

This looks like it would break everything that currently
assumes pipe->pipe will match the index.

I have a feeling that if we aren't going to do the rename from
pipe to crtc (or whatever) then we should just add a .hw_pipe
which reflects the actual hardware pipe used. Then any test
which relies on hw specific things can examine that instead.
Also reviewing all the tests for such usages of pipe is going
to be some amount of work I imagine.

And this still wouldn't fix the problem of the logs saying
eg. pipe B when in fact it's hw pipe C. But I don't know if
there's any trivial solution for that.

As for the implementation, this needs an i915 check so we don't
blow up on other drivers when using that i915 specific ioctl.

>  		pipe->plane_cursor = -1;
>  		pipe->plane_primary = -1;
>  		pipe->planes = NULL;
> -- 
> 2.24.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev