Message ID | 1446169473-20563-1-git-send-email-vivek.kasireddy@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 30/10/15 01:44, Vivek Kasireddy wrote: > The main goal of this subtest is to trigger the following warning in > the function i915_gem_object_get_fence(): > if (WARN_ON(!obj->map_and_fenceable)) > > To trigger this warning, the subtest first creates a Y-tiled object and > an associated framebuffer with the Y-fb modifier. Furthermore, to > prevent the map_and_fenceable from being set, we make sure that > the object does not have a normal VMA by refraining from rendering to the > object and by setting the rotation property upfront before calling commit. > > v2: Do not call paint_squares and just use one output. > > v3: Convert an if condition to igt_require and move the plane rotation > requirement further up before the fb allocation. > > v4: After setting rotation to 90 and committing, change the rotation to > 0 and commit once more. This is to test if the i915 driver hits any > warnings while pinning and unpinning an object that has both normal > and rotated views. > > v5: > - Add another subtest to toggle the order of rotation > - Exhaustively test the i915 driver's pinning and unpinning code paths > for any fence leaks by iterating until MAX available fences. > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > --- > tests/kms_rotation_crc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 84 insertions(+) > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c > index cc9847e..34f8150 100644 > --- a/tests/kms_rotation_crc.c > +++ b/tests/kms_rotation_crc.c > @@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type) > igt_require_f(valid_tests, "no valid crtc/connector combinations found\n"); > } > > +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type, > + int toggle) > +{ > + igt_display_t *display = &data->display; > + uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; > + uint32_t format = DRM_FORMAT_XRGB8888; > + int bpp = igt_drm_format_to_bpp(format); > + enum igt_commit_style commit = COMMIT_LEGACY; > + int fd = data->gfx_fd; > + igt_output_t *output = &display->outputs[0]; > + igt_plane_t *plane; > + drmModeModeInfo *mode; > + unsigned int stride, size, w, h; > + uint32_t gem_handle; > + int num_fences = gem_available_fences(fd); > + int i, ret; > + > + igt_require(output != NULL && output->valid == true); > + > + plane = igt_output_get_plane(output, plane_type); > + igt_require(igt_plane_supports_rotation(plane)); > + > + if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) { > + igt_require(data->display.has_universal_planes); > + commit = COMMIT_UNIVERSAL; > + } > + > + mode = igt_output_get_mode(output); > + w = mode->hdisplay; > + h = mode->vdisplay; > + > + for (stride = 512; stride < (w * bpp / 8); stride *= 2) > + ; > + for (size = 1024*1024; size < stride * h; size *= 2) > + ; > + > + gem_handle = gem_create(fd, size); > + ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride); > + igt_assert(ret == 0); > + > + do_or_die(__kms_addfb(fd, gem_handle, w, h, stride, > + format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS, > + &data->fb.fb_id)); > + data->fb.width = w; > + data->fb.height = h; > + data->fb.gem_handle = gem_handle; > + > + igt_plane_set_fb(plane, NULL); > + igt_display_commit(display); > + > + igt_plane_set_fb(plane, &data->fb); > + > + for (i = 0; i < num_fences + 1; i++) { > + igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_0 : IGT_ROTATION_90); > + drmModeObjectSetProperty(fd, plane->drm_plane->plane_id, > + DRM_MODE_OBJECT_PLANE, > + plane->rotation_property, > + plane->rotation); > + ret = igt_display_try_commit2(display, commit); > + igt_assert(ret == 0); > + > + igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_90 : IGT_ROTATION_0); > + drmModeObjectSetProperty(fd, plane->drm_plane->plane_id, > + DRM_MODE_OBJECT_PLANE, > + plane->rotation_property, > + plane->rotation); > + ret = igt_display_try_commit2(display, commit); > + igt_assert(ret == 0); > + } It manages to exhaust the fences with only one object? I was expecting that multiple objects will be required since if it is only one how come it allocates more than one fence register? Regards, Tvrtko
Hi Tvrtko, On Fri, 30 Oct 2015 10:22:08 +0000 Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > > On 30/10/15 01:44, Vivek Kasireddy wrote: > > The main goal of this subtest is to trigger the following warning in > > the function i915_gem_object_get_fence(): > > if (WARN_ON(!obj->map_and_fenceable)) > > > > To trigger this warning, the subtest first creates a Y-tiled object > > and an associated framebuffer with the Y-fb modifier. Furthermore, > > to prevent the map_and_fenceable from being set, we make sure that > > the object does not have a normal VMA by refraining from rendering > > to the object and by setting the rotation property upfront before > > calling commit. > > > > v2: Do not call paint_squares and just use one output. > > > > v3: Convert an if condition to igt_require and move the plane > > rotation requirement further up before the fb allocation. > > > > v4: After setting rotation to 90 and committing, change the > > rotation to 0 and commit once more. This is to test if the i915 > > driver hits any warnings while pinning and unpinning an object that > > has both normal and rotated views. > > > > v5: > > - Add another subtest to toggle the order of rotation > > - Exhaustively test the i915 driver's pinning and unpinning code > > paths for any fence leaks by iterating until MAX available fences. > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > > --- > > tests/kms_rotation_crc.c | 84 > > ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 > > insertions(+) > > > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c > > index cc9847e..34f8150 100644 > > --- a/tests/kms_rotation_crc.c > > +++ b/tests/kms_rotation_crc.c > > @@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data, > > enum igt_plane plane_type) igt_require_f(valid_tests, "no valid > > crtc/connector combinations found\n"); } > > > > +static void test_plane_rotation_ytiled_obj(data_t *data, enum > > igt_plane plane_type, > > + int toggle) > > +{ > > + igt_display_t *display = &data->display; > > + uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; > > + uint32_t format = DRM_FORMAT_XRGB8888; > > + int bpp = igt_drm_format_to_bpp(format); > > + enum igt_commit_style commit = COMMIT_LEGACY; > > + int fd = data->gfx_fd; > > + igt_output_t *output = &display->outputs[0]; > > + igt_plane_t *plane; > > + drmModeModeInfo *mode; > > + unsigned int stride, size, w, h; > > + uint32_t gem_handle; > > + int num_fences = gem_available_fences(fd); > > + int i, ret; > > + > > + igt_require(output != NULL && output->valid == true); > > + > > + plane = igt_output_get_plane(output, plane_type); > > + igt_require(igt_plane_supports_rotation(plane)); > > + > > + if (plane_type == IGT_PLANE_PRIMARY || plane_type == > > IGT_PLANE_CURSOR) { > > + igt_require(data->display.has_universal_planes); > > + commit = COMMIT_UNIVERSAL; > > + } > > + > > + mode = igt_output_get_mode(output); > > + w = mode->hdisplay; > > + h = mode->vdisplay; > > + > > + for (stride = 512; stride < (w * bpp / 8); stride *= 2) > > + ; > > + for (size = 1024*1024; size < stride * h; size *= 2) > > + ; > > + > > + gem_handle = gem_create(fd, size); > > + ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, > > stride); > > + igt_assert(ret == 0); > > + > > + do_or_die(__kms_addfb(fd, gem_handle, w, h, stride, > > + format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS, > > + &data->fb.fb_id)); > > + data->fb.width = w; > > + data->fb.height = h; > > + data->fb.gem_handle = gem_handle; > > + > > + igt_plane_set_fb(plane, NULL); > > + igt_display_commit(display); > > + > > + igt_plane_set_fb(plane, &data->fb); > > + > > + for (i = 0; i < num_fences + 1; i++) { > > + igt_plane_set_rotation(plane, toggle ? > > IGT_ROTATION_0 : IGT_ROTATION_90); > > + drmModeObjectSetProperty(fd, > > plane->drm_plane->plane_id, > > + DRM_MODE_OBJECT_PLANE, > > + plane->rotation_property, > > + plane->rotation); > > + ret = igt_display_try_commit2(display, commit); > > + igt_assert(ret == 0); > > + > > + igt_plane_set_rotation(plane, toggle ? > > IGT_ROTATION_90 : IGT_ROTATION_0); > > + drmModeObjectSetProperty(fd, > > plane->drm_plane->plane_id, > > + DRM_MODE_OBJECT_PLANE, > > + plane->rotation_property, > > + plane->rotation); > > + ret = igt_display_try_commit2(display, commit); > > + igt_assert(ret == 0); > > + } > > It manages to exhaust the fences with only one object? I was > expecting that multiple objects will be required since if it is only > one how come it allocates more than one fence register? Before I sent out this version, I did try with two objects to see if it triggers any WARNs but it didn't. I am not sure if I did it right though, I'll take a look at it again. However, when I tried this version, that is, single-object-multiple-views, it did trigger the following WARNS: WARN_ON(!ggtt_vma || dev_priv->fence_regs[obj->fence_reg].pin_count > ggtt_vma->pin_count) if (WARN_ON(fence->pin_count)) I tested these with v3 of my i915 driver patch. Of-course, with v4, I don't seen any WARNs. I'll take a closer look at triggering any WARNs with 2 or more objects. However, one thing that was not clear to me was when you said unpin both (in your previous comment), I was wondering how I can do that from the test. Did you mean something like this: igt_plane_set_fb(plane, NULL); igt_display_commit(display); Thanks and Regards, Vivek > > Regards, > > Tvrtko
On 31/10/15 01:45, Vivek Kasireddy wrote: > Hi Tvrtko, > > On Fri, 30 Oct 2015 10:22:08 +0000 > Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > >> >> On 30/10/15 01:44, Vivek Kasireddy wrote: >>> The main goal of this subtest is to trigger the following warning in >>> the function i915_gem_object_get_fence(): >>> if (WARN_ON(!obj->map_and_fenceable)) >>> >>> To trigger this warning, the subtest first creates a Y-tiled object >>> and an associated framebuffer with the Y-fb modifier. Furthermore, >>> to prevent the map_and_fenceable from being set, we make sure that >>> the object does not have a normal VMA by refraining from rendering >>> to the object and by setting the rotation property upfront before >>> calling commit. >>> >>> v2: Do not call paint_squares and just use one output. >>> >>> v3: Convert an if condition to igt_require and move the plane >>> rotation requirement further up before the fb allocation. >>> >>> v4: After setting rotation to 90 and committing, change the >>> rotation to 0 and commit once more. This is to test if the i915 >>> driver hits any warnings while pinning and unpinning an object that >>> has both normal and rotated views. >>> >>> v5: >>> - Add another subtest to toggle the order of rotation >>> - Exhaustively test the i915 driver's pinning and unpinning code >>> paths for any fence leaks by iterating until MAX available fences. >>> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> >>> --- >>> tests/kms_rotation_crc.c | 84 >>> ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 >>> insertions(+) >>> >>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c >>> index cc9847e..34f8150 100644 >>> --- a/tests/kms_rotation_crc.c >>> +++ b/tests/kms_rotation_crc.c >>> @@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data, >>> enum igt_plane plane_type) igt_require_f(valid_tests, "no valid >>> crtc/connector combinations found\n"); } >>> >>> +static void test_plane_rotation_ytiled_obj(data_t *data, enum >>> igt_plane plane_type, >>> + int toggle) >>> +{ >>> + igt_display_t *display = &data->display; >>> + uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; >>> + uint32_t format = DRM_FORMAT_XRGB8888; >>> + int bpp = igt_drm_format_to_bpp(format); >>> + enum igt_commit_style commit = COMMIT_LEGACY; >>> + int fd = data->gfx_fd; >>> + igt_output_t *output = &display->outputs[0]; >>> + igt_plane_t *plane; >>> + drmModeModeInfo *mode; >>> + unsigned int stride, size, w, h; >>> + uint32_t gem_handle; >>> + int num_fences = gem_available_fences(fd); >>> + int i, ret; >>> + >>> + igt_require(output != NULL && output->valid == true); >>> + >>> + plane = igt_output_get_plane(output, plane_type); >>> + igt_require(igt_plane_supports_rotation(plane)); >>> + >>> + if (plane_type == IGT_PLANE_PRIMARY || plane_type == >>> IGT_PLANE_CURSOR) { >>> + igt_require(data->display.has_universal_planes); >>> + commit = COMMIT_UNIVERSAL; >>> + } >>> + >>> + mode = igt_output_get_mode(output); >>> + w = mode->hdisplay; >>> + h = mode->vdisplay; >>> + >>> + for (stride = 512; stride < (w * bpp / 8); stride *= 2) >>> + ; >>> + for (size = 1024*1024; size < stride * h; size *= 2) >>> + ; >>> + >>> + gem_handle = gem_create(fd, size); >>> + ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, >>> stride); >>> + igt_assert(ret == 0); >>> + >>> + do_or_die(__kms_addfb(fd, gem_handle, w, h, stride, >>> + format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS, >>> + &data->fb.fb_id)); >>> + data->fb.width = w; >>> + data->fb.height = h; >>> + data->fb.gem_handle = gem_handle; >>> + >>> + igt_plane_set_fb(plane, NULL); >>> + igt_display_commit(display); >>> + >>> + igt_plane_set_fb(plane, &data->fb); >>> + >>> + for (i = 0; i < num_fences + 1; i++) { >>> + igt_plane_set_rotation(plane, toggle ? >>> IGT_ROTATION_0 : IGT_ROTATION_90); >>> + drmModeObjectSetProperty(fd, >>> plane->drm_plane->plane_id, >>> + DRM_MODE_OBJECT_PLANE, >>> + plane->rotation_property, >>> + plane->rotation); >>> + ret = igt_display_try_commit2(display, commit); >>> + igt_assert(ret == 0); >>> + >>> + igt_plane_set_rotation(plane, toggle ? >>> IGT_ROTATION_90 : IGT_ROTATION_0); >>> + drmModeObjectSetProperty(fd, >>> plane->drm_plane->plane_id, >>> + DRM_MODE_OBJECT_PLANE, >>> + plane->rotation_property, >>> + plane->rotation); >>> + ret = igt_display_try_commit2(display, commit); >>> + igt_assert(ret == 0); >>> + } >> >> It manages to exhaust the fences with only one object? I was >> expecting that multiple objects will be required since if it is only >> one how come it allocates more than one fence register? > > Before I sent out this version, I did try with two objects to see if it > triggers any WARNs but it didn't. I am not sure if I did it right > though, I'll take a look at it again. However, when I tried this > version, that is, single-object-multiple-views, it did trigger the > following WARNS: > > WARN_ON(!ggtt_vma || dev_priv->fence_regs[obj->fence_reg].pin_count > > ggtt_vma->pin_count) > > if (WARN_ON(fence->pin_count)) > > I tested these with v3 of my i915 driver patch. Of-course, with v4, > I don't seen any WARNs. I'll take a closer look at triggering > any WARNs with 2 or more objects. However, one thing that was not clear I thought you would need number of framebuffers / objects > num_fences and display all of them to trigger the running out of fences ref leak. > to me was when you said unpin both (in your previous comment), I was > wondering how I can do that from the test. Did you mean something like > this: > > igt_plane_set_fb(plane, NULL); > igt_display_commit(display); Couldn't find in this thread where I said "unpin both" so don't know. Regards, Tvrtko
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c index cc9847e..34f8150 100644 --- a/tests/kms_rotation_crc.c +++ b/tests/kms_rotation_crc.c @@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type) igt_require_f(valid_tests, "no valid crtc/connector combinations found\n"); } +static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane plane_type, + int toggle) +{ + igt_display_t *display = &data->display; + uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED; + uint32_t format = DRM_FORMAT_XRGB8888; + int bpp = igt_drm_format_to_bpp(format); + enum igt_commit_style commit = COMMIT_LEGACY; + int fd = data->gfx_fd; + igt_output_t *output = &display->outputs[0]; + igt_plane_t *plane; + drmModeModeInfo *mode; + unsigned int stride, size, w, h; + uint32_t gem_handle; + int num_fences = gem_available_fences(fd); + int i, ret; + + igt_require(output != NULL && output->valid == true); + + plane = igt_output_get_plane(output, plane_type); + igt_require(igt_plane_supports_rotation(plane)); + + if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) { + igt_require(data->display.has_universal_planes); + commit = COMMIT_UNIVERSAL; + } + + mode = igt_output_get_mode(output); + w = mode->hdisplay; + h = mode->vdisplay; + + for (stride = 512; stride < (w * bpp / 8); stride *= 2) + ; + for (size = 1024*1024; size < stride * h; size *= 2) + ; + + gem_handle = gem_create(fd, size); + ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride); + igt_assert(ret == 0); + + do_or_die(__kms_addfb(fd, gem_handle, w, h, stride, + format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS, + &data->fb.fb_id)); + data->fb.width = w; + data->fb.height = h; + data->fb.gem_handle = gem_handle; + + igt_plane_set_fb(plane, NULL); + igt_display_commit(display); + + igt_plane_set_fb(plane, &data->fb); + + for (i = 0; i < num_fences + 1; i++) { + igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_0 : IGT_ROTATION_90); + drmModeObjectSetProperty(fd, plane->drm_plane->plane_id, + DRM_MODE_OBJECT_PLANE, + plane->rotation_property, + plane->rotation); + ret = igt_display_try_commit2(display, commit); + igt_assert(ret == 0); + + igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_90 : IGT_ROTATION_0); + drmModeObjectSetProperty(fd, plane->drm_plane->plane_id, + DRM_MODE_OBJECT_PLANE, + plane->rotation_property, + plane->rotation); + ret = igt_display_try_commit2(display, commit); + igt_assert(ret == 0); + } + + kmstest_restore_vt_mode(); + igt_remove_fb(fd, &data->fb); +} + igt_main { data_t data = {}; @@ -345,6 +419,16 @@ igt_main test_plane_rotation(&data, IGT_PLANE_PRIMARY); } + igt_subtest_f("primary-rotation-90-to-0-Y-tiled") { + igt_require(gen >= 9); + test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY, 0); + } + + igt_subtest_f("primary-rotation-0-to-90-Y-tiled") { + igt_require(gen >= 9); + test_plane_rotation_ytiled_obj(&data, IGT_PLANE_PRIMARY, 1); + } + igt_fixture { igt_display_fini(&data.display); }
The main goal of this subtest is to trigger the following warning in the function i915_gem_object_get_fence(): if (WARN_ON(!obj->map_and_fenceable)) To trigger this warning, the subtest first creates a Y-tiled object and an associated framebuffer with the Y-fb modifier. Furthermore, to prevent the map_and_fenceable from being set, we make sure that the object does not have a normal VMA by refraining from rendering to the object and by setting the rotation property upfront before calling commit. v2: Do not call paint_squares and just use one output. v3: Convert an if condition to igt_require and move the plane rotation requirement further up before the fb allocation. v4: After setting rotation to 90 and committing, change the rotation to 0 and commit once more. This is to test if the i915 driver hits any warnings while pinning and unpinning an object that has both normal and rotated views. v5: - Add another subtest to toggle the order of rotation - Exhaustively test the i915 driver's pinning and unpinning code paths for any fence leaks by iterating until MAX available fences. Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> --- tests/kms_rotation_crc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+)