Re: [Nouveau] [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_cursor_crc: Probe kernel for cursor size support
On Fri, Mar 19, 2021 at 10:21 PM Ville Syrjälä
wrote:
>
> On Fri, Mar 19, 2021 at 02:43:56PM -0400, Lyude Paul wrote:
> > On Fri, 2021-03-19 at 20:00 +0200, Ville Syrjälä wrote:
> > > On Fri, Mar 19, 2021 at 01:40:52PM -0400, Lyude Paul wrote:
> > > > On Fri, 2021-03-19 at 17:01 +0200, Ville Syrjälä wrote:
> > > > > On Thu, Mar 18, 2021 at 06:21:23PM -0400, Lyude wrote:
> > > > > > From: Lyude Paul
> > > > > >
> > > > > > Currently we just assume that every cursor size up to data-
> > > > > > >cursor_max_w/h
> > > > > > will
> > > > > > be supported by the driver, and check for support of nonsquare
> > > > > > cursors
> > > > > > by
> > > > > > checking if we're running on u815 and if so, which variant of intel
> > > > > > hardware
> > > > > > we're running on. This isn't really ideal as we're about to enable
> > > > > > 32x32
> > > > > > cursor
> > > > > > size tests for nouveau, and Intel hardware doesn't support cursor
> > > > > > sizes
> > > > > > that
> > > > > > small.
> > > > > >
> > > > > > So, fix this by removing has_nonsquare_cursors() and replacing it
> > > > > > with a
> > > > > > more
> > > > > > generic require_cursor_size() function which checks whether or not
> > > > > > the
> > > > > > driver
> > > > > > we're using supports a given cursor size by attempting a test-only
> > > > > > atomic
> > > > > > commit.
> > > > > >
> > > > > > Signed-off-by: Lyude Paul
> > > > > > Cc: Martin Peres
> > > > > > Cc: Ben Skeggs
> > > > > > Cc: Jeremy Cline
> > > > > > ---
> > > > > > tests/kms_cursor_crc.c | 131
> > > > > > -
> > > > > > 1 file changed, 76 insertions(+), 55 deletions(-)
> > > > > >
> > > > > > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > > > > > index 3541ea06..b9c05472 100644
> > > > > > --- a/tests/kms_cursor_crc.c
> > > > > > +++ b/tests/kms_cursor_crc.c
> > > > > > @@ -523,26 +523,43 @@ static void create_cursor_fb(data_t *data, int
> > > > > > cur_w,
> > > > > > int cur_h)
> > > > > > igt_put_cairo_ctx(cr);
> > > > > > }
> > > > > >
> > > > > > -static bool has_nonsquare_cursors(data_t *data)
> > > > > > +static void require_cursor_size(data_t *data, int w, int h)
> > > > > > {
> > > > > > - uint32_t devid;
> > > > > > + igt_fb_t primary_fb;
> > > > > > + drmModeModeInfo *mode;
> > > > > > + igt_display_t *display = &data->display;
> > > > > > + igt_output_t *output = data->output;
> > > > > > + igt_plane_t *primary, *cursor;
> > > > > > + int ret;
> > > > > >
> > > > > > - if (!is_i915_device(data->drm_fd))
> > > > > > - return false;
> > > > > > + igt_output_set_pipe(output, data->pipe);
> > > > > >
> > > > > > - devid = intel_get_drm_devid(data->drm_fd);
> > > > > > + mode = igt_output_get_mode(output);
> > > > > > + primary = igt_output_get_plane_type(output,
> > > > > > DRM_PLANE_TYPE_PRIMARY);
> > > > > > + cursor = igt_output_get_plane_type(output,
> > > > > > DRM_PLANE_TYPE_CURSOR);
> > > > > >
> > > > > > - /*
> > > > > > -* Test non-square cursors a bit on the platforms
> > > > > > -* that support such things.
> > > > > > -*/
> > > > > > - if (devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G)
> > > > > > - return true;
> > > > > > + /* Create temporary primary fb for testing */
> > > > > > + igt_assert(igt_create_fb(data->drm_fd, mode->hdisplay, mode-
> > > > > > > vdisplay, DRM_FORMAT_XRGB,
> > > > > > +LOCAL_DRM_FORMAT_MOD_NONE,
> > > > > > &primary_fb));
> > > > > >
> > > > > > - if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid))
> > > > > > - return false;
> > > > > > + igt_plane_set_fb(primary, &primary_fb);
> > > > > > + igt_plane_set_fb(cursor, &data->fb);
> > > > > > + igt_plane_set_size(cursor, w, h);
> > > > > > + igt_fb_set_size(&data->fb, cursor, w, h);
> > > > > > +
> > > > > > + /* Test if the kernel supports the given cursor size or not
> > > > > > */
> > > > > > + ret = igt_display_try_commit_atomic(display,
> > > > > > +
> > > > > > DRM_MODE_ATOMIC_TEST_ONLY |
> > > > > > +
> > > > > > DRM_MODE_ATOMIC_ALLOW_MODESET,
> > > > > > + NULL);
> > > > >
> > > > > Would be better to not depend on atomic. We have platforms
> > > > > that don't expose it yet.
> > > >
> > > > Do you have any other ideas how we could probe for this then? it seems
> > > > like
> > > > the
> > > > only alternative would be to add intel-specific checks to fix that, or
> > > > add
> > > > some
> > > > ioctl for querying the minimum cursor size (which sounds preferable
> > > > imo).
> > > > would
> > > > the latter work for you, or do you have another idea?
> > >
> > > Just do it for real instead of TEST_ONLY.
> >
> > ah-and it'll still fail in that case I assume?
>
> Yeah, sh
Re: [Nouveau] [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_cursor_crc: Probe kernel for cursor size support
On Fri, Mar 19, 2021 at 02:43:56PM -0400, Lyude Paul wrote:
> On Fri, 2021-03-19 at 20:00 +0200, Ville Syrjälä wrote:
> > On Fri, Mar 19, 2021 at 01:40:52PM -0400, Lyude Paul wrote:
> > > On Fri, 2021-03-19 at 17:01 +0200, Ville Syrjälä wrote:
> > > > On Thu, Mar 18, 2021 at 06:21:23PM -0400, Lyude wrote:
> > > > > From: Lyude Paul
> > > > >
> > > > > Currently we just assume that every cursor size up to data-
> > > > > >cursor_max_w/h
> > > > > will
> > > > > be supported by the driver, and check for support of nonsquare cursors
> > > > > by
> > > > > checking if we're running on u815 and if so, which variant of intel
> > > > > hardware
> > > > > we're running on. This isn't really ideal as we're about to enable
> > > > > 32x32
> > > > > cursor
> > > > > size tests for nouveau, and Intel hardware doesn't support cursor
> > > > > sizes
> > > > > that
> > > > > small.
> > > > >
> > > > > So, fix this by removing has_nonsquare_cursors() and replacing it
> > > > > with a
> > > > > more
> > > > > generic require_cursor_size() function which checks whether or not the
> > > > > driver
> > > > > we're using supports a given cursor size by attempting a test-only
> > > > > atomic
> > > > > commit.
> > > > >
> > > > > Signed-off-by: Lyude Paul
> > > > > Cc: Martin Peres
> > > > > Cc: Ben Skeggs
> > > > > Cc: Jeremy Cline
> > > > > ---
> > > > > tests/kms_cursor_crc.c | 131
> > > > > -
> > > > > 1 file changed, 76 insertions(+), 55 deletions(-)
> > > > >
> > > > > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > > > > index 3541ea06..b9c05472 100644
> > > > > --- a/tests/kms_cursor_crc.c
> > > > > +++ b/tests/kms_cursor_crc.c
> > > > > @@ -523,26 +523,43 @@ static void create_cursor_fb(data_t *data, int
> > > > > cur_w,
> > > > > int cur_h)
> > > > > igt_put_cairo_ctx(cr);
> > > > > }
> > > > >
> > > > > -static bool has_nonsquare_cursors(data_t *data)
> > > > > +static void require_cursor_size(data_t *data, int w, int h)
> > > > > {
> > > > > - uint32_t devid;
> > > > > + igt_fb_t primary_fb;
> > > > > + drmModeModeInfo *mode;
> > > > > + igt_display_t *display = &data->display;
> > > > > + igt_output_t *output = data->output;
> > > > > + igt_plane_t *primary, *cursor;
> > > > > + int ret;
> > > > >
> > > > > - if (!is_i915_device(data->drm_fd))
> > > > > - return false;
> > > > > + igt_output_set_pipe(output, data->pipe);
> > > > >
> > > > > - devid = intel_get_drm_devid(data->drm_fd);
> > > > > + mode = igt_output_get_mode(output);
> > > > > + primary = igt_output_get_plane_type(output,
> > > > > DRM_PLANE_TYPE_PRIMARY);
> > > > > + cursor = igt_output_get_plane_type(output,
> > > > > DRM_PLANE_TYPE_CURSOR);
> > > > >
> > > > > - /*
> > > > > - * Test non-square cursors a bit on the platforms
> > > > > - * that support such things.
> > > > > - */
> > > > > - if (devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G)
> > > > > - return true;
> > > > > + /* Create temporary primary fb for testing */
> > > > > + igt_assert(igt_create_fb(data->drm_fd, mode->hdisplay, mode-
> > > > > > vdisplay, DRM_FORMAT_XRGB,
> > > > > + LOCAL_DRM_FORMAT_MOD_NONE,
> > > > > &primary_fb));
> > > > >
> > > > > - if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid))
> > > > > - return false;
> > > > > + igt_plane_set_fb(primary, &primary_fb);
> > > > > + igt_plane_set_fb(cursor, &data->fb);
> > > > > + igt_plane_set_size(cursor, w, h);
> > > > > + igt_fb_set_size(&data->fb, cursor, w, h);
> > > > > +
> > > > > + /* Test if the kernel supports the given cursor size or not */
> > > > > + ret = igt_display_try_commit_atomic(display,
> > > > > + DRM_MODE_ATOMIC_TEST_ONLY
> > > > > |
> > > > > +
> > > > > DRM_MODE_ATOMIC_ALLOW_MODESET,
> > > > > + NULL);
> > > >
> > > > Would be better to not depend on atomic. We have platforms
> > > > that don't expose it yet.
> > >
> > > Do you have any other ideas how we could probe for this then? it seems
> > > like
> > > the
> > > only alternative would be to add intel-specific checks to fix that, or add
> > > some
> > > ioctl for querying the minimum cursor size (which sounds preferable imo).
> > > would
> > > the latter work for you, or do you have another idea?
> >
> > Just do it for real instead of TEST_ONLY.
>
> ah-and it'll still fail in that case I assume?
Yeah, should fail just the same if the driver doesn't like it.
--
Ville Syrjälä
Intel
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_cursor_crc: Probe kernel for cursor size support
On Fri, 2021-03-19 at 20:00 +0200, Ville Syrjälä wrote:
> On Fri, Mar 19, 2021 at 01:40:52PM -0400, Lyude Paul wrote:
> > On Fri, 2021-03-19 at 17:01 +0200, Ville Syrjälä wrote:
> > > On Thu, Mar 18, 2021 at 06:21:23PM -0400, Lyude wrote:
> > > > From: Lyude Paul
> > > >
> > > > Currently we just assume that every cursor size up to data-
> > > > >cursor_max_w/h
> > > > will
> > > > be supported by the driver, and check for support of nonsquare cursors
> > > > by
> > > > checking if we're running on u815 and if so, which variant of intel
> > > > hardware
> > > > we're running on. This isn't really ideal as we're about to enable 32x32
> > > > cursor
> > > > size tests for nouveau, and Intel hardware doesn't support cursor sizes
> > > > that
> > > > small.
> > > >
> > > > So, fix this by removing has_nonsquare_cursors() and replacing it with a
> > > > more
> > > > generic require_cursor_size() function which checks whether or not the
> > > > driver
> > > > we're using supports a given cursor size by attempting a test-only
> > > > atomic
> > > > commit.
> > > >
> > > > Signed-off-by: Lyude Paul
> > > > Cc: Martin Peres
> > > > Cc: Ben Skeggs
> > > > Cc: Jeremy Cline
> > > > ---
> > > > tests/kms_cursor_crc.c | 131 -
> > > > 1 file changed, 76 insertions(+), 55 deletions(-)
> > > >
> > > > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > > > index 3541ea06..b9c05472 100644
> > > > --- a/tests/kms_cursor_crc.c
> > > > +++ b/tests/kms_cursor_crc.c
> > > > @@ -523,26 +523,43 @@ static void create_cursor_fb(data_t *data, int
> > > > cur_w,
> > > > int cur_h)
> > > > igt_put_cairo_ctx(cr);
> > > > }
> > > >
> > > > -static bool has_nonsquare_cursors(data_t *data)
> > > > +static void require_cursor_size(data_t *data, int w, int h)
> > > > {
> > > > - uint32_t devid;
> > > > + igt_fb_t primary_fb;
> > > > + drmModeModeInfo *mode;
> > > > + igt_display_t *display = &data->display;
> > > > + igt_output_t *output = data->output;
> > > > + igt_plane_t *primary, *cursor;
> > > > + int ret;
> > > >
> > > > - if (!is_i915_device(data->drm_fd))
> > > > - return false;
> > > > + igt_output_set_pipe(output, data->pipe);
> > > >
> > > > - devid = intel_get_drm_devid(data->drm_fd);
> > > > + mode = igt_output_get_mode(output);
> > > > + primary = igt_output_get_plane_type(output,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > + cursor = igt_output_get_plane_type(output,
> > > > DRM_PLANE_TYPE_CURSOR);
> > > >
> > > > - /*
> > > > - * Test non-square cursors a bit on the platforms
> > > > - * that support such things.
> > > > - */
> > > > - if (devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G)
> > > > - return true;
> > > > + /* Create temporary primary fb for testing */
> > > > + igt_assert(igt_create_fb(data->drm_fd, mode->hdisplay, mode-
> > > > > vdisplay, DRM_FORMAT_XRGB,
> > > > + LOCAL_DRM_FORMAT_MOD_NONE,
> > > > &primary_fb));
> > > >
> > > > - if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid))
> > > > - return false;
> > > > + igt_plane_set_fb(primary, &primary_fb);
> > > > + igt_plane_set_fb(cursor, &data->fb);
> > > > + igt_plane_set_size(cursor, w, h);
> > > > + igt_fb_set_size(&data->fb, cursor, w, h);
> > > > +
> > > > + /* Test if the kernel supports the given cursor size or not */
> > > > + ret = igt_display_try_commit_atomic(display,
> > > > + DRM_MODE_ATOMIC_TEST_ONLY |
> > > > +
> > > > DRM_MODE_ATOMIC_ALLOW_MODESET,
> > > > + NULL);
> > >
> > > Would be better to not depend on atomic. We have platforms
> > > that don't expose it yet.
> >
> > Do you have any other ideas how we could probe for this then? it seems like
> > the
> > only alternative would be to add intel-specific checks to fix that, or add
> > some
> > ioctl for querying the minimum cursor size (which sounds preferable imo).
> > would
> > the latter work for you, or do you have another idea?
>
> Just do it for real instead of TEST_ONLY.
ah-and it'll still fail in that case I assume?
>
> > >
> > > > +
> > > > + igt_plane_set_fb(primary, NULL);
> > > > + igt_plane_set_fb(cursor, NULL);
> > > > +
> > > > + igt_remove_fb(data->drm_fd, &primary_fb);
> > > > + igt_output_set_pipe(output, PIPE_NONE);
> > > >
> > > > - return intel_gen(devid) >= 7;
> > > > + igt_skip_on_f(ret, "Cursor size %dx%d not supported by
> > > > driver\n", w,
> > > > h);
> > > > }
> > > >
> > > > static void test_cursor_size(data_t *data)
> > > > @@ -697,27 +714,33 @@ static void run_tests_on_pipe(data_t *data, enum
> > > > pipe
> > > > pipe)
> > > > create_cursor_fb
Re: [Nouveau] [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_cursor_crc: Probe kernel for cursor size support
On Fri, Mar 19, 2021 at 01:40:52PM -0400, Lyude Paul wrote:
> On Fri, 2021-03-19 at 17:01 +0200, Ville Syrjälä wrote:
> > On Thu, Mar 18, 2021 at 06:21:23PM -0400, Lyude wrote:
> > > From: Lyude Paul
> > >
> > > Currently we just assume that every cursor size up to data->cursor_max_w/h
> > > will
> > > be supported by the driver, and check for support of nonsquare cursors by
> > > checking if we're running on u815 and if so, which variant of intel
> > > hardware
> > > we're running on. This isn't really ideal as we're about to enable 32x32
> > > cursor
> > > size tests for nouveau, and Intel hardware doesn't support cursor sizes
> > > that
> > > small.
> > >
> > > So, fix this by removing has_nonsquare_cursors() and replacing it with a
> > > more
> > > generic require_cursor_size() function which checks whether or not the
> > > driver
> > > we're using supports a given cursor size by attempting a test-only atomic
> > > commit.
> > >
> > > Signed-off-by: Lyude Paul
> > > Cc: Martin Peres
> > > Cc: Ben Skeggs
> > > Cc: Jeremy Cline
> > > ---
> > > tests/kms_cursor_crc.c | 131 -
> > > 1 file changed, 76 insertions(+), 55 deletions(-)
> > >
> > > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > > index 3541ea06..b9c05472 100644
> > > --- a/tests/kms_cursor_crc.c
> > > +++ b/tests/kms_cursor_crc.c
> > > @@ -523,26 +523,43 @@ static void create_cursor_fb(data_t *data, int
> > > cur_w,
> > > int cur_h)
> > > igt_put_cairo_ctx(cr);
> > > }
> > >
> > > -static bool has_nonsquare_cursors(data_t *data)
> > > +static void require_cursor_size(data_t *data, int w, int h)
> > > {
> > > - uint32_t devid;
> > > + igt_fb_t primary_fb;
> > > + drmModeModeInfo *mode;
> > > + igt_display_t *display = &data->display;
> > > + igt_output_t *output = data->output;
> > > + igt_plane_t *primary, *cursor;
> > > + int ret;
> > >
> > > - if (!is_i915_device(data->drm_fd))
> > > - return false;
> > > + igt_output_set_pipe(output, data->pipe);
> > >
> > > - devid = intel_get_drm_devid(data->drm_fd);
> > > + mode = igt_output_get_mode(output);
> > > + primary = igt_output_get_plane_type(output,
> > > DRM_PLANE_TYPE_PRIMARY);
> > > + cursor = igt_output_get_plane_type(output, DRM_PLANE_TYPE_CURSOR);
> > >
> > > - /*
> > > - * Test non-square cursors a bit on the platforms
> > > - * that support such things.
> > > - */
> > > - if (devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G)
> > > - return true;
> > > + /* Create temporary primary fb for testing */
> > > + igt_assert(igt_create_fb(data->drm_fd, mode->hdisplay, mode-
> > > >vdisplay, DRM_FORMAT_XRGB,
> > > + LOCAL_DRM_FORMAT_MOD_NONE, &primary_fb));
> > >
> > > - if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid))
> > > - return false;
> > > + igt_plane_set_fb(primary, &primary_fb);
> > > + igt_plane_set_fb(cursor, &data->fb);
> > > + igt_plane_set_size(cursor, w, h);
> > > + igt_fb_set_size(&data->fb, cursor, w, h);
> > > +
> > > + /* Test if the kernel supports the given cursor size or not */
> > > + ret = igt_display_try_commit_atomic(display,
> > > + DRM_MODE_ATOMIC_TEST_ONLY |
> > > + DRM_MODE_ATOMIC_ALLOW_MODESET,
> > > + NULL);
> >
> > Would be better to not depend on atomic. We have platforms
> > that don't expose it yet.
>
> Do you have any other ideas how we could probe for this then? it seems like
> the
> only alternative would be to add intel-specific checks to fix that, or add
> some
> ioctl for querying the minimum cursor size (which sounds preferable imo).
> would
> the latter work for you, or do you have another idea?
Just do it for real instead of TEST_ONLY.
> >
> > > +
> > > + igt_plane_set_fb(primary, NULL);
> > > + igt_plane_set_fb(cursor, NULL);
> > > +
> > > + igt_remove_fb(data->drm_fd, &primary_fb);
> > > + igt_output_set_pipe(output, PIPE_NONE);
> > >
> > > - return intel_gen(devid) >= 7;
> > > + igt_skip_on_f(ret, "Cursor size %dx%d not supported by driver\n",
> > > w,
> > > h);
> > > }
> > >
> > > static void test_cursor_size(data_t *data)
> > > @@ -697,27 +714,33 @@ static void run_tests_on_pipe(data_t *data, enum
> > > pipe
> > > pipe)
> > > create_cursor_fb(data, w, h);
> > > }
> > >
> > > - /* Using created cursor FBs to test cursor support */
> > > - igt_describe("Check if a given-size cursor is well-
> > > positioned inside the screen.");
> > > - igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen",
> > > kmstest_pipe_name(pipe), w, h)
> > > - run_test(da
Re: [Nouveau] [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_cursor_crc: Probe kernel for cursor size support
On Fri, 2021-03-19 at 17:01 +0200, Ville Syrjälä wrote:
> On Thu, Mar 18, 2021 at 06:21:23PM -0400, Lyude wrote:
> > From: Lyude Paul
> >
> > Currently we just assume that every cursor size up to data->cursor_max_w/h
> > will
> > be supported by the driver, and check for support of nonsquare cursors by
> > checking if we're running on u815 and if so, which variant of intel hardware
> > we're running on. This isn't really ideal as we're about to enable 32x32
> > cursor
> > size tests for nouveau, and Intel hardware doesn't support cursor sizes that
> > small.
> >
> > So, fix this by removing has_nonsquare_cursors() and replacing it with a
> > more
> > generic require_cursor_size() function which checks whether or not the
> > driver
> > we're using supports a given cursor size by attempting a test-only atomic
> > commit.
> >
> > Signed-off-by: Lyude Paul
> > Cc: Martin Peres
> > Cc: Ben Skeggs
> > Cc: Jeremy Cline
> > ---
> > tests/kms_cursor_crc.c | 131 -
> > 1 file changed, 76 insertions(+), 55 deletions(-)
> >
> > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > index 3541ea06..b9c05472 100644
> > --- a/tests/kms_cursor_crc.c
> > +++ b/tests/kms_cursor_crc.c
> > @@ -523,26 +523,43 @@ static void create_cursor_fb(data_t *data, int cur_w,
> > int cur_h)
> > igt_put_cairo_ctx(cr);
> > }
> >
> > -static bool has_nonsquare_cursors(data_t *data)
> > +static void require_cursor_size(data_t *data, int w, int h)
> > {
> > - uint32_t devid;
> > + igt_fb_t primary_fb;
> > + drmModeModeInfo *mode;
> > + igt_display_t *display = &data->display;
> > + igt_output_t *output = data->output;
> > + igt_plane_t *primary, *cursor;
> > + int ret;
> >
> > - if (!is_i915_device(data->drm_fd))
> > - return false;
> > + igt_output_set_pipe(output, data->pipe);
> >
> > - devid = intel_get_drm_devid(data->drm_fd);
> > + mode = igt_output_get_mode(output);
> > + primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> > + cursor = igt_output_get_plane_type(output, DRM_PLANE_TYPE_CURSOR);
> >
> > - /*
> > - * Test non-square cursors a bit on the platforms
> > - * that support such things.
> > - */
> > - if (devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G)
> > - return true;
> > + /* Create temporary primary fb for testing */
> > + igt_assert(igt_create_fb(data->drm_fd, mode->hdisplay, mode-
> > >vdisplay, DRM_FORMAT_XRGB,
> > + LOCAL_DRM_FORMAT_MOD_NONE, &primary_fb));
> >
> > - if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid))
> > - return false;
> > + igt_plane_set_fb(primary, &primary_fb);
> > + igt_plane_set_fb(cursor, &data->fb);
> > + igt_plane_set_size(cursor, w, h);
> > + igt_fb_set_size(&data->fb, cursor, w, h);
> > +
> > + /* Test if the kernel supports the given cursor size or not */
> > + ret = igt_display_try_commit_atomic(display,
> > + DRM_MODE_ATOMIC_TEST_ONLY |
> > + DRM_MODE_ATOMIC_ALLOW_MODESET,
> > + NULL);
>
> Would be better to not depend on atomic. We have platforms
> that don't expose it yet.
Do you have any other ideas how we could probe for this then? it seems like the
only alternative would be to add intel-specific checks to fix that, or add some
ioctl for querying the minimum cursor size (which sounds preferable imo). would
the latter work for you, or do you have another idea?
>
> > +
> > + igt_plane_set_fb(primary, NULL);
> > + igt_plane_set_fb(cursor, NULL);
> > +
> > + igt_remove_fb(data->drm_fd, &primary_fb);
> > + igt_output_set_pipe(output, PIPE_NONE);
> >
> > - return intel_gen(devid) >= 7;
> > + igt_skip_on_f(ret, "Cursor size %dx%d not supported by driver\n", w,
> > h);
> > }
> >
> > static void test_cursor_size(data_t *data)
> > @@ -697,27 +714,33 @@ static void run_tests_on_pipe(data_t *data, enum pipe
> > pipe)
> > create_cursor_fb(data, w, h);
> > }
> >
> > - /* Using created cursor FBs to test cursor support */
> > - igt_describe("Check if a given-size cursor is well-
> > positioned inside the screen.");
> > - igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen",
> > kmstest_pipe_name(pipe), w, h)
> > - run_test(data, test_crc_onscreen, w, h);
> > -
> > - igt_describe("Check if a given-size cursor is well-
> > positioned outside the screen.");
> > - igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen",
> > kmstest_pipe_name(pipe), w, h)
> > - run_test(data, test_crc_offscreen, w, h);
> > -
> > - igt_describe("Check the smooth and p
Re: [Nouveau] [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_cursor_crc: Probe kernel for cursor size support
On Thu, Mar 18, 2021 at 06:21:23PM -0400, Lyude wrote:
> From: Lyude Paul
>
> Currently we just assume that every cursor size up to data->cursor_max_w/h
> will
> be supported by the driver, and check for support of nonsquare cursors by
> checking if we're running on u815 and if so, which variant of intel hardware
> we're running on. This isn't really ideal as we're about to enable 32x32
> cursor
> size tests for nouveau, and Intel hardware doesn't support cursor sizes that
> small.
>
> So, fix this by removing has_nonsquare_cursors() and replacing it with a more
> generic require_cursor_size() function which checks whether or not the driver
> we're using supports a given cursor size by attempting a test-only atomic
> commit.
>
> Signed-off-by: Lyude Paul
> Cc: Martin Peres
> Cc: Ben Skeggs
> Cc: Jeremy Cline
> ---
> tests/kms_cursor_crc.c | 131 -
> 1 file changed, 76 insertions(+), 55 deletions(-)
>
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 3541ea06..b9c05472 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -523,26 +523,43 @@ static void create_cursor_fb(data_t *data, int cur_w,
> int cur_h)
> igt_put_cairo_ctx(cr);
> }
>
> -static bool has_nonsquare_cursors(data_t *data)
> +static void require_cursor_size(data_t *data, int w, int h)
> {
> - uint32_t devid;
> + igt_fb_t primary_fb;
> + drmModeModeInfo *mode;
> + igt_display_t *display = &data->display;
> + igt_output_t *output = data->output;
> + igt_plane_t *primary, *cursor;
> + int ret;
>
> - if (!is_i915_device(data->drm_fd))
> - return false;
> + igt_output_set_pipe(output, data->pipe);
>
> - devid = intel_get_drm_devid(data->drm_fd);
> + mode = igt_output_get_mode(output);
> + primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> + cursor = igt_output_get_plane_type(output, DRM_PLANE_TYPE_CURSOR);
>
> - /*
> - * Test non-square cursors a bit on the platforms
> - * that support such things.
> - */
> - if (devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G)
> - return true;
> + /* Create temporary primary fb for testing */
> + igt_assert(igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> DRM_FORMAT_XRGB,
> + LOCAL_DRM_FORMAT_MOD_NONE, &primary_fb));
>
> - if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid))
> - return false;
> + igt_plane_set_fb(primary, &primary_fb);
> + igt_plane_set_fb(cursor, &data->fb);
> + igt_plane_set_size(cursor, w, h);
> + igt_fb_set_size(&data->fb, cursor, w, h);
> +
> + /* Test if the kernel supports the given cursor size or not */
> + ret = igt_display_try_commit_atomic(display,
> + DRM_MODE_ATOMIC_TEST_ONLY |
> + DRM_MODE_ATOMIC_ALLOW_MODESET,
> + NULL);
Would be better to not depend on atomic. We have platforms
that don't expose it yet.
> +
> + igt_plane_set_fb(primary, NULL);
> + igt_plane_set_fb(cursor, NULL);
> +
> + igt_remove_fb(data->drm_fd, &primary_fb);
> + igt_output_set_pipe(output, PIPE_NONE);
>
> - return intel_gen(devid) >= 7;
> + igt_skip_on_f(ret, "Cursor size %dx%d not supported by driver\n", w, h);
> }
>
> static void test_cursor_size(data_t *data)
> @@ -697,27 +714,33 @@ static void run_tests_on_pipe(data_t *data, enum pipe
> pipe)
> create_cursor_fb(data, w, h);
> }
>
> - /* Using created cursor FBs to test cursor support */
> - igt_describe("Check if a given-size cursor is well-positioned
> inside the screen.");
> - igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen",
> kmstest_pipe_name(pipe), w, h)
> - run_test(data, test_crc_onscreen, w, h);
> -
> - igt_describe("Check if a given-size cursor is well-positioned
> outside the screen.");
> - igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen",
> kmstest_pipe_name(pipe), w, h)
> - run_test(data, test_crc_offscreen, w, h);
> -
> - igt_describe("Check the smooth and pixel-by-pixel given-size
> cursor movements on"
> - "horizontal, vertical and diagonal.");
> - igt_subtest_f("pipe-%s-cursor-%dx%d-sliding",
> kmstest_pipe_name(pipe), w, h)
> - run_test(data, test_crc_sliding, w, h);
> -
> - igt_describe("Check random placement of a cursor with given
> size.");
> - igt_subtest_f("pipe-%s-cursor-%dx%d-random",
> kmstest_pipe_name(pipe), w, h)
> - run_test(data, test_crc_random, w, h);
> -
> - igt_describe("Check the rapid update of given-size cursor
> movements.");
> - igt_subtest_f("pipe-%s-cursor-%dx%d-rapid-mov
Re: [Nouveau] [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_cursor_crc: Probe kernel for cursor size support
On 19/03/2021 00:21, Lyude wrote:
From: Lyude Paul
Currently we just assume that every cursor size up to data->cursor_max_w/h will
be supported by the driver, and check for support of nonsquare cursors by
checking if we're running on u815 and if so, which variant of intel hardware
we're running on. This isn't really ideal as we're about to enable 32x32 cursor
size tests for nouveau, and Intel hardware doesn't support cursor sizes that
small.
So, fix this by removing has_nonsquare_cursors() and replacing it with a more
generic require_cursor_size() function which checks whether or not the driver
we're using supports a given cursor size by attempting a test-only atomic
commit.
Looks clean to me, and results in no new failures in Intel's results
(https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5625/shards-all.html?testfilter=kms_cursor_crc),
and no changes in the results for any other test.
Not sure I can comment on the implementation details of
require_cursor_size(), but everything else, and the series is:
Reviewed-by: Martin Peres
Signed-off-by: Lyude Paul
Cc: Martin Peres
Cc: Ben Skeggs
Cc: Jeremy Cline
---
tests/kms_cursor_crc.c | 131 -
1 file changed, 76 insertions(+), 55 deletions(-)
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 3541ea06..b9c05472 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -523,26 +523,43 @@ static void create_cursor_fb(data_t *data, int cur_w, int
cur_h)
igt_put_cairo_ctx(cr);
}
-static bool has_nonsquare_cursors(data_t *data)
+static void require_cursor_size(data_t *data, int w, int h)
{
- uint32_t devid;
+ igt_fb_t primary_fb;
+ drmModeModeInfo *mode;
+ igt_display_t *display = &data->display;
+ igt_output_t *output = data->output;
+ igt_plane_t *primary, *cursor;
+ int ret;
- if (!is_i915_device(data->drm_fd))
- return false;
+ igt_output_set_pipe(output, data->pipe);
- devid = intel_get_drm_devid(data->drm_fd);
+ mode = igt_output_get_mode(output);
+ primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+ cursor = igt_output_get_plane_type(output, DRM_PLANE_TYPE_CURSOR);
- /*
-* Test non-square cursors a bit on the platforms
-* that support such things.
-*/
- if (devid == PCI_CHIP_845_G || devid == PCI_CHIP_I865_G)
- return true;
+ /* Create temporary primary fb for testing */
+ igt_assert(igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
DRM_FORMAT_XRGB,
+LOCAL_DRM_FORMAT_MOD_NONE, &primary_fb));
- if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid))
- return false;
+ igt_plane_set_fb(primary, &primary_fb);
+ igt_plane_set_fb(cursor, &data->fb);
+ igt_plane_set_size(cursor, w, h);
+ igt_fb_set_size(&data->fb, cursor, w, h);
+
+ /* Test if the kernel supports the given cursor size or not */
+ ret = igt_display_try_commit_atomic(display,
+ DRM_MODE_ATOMIC_TEST_ONLY |
+ DRM_MODE_ATOMIC_ALLOW_MODESET,
+ NULL);
+
+ igt_plane_set_fb(primary, NULL);
+ igt_plane_set_fb(cursor, NULL);
+
+ igt_remove_fb(data->drm_fd, &primary_fb);
+ igt_output_set_pipe(output, PIPE_NONE);
- return intel_gen(devid) >= 7;
+ igt_skip_on_f(ret, "Cursor size %dx%d not supported by driver\n", w, h);
}
static void test_cursor_size(data_t *data)
@@ -697,27 +714,33 @@ static void run_tests_on_pipe(data_t *data, enum pipe
pipe)
create_cursor_fb(data, w, h);
}
- /* Using created cursor FBs to test cursor support */
- igt_describe("Check if a given-size cursor is well-positioned inside
the screen.");
- igt_subtest_f("pipe-%s-cursor-%dx%d-onscreen",
kmstest_pipe_name(pipe), w, h)
- run_test(data, test_crc_onscreen, w, h);
-
- igt_describe("Check if a given-size cursor is well-positioned
outside the screen.");
- igt_subtest_f("pipe-%s-cursor-%dx%d-offscreen",
kmstest_pipe_name(pipe), w, h)
- run_test(data, test_crc_offscreen, w, h);
-
- igt_describe("Check the smooth and pixel-by-pixel given-size cursor
movements on"
-"horizontal, vertical and diagonal.");
- igt_subtest_f("pipe-%s-cursor-%dx%d-sliding",
kmstest_pipe_name(pipe), w, h)
- run_test(data, test_crc_sliding, w, h);
-
- igt_describe("Check random placement of a cursor with given
size.");
- igt_subtest_f("pipe-%s-cursor-%dx%d-random",
kmstest_pipe_name(pipe), w, h)
- run_test(data, test_crc_random, w, h);
-
- igt_describe("Ch
