diff mbox

[i-g-t] kms_cursor_crc: Add a subtest with a 256x256 gradient cursor

Message ID 20170310101835.29845-2-ander.conselvan.de.oliveira@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ander Conselvan de Oliveira March 10, 2017, 10:18 a.m. UTC 
Some of the kms_cursor_crc subtests where failing on Geminilake. The
root cause was an error on programming the pre-CSC gamma tables, which
led to small rounding errors that, although not sufficient to change the
image as captured at 8bpc with the Chamelium, were enough to generate a
different CRC. It is not clear why the rounding is different when the
cursor is enabled, but this was caught by chance, since the cursor would
only be exercised with limited color combinations.

To improve coverage, add a test that uses a gradient covering all
possible 8bit values for a channel.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 tests/kms_cursor_crc.c | 42 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)

Comments

Ander Conselvan de Oliveira March 10, 2017, 10:27 a.m. UTC | #1 
On Fri, 2017-03-10 at 12:18 +0200, Ander Conselvan de Oliveira wrote:
> Some of the kms_cursor_crc subtests where failing on Geminilake. The
> root cause was an error on programming the pre-CSC gamma tables, which
> led to small rounding errors that, although not sufficient to change the
> image as captured at 8bpc with the Chamelium, were enough to generate a
> different CRC. It is not clear why the rounding is different when the
> cursor is enabled, but this was caught by chance, since the cursor would
> only be exercised with limited color combinations.
> 
> To improve coverage, add a test that uses a gradient covering all
> possible 8bit values for a channel.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  tests/kms_cursor_crc.c | 42 ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 4851e18..0b232bf 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -87,6 +87,8 @@ static void draw_cursor(cairo_t *cr, int x, int y, int cw, int ch)
>  	igt_paint_color_alpha(cr, x + wl, y + ht, wr, hb, 0.5, 0.5, 0.5, 1.0);
>  }
>  
> +typedef void(*draw_cursor_cb)(cairo_t *cr, int x, int y, int cw, int ch);
> +
>  static void cursor_enable(data_t *data)
>  {
>  	igt_output_t *output = data->output;
> @@ -107,7 +109,8 @@ static void cursor_disable(data_t *data)
>  }
>  
>  
> -static void do_single_test(data_t *data, int x, int y)
> +static void do_single_test_with_cursor_cb(data_t *data, int x, int y,
> +					  draw_cursor_cb cursor_cb)
>  {
>  	igt_display_t *display = &data->display;
>  	igt_pipe_crc_t *pipe_crc = data->pipe_crc;
> @@ -151,7 +154,7 @@ static void do_single_test(data_t *data, int x, int y)
>  	igt_display_commit(display);
>  
>  	/* Now render the same in software and collect crc */
> -	draw_cursor(cr, x, y, data->curw, data->curh);
> +	cursor_cb(cr, x, y, data->curw, data->curh);
>  	igt_display_commit(display);
>  
>  	igt_wait_for_vblank(data->drm_fd, data->pipe);
> @@ -162,6 +165,11 @@ static void do_single_test(data_t *data, int x, int y)
>  	igt_paint_color(cr, 0, 0, data->screenw, data->screenh, 0.0, 0.0, 0.0);
>  }
>  
> +static void do_single_test(data_t *data, int x, int y)
> +{
> +	do_single_test_with_cursor_cb(data, x, y, draw_cursor);
> +}
> +
>  static void do_fail_test(data_t *data, int x, int y, int expect)
>  {
>  	igt_display_t *display = &data->display;
> @@ -386,7 +394,8 @@ static void run_test(data_t *data, void (*testfunc)(data_t *), int cursor_w, int
>  	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>  }
>  
> -static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
> +static void create_cursor_fb_with_cb(data_t *data, int cur_w, int cur_h,
> +				     draw_cursor_cb cursor_cb)
>  {
>  	cairo_t *cr;
>  	uint32_t fb_id;
> @@ -406,10 +415,15 @@ static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
>  	igt_assert(fb_id);
>  
>  	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
> -	draw_cursor(cr, 0, 0, cur_w, cur_h);
> +	cursor_cb(cr, 0, 0, cur_w, cur_h);
>  	igt_assert(cairo_status(cr) == 0);
>  }
>  
> +static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
> +{
> +	create_cursor_fb_with_cb(data, cur_w, cur_h, draw_cursor);
> +}
> +
>  static bool has_nonsquare_cursors(uint32_t devid)
>  {
>  	/*
> @@ -506,6 +520,23 @@ static void test_rapid_movement(data_t *data)
>  
>  }
>  
> +static void draw_cursor_gradient(cairo_t *cr, int x, int y, int cw, int ch)
> +{
> +	cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE);
> +	igt_paint_color_gradient(cr, x, y, cw, ch, 1.0, 1.0, 1.0);
> +}
> +
> +static void test_cursor_colors(data_t *data)
> +{
> +	int cursor_w = data->curw;
> +	int cursor_h = data->curh;
> +
> +	create_cursor_fb_with_cb(data, cursor_w, cursor_h,
> +				 draw_cursor_gradient);
> +	do_single_test_with_cursor_cb(data, 0, 0, draw_cursor_gradient);
> +	igt_remove_fb(data->drm_fd, &data->fb);
> +}
> +
>  static void run_test_generic(data_t *data)
>  {
>  	int cursor_size;
> @@ -613,6 +644,9 @@ igt_main
>  	igt_subtest_f("cursor-size-change")
>  		run_test(&data, test_cursor_size, cursor_width, cursor_height);
>  
> +	igt_subtest_f("gradient-cursor")

Just realized this needs an igt_require(max_cursor_[wh] >= 256).

Ander

> +		run_test(&data, test_cursor_colors, 256, 256);
> +
>  	run_test_generic(&data);
>  
>  	igt_fixture {
Ander Conselvan de Oliveira March 14, 2017, 2:24 p.m. UTC | #2 
On Fri, 2017-03-10 at 12:27 +0200, Ander Conselvan De Oliveira wrote:
> On Fri, 2017-03-10 at 12:18 +0200, Ander Conselvan de Oliveira wrote:
> > Some of the kms_cursor_crc subtests where failing on Geminilake. The
> > root cause was an error on programming the pre-CSC gamma tables, which
> > led to small rounding errors that, although not sufficient to change the
> > image as captured at 8bpc with the Chamelium, were enough to generate a
> > different CRC. It is not clear why the rounding is different when the
> > cursor is enabled, but this was caught by chance, since the cursor would
> > only be exercised with limited color combinations.
> > 
> > To improve coverage, add a test that uses a gradient covering all
> > possible 8bit values for a channel.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > ---
> >  tests/kms_cursor_crc.c | 42 ++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 38 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > index 4851e18..0b232bf 100644
> > --- a/tests/kms_cursor_crc.c
> > +++ b/tests/kms_cursor_crc.c
> > @@ -87,6 +87,8 @@ static void draw_cursor(cairo_t *cr, int x, int y, int cw, int ch)
> >  	igt_paint_color_alpha(cr, x + wl, y + ht, wr, hb, 0.5, 0.5, 0.5, 1.0);
> >  }
> >  
> > +typedef void(*draw_cursor_cb)(cairo_t *cr, int x, int y, int cw, int ch);
> > +
> >  static void cursor_enable(data_t *data)
> >  {
> >  	igt_output_t *output = data->output;
> > @@ -107,7 +109,8 @@ static void cursor_disable(data_t *data)
> >  }
> >  
> >  
> > -static void do_single_test(data_t *data, int x, int y)
> > +static void do_single_test_with_cursor_cb(data_t *data, int x, int y,
> > +					  draw_cursor_cb cursor_cb)
> >  {
> >  	igt_display_t *display = &data->display;
> >  	igt_pipe_crc_t *pipe_crc = data->pipe_crc;
> > @@ -151,7 +154,7 @@ static void do_single_test(data_t *data, int x, int y)
> >  	igt_display_commit(display);
> >  
> >  	/* Now render the same in software and collect crc */
> > -	draw_cursor(cr, x, y, data->curw, data->curh);
> > +	cursor_cb(cr, x, y, data->curw, data->curh);
> >  	igt_display_commit(display);
> >  
> >  	igt_wait_for_vblank(data->drm_fd, data->pipe);
> > @@ -162,6 +165,11 @@ static void do_single_test(data_t *data, int x, int y)
> >  	igt_paint_color(cr, 0, 0, data->screenw, data->screenh, 0.0, 0.0, 0.0);
> >  }
> >  
> > +static void do_single_test(data_t *data, int x, int y)
> > +{
> > +	do_single_test_with_cursor_cb(data, x, y, draw_cursor);
> > +}
> > +
> >  static void do_fail_test(data_t *data, int x, int y, int expect)
> >  {
> >  	igt_display_t *display = &data->display;
> > @@ -386,7 +394,8 @@ static void run_test(data_t *data, void (*testfunc)(data_t *), int cursor_w, int
> >  	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
> >  }
> >  
> > -static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
> > +static void create_cursor_fb_with_cb(data_t *data, int cur_w, int cur_h,
> > +				     draw_cursor_cb cursor_cb)
> >  {
> >  	cairo_t *cr;
> >  	uint32_t fb_id;
> > @@ -406,10 +415,15 @@ static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
> >  	igt_assert(fb_id);
> >  
> >  	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
> > -	draw_cursor(cr, 0, 0, cur_w, cur_h);
> > +	cursor_cb(cr, 0, 0, cur_w, cur_h);
> >  	igt_assert(cairo_status(cr) == 0);
> >  }
> >  
> > +static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
> > +{
> > +	create_cursor_fb_with_cb(data, cur_w, cur_h, draw_cursor);
> > +}
> > +
> >  static bool has_nonsquare_cursors(uint32_t devid)
> >  {
> >  	/*
> > @@ -506,6 +520,23 @@ static void test_rapid_movement(data_t *data)
> >  
> >  }
> >  
> > +static void draw_cursor_gradient(cairo_t *cr, int x, int y, int cw, int ch)
> > +{
> > +	cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE);
> > +	igt_paint_color_gradient(cr, x, y, cw, ch, 1.0, 1.0, 1.0);
> > +}
> > +
> > +static void test_cursor_colors(data_t *data)
> > +{
> > +	int cursor_w = data->curw;
> > +	int cursor_h = data->curh;
> > +
> > +	create_cursor_fb_with_cb(data, cursor_w, cursor_h,
> > +				 draw_cursor_gradient);
> > +	do_single_test_with_cursor_cb(data, 0, 0, draw_cursor_gradient);
> > +	igt_remove_fb(data->drm_fd, &data->fb);
> > +}
> > +
> >  static void run_test_generic(data_t *data)
> >  {
> >  	int cursor_size;
> > @@ -613,6 +644,9 @@ igt_main
> >  	igt_subtest_f("cursor-size-change")
> >  		run_test(&data, test_cursor_size, cursor_width, cursor_height);
> >  
> > +	igt_subtest_f("gradient-cursor")
> 
> Just realized this needs an igt_require(max_cursor_[wh] >= 256).

I spoke too soon; run_test() actually does that check.


> Ander
> 
> > +		run_test(&data, test_cursor_colors, 256, 256);
> > +
> >  	run_test_generic(&data);
> >  
> >  	igt_fixture {
diff mbox

Patch

diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 4851e18..0b232bf 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -87,6 +87,8 @@  static void draw_cursor(cairo_t *cr, int x, int y, int cw, int ch)
 	igt_paint_color_alpha(cr, x + wl, y + ht, wr, hb, 0.5, 0.5, 0.5, 1.0);
 }
 
+typedef void(*draw_cursor_cb)(cairo_t *cr, int x, int y, int cw, int ch);
+
 static void cursor_enable(data_t *data)
 {
 	igt_output_t *output = data->output;
@@ -107,7 +109,8 @@  static void cursor_disable(data_t *data)
 }
 
 
-static void do_single_test(data_t *data, int x, int y)
+static void do_single_test_with_cursor_cb(data_t *data, int x, int y,
+					  draw_cursor_cb cursor_cb)
 {
 	igt_display_t *display = &data->display;
 	igt_pipe_crc_t *pipe_crc = data->pipe_crc;
@@ -151,7 +154,7 @@  static void do_single_test(data_t *data, int x, int y)
 	igt_display_commit(display);
 
 	/* Now render the same in software and collect crc */
-	draw_cursor(cr, x, y, data->curw, data->curh);
+	cursor_cb(cr, x, y, data->curw, data->curh);
 	igt_display_commit(display);
 
 	igt_wait_for_vblank(data->drm_fd, data->pipe);
@@ -162,6 +165,11 @@  static void do_single_test(data_t *data, int x, int y)
 	igt_paint_color(cr, 0, 0, data->screenw, data->screenh, 0.0, 0.0, 0.0);
 }
 
+static void do_single_test(data_t *data, int x, int y)
+{
+	do_single_test_with_cursor_cb(data, x, y, draw_cursor);
+}
+
 static void do_fail_test(data_t *data, int x, int y, int expect)
 {
 	igt_display_t *display = &data->display;
@@ -386,7 +394,8 @@  static void run_test(data_t *data, void (*testfunc)(data_t *), int cursor_w, int
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
 
-static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
+static void create_cursor_fb_with_cb(data_t *data, int cur_w, int cur_h,
+				     draw_cursor_cb cursor_cb)
 {
 	cairo_t *cr;
 	uint32_t fb_id;
@@ -406,10 +415,15 @@  static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
 	igt_assert(fb_id);
 
 	cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
-	draw_cursor(cr, 0, 0, cur_w, cur_h);
+	cursor_cb(cr, 0, 0, cur_w, cur_h);
 	igt_assert(cairo_status(cr) == 0);
 }
 
+static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
+{
+	create_cursor_fb_with_cb(data, cur_w, cur_h, draw_cursor);
+}
+
 static bool has_nonsquare_cursors(uint32_t devid)
 {
 	/*
@@ -506,6 +520,23 @@  static void test_rapid_movement(data_t *data)
 
 }
 
+static void draw_cursor_gradient(cairo_t *cr, int x, int y, int cw, int ch)
+{
+	cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE);
+	igt_paint_color_gradient(cr, x, y, cw, ch, 1.0, 1.0, 1.0);
+}
+
+static void test_cursor_colors(data_t *data)
+{
+	int cursor_w = data->curw;
+	int cursor_h = data->curh;
+
+	create_cursor_fb_with_cb(data, cursor_w, cursor_h,
+				 draw_cursor_gradient);
+	do_single_test_with_cursor_cb(data, 0, 0, draw_cursor_gradient);
+	igt_remove_fb(data->drm_fd, &data->fb);
+}
+
 static void run_test_generic(data_t *data)
 {
 	int cursor_size;
@@ -613,6 +644,9 @@  igt_main
 	igt_subtest_f("cursor-size-change")
 		run_test(&data, test_cursor_size, cursor_width, cursor_height);
 
+	igt_subtest_f("gradient-cursor")
+		run_test(&data, test_cursor_colors, 256, 256);
+
 	run_test_generic(&data);
 
 	igt_fixture {