[Intel-gfx] [PATCH i-g-t v4 2/7] chamelium: Calculate CRC from framebuffer instead of hardcoding it

Lyude Paul lyude at redhat.com
Mon Jul 17 16:29:12 UTC 2017


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 at linux.intel.com>
> ---
>  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);
>  	}


More information about the Intel-gfx mailing list