On Fri, Dec 18, 2015 at 07:25:48PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Add support for reading the CRC in non-blocking mode. Useful for tests
> that want to start the CRC capture, then do a bunch of operations, then
> collect however many CRCs that got generated. The current
> igt_pipe_crc_new() + igt_pipe_crc_get_crcs() method would block until
> it gets the requested number of CRCs, whreas in non-blocking mode we
> can just read as many as got generated thus far.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> lib/igt_debugfs.c | 136 +++++++++++++++++++++++++++++++++++-------------------
> lib/igt_debugfs.h | 4 +-
> 2 files changed, 91 insertions(+), 49 deletions(-)
>
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index a3d015267e15..1d625ad4ba50 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -298,6 +298,7 @@ struct _igt_pipe_crc {
> int crc_fd;
> int line_len;
> int buffer_len;
> + int flags;
>
> enum pipe pipe;
> enum intel_pipe_crc_source source;
> @@ -389,19 +390,8 @@ void igt_require_pipe_crc(void)
> fclose(ctl);
> }
>
> -/**
> - * igt_pipe_crc_new:
> - * @pipe: display pipe to use as source
> - * @source: CRC tap point to use as source
> - *
> - * This sets up a new pipe CRC capture object for the given @pipe and @source.
> - *
> - * Returns: A pipe CRC object if the given @pipe and @source. The library
> - * assumes that the source is always available since recent kernels support at
> - * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
> - */
> -igt_pipe_crc_t *
> -igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source)
> +static igt_pipe_crc_t *
> +pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source, int flags)
> {
> igt_pipe_crc_t *pipe_crc;
> char buf[128];
> @@ -414,18 +404,55 @@ igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source)
> igt_assert(pipe_crc->ctl_fd != -1);
>
> sprintf(buf, "i915_pipe_%s_crc", kmstest_pipe_name(pipe));
> - pipe_crc->crc_fd = igt_debugfs_open(buf, O_RDONLY);
> + pipe_crc->crc_fd = igt_debugfs_open(buf, flags);
> igt_assert(pipe_crc->crc_fd != -1);
>
> pipe_crc->line_len = PIPE_CRC_LINE_LEN;
> pipe_crc->buffer_len = PIPE_CRC_BUFFER_LEN;
> pipe_crc->pipe = pipe;
> pipe_crc->source = source;
> + pipe_crc->flags = flags;
>
> return pipe_crc;
> }
>
> /**
> + * igt_pipe_crc_new:
> + * @pipe: display pipe to use as source
> + * @source: CRC tap point to use as source
> + *
> + * This sets up a new pipe CRC capture object for the given @pipe and @source
> + * in blocking mode.
> + *
> + * Returns: A pipe CRC object if the given @pipe and @source. The library
> + * assumes that the source is always available since recent kernels support at
> + * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
> + */
> +igt_pipe_crc_t *
> +igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source)
> +{
> + return pipe_crc_new(pipe, source, O_RDONLY);
> +}
> +
> +/**
> + * igt_pipe_crc_new_nonblock:
> + * @pipe: display pipe to use as source
> + * @source: CRC tap point to use as source
> + *
> + * This sets up a new pipe CRC capture object for the given @pipe and @source
> + * in nonblocking mode.
> + *
> + * Returns: A pipe CRC object if the given @pipe and @source. The library
> + * assumes that the source is always available since recent kernels support at
> + * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
> + */
> +igt_pipe_crc_t *
> +igt_pipe_crc_new_nonblock(enum pipe pipe, enum intel_pipe_crc_source source)
> +{
> + return pipe_crc_new(pipe, source, O_RDONLY | O_NONBLOCK);
> +}
> +
> +/**
> * igt_pipe_crc_free:
> * @pipe_crc: pipe CRC object
> *
> @@ -441,6 +468,39 @@ void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc)
> free(pipe_crc);
> }
>
> +static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
> +{
> + int n;
> +
> + crc->n_words = 5;
> + n = sscanf(line, "%8u %8x %8x %8x %8x %8x", &crc->frame, &crc->crc[0],
> + &crc->crc[1], &crc->crc[2], &crc->crc[3], &crc->crc[4]);
> + return n == 6;
> +}
> +
> +static int read_one_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
> +{
> + ssize_t bytes_read;
> + char buf[pipe_crc->buffer_len];
> +
> + igt_set_timeout(5, "CRC reading");
> + bytes_read = read(pipe_crc->crc_fd, &buf, pipe_crc->line_len);
> + igt_reset_timeout();
> +
> + if (bytes_read < 0 && errno == EAGAIN) {
> + igt_assert(pipe_crc->flags & O_NONBLOCK);
> + bytes_read = 0;
> + } else {
> + igt_assert_eq(bytes_read, pipe_crc->line_len);
> + }
> + buf[bytes_read] = '\0';
> +
> + if (bytes_read && !pipe_crc_init_from_string(out, buf))
> + return -EINVAL;
> +
> + return bytes_read;
> +}
> +
> /**
> * igt_pipe_crc_start:
> * @pipe_crc: pipe CRC object
> @@ -449,7 +509,7 @@ void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc)
> */
> void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
> {
> - igt_crc_t *crcs = NULL;
> + igt_crc_t crc;
>
> igt_assert(igt_pipe_crc_do_start(pipe_crc));
>
> @@ -460,8 +520,10 @@ void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
> * On CHV sometimes the second CRC is bonkers as well, so don't trust
> * that one either.
> */
> - igt_pipe_crc_get_crcs(pipe_crc, 2, &crcs);
> - free(crcs);
> + while (read_one_crc(pipe_crc, &crc) == 0)
> + usleep(1000);
> + while (read_one_crc(pipe_crc, &crc) == 0)
> + usleep(1000);
> }
>
> /**
> @@ -478,34 +540,6 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
> igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)), strlen(buf));
> }
>
> -static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
> -{
> - int n;
> -
> - crc->n_words = 5;
> - n = sscanf(line, "%8u %8x %8x %8x %8x %8x", &crc->frame, &crc->crc[0],
> - &crc->crc[1], &crc->crc[2], &crc->crc[3], &crc->crc[4]);
> - return n == 6;
> -}
> -
> -static bool read_one_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
> -{
> - ssize_t bytes_read;
> - char buf[pipe_crc->buffer_len];
> -
> - igt_set_timeout(5, "CRC reading");
> - bytes_read = read(pipe_crc->crc_fd, &buf, pipe_crc->line_len);
> - igt_reset_timeout();
> -
> - igt_assert_eq(bytes_read, pipe_crc->line_len);
> - buf[bytes_read] = '\0';
> -
> - if (!pipe_crc_init_from_string(out, buf))
> - return false;
> -
> - return true;
> -}
> -
> /**
> * igt_pipe_crc_get_crcs:
> * @pipe_crc: pipe CRC object
> @@ -519,7 +553,7 @@ static bool read_one_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
> * Callers must start and stop the capturing themselves by calling
> * igt_pipe_crc_start() and igt_pipe_crc_stop().
Please update the gtkdoc with the return value semantics.
> */
> -void
> +int
> igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
> igt_crc_t **out_crcs)
> {
> @@ -530,14 +564,19 @@ igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
>
> do {
> igt_crc_t *crc = &crcs[n];
> + int ret;
>
> - if (!read_one_crc(pipe_crc, crc))
> + ret = read_one_crc(pipe_crc, crc);
> + if (ret < 0)
> continue;
> + if (ret == 0)
> + break;
>
> n++;
> } while (n < n_crcs);
>
> *out_crcs = crcs;
> + return n;
> }
>
> static void crc_sanity_checks(igt_crc_t *crc)
> @@ -576,7 +615,8 @@ void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc)
> igt_debug_wait_for_keypress("crc");
>
> igt_pipe_crc_start(pipe_crc);
> - read_one_crc(pipe_crc, out_crc);
> + while (read_one_crc(pipe_crc, out_crc) == 0)
> + usleep(1000);
> igt_pipe_crc_stop(pipe_crc);
>
> crc_sanity_checks(out_crc);
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index 24018eb355e4..6c6e2858d8f6 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -115,10 +115,12 @@ char *igt_crc_to_string(igt_crc_t *crc);
> void igt_require_pipe_crc(void);
> igt_pipe_crc_t *
> igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source);
> +igt_pipe_crc_t *
> +igt_pipe_crc_new_nonblock(enum pipe pipe, enum intel_pipe_crc_source source);
> void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc);
> void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc);
> void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
> -void igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
> +int igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
> igt_crc_t **out_crcs);
__must_check here to make sure callers check that they have enough crcs or
do something else sensible. I think we should also update
kms_pipe_crc_basic.
-Daniel
> void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc);
>
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
