Re: [igt-dev] [PATCH i-g-t] tests: Make a fb-tiling subtest to check for invalid tiling changes.

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

Look good to me. Those autotools and meson file changes I think you'll 
need to rebase but with that

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

/Juha-Pekka


On 30.10.2018 19.17, Maarten Lankhorst wrote:
> In 2013 we prevented changing the tiling of an active framebuffer:
> 
> commit 80075d492f8773209e26d11d6bb13ba624ef95a4
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Oct 9 21:23:52 2013 +0200
> 
>      drm/i915: prevent tiling changes on framebuffer backing storage
> 
> After this we no longer have any use for the bad-tiling subtest, and
> only have to make sure changing tiling on a fb is not allowed.
> 
> Remove the original bad-tiling subtest, and add a i915_fb_tiling
> subtest for this case.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>   tests/Makefile.sources |  3 +++
>   tests/i915/fb_tiling.c | 46 ++++++++++++++++++++++++++++++++++++++++++
>   tests/kms_flip.c       | 35 ++------------------------------
>   tests/meson.build      |  1 +
>   4 files changed, 52 insertions(+), 33 deletions(-)
>   create mode 100644 tests/i915/fb_tiling.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index bdf49d5d5b98..b1aa6a02a03f 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -479,6 +479,9 @@ gen3_render_tiledx_blits_SOURCES = i915/gen3_render_tiledx_blits.c
>   TESTS_progs += gen3_render_tiledy_blits
>   gen3_render_tiledy_blits_SOURCES = i915/gen3_render_tiledy_blits.c
>   
> +TESTS_progs += fb_tiling
> +i915_fb_tiling_SOURCES = i915/fb_tiling.c
> +
>   TESTS_progs += i915_query
>   i915_query_SOURCES = i915/query.c
>   
> diff --git a/tests/i915/fb_tiling.c b/tests/i915/fb_tiling.c
> new file mode 100644
> index 000000000000..7d5c3f1fafe6
> --- /dev/null
> +++ b/tests/i915/fb_tiling.c
> @@ -0,0 +1,46 @@
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "igt.h"
> +
> +IGT_TEST_DESCRIPTION("Object tiling must be fixed after framebuffer creation.");
> +
> +igt_simple_main
> +{
> +	int drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +	struct igt_fb fb;
> +	int ret;
> +
> +	igt_create_fb(drm_fd, 512, 512, DRM_FORMAT_XRGB8888,
> +		      LOCAL_I915_FORMAT_MOD_X_TILED, &fb);
> +
> +	ret = __gem_set_tiling(drm_fd, fb.gem_handle, I915_TILING_X, fb.strides[0]);
> +	igt_assert_eq(ret, 0);
> +
> +	ret = __gem_set_tiling(drm_fd, fb.gem_handle, I915_TILING_NONE, fb.strides[0]);
> +	igt_assert_eq(ret, -EBUSY);
> +
> +	igt_remove_fb(drm_fd, &fb);
> +	close(drm_fd);
> +}
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index f28272dd1e3f..dc70ee23dc0a 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -60,7 +60,7 @@
>   #define TEST_RMFB		(1 << 14)
>   #define TEST_HANG		(1 << 15)
>   #define TEST_NOEVENT		(1 << 16)
> -#define TEST_FB_BAD_TILING	(1 << 17)
> +
>   #define TEST_SINGLE_BUFFER	(1 << 18)
>   #define TEST_DPMS_OFF		(1 << 19)
>   #define TEST_NO_2X_OUTPUT	(1 << 20)
> @@ -603,20 +603,6 @@ static void recreate_fb(struct test_output *o)
>   	o->fb_info[o->current_fb_id].fb_id = new_fb_id;
>   }
>   
> -static void set_y_tiling(struct test_output *o, int fb_idx)
> -{
> -	drmModeFBPtr r;
> -	struct igt_fb *fb_info = &o->fb_info[fb_idx];
> -
> -	/* Call rmfb/getfb/addfb to ensure those don't introduce stalls */
> -	r = drmModeGetFB(drm_fd, fb_info->fb_id);
> -	igt_assert(r);
> -	/* Newer kernels don't allow such shenagians any more, so skip the test. */
> -	igt_require(__gem_set_tiling(drm_fd, r->handle, I915_TILING_Y, fb_info->strides[0]) == 0);
> -	gem_close(drm_fd, r->handle);
> -	drmFree(r);
> -}
> -
>   static igt_hang_t hang_gpu(int fd)
>   {
>   	return igt_hang_ring(fd, I915_EXEC_DEFAULT);
> @@ -695,9 +681,6 @@ static unsigned int run_test_step(struct test_output *o)
>   		recreate_fb(o);
>   	new_fb_id = o->fb_ids[o->current_fb_id];
>   
> -	if (o->flags & TEST_FB_BAD_TILING)
> -		new_fb_id = o->fb_ids[2];
> -
>   	if ((o->flags & TEST_VBLANK_EXPIRED_SEQ) &&
>   	    !(o->pending_events & EVENT_VBLANK) && o->flip_state.count > 0) {
>   		struct vblank_reply reply;
> @@ -723,9 +706,6 @@ static unsigned int run_test_step(struct test_output *o)
>   	if (do_flip && (o->flags & TEST_EINVAL) && o->flip_state.count > 0)
>   		igt_assert(do_page_flip(o, new_fb_id, true) == expected_einval);
>   
> -	if (o->flags & TEST_FB_BAD_TILING)
> -		new_fb_id = o->fb_ids[o->current_fb_id];
> -
>   	if (do_vblank && (o->flags & TEST_EINVAL) && o->vblank_state.count > 0)
>   		igt_assert(do_wait_for_vblank(o, o->pipe, target_seq, &vbl_reply)
>   		       == -EINVAL);
> @@ -840,7 +820,7 @@ static unsigned int run_test_step(struct test_output *o)
>   		igt_assert(do_wait_for_vblank(o, o->pipe, target_seq, &vbl_reply)
>   			   == -EINVAL);
>   
> -	if (do_flip && (o->flags & TEST_EINVAL) && !(o->flags & TEST_FB_BAD_TILING))
> +	if (do_flip && (o->flags & TEST_EINVAL))
>   		igt_assert(do_page_flip(o, new_fb_id, true) == expected_einval);
>   
>   	unhang_gpu(drm_fd, hang);
> @@ -1265,22 +1245,12 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
>   	igt_assert(o->fb_ids[0]);
>   	igt_assert(o->fb_ids[1]);
>   
> -	if (o->flags & TEST_FB_BAD_TILING) {
> -		o->fb_ids[2] = igt_create_fb(drm_fd, o->fb_width, o->fb_height,
> -				igt_bpp_depth_to_drm_format(o->bpp, o->depth),
> -				LOCAL_I915_FORMAT_MOD_X_TILED, &o->fb_info[2]);
> -		igt_require(o->fb_ids[2]);
> -	}
> -
>   	paint_flip_mode(&o->fb_info[0], false);
>   	if (!(o->flags & TEST_BO_TOOBIG))
>   		paint_flip_mode(&o->fb_info[1], true);
>   	if (o->fb_ids[2])
>   		paint_flip_mode(&o->fb_info[2], true);
>   
> -	if (o->flags & TEST_FB_BAD_TILING)
> -		set_y_tiling(o, 2);
> -
>   	for (i = 0; i < o->count; i++)
>   		kmstest_dump_mode(&o->kmode[i]);
>   
> @@ -1556,7 +1526,6 @@ int main(int argc, char **argv)
>   			TEST_CHECK_TS, "flip-vs-blocking-wf-vblank" },
>   		{ 30, TEST_FLIP | TEST_MODESET | TEST_HANG | TEST_NOEVENT, "flip-vs-modeset-vs-hang" },
>   		{ 30, TEST_FLIP | TEST_PAN | TEST_HANG, "flip-vs-panning-vs-hang" },
> -		{ 1, TEST_FLIP | TEST_EINVAL | TEST_FB_BAD_TILING, "flip-vs-bad-tiling" },
>   
>   		{ 1, TEST_DPMS_OFF | TEST_MODESET | TEST_FLIP,
>   					"flip-vs-dpms-off-vs-modeset" },
> diff --git a/tests/meson.build b/tests/meson.build
> index b23d34ce7a95..878c4a0c7465 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -98,6 +98,7 @@ test_progs = [
>   ]
>   
>   i915_progs = [
> +	'fb_tiling',
>   	'query',
>   ]
>   
> 

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev