On Wed, 2017-07-12 at 17:50 +0300, Paul Kocialkowski wrote: > This introduces CRC calculation for reference frames, instead of > using > hardcoded values for them. The rendering of reference frames may > differ > from machine to machine, especially due to font rendering, and the > frame itself may change with subsequent IGT changes. > > These differences would cause the CRC checks to fail on different > setups. This allows them to pass regardless of the setup. Just one question before I push this since I didn't think of testing this previously and don't have access to my chamelium at the moment. Have you made sure that this doesn't break things with igt if a test gets interrupted due to failure in the middle of an asynchronous CRC calculation? Other then that, everything here looks good. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxxxxxx> > --- > lib/igt_chamelium.c | 143 > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > lib/igt_chamelium.h | 5 ++ > tests/chamelium.c | 77 +++++++--------------------- > 3 files changed, 167 insertions(+), 58 deletions(-) > > diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c > index 93392af7..baa6399c 100644 > --- a/lib/igt_chamelium.c > +++ b/lib/igt_chamelium.c > @@ -94,6 +94,14 @@ struct chamelium_frame_dump { > struct chamelium_port *port; > }; > > +struct chamelium_fb_crc_async_data { > + int fd; > + struct igt_fb *fb; > + > + pthread_t thread_id; > + igt_crc_t *ret; > +}; > + > struct chamelium { > xmlrpc_env env; > xmlrpc_client *client; > @@ -998,6 +1006,141 @@ int chamelium_get_frame_limit(struct chamelium > *chamelium, > return ret; > } > > +static uint32_t chamelium_xrgb_hash16(const unsigned char *buffer, > int width, > + int height, int k, int m) > +{ > + unsigned char r, g, b; > + uint64_t sum = 0; > + uint64_t count = 0; > + uint64_t value; > + uint32_t hash; > + int index; > + int i; > + > + for (i=0; i < width * height; i++) { > + if ((i % m) != k) > + continue; > + > + index = i * 4; > + > + r = buffer[index + 2]; > + g = buffer[index + 1]; > + b = buffer[index + 0]; > + > + value = r | (g << 8) | (b << 16); > + sum += ++count * value; > + } > + > + hash = ((sum >> 0) ^ (sum >> 16) ^ (sum >> 32) ^ (sum >> > 48)) & 0xffff; > + > + return hash; > +} > + > +static void chamelium_do_calculate_fb_crc(int fd, struct igt_fb *fb, > + igt_crc_t *out) > +{ > + unsigned char *buffer; > + cairo_surface_t *fb_surface; > + int n = 4; > + int w, h; > + int i, j; > + > + /* Get the cairo surface for the framebuffer */ > + fb_surface = igt_get_cairo_surface(fd, fb); > + > + buffer = cairo_image_surface_get_data(fb_surface); > + w = fb->width; > + h = fb->height; > + > + for (i = 0; i < n; i++) { > + j = n - i - 1; > + out->crc[i] = chamelium_xrgb_hash16(buffer, w, h, j, > n); > + } > + > + out->n_words = n; > +} > + > +/** > + * chamelium_calculate_fb_crc: > + * @fd: The drm file descriptor > + * @fb: The framebuffer to calculate the CRC for > + * > + * Calculates the CRC for the provided framebuffer, using the > Chamelium's CRC > + * algorithm. This calculates the CRC in a synchronous fashion. > + * > + * Returns: The calculated CRC > + */ > +igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb) > +{ > + igt_crc_t *ret = calloc(1, sizeof(igt_crc_t)); > + > + chamelium_do_calculate_fb_crc(fd, fb, ret); > + > + return ret; > +} > + > +static void *chamelium_calculate_fb_crc_async_work(void *data) > +{ > + struct chamelium_fb_crc_async_data *fb_crc; > + > + fb_crc = (struct chamelium_fb_crc_async_data *) data; > + > + chamelium_do_calculate_fb_crc(fb_crc->fd, fb_crc->fb, > fb_crc->ret); > + > + return NULL; > +} > + > +/** > + * chamelium_calculate_fb_crc_launch: > + * @fd: The drm file descriptor > + * @fb: The framebuffer to calculate the CRC for > + * > + * Launches the CRC calculation for the provided framebuffer, using > the > + * Chamelium's CRC algorithm. This calculates the CRC in an > asynchronous > + * fashion. > + * > + * The returned structure should be passed to a subsequent call to > + * chamelium_calculate_fb_crc_result. It should not be freed. > + * > + * Returns: An intermediate structure for the CRC calculation work. > + */ > +struct chamelium_fb_crc_async_data > *chamelium_calculate_fb_crc_async_start(int fd, > + > struct igt_fb *fb) > +{ > + struct chamelium_fb_crc_async_data *fb_crc; > + > + fb_crc = calloc(1, sizeof(struct > chamelium_fb_crc_async_data)); > + fb_crc->ret = calloc(1, sizeof(igt_crc_t)); > + fb_crc->fd = fd; > + fb_crc->fb = fb; > + > + pthread_create(&fb_crc->thread_id, NULL, > + chamelium_calculate_fb_crc_async_work, > fb_crc); > + > + return fb_crc; > +} > + > +/** > + * chamelium_calculate_fb_crc_result: > + * @fb_crc: An intermediate structure with thread-related > information > + * > + * Blocks until the asynchronous CRC calculation is finished, and > then returns > + * its result. > + * > + * Returns: The calculated CRC > + */ > +igt_crc_t *chamelium_calculate_fb_crc_async_finish(struct > chamelium_fb_crc_async_data *fb_crc) > +{ > + igt_crc_t *ret; > + > + pthread_join(fb_crc->thread_id, NULL); > + > + ret = fb_crc->ret; > + free(fb_crc); > + > + return ret; > +} > + > static unsigned int chamelium_get_port_type(struct chamelium > *chamelium, > struct chamelium_port > *port) > { > diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h > index 81322ad2..2bfbfdc7 100644 > --- a/lib/igt_chamelium.h > +++ b/lib/igt_chamelium.h > @@ -36,6 +36,7 @@ > struct chamelium; > struct chamelium_port; > struct chamelium_frame_dump; > +struct chamelium_fb_crc_async_data; > > struct chamelium *chamelium_init(int drm_fd); > void chamelium_deinit(struct chamelium *chamelium); > @@ -92,6 +93,10 @@ struct chamelium_frame_dump > *chamelium_port_dump_pixels(struct chamelium *chamel > struct > chamelium_port *port, > int x, int > y, > int w, int > h); > +igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb); > +struct chamelium_fb_crc_async_data > *chamelium_calculate_fb_crc_async_start(int fd, > + > struct igt_fb *fb); > +igt_crc_t *chamelium_calculate_fb_crc_async_finish(struct > chamelium_fb_crc_async_data *fb_crc); > int chamelium_get_captured_frame_count(struct chamelium *chamelium); > int chamelium_get_frame_limit(struct chamelium *chamelium, > struct chamelium_port *port, > diff --git a/tests/chamelium.c b/tests/chamelium.c > index e26f0557..da98de47 100644 > --- a/tests/chamelium.c > +++ b/tests/chamelium.c > @@ -49,43 +49,6 @@ typedef struct { > #define HPD_TOGGLE_COUNT_VGA 5 > #define HPD_TOGGLE_COUNT_DP_HDMI 15 > > -/* Pre-calculated CRCs for the pattern fb, for all the modes in the > default > - * chamelium edid > - */ > -struct crc_entry { > - int width; > - int height; > - igt_crc_t crc; > -}; > - > -#define CRC_ENTRY(w_, h_, ...) \ > - { w_, h_, { .n_words = 4, .crc = { __VA_ARGS__ } } } > - > -static const struct crc_entry pattern_fb_crcs[] = { > - CRC_ENTRY(1920, 1080, 0xf859, 0xa751, 0x8c81, 0x45a1), > - CRC_ENTRY(1280, 720, 0xcec2, 0x4246, 0x6cfd, 0xeb43), > - CRC_ENTRY(1024, 768, 0x85e5, 0xf0cd, 0xafe3, 0x7f18), > - CRC_ENTRY( 800, 600, 0x6b39, 0x32b6, 0x831a, 0xb03e), > - CRC_ENTRY( 640, 480, 0xa121, 0x2473, 0xb150, 0x8c47), > -}; > -#undef CRC_ENTRY > - > -static const igt_crc_t * > -get_precalculated_crc(struct chamelium_port *port, int w, int h) > -{ > - int i; > - const struct crc_entry *entry; > - > - for (i = 0; i < ARRAY_SIZE(pattern_fb_crcs); i++) { > - entry = &pattern_fb_crcs[i]; > - > - if (entry->width == w && entry->height == h) > - return &entry->crc; > - } > - > - return NULL; > -} > - > static void > require_connector_present(data_t *data, unsigned int type) > { > @@ -422,7 +385,8 @@ test_display_crc_single(data_t *data, struct > chamelium_port *port) > igt_output_t *output; > igt_plane_t *primary; > igt_crc_t *crc; > - const igt_crc_t *expected_crc; > + igt_crc_t *expected_crc; > + struct chamelium_fb_crc_async_data *fb_crc; > struct igt_fb fb; > drmModeModeInfo *mode; > drmModeConnector *connector; > @@ -445,24 +409,21 @@ test_display_crc_single(data_t *data, struct > chamelium_port *port) > 0, 0, 0, &fb); > igt_assert(fb_id > 0); > > + fb_crc = > chamelium_calculate_fb_crc_async_start(data->drm_fd, > + &fb) > ; > enable_output(data, port, output, mode, &fb); > > - expected_crc = get_precalculated_crc(port, > - mode->hdisplay, > - mode- > >vdisplay); > - if (!expected_crc) { > - igt_warn("No precalculated CRC found for > %dx%d, skipping CRC check\n", > - mode->hdisplay, mode->vdisplay); > - goto next; > - } > - > igt_debug("Testing single CRC fetch\n"); > + > crc = chamelium_get_crc_for_area(data->chamelium, > port, > 0, 0, 0, 0); > + > + expected_crc = > chamelium_calculate_fb_crc_async_finish(fb_crc); > + > igt_assert_crc_equal(crc, expected_crc); > + free(expected_crc); > free(crc); > > -next: > disable_output(data, port, output); > igt_remove_fb(data->drm_fd, &fb); > } > @@ -478,7 +439,8 @@ test_display_crc_multiple(data_t *data, struct > chamelium_port *port) > igt_output_t *output; > igt_plane_t *primary; > igt_crc_t *crc; > - const igt_crc_t *expected_crc; > + igt_crc_t *expected_crc; > + struct chamelium_fb_crc_async_data *fb_crc; > struct igt_fb fb; > drmModeModeInfo *mode; > drmModeConnector *connector; > @@ -501,15 +463,10 @@ test_display_crc_multiple(data_t *data, struct > chamelium_port *port) > 0, 0, 0, &fb); > igt_assert(fb_id > 0); > > - enable_output(data, port, output, mode, &fb); > + fb_crc = > chamelium_calculate_fb_crc_async_start(data->drm_fd, > + &fb) > ; > > - expected_crc = get_precalculated_crc(port, mode- > >hdisplay, > - mode- > >vdisplay); > - if (!expected_crc) { > - igt_warn("No precalculated CRC found for > %dx%d, skipping CRC check\n", > - mode->hdisplay, mode->vdisplay); > - goto next; > - } > + enable_output(data, port, output, mode, &fb); > > /* We want to keep the display running for a little > bit, since > * there's always the potential the driver isn't > able to keep > @@ -520,11 +477,15 @@ test_display_crc_multiple(data_t *data, struct > chamelium_port *port) > &captured_frame_c > ount); > > igt_debug("Captured %d frames\n", > captured_frame_count); > + > + expected_crc = > chamelium_calculate_fb_crc_async_finish(fb_crc); > + > for (j = 0; j < captured_frame_count; j++) > igt_assert_crc_equal(&crc[j], expected_crc); > + > + free(expected_crc); > free(crc); > > -next: > disable_output(data, port, output); > igt_remove_fb(data->drm_fd, &fb); > } _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx