Re: [igt-dev] [PATCH i-g-t 3/3] lib/igt_kms: Remove fragile display_info debugfs parsing

Kahola, Mika
2019-11-19T10:43:14Z

On Fri, 2019-11-08 at 17:57 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Relying on the specific layout of the display_info debugfs
> file is fragile. Any change in the file layout will likely
> result in test failure. Let's just get rid of it.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

> ---
> lib/igt_kms.c | 141 ------------------------------------------------
> --
> lib/igt_kms.h | 1 -
> 2 files changed, 142 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e9b80b9bcb81..7095d8252262 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1520,147 +1520,6 @@ void kmstest_wait_for_pageflip(int fd)
> igt_assert(drmHandleEvent(fd, &evctx) == 0);
> }
>
> -static void get_plane(char *str, int type, struct kmstest_plane
> *plane)
> -{
> - int ret;
> - char buf[256];
> -
> - plane->type = type;
> - ret = sscanf(str + 12, "%d%*c %*s %[^n]s",
> - &plane->id,
> - buf);
> - igt_assert_eq(ret, 2);
> -
> - ret = sscanf(buf + 9, "%4d%*c%4d%*c", &plane->pos_x, &plane-
> >pos_y);
> - igt_assert_eq(ret, 2);
> -
> - ret = sscanf(buf + 30, "%4d%*c%4d%*c", &plane->width, &plane-
> >height);
> - igt_assert_eq(ret, 2);
> -}
> -
> -static int parse_planes(FILE *fid, struct kmstest_plane *planes)
> -{
> - char tmp[256];
> - int n_planes;
> -
> - n_planes = 0;
> - while (fgets(tmp, 256, fid) != NULL) {
> - if (strstr(tmp, "type=PRI") != NULL) {
> - if (planes) {
> - get_plane(tmp, DRM_PLANE_TYPE_PRIMARY,
> &planes[n_planes]);
> - planes[n_planes].index = n_planes;
> - }
> - n_planes++;
> - } else if (strstr(tmp, "type=OVL") != NULL) {
> - if (planes) {
> - get_plane(tmp, DRM_PLANE_TYPE_OVERLAY,
> &planes[n_planes]);
> - planes[n_planes].index = n_planes;
> - }
> - n_planes++;
> - } else if (strstr(tmp, "type=CUR") != NULL) {
> - if (planes) {
> - get_plane(tmp, DRM_PLANE_TYPE_CURSOR,
> &planes[n_planes]);
> - planes[n_planes].index = n_planes;
> - }
> - n_planes++;
> - break;
> - }
> - }
> -
> - return n_planes;
> -}
> -
> -static void parse_crtc(char *info, struct kmstest_crtc *crtc)
> -{
> - char buf[256];
> - int ret;
> - char pipe;
> -
> - ret = sscanf(info + 4, "%d%*c %*s %c%*c %*s %s%*c",
> - &crtc->id, &pipe, buf);
> - igt_assert_eq(ret, 3);
> -
> - crtc->pipe = kmstest_pipe_to_index(pipe);
> - igt_assert(crtc->pipe >= 0);
> -
> - ret = sscanf(buf + 6, "%d%*c%d%*c",
> - &crtc->width, &crtc->height);
> - igt_assert_eq(ret, 2);
> -}
> -
> -static void kmstest_get_crtc(int device, enum pipe pipe, struct
> kmstest_crtc *crtc)
> -{
> - char tmp[256];
> - FILE *file;
> - int ncrtc;
> - int line;
> - long int n;
> - int fd;
> -
> - fd = igt_debugfs_open(device, "i915_display_info", O_RDONLY);
> - file = fdopen(fd, "r");
> - igt_skip_on(file == NULL);
> -
> - ncrtc = 0;
> - line = 0;
> - while (fgets(tmp, 256, file) != NULL) {
> - if ((strstr(tmp, "CRTC") != NULL) && (line > 0)) {
> - if (strstr(tmp, "active=yes") != NULL) {
> - crtc->active = true;
> - parse_crtc(tmp, crtc);
> -
> - n = ftell(file);
> - crtc->n_planes = parse_planes(file,
> NULL);
> - igt_assert_lt(0, crtc->n_planes);
> - crtc->planes = calloc(crtc->n_planes,
> sizeof(*crtc->planes));
> - igt_assert_f(crtc->planes, "Failed to
> allocate memory for %d planes\n", crtc->n_planes);
> -
> - fseek(file, n, SEEK_SET);
> - parse_planes(file, crtc->planes);
> -
> - if (crtc->pipe != pipe) {
> - free(crtc->planes);
> - } else {
> - ncrtc++;
> - break;
> - }
> - }
> - }
> -
> - line++;
> - }
> -
> - fclose(file);
> - close(fd);
> -
> - igt_assert(ncrtc == 1);
> -}
> -
> -/**
> - * igt_assert_plane_visible:
> - * @fd: Opened file descriptor
> - * @pipe: Display pipe
> - * @visibility: Boolean parameter to test against the plane's
> current visibility state
> - *
> - * Asserts only if the plane's visibility state matches the status
> being passed by @visibility
> - */
> -void igt_assert_plane_visible(int fd, enum pipe pipe, int
> plane_index, bool visibility)
> -{
> - struct kmstest_crtc crtc;
> - bool visible = true;
> -
> - kmstest_get_crtc(fd, pipe, &crtc);
> -
> - igt_assert(plane_index < crtc.n_planes);
> -
> - if (crtc.planes[plane_index].pos_x > crtc.width ||
> - crtc.planes[plane_index].pos_y > crtc.height)
> - visible = false;
> -
> - free(crtc.planes);
> - igt_assert_eq(visible, visibility);
> -}
> -
> /**
> * kms_has_vblank:
> * @fd: DRM fd
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 7193f9a50ea2..3f798392f8a0 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -228,7 +228,6 @@ void *kmstest_dumb_map_buffer(int fd, uint32_t
> handle, uint64_t size,
> void kmstest_dumb_destroy(int fd, uint32_t handle);
> void kmstest_wait_for_pageflip(int fd);
> unsigned int kmstest_get_vblank(int fd, int pipe, unsigned int
> flags);
> -void igt_assert_plane_visible(int fd, enum pipe pipe, int
> plane_index, bool visibility);
>
> bool kms_has_vblank(int fd);
>