On Thu, Oct 26, 2017 at 01:38:51PM +0200, Maarten Lankhorst wrote:
> In kernel v4.10 the legacy crc api has been replaced by a generic
> drm crc API. While at it, fix igt_require_pipe_crc, the file cannot be
> opened any more when the crtc is not active after kernel commit 8038e09be5a3
> ("drm/crc: Only open CRC on atomic drivers when the CRTC is active.").
> Statting the file should be enough for testing it's supported.
What's the impact of this change on devices running older kernels - such
as KBL ChromeOS on 4.4?
>
> Cc: Tomi Sarvela <tomi.p.sarvela@xxxxxxxxx>
> Cc: Petri Latvala <petri.latvala@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> lib/igt_debugfs.c | 231 +++++++-----------------------------------------------
> 1 file changed, 28 insertions(+), 203 deletions(-)
>
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 8b33b478a9a9..63a0989e5ceb 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -416,16 +416,12 @@ char *igt_crc_to_string(igt_crc_t *crc)
> #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 LEGACY_LINE_LEN (6 * 8 + 5 + 1)
> -
> struct _igt_pipe_crc {
> int fd;
> int dir;
> int ctl_fd;
> int crc_fd;
> int flags;
> - bool is_legacy;
>
> enum pipe pipe;
> enum intel_pipe_crc_source source;
> @@ -449,130 +445,6 @@ static const char *pipe_crc_source_name(enum intel_pipe_crc_source source)
> return pipe_crc_sources[source];
> }
>
> -static bool igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
> -{
> - char buf[64];
> -
> - /* Stop first just to make sure we don't have lingering state left. */
> - igt_pipe_crc_stop(pipe_crc);
> -
> - 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));
> -
> - igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)), strlen(buf));
> -
> - if (!pipe_crc->is_legacy) {
> - int err;
> -
> - sprintf(buf, "crtc-%d/crc/data", pipe_crc->pipe);
> - err = 0;
> -
> - pipe_crc->crc_fd = openat(pipe_crc->dir, buf, pipe_crc->flags);
> - if (pipe_crc->crc_fd < 0)
> - err = -errno;
> -
> - if (err == -EINVAL)
> - return false;
> -
> - igt_assert_eq(err, 0);
> - }
> -
> - errno = 0;
> - return true;
> -}
> -
> -static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
> -{
> - char buf[32];
> -
> - sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe));
> - igt_assert_eq(write(fd, buf, strlen(buf)), strlen(buf));
> -}
> -
> -static void igt_pipe_crc_reset(int drm_fd)
> -{
> - struct dirent *dirent;
> - const char *cmd = "none";
> - bool done = false;
> - DIR *dir;
> - int fdir;
> - int fd;
> -
> - fdir = igt_debugfs_dir(drm_fd);
> - if (fdir < 0)
> - return;
> -
> - dir = fdopendir(fdir);
> - if (!dir) {
> - close(fdir);
> - return;
> - }
> -
> - while ((dirent = readdir(dir))) {
> - char buf[PATH_MAX];
> -
> - if (strcmp(dirent->d_name, "crtc-") != 0)
> - continue;
> -
> - sprintf(buf, "%s/crc/control", dirent->d_name);
> - fd = openat(fdir, buf, O_WRONLY);
> - if (fd < 0)
> - continue;
> -
> - igt_assert_eq(write(fd, cmd, strlen(cmd)), strlen(cmd));
> - close(fd);
> -
> - done = true;
> - }
> - closedir(dir);
> -
> - if (!done) {
> - fd = openat(fdir, "i915_display_crtc_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(fdir);
> -}
> -
> -static void pipe_crc_exit_handler(int sig)
> -{
> - struct dirent *dirent;
> - char buf[PATH_MAX];
> - DIR *dir;
> - int fd;
> -
> - dir = opendir("/dev/dri");
> - if (!dir)
> - return;
> -
> - /*
> - * Try to reset CRC capture for all DRM devices, this is only needed
> - * for the legacy CRC ABI and can be completely removed once the
> - * legacy codepaths are removed.
> - */
> - while ((dirent = readdir(dir))) {
> - if (strncmp(dirent->d_name, "card", 4) != 0)
> - continue;
> -
> - sprintf(buf, "/dev/dri/%s", dirent->d_name);
> - fd = open(buf, O_WRONLY);
> -
> - igt_pipe_crc_reset(fd);
> -
> - close(fd);
> - }
> - closedir(dir);
> -}
> -
> /**
> * igt_require_pipe_crc:
> *
> @@ -582,20 +454,15 @@ static void pipe_crc_exit_handler(int sig)
> */
> void igt_require_pipe_crc(int fd)
> {
> - const char *cmd = "pipe A none";
> - int ctl, written;
> -
> - ctl = igt_debugfs_open(fd, "crtc-0/crc/control", O_RDONLY);
> - if (ctl < 0) {
> - ctl = igt_debugfs_open(fd, "i915_display_crc_ctl", O_WRONLY);
> - igt_require_f(ctl,
> - "No display_crc_ctl found, kernel too old\n");
> -
> - written = write(ctl, cmd, strlen(cmd));
> - igt_require_f(written < 0,
> - "CRCs not supported on this platform\n");
> - }
> - close(ctl);
> + int dir;
> + struct stat stat;
> +
> + dir = igt_debugfs_dir(fd);
> + igt_require_f(dir >= 0, "Could not open debugfs directory\n");
> + igt_require_f(fstatat(dir, "crtc-0/crc/control", &stat, 0) == 0,
> + "CRCs not supported on this platform\n");
> +
> + close(dir);
> }
>
> static void igt_hpd_storm_exit_handler(int sig)
> @@ -729,29 +596,13 @@ pipe_crc_new(int fd, enum pipe pipe, enum intel_pipe_crc_source source, int flag
> debugfs = igt_debugfs_dir(fd);
> igt_assert(debugfs != -1);
>
> - igt_install_exit_handler(pipe_crc_exit_handler);
> -
> pipe_crc = calloc(1, sizeof(struct _igt_pipe_crc));
>
> sprintf(buf, "crtc-%d/crc/control", pipe);
> pipe_crc->ctl_fd = openat(debugfs, buf, O_WRONLY);
> - if (pipe_crc->ctl_fd == -1) {
> - pipe_crc->ctl_fd = openat(debugfs,
> - "i915_display_crc_ctl", O_WRONLY);
> - igt_assert(pipe_crc->ctl_fd != -1);
> - pipe_crc->is_legacy = true;
> - }
> -
> - if (pipe_crc->is_legacy) {
> - sprintf(buf, "i915_pipe_%s_crc", kmstest_pipe_name(pipe));
> - pipe_crc->crc_fd = openat(debugfs, 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");
> - }
> + igt_assert(pipe_crc->ctl_fd != -1);
>
> + pipe_crc->crc_fd = -1;
> pipe_crc->fd = fd;
> pipe_crc->dir = debugfs;
> pipe_crc->pipe = pipe;
> @@ -817,18 +668,9 @@ void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc)
> static bool pipe_crc_init_from_string(igt_pipe_crc_t *pipe_crc, igt_crc_t *crc,
> const char *line)
> {
> - int n, i;
> + int i;
> const char *buf;
>
> - 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 {
> @@ -849,15 +691,9 @@ static int read_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
> {
> ssize_t bytes_read;
> 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, read_len);
> + bytes_read = read(pipe_crc->crc_fd, &buf, MAX_LINE_LEN);
> igt_reset_timeout();
>
> if (bytes_read < 0 && errno == EAGAIN)
> @@ -888,22 +724,19 @@ static void read_one_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
> */
> void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
> {
> - igt_crc_t crc;
> -
> - igt_assert(igt_pipe_crc_do_start(pipe_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);
> - }
> + const char *src = pipe_crc_source_name(pipe_crc->source);
> + char buf[32];
> +
> + /* Stop first just to make sure we don't have lingering state left. */
> + igt_pipe_crc_stop(pipe_crc);
> +
> + igt_assert_eq(write(pipe_crc->ctl_fd, src, strlen(src)), strlen(src));
> +
> + sprintf(buf, "crtc-%d/crc/data", pipe_crc->pipe);
> +
> + pipe_crc->crc_fd = openat(pipe_crc->dir, buf, pipe_crc->flags);
> + igt_assert(pipe_crc->crc_fd != -1);
> + errno = 0;
> }
>
> /**
> @@ -914,16 +747,8 @@ void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
> */
> void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
> {
> - char buf[32];
> -
> - 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;
> - }
> + close(pipe_crc->crc_fd);
> + pipe_crc->crc_fd = -1;
> }
>
> /**
> --
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
