[igt-dev] [PATCH i-g-t v4 4/6] kms_writeback: Add writeback-check-output

[Brian Starkey]

Hi Liviu,

On Mon, Dec 10, 2018 at 01:54:05PM +0000, Liviu Dudau wrote:
> On Fri, Nov 09, 2018 at 03:09:54PM +0000, Brian Starkey wrote:
> > >+static void writeback_sequence(igt_output_t *output, igt_plane_t *plane,
> > >+				igt_fb_t *in_fb, igt_fb_t *out_fbs[], int n_commits)
> > >+{
> > >+	int i, color_idx = 0;
> > >+	double in_fb_colors[2][3] = {
> > >+		{ 1.0, 0.0, 0.0 },
> > >+		{ 0.0, 1.0, 0.0 },
> > >+	};
> > >+	double clear_color[3] = { 1.0, 1.0, 1.0 };
> > >+	igt_crc_t cleared_crc, out_expected;
> > >+
> > >+	for (i = 0; i < n_commits; i++, color_idx++) {
> > >+		/* Change the input color each time */
> > >+		fill_fb(in_fb, in_fb_colors[color_idx % 2]);
> > >+
> > >+		if (out_fbs[i]) {
> > >+			igt_crc_t out_before;
> > >+
> > >+			/* Get the expected CRC */
> > >+			fill_fb(out_fbs[i], in_fb_colors[color_idx % 2]);
> > >+			igt_fb_get_crc(out_fbs[i], &out_expected);
> > >+
> > >+			fill_fb(out_fbs[i], clear_color);
> > >+			if (i == 0)
> > >+				igt_fb_get_crc(out_fbs[i], &cleared_crc);
> > >+			igt_fb_get_crc(out_fbs[i], &out_before);
> > >+			igt_assert_crc_equal(&cleared_crc, &out_before);
> > >+		}
> > >+
> > >+		/* Commit */
> > >+		igt_plane_set_fb(plane, in_fb);
> > >+		if (out_fbs[i])
> > >+			igt_output_set_writeback_fb(output, out_fbs[i]);
> > >+		else
> > >+			/* avoid setting the out fence ptr that is done via igt_output_set_writeback_fb() */
> > >+			igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
> > 
> > Would it work to put this check in igt_output_set_writeback_fb() so
> > that it only requests an out-fence if fb_id != 0 ?
> 
> It would work, but then we should drop or modify the first invalid_out_fence test
> as the commit will fail, rather than set the out_fence_ptr to -1.

It looks like invalid_out_fence is explicitly calling
igt_output_set_prop_value() rather than the
igt_output_set_writeback_fb() helper, so we should be able to test
whatever we want there.

My theory is that given this fb_id != 0 check is the "correct" thing
in the normal case, we could put it in the helper. Any tests which
need to explicitly do the incorrect thing can open-code whatever
nastiness they're trying to exercise.

Cheers,
-Brian