intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH i-g-t 0/3] Various fixes for non-Intel compatibility
@ 2016-11-21 22:32 Lyude
  2016-11-21 22:32 ` [PATCH i-g-t 1/3] igt_kms: Don't assume we have cursor planes if we have primary ones Lyude
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Lyude @ 2016-11-21 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lyude

Here's a couple of fixes I ended up needing when I was working on trying to get
the chamelium tests to pass on the other GPUs I've got available. This should
make both igt_display_init() and kmstest_force_connector() work properly on
most non-intel hardware.

Lyude (3):
  igt_kms: Don't assume we have cursor planes if we have primary ones
  igt_kms: Change the max number of pipes to 6
  igt_kms: Don't require intel hardware for kmstest_force_connector

 lib/igt_kms.c | 50 +++++++++++++++++++++++++++++---------------------
 lib/igt_kms.h | 23 ++++++++++++++++++++---
 2 files changed, 49 insertions(+), 24 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH i-g-t 1/3] igt_kms: Don't assume we have cursor planes if we have primary ones
  2016-11-21 22:32 [PATCH i-g-t 0/3] Various fixes for non-Intel compatibility Lyude
@ 2016-11-21 22:32 ` Lyude
  2016-12-07  7:34   ` Tomeu Vizoso
  2016-11-21 22:32 ` [PATCH i-g-t 2/3] igt_kms: Change the max number of pipes to 6 Lyude
  2016-11-21 22:32 ` [PATCH i-g-t 3/3] igt_kms: Don't require intel hardware for kmstest_force_connector Lyude
  2 siblings, 1 reply; 6+ messages in thread
From: Lyude @ 2016-11-21 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lyude

On certain models of nvidia and AMD GPUs, we can have a primary plane
without any DRM plane for the cursor plane. Check for this so we don't
segfault on non-intel hardware.

Signed-off-by: Lyude <lyude@redhat.com>
---
 lib/igt_kms.c | 27 +++++++++++++++------------
 lib/igt_kms.h |  1 +
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 433a721..13d323e 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1419,7 +1419,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 				plane = &pipe->planes[IGT_PLANE_CURSOR];
 				plane->is_cursor = 1;
 				plane->index = IGT_PLANE_CURSOR;
-				display->has_universal_planes = 1;
+				display->has_cursor_plane = 1;
 				break;
 			default:
 				plane = &pipe->planes[p];
@@ -1444,9 +1444,20 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 			plane->rotation = (igt_rotation_t)prop_value;
 		}
 
-		if (display->has_universal_planes) {
+		if (!display->has_universal_planes) {
+			/*
+			 * No universal plane support.  Add drm_plane-less
+			 * primary and cursor planes.
+			 */
+			plane = &pipe->planes[IGT_PLANE_PRIMARY];
+			plane->pipe = pipe;
+			plane->index = IGT_PLANE_PRIMARY;
+			plane->is_primary = true;
+		}
+
+		if (display->has_cursor_plane) {
 			/*
-			 * If we have universal planes, we should have both
+			 * If we have a cursor plane, we should have both
 			 * primary and cursor planes setup now.
 			 */
 			igt_assert(pipe->planes[IGT_PLANE_PRIMARY].drm_plane &&
@@ -1464,15 +1475,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 				       sizeof *plane);
 			}
 		} else {
-			/*
-			 * No universal plane support.  Add drm_plane-less
-			 * primary and cursor planes.
-			 */
-			plane = &pipe->planes[IGT_PLANE_PRIMARY];
-			plane->pipe = pipe;
-			plane->index = IGT_PLANE_PRIMARY;
-			plane->is_primary = true;
-
+			/* Add drm_plane-less cursor */
 			plane = &pipe->planes[p];
 			plane->pipe = pipe;
 			plane->index = p;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 95395cd..95d81c3 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -312,6 +312,7 @@ struct igt_display {
 	igt_output_t *outputs;
 	igt_pipe_t pipes[I915_MAX_PIPES];
 	bool has_universal_planes;
+	bool has_cursor_plane;
 	bool is_atomic;
 };
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH i-g-t 2/3] igt_kms: Change the max number of pipes to 6
  2016-11-21 22:32 [PATCH i-g-t 0/3] Various fixes for non-Intel compatibility Lyude
  2016-11-21 22:32 ` [PATCH i-g-t 1/3] igt_kms: Don't assume we have cursor planes if we have primary ones Lyude
