Hi tomeu,
This patch does not seem to apply cleanly on upstream/master.
Rob.
> The kernel has now a new debugfs ABI that can also allow capturing
> frame
> CRCs for drivers other than i915.
>
> Add alternative codepaths so the new ABI is used if the kernel is
> recent
> enough, and fall back to the legacy ABI if not.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx>
>
> ---
>
> Have just rebased this, and made sure we fallback to the old ABI when
> accessing the debugfs files directly from kms_pipe_crc_basic.
>
> Thanks,
>
> Tomeu
> ---
> lib/igt_debugfs.c | 190 +++++++++++++++++++++++++++++++++
> ------------
> lib/igt_debugfs.h | 4 +-
> tests/kms_pipe_crc_basic.c | 30 ++++++-
> 3 files changed, 171 insertions(+), 53 deletions(-)
>
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 9142e3f78868..3d92c6a10a41 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -25,6 +25,7 @@
> #include <inttypes.h>
> #include <sys/stat.h>
> #include <sys/mount.h>
> +#include <dirent.h>
> #include <errno.h>
> #include <stdio.h>
> #include <stdlib.h>
> @@ -291,25 +292,23 @@ char *igt_crc_to_string(igt_crc_t *crc)
> {
> char buf[128];
>
> - igt_assert_eq(crc->n_words, 5);
> -
> sprintf(buf, "%08x %08x %08x %08x %08x", crc->crc[0],
> crc->crc[1], crc->crc[2], crc->crc[3], crc->crc[4]);
>
> return strdup(buf);
> }
>
> +#define MAX_CRC_ENTRIES 10
> +#define MAX_LINE_LEN (10 + 11 * MAX_CRC_ENTRIES + 1)
> +
> /* (6 fields, 8 chars each, space separated (5) + '\n') */
> -#define PIPE_CRC_LINE_LEN (6 * 8 + 5 + 1)
> -/* account for \'0' */
> -#define PIPE_CRC_BUFFER_LEN (PIPE_CRC_LINE_LEN + 1)
> +#define LEGACY_LINE_LEN (6 * 8 + 5 + 1)
>
> struct _igt_pipe_crc {
> int ctl_fd;
> int crc_fd;
> - int line_len;
> - int buffer_len;
> int flags;
> + bool is_legacy;
>
> enum pipe pipe;
> enum intel_pipe_crc_source source;
> @@ -340,13 +339,26 @@ static bool
> igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
> /* Stop first just to make sure we don't have lingering
> state left. */
> igt_pipe_crc_stop(pipe_crc);
>
> - sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc-
> >pipe),
> - pipe_crc_source_name(pipe_crc->source));
> + if (pipe_crc->is_legacy)
> + sprintf(buf, "pipe %s %s",
> kmstest_pipe_name(pipe_crc->pipe),
> + pipe_crc_source_name(pipe_crc->source));
> + else
> + sprintf(buf, "%s", pipe_crc_source_name(pipe_crc-
> >source));
> +
> errno = 0;
> igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)),
> strlen(buf));
> if (errno != 0)
> return false;
>
> + if (!pipe_crc->is_legacy) {
> + sprintf(buf, "crtc-%d/crc/data", pipe_crc->pipe);
> + errno = 0;
> + pipe_crc->crc_fd = igt_debugfs_open(buf, pipe_crc-
> >flags);
> + if (pipe_crc->crc_fd == -1 && errno == EINVAL)
> + return false;
> + igt_assert_eq(errno, 0);
> + }
> +
> return true;
> }
>
> @@ -360,15 +372,45 @@ static void igt_pipe_crc_pipe_off(int fd, enum
> pipe pipe)
>
> static void igt_pipe_crc_reset(void)
> {
> + igt_debugfs_t *debugfs = __igt_debugfs_singleton();
> int fd;
> + struct dirent *dirent;
> + char buf[128];
> + const char *cmd = "none";
> + bool done = false;
> + DIR *dir;
> +
> + dir = opendir(debugfs->dri_path);
> + if (dir) {
> + while ((dirent = readdir(dir))) {
> + if (strcmp(dirent->d_name, "crtc-") != 0)
> + continue;
> +
> + sprintf(buf, "%s/%s/crc/control", debugfs-
> >dri_path,
> + dirent->d_name);
> + fd = open(buf, O_WRONLY);
> + if (fd == -1)
> + continue;
> +
> + igt_assert_eq(write(fd, cmd, strlen(cmd)),
> strlen(cmd));
> + done = true;
> +
> + close(fd);
> + }
> + closedir(dir);
> + }
>
> - fd = igt_debugfs_open("i915_display_crc_ctl", O_WRONLY);
> + if (done)
> + return;
>
> - igt_pipe_crc_pipe_off(fd, PIPE_A);
> - igt_pipe_crc_pipe_off(fd, PIPE_B);
> - igt_pipe_crc_pipe_off(fd, PIPE_C);
> + fd = igt_debugfs_open("i915_display_crc_ctl", O_WRONLY);
> + if (fd != -1) {
> + igt_pipe_crc_pipe_off(fd, PIPE_A);
> + igt_pipe_crc_pipe_off(fd, PIPE_B);
> + igt_pipe_crc_pipe_off(fd, PIPE_C);
>
> - close(fd);
> + close(fd);
> + }
> }
>
> static void pipe_crc_exit_handler(int sig)
> @@ -390,13 +432,16 @@ void igt_require_pipe_crc(void)
> size_t written;
> int ret;
>
> - ctl = igt_debugfs_fopen("i915_display_crc_ctl", "r+");
> - igt_require_f(ctl,
> - "No display_crc_ctl found, kernel too old\n");
> - written = fwrite(cmd, 1, strlen(cmd), ctl);
> - ret = fflush(ctl);
> - igt_require_f((written == strlen(cmd) && ret == 0) || errno
> != ENODEV,
> - "CRCs not supported on this platform\n");
> + ctl = igt_debugfs_fopen("crtc-0/crc/control", "r+");
> + if (!ctl) {
> + ctl = igt_debugfs_fopen("i915_display_crc_ctl",
> "r+");
> + igt_require_f(ctl,
> + "No display_crc_ctl found, kernel too
> old\n");
> + written = fwrite(cmd, 1, strlen(cmd), ctl);
> + ret = fflush(ctl);
> + igt_require_f((written == strlen(cmd) && ret == 0)
> || errno != ENODEV,
> + "CRCs not supported on this
> platform\n");
> + }
>
> fclose(ctl);
> }
> @@ -411,15 +456,25 @@ pipe_crc_new(enum pipe pipe, enum
> intel_pipe_crc_source source, int flags)
>
> pipe_crc = calloc(1, sizeof(struct _igt_pipe_crc));
>
> - pipe_crc->ctl_fd = igt_debugfs_open("i915_display_crc_ctl",
> O_WRONLY);
> - igt_assert(pipe_crc->ctl_fd != -1);
> + sprintf(buf, "crtc-%d/crc/control", pipe);
> + pipe_crc->ctl_fd = igt_debugfs_open(buf, O_WRONLY);
> + if (pipe_crc->ctl_fd == -1) {
> + pipe_crc->ctl_fd =
> igt_debugfs_open("i915_display_crc_ctl",
> + O_WRONLY);
> + igt_assert(pipe_crc->ctl_fd != -1);
> + pipe_crc->is_legacy = true;
> + }
>
> - sprintf(buf, "i915_pipe_%s_crc", kmstest_pipe_name(pipe));
> - pipe_crc->crc_fd = igt_debugfs_open(buf, flags);
> - igt_assert(pipe_crc->crc_fd != -1);
> + if (pipe_crc->is_legacy) {
> + sprintf(buf, "i915_pipe_%s_crc",
> kmstest_pipe_name(pipe));
> + pipe_crc->crc_fd = igt_debugfs_open(buf, flags);
> + igt_assert(pipe_crc->crc_fd != -1);
> + igt_debug("Using legacy frame CRC ABI\n");
> + } else {
> + pipe_crc->crc_fd = -1;
> + igt_debug("Using generic frame CRC ABI\n");
> + }
>
> - 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;
> @@ -479,34 +534,59 @@ 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)
> +static bool pipe_crc_init_from_string(igt_pipe_crc_t *pipe_crc,
> igt_crc_t *crc,
> + const char *line)
> {
> - int n;
> + int n, i;
> + const char *buf;
>
> - 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;
> + if (pipe_crc->is_legacy) {
> + crc->has_valid_frame = true;
> + 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;
> + }
> +
> + if (strncmp(line, "XXXXXXXXXX", 10) == 0)
> + crc->has_valid_frame = false;
> + else {
> + crc->has_valid_frame = true;
> + crc->frame = strtoul(line, NULL, 16);
> + }
> +
> + buf = line + 10;
> + for (i = 0; *buf != '\n'; i++, buf += 11)
> + crc->crc[i] = strtoul(buf, NULL, 16);
> +
> + crc->n_words = i;
> +
> + return true;
> }
>
> static int read_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
> {
> ssize_t bytes_read;
> - char buf[pipe_crc->buffer_len];
> + char buf[MAX_LINE_LEN + 1];
> + size_t read_len;
> +
> + if (pipe_crc->is_legacy)
> + read_len = LEGACY_LINE_LEN;
> + else
> + read_len = MAX_LINE_LEN;
>
> igt_set_timeout(5, "CRC reading");
> - bytes_read = read(pipe_crc->crc_fd, &buf, pipe_crc-
> >line_len);
> + bytes_read = read(pipe_crc->crc_fd, &buf, read_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))
> + if (bytes_read && !pipe_crc_init_from_string(pipe_crc, out,
> buf))
> return -EINVAL;
>
> return bytes_read;
> @@ -530,15 +610,18 @@ void igt_pipe_crc_start(igt_pipe_crc_t
> *pipe_crc)
>
> igt_assert(igt_pipe_crc_do_start(pipe_crc));
>
> - /*
> - * For some no yet identified reason, the first CRC is
> bonkers. So
> - * let's just wait for the next vblank and read out the
> buggy result.
> - *
> - * On CHV sometimes the second CRC is bonkers as well, so
> don't trust
> - * that one either.
> - */
> - read_one_crc(pipe_crc, &crc);
> - read_one_crc(pipe_crc, &crc);
> + if (pipe_crc->is_legacy) {
> + /*
> + * For some no yet identified reason, the first CRC
> is
> + * bonkers. So let's just wait for the next vblank
> and read
> + * out the buggy result.
> + *
> + * On CHV sometimes the second CRC is bonkers as
> well, so
> + * don't trust that one either.
> + */
> + read_one_crc(pipe_crc, &crc);
> + read_one_crc(pipe_crc, &crc);
> + }
> }
>
> /**
> @@ -551,8 +634,15 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
> {
> char buf[32];
>
> - sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc-
> >pipe));
> - igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)),
> strlen(buf));
> + if (pipe_crc->is_legacy) {
> + sprintf(buf, "pipe %s none",
> + kmstest_pipe_name(pipe_crc->pipe));
> + igt_assert_eq(write(pipe_crc->ctl_fd, buf,
> strlen(buf)),
> + strlen(buf));
> + } else {
> + close(pipe_crc->crc_fd);
> + pipe_crc->crc_fd = -1;
> + }
> }
>
> /**
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index fb189322633f..4c6572cd244c 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -62,6 +62,7 @@ bool igt_debugfs_search(const char *filename, const
> char *substring);
> */
> typedef struct _igt_pipe_crc igt_pipe_crc_t;
>
> +#define DRM_MAX_CRC_NR 10
> /**
> * igt_crc_t:
> * @frame: frame number of the capture CRC
> @@ -73,8 +74,9 @@ typedef struct _igt_pipe_crc igt_pipe_crc_t;
> */
> typedef struct {
> uint32_t frame;
> + bool has_valid_frame;
> int n_words;
> - uint32_t crc[5];
> + uint32_t crc[DRM_MAX_CRC_NR];
> } igt_crc_t;
>
> /**
> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
> index 04d5a1345760..b106f9bd05ee 100644
> --- a/tests/kms_pipe_crc_basic.c
> +++ b/tests/kms_pipe_crc_basic.c
> @@ -49,6 +49,8 @@ static void test_bad_command(data_t *data, const
> char *cmd)
> size_t written;
>
> ctl = igt_debugfs_fopen("i915_display_crc_ctl", "r+");
> + igt_require(ctl);
> +
> written = fwrite(cmd, 1, strlen(cmd), ctl);
> fflush(ctl);
> igt_assert_eq(written, strlen(cmd));
> @@ -58,6 +60,30 @@ static void test_bad_command(data_t *data, const
> char *cmd)
> fclose(ctl);
> }
>
> +static void test_bad_source(data_t *data)
> +{
> + FILE *f;
> + size_t written;
> + const char *source = "foo";
> +
> + f = igt_debugfs_fopen("crtc-0/crc/control", "w");
> + if (!f) {
> + test_bad_command(data, "pipe A foo");
> + return;
> + }
> +
> + written = fwrite(source, 1, strlen(source), f);
> + fflush(f);
> + igt_assert_eq(written, strlen(source));
> + igt_assert(!ferror(f));
> + igt_assert(!errno);
> + fclose(f);
> +
> + f = igt_debugfs_fopen("crtc-0/crc/data", "w");
> + igt_assert(!f);
> + igt_assert_eq(errno, EINVAL);
> +}
> +
> #define N_CRCS 3
>
> #define TEST_SEQUENCE (1<<0)
> @@ -185,7 +211,7 @@ igt_main
> igt_skip_on_simulation();
>
> igt_fixture {
> - data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> + data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>
> igt_enable_connectors();
>
> @@ -200,7 +226,7 @@ igt_main
> test_bad_command(&data, "pipe D none");
>
> igt_subtest("bad-source")
> - test_bad_command(&data, "pipe A foo");
> + test_bad_source(&data);
>
> igt_subtest("bad-nb-words-1")
> test_bad_command(&data, "pipe foo");
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
