[PATCH igt] igt/kms_atomic: Match CRTC harder for special planes

[PATCH igt] igt/kms_atomic: Match CRTC harder for special planes

[Daniel Stone]

Our heuristic for finding planes was previously matching the type, and
ensuring that the plane was valid for that CRTC. However, VC4 now has
primary/cursor planes which can wander multiple CRTCs, so we could pick
a PRIMARY plane which was not the kernel's idea of crtc->primary,
causing plane_primary_legacy to fail; ditto for cursor.

Make find_plane try harder, by preferring to return planes which are
already on the requested CRTC.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reported-by: Robert Foss <robert.foss@collabora.com>
Cc: Eric Anholt <eric@anholt.net>
---
 tests/kms_atomic.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index 1441fdf..e6d71c3 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -451,6 +451,7 @@ static struct kms_atomic_plane_state *
 find_plane(struct kms_atomic_state *state, enum plane_type type,
 	   struct kms_atomic_crtc_state *crtc)
 {
+	struct kms_atomic_plane_state *ret = NULL;
 	int i;
 
 	for (i = 0; i < state->num_planes; i++) {
@@ -464,10 +465,18 @@ find_plane(struct kms_atomic_state *state, enum plane_type type,
 			continue;
 
 		plane_get_current_state(plane);
-		return plane;
+
+		/* Try to find a plane that's already on this CRTC. In
+		 * particular, this ensures that for special (primary/cursor)
+		 * planes that can be on multiple CRTCs, we find the same
+		 * one that the legacy ioctls will. */
+		if (!crtc || plane->crtc_id == crtc->obj)
+			return plane;
+
+		ret = plane;
 	}
 
-	return NULL;
+	return ret;
 }
 
 static void crtc_populate_req(struct kms_atomic_crtc_state *crtc,
-- 
2.10.2

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

Re: [PATCH igt] igt/kms_atomic: Match CRTC harder for special planes

[Robert Foss]

Hey Daniel,

This patch looks good to me.

On 2016-12-13 01:15 PM, Daniel Stone wrote:
> Our heuristic for finding planes was previously matching the type, and
> ensuring that the plane was valid for that CRTC. However, VC4 now has
> primary/cursor planes which can wander multiple CRTCs, so we could pick
> a PRIMARY plane which was not the kernel's idea of crtc->primary,
> causing plane_primary_legacy to fail; ditto for cursor.
>
> Make find_plane try harder, by preferring to return planes which are
> already on the requested CRTC.
>
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> Reported-by: Robert Foss <robert.foss@collabora.com>
> Cc: Eric Anholt <eric@anholt.net>
> ---
>  tests/kms_atomic.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> index 1441fdf..e6d71c3 100644
> --- a/tests/kms_atomic.c
> +++ b/tests/kms_atomic.c
> @@ -451,6 +451,7 @@ static struct kms_atomic_plane_state *
>  find_plane(struct kms_atomic_state *state, enum plane_type type,
>  	   struct kms_atomic_crtc_state *crtc)
>  {
> +	struct kms_atomic_plane_state *ret = NULL;
>  	int i;
>
>  	for (i = 0; i < state->num_planes; i++) {
> @@ -464,10 +465,18 @@ find_plane(struct kms_atomic_state *state, enum plane_type type,
>  			continue;
>
>  		plane_get_current_state(plane);
> -		return plane;
> +
> +		/* Try to find a plane that's already on this CRTC. In
> +		 * particular, this ensures that for special (primary/cursor)
> +		 * planes that can be on multiple CRTCs, we find the same
> +		 * one that the legacy ioctls will. */
> +		if (!crtc || plane->crtc_id == crtc->obj)
> +			return plane;
> +
> +		ret = plane;
>  	}
>
> -	return NULL;
> +	return ret;
>  }
>
>  static void crtc_populate_req(struct kms_atomic_crtc_state *crtc,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt] igt/kms_atomic: Match CRTC harder for special planes

[Robert Foss]

Pushed, thanks.


Rob.

On 2016-12-13 01:15 PM, Daniel Stone wrote:
> Our heuristic for finding planes was previously matching the type, and
> ensuring that the plane was valid for that CRTC. However, VC4 now has
> primary/cursor planes which can wander multiple CRTCs, so we could pick
> a PRIMARY plane which was not the kernel's idea of crtc->primary,
> causing plane_primary_legacy to fail; ditto for cursor.
>
> Make find_plane try harder, by preferring to return planes which are
> already on the requested CRTC.
>
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> Reported-by: Robert Foss <robert.foss@collabora.com>
> Cc: Eric Anholt <eric@anholt.net>
> ---
>  tests/kms_atomic.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> index 1441fdf..e6d71c3 100644
> --- a/tests/kms_atomic.c
> +++ b/tests/kms_atomic.c
> @@ -451,6 +451,7 @@ static struct kms_atomic_plane_state *
>  find_plane(struct kms_atomic_state *state, enum plane_type type,
>  	   struct kms_atomic_crtc_state *crtc)
>  {
> +	struct kms_atomic_plane_state *ret = NULL;
>  	int i;
>
>  	for (i = 0; i < state->num_planes; i++) {
> @@ -464,10 +465,18 @@ find_plane(struct kms_atomic_state *state, enum plane_type type,
>  			continue;
>
>  		plane_get_current_state(plane);
> -		return plane;
> +
> +		/* Try to find a plane that's already on this CRTC. In
> +		 * particular, this ensures that for special (primary/cursor)
> +		 * planes that can be on multiple CRTCs, we find the same
> +		 * one that the legacy ioctls will. */
> +		if (!crtc || plane->crtc_id == crtc->obj)
> +			return plane;
> +
> +		ret = plane;
>  	}
>
> -	return NULL;
> +	return ret;
>  }
>
>  static void crtc_populate_req(struct kms_atomic_crtc_state *crtc,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx