| Message ID | 1396436790-14313-6-git-send-email-antti.koskipaa@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 02, 2014 at 02:06:28PM +0300, Antti Koskipaa wrote: <snip> > @@ -184,9 +192,6 @@ static void test_crc_offscreen(test_data_t *test_data) > do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top , bottom ); > do_test(test_data, left , right , top - (cursor_h+512), bottom + (cursor_h+512)); > do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top - (cursor_h+512), bottom + (cursor_h+512)); > - > - /* go nuts */ > - do_test(test_data, INT_MIN, INT_MAX, INT_MIN, INT_MAX); Why are you dropping the go nuts test? We had an actual bug in the driver that this managed to hit. We should keep this test to avoid regressions.
On 04/02/2014 02:21 PM, Ville Syrjälä wrote: > On Wed, Apr 02, 2014 at 02:06:28PM +0300, Antti Koskipaa wrote: > <snip> >> @@ -184,9 +192,6 @@ static void test_crc_offscreen(test_data_t *test_data) >> do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top , bottom ); >> do_test(test_data, left , right , top - (cursor_h+512), bottom + (cursor_h+512)); >> do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top - (cursor_h+512), bottom + (cursor_h+512)); >> - >> - /* go nuts */ >> - do_test(test_data, INT_MIN, INT_MAX, INT_MIN, INT_MAX); > > Why are you dropping the go nuts test? We had an actual bug in the > driver that this managed to hit. We should keep this test to avoid > regressions. Link please. Cairo went nuts with that one, but if there is a case for the test, I'll work around Cairo to keep it.
On Wed, Apr 02, 2014 at 02:27:40PM +0300, Antti Koskipää wrote: > On 04/02/2014 02:21 PM, Ville Syrjälä wrote: > > On Wed, Apr 02, 2014 at 02:06:28PM +0300, Antti Koskipaa wrote: > > <snip> > >> @@ -184,9 +192,6 @@ static void test_crc_offscreen(test_data_t *test_data) > >> do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top , bottom ); > >> do_test(test_data, left , right , top - (cursor_h+512), bottom + (cursor_h+512)); > >> do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top - (cursor_h+512), bottom + (cursor_h+512)); > >> - > >> - /* go nuts */ > >> - do_test(test_data, INT_MIN, INT_MAX, INT_MIN, INT_MAX); > > > > Why are you dropping the go nuts test? We had an actual bug in the > > driver that this managed to hit. We should keep this test to avoid > > regressions. > > Link please. Cairo went nuts with that one, but if there is a case for > the test, I'll work around Cairo to keep it. commit 92e76c8c7e436a07af3e0b594480ff8689add078 Author: Ville Syrjälä <ville.syrjala@linux.intel.com> Date: Mon Oct 21 19:01:58 2013 +0300 drm/i915: Clamp cursor coordinates to int16_t range is the commit that fixed the bug.
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c index 94baa94..021d58a 100644 --- a/tests/kms_cursor_crc.c +++ b/tests/kms_cursor_crc.c @@ -52,8 +52,8 @@ typedef struct { igt_output_t *output; enum pipe pipe; igt_crc_t ref_crc; - bool crc_must_match; int left, right, top, bottom; + int screenw, screenh; int curw, curh; /* cursor size */ } test_data_t; @@ -105,23 +105,31 @@ static void do_single_test(test_data_t *test_data, int x, int y) data_t *data = test_data->data; igt_display_t *display = &data->display; igt_pipe_crc_t *pipe_crc = data->pipe_crc[test_data->pipe]; - igt_crc_t crc; + igt_crc_t crc, ref_crc; igt_plane_t *cursor; + cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb); printf("."); fflush(stdout); + /* Hardware test */ cursor_enable(test_data); cursor = igt_output_get_plane(test_data->output, IGT_PLANE_CURSOR); igt_plane_set_position(cursor, x, y); igt_display_commit(display); igt_wait_for_vblank(data->drm_fd, test_data->pipe); + igt_pipe_crc_collect_crc(pipe_crc, &crc); cursor_disable(test_data); - igt_pipe_crc_collect_crc(pipe_crc, &crc); - if (test_data->crc_must_match) - igt_assert(igt_crc_equal(&crc, &test_data->ref_crc)); - else - igt_assert(!igt_crc_equal(&crc, &test_data->ref_crc)); + /* Now render the same in software and collect crc */ + draw_cursor(cr, x, y, test_data->curw); + igt_display_commit(display); + igt_wait_for_vblank(data->drm_fd, test_data->pipe); + igt_pipe_crc_collect_crc(pipe_crc, &ref_crc); + /* Clear screen afterwards */ + igt_paint_color(cr, 0, 0, test_data->screenw, test_data->screenh, + 0.0, 0.0, 0.0); + + igt_assert(igt_crc_equal(&crc, &ref_crc)); } static void do_test(test_data_t *test_data, @@ -184,9 +192,6 @@ static void test_crc_offscreen(test_data_t *test_data) do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top , bottom ); do_test(test_data, left , right , top - (cursor_h+512), bottom + (cursor_h+512)); do_test(test_data, left - (cursor_w+512), right + (cursor_w+512), top - (cursor_h+512), bottom + (cursor_h+512)); - - /* go nuts */ - do_test(test_data, INT_MIN, INT_MAX, INT_MIN, INT_MAX); } static bool prepare_crtc(test_data_t *test_data, igt_output_t *output, @@ -232,6 +237,8 @@ static bool prepare_crtc(test_data_t *test_data, igt_output_t *output, test_data->right = mode->hdisplay - cursor_w; test_data->top = 0; test_data->bottom = mode->vdisplay - cursor_h; + test_data->screenw = mode->hdisplay; + test_data->screenh = mode->vdisplay; test_data->curw = cursor_w; test_data->curh = cursor_h;
This patch first render the cursor with hardware rendering and then with software, acquiring the CRC in both cases so they can be properly compared. Say goodbye to crc_must_match variable. Signed-off-by: Antti Koskipaa <antti.koskipaa@linux.intel.com> --- tests/kms_cursor_crc.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-)