[igt-dev] [PATCH v2 1/1] tests/kms_cursor_legacy: Skip 2x-cursor-vs-flip subtests for gen > 9

[Neel Desai]

On Tue, Mar 12, 2019 at 07:06:22PM +0200, Ville Syrjälä wrote:
>On Tue, Mar 12, 2019 at 04:46:56PM +0000, Desai, Neel wrote:
>> 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.
>
>>
>> -----Original Message-----
>> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
>> Sent: Tuesday, March 12, 2019 9:45 AM
>> To: Desai, Neel <neel.desai at intel.com>
>> Cc: Dutt, Sudeep <sudeep.dutt at intel.com>; maarten.lankhorst at linux.intel.com; igt-dev at lists.freedesktop.org
>> Subject: Re: [igt-dev] [PATCH v2 1/1] tests/kms_cursor_legacy: Skip 2x-cursor-vs-flip subtests for gen > 9
>>
>> On Tue, Mar 12, 2019 at 09:12:44AM -0700, Neel Desai wrote:
>> > 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.
>>
>> > 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 <neel.desai at intel.com>
>> > cc: Sudeep Dutt <sudeep.dutt at intel.com>
>> > cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> > ---
>> >  tests/kms_cursor_legacy.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>> >
>> > diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
>> > index d6987eea..645c0b93 100644
>> > --- a/tests/kms_cursor_legacy.c
>> > +++ b/tests/kms_cursor_legacy.c
>> > @@ -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
>> > igt-dev at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
>>
>> --
>> Ville Syrjälä
>> Intel
>
>-- 
>Ville Syrjälä
>Intel

>Sure, but we're just testing flips vs. cursor. There should be no
>modesets apart from the initial setup
We are testing cursor vs flips on two screens