@ 2016-11-21 22:32 ` Lyude
  2016-12-07  7:26   ` Tomeu Vizoso
  2016-11-21 22:32 ` [PATCH i-g-t 3/3] igt_kms: Don't require intel hardware for kmstest_force_connector Lyude
  2 siblings, 1 reply; 6+ messages in thread
From: Lyude @ 2016-11-21 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lyude

Unfortunately the assumption that we only have 6 display pipes available
is specific to Intel, and seems to be breaking igt_display_init() on
both radeon and nouveau since this causes us not to leave enough space
in the igt_display_t struct to hold information for all 6 pipes.

So, up the max to 6. As well, add IGT_MAX_PIPES and use that instead
since this is no longer specific to Intel. We also leave I915_MAX_PIPES
defined as 3 so as to not break existing tests relying on this.

Signed-off-by: Lyude <lyude@redhat.com>
---
 lib/igt_kms.h | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 95d81c3..0ea2454 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -46,7 +46,10 @@
  * @PIPE_A: First crtc.
  * @PIPE_B: Second crtc.
  * @PIPE_C: Third crtc.
- * @I915_MAX_PIPES: Max number of pipes allowed.
+ * @PIPE_D: Fourth crtc.
+ * @PIPE_E: Fifth crtc.
+ * @PIPE_F: Sixth crtc.
+ * @IGT_MAX_PIPES: Max number of pipes allowed.
  */
 enum pipe {
         PIPE_NONE = -1,
@@ -54,10 +57,23 @@ enum pipe {
         PIPE_A = 0,
         PIPE_B,
         PIPE_C,
-        I915_MAX_PIPES
+	PIPE_D,
+	PIPE_E,
+	PIPE_F,
+	IGT_MAX_PIPES,
 };
 const char *kmstest_pipe_name(enum pipe pipe);
 
+/**
+ * I915_MAX_PIPES:
+ *
+ * The max number of pipes on i915 devices. This should only be used for tests
+ * which are guaranteed to only be used on intel hardware. All other tests
+ * should use @IGT_MAX_PIPES and/or check the number of reported pipes on the
+ * hardware.
+ */
+#define I915_MAX_PIPES 3
+
 /* We namespace this enum to not conflict with the Android i915_drm.h */
 enum igt_plane {
 	IGT_PLANE_1 = 0,
@@ -310,7 +326,7 @@ struct igt_display {
 	int n_outputs;
 	unsigned long pipes_in_use;
 	igt_output_t *outputs;
-	igt_pipe_t pipes[I915_MAX_PIPES];
+	igt_pipe_t pipes[IGT_MAX_PIPES];
 	bool has_universal_planes;
 	bool has_cursor_plane;
 	bool is_atomic;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH i-g-t 3/3] igt_kms: Don't require intel hardware for kmstest_force_connector
  2016-11-21 22:32 [PATCH i-g-t 0/3] Various fixes for non-Intel compatibility Lyude
  2016-11-21 22:32 ` [PATCH i-g-t 1/3] igt_kms: Don't assume we have cursor planes if we have primary ones Lyude
  2016-11-21 22:32 ` [PATCH i-g-t 2/3] igt_kms: Change the max number of pipes to 6 Lyude
@ 2016-11-21 22:32 ` Lyude
  2 siblings, 0 replies; 6+ messages in thread
From: Lyude @ 2016-11-21 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lyude

Due to the unconditional call to intel_get_drm_devid() in
kmstest_force_connector(), trying to use this function on anything that
isn't intel hardware results in the current fixture being skipped. So,
don't try to get the devid in kmstest_force_connector() unless we're on
an Intel chipset.

Signed-off-by: Lyude <lyude@redhat.com>
---
 lib/igt_kms.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 13d323e..8c9bbbf 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -627,15 +627,20 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 	uint32_t devid;
 	int len, dir, idx;
 
-	devid = intel_get_drm_devid(drm_fd);
-
-	/* forcing hdmi or dp connectors on HSW and BDW doesn't currently work,
-	 * so fail early to allow the test to skip if required */
-	if ((connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
-	     connector->connector_type == DRM_MODE_CONNECTOR_HDMIB ||
-	     connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
-	    && (IS_HASWELL(devid) || IS_BROADWELL(devid)))
-		return false;
+	if (is_i915_device(drm_fd)) {
+		devid = intel_get_drm_devid(drm_fd);
+
+		/*
+		 * forcing hdmi or dp connectors on HSW and BDW doesn't
+		 * currently work, so fail early to allow the test to skip if
+		 * required
+		 */
+		if ((connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
+		     connector->connector_type == DRM_MODE_CONNECTOR_HDMIB ||
+		     connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
+		    && (IS_HASWELL(devid) || IS_BROADWELL(devid)))
+			return false;
+	}
 
 	switch (state) {
 	case FORCE_CONNECTOR_ON:
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH i-g-t 2/3] igt_kms: Change the max number of pipes to 6
  2016-11-21 22:32 ` [PATCH i-g-t 2/3] igt_kms: Change the max number of pipes to 6 Lyude
@ 2016-12-07  7:26   ` Tomeu Vizoso
  0 siblings, 0 replies; 6+ messages in thread
From: Tomeu Vizoso @ 2016-12-07  7:26 UTC (permalink / raw)
  To: Lyude; +Cc: Intel Graphics Development

On 21 November 2016 at 23:32, Lyude <lyude@redhat.com> wrote:
> Unfortunately the assumption that we only have 6 display pipes available
> is specific to Intel, and seems to be breaking igt_display_init() on
> both radeon and nouveau since this causes us not to leave enough space
> in the igt_display_t struct to hold information for all 6 pipes.
>
> So, up the max to 6. As well, add IGT_MAX_PIPES and use that instead
> since this is no longer specific to Intel. We also leave I915_MAX_PIPES
> defined as 3 so as to not break existing tests relying on this.

Hi Lyude,

we already have igt_display_t.n_pipes for this, which is count_crtcs.

So I think any tests should stop using the hardcoded values and use
the value we get from the kernel instead.

Regards,

Tomeu

> Signed-off-by: Lyude <lyude@redhat.com>
> ---
>  lib/igt_kms.h | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 95d81c3..0ea2454 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -46,7 +46,10 @@
>   * @PIPE_A: First crtc.
>   * @PIPE_B: Second crtc.
>   * @PIPE_C: Third crtc.
> - * @I915_MAX_PIPES: Max number of pipes allowed.
> + * @PIPE_D: Fourth crtc.
> + * @PIPE_E: Fifth crtc.
> + * @PIPE_F: Sixth crtc.
> + * @IGT_MAX_PIPES: Max number of pipes allowed.
>   */
>  enum pipe {
>          PIPE_NONE = -1,
> @@ -54,10 +57,23 @@ enum pipe {
>          PIPE_A = 0,
>          PIPE_B,
>          PIPE_C,
> -        I915_MAX_PIPES
> +       PIPE_D,
> +       PIPE_E,
> +       PIPE_F,
> +       IGT_MAX_PIPES,
>  };
>  const char *kmstest_pipe_name(enum pipe pipe);
>
> +/**
> + * I915_MAX_PIPES:
> + *
> + * The max number of pipes on i915 devices. This should only be used for tests
> + * which are guaranteed to only be used on intel hardware. All other tests
> + * should use @IGT_MAX_PIPES and/or check the number of reported pipes on the
> + * hardware.
> + */
> +#define I915_MAX_PIPES 3
> +
>  /* We namespace this enum to not conflict with the Android i915_drm.h */
>  enum igt_plane {
>         IGT_PLANE_1 = 0,
> @@ -310,7 +326,7 @@ struct igt_display {
>         int n_outputs;
>         unsigned long pipes_in_use;
>         igt_output_t *outputs;
> -       igt_pipe_t pipes[I915_MAX_PIPES];
> +       igt_pipe_t pipes[IGT_MAX_PIPES];
>         bool has_universal_planes;
>         bool has_cursor_plane;
>         bool is_atomic;
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH i-g-t 1/3] igt_kms: Don't assume we have cursor planes if we have primary ones
  2016-11-21 22:32 ` [PATCH i-g-t 1/3] igt_kms: Don't assume we have cursor planes if we have primary ones Lyude
@ 2016-12-07  7:34   ` Tomeu Vizoso
  0 siblings, 0 replies; 6+ messages in thread
From: Tomeu Vizoso @ 2016-12-07  7:34 UTC (permalink / raw)
  To: Lyude; +Cc: Intel Graphics Development

On 21 November 2016 at 23:32, Lyude <lyude@redhat.com> wrote:
> On certain models of nvidia and AMD GPUs, we can have a primary plane
> without any DRM plane for the cursor plane. Check for this so we don't
> segfault on non-intel hardware.
>
> Signed-off-by: Lyude <lyude@redhat.com>
> ---
>  lib/igt_kms.c | 27 +++++++++++++++------------
>  lib/igt_kms.h |  1 +
>  2 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 433a721..13d323e 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1419,7 +1419,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>                                 plane = &pipe->planes[IGT_PLANE_CURSOR];
>                                 plane->is_cursor = 1;
>                                 plane->index = IGT_PLANE_CURSOR;
> -                               display->has_universal_planes = 1;
> +                               display->has_cursor_plane = 1;

Looks good to me, but Daniel Vetter voiced support the other day for
dropping support for !has_universal_planes so it may not be a good
idea to complicate this further.

But if we go this way, the has_cursor_plane name could be misleading
in the !universal_plane case if there's a cursor plane.

Regards,

Tomeu

>                                 break;
>                         default:
>                                 plane = &pipe->planes[p];
> @@ -1444,9 +1444,20 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>                         plane->rotation = (igt_rotation_t)prop_value;
>                 }
>
> -               if (display->has_universal_planes) {
> +               if (!display->has_universal_planes) {
> +                       /*
> +                        * No universal plane support.  Add drm_plane-less
> +                        * primary and cursor planes.
> +                        */
> +                       plane = &pipe->planes[IGT_PLANE_PRIMARY];
> +                       plane->pipe = pipe;
> +                       plane->index = IGT_PLANE_PRIMARY;
> +                       plane->is_primary = true;
> +               }
> +
> +               if (display->has_cursor_plane) {
>                         /*
> -                        * If we have universal planes, we should have both
> +                        * If we have a cursor plane, we should have both
>                          * primary and cursor planes setup now.
>                          */
>                         igt_assert(pipe->planes[IGT_PLANE_PRIMARY].drm_plane &&
> @@ -1464,15 +1475,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>                                        sizeof *plane);
>                         }
>                 } else {
> -                       /*
> -                        * No universal plane support.  Add drm_plane-less
> -                        * primary and cursor planes.
> -                        */
> -                       plane = &pipe->planes[IGT_PLANE_PRIMARY];
> -                       plane->pipe = pipe;
> -                       plane->index = IGT_PLANE_PRIMARY;
> -                       plane->is_primary = true;
> -
> +                       /* Add drm_plane-less cursor */
>                         plane = &pipe->planes[p];
>                         plane->pipe = pipe;
>                         plane->index = p;
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 95395cd..95d81c3 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -312,6 +312,7 @@ struct igt_display {
>         igt_output_t *outputs;
>         igt_pipe_t pipes[I915_MAX_PIPES];
>         bool has_universal_planes;
> +       bool has_cursor_plane;
>         bool is_atomic;
>  };
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-07  7:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-21 22:32 [PATCH i-g-t 0/3] Various fixes for non-Intel compatibility Lyude
2016-11-21 22:32 ` [PATCH i-g-t 1/3] igt_kms: Don't assume we have cursor planes if we have primary ones Lyude
2016-12-07  7:34   ` Tomeu Vizoso
2016-11-21 22:32 ` [PATCH i-g-t 2/3] igt_kms: Change the max number of pipes to 6 Lyude
2016-12-07  7:26   ` Tomeu Vizoso
2016-11-21 22:32 ` [PATCH i-g-t 3/3] igt_kms: Don't require intel hardware for kmstest_force_connector Lyude

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).