Re: [PATCH i-g-t v4 6/6] tests/kms_ccs: Test case for wrong aux buffer stride size

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: daniel.vetter@intel.com, intel-gfx@lists.freedesktop.org,
	tomeu.vizoso@collabora.com
Subject: Re: [PATCH i-g-t v4 6/6] tests/kms_ccs: Test case for wrong aux buffer stride size
Date: Mon, 30 Oct 2017 18:04:02 +0200	[thread overview]
Message-ID: <20171030160402.GS10981@intel.com> (raw)
In-Reply-To: <20171003164059.GD19707@mail.bwidawsk.net>

On Tue, Oct 03, 2017 at 09:40:59AM -0700, Ben Widawsky wrote:
> On 17-09-27 15:34:19, Gabriel Krisman Bertazi wrote:
> >Two scenarios tested:
> >  - unaligned stride
> >  - Stride too small
> >
> >Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
> 
> Jason, could you provide your opinion on this? I've always felt the kernel
> interface shouldn't be validating stride at all.

I don't recall how the hardware reacts to a bogus CCS stride. In general
the SKL display engine is very sensitive and the slightest mistake might
end up hard hanging the box, so checking seems like the safer option.

> 
> >---
> > tests/kms_ccs.c | 23 ++++++++++++++++++++++-
> > 1 file changed, 22 insertions(+), 1 deletion(-)
> >
> >diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
> >index 2d7105eb1323..cb6e69c69ae5 100644
> >--- a/tests/kms_ccs.c
> >+++ b/tests/kms_ccs.c
> >@@ -35,14 +35,18 @@ enum test_flags {
> > 	TEST_BAD_ROTATION_90		= 1 << 4,
> > 	TEST_NO_AUX_BUFFER		= 1 << 5,
> > 	TEST_BAD_CCS_HANDLE		= 1 << 6,
> >+	TEST_BAD_AUX_STRIDE		= 1 << 7,
> > };
> >
> > #define TEST_FAIL_ON_ADDFB2 \
> >-	(TEST_BAD_PIXEL_FORMAT | TEST_NO_AUX_BUFFER | TEST_BAD_CCS_HANDLE)
> >+	(TEST_BAD_PIXEL_FORMAT | TEST_NO_AUX_BUFFER | TEST_BAD_CCS_HANDLE | \
> >+	 TEST_BAD_AUX_STRIDE)
> >
> > enum test_fb_flags {
> > 	FB_COMPRESSED			= 1 << 0,
> > 	FB_HAS_PLANE			= 1 << 1,
> >+	FB_MISALIGN_AUX_STRIDE		= 1 << 2,
> >+	FB_SMALL_AUX_STRIDE		= 1 << 3,
> > };
> >
> > typedef struct {
> >@@ -324,6 +328,14 @@ static void generate_fb(data_t *data, struct igt_fb *fb,
> > 		int ccs_pitches = ALIGN(ccs_width * 1, 128);
> > 		int ccs_offsets = size[0];
> >
> >+		if (fb_flags & FB_MISALIGN_AUX_STRIDE)
> >+			ccs_pitches -= 64;
> >+		else if (fb_flags & FB_SMALL_AUX_STRIDE) {
> >+			igt_skip_on_f(width <= 1024,
> >+				      "FB already has the smallest possible stride\n");
> >+			ccs_pitches = ALIGN(ccs_width/2, 128);
> >+		}
> >+
> > 		size[1] = ccs_pitches * ALIGN(ccs_height, 32);
> >
> > 		f.handles[0] = gem_create(data->drm_fd, size[0] + size[1]);
> >@@ -470,6 +482,11 @@ static void test_output(data_t *data)
> > 		try_config(data, fb_flags | FB_COMPRESSED);
> > 	}
> >
> >+	if (data->flags & TEST_BAD_AUX_STRIDE) {
> >+		try_config(data, fb_flags | FB_COMPRESSED | FB_MISALIGN_AUX_STRIDE);
> >+		try_config(data, fb_flags | FB_COMPRESSED | FB_SMALL_AUX_STRIDE);
> >+	}
> >+
> > 	primary = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
> > 	igt_plane_set_fb(primary, NULL);
> > 	igt_plane_set_rotation(primary, IGT_ROTATION_0);
> >@@ -543,6 +560,10 @@ igt_main
> > 		data.flags = TEST_BAD_CCS_HANDLE;
> > 		igt_subtest_f("pipe-%s-ccs-on-another-bo", pipe_name)
> > 			test_output(&data);
> >+
> >+		data.flags = TEST_BAD_AUX_STRIDE;
> >+		igt_subtest_f("pipe-%s-bad-aux-stride", pipe_name)
> >+			test_output(&data);
> > 	}
> >
> > 	igt_fixture
> >-- 
> >2.11.0
> >
> >_______________________________________________
> >Intel-gfx mailing list
> >Intel-gfx@lists.freedesktop.org
> >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx