[Intel-gfx] [PATCH i-g-t v4 2/7] chamelium: Calculate CRC from framebuffer instead of hardcoding it
Paul Kocialkowski
paul.kocialkowski at linux.intel.com
Wed Jul 19 11:11:16 UTC 2017
On Mon, 2017-07-17 at 12:29 -0400, Lyude Paul wrote:
> 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?
So I have now tested that a failed assert (in the main thread) in the
middle of the threaded calculation will not cause any segfault. There is
no reason why it should, because none of the ressources used for the
calculation are liberated prior to the end of the process. Everything is
really cleaned up when the process dies.
On the other hand, it may happen, due to the call to
igt_get_cairo_surface that an igt_assert fails in the calculation
thread, which definitely causes a segfault.
I will rework the patches so that the call to the function is made prior
to starting the thread, thus avoiding any possibility of hitting a
failed igt_assert in the thread itself.
> 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);
> > }
--
Paul Kocialkowski <paul.kocialkowski at linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland
More information about the Intel-gfx
mailing list