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

Ville Syrjälä
2019-03-12T21:02:00Z

On Tue, Mar 12, 2019 at 06:44:41PM +0200, Ville Syrjälä wrote:
> 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.

Yeah, turns out the currect code was totally broke. I just fired
off some patches to fix it.

>
> > 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@intel.com>
> > cc: Sudeep Dutt <sudeep.dutt@intel.com>
> > cc: Maarten Lankhorst <maarten.lankhorst@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
> --
> Ville Syrjälä
> Intel

--
Ville Syrjälä
Intel