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

["Kahola, Mika" <mika.kahola@intel.com>]

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);
>  
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev