Skip 2x-cursor-vs-flip subtests for gen > 9

Due to DDB allocation changes, we have to add all the planes to the drm_atomic_state post gen 9.

When the watermarks are computed by the
kernel, the cursor plane is added to the drm_atomic_state which introduces a dependency on hw_done object when the kernel processes DRM_IOCTL_MODE_ATOMIC and DRM_IOCTL_MODE_CURSOR in parallel. There is no race-free way to handle this corner case. These subtests made sense for older generations but not anymore. Hence, skipping these sub-tests post gen 9.

Reference: https://bugs.freedesktop.org/show_bug.cgi?id=109079

Signed-off-by Neel Desai <[email protected]>

cc Sudeep Dutt <[email protected]>

cc Maarten Lankhorst <[email protected]>

---
 tests/kms_cursor_legacy.c | 3 +++
 1 file changed, 3 insertions(+)

tests/kms_cursor_legacy.c +3 -1

index d6987eea..645c0b93 100644
@@ -1115,6 +1115,7 @@ static void cursor_vs_flip(igt_display_t *display, enum flip_test mode, int nloo
 
 static void two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool atomic)
 {
+	const int gen = intel_gen(intel_get_drm_devid(display->drm_fd));
 	struct drm_mode_cursor arg[2][2];
 	struct drm_event_vblank vbl;
 	struct igt_fb fb_info[2], cursor_fb;

@@ -1126,6 +1127,8 @@ static void two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool
 	};
 	igt_output_t *outputs[2];
 
+	igt_require(gen <= 9);
+
 	shared = mmap(NULL, PAGE_SIZE, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
 	igt_assert(shared != MAP_FAILED);
 
--

2.17.1

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Due to DDB allocation changes, we have to add all the planes to the drm_atomic_state post gen 9.

When the watermarks are computed by the kernel, the cursor plane is added to the drm_atomic_state which introduces a dependency on hw_done object when the kernel processes DRM_IOCTL_MODE_ATOMIC and DRM_IOCTL_MODE_CURSOR in parallel. There is no race-free way to handle this corner case. These subtests made sense for older generations but not anymore. Hence, skipping these sub-tests post gen 9.

Reference: https://bugs.freedesktop.org/show_bug.cgi?id=109079

Signed-off-by Neel Desai <[email protected]>

cc Sudeep Dutt <[email protected]>

cc Maarten Lankhorst <[email protected]>

---
 tests/kms_cursor_legacy.c | 3 +++
 1 file changed, 3 insertions(+)

tests/kms_cursor_legacy.c +1 -0

index d6987eea..645c0b93 100644
@@ -1115,6 +1115,7 @@ static void cursor_vs_flip(igt_display_t 
 *display, enum flip_test mode, int nloo
 
 static void two_screens_cursor_vs_flip(igt_display_t *display, int 
 nloops, bool atomic)  {
+	const int gen = intel_gen(intel_get_drm_devid(display->drm_fd));
 	struct drm_mode_cursor arg[2][2];
 	struct drm_event_vblank vbl;

struct igt_fb fb_info[2], cursor_fb; @@ -1126,6 +1127,8 @@ static
void two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool

tests/kms_cursor_legacy.c +2 -1

 	};
 	igt_output_t *outputs[2];
 
+	igt_require(gen <= 9);
+
 	shared = mmap(NULL, PAGE_SIZE, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
 	igt_assert(shared != MAP_FAILED);
 
--

2.17.1

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

According to Maarten, it does change during modeset of the other pipe. The ddb allocation might not change in size, but during modeset of the other pipe, the position of the cursor DDB may change and CUR_BUF_CFG() will have to be called..

-----Original Message-----
From: Ville Syrjälä [mailto:[email protected]]
Sent: Tuesday, March 12, 2019 9:45 AM
To: Desai, Neel <[email protected]>
Cc: Dutt, Sudeep <[email protected]>; [email protected]; [email protected]
Subject: Re: [igt-dev] [PATCH v2 1/1] tests/kms_cursor_legacy: Skip 2x-cursor-vs-flip subtests for gen > 9

Quoted 2 lines · Neel Desai

Due to DDB allocation changes, we have to add all the planes to the drm_atomic_state post gen 9.

As we discussed on irc that is not true. The cursor has a fixed allocation and thus never needs to be added to the state on account of ddb allocation changing (except for full modesets). So the real reason for the cursor being added to the state is something else. I think the answer has something more to do with the selected max watermark level.

Also I'm not entirely convinced that the fixed allocation scheme is working quite right currently. We never seem to check that we actually have enough ddb space for the selected max watermark level for the cursor. We should add such a check, and we also may have to change how we calculate the size of the fixed cursor ddb allocation to make sure the cursor isn't needlessly limiting the max watermark level we can use.

Quoted 40 lines

When the watermarks are computed by the kernel, the cursor plane is added to the drm_atomic_state which introduces a dependency on hw_done object when the kernel processes DRM_IOCTL_MODE_ATOMIC and DRM_IOCTL_MODE_CURSOR in parallel. There is no race-free way to handle this corner case. These subtests made sense for older generations but not anymore. Hence, skipping these sub-tests post gen 9.

Reference: https://bugs.freedesktop.org/show_bug.cgi?id=109079

Signed-off-by Neel Desai <[email protected]>

cc Sudeep Dutt <[email protected]>

cc Maarten Lankhorst <[email protected]>

---
 tests/kms_cursor_legacy.c | 3 +++
 1 file changed, 3 insertions(+)

tests/kms_cursor_legacy.c +1 -0

index d6987eea..645c0b93 100644
@@ -1115,6 +1115,7 @@ static void cursor_vs_flip(igt_display_t 
 *display, enum flip_test mode, int nloo
 
 static void two_screens_cursor_vs_flip(igt_display_t *display, int 
 nloops, bool atomic)  {
+	const int gen = intel_gen(intel_get_drm_devid(display->drm_fd));
 	struct drm_mode_cursor arg[2][2];
 	struct drm_event_vblank vbl;

struct igt_fb fb_info[2], cursor_fb; @@ -1126,6 +1127,8 @@ static
void two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool

tests/kms_cursor_legacy.c +2 -1

 	};
 	igt_output_t *outputs[2];
 
+	igt_require(gen <= 9);
+
 	shared = mmap(NULL, PAGE_SIZE, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
 	igt_assert(shared != MAP_FAILED);
 
--

2.17.1

Quoted 4 lines

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

--
Ville Syrjälä
Intel

Quoted 1 line · Neel

According to Maarten, it does change during modeset of the other pipe. The ddb allocation might not change in size, but during modeset of the other pipe, the position of the cursor DDB may change and CUR_BUF_CFG() will have to be called..

Sure, but we're just testing flips vs. cursor. There should be no modesets apart from the initial setup.

Quoted 57 lines

-----Original Message-----
From: Ville Syrjälä [mailto:[email protected]]
Sent: Tuesday, March 12, 2019 9:45 AM
To: Desai, Neel <[email protected]>
Cc: Dutt, Sudeep <[email protected]>; [email protected]; [email protected]
Subject: Re: [igt-dev] [PATCH v2 1/1] tests/kms_cursor_legacy: Skip 2x-cursor-vs-flip subtests for gen > 9

Quoted 2 lines · Neel Desai

Due to DDB allocation changes, we have to add all the planes to the drm_atomic_state post gen 9.

As we discussed on irc that is not true. The cursor has a fixed allocation and thus never needs to be added to the state on account of ddb allocation changing (except for full modesets). So the real reason for the cursor being added to the state is something else. I think the answer has something more to do with the selected max watermark level.

Also I'm not entirely convinced that the fixed allocation scheme is working quite right currently. We never seem to check that we actually have enough ddb space for the selected max watermark level for the cursor. We should add such a check, and we also may have to change how we calculate the size of the fixed cursor ddb allocation to make sure the cursor isn't needlessly limiting the max watermark level we can use.

Quoted 40 lines

When the watermarks are computed by the kernel, the cursor plane is added to the drm_atomic_state which introduces a dependency on hw_done object when the kernel processes DRM_IOCTL_MODE_ATOMIC and DRM_IOCTL_MODE_CURSOR in parallel. There is no race-free way to handle this corner case. These subtests made sense for older generations but not anymore. Hence, skipping these sub-tests post gen 9.

Reference: https://bugs.freedesktop.org/show_bug.cgi?id=109079

Signed-off-by Neel Desai <[email protected]>

cc Sudeep Dutt <[email protected]>

cc Maarten Lankhorst <[email protected]>

---
 tests/kms_cursor_legacy.c | 3 +++
 1 file changed, 3 insertions(+)

tests/kms_cursor_legacy.c +1 -0

index d6987eea..645c0b93 100644
@@ -1115,6 +1115,7 @@ static void cursor_vs_flip(igt_display_t
 *display, enum flip_test mode, int nloo
 
 static void two_screens_cursor_vs_flip(igt_display_t *display, int
 nloops, bool atomic)  {
+	const int gen = intel_gen(intel_get_drm_devid(display->drm_fd));
 	struct drm_mode_cursor arg[2][2];
 	struct drm_event_vblank vbl;

struct igt_fb fb_info[2], cursor_fb; @@ -1126,6 +1127,8 @@ static
void two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool

tests/kms_cursor_legacy.c +2 -1

 	};
 	igt_output_t *outputs[2];
 
+	igt_require(gen <= 9);
+
 	shared = mmap(NULL, PAGE_SIZE, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
 	igt_assert(shared != MAP_FAILED);
 
--

2.17.1

Quoted 8 lines

Quoted 4 lines

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

--
Ville Syrjälä
Intel

Signature · 5 lines

Ville Syrjälä
Intel

Sure, but we're just testing flips vs. cursor. There should be no
modesets apart from the initial setup

Quoted 2 lines · Neel Desai

Due to DDB allocation changes, we have to add all the planes to the drm_atomic_state post gen 9.

As we discussed on irc that is not true. The cursor has a fixed allocation and thus never needs to be added to the state on account of ddb allocation changing (except for full modesets). So the real reason for the cursor being added to the state is something else. I think the answer has something more to do with the selected max watermark level.

Also I'm not entirely convinced that the fixed allocation scheme is working quite right currently. We never seem to check that we actually have enough ddb space for the selected max watermark level for the cursor. We should add such a check, and we also may have to change how we calculate the size of the fixed cursor ddb allocation to make sure the cursor isn't needlessly limiting the max watermark level we can use.

Quoted 45 lines

When the watermarks are computed by the
kernel, the cursor plane is added to the drm_atomic_state which introduces a dependency on hw_done object when the kernel processes DRM_IOCTL_MODE_ATOMIC and DRM_IOCTL_MODE_CURSOR in parallel. There is no race-free way to handle this corner case. These subtests made sense for older generations but not anymore. Hence, skipping these sub-tests post gen 9.

Reference: https://bugs.freedesktop.org/show_bug.cgi?id=109079

Signed-off-by Neel Desai <[email protected]>

cc Sudeep Dutt <[email protected]>

cc Maarten Lankhorst <[email protected]>

---
 tests/kms_cursor_legacy.c | 3 +++
 1 file changed, 3 insertions(+)

tests/kms_cursor_legacy.c +3 -1

index d6987eea..645c0b93 100644
@@ -1115,6 +1115,7 @@ static void cursor_vs_flip(igt_display_t *display, enum flip_test mode, int nloo
 
 static void two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool atomic)
 {
+	const int gen = intel_gen(intel_get_drm_devid(display->drm_fd));
 	struct drm_mode_cursor arg[2][2];
 	struct drm_event_vblank vbl;
 	struct igt_fb fb_info[2], cursor_fb;

@@ -1126,6 +1127,8 @@ static void two_screens_cursor_vs_flip(igt_display_t *display, int nloops, bool
 	};
 	igt_output_t *outputs[2];
 
+	igt_require(gen <= 9);
+
 	shared = mmap(NULL, PAGE_SIZE, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
 	igt_assert(shared != MAP_FAILED);
 
--

2.17.1

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Signature · 2 lines

Ville Syrjälä
Intel

Skip the following for gen > 9

1.) tests/kms_cursor_legacy@2x-cursor-vs-flip-atomic 2.) tests/kms_cursor_legacy@2x-cursor-vs-flip-legacy 3.) tests/kms_cursor_legacy@2x-long-cursor-vs-flip-atomic 4.) tests/kms_cursor_legacy@2x-long-cursor-vs-flip-legacy

Due to DDB allocation changes, we have to add all the planes to the drm_atomic_state post gen 9. When the watermarks are computed by the kernel, the cursor plane is added to the drm_atomic_state which introduces a dependency on hw_done object when the kernel processes DRM_IOCTL_MODE_ATOMIC and DRM_IOCTL_MODE_CURSOR in parallel. There is no race-free way to handle this corner case. These subtests made sense for older generations but not anymore. Hence, skipping these sub-tests post gen 9.

Quoting Neel Desai (2019-03-12 16:12:43)

Quoted 7 lines

Skip the following for gen > 9

1.) tests/kms_cursor_legacy@2x-cursor-vs-flip-atomic 2.) tests/kms_cursor_legacy@2x-cursor-vs-flip-legacy 3.) tests/kms_cursor_legacy@2x-long-cursor-vs-flip-atomic 4.) tests/kms_cursor_legacy@2x-long-cursor-vs-flip-legacy

Quoted 8 lines

Due to DDB allocation changes, we have to add all the planes to the drm_atomic_state post gen 9. When the watermarks are computed by the kernel, the cursor plane is added to the drm_atomic_state which introduces a dependency on hw_done object when the kernel processes DRM_IOCTL_MODE_ATOMIC and DRM_IOCTL_MODE_CURSOR in parallel. There is no race-free way to handle this corner case. These subtests made sense for older generations but not anymore. Hence, skipping these sub-tests post gen 9.

Or fail. Mark it as expected; come back when problem is solved or HW has moved on.