All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t v8 1/3] include: bump drm uAPI headers
@ 2018-03-09  9:31 Lionel Landwerlin
  2018-03-09  9:31 ` [igt-dev] [PATCH i-g-t v8 2/3] intel_chipsets: store GT information in device info Lionel Landwerlin
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Lionel Landwerlin @ 2018-03-09  9:31 UTC (permalink / raw)
  To: igt-dev

Taken from drm-tip :

commit c029d45203be4d1f9d32606d49633ecff63d6ba9
Author: Heiko Stuebner <heiko@sntech.de>
Date:   Thu Mar 8 17:34:44 2018 +0100

    drm-tip: 2018y-03m-08d-16h-33m-17s UTC integration manifest

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 include/drm-uapi/amdgpu_drm.h  |   4 +
 include/drm-uapi/drm_fourcc.h  |  38 ++++----
 include/drm-uapi/drm_mode.h    |  43 +++++++--
 include/drm-uapi/exynos_drm.h  | 192 +----------------------------------------
 include/drm-uapi/i915_drm.h    | 152 +++++++++++++++++++++++++++++++-
 include/drm-uapi/vc4_drm.h     |  76 ++++++++++++++++
 include/drm-uapi/virtgpu_drm.h |   1 +
 lib/igt_perf.h                 |   7 --
 8 files changed, 284 insertions(+), 229 deletions(-)

diff --git a/include/drm-uapi/amdgpu_drm.h b/include/drm-uapi/amdgpu_drm.h
index 4d21191a..1816bd82 100644
--- a/include/drm-uapi/amdgpu_drm.h
+++ b/include/drm-uapi/amdgpu_drm.h
@@ -664,6 +664,10 @@ struct drm_amdgpu_cs_chunk_data {
 	#define AMDGPU_INFO_SENSOR_VDDNB		0x6
 	/* Subquery id: Query graphics voltage */
 	#define AMDGPU_INFO_SENSOR_VDDGFX		0x7
+	/* Subquery id: Query GPU stable pstate shader clock */
+	#define AMDGPU_INFO_SENSOR_STABLE_PSTATE_GFX_SCLK		0x8
+	/* Subquery id: Query GPU stable pstate memory clock */
+	#define AMDGPU_INFO_SENSOR_STABLE_PSTATE_GFX_MCLK		0x9
 /* Number of VRAM page faults on CPU access. */
 #define AMDGPU_INFO_NUM_VRAM_CPU_PAGE_FAULTS	0x1E
 #define AMDGPU_INFO_VRAM_LOST_COUNTER		0x1F
diff --git a/include/drm-uapi/drm_fourcc.h b/include/drm-uapi/drm_fourcc.h
index 3ad838d3..e04613d3 100644
--- a/include/drm-uapi/drm_fourcc.h
+++ b/include/drm-uapi/drm_fourcc.h
@@ -178,7 +178,7 @@ extern "C" {
 #define DRM_FORMAT_MOD_VENDOR_NONE    0
 #define DRM_FORMAT_MOD_VENDOR_INTEL   0x01
 #define DRM_FORMAT_MOD_VENDOR_AMD     0x02
-#define DRM_FORMAT_MOD_VENDOR_NV      0x03
+#define DRM_FORMAT_MOD_VENDOR_NVIDIA  0x03
 #define DRM_FORMAT_MOD_VENDOR_SAMSUNG 0x04
 #define DRM_FORMAT_MOD_VENDOR_QCOM    0x05
 #define DRM_FORMAT_MOD_VENDOR_VIVANTE 0x06
@@ -188,7 +188,7 @@ extern "C" {
 #define DRM_FORMAT_RESERVED	      ((1ULL << 56) - 1)
 
 #define fourcc_mod_code(vendor, val) \
-	((((__u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 0x00ffffffffffffffULL))
+	((((__u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | ((val) & 0x00ffffffffffffffULL))
 
 /*
  * Format Modifier tokens:
@@ -338,29 +338,17 @@ extern "C" {
  */
 #define DRM_FORMAT_MOD_VIVANTE_SPLIT_SUPER_TILED fourcc_mod_code(VIVANTE, 4)
 
-/* NVIDIA Tegra frame buffer modifiers */
-
-/*
- * Some modifiers take parameters, for example the number of vertical GOBs in
- * a block. Reserve the lower 32 bits for parameters
- */
-#define __fourcc_mod_tegra_mode_shift 32
-#define fourcc_mod_tegra_code(val, params) \
-	fourcc_mod_code(NV, ((((__u64)val) << __fourcc_mod_tegra_mode_shift) | params))
-#define fourcc_mod_tegra_mod(m) \
-	(m & ~((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
-#define fourcc_mod_tegra_param(m) \
-	(m & ((1ULL << __fourcc_mod_tegra_mode_shift) - 1))
+/* NVIDIA frame buffer modifiers */
 
 /*
  * Tegra Tiled Layout, used by Tegra 2, 3 and 4.
  *
  * Pixels are arranged in simple tiles of 16 x 16 bytes.
  */
-#define NV_FORMAT_MOD_TEGRA_TILED fourcc_mod_tegra_code(1, 0)
+#define DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED fourcc_mod_code(NVIDIA, 1)
 
 /*
- * Tegra 16Bx2 Block Linear layout, used by TK1/TX1
+ * 16Bx2 Block Linear layout, used by desktop GPUs, and Tegra K1 and later
  *
  * Pixels are arranged in 64x8 Groups Of Bytes (GOBs). GOBs are then stacked
  * vertically by a power of 2 (1 to 32 GOBs) to form a block.
@@ -380,7 +368,21 @@ extern "C" {
  * Chapter 20 "Pixel Memory Formats" of the Tegra X1 TRM describes this format
  * in full detail.
  */
-#define NV_FORMAT_MOD_TEGRA_16BX2_BLOCK(v) fourcc_mod_tegra_code(2, v)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(v) \
+	fourcc_mod_code(NVIDIA, 0x10 | ((v) & 0xf))
+
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_ONE_GOB \
+	fourcc_mod_code(NVIDIA, 0x10)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_TWO_GOB \
+	fourcc_mod_code(NVIDIA, 0x11)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_FOUR_GOB \
+	fourcc_mod_code(NVIDIA, 0x12)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_EIGHT_GOB \
+	fourcc_mod_code(NVIDIA, 0x13)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_SIXTEEN_GOB \
+	fourcc_mod_code(NVIDIA, 0x14)
+#define DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_THIRTYTWO_GOB \
+	fourcc_mod_code(NVIDIA, 0x15)
 
 /*
  * Broadcom VC4 "T" format
diff --git a/include/drm-uapi/drm_mode.h b/include/drm-uapi/drm_mode.h
index 5597a871..50bcf421 100644
--- a/include/drm-uapi/drm_mode.h
+++ b/include/drm-uapi/drm_mode.h
@@ -38,14 +38,18 @@ extern "C" {
 #define DRM_DISPLAY_MODE_LEN	32
 #define DRM_PROP_NAME_LEN	32
 
-#define DRM_MODE_TYPE_BUILTIN	(1<<0)
-#define DRM_MODE_TYPE_CLOCK_C	((1<<1) | DRM_MODE_TYPE_BUILTIN)
-#define DRM_MODE_TYPE_CRTC_C	((1<<2) | DRM_MODE_TYPE_BUILTIN)
+#define DRM_MODE_TYPE_BUILTIN	(1<<0) /* deprecated */
+#define DRM_MODE_TYPE_CLOCK_C	((1<<1) | DRM_MODE_TYPE_BUILTIN) /* deprecated */
+#define DRM_MODE_TYPE_CRTC_C	((1<<2) | DRM_MODE_TYPE_BUILTIN) /* deprecated */
 #define DRM_MODE_TYPE_PREFERRED	(1<<3)
-#define DRM_MODE_TYPE_DEFAULT	(1<<4)
+#define DRM_MODE_TYPE_DEFAULT	(1<<4) /* deprecated */
 #define DRM_MODE_TYPE_USERDEF	(1<<5)
 #define DRM_MODE_TYPE_DRIVER	(1<<6)
 
+#define DRM_MODE_TYPE_ALL	(DRM_MODE_TYPE_PREFERRED |	\
+				 DRM_MODE_TYPE_USERDEF |	\
+				 DRM_MODE_TYPE_DRIVER)
+
 /* Video mode flags */
 /* bit compatible with the xrandr RR_ definitions (bits 0-13)
  *
@@ -66,8 +70,8 @@ extern "C" {
 #define DRM_MODE_FLAG_PCSYNC			(1<<7)
 #define DRM_MODE_FLAG_NCSYNC			(1<<8)
 #define DRM_MODE_FLAG_HSKEW			(1<<9) /* hskew provided */
-#define DRM_MODE_FLAG_BCAST			(1<<10)
-#define DRM_MODE_FLAG_PIXMUX			(1<<11)
+#define DRM_MODE_FLAG_BCAST			(1<<10) /* deprecated */
+#define DRM_MODE_FLAG_PIXMUX			(1<<11) /* deprecated */
 #define DRM_MODE_FLAG_DBLCLK			(1<<12)
 #define DRM_MODE_FLAG_CLKDIV2			(1<<13)
  /*
@@ -99,6 +103,20 @@ extern "C" {
 #define  DRM_MODE_FLAG_PIC_AR_16_9 \
 			(DRM_MODE_PICTURE_ASPECT_16_9<<19)
 
+#define  DRM_MODE_FLAG_ALL	(DRM_MODE_FLAG_PHSYNC |		\
+				 DRM_MODE_FLAG_NHSYNC |		\
+				 DRM_MODE_FLAG_PVSYNC |		\
+				 DRM_MODE_FLAG_NVSYNC |		\
+				 DRM_MODE_FLAG_INTERLACE |	\
+				 DRM_MODE_FLAG_DBLSCAN |	\
+				 DRM_MODE_FLAG_CSYNC |		\
+				 DRM_MODE_FLAG_PCSYNC |		\
+				 DRM_MODE_FLAG_NCSYNC |		\
+				 DRM_MODE_FLAG_HSKEW |		\
+				 DRM_MODE_FLAG_DBLCLK |		\
+				 DRM_MODE_FLAG_CLKDIV2 |	\
+				 DRM_MODE_FLAG_3D_MASK)
+
 /* DPMS flags */
 /* bit compatible with the xorg definitions. */
 #define DRM_MODE_DPMS_ON	0
@@ -173,6 +191,10 @@ extern "C" {
 		DRM_MODE_REFLECT_X | \
 		DRM_MODE_REFLECT_Y)
 
+/* Content Protection Flags */
+#define DRM_MODE_CONTENT_PROTECTION_UNDESIRED	0
+#define DRM_MODE_CONTENT_PROTECTION_DESIRED     1
+#define DRM_MODE_CONTENT_PROTECTION_ENABLED     2
 
 struct drm_mode_modeinfo {
 	__u32 clock;
@@ -341,7 +363,7 @@ struct drm_mode_get_connector {
 	__u32 pad;
 };
 
-#define DRM_MODE_PROP_PENDING	(1<<0)
+#define DRM_MODE_PROP_PENDING	(1<<0) /* deprecated, do not use */
 #define DRM_MODE_PROP_RANGE	(1<<1)
 #define DRM_MODE_PROP_IMMUTABLE	(1<<2)
 #define DRM_MODE_PROP_ENUM	(1<<3) /* enumerated type with text strings */
@@ -576,8 +598,11 @@ struct drm_mode_crtc_lut {
 };
 
 struct drm_color_ctm {
-	/* Conversion matrix in S31.32 format. */
-	__s64 matrix[9];
+	/*
+	 * Conversion matrix in S31.32 sign-magnitude
+	 * (not two's complement!) format.
+	 */
+	__u64 matrix[9];
 };
 
 struct drm_color_lut {
diff --git a/include/drm-uapi/exynos_drm.h b/include/drm-uapi/exynos_drm.h
index 76c34dd5..a00116b5 100644
--- a/include/drm-uapi/exynos_drm.h
+++ b/include/drm-uapi/exynos_drm.h
@@ -135,172 +135,6 @@ struct drm_exynos_g2d_exec {
 	__u64					async;
 };
 
-enum drm_exynos_ops_id {
-	EXYNOS_DRM_OPS_SRC,
-	EXYNOS_DRM_OPS_DST,
-	EXYNOS_DRM_OPS_MAX,
-};
-
-struct drm_exynos_sz {
-	__u32	hsize;
-	__u32	vsize;
-};
-
-struct drm_exynos_pos {
-	__u32	x;
-	__u32	y;
-	__u32	w;
-	__u32	h;
-};
-
-enum drm_exynos_flip {
-	EXYNOS_DRM_FLIP_NONE = (0 << 0),
-	EXYNOS_DRM_FLIP_VERTICAL = (1 << 0),
-	EXYNOS_DRM_FLIP_HORIZONTAL = (1 << 1),
-	EXYNOS_DRM_FLIP_BOTH = EXYNOS_DRM_FLIP_VERTICAL |
-			EXYNOS_DRM_FLIP_HORIZONTAL,
-};
-
-enum drm_exynos_degree {
-	EXYNOS_DRM_DEGREE_0,
-	EXYNOS_DRM_DEGREE_90,
-	EXYNOS_DRM_DEGREE_180,
-	EXYNOS_DRM_DEGREE_270,
-};
-
-enum drm_exynos_planer {
-	EXYNOS_DRM_PLANAR_Y,
-	EXYNOS_DRM_PLANAR_CB,
-	EXYNOS_DRM_PLANAR_CR,
-	EXYNOS_DRM_PLANAR_MAX,
-};
-
-/**
- * A structure for ipp supported property list.
- *
- * @version: version of this structure.
- * @ipp_id: id of ipp driver.
- * @count: count of ipp driver.
- * @writeback: flag of writeback supporting.
- * @flip: flag of flip supporting.
- * @degree: flag of degree information.
- * @csc: flag of csc supporting.
- * @crop: flag of crop supporting.
- * @scale: flag of scale supporting.
- * @refresh_min: min hz of refresh.
- * @refresh_max: max hz of refresh.
- * @crop_min: crop min resolution.
- * @crop_max: crop max resolution.
- * @scale_min: scale min resolution.
- * @scale_max: scale max resolution.
- */
-struct drm_exynos_ipp_prop_list {
-	__u32	version;
-	__u32	ipp_id;
-	__u32	count;
-	__u32	writeback;
-	__u32	flip;
-	__u32	degree;
-	__u32	csc;
-	__u32	crop;
-	__u32	scale;
-	__u32	refresh_min;
-	__u32	refresh_max;
-	__u32	reserved;
-	struct drm_exynos_sz	crop_min;
-	struct drm_exynos_sz	crop_max;
-	struct drm_exynos_sz	scale_min;
-	struct drm_exynos_sz	scale_max;
-};
-
-/**
- * A structure for ipp config.
- *
- * @ops_id: property of operation directions.
- * @flip: property of mirror, flip.
- * @degree: property of rotation degree.
- * @fmt: property of image format.
- * @sz: property of image size.
- * @pos: property of image position(src-cropped,dst-scaler).
- */
-struct drm_exynos_ipp_config {
-	__u32 ops_id;
-	__u32 flip;
-	__u32 degree;
-	__u32	fmt;
-	struct drm_exynos_sz	sz;
-	struct drm_exynos_pos	pos;
-};
-
-enum drm_exynos_ipp_cmd {
-	IPP_CMD_NONE,
-	IPP_CMD_M2M,
-	IPP_CMD_WB,
-	IPP_CMD_OUTPUT,
-	IPP_CMD_MAX,
-};
-
-/**
- * A structure for ipp property.
- *
- * @config: source, destination config.
- * @cmd: definition of command.
- * @ipp_id: id of ipp driver.
- * @prop_id: id of property.
- * @refresh_rate: refresh rate.
- */
-struct drm_exynos_ipp_property {
-	struct drm_exynos_ipp_config config[EXYNOS_DRM_OPS_MAX];
-	__u32	cmd;
-	__u32	ipp_id;
-	__u32	prop_id;
-	__u32	refresh_rate;
-};
-
-enum drm_exynos_ipp_buf_type {
-	IPP_BUF_ENQUEUE,
-	IPP_BUF_DEQUEUE,
-};
-
-/**
- * A structure for ipp buffer operations.
- *
- * @ops_id: operation directions.
- * @buf_type: definition of buffer.
- * @prop_id: id of property.
- * @buf_id: id of buffer.
- * @handle: Y, Cb, Cr each planar handle.
- * @user_data: user data.
- */
-struct drm_exynos_ipp_queue_buf {
-	__u32	ops_id;
-	__u32	buf_type;
-	__u32	prop_id;
-	__u32	buf_id;
-	__u32	handle[EXYNOS_DRM_PLANAR_MAX];
-	__u32	reserved;
-	__u64	user_data;
-};
-
-enum drm_exynos_ipp_ctrl {
-	IPP_CTRL_PLAY,
-	IPP_CTRL_STOP,
-	IPP_CTRL_PAUSE,
-	IPP_CTRL_RESUME,
-	IPP_CTRL_MAX,
-};
-
-/**
- * A structure for ipp start/stop operations.
- *
- * @prop_id: id of property.
- * @ctrl: definition of control.
- */
-struct drm_exynos_ipp_cmd_ctrl {
-	__u32	prop_id;
-	__u32	ctrl;
-};
-
 #define DRM_EXYNOS_GEM_CREATE		0x00
 #define DRM_EXYNOS_GEM_MAP		0x01
 /* Reserved 0x03 ~ 0x05 for exynos specific gem ioctl */
@@ -312,11 +146,7 @@ struct drm_exynos_ipp_cmd_ctrl {
 #define DRM_EXYNOS_G2D_SET_CMDLIST	0x21
 #define DRM_EXYNOS_G2D_EXEC		0x22
 
-/* IPP - Image Post Processing */
-#define DRM_EXYNOS_IPP_GET_PROPERTY	0x30
-#define DRM_EXYNOS_IPP_SET_PROPERTY	0x31
-#define DRM_EXYNOS_IPP_QUEUE_BUF	0x32
-#define DRM_EXYNOS_IPP_CMD_CTRL	0x33
+/* Reserved 0x30 ~ 0x33 for obsolete Exynos IPP ioctls */
 
 #define DRM_IOCTL_EXYNOS_GEM_CREATE		DRM_IOWR(DRM_COMMAND_BASE + \
 		DRM_EXYNOS_GEM_CREATE, struct drm_exynos_gem_create)
@@ -335,18 +165,8 @@ struct drm_exynos_ipp_cmd_ctrl {
 #define DRM_IOCTL_EXYNOS_G2D_EXEC		DRM_IOWR(DRM_COMMAND_BASE + \
 		DRM_EXYNOS_G2D_EXEC, struct drm_exynos_g2d_exec)
 
-#define DRM_IOCTL_EXYNOS_IPP_GET_PROPERTY	DRM_IOWR(DRM_COMMAND_BASE + \
-		DRM_EXYNOS_IPP_GET_PROPERTY, struct drm_exynos_ipp_prop_list)
-#define DRM_IOCTL_EXYNOS_IPP_SET_PROPERTY	DRM_IOWR(DRM_COMMAND_BASE + \
-		DRM_EXYNOS_IPP_SET_PROPERTY, struct drm_exynos_ipp_property)
-#define DRM_IOCTL_EXYNOS_IPP_QUEUE_BUF	DRM_IOWR(DRM_COMMAND_BASE + \
-		DRM_EXYNOS_IPP_QUEUE_BUF, struct drm_exynos_ipp_queue_buf)
-#define DRM_IOCTL_EXYNOS_IPP_CMD_CTRL		DRM_IOWR(DRM_COMMAND_BASE + \
-		DRM_EXYNOS_IPP_CMD_CTRL, struct drm_exynos_ipp_cmd_ctrl)
-
 /* EXYNOS specific events */
 #define DRM_EXYNOS_G2D_EVENT		0x80000000
-#define DRM_EXYNOS_IPP_EVENT		0x80000001
 
 struct drm_exynos_g2d_event {
 	struct drm_event	base;
@@ -357,16 +177,6 @@ struct drm_exynos_g2d_event {
 	__u32			reserved;
 };
 
-struct drm_exynos_ipp_event {
-	struct drm_event	base;
-	__u64			user_data;
-	__u32			tv_sec;
-	__u32			tv_usec;
-	__u32			prop_id;
-	__u32			reserved;
-	__u32			buf_id[EXYNOS_DRM_OPS_MAX];
-};
-
 #if defined(__cplusplus)
 }
 #endif
diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
index 7f28eea4..16e452aa 100644
--- a/include/drm-uapi/i915_drm.h
+++ b/include/drm-uapi/i915_drm.h
@@ -102,6 +102,46 @@ enum drm_i915_gem_engine_class {
 	I915_ENGINE_CLASS_INVALID	= -1
 };
 
+/**
+ * DOC: perf_events exposed by i915 through /sys/bus/event_sources/drivers/i915
+ *
+ */
+
+enum drm_i915_pmu_engine_sample {
+	I915_SAMPLE_BUSY = 0,
+	I915_SAMPLE_WAIT = 1,
+	I915_SAMPLE_SEMA = 2
+};
+
+#define I915_PMU_SAMPLE_BITS (4)
+#define I915_PMU_SAMPLE_MASK (0xf)
+#define I915_PMU_SAMPLE_INSTANCE_BITS (8)
+#define I915_PMU_CLASS_SHIFT \
+	(I915_PMU_SAMPLE_BITS + I915_PMU_SAMPLE_INSTANCE_BITS)
+
+#define __I915_PMU_ENGINE(class, instance, sample) \
+	((class) << I915_PMU_CLASS_SHIFT | \
+	(instance) << I915_PMU_SAMPLE_BITS | \
+	(sample))
+
+#define I915_PMU_ENGINE_BUSY(class, instance) \
+	__I915_PMU_ENGINE(class, instance, I915_SAMPLE_BUSY)
+
+#define I915_PMU_ENGINE_WAIT(class, instance) \
+	__I915_PMU_ENGINE(class, instance, I915_SAMPLE_WAIT)
+
+#define I915_PMU_ENGINE_SEMA(class, instance) \
+	__I915_PMU_ENGINE(class, instance, I915_SAMPLE_SEMA)
+
+#define __I915_PMU_OTHER(x) (__I915_PMU_ENGINE(0xff, 0xff, 0xf) + 1 + (x))
+
+#define I915_PMU_ACTUAL_FREQUENCY	__I915_PMU_OTHER(0)
+#define I915_PMU_REQUESTED_FREQUENCY	__I915_PMU_OTHER(1)
+#define I915_PMU_INTERRUPTS		__I915_PMU_OTHER(2)
+#define I915_PMU_RC6_RESIDENCY		__I915_PMU_OTHER(3)
+
+#define I915_PMU_LAST I915_PMU_RC6_RESIDENCY
+
 /* Each region is a minimum of 16k, and there are at most 255 of them.
  */
 #define I915_NR_TEX_REGIONS 255	/* table size 2k - maximum due to use
@@ -278,6 +318,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_I915_PERF_OPEN		0x36
 #define DRM_I915_PERF_ADD_CONFIG	0x37
 #define DRM_I915_PERF_REMOVE_CONFIG	0x38
+#define DRM_I915_QUERY			0x39
 
 #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
 #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
@@ -335,6 +376,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_PERF_OPEN	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param)
 #define DRM_IOCTL_I915_PERF_ADD_CONFIG	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config)
 #define DRM_IOCTL_I915_PERF_REMOVE_CONFIG	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64)
+#define DRM_IOCTL_I915_QUERY			DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY, struct drm_i915_query)
 
 /* Allow drivers to submit batchbuffers directly to hardware, relying
  * on the security mechanisms provided by hardware.
@@ -1318,7 +1360,9 @@ struct drm_intel_overlay_attrs {
  * active on a given plane.
  */
 
-#define I915_SET_COLORKEY_NONE		(1<<0) /* disable color key matching */
+#define I915_SET_COLORKEY_NONE		(1<<0) /* Deprecated. Instead set
+						* flags==0 to disable colorkeying.
+						*/
 #define I915_SET_COLORKEY_DESTINATION	(1<<1)
 #define I915_SET_COLORKEY_SOURCE	(1<<2)
 struct drm_intel_sprite_colorkey {
@@ -1564,15 +1608,115 @@ struct drm_i915_perf_oa_config {
 	__u32 n_flex_regs;
 
 	/*
-	 * These fields are pointers to tuples of u32 values (register
-	 * address, value). For example the expected length of the buffer
-	 * pointed by mux_regs_ptr is (2 * sizeof(u32) * n_mux_regs).
+	 * These fields are pointers to tuples of u32 values (register address,
+	 * value). For example the expected length of the buffer pointed by
+	 * mux_regs_ptr is (2 * sizeof(u32) * n_mux_regs).
 	 */
 	__u64 mux_regs_ptr;
 	__u64 boolean_regs_ptr;
 	__u64 flex_regs_ptr;
 };
 
+struct drm_i915_query_item {
+	__u64 query_id;
+#define DRM_I915_QUERY_TOPOLOGY_INFO    1
+
+	/*
+	 * When set to zero by userspace, this is filled with the size of the
+	 * data to be written at the data_ptr pointer. The kernel sets this
+	 * value to a negative value to signal an error on a particular query
+	 * item.
+	 */
+	__s32 length;
+
+	/*
+	 * Unused for now. Must be cleared to zero.
+	 */
+	__u32 flags;
+
+	/*
+	 * Data will be written at the location pointed by data_ptr when the
+	 * value of length matches the length of the data to be written by the
+	 * kernel.
+	 */
+	__u64 data_ptr;
+};
+
+struct drm_i915_query {
+	__u32 num_items;
+
+	/*
+	 * Unused for now. Must be cleared to zero.
+	 */
+	__u32 flags;
+
+	/*
+	 * This points to an array of num_items drm_i915_query_item structures.
+	 */
+	__u64 items_ptr;
+};
+
+/*
+ * Data written by the kernel with query DRM_I915_QUERY_TOPOLOGY_INFO :
+ *
+ * data: contains the 3 pieces of information :
+ *
+ * - the slice mask with one bit per slice telling whether a slice is
+ *   available. The availability of slice X can be queried with the following
+ *   formula :
+ *
+ *           (data[X / 8] >> (X % 8)) & 1
+ *
+ * - the subslice mask for each slice with one bit per subslice telling
+ *   whether a subslice is available. The availability of subslice Y in slice
+ *   X can be queried with the following formula :
+ *
+ *           (data[subslice_offset +
+ *                 X * subslice_stride +
+ *                 Y / 8] >> (Y % 8)) & 1
+ *
+ * - the EU mask for each subslice in each slice with one bit per EU telling
+ *   whether an EU is available. The availability of EU Z in subslice Y in
+ *   slice X can be queried with the following formula :
+ *
+ *           (data[eu_offset +
+ *                 (X * max_subslices + Y) * eu_stride +
+ *                 Z / 8] >> (Z % 8)) & 1
+ */
+struct drm_i915_query_topology_info {
+	/*
+	 * Unused for now. Must be cleared to zero.
+	 */
+	__u16 flags;
+
+	__u16 max_slices;
+	__u16 max_subslices;
+	__u16 max_eus_per_subslice;
+
+	/*
+	 * Offset in data[] at which the subslice masks are stored.
+	 */
+	__u16 subslice_offset;
+
+	/*
+	 * Stride at which each of the subslice masks for each slice are
+	 * stored.
+	 */
+	__u16 subslice_stride;
+
+	/*
+	 * Offset in data[] at which the EU masks are stored.
+	 */
+	__u16 eu_offset;
+
+	/*
+	 * Stride at which each of the EU masks for each subslice are stored.
+	 */
+	__u16 eu_stride;
+
+	__u8 data[];
+};
+
 #if defined(__cplusplus)
 }
 #endif
diff --git a/include/drm-uapi/vc4_drm.h b/include/drm-uapi/vc4_drm.h
index 3415a4b7..4117117b 100644
--- a/include/drm-uapi/vc4_drm.h
+++ b/include/drm-uapi/vc4_drm.h
@@ -42,6 +42,9 @@ extern "C" {
 #define DRM_VC4_GET_TILING                        0x09
 #define DRM_VC4_LABEL_BO                          0x0a
 #define DRM_VC4_GEM_MADVISE                       0x0b
+#define DRM_VC4_PERFMON_CREATE                    0x0c
+#define DRM_VC4_PERFMON_DESTROY                   0x0d
+#define DRM_VC4_PERFMON_GET_VALUES                0x0e
 
 #define DRM_IOCTL_VC4_SUBMIT_CL           DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_SUBMIT_CL, struct drm_vc4_submit_cl)
 #define DRM_IOCTL_VC4_WAIT_SEQNO          DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_WAIT_SEQNO, struct drm_vc4_wait_seqno)
@@ -55,6 +58,9 @@ extern "C" {
 #define DRM_IOCTL_VC4_GET_TILING          DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_GET_TILING, struct drm_vc4_get_tiling)
 #define DRM_IOCTL_VC4_LABEL_BO            DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_LABEL_BO, struct drm_vc4_label_bo)
 #define DRM_IOCTL_VC4_GEM_MADVISE         DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_GEM_MADVISE, struct drm_vc4_gem_madvise)
+#define DRM_IOCTL_VC4_PERFMON_CREATE      DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_PERFMON_CREATE, struct drm_vc4_perfmon_create)
+#define DRM_IOCTL_VC4_PERFMON_DESTROY     DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_PERFMON_DESTROY, struct drm_vc4_perfmon_destroy)
+#define DRM_IOCTL_VC4_PERFMON_GET_VALUES  DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_PERFMON_GET_VALUES, struct drm_vc4_perfmon_get_values)
 
 struct drm_vc4_submit_rcl_surface {
 	__u32 hindex; /* Handle index, or ~0 if not present. */
@@ -173,6 +179,15 @@ struct drm_vc4_submit_cl {
 	 * wait ioctl).
 	 */
 	__u64 seqno;
+
+	/* ID of the perfmon to attach to this job. 0 means no perfmon. */
+	__u32 perfmonid;
+
+	/* Unused field to align this struct on 64 bits. Must be set to 0.
+	 * If one ever needs to add an u32 field to this struct, this field
+	 * can be used.
+	 */
+	__u32 pad2;
 };
 
 /**
@@ -308,6 +323,7 @@ struct drm_vc4_get_hang_state {
 #define DRM_VC4_PARAM_SUPPORTS_THREADED_FS	5
 #define DRM_VC4_PARAM_SUPPORTS_FIXED_RCL_ORDER	6
 #define DRM_VC4_PARAM_SUPPORTS_MADVISE		7
+#define DRM_VC4_PARAM_SUPPORTS_PERFMON		8
 
 struct drm_vc4_get_param {
 	__u32 param;
@@ -352,6 +368,66 @@ struct drm_vc4_gem_madvise {
 	__u32 pad;
 };
 
+enum {
+	VC4_PERFCNT_FEP_VALID_PRIMS_NO_RENDER,
+	VC4_PERFCNT_FEP_VALID_PRIMS_RENDER,
+	VC4_PERFCNT_FEP_CLIPPED_QUADS,
+	VC4_PERFCNT_FEP_VALID_QUADS,
+	VC4_PERFCNT_TLB_QUADS_NOT_PASSING_STENCIL,
+	VC4_PERFCNT_TLB_QUADS_NOT_PASSING_Z_AND_STENCIL,
+	VC4_PERFCNT_TLB_QUADS_PASSING_Z_AND_STENCIL,
+	VC4_PERFCNT_TLB_QUADS_ZERO_COVERAGE,
+	VC4_PERFCNT_TLB_QUADS_NON_ZERO_COVERAGE,
+	VC4_PERFCNT_TLB_QUADS_WRITTEN_TO_COLOR_BUF,
+	VC4_PERFCNT_PLB_PRIMS_OUTSIDE_VIEWPORT,
+	VC4_PERFCNT_PLB_PRIMS_NEED_CLIPPING,
+	VC4_PERFCNT_PSE_PRIMS_REVERSED,
+	VC4_PERFCNT_QPU_TOTAL_IDLE_CYCLES,
+	VC4_PERFCNT_QPU_TOTAL_CLK_CYCLES_VERTEX_COORD_SHADING,
+	VC4_PERFCNT_QPU_TOTAL_CLK_CYCLES_FRAGMENT_SHADING,
+	VC4_PERFCNT_QPU_TOTAL_CLK_CYCLES_EXEC_VALID_INST,
+	VC4_PERFCNT_QPU_TOTAL_CLK_CYCLES_WAITING_TMUS,
+	VC4_PERFCNT_QPU_TOTAL_CLK_CYCLES_WAITING_SCOREBOARD,
+	VC4_PERFCNT_QPU_TOTAL_CLK_CYCLES_WAITING_VARYINGS,
+	VC4_PERFCNT_QPU_TOTAL_INST_CACHE_HIT,
+	VC4_PERFCNT_QPU_TOTAL_INST_CACHE_MISS,
+	VC4_PERFCNT_QPU_TOTAL_UNIFORM_CACHE_HIT,
+	VC4_PERFCNT_QPU_TOTAL_UNIFORM_CACHE_MISS,
+	VC4_PERFCNT_TMU_TOTAL_TEXT_QUADS_PROCESSED,
+	VC4_PERFCNT_TMU_TOTAL_TEXT_CACHE_MISS,
+	VC4_PERFCNT_VPM_TOTAL_CLK_CYCLES_VDW_STALLED,
+	VC4_PERFCNT_VPM_TOTAL_CLK_CYCLES_VCD_STALLED,
+	VC4_PERFCNT_L2C_TOTAL_L2_CACHE_HIT,
+	VC4_PERFCNT_L2C_TOTAL_L2_CACHE_MISS,
+	VC4_PERFCNT_NUM_EVENTS,
+};
+
+#define DRM_VC4_MAX_PERF_COUNTERS	16
+
+struct drm_vc4_perfmon_create {
+	__u32 id;
+	__u32 ncounters;
+	__u8 events[DRM_VC4_MAX_PERF_COUNTERS];
+};
+
+struct drm_vc4_perfmon_destroy {
+	__u32 id;
+};
+
+/*
+ * Returns the values of the performance counters tracked by this
+ * perfmon (as an array of ncounters u64 values).
+ *
+ * No implicit synchronization is performed, so the user has to
+ * guarantee that any jobs using this perfmon have already been
+ * completed  (probably by blocking on the seqno returned by the
+ * last exec that used the perfmon).
+ */
+struct drm_vc4_perfmon_get_values {
+	__u32 id;
+	__u64 values_ptr;
+};
+
 #if defined(__cplusplus)
 }
 #endif
diff --git a/include/drm-uapi/virtgpu_drm.h b/include/drm-uapi/virtgpu_drm.h
index 91a31ffe..9a781f06 100644
--- a/include/drm-uapi/virtgpu_drm.h
+++ b/include/drm-uapi/virtgpu_drm.h
@@ -63,6 +63,7 @@ struct drm_virtgpu_execbuffer {
 };
 
 #define VIRTGPU_PARAM_3D_FEATURES 1 /* do we have 3D features in the hw */
+#define VIRTGPU_PARAM_CAPSET_QUERY_FIX 2 /* do we have the capset fix */
 
 struct drm_virtgpu_getparam {
 	__u64 param;
diff --git a/lib/igt_perf.h b/lib/igt_perf.h
index 7b66fc58..105b8cd9 100644
--- a/lib/igt_perf.h
+++ b/lib/igt_perf.h
@@ -31,13 +31,6 @@
 
 #include "igt_gt.h"
 
-enum drm_i915_pmu_engine_sample {
-	I915_SAMPLE_BUSY = 0,
-	I915_SAMPLE_WAIT = 1,
-	I915_SAMPLE_SEMA = 2,
-	I915_ENGINE_SAMPLE_MAX /* non-ABI */
-};
-
 #define I915_PMU_SAMPLE_BITS (4)
 #define I915_PMU_SAMPLE_MASK (0xf)
 #define I915_PMU_SAMPLE_INSTANCE_BITS (8)
-- 
2.16.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] [PATCH i-g-t v8 2/3] intel_chipsets: store GT information in device info
  2018-03-09  9:31 [igt-dev] [PATCH i-g-t v8 1/3] include: bump drm uAPI headers Lionel Landwerlin
@ 2018-03-09  9:31 ` Lionel Landwerlin
  2018-03-09 10:02   ` Chris Wilson
  2018-03-09  9:31 ` [igt-dev] [PATCH i-g-t v8 3/3] tests: add i915 query tests Lionel Landwerlin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Lionel Landwerlin @ 2018-03-09  9:31 UTC (permalink / raw)
  To: igt-dev

Right now we define this only for big core skus and leave the gt field
to 0 to mean unknown.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 lib/intel_chipset.h     |   1 +
 lib/intel_device_info.c | 142 +++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 122 insertions(+), 21 deletions(-)

diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
index 7fc9b3bf..9250f4f9 100644
--- a/lib/intel_chipset.h
+++ b/lib/intel_chipset.h
@@ -36,6 +36,7 @@ uint32_t intel_get_drm_devid(int fd);
 
 struct intel_device_info {
 	unsigned gen;
+	unsigned gt; /* 0 if unknown */
 	bool is_mobile : 1;
 	bool is_whitney : 1;
 	bool is_almador : 1;
diff --git a/lib/intel_device_info.c b/lib/intel_device_info.c
index 8ea19f21..1c710733 100644
--- a/lib/intel_device_info.c
+++ b/lib/intel_device_info.c
@@ -145,16 +145,48 @@ static const struct intel_device_info intel_valleyview_info = {
 	.codename = "valleyview"
 };
 
-static const struct intel_device_info intel_haswell_info = {
-	.gen = BIT(6),
-	.is_haswell = true,
+#define HASWELL_FIELDS \
+	.gen = BIT(6), \
+	.is_haswell = true, \
 	.codename = "haswell"
+
+static const struct intel_device_info intel_haswell_gt1_info = {
+	HASWELL_FIELDS,
+	.gt = 1,
 };
 
-static const struct intel_device_info intel_broadwell_info = {
-	.gen = BIT(7),
-	.is_broadwell = true,
+static const struct intel_device_info intel_haswell_gt2_info = {
+	HASWELL_FIELDS,
+	.gt = 2,
+};
+
+static const struct intel_device_info intel_haswell_gt3_info = {
+	HASWELL_FIELDS,
+	.gt = 3,
+};
+
+#define BROADWELL_FIELDS \
+	.gen = BIT(7), \
+	.is_broadwell = true, \
 	.codename = "broadwell"
+
+static const struct intel_device_info intel_broadwell_gt1_info = {
+	BROADWELL_FIELDS,
+	.gt = 1,
+};
+
+static const struct intel_device_info intel_broadwell_gt2_info = {
+	BROADWELL_FIELDS,
+	.gt = 2,
+};
+
+static const struct intel_device_info intel_broadwell_gt3_info = {
+	BROADWELL_FIELDS,
+	.gt = 3,
+};
+
+static const struct intel_device_info intel_broadwell_unknown_info = {
+	BROADWELL_FIELDS,
 };
 
 static const struct intel_device_info intel_cherryview_info = {
@@ -163,10 +195,29 @@ static const struct intel_device_info intel_cherryview_info = {
 	.codename = "cherryview"
 };
 
-static const struct intel_device_info intel_skylake_info = {
-	.gen = BIT(8),
-	.is_skylake = true,
-	.codename = "skylake"
+#define SKYLAKE_FIELDS \
+	.gen = BIT(8), \
+	.codename = "skylake", \
+	.is_skylake = true
+
+static const struct intel_device_info intel_skylake_gt1_info = {
+	SKYLAKE_FIELDS,
+	.gt = 1,
+};
+
+static const struct intel_device_info intel_skylake_gt2_info = {
+	SKYLAKE_FIELDS,
+	.gt = 2,
+};
+
+static const struct intel_device_info intel_skylake_gt3_info = {
+	SKYLAKE_FIELDS,
+	.gt = 3,
+};
+
+static const struct intel_device_info intel_skylake_gt4_info = {
+	SKYLAKE_FIELDS,
+	.gt = 4,
 };
 
 static const struct intel_device_info intel_broxton_info = {
@@ -175,10 +226,29 @@ static const struct intel_device_info intel_broxton_info = {
 	.codename = "broxton"
 };
 
-static const struct intel_device_info intel_kabylake_info = {
-	.gen = BIT(8),
-	.is_kabylake = true,
+#define KABYLAKE_FIELDS \
+	.gen = BIT(8), \
+	.is_kabylake = true, \
 	.codename = "kabylake"
+
+static const struct intel_device_info intel_kabylake_gt1_info = {
+	KABYLAKE_FIELDS,
+	.gt = 1,
+};
+
+static const struct intel_device_info intel_kabylake_gt2_info = {
+	KABYLAKE_FIELDS,
+	.gt = 2,
+};
+
+static const struct intel_device_info intel_kabylake_gt3_info = {
+	KABYLAKE_FIELDS,
+	.gt = 3,
+};
+
+static const struct intel_device_info intel_kabylake_gt4_info = {
+	KABYLAKE_FIELDS,
+	.gt = 4,
 };
 
 static const struct intel_device_info intel_geminilake_info = {
@@ -187,10 +257,24 @@ static const struct intel_device_info intel_geminilake_info = {
 	.codename = "geminilake"
 };
 
-static const struct intel_device_info intel_coffeelake_info = {
-	.gen = BIT(8),
-	.is_coffeelake = true,
+#define COFFEELAKE_FIELDS \
+	.gen = BIT(8), \
+	.is_coffeelake = true, \
 	.codename = "coffeelake"
+
+static const struct intel_device_info intel_coffeelake_gt1_info = {
+	COFFEELAKE_FIELDS,
+	.gt = 1,
+};
+
+static const struct intel_device_info intel_coffeelake_gt2_info = {
+	COFFEELAKE_FIELDS,
+	.gt = 2,
+};
+
+static const struct intel_device_info intel_coffeelake_gt3_info = {
+	COFFEELAKE_FIELDS,
+	.gt = 3,
 };
 
 static const struct intel_device_info intel_cannonlake_info = {
@@ -231,23 +315,39 @@ static const struct pci_id_match intel_device_match[] = {
 	INTEL_IVB_D_IDS(&intel_ivybridge_info),
 	INTEL_IVB_M_IDS(&intel_ivybridge_m_info),
 
-	INTEL_HSW_IDS(&intel_haswell_info),
+	INTEL_HSW_GT1_IDS(&intel_haswell_gt1_info),
+	INTEL_HSW_GT2_IDS(&intel_haswell_gt2_info),
+	INTEL_HSW_GT3_IDS(&intel_haswell_gt3_info),
 
 	INTEL_VLV_IDS(&intel_valleyview_info),
 
-	INTEL_BDW_IDS(&intel_broadwell_info),
+	INTEL_BDW_GT1_IDS(&intel_broadwell_gt1_info),
+	INTEL_BDW_GT2_IDS(&intel_broadwell_gt2_info),
+	INTEL_BDW_GT3_IDS(&intel_broadwell_gt3_info),
+	INTEL_BDW_RSVD_IDS(&intel_broadwell_unknown_info),
 
 	INTEL_CHV_IDS(&intel_cherryview_info),
 
-	INTEL_SKL_IDS(&intel_skylake_info),
+	INTEL_SKL_GT1_IDS(&intel_skylake_gt1_info),
+	INTEL_SKL_GT2_IDS(&intel_skylake_gt2_info),
+	INTEL_SKL_GT3_IDS(&intel_skylake_gt3_info),
+	INTEL_SKL_GT4_IDS(&intel_skylake_gt4_info),
 
 	INTEL_BXT_IDS(&intel_broxton_info),
 
-	INTEL_KBL_IDS(&intel_kabylake_info),
+	INTEL_KBL_GT1_IDS(&intel_kabylake_gt1_info),
+	INTEL_KBL_GT2_IDS(&intel_kabylake_gt2_info),
+	INTEL_KBL_GT3_IDS(&intel_kabylake_gt3_info),
+	INTEL_KBL_GT4_IDS(&intel_kabylake_gt4_info),
 
 	INTEL_GLK_IDS(&intel_geminilake_info),
 
-	INTEL_CFL_IDS(&intel_coffeelake_info),
+	INTEL_CFL_S_GT1_IDS(&intel_coffeelake_gt1_info),
+	INTEL_CFL_U_GT1_IDS(&intel_coffeelake_gt1_info),
+	INTEL_CFL_S_GT2_IDS(&intel_coffeelake_gt2_info),
+	INTEL_CFL_H_GT2_IDS(&intel_coffeelake_gt2_info),
+	INTEL_CFL_U_GT2_IDS(&intel_coffeelake_gt2_info),
+	INTEL_CFL_U_GT3_IDS(&intel_coffeelake_gt3_info),
 
 	INTEL_CNL_IDS(&intel_cannonlake_info),
 
-- 
2.16.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] [PATCH i-g-t v8 3/3] tests: add i915 query tests
  2018-03-09  9:31 [igt-dev] [PATCH i-g-t v8 1/3] include: bump drm uAPI headers Lionel Landwerlin
  2018-03-09  9:31 ` [igt-dev] [PATCH i-g-t v8 2/3] intel_chipsets: store GT information in device info Lionel Landwerlin
@ 2018-03-09  9:31 ` Lionel Landwerlin
  2018-03-09 13:23   ` Tvrtko Ursulin
  2018-03-09  9:51 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v8,1/3] include: bump drm uAPI headers Patchwork
  2018-03-09 11:05 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
  3 siblings, 1 reply; 10+ messages in thread
From: Lionel Landwerlin @ 2018-03-09  9:31 UTC (permalink / raw)
  To: igt-dev

v2: Complete invalid cases (Chris)
    Some styling (to_user_pointer, etc...) (Chris)
    New error check, through item.length (Chris)

v3: Update for new uAPI iteration (Lionel)

v4: Return errno from a single point (Chris)
    Poising checks (Chris)

v5: Add more debug traces (Lionel)
    Update uAPI (Joonas/Lionel)
    Make sure Haswell is tested (Lionel)

v6: s/query_item/query_items/ (Tvrtko)
    test that flags fields != 0 fail (Tvrtko)
    Split kernel writes checks out (Tvrtko)
    Verify that when an EU is available, so is slice & subslice it
    belongs to (same with subslice). (Tvrtko)
    Verify kernel errors out with read only memory (Tvrtko)

v7: Add a special Haswell test to verify correct values (Tvrtko)
    Simplify igt_require() in front of tests (Tvrtko)

v8: Reuse the GT field from device info to verify slice/subslices
    numbers on wider number of platforms (Lionel)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 tests/Makefile.sources |   1 +
 tests/i915_query.c     | 476 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/meson.build      |   1 +
 3 files changed, 478 insertions(+)
 create mode 100644 tests/i915_query.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 4a81ac4a..4e6f5319 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -167,6 +167,7 @@ TESTS_progs = \
 	gen3_render_tiledx_blits \
 	gen3_render_tiledy_blits \
 	gvt_basic \
+	i915_query \
 	kms_3d \
 	kms_addfb_basic \
 	kms_atomic \
diff --git a/tests/i915_query.c b/tests/i915_query.c
new file mode 100644
index 00000000..14ad2146
--- /dev/null
+++ b/tests/i915_query.c
@@ -0,0 +1,476 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt.h"
+
+#include <limits.h>
+
+IGT_TEST_DESCRIPTION("Testing the i915 query uAPI.");
+
+/* We should at least get 3 bytes for data for each slices, subslices & EUs
+ * masks.
+ */
+#define MIN_TOPOLOGY_ITEM_SIZE (sizeof(struct drm_i915_query_topology_info) + 3)
+
+static int
+__i915_query(int fd, struct drm_i915_query *q)
+{
+	if (igt_ioctl(fd, DRM_IOCTL_I915_QUERY, q))
+		return -errno;
+	return 0;
+}
+
+static int
+__i915_query_items(int fd, struct drm_i915_query_item *items, uint32_t n_items)
+{
+	struct drm_i915_query q = {
+		.num_items = n_items,
+		.items_ptr = to_user_pointer(items),
+	};
+	return __i915_query(fd, &q);
+}
+
+#define i915_query_items(fd, items, n_items) do { \
+		igt_assert_eq(__i915_query_items(fd, items, n_items), 0); \
+		errno = 0; \
+	} while (0)
+#define i915_query_items_err(fd, items, n_items, err) do { \
+		igt_assert_eq(__i915_query_items(fd, items, n_items), -err); \
+	} while (0)
+
+static bool has_query_supports(int fd)
+{
+	struct drm_i915_query query = {};
+
+	return __i915_query(fd, &query) == 0;
+}
+
+static void test_query_garbage(int fd)
+{
+	struct drm_i915_query query;
+	struct drm_i915_query_item items[2];
+	struct drm_i915_query_item *items_ptr;
+	int i, n_items;
+
+	/* Query flags field is currently valid only if equals to 0. This might
+	 * change in the future.
+	 */
+	memset(&query, 0, sizeof(query));
+	query.flags = 42;
+	igt_assert_eq(__i915_query(fd, &query), -EINVAL);
+
+	/* Test a couple of invalid pointers. */
+	i915_query_items_err(fd, (void *) ULONG_MAX, 1, EFAULT);
+	i915_query_items_err(fd, (void *) 0, 1, EFAULT);
+
+	/* Test the invalid query id = 0. */
+	memset(items, 0, sizeof(items));
+	i915_query_items_err(fd, items, 1, EINVAL);
+
+	/* Query item flags field is currently valid only if equals to 0.
+	 * Subject to change in the future.
+	 */
+	memset(items, 0, sizeof(items));
+	items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	items[0].flags = 42;
+	i915_query_items(fd, items, 1);
+	igt_assert_eq(items[0].length, -EINVAL);
+
+	/* Test an invalid query id in the second item and verify that the first
+	 * one is properly processed.
+	 */
+	memset(items, 0, sizeof(items));
+	items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	items[1].query_id = ULONG_MAX;
+	i915_query_items(fd, items, 2);
+	igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, items[0].length);
+	igt_assert_eq(items[1].length, -EINVAL);
+
+	/* Test a invalid query id in the first item and verify that the second
+	 * one is properly processed (the driver is expected to go through them
+	 * all and place error codes in the failed items).
+	 */
+	memset(items, 0, sizeof(items));
+	items[0].query_id = ULONG_MAX;
+	items[1].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	i915_query_items(fd, items, 2);
+	igt_assert_eq(items[0].length, -EINVAL);
+	igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, items[1].length);
+
+	/* Test an invalid query item length. */
+	memset(items, 0, sizeof(items));
+	items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	items[1].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	items[1].length = sizeof(struct drm_i915_query_topology_info) - 1;
+	i915_query_items(fd, items, 2);
+	igt_assert_lte(0, items[0].length);
+	igt_assert_eq(items[1].length, -EINVAL);
+
+	/* Map memory for a query item in which the kernel is going to write the
+	 * length of the item in the first ioctl(). Then unmap that memory and
+	 * verify that the kernel correctly returns EFAULT as memory of the item
+	 * has been removed from our address space.
+	 */
+	items_ptr = mmap(0, 4096, PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
+	items_ptr[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	i915_query_items(fd, items_ptr, 1);
+	igt_assert(items_ptr[0].length >= sizeof(struct drm_i915_query_topology_info));
+	munmap(items_ptr, 4096);
+	i915_query_items_err(fd, items_ptr, 1, EFAULT);
+
+	/* Map memory for a query item, then make it read only and verify that
+	 * the kernel errors out with EFAULT.
+	 */
+	items_ptr = mmap(0, 4096, PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
+	items_ptr[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	igt_assert_eq(0, mprotect(items_ptr, 4096, PROT_READ));
+	i915_query_items_err(fd, items_ptr, 1, EFAULT);
+	munmap(items_ptr, 4096);
+
+	/* Allocate 2 pages, prepare those 2 pages with valid query items, then
+	 * switch the second page to read only and expect an EFAULT error.
+	 */
+	items_ptr = mmap(0, 8192, PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
+	memset(items_ptr, 0, 8192);
+	n_items = 8192 / sizeof(struct drm_i915_query_item);
+	for (i = 0; i < n_items; i++)
+		items_ptr[i].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	mprotect(((uint8_t *)items_ptr) + 4096, 4096, PROT_READ);
+	i915_query_items_err(fd, items_ptr, n_items, EFAULT);
+	munmap(items_ptr, 8192);
+}
+
+/* Allocate more on both sides of where the kernel is going to write and verify
+ * that it writes only where it's supposed to.
+ */
+static void test_query_topology_kernel_writes(int fd)
+{
+	struct drm_i915_query_item item;
+	struct drm_i915_query_topology_info *topo_info;
+	uint8_t *_topo_info;
+	int b, total_size;
+
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	i915_query_items(fd, &item, 1);
+	igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
+
+	total_size = item.length + 2 * sizeof(*_topo_info);
+	_topo_info = malloc(total_size);
+	memset(_topo_info, 0xff, total_size);
+	topo_info = (struct drm_i915_query_topology_info *) (_topo_info + sizeof(*_topo_info));
+	memset(topo_info, 0, item.length);
+
+	item.data_ptr = to_user_pointer(topo_info);
+	i915_query_items(fd, &item, 1);
+
+	for (b = 0; b < sizeof(*_topo_info); b++) {
+		igt_assert_eq(_topo_info[b], 0xff);
+		igt_assert_eq(_topo_info[sizeof(*_topo_info) + item.length + b], 0xff);
+	}
+}
+
+static bool query_topology_supported(int fd)
+{
+	struct drm_i915_query_item item = {
+		.query_id = DRM_I915_QUERY_TOPOLOGY_INFO,
+	};
+
+	return __i915_query_items(fd, &item, 1) == 0 && item.length > 0;
+}
+
+static void test_query_topology_unsupported(int fd)
+{
+	struct drm_i915_query_item item = {
+		.query_id = DRM_I915_QUERY_TOPOLOGY_INFO,
+	};
+
+	i915_query_items(fd, &item, 1);
+	igt_assert_eq(item.length, -ENODEV);
+}
+
+#define DIV_ROUND_UP(val, div) (ALIGN(val, div) / div)
+
+static bool
+slice_available(const struct drm_i915_query_topology_info *topo_info,
+		int s)
+{
+	return (topo_info->data[s / 8] >> (s % 8)) & 1;
+}
+
+static bool
+subslice_available(const struct drm_i915_query_topology_info *topo_info,
+		   int s, int ss)
+{
+	return (topo_info->data[topo_info->subslice_offset +
+				s * topo_info->subslice_stride +
+				ss / 8] >> (ss % 8)) & 1;
+}
+
+static bool
+eu_available(const struct drm_i915_query_topology_info *topo_info,
+	     int s, int ss, int eu)
+{
+	return (topo_info->data[topo_info->eu_offset +
+				(s * topo_info->max_subslices + ss) * topo_info->eu_stride +
+				eu / 8] >> (eu % 8)) & 1;
+}
+
+/* Verify that we get coherent values between the legacy getparam slice/subslice
+ * masks and the new topology query.
+ */
+static void
+test_query_topology_coherent_slice_mask(int fd)
+{
+	struct drm_i915_query_item item;
+	struct drm_i915_query_topology_info *topo_info;
+	drm_i915_getparam_t gp;
+	int slice_mask, subslice_mask;
+	int s, topology_slices, topology_subslices_slice0;
+
+	gp.param = I915_PARAM_SLICE_MASK;
+	gp.value = &slice_mask;
+	igt_skip_on(igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0);
+
+	gp.param = I915_PARAM_SUBSLICE_MASK;
+	gp.value = &subslice_mask;
+	igt_skip_on(igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0);
+
+	/* Slices */
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	i915_query_items(fd, &item, 1);
+	/* We expect at least one byte for each slices, subslices & EUs masks. */
+	igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
+
+	topo_info = malloc(item.length);
+	memset(topo_info, 0, item.length);
+
+	item.data_ptr = to_user_pointer(topo_info);
+	i915_query_items(fd, &item, 1);
+	/* We expect at least one byte for each slices, subslices & EUs masks. */
+	igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
+
+	topology_slices = 0;
+	for (s = 0; s < topo_info->max_slices; s++) {
+		if (slice_available(topo_info, s))
+			topology_slices |= 1UL << s;
+	}
+
+	igt_debug("slice mask getparam=0x%x / query=0x%x\n",
+		  slice_mask, topology_slices);
+
+	/* These 2 should always match. */
+	igt_assert_eq(slice_mask, topology_slices);
+
+	topology_subslices_slice0 = 0;
+	for (s = 0; s < topo_info->max_subslices; s++) {
+		if (subslice_available(topo_info, 0, s))
+			topology_subslices_slice0 |= 1UL << s;
+	}
+
+	igt_debug("subslice mask getparam=0x%x / query=0x%x\n",
+		  subslice_mask, topology_subslices_slice0);
+
+	/* I915_PARAM_SUBSLICE_MASK returns the value for slice0, we should
+	 * match the values for the first slice of the topology.
+	 */
+	igt_assert_eq(subslice_mask, topology_subslices_slice0);
+
+	free(topo_info);
+}
+
+/* Verify that we get same total number of EUs from getparam and topology query.
+ */
+static void
+test_query_topology_matches_eu_total(int fd)
+{
+	struct drm_i915_query_item item;
+	struct drm_i915_query_topology_info *topo_info;
+	drm_i915_getparam_t gp;
+	int n_eus, n_eus_topology, s;
+
+	gp.param = I915_PARAM_EU_TOTAL;
+	gp.value = &n_eus;
+	do_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+	igt_debug("n_eus=%i\n", n_eus);
+
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	i915_query_items(fd, &item, 1);
+
+	topo_info = (struct drm_i915_query_topology_info *) calloc(1, item.length);
+
+	item.data_ptr = to_user_pointer(topo_info);
+	i915_query_items(fd, &item, 1);
+
+	igt_debug("max_slices=%hu max_subslices=%hu max_eus_per_subslice=%hu\n",
+		  topo_info->max_slices, topo_info->max_subslices,
+		  topo_info->max_eus_per_subslice);
+	igt_debug(" subslice_offset=%hu subslice_stride=%hu\n",
+		  topo_info->subslice_offset, topo_info->subslice_stride);
+	igt_debug(" eu_offset=%hu eu_stride=%hu\n",
+		  topo_info->eu_offset, topo_info->eu_stride);
+
+	n_eus_topology = 0;
+	for (s = 0; s < topo_info->max_slices; s++) {
+		int ss;
+
+		igt_debug("slice%i:\n", s);
+
+		for (ss = 0; ss < topo_info->max_subslices; ss++) {
+			int eu, n_subslice_eus = 0;
+
+			igt_debug("\tsubslice: %i\n", ss);
+
+			igt_debug("\t\teu_mask: 0b");
+			for (eu = 0; eu < topo_info->max_eus_per_subslice; eu++) {
+				uint8_t val = eu_available(topo_info, s, ss,
+							   topo_info->max_eus_per_subslice - 1 - eu);
+				igt_debug("%hhi", val);
+				n_subslice_eus += __builtin_popcount(val);
+				n_eus_topology += __builtin_popcount(val);
+			}
+
+			igt_debug(" (%i)\n", n_subslice_eus);
+
+			/* Sanity checks. */
+			if (n_subslice_eus > 0) {
+				igt_assert(slice_available(topo_info, s));
+				igt_assert(subslice_available(topo_info, s, ss));
+			}
+			if (subslice_available(topo_info, s, ss)) {
+				igt_assert(slice_available(topo_info, s));
+			}
+		}
+	}
+
+	free(topo_info);
+
+	igt_assert(n_eus_topology == n_eus);
+}
+
+/* Verify some numbers on Gens that we know for sure the characteristics from
+ * the PCI ids.
+ */
+static void
+test_query_topology_known_pci_ids(int fd, int devid)
+{
+	const struct intel_device_info *dev_info = intel_get_device_info(devid);
+	struct drm_i915_query_item item;
+	struct drm_i915_query_topology_info *topo_info;
+	int n_slices = 0, n_subslices = 0;
+	int s, ss;
+
+	/* The GT size on some Broadwell skus is not defined, skip those. */
+	igt_skip_on(dev_info->gt == 0);
+
+	memset(&item, 0, sizeof(item));
+	item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
+	i915_query_items(fd, &item, 1);
+
+	topo_info = (struct drm_i915_query_topology_info *) calloc(1, item.length);
+
+	item.data_ptr = to_user_pointer(topo_info);
+	i915_query_items(fd, &item, 1);
+
+	for (s = 0; s < topo_info->max_slices; s++) {
+		if (slice_available(topo_info, s))
+			n_slices++;
+
+		for (ss = 0; ss < topo_info->max_subslices; ss++) {
+			if (subslice_available(topo_info, s, ss))
+				n_subslices++;
+		}
+	}
+
+	switch (dev_info->gt) {
+	case 1:
+		igt_assert_eq(n_slices, 1);
+		igt_assert(n_subslices == 2 || n_subslices == 3);
+		break;
+	case 2:
+		igt_assert_eq(n_slices, 1);
+		igt_assert_eq(n_subslices, 3);
+		break;
+	case 3:
+		igt_assert_eq(n_slices, 2);
+		igt_assert_eq(n_subslices, 6);
+		break;
+	case 4:
+		igt_assert_eq(n_slices, 3);
+		igt_assert_eq(n_subslices, 6);
+		break;
+	default:
+		igt_assert(false);
+	}
+
+	free(topo_info);
+}
+
+igt_main
+{
+	int fd = -1;
+	int devid;
+
+	igt_fixture {
+		fd = drm_open_driver(DRIVER_INTEL);
+		igt_require(has_query_supports(fd));
+		devid = intel_get_drm_devid(fd);
+	}
+
+	igt_subtest("query-garbage")
+		test_query_garbage(fd);
+
+	igt_subtest("query-topology-kernel-writes") {
+		igt_require(query_topology_supported(fd));
+		test_query_topology_kernel_writes(fd);
+	}
+
+	igt_subtest("query-topology-unsupported") {
+		igt_require(!query_topology_supported(fd));
+		test_query_topology_unsupported(fd);
+	}
+
+	igt_subtest("query-topology-coherent-slice-mask") {
+		igt_require(query_topology_supported(fd));
+		test_query_topology_coherent_slice_mask(fd);
+	}
+
+	igt_subtest("query-topology-matches-eu-total") {
+		igt_require(query_topology_supported(fd));
+		test_query_topology_matches_eu_total(fd);
+	}
+
+	igt_subtest("query-topology-haswell") {
+		igt_require(query_topology_supported(fd));
+		igt_require(IS_HASWELL(devid) || IS_BROADWELL(devid) ||
+			    IS_SKYLAKE(devid) || IS_KABYLAKE(devid) ||
+			    IS_COFFEELAKE(devid));
+		test_query_topology_known_pci_ids(fd, devid);
+	}
+
+	igt_fixture {
+		close(fd);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 58729231..5788dfd0 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -143,6 +143,7 @@ test_progs = [
 	'gen3_render_tiledx_blits',
 	'gen3_render_tiledy_blits',
 	'gvt_basic',
+	'i915_query',
 	'kms_3d',
 	'kms_addfb_basic',
 	'kms_atomic',
-- 
2.16.2

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v8,1/3] include: bump drm uAPI headers
  2018-03-09  9:31 [igt-dev] [PATCH i-g-t v8 1/3] include: bump drm uAPI headers Lionel Landwerlin
  2018-03-09  9:31 ` [igt-dev] [PATCH i-g-t v8 2/3] intel_chipsets: store GT information in device info Lionel Landwerlin
  2018-03-09  9:31 ` [igt-dev] [PATCH i-g-t v8 3/3] tests: add i915 query tests Lionel Landwerlin
@ 2018-03-09  9:51 ` Patchwork
  2018-03-09 11:05 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-03-09  9:51 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v8,1/3] include: bump drm uAPI headers
URL   : https://patchwork.freedesktop.org/series/39673/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
b4689dce36d0fbd9aec70d5a4b077c43a6b9c254 igt: Remove gen7_forcewake_mt

with latest DRM-Tip kernel build CI_DRM_3902
469c28df8d66 drm-tip: 2018y-03m-08d-22h-40m-12s UTC integration manifest

Testlist changes:
+igt@i915_query@query-garbage
+igt@i915_query@query-topology-coherent-slice-mask
+igt@i915_query@query-topology-haswell
+igt@i915_query@query-topology-kernel-writes
+igt@i915_query@query-topology-matches-eu-total
+igt@i915_query@query-topology-unsupported

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> FAIL       (fi-cnl-y3) fdo#103167

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:424s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:431s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:373s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:509s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:495s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:498s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:484s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:473s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:407s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:582s
fi-cnl-y3        total:288  pass:261  dwarn:0   dfail:0   fail:1   skip:26  time:576s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:409s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:291s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:521s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:397s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:417s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:460s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:425s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:471s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:460s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:512s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:587s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:436s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:520s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:533s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:503s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:485s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:424s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:524s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:404s
Blacklisted hosts:
fi-cnl-drrs      total:288  pass:257  dwarn:3   dfail:0   fail:0   skip:19  time:518s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1089/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v8 2/3] intel_chipsets: store GT information in device info
  2018-03-09  9:31 ` [igt-dev] [PATCH i-g-t v8 2/3] intel_chipsets: store GT information in device info Lionel Landwerlin
@ 2018-03-09 10:02   ` Chris Wilson
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2018-03-09 10:02 UTC (permalink / raw)
  To: Lionel Landwerlin, igt-dev

Quoting Lionel Landwerlin (2018-03-09 09:31:43)
> Right now we define this only for big core skus and leave the gt field
> to 0 to mean unknown.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,v8,1/3] include: bump drm uAPI headers
  2018-03-09  9:31 [igt-dev] [PATCH i-g-t v8 1/3] include: bump drm uAPI headers Lionel Landwerlin
                   ` (2 preceding siblings ...)
  2018-03-09  9:51 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v8,1/3] include: bump drm uAPI headers Patchwork
@ 2018-03-09 11:05 ` Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-03-09 11:05 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v8,1/3] include: bump drm uAPI headers
URL   : https://patchwork.freedesktop.org/series/39673/
State : failure

== Summary ==

---- Possible new issues:

Test kms_color:
        Subgroup pipe-a-ctm-max:
                pass       -> FAIL       (shard-apl)
Test kms_cursor_legacy:
        Subgroup cursor-vs-flip-toggle:
                pass       -> SKIP       (shard-snb)
        Subgroup cursora-vs-flipa-atomic-transitions-varying-size:
                pass       -> SKIP       (shard-snb)
        Subgroup long-nonblocking-modeset-vs-cursor-atomic:
                pass       -> SKIP       (shard-snb)
Test pm_rc6_residency:
        Subgroup rc6-accuracy:
                skip       -> PASS       (shard-snb)

---- Known issues:

Test gem_eio:
        Subgroup in-flight-contexts:
                pass       -> INCOMPLETE (shard-apl) fdo#105341
Test gem_softpin:
        Subgroup noreloc-s3:
                skip       -> PASS       (shard-snb) fdo#103375
Test kms_atomic_transition:
        Subgroup 1x-modeset-transitions-fencing:
                pass       -> FAIL       (shard-apl) fdo#103207
Test kms_frontbuffer_tracking:
        Subgroup fbc-suspend:
                fail       -> PASS       (shard-apl) fdo#101623
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-a-planes:
                pass       -> INCOMPLETE (shard-hsw) fdo#103540
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047
Test pm_lpsp:
        Subgroup screens-disabled:
                pass       -> FAIL       (shard-hsw) fdo#104941
Test pm_rps:
        Subgroup waitboost:
                pass       -> FAIL       (shard-apl) fdo#102250

fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#103207 https://bugs.freedesktop.org/show_bug.cgi?id=103207
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#104941 https://bugs.freedesktop.org/show_bug.cgi?id=104941
fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250

shard-apl        total:3460 pass:1821 dwarn:1   dfail:0   fail:11  skip:1625 time:11663s
shard-hsw        total:3436 pass:1759 dwarn:1   dfail:0   fail:2   skip:1672 time:11097s
shard-snb        total:3473 pass:1363 dwarn:1   dfail:0   fail:3   skip:2106 time:6882s
Blacklisted hosts:
shard-kbl        total:3473 pass:1942 dwarn:15  dfail:0   fail:9   skip:1507 time:9458s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1089/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v8 3/3] tests: add i915 query tests
  2018-03-09  9:31 ` [igt-dev] [PATCH i-g-t v8 3/3] tests: add i915 query tests Lionel Landwerlin
@ 2018-03-09 13:23   ` Tvrtko Ursulin
  2018-03-09 14:07     ` Lionel Landwerlin
  0 siblings, 1 reply; 10+ messages in thread
From: Tvrtko Ursulin @ 2018-03-09 13:23 UTC (permalink / raw)
  To: Lionel Landwerlin, igt-dev


You did not like my suggestion to add a commit message? :)

On 09/03/2018 09:31, Lionel Landwerlin wrote:
> v2: Complete invalid cases (Chris)
>      Some styling (to_user_pointer, etc...) (Chris)
>      New error check, through item.length (Chris)
> 
> v3: Update for new uAPI iteration (Lionel)
> 
> v4: Return errno from a single point (Chris)
>      Poising checks (Chris)
> 
> v5: Add more debug traces (Lionel)
>      Update uAPI (Joonas/Lionel)
>      Make sure Haswell is tested (Lionel)
> 
> v6: s/query_item/query_items/ (Tvrtko)
>      test that flags fields != 0 fail (Tvrtko)
>      Split kernel writes checks out (Tvrtko)
>      Verify that when an EU is available, so is slice & subslice it
>      belongs to (same with subslice). (Tvrtko)
>      Verify kernel errors out with read only memory (Tvrtko)
> 
> v7: Add a special Haswell test to verify correct values (Tvrtko)
>      Simplify igt_require() in front of tests (Tvrtko)
> 
> v8: Reuse the GT field from device info to verify slice/subslices
>      numbers on wider number of platforms (Lionel)
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   tests/Makefile.sources |   1 +
>   tests/i915_query.c     | 476 +++++++++++++++++++++++++++++++++++++++++++++++++
>   tests/meson.build      |   1 +
>   3 files changed, 478 insertions(+)
>   create mode 100644 tests/i915_query.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 4a81ac4a..4e6f5319 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -167,6 +167,7 @@ TESTS_progs = \
>   	gen3_render_tiledx_blits \
>   	gen3_render_tiledy_blits \
>   	gvt_basic \
> +	i915_query \
>   	kms_3d \
>   	kms_addfb_basic \
>   	kms_atomic \
> diff --git a/tests/i915_query.c b/tests/i915_query.c
> new file mode 100644
> index 00000000..14ad2146
> --- /dev/null
> +++ b/tests/i915_query.c
> @@ -0,0 +1,476 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "igt.h"
> +
> +#include <limits.h>
> +
> +IGT_TEST_DESCRIPTION("Testing the i915 query uAPI.");
> +
> +/* We should at least get 3 bytes for data for each slices, subslices & EUs
> + * masks.
> + */

We try to avoid this comment style in i915 and IGT.

> +#define MIN_TOPOLOGY_ITEM_SIZE (sizeof(struct drm_i915_query_topology_info) + 3)
> +
> +static int
> +__i915_query(int fd, struct drm_i915_query *q)
> +{
> +	if (igt_ioctl(fd, DRM_IOCTL_I915_QUERY, q))
> +		return -errno;
> +	return 0;
> +}
> +
> +static int
> +__i915_query_items(int fd, struct drm_i915_query_item *items, uint32_t n_items)
> +{
> +	struct drm_i915_query q = {
> +		.num_items = n_items,
> +		.items_ptr = to_user_pointer(items),
> +	};
> +	return __i915_query(fd, &q);
> +}
> +
> +#define i915_query_items(fd, items, n_items) do { \
> +		igt_assert_eq(__i915_query_items(fd, items, n_items), 0); \
> +		errno = 0; \
> +	} while (0)
> +#define i915_query_items_err(fd, items, n_items, err) do { \
> +		igt_assert_eq(__i915_query_items(fd, items, n_items), -err); \
> +	} while (0)
> +
> +static bool has_query_supports(int fd)
> +{
> +	struct drm_i915_query query = {};
> +
> +	return __i915_query(fd, &query) == 0;
> +}
> +
> +static void test_query_garbage(int fd)
> +{
> +	struct drm_i915_query query;
> +	struct drm_i915_query_item items[2];
> +	struct drm_i915_query_item *items_ptr;
> +	int i, n_items;
> +
> +	/* Query flags field is currently valid only if equals to 0. This might
> +	 * change in the future.
> +	 */
> +	memset(&query, 0, sizeof(query));
> +	query.flags = 42;
> +	igt_assert_eq(__i915_query(fd, &query), -EINVAL);
> +
> +	/* Test a couple of invalid pointers. */
> +	i915_query_items_err(fd, (void *) ULONG_MAX, 1, EFAULT);
> +	i915_query_items_err(fd, (void *) 0, 1, EFAULT);

Also need one where struct drm_i915_query itself it invalid or not mapped.

> +
> +	/* Test the invalid query id = 0. */
> +	memset(items, 0, sizeof(items));
> +	i915_query_items_err(fd, items, 1, EINVAL);
> +
> +	/* Query item flags field is currently valid only if equals to 0.
> +	 * Subject to change in the future.
> +	 */
> +	memset(items, 0, sizeof(items));
> +	items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	items[0].flags = 42;
> +	i915_query_items(fd, items, 1);
> +	igt_assert_eq(items[0].length, -EINVAL);
> +
> +	/* Test an invalid query id in the second item and verify that the first
> +	 * one is properly processed.
> +	 */
> +	memset(items, 0, sizeof(items));
> +	items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	items[1].query_id = ULONG_MAX;
> +	i915_query_items(fd, items, 2);
> +	igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, items[0].length);

Someone needs to add igt_assert_gt(e) to save me from reversed 
conditions! :)

> +	igt_assert_eq(items[1].length, -EINVAL);
> +
> +	/* Test a invalid query id in the first item and verify that the second
> +	 * one is properly processed (the driver is expected to go through them
> +	 * all and place error codes in the failed items).
> +	 */
> +	memset(items, 0, sizeof(items));
> +	items[0].query_id = ULONG_MAX;
> +	items[1].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	i915_query_items(fd, items, 2);
> +	igt_assert_eq(items[0].length, -EINVAL);
> +	igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, items[1].length);
> +
> +	/* Test an invalid query item length. */
> +	memset(items, 0, sizeof(items));
> +	items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	items[1].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	items[1].length = sizeof(struct drm_i915_query_topology_info) - 1;
> +	i915_query_items(fd, items, 2);
> +	igt_assert_lte(0, items[0].length);

Length of zero would be a bug - MIN_TOPOLOGY_ITEM_SIZE ?

> +	igt_assert_eq(items[1].length, -EINVAL);
> +
> +	/* Map memory for a query item in which the kernel is going to write the
> +	 * length of the item in the first ioctl(). Then unmap that memory and
> +	 * verify that the kernel correctly returns EFAULT as memory of the item
> +	 * has been removed from our address space.
> +	 */
> +	items_ptr = mmap(0, 4096, PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
> +	items_ptr[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	i915_query_items(fd, items_ptr, 1);
> +	igt_assert(items_ptr[0].length >= sizeof(struct drm_i915_query_topology_info));

MIN_TOPOLOGY_ITEM_SIZE? I welcome the >= though. :)

> +	munmap(items_ptr, 4096);
> +	i915_query_items_err(fd, items_ptr, 1, EFAULT);
> +
> +	/* Map memory for a query item, then make it read only and verify that
> +	 * the kernel errors out with EFAULT.
> +	 */
> +	items_ptr = mmap(0, 4096, PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
> +	items_ptr[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	igt_assert_eq(0, mprotect(items_ptr, 4096, PROT_READ));
> +	i915_query_items_err(fd, items_ptr, 1, EFAULT);
> +	munmap(items_ptr, 4096);
> +
> +	/* Allocate 2 pages, prepare those 2 pages with valid query items, then
> +	 * switch the second page to read only and expect an EFAULT error.
> +	 */
> +	items_ptr = mmap(0, 8192, PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
> +	memset(items_ptr, 0, 8192);
> +	n_items = 8192 / sizeof(struct drm_i915_query_item);
> +	for (i = 0; i < n_items; i++)
> +		items_ptr[i].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	mprotect(((uint8_t *)items_ptr) + 4096, 4096, PROT_READ);
> +	i915_query_items_err(fd, items_ptr, n_items, EFAULT);
> +	munmap(items_ptr, 8192);

Another missing subtest is where query items ptr is valid, but the topo 
data ptr is not. So -EFAULT is returned in item->length.

> +}
> +
> +/* Allocate more on both sides of where the kernel is going to write and verify
> + * that it writes only where it's supposed to.
> + */
> +static void test_query_topology_kernel_writes(int fd)
> +{
> +	struct drm_i915_query_item item;
> +	struct drm_i915_query_topology_info *topo_info;
> +	uint8_t *_topo_info;
> +	int b, total_size;
> +
> +	memset(&item, 0, sizeof(item));
> +	item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	i915_query_items(fd, &item, 1);
> +	igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
> +
> +	total_size = item.length + 2 * sizeof(*_topo_info);
> +	_topo_info = malloc(total_size);
> +	memset(_topo_info, 0xff, total_size);
> +	topo_info = (struct drm_i915_query_topology_info *) (_topo_info + sizeof(*_topo_info));
> +	memset(topo_info, 0, item.length);
> +
> +	item.data_ptr = to_user_pointer(topo_info);
> +	i915_query_items(fd, &item, 1);
> +
> +	for (b = 0; b < sizeof(*_topo_info); b++) {
> +		igt_assert_eq(_topo_info[b], 0xff);
> +		igt_assert_eq(_topo_info[sizeof(*_topo_info) + item.length + b], 0xff);
> +	}
> +}
> +
> +static bool query_topology_supported(int fd)
> +{
> +	struct drm_i915_query_item item = {
> +		.query_id = DRM_I915_QUERY_TOPOLOGY_INFO,
> +	};
> +
> +	return __i915_query_items(fd, &item, 1) == 0 && item.length > 0;
> +}
> +
> +static void test_query_topology_unsupported(int fd)
> +{
> +	struct drm_i915_query_item item = {
> +		.query_id = DRM_I915_QUERY_TOPOLOGY_INFO,
> +	};
> +
> +	i915_query_items(fd, &item, 1);
> +	igt_assert_eq(item.length, -ENODEV);
> +}
> +
> +#define DIV_ROUND_UP(val, div) (ALIGN(val, div) / div)
> +
> +static bool
> +slice_available(const struct drm_i915_query_topology_info *topo_info,
> +		int s)
> +{
> +	return (topo_info->data[s / 8] >> (s % 8)) & 1;
> +}
> +
> +static bool
> +subslice_available(const struct drm_i915_query_topology_info *topo_info,
> +		   int s, int ss)
> +{
> +	return (topo_info->data[topo_info->subslice_offset +
> +				s * topo_info->subslice_stride +
> +				ss / 8] >> (ss % 8)) & 1;
> +}
> +
> +static bool
> +eu_available(const struct drm_i915_query_topology_info *topo_info,
> +	     int s, int ss, int eu)
> +{
> +	return (topo_info->data[topo_info->eu_offset +
> +				(s * topo_info->max_subslices + ss) * topo_info->eu_stride +
> +				eu / 8] >> (eu % 8)) & 1;
> +}
> +
> +/* Verify that we get coherent values between the legacy getparam slice/subslice
> + * masks and the new topology query.
> + */
> +static void
> +test_query_topology_coherent_slice_mask(int fd)
> +{
> +	struct drm_i915_query_item item;
> +	struct drm_i915_query_topology_info *topo_info;
> +	drm_i915_getparam_t gp;
> +	int slice_mask, subslice_mask;
> +	int s, topology_slices, topology_subslices_slice0;
> +
> +	gp.param = I915_PARAM_SLICE_MASK;
> +	gp.value = &slice_mask;
> +	igt_skip_on(igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0);
> +
> +	gp.param = I915_PARAM_SUBSLICE_MASK;
> +	gp.value = &subslice_mask;
> +	igt_skip_on(igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0);
> +
> +	/* Slices */
> +	memset(&item, 0, sizeof(item));
> +	item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	i915_query_items(fd, &item, 1);
> +	/* We expect at least one byte for each slices, subslices & EUs masks. */
> +	igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
> +
> +	topo_info = malloc(item.length);
> +	memset(topo_info, 0, item.length);
> +
> +	item.data_ptr = to_user_pointer(topo_info);
> +	i915_query_items(fd, &item, 1);
> +	/* We expect at least one byte for each slices, subslices & EUs masks. */
> +	igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);

I'd check length here is exactly the same as the initial query.

> +
> +	topology_slices = 0;
> +	for (s = 0; s < topo_info->max_slices; s++) {
> +		if (slice_available(topo_info, s))
> +			topology_slices |= 1UL << s;
> +	}
> +
> +	igt_debug("slice mask getparam=0x%x / query=0x%x\n",
> +		  slice_mask, topology_slices);
> +
> +	/* These 2 should always match. */
> +	igt_assert_eq(slice_mask, topology_slices);
> +
> +	topology_subslices_slice0 = 0;
> +	for (s = 0; s < topo_info->max_subslices; s++) {
> +		if (subslice_available(topo_info, 0, s))
> +			topology_subslices_slice0 |= 1UL << s;
> +	}
> +
> +	igt_debug("subslice mask getparam=0x%x / query=0x%x\n",
> +		  subslice_mask, topology_subslices_slice0);
> +
> +	/* I915_PARAM_SUBSLICE_MASK returns the value for slice0, we should
> +	 * match the values for the first slice of the topology.
> +	 */
> +	igt_assert_eq(subslice_mask, topology_subslices_slice0);
> +
> +	free(topo_info);
> +}
> +
> +/* Verify that we get same total number of EUs from getparam and topology query.
> + */
> +static void
> +test_query_topology_matches_eu_total(int fd)
> +{
> +	struct drm_i915_query_item item;
> +	struct drm_i915_query_topology_info *topo_info;
> +	drm_i915_getparam_t gp;
> +	int n_eus, n_eus_topology, s;
> +
> +	gp.param = I915_PARAM_EU_TOTAL;
> +	gp.value = &n_eus;
> +	do_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> +	igt_debug("n_eus=%i\n", n_eus);
> +
> +	memset(&item, 0, sizeof(item));
> +	item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	i915_query_items(fd, &item, 1);
> +
> +	topo_info = (struct drm_i915_query_topology_info *) calloc(1, item.length);

Oookay.. but above you use malloc and memset. Doesn't matter I guess.

> +
> +	item.data_ptr = to_user_pointer(topo_info);
> +	i915_query_items(fd, &item, 1);
> +
> +	igt_debug("max_slices=%hu max_subslices=%hu max_eus_per_subslice=%hu\n",
> +		  topo_info->max_slices, topo_info->max_subslices,
> +		  topo_info->max_eus_per_subslice);
> +	igt_debug(" subslice_offset=%hu subslice_stride=%hu\n",
> +		  topo_info->subslice_offset, topo_info->subslice_stride);
> +	igt_debug(" eu_offset=%hu eu_stride=%hu\n",
> +		  topo_info->eu_offset, topo_info->eu_stride);
> +
> +	n_eus_topology = 0;
> +	for (s = 0; s < topo_info->max_slices; s++) {
> +		int ss;
> +
> +		igt_debug("slice%i:\n", s);
> +
> +		for (ss = 0; ss < topo_info->max_subslices; ss++) {
> +			int eu, n_subslice_eus = 0;
> +
> +			igt_debug("\tsubslice: %i\n", ss);
> +
> +			igt_debug("\t\teu_mask: 0b");
> +			for (eu = 0; eu < topo_info->max_eus_per_subslice; eu++) {
> +				uint8_t val = eu_available(topo_info, s, ss,
> +							   topo_info->max_eus_per_subslice - 1 - eu);
> +				igt_debug("%hhi", val);
> +				n_subslice_eus += __builtin_popcount(val);
> +				n_eus_topology += __builtin_popcount(val);
> +			}
> +
> +			igt_debug(" (%i)\n", n_subslice_eus);
> +
> +			/* Sanity checks. */
> +			if (n_subslice_eus > 0) {
> +				igt_assert(slice_available(topo_info, s));
> +				igt_assert(subslice_available(topo_info, s, ss));
> +			}
> +			if (subslice_available(topo_info, s, ss)) {
> +				igt_assert(slice_available(topo_info, s));
> +			}
> +		}
> +	}
> +
> +	free(topo_info);
> +
> +	igt_assert(n_eus_topology == n_eus);
> +}
> +
> +/* Verify some numbers on Gens that we know for sure the characteristics from
> + * the PCI ids.
> + */
> +static void
> +test_query_topology_known_pci_ids(int fd, int devid)
> +{
> +	const struct intel_device_info *dev_info = intel_get_device_info(devid);
> +	struct drm_i915_query_item item;
> +	struct drm_i915_query_topology_info *topo_info;
> +	int n_slices = 0, n_subslices = 0;
> +	int s, ss;
> +
> +	/* The GT size on some Broadwell skus is not defined, skip those. */
> +	igt_skip_on(dev_info->gt == 0);
> +
> +	memset(&item, 0, sizeof(item));
> +	item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
> +	i915_query_items(fd, &item, 1);
> +
> +	topo_info = (struct drm_i915_query_topology_info *) calloc(1, item.length);
> +
> +	item.data_ptr = to_user_pointer(topo_info);
> +	i915_query_items(fd, &item, 1);
> +
> +	for (s = 0; s < topo_info->max_slices; s++) {
> +		if (slice_available(topo_info, s))
> +			n_slices++;
> +
> +		for (ss = 0; ss < topo_info->max_subslices; ss++) {
> +			if (subslice_available(topo_info, s, ss))
> +				n_subslices++;
> +		}
> +	}
> +
> +	switch (dev_info->gt) {
> +	case 1:
> +		igt_assert_eq(n_slices, 1);
> +		igt_assert(n_subslices == 2 || n_subslices == 3);
> +		break;
> +	case 2:
> +		igt_assert_eq(n_slices, 1);
> +		igt_assert_eq(n_subslices, 3);
> +		break;
> +	case 3:
> +		igt_assert_eq(n_slices, 2);
> +		igt_assert_eq(n_subslices, 6);
> +		break;
> +	case 4:
> +		igt_assert_eq(n_slices, 3);
> +		igt_assert_eq(n_subslices, 6);
> +		break;
> +	default:
> +		igt_assert(false);
> +	}
> +
> +	free(topo_info);
> +}
> +
> +igt_main
> +{
> +	int fd = -1;
> +	int devid;
> +
> +	igt_fixture {
> +		fd = drm_open_driver(DRIVER_INTEL);
> +		igt_require(has_query_supports(fd));
> +		devid = intel_get_drm_devid(fd);
> +	}
> +
> +	igt_subtest("query-garbage")
> +		test_query_garbage(fd);
> +
> +	igt_subtest("query-topology-kernel-writes") {
> +		igt_require(query_topology_supported(fd));
> +		test_query_topology_kernel_writes(fd);
> +	}
> +
> +	igt_subtest("query-topology-unsupported") {
> +		igt_require(!query_topology_supported(fd));
> +		test_query_topology_unsupported(fd);
> +	}
> +
> +	igt_subtest("query-topology-coherent-slice-mask") {
> +		igt_require(query_topology_supported(fd));
> +		test_query_topology_coherent_slice_mask(fd);
> +	}
> +
> +	igt_subtest("query-topology-matches-eu-total") {
> +		igt_require(query_topology_supported(fd));
> +		test_query_topology_matches_eu_total(fd);
> +	}
> +
> +	igt_subtest("query-topology-haswell") {
> +		igt_require(query_topology_supported(fd));
> +		igt_require(IS_HASWELL(devid) || IS_BROADWELL(devid) ||
> +			    IS_SKYLAKE(devid) || IS_KABYLAKE(devid) ||
> +			    IS_COFFEELAKE(devid));

So it's possible, great!

> +		test_query_topology_known_pci_ids(fd, devid);
> +	}
> +
> +	igt_fixture {
> +		close(fd);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 58729231..5788dfd0 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -143,6 +143,7 @@ test_progs = [
>   	'gen3_render_tiledx_blits',
>   	'gen3_render_tiledy_blits',
>   	'gvt_basic',
> +	'i915_query',
>   	'kms_3d',
>   	'kms_addfb_basic',
>   	'kms_atomic',
> 

AFAIR only a few missing corner cases to test, the rest are either style 
or similar comments.

I gather you did not like my suggestion to abstract the query item 
testing to be able to test many invalid combinations easily, since you 
did not comment on it?

Regards,

Tvrtko

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v8 3/3] tests: add i915 query tests
  2018-03-09 13:23   ` Tvrtko Ursulin
@ 2018-03-09 14:07     ` Lionel Landwerlin
  2018-03-09 14:21       ` Tvrtko Ursulin
  0 siblings, 1 reply; 10+ messages in thread
From: Lionel Landwerlin @ 2018-03-09 14:07 UTC (permalink / raw)
  To: Tvrtko Ursulin, igt-dev

On 09/03/18 13:23, Tvrtko Ursulin wrote:
>
> You did not like my suggestion to add a commit message? :)

Sorry, I forgot...

>
> On 09/03/2018 09:31, Lionel Landwerlin wrote:
>> v2: Complete invalid cases (Chris)
>>      Some styling (to_user_pointer, etc...) (Chris)
>>      New error check, through item.length (Chris)
>>
>> v3: Update for new uAPI iteration (Lionel)
>>
>> v4: Return errno from a single point (Chris)
>>      Poising checks (Chris)
>>
>> v5: Add more debug traces (Lionel)
>>      Update uAPI (Joonas/Lionel)
>>      Make sure Haswell is tested (Lionel)
>>
>> v6: s/query_item/query_items/ (Tvrtko)
>>      test that flags fields != 0 fail (Tvrtko)
>>      Split kernel writes checks out (Tvrtko)
>>      Verify that when an EU is available, so is slice & subslice it
>>      belongs to (same with subslice). (Tvrtko)
>>      Verify kernel errors out with read only memory (Tvrtko)
>>
>> v7: Add a special Haswell test to verify correct values (Tvrtko)
>>      Simplify igt_require() in front of tests (Tvrtko)
>>
>> v8: Reuse the GT field from device info to verify slice/subslices
>>      numbers on wider number of platforms (Lionel)
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> ---
>>   tests/Makefile.sources |   1 +
>>   tests/i915_query.c     | 476 
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/meson.build      |   1 +
>>   3 files changed, 478 insertions(+)
>>   create mode 100644 tests/i915_query.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 4a81ac4a..4e6f5319 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -167,6 +167,7 @@ TESTS_progs = \
>>       gen3_render_tiledx_blits \
>>       gen3_render_tiledy_blits \
>>       gvt_basic \
>> +    i915_query \
>>       kms_3d \
>>       kms_addfb_basic \
>>       kms_atomic \
>> diff --git a/tests/i915_query.c b/tests/i915_query.c
>> new file mode 100644
>> index 00000000..14ad2146
>> --- /dev/null
>> +++ b/tests/i915_query.c
>> @@ -0,0 +1,476 @@
>> +/*
>> + * Copyright © 2017 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom 
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including 
>> the next
>> + * paragraph) shall be included in all copies or substantial 
>> portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +
>> +#include "igt.h"
>> +
>> +#include <limits.h>
>> +
>> +IGT_TEST_DESCRIPTION("Testing the i915 query uAPI.");
>> +
>> +/* We should at least get 3 bytes for data for each slices, 
>> subslices & EUs
>> + * masks.
>> + */
>
> We try to avoid this comment style in i915 and IGT.

Ah, same rules then?

>
>> +#define MIN_TOPOLOGY_ITEM_SIZE (sizeof(struct 
>> drm_i915_query_topology_info) + 3)
>> +
>> +static int
>> +__i915_query(int fd, struct drm_i915_query *q)
>> +{
>> +    if (igt_ioctl(fd, DRM_IOCTL_I915_QUERY, q))
>> +        return -errno;
>> +    return 0;
>> +}
>> +
>> +static int
>> +__i915_query_items(int fd, struct drm_i915_query_item *items, 
>> uint32_t n_items)
>> +{
>> +    struct drm_i915_query q = {
>> +        .num_items = n_items,
>> +        .items_ptr = to_user_pointer(items),
>> +    };
>> +    return __i915_query(fd, &q);
>> +}
>> +
>> +#define i915_query_items(fd, items, n_items) do { \
>> +        igt_assert_eq(__i915_query_items(fd, items, n_items), 0); \
>> +        errno = 0; \
>> +    } while (0)
>> +#define i915_query_items_err(fd, items, n_items, err) do { \
>> +        igt_assert_eq(__i915_query_items(fd, items, n_items), -err); \
>> +    } while (0)
>> +
>> +static bool has_query_supports(int fd)
>> +{
>> +    struct drm_i915_query query = {};
>> +
>> +    return __i915_query(fd, &query) == 0;
>> +}
>> +
>> +static void test_query_garbage(int fd)
>> +{
>> +    struct drm_i915_query query;
>> +    struct drm_i915_query_item items[2];
>> +    struct drm_i915_query_item *items_ptr;
>> +    int i, n_items;
>> +
>> +    /* Query flags field is currently valid only if equals to 0. 
>> This might
>> +     * change in the future.
>> +     */
>> +    memset(&query, 0, sizeof(query));
>> +    query.flags = 42;
>> +    igt_assert_eq(__i915_query(fd, &query), -EINVAL);
>> +
>> +    /* Test a couple of invalid pointers. */
>> +    i915_query_items_err(fd, (void *) ULONG_MAX, 1, EFAULT);
>> +    i915_query_items_err(fd, (void *) 0, 1, EFAULT);
>
> Also need one where struct drm_i915_query itself it invalid or not 
> mapped.

Doesn't the tests below (unmapping the items pointer) cover that case?

>
>> +
>> +    /* Test the invalid query id = 0. */
>> +    memset(items, 0, sizeof(items));
>> +    i915_query_items_err(fd, items, 1, EINVAL);
>> +
>> +    /* Query item flags field is currently valid only if equals to 0.
>> +     * Subject to change in the future.
>> +     */
>> +    memset(items, 0, sizeof(items));
>> +    items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    items[0].flags = 42;
>> +    i915_query_items(fd, items, 1);
>> +    igt_assert_eq(items[0].length, -EINVAL);
>> +
>> +    /* Test an invalid query id in the second item and verify that 
>> the first
>> +     * one is properly processed.
>> +     */
>> +    memset(items, 0, sizeof(items));
>> +    items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    items[1].query_id = ULONG_MAX;
>> +    i915_query_items(fd, items, 2);
>> +    igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, items[0].length);
>
> Someone needs to add igt_assert_gt(e) to save me from reversed 
> conditions! :)

I know....

>
>> +    igt_assert_eq(items[1].length, -EINVAL);
>> +
>> +    /* Test a invalid query id in the first item and verify that the 
>> second
>> +     * one is properly processed (the driver is expected to go 
>> through them
>> +     * all and place error codes in the failed items).
>> +     */
>> +    memset(items, 0, sizeof(items));
>> +    items[0].query_id = ULONG_MAX;
>> +    items[1].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    i915_query_items(fd, items, 2);
>> +    igt_assert_eq(items[0].length, -EINVAL);
>> +    igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, items[1].length);
>> +
>> +    /* Test an invalid query item length. */
>> +    memset(items, 0, sizeof(items));
>> +    items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    items[1].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    items[1].length = sizeof(struct drm_i915_query_topology_info) - 1;
>> +    i915_query_items(fd, items, 2);
>> +    igt_assert_lte(0, items[0].length);
>
> Length of zero would be a bug - MIN_TOPOLOGY_ITEM_SIZE ?

Thanks!

>
>> +    igt_assert_eq(items[1].length, -EINVAL);
>> +
>> +    /* Map memory for a query item in which the kernel is going to 
>> write the
>> +     * length of the item in the first ioctl(). Then unmap that 
>> memory and
>> +     * verify that the kernel correctly returns EFAULT as memory of 
>> the item
>> +     * has been removed from our address space.
>> +     */
>> +    items_ptr = mmap(0, 4096, PROT_WRITE, MAP_PRIVATE | MAP_ANON, 
>> -1, 0);
>> +    items_ptr[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    i915_query_items(fd, items_ptr, 1);
>> +    igt_assert(items_ptr[0].length >= sizeof(struct 
>> drm_i915_query_topology_info));
>
> MIN_TOPOLOGY_ITEM_SIZE? I welcome the >= though. :)

Done.

>
>> +    munmap(items_ptr, 4096);
>> +    i915_query_items_err(fd, items_ptr, 1, EFAULT);
>> +
>> +    /* Map memory for a query item, then make it read only and 
>> verify that
>> +     * the kernel errors out with EFAULT.
>> +     */
>> +    items_ptr = mmap(0, 4096, PROT_WRITE, MAP_PRIVATE | MAP_ANON, 
>> -1, 0);
>> +    items_ptr[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    igt_assert_eq(0, mprotect(items_ptr, 4096, PROT_READ));
>> +    i915_query_items_err(fd, items_ptr, 1, EFAULT);
>> +    munmap(items_ptr, 4096);
>> +
>> +    /* Allocate 2 pages, prepare those 2 pages with valid query 
>> items, then
>> +     * switch the second page to read only and expect an EFAULT error.
>> +     */
>> +    items_ptr = mmap(0, 8192, PROT_WRITE, MAP_PRIVATE | MAP_ANON, 
>> -1, 0);
>> +    memset(items_ptr, 0, 8192);
>> +    n_items = 8192 / sizeof(struct drm_i915_query_item);
>> +    for (i = 0; i < n_items; i++)
>> +        items_ptr[i].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    mprotect(((uint8_t *)items_ptr) + 4096, 4096, PROT_READ);
>> +    i915_query_items_err(fd, items_ptr, n_items, EFAULT);
>> +    munmap(items_ptr, 8192);
>
> Another missing subtest is where query items ptr is valid, but the 
> topo data ptr is not. So -EFAULT is returned in item->length.

Thanks, adding that.

>
>> +}
>> +
>> +/* Allocate more on both sides of where the kernel is going to write 
>> and verify
>> + * that it writes only where it's supposed to.
>> + */
>> +static void test_query_topology_kernel_writes(int fd)
>> +{
>> +    struct drm_i915_query_item item;
>> +    struct drm_i915_query_topology_info *topo_info;
>> +    uint8_t *_topo_info;
>> +    int b, total_size;
>> +
>> +    memset(&item, 0, sizeof(item));
>> +    item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    i915_query_items(fd, &item, 1);
>> +    igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
>> +
>> +    total_size = item.length + 2 * sizeof(*_topo_info);
>> +    _topo_info = malloc(total_size);
>> +    memset(_topo_info, 0xff, total_size);
>> +    topo_info = (struct drm_i915_query_topology_info *) (_topo_info 
>> + sizeof(*_topo_info));
>> +    memset(topo_info, 0, item.length);
>> +
>> +    item.data_ptr = to_user_pointer(topo_info);
>> +    i915_query_items(fd, &item, 1);
>> +
>> +    for (b = 0; b < sizeof(*_topo_info); b++) {
>> +        igt_assert_eq(_topo_info[b], 0xff);
>> +        igt_assert_eq(_topo_info[sizeof(*_topo_info) + item.length + 
>> b], 0xff);
>> +    }
>> +}
>> +
>> +static bool query_topology_supported(int fd)
>> +{
>> +    struct drm_i915_query_item item = {
>> +        .query_id = DRM_I915_QUERY_TOPOLOGY_INFO,
>> +    };
>> +
>> +    return __i915_query_items(fd, &item, 1) == 0 && item.length > 0;
>> +}
>> +
>> +static void test_query_topology_unsupported(int fd)
>> +{
>> +    struct drm_i915_query_item item = {
>> +        .query_id = DRM_I915_QUERY_TOPOLOGY_INFO,
>> +    };
>> +
>> +    i915_query_items(fd, &item, 1);
>> +    igt_assert_eq(item.length, -ENODEV);
>> +}
>> +
>> +#define DIV_ROUND_UP(val, div) (ALIGN(val, div) / div)
>> +
>> +static bool
>> +slice_available(const struct drm_i915_query_topology_info *topo_info,
>> +        int s)
>> +{
>> +    return (topo_info->data[s / 8] >> (s % 8)) & 1;
>> +}
>> +
>> +static bool
>> +subslice_available(const struct drm_i915_query_topology_info 
>> *topo_info,
>> +           int s, int ss)
>> +{
>> +    return (topo_info->data[topo_info->subslice_offset +
>> +                s * topo_info->subslice_stride +
>> +                ss / 8] >> (ss % 8)) & 1;
>> +}
>> +
>> +static bool
>> +eu_available(const struct drm_i915_query_topology_info *topo_info,
>> +         int s, int ss, int eu)
>> +{
>> +    return (topo_info->data[topo_info->eu_offset +
>> +                (s * topo_info->max_subslices + ss) * 
>> topo_info->eu_stride +
>> +                eu / 8] >> (eu % 8)) & 1;
>> +}
>> +
>> +/* Verify that we get coherent values between the legacy getparam 
>> slice/subslice
>> + * masks and the new topology query.
>> + */
>> +static void
>> +test_query_topology_coherent_slice_mask(int fd)
>> +{
>> +    struct drm_i915_query_item item;
>> +    struct drm_i915_query_topology_info *topo_info;
>> +    drm_i915_getparam_t gp;
>> +    int slice_mask, subslice_mask;
>> +    int s, topology_slices, topology_subslices_slice0;
>> +
>> +    gp.param = I915_PARAM_SLICE_MASK;
>> +    gp.value = &slice_mask;
>> +    igt_skip_on(igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0);
>> +
>> +    gp.param = I915_PARAM_SUBSLICE_MASK;
>> +    gp.value = &subslice_mask;
>> +    igt_skip_on(igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0);
>> +
>> +    /* Slices */
>> +    memset(&item, 0, sizeof(item));
>> +    item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    i915_query_items(fd, &item, 1);
>> +    /* We expect at least one byte for each slices, subslices & EUs 
>> masks. */
>> +    igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
>> +
>> +    topo_info = malloc(item.length);
>> +    memset(topo_info, 0, item.length);
>> +
>> +    item.data_ptr = to_user_pointer(topo_info);
>> +    i915_query_items(fd, &item, 1);
>> +    /* We expect at least one byte for each slices, subslices & EUs 
>> masks. */
>> +    igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
>
> I'd check length here is exactly the same as the initial query.

Adding.

>
>> +
>> +    topology_slices = 0;
>> +    for (s = 0; s < topo_info->max_slices; s++) {
>> +        if (slice_available(topo_info, s))
>> +            topology_slices |= 1UL << s;
>> +    }
>> +
>> +    igt_debug("slice mask getparam=0x%x / query=0x%x\n",
>> +          slice_mask, topology_slices);
>> +
>> +    /* These 2 should always match. */
>> +    igt_assert_eq(slice_mask, topology_slices);
>> +
>> +    topology_subslices_slice0 = 0;
>> +    for (s = 0; s < topo_info->max_subslices; s++) {
>> +        if (subslice_available(topo_info, 0, s))
>> +            topology_subslices_slice0 |= 1UL << s;
>> +    }
>> +
>> +    igt_debug("subslice mask getparam=0x%x / query=0x%x\n",
>> +          subslice_mask, topology_subslices_slice0);
>> +
>> +    /* I915_PARAM_SUBSLICE_MASK returns the value for slice0, we should
>> +     * match the values for the first slice of the topology.
>> +     */
>> +    igt_assert_eq(subslice_mask, topology_subslices_slice0);
>> +
>> +    free(topo_info);
>> +}
>> +
>> +/* Verify that we get same total number of EUs from getparam and 
>> topology query.
>> + */
>> +static void
>> +test_query_topology_matches_eu_total(int fd)
>> +{
>> +    struct drm_i915_query_item item;
>> +    struct drm_i915_query_topology_info *topo_info;
>> +    drm_i915_getparam_t gp;
>> +    int n_eus, n_eus_topology, s;
>> +
>> +    gp.param = I915_PARAM_EU_TOTAL;
>> +    gp.value = &n_eus;
>> +    do_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
>> +    igt_debug("n_eus=%i\n", n_eus);
>> +
>> +    memset(&item, 0, sizeof(item));
>> +    item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    i915_query_items(fd, &item, 1);
>> +
>> +    topo_info = (struct drm_i915_query_topology_info *) calloc(1, 
>> item.length);
>
> Oookay.. but above you use malloc and memset. Doesn't matter I guess.

Changing to calloc everywhere. (Just a left over of the outside bounds 
writing test).

>
>> +
>> +    item.data_ptr = to_user_pointer(topo_info);
>> +    i915_query_items(fd, &item, 1);
>> +
>> +    igt_debug("max_slices=%hu max_subslices=%hu 
>> max_eus_per_subslice=%hu\n",
>> +          topo_info->max_slices, topo_info->max_subslices,
>> +          topo_info->max_eus_per_subslice);
>> +    igt_debug(" subslice_offset=%hu subslice_stride=%hu\n",
>> +          topo_info->subslice_offset, topo_info->subslice_stride);
>> +    igt_debug(" eu_offset=%hu eu_stride=%hu\n",
>> +          topo_info->eu_offset, topo_info->eu_stride);
>> +
>> +    n_eus_topology = 0;
>> +    for (s = 0; s < topo_info->max_slices; s++) {
>> +        int ss;
>> +
>> +        igt_debug("slice%i:\n", s);
>> +
>> +        for (ss = 0; ss < topo_info->max_subslices; ss++) {
>> +            int eu, n_subslice_eus = 0;
>> +
>> +            igt_debug("\tsubslice: %i\n", ss);
>> +
>> +            igt_debug("\t\teu_mask: 0b");
>> +            for (eu = 0; eu < topo_info->max_eus_per_subslice; eu++) {
>> +                uint8_t val = eu_available(topo_info, s, ss,
>> + topo_info->max_eus_per_subslice - 1 - eu);
>> +                igt_debug("%hhi", val);
>> +                n_subslice_eus += __builtin_popcount(val);
>> +                n_eus_topology += __builtin_popcount(val);
>> +            }
>> +
>> +            igt_debug(" (%i)\n", n_subslice_eus);
>> +
>> +            /* Sanity checks. */
>> +            if (n_subslice_eus > 0) {
>> +                igt_assert(slice_available(topo_info, s));
>> +                igt_assert(subslice_available(topo_info, s, ss));
>> +            }
>> +            if (subslice_available(topo_info, s, ss)) {
>> +                igt_assert(slice_available(topo_info, s));
>> +            }
>> +        }
>> +    }
>> +
>> +    free(topo_info);
>> +
>> +    igt_assert(n_eus_topology == n_eus);
>> +}
>> +
>> +/* Verify some numbers on Gens that we know for sure the 
>> characteristics from
>> + * the PCI ids.
>> + */
>> +static void
>> +test_query_topology_known_pci_ids(int fd, int devid)
>> +{
>> +    const struct intel_device_info *dev_info = 
>> intel_get_device_info(devid);
>> +    struct drm_i915_query_item item;
>> +    struct drm_i915_query_topology_info *topo_info;
>> +    int n_slices = 0, n_subslices = 0;
>> +    int s, ss;
>> +
>> +    /* The GT size on some Broadwell skus is not defined, skip 
>> those. */
>> +    igt_skip_on(dev_info->gt == 0);
>> +
>> +    memset(&item, 0, sizeof(item));
>> +    item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>> +    i915_query_items(fd, &item, 1);
>> +
>> +    topo_info = (struct drm_i915_query_topology_info *) calloc(1, 
>> item.length);
>> +
>> +    item.data_ptr = to_user_pointer(topo_info);
>> +    i915_query_items(fd, &item, 1);
>> +
>> +    for (s = 0; s < topo_info->max_slices; s++) {
>> +        if (slice_available(topo_info, s))
>> +            n_slices++;
>> +
>> +        for (ss = 0; ss < topo_info->max_subslices; ss++) {
>> +            if (subslice_available(topo_info, s, ss))
>> +                n_subslices++;
>> +        }
>> +    }
>> +
>> +    switch (dev_info->gt) {
>> +    case 1:
>> +        igt_assert_eq(n_slices, 1);
>> +        igt_assert(n_subslices == 2 || n_subslices == 3);
>> +        break;
>> +    case 2:
>> +        igt_assert_eq(n_slices, 1);
>> +        igt_assert_eq(n_subslices, 3);
>> +        break;
>> +    case 3:
>> +        igt_assert_eq(n_slices, 2);
>> +        igt_assert_eq(n_subslices, 6);
>> +        break;
>> +    case 4:
>> +        igt_assert_eq(n_slices, 3);
>> +        igt_assert_eq(n_subslices, 6);
>> +        break;
>> +    default:
>> +        igt_assert(false);
>> +    }
>> +
>> +    free(topo_info);
>> +}
>> +
>> +igt_main
>> +{
>> +    int fd = -1;
>> +    int devid;
>> +
>> +    igt_fixture {
>> +        fd = drm_open_driver(DRIVER_INTEL);
>> +        igt_require(has_query_supports(fd));
>> +        devid = intel_get_drm_devid(fd);
>> +    }
>> +
>> +    igt_subtest("query-garbage")
>> +        test_query_garbage(fd);
>> +
>> +    igt_subtest("query-topology-kernel-writes") {
>> +        igt_require(query_topology_supported(fd));
>> +        test_query_topology_kernel_writes(fd);
>> +    }
>> +
>> +    igt_subtest("query-topology-unsupported") {
>> +        igt_require(!query_topology_supported(fd));
>> +        test_query_topology_unsupported(fd);
>> +    }
>> +
>> +    igt_subtest("query-topology-coherent-slice-mask") {
>> +        igt_require(query_topology_supported(fd));
>> +        test_query_topology_coherent_slice_mask(fd);
>> +    }
>> +
>> +    igt_subtest("query-topology-matches-eu-total") {
>> +        igt_require(query_topology_supported(fd));
>> +        test_query_topology_matches_eu_total(fd);
>> +    }
>> +
>> +    igt_subtest("query-topology-haswell") {
>> +        igt_require(query_topology_supported(fd));
>> +        igt_require(IS_HASWELL(devid) || IS_BROADWELL(devid) ||
>> +                IS_SKYLAKE(devid) || IS_KABYLAKE(devid) ||
>> +                IS_COFFEELAKE(devid));
>
> So it's possible, great!

Fingers crossed. I wouldn't be surprised to see weird things in the wild...

>
>> + test_query_topology_known_pci_ids(fd, devid);
>> +    }
>> +
>> +    igt_fixture {
>> +        close(fd);
>> +    }
>> +}
>> diff --git a/tests/meson.build b/tests/meson.build
>> index 58729231..5788dfd0 100644
>> --- a/tests/meson.build
>> +++ b/tests/meson.build
>> @@ -143,6 +143,7 @@ test_progs = [
>>       'gen3_render_tiledx_blits',
>>       'gen3_render_tiledy_blits',
>>       'gvt_basic',
>> +    'i915_query',
>>       'kms_3d',
>>       'kms_addfb_basic',
>>       'kms_atomic',
>>
>
> AFAIR only a few missing corner cases to test, the rest are either 
> style or similar comments.
>
> I gather you did not like my suggestion to abstract the query item 
> testing to be able to test many invalid combinations easily, since you 
> did not comment on it?

It felt like more code than what I've written here.
Maybe that will change in the future.

Thanks a lot for your time on this!

>
> Regards,
>
> Tvrtko
>
>

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v8 3/3] tests: add i915 query tests
  2018-03-09 14:07     ` Lionel Landwerlin
@ 2018-03-09 14:21       ` Tvrtko Ursulin
  2018-03-09 14:27         ` Chris Wilson
  0 siblings, 1 reply; 10+ messages in thread
From: Tvrtko Ursulin @ 2018-03-09 14:21 UTC (permalink / raw)
  To: Lionel Landwerlin, igt-dev


On 09/03/2018 14:07, Lionel Landwerlin wrote:
> On 09/03/18 13:23, Tvrtko Ursulin wrote:
>>
>> You did not like my suggestion to add a commit message? :)
> 
> Sorry, I forgot...
> 
>>
>> On 09/03/2018 09:31, Lionel Landwerlin wrote:
>>> v2: Complete invalid cases (Chris)
>>>      Some styling (to_user_pointer, etc...) (Chris)
>>>      New error check, through item.length (Chris)
>>>
>>> v3: Update for new uAPI iteration (Lionel)
>>>
>>> v4: Return errno from a single point (Chris)
>>>      Poising checks (Chris)
>>>
>>> v5: Add more debug traces (Lionel)
>>>      Update uAPI (Joonas/Lionel)
>>>      Make sure Haswell is tested (Lionel)
>>>
>>> v6: s/query_item/query_items/ (Tvrtko)
>>>      test that flags fields != 0 fail (Tvrtko)
>>>      Split kernel writes checks out (Tvrtko)
>>>      Verify that when an EU is available, so is slice & subslice it
>>>      belongs to (same with subslice). (Tvrtko)
>>>      Verify kernel errors out with read only memory (Tvrtko)
>>>
>>> v7: Add a special Haswell test to verify correct values (Tvrtko)
>>>      Simplify igt_require() in front of tests (Tvrtko)
>>>
>>> v8: Reuse the GT field from device info to verify slice/subslices
>>>      numbers on wider number of platforms (Lionel)
>>>
>>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> ---
>>>   tests/Makefile.sources |   1 +
>>>   tests/i915_query.c     | 476 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>>   tests/meson.build      |   1 +
>>>   3 files changed, 478 insertions(+)
>>>   create mode 100644 tests/i915_query.c
>>>
>>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>>> index 4a81ac4a..4e6f5319 100644
>>> --- a/tests/Makefile.sources
>>> +++ b/tests/Makefile.sources
>>> @@ -167,6 +167,7 @@ TESTS_progs = \
>>>       gen3_render_tiledx_blits \
>>>       gen3_render_tiledy_blits \
>>>       gvt_basic \
>>> +    i915_query \
>>>       kms_3d \
>>>       kms_addfb_basic \
>>>       kms_atomic \
>>> diff --git a/tests/i915_query.c b/tests/i915_query.c
>>> new file mode 100644
>>> index 00000000..14ad2146
>>> --- /dev/null
>>> +++ b/tests/i915_query.c
>>> @@ -0,0 +1,476 @@
>>> +/*
>>> + * Copyright © 2017 Intel Corporation
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person 
>>> obtaining a
>>> + * copy of this software and associated documentation files (the 
>>> "Software"),
>>> + * to deal in the Software without restriction, including without 
>>> limitation
>>> + * the rights to use, copy, modify, merge, publish, distribute, 
>>> sublicense,
>>> + * and/or sell copies of the Software, and to permit persons to whom 
>>> the
>>> + * Software is furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice (including 
>>> the next
>>> + * paragraph) shall be included in all copies or substantial 
>>> portions of the
>>> + * Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>>> EXPRESS OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>>> MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>>> EVENT SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>>> OR OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>>> ARISING
>>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
>>> OTHER DEALINGS
>>> + * IN THE SOFTWARE.
>>> + */
>>> +
>>> +#include "igt.h"
>>> +
>>> +#include <limits.h>
>>> +
>>> +IGT_TEST_DESCRIPTION("Testing the i915 query uAPI.");
>>> +
>>> +/* We should at least get 3 bytes for data for each slices, 
>>> subslices & EUs
>>> + * masks.
>>> + */
>>
>> We try to avoid this comment style in i915 and IGT.
> 
> Ah, same rules then?
> 
>>
>>> +#define MIN_TOPOLOGY_ITEM_SIZE (sizeof(struct 
>>> drm_i915_query_topology_info) + 3)
>>> +
>>> +static int
>>> +__i915_query(int fd, struct drm_i915_query *q)
>>> +{
>>> +    if (igt_ioctl(fd, DRM_IOCTL_I915_QUERY, q))
>>> +        return -errno;
>>> +    return 0;
>>> +}
>>> +
>>> +static int
>>> +__i915_query_items(int fd, struct drm_i915_query_item *items, 
>>> uint32_t n_items)
>>> +{
>>> +    struct drm_i915_query q = {
>>> +        .num_items = n_items,
>>> +        .items_ptr = to_user_pointer(items),
>>> +    };
>>> +    return __i915_query(fd, &q);
>>> +}
>>> +
>>> +#define i915_query_items(fd, items, n_items) do { \
>>> +        igt_assert_eq(__i915_query_items(fd, items, n_items), 0); \
>>> +        errno = 0; \
>>> +    } while (0)
>>> +#define i915_query_items_err(fd, items, n_items, err) do { \
>>> +        igt_assert_eq(__i915_query_items(fd, items, n_items), -err); \
>>> +    } while (0)
>>> +
>>> +static bool has_query_supports(int fd)
>>> +{
>>> +    struct drm_i915_query query = {};
>>> +
>>> +    return __i915_query(fd, &query) == 0;
>>> +}
>>> +
>>> +static void test_query_garbage(int fd)
>>> +{
>>> +    struct drm_i915_query query;
>>> +    struct drm_i915_query_item items[2];
>>> +    struct drm_i915_query_item *items_ptr;
>>> +    int i, n_items;
>>> +
>>> +    /* Query flags field is currently valid only if equals to 0. 
>>> This might
>>> +     * change in the future.
>>> +     */
>>> +    memset(&query, 0, sizeof(query));
>>> +    query.flags = 42;
>>> +    igt_assert_eq(__i915_query(fd, &query), -EINVAL);
>>> +
>>> +    /* Test a couple of invalid pointers. */
>>> +    i915_query_items_err(fd, (void *) ULONG_MAX, 1, EFAULT);
>>> +    i915_query_items_err(fd, (void *) 0, 1, EFAULT);
>>
>> Also need one where struct drm_i915_query itself it invalid or not 
>> mapped.
> 
> Doesn't the tests below (unmapping the items pointer) cover that case?

Don't think so. You need to hit the failure in :

+int i915_query_ioctl(struct drm_device *dev, void *data, struct 
drm_file *file)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_query *args = data;
+	struct drm_i915_query_item __user *user_item_ptr =
+		u64_to_user_ptr(args->items_ptr);

This dereference here. Or is DRM core already covering for that? I mean 
the fact...

+static int
+__i915_query_items(int fd, struct drm_i915_query_item *items, uint32_t 
n_items)
+{
+    struct drm_i915_query q = {
+        .num_items = n_items,
+        .items_ptr = to_user_pointer(items),
+    };
+    return __i915_query(fd, &q);

... &q always points to good memory?

>>
>>> +
>>> +    /* Test the invalid query id = 0. */
>>> +    memset(items, 0, sizeof(items));
>>> +    i915_query_items_err(fd, items, 1, EINVAL);
>>> +
>>> +    /* Query item flags field is currently valid only if equals to 0.
>>> +     * Subject to change in the future.
>>> +     */
>>> +    memset(items, 0, sizeof(items));
>>> +    items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    items[0].flags = 42;
>>> +    i915_query_items(fd, items, 1);
>>> +    igt_assert_eq(items[0].length, -EINVAL);
>>> +
>>> +    /* Test an invalid query id in the second item and verify that 
>>> the first
>>> +     * one is properly processed.
>>> +     */
>>> +    memset(items, 0, sizeof(items));
>>> +    items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    items[1].query_id = ULONG_MAX;
>>> +    i915_query_items(fd, items, 2);
>>> +    igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, items[0].length);
>>
>> Someone needs to add igt_assert_gt(e) to save me from reversed 
>> conditions! :)
> 
> I know....
> 
>>
>>> +    igt_assert_eq(items[1].length, -EINVAL);
>>> +
>>> +    /* Test a invalid query id in the first item and verify that the 
>>> second
>>> +     * one is properly processed (the driver is expected to go 
>>> through them
>>> +     * all and place error codes in the failed items).
>>> +     */
>>> +    memset(items, 0, sizeof(items));
>>> +    items[0].query_id = ULONG_MAX;
>>> +    items[1].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    i915_query_items(fd, items, 2);
>>> +    igt_assert_eq(items[0].length, -EINVAL);
>>> +    igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, items[1].length);
>>> +
>>> +    /* Test an invalid query item length. */
>>> +    memset(items, 0, sizeof(items));
>>> +    items[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    items[1].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    items[1].length = sizeof(struct drm_i915_query_topology_info) - 1;
>>> +    i915_query_items(fd, items, 2);
>>> +    igt_assert_lte(0, items[0].length);
>>
>> Length of zero would be a bug - MIN_TOPOLOGY_ITEM_SIZE ?
> 
> Thanks!
> 
>>
>>> +    igt_assert_eq(items[1].length, -EINVAL);
>>> +
>>> +    /* Map memory for a query item in which the kernel is going to 
>>> write the
>>> +     * length of the item in the first ioctl(). Then unmap that 
>>> memory and
>>> +     * verify that the kernel correctly returns EFAULT as memory of 
>>> the item
>>> +     * has been removed from our address space.
>>> +     */
>>> +    items_ptr = mmap(0, 4096, PROT_WRITE, MAP_PRIVATE | MAP_ANON, 
>>> -1, 0);
>>> +    items_ptr[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    i915_query_items(fd, items_ptr, 1);
>>> +    igt_assert(items_ptr[0].length >= sizeof(struct 
>>> drm_i915_query_topology_info));
>>
>> MIN_TOPOLOGY_ITEM_SIZE? I welcome the >= though. :)
> 
> Done.
> 
>>
>>> +    munmap(items_ptr, 4096);
>>> +    i915_query_items_err(fd, items_ptr, 1, EFAULT);
>>> +
>>> +    /* Map memory for a query item, then make it read only and 
>>> verify that
>>> +     * the kernel errors out with EFAULT.
>>> +     */
>>> +    items_ptr = mmap(0, 4096, PROT_WRITE, MAP_PRIVATE | MAP_ANON, 
>>> -1, 0);
>>> +    items_ptr[0].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    igt_assert_eq(0, mprotect(items_ptr, 4096, PROT_READ));
>>> +    i915_query_items_err(fd, items_ptr, 1, EFAULT);
>>> +    munmap(items_ptr, 4096);
>>> +
>>> +    /* Allocate 2 pages, prepare those 2 pages with valid query 
>>> items, then
>>> +     * switch the second page to read only and expect an EFAULT error.
>>> +     */
>>> +    items_ptr = mmap(0, 8192, PROT_WRITE, MAP_PRIVATE | MAP_ANON, 
>>> -1, 0);
>>> +    memset(items_ptr, 0, 8192);
>>> +    n_items = 8192 / sizeof(struct drm_i915_query_item);
>>> +    for (i = 0; i < n_items; i++)
>>> +        items_ptr[i].query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    mprotect(((uint8_t *)items_ptr) + 4096, 4096, PROT_READ);
>>> +    i915_query_items_err(fd, items_ptr, n_items, EFAULT);
>>> +    munmap(items_ptr, 8192);
>>
>> Another missing subtest is where query items ptr is valid, but the 
>> topo data ptr is not. So -EFAULT is returned in item->length.
> 
> Thanks, adding that.
> 
>>
>>> +}
>>> +
>>> +/* Allocate more on both sides of where the kernel is going to write 
>>> and verify
>>> + * that it writes only where it's supposed to.
>>> + */
>>> +static void test_query_topology_kernel_writes(int fd)
>>> +{
>>> +    struct drm_i915_query_item item;
>>> +    struct drm_i915_query_topology_info *topo_info;
>>> +    uint8_t *_topo_info;
>>> +    int b, total_size;
>>> +
>>> +    memset(&item, 0, sizeof(item));
>>> +    item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    i915_query_items(fd, &item, 1);
>>> +    igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
>>> +
>>> +    total_size = item.length + 2 * sizeof(*_topo_info);
>>> +    _topo_info = malloc(total_size);
>>> +    memset(_topo_info, 0xff, total_size);
>>> +    topo_info = (struct drm_i915_query_topology_info *) (_topo_info 
>>> + sizeof(*_topo_info));
>>> +    memset(topo_info, 0, item.length);
>>> +
>>> +    item.data_ptr = to_user_pointer(topo_info);
>>> +    i915_query_items(fd, &item, 1);
>>> +
>>> +    for (b = 0; b < sizeof(*_topo_info); b++) {
>>> +        igt_assert_eq(_topo_info[b], 0xff);
>>> +        igt_assert_eq(_topo_info[sizeof(*_topo_info) + item.length + 
>>> b], 0xff);
>>> +    }
>>> +}
>>> +
>>> +static bool query_topology_supported(int fd)
>>> +{
>>> +    struct drm_i915_query_item item = {
>>> +        .query_id = DRM_I915_QUERY_TOPOLOGY_INFO,
>>> +    };
>>> +
>>> +    return __i915_query_items(fd, &item, 1) == 0 && item.length > 0;
>>> +}
>>> +
>>> +static void test_query_topology_unsupported(int fd)
>>> +{
>>> +    struct drm_i915_query_item item = {
>>> +        .query_id = DRM_I915_QUERY_TOPOLOGY_INFO,
>>> +    };
>>> +
>>> +    i915_query_items(fd, &item, 1);
>>> +    igt_assert_eq(item.length, -ENODEV);
>>> +}
>>> +
>>> +#define DIV_ROUND_UP(val, div) (ALIGN(val, div) / div)
>>> +
>>> +static bool
>>> +slice_available(const struct drm_i915_query_topology_info *topo_info,
>>> +        int s)
>>> +{
>>> +    return (topo_info->data[s / 8] >> (s % 8)) & 1;
>>> +}
>>> +
>>> +static bool
>>> +subslice_available(const struct drm_i915_query_topology_info 
>>> *topo_info,
>>> +           int s, int ss)
>>> +{
>>> +    return (topo_info->data[topo_info->subslice_offset +
>>> +                s * topo_info->subslice_stride +
>>> +                ss / 8] >> (ss % 8)) & 1;
>>> +}
>>> +
>>> +static bool
>>> +eu_available(const struct drm_i915_query_topology_info *topo_info,
>>> +         int s, int ss, int eu)
>>> +{
>>> +    return (topo_info->data[topo_info->eu_offset +
>>> +                (s * topo_info->max_subslices + ss) * 
>>> topo_info->eu_stride +
>>> +                eu / 8] >> (eu % 8)) & 1;
>>> +}
>>> +
>>> +/* Verify that we get coherent values between the legacy getparam 
>>> slice/subslice
>>> + * masks and the new topology query.
>>> + */
>>> +static void
>>> +test_query_topology_coherent_slice_mask(int fd)
>>> +{
>>> +    struct drm_i915_query_item item;
>>> +    struct drm_i915_query_topology_info *topo_info;
>>> +    drm_i915_getparam_t gp;
>>> +    int slice_mask, subslice_mask;
>>> +    int s, topology_slices, topology_subslices_slice0;
>>> +
>>> +    gp.param = I915_PARAM_SLICE_MASK;
>>> +    gp.value = &slice_mask;
>>> +    igt_skip_on(igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0);
>>> +
>>> +    gp.param = I915_PARAM_SUBSLICE_MASK;
>>> +    gp.value = &subslice_mask;
>>> +    igt_skip_on(igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0);
>>> +
>>> +    /* Slices */
>>> +    memset(&item, 0, sizeof(item));
>>> +    item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    i915_query_items(fd, &item, 1);
>>> +    /* We expect at least one byte for each slices, subslices & EUs 
>>> masks. */
>>> +    igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
>>> +
>>> +    topo_info = malloc(item.length);
>>> +    memset(topo_info, 0, item.length);
>>> +
>>> +    item.data_ptr = to_user_pointer(topo_info);
>>> +    i915_query_items(fd, &item, 1);
>>> +    /* We expect at least one byte for each slices, subslices & EUs 
>>> masks. */
>>> +    igt_assert_lte(MIN_TOPOLOGY_ITEM_SIZE, item.length);
>>
>> I'd check length here is exactly the same as the initial query.
> 
> Adding.
> 
>>
>>> +
>>> +    topology_slices = 0;
>>> +    for (s = 0; s < topo_info->max_slices; s++) {
>>> +        if (slice_available(topo_info, s))
>>> +            topology_slices |= 1UL << s;
>>> +    }
>>> +
>>> +    igt_debug("slice mask getparam=0x%x / query=0x%x\n",
>>> +          slice_mask, topology_slices);
>>> +
>>> +    /* These 2 should always match. */
>>> +    igt_assert_eq(slice_mask, topology_slices);
>>> +
>>> +    topology_subslices_slice0 = 0;
>>> +    for (s = 0; s < topo_info->max_subslices; s++) {
>>> +        if (subslice_available(topo_info, 0, s))
>>> +            topology_subslices_slice0 |= 1UL << s;
>>> +    }
>>> +
>>> +    igt_debug("subslice mask getparam=0x%x / query=0x%x\n",
>>> +          subslice_mask, topology_subslices_slice0);
>>> +
>>> +    /* I915_PARAM_SUBSLICE_MASK returns the value for slice0, we should
>>> +     * match the values for the first slice of the topology.
>>> +     */
>>> +    igt_assert_eq(subslice_mask, topology_subslices_slice0);
>>> +
>>> +    free(topo_info);
>>> +}
>>> +
>>> +/* Verify that we get same total number of EUs from getparam and 
>>> topology query.
>>> + */
>>> +static void
>>> +test_query_topology_matches_eu_total(int fd)
>>> +{
>>> +    struct drm_i915_query_item item;
>>> +    struct drm_i915_query_topology_info *topo_info;
>>> +    drm_i915_getparam_t gp;
>>> +    int n_eus, n_eus_topology, s;
>>> +
>>> +    gp.param = I915_PARAM_EU_TOTAL;
>>> +    gp.value = &n_eus;
>>> +    do_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
>>> +    igt_debug("n_eus=%i\n", n_eus);
>>> +
>>> +    memset(&item, 0, sizeof(item));
>>> +    item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    i915_query_items(fd, &item, 1);
>>> +
>>> +    topo_info = (struct drm_i915_query_topology_info *) calloc(1, 
>>> item.length);
>>
>> Oookay.. but above you use malloc and memset. Doesn't matter I guess.
> 
> Changing to calloc everywhere. (Just a left over of the outside bounds 
> writing test).
> 
>>
>>> +
>>> +    item.data_ptr = to_user_pointer(topo_info);
>>> +    i915_query_items(fd, &item, 1);
>>> +
>>> +    igt_debug("max_slices=%hu max_subslices=%hu 
>>> max_eus_per_subslice=%hu\n",
>>> +          topo_info->max_slices, topo_info->max_subslices,
>>> +          topo_info->max_eus_per_subslice);
>>> +    igt_debug(" subslice_offset=%hu subslice_stride=%hu\n",
>>> +          topo_info->subslice_offset, topo_info->subslice_stride);
>>> +    igt_debug(" eu_offset=%hu eu_stride=%hu\n",
>>> +          topo_info->eu_offset, topo_info->eu_stride);
>>> +
>>> +    n_eus_topology = 0;
>>> +    for (s = 0; s < topo_info->max_slices; s++) {
>>> +        int ss;
>>> +
>>> +        igt_debug("slice%i:\n", s);
>>> +
>>> +        for (ss = 0; ss < topo_info->max_subslices; ss++) {
>>> +            int eu, n_subslice_eus = 0;
>>> +
>>> +            igt_debug("\tsubslice: %i\n", ss);
>>> +
>>> +            igt_debug("\t\teu_mask: 0b");
>>> +            for (eu = 0; eu < topo_info->max_eus_per_subslice; eu++) {
>>> +                uint8_t val = eu_available(topo_info, s, ss,
>>> + topo_info->max_eus_per_subslice - 1 - eu);
>>> +                igt_debug("%hhi", val);
>>> +                n_subslice_eus += __builtin_popcount(val);
>>> +                n_eus_topology += __builtin_popcount(val);
>>> +            }
>>> +
>>> +            igt_debug(" (%i)\n", n_subslice_eus);
>>> +
>>> +            /* Sanity checks. */
>>> +            if (n_subslice_eus > 0) {
>>> +                igt_assert(slice_available(topo_info, s));
>>> +                igt_assert(subslice_available(topo_info, s, ss));
>>> +            }
>>> +            if (subslice_available(topo_info, s, ss)) {
>>> +                igt_assert(slice_available(topo_info, s));
>>> +            }
>>> +        }
>>> +    }
>>> +
>>> +    free(topo_info);
>>> +
>>> +    igt_assert(n_eus_topology == n_eus);
>>> +}
>>> +
>>> +/* Verify some numbers on Gens that we know for sure the 
>>> characteristics from
>>> + * the PCI ids.
>>> + */
>>> +static void
>>> +test_query_topology_known_pci_ids(int fd, int devid)
>>> +{
>>> +    const struct intel_device_info *dev_info = 
>>> intel_get_device_info(devid);
>>> +    struct drm_i915_query_item item;
>>> +    struct drm_i915_query_topology_info *topo_info;
>>> +    int n_slices = 0, n_subslices = 0;
>>> +    int s, ss;
>>> +
>>> +    /* The GT size on some Broadwell skus is not defined, skip 
>>> those. */
>>> +    igt_skip_on(dev_info->gt == 0);
>>> +
>>> +    memset(&item, 0, sizeof(item));
>>> +    item.query_id = DRM_I915_QUERY_TOPOLOGY_INFO;
>>> +    i915_query_items(fd, &item, 1);
>>> +
>>> +    topo_info = (struct drm_i915_query_topology_info *) calloc(1, 
>>> item.length);
>>> +
>>> +    item.data_ptr = to_user_pointer(topo_info);
>>> +    i915_query_items(fd, &item, 1);
>>> +
>>> +    for (s = 0; s < topo_info->max_slices; s++) {
>>> +        if (slice_available(topo_info, s))
>>> +            n_slices++;
>>> +
>>> +        for (ss = 0; ss < topo_info->max_subslices; ss++) {
>>> +            if (subslice_available(topo_info, s, ss))
>>> +                n_subslices++;
>>> +        }
>>> +    }
>>> +
>>> +    switch (dev_info->gt) {
>>> +    case 1:
>>> +        igt_assert_eq(n_slices, 1);
>>> +        igt_assert(n_subslices == 2 || n_subslices == 3);
>>> +        break;
>>> +    case 2:
>>> +        igt_assert_eq(n_slices, 1);
>>> +        igt_assert_eq(n_subslices, 3);
>>> +        break;
>>> +    case 3:
>>> +        igt_assert_eq(n_slices, 2);
>>> +        igt_assert_eq(n_subslices, 6);
>>> +        break;
>>> +    case 4:
>>> +        igt_assert_eq(n_slices, 3);
>>> +        igt_assert_eq(n_subslices, 6);
>>> +        break;
>>> +    default:
>>> +        igt_assert(false);
>>> +    }
>>> +
>>> +    free(topo_info);
>>> +}
>>> +
>>> +igt_main
>>> +{
>>> +    int fd = -1;
>>> +    int devid;
>>> +
>>> +    igt_fixture {
>>> +        fd = drm_open_driver(DRIVER_INTEL);
>>> +        igt_require(has_query_supports(fd));
>>> +        devid = intel_get_drm_devid(fd);
>>> +    }
>>> +
>>> +    igt_subtest("query-garbage")
>>> +        test_query_garbage(fd);
>>> +
>>> +    igt_subtest("query-topology-kernel-writes") {
>>> +        igt_require(query_topology_supported(fd));
>>> +        test_query_topology_kernel_writes(fd);
>>> +    }
>>> +
>>> +    igt_subtest("query-topology-unsupported") {
>>> +        igt_require(!query_topology_supported(fd));
>>> +        test_query_topology_unsupported(fd);
>>> +    }
>>> +
>>> +    igt_subtest("query-topology-coherent-slice-mask") {
>>> +        igt_require(query_topology_supported(fd));
>>> +        test_query_topology_coherent_slice_mask(fd);
>>> +    }
>>> +
>>> +    igt_subtest("query-topology-matches-eu-total") {
>>> +        igt_require(query_topology_supported(fd));
>>> +        test_query_topology_matches_eu_total(fd);
>>> +    }
>>> +
>>> +    igt_subtest("query-topology-haswell") {
>>> +        igt_require(query_topology_supported(fd));
>>> +        igt_require(IS_HASWELL(devid) || IS_BROADWELL(devid) ||
>>> +                IS_SKYLAKE(devid) || IS_KABYLAKE(devid) ||
>>> +                IS_COFFEELAKE(devid));
>>
>> So it's possible, great!
> 
> Fingers crossed. I wouldn't be surprised to see weird things in the wild...
> 
>>
>>> + test_query_topology_known_pci_ids(fd, devid);
>>> +    }
>>> +
>>> +    igt_fixture {
>>> +        close(fd);
>>> +    }
>>> +}
>>> diff --git a/tests/meson.build b/tests/meson.build
>>> index 58729231..5788dfd0 100644
>>> --- a/tests/meson.build
>>> +++ b/tests/meson.build
>>> @@ -143,6 +143,7 @@ test_progs = [
>>>       'gen3_render_tiledx_blits',
>>>       'gen3_render_tiledy_blits',
>>>       'gvt_basic',
>>> +    'i915_query',
>>>       'kms_3d',
>>>       'kms_addfb_basic',
>>>       'kms_atomic',
>>>
>>
>> AFAIR only a few missing corner cases to test, the rest are either 
>> style or similar comments.
>>
>> I gather you did not like my suggestion to abstract the query item 
>> testing to be able to test many invalid combinations easily, since you 
>> did not comment on it?
> 
> It felt like more code than what I've written here.
> Maybe that will change in the future.

Not sure it would be more code, but definitely more and easier coverage. 
But OK. Above should be good enough in the next iteration.

> 
> Thanks a lot for your time on this!

Np, feel free to ping next time if one part of the story got neglected.

Regards,

Tvrtko

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v8 3/3] tests: add i915 query tests
  2018-03-09 14:21       ` Tvrtko Ursulin
@ 2018-03-09 14:27         ` Chris Wilson
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2018-03-09 14:27 UTC (permalink / raw)
  To: Tvrtko Ursulin, Lionel Landwerlin, igt-dev

Quoting Tvrtko Ursulin (2018-03-09 14:21:45)
> 
> On 09/03/2018 14:07, Lionel Landwerlin wrote:
> > On 09/03/18 13:23, Tvrtko Ursulin wrote:
> >>> +    /* Test a couple of invalid pointers. */
> >>> +    i915_query_items_err(fd, (void *) ULONG_MAX, 1, EFAULT);
> >>> +    i915_query_items_err(fd, (void *) 0, 1, EFAULT);
> >>
> >> Also need one where struct drm_i915_query itself it invalid or not 
> >> mapped.
> > 
> > Doesn't the tests below (unmapping the items pointer) cover that case?
> 
> Don't think so. You need to hit the failure in :
> 
> +int i915_query_ioctl(struct drm_device *dev, void *data, struct 
> drm_file *file)
> +{
> +       struct drm_i915_private *dev_priv = to_i915(dev);
> +       struct drm_i915_query *args = data;
> +       struct drm_i915_query_item __user *user_item_ptr =
> +               u64_to_user_ptr(args->items_ptr);
> 
> This dereference here. Or is DRM core already covering for that? I mean 
> the fact...
> 
> +static int
> +__i915_query_items(int fd, struct drm_i915_query_item *items, uint32_t 
> n_items)
> +{
> +    struct drm_i915_query q = {
> +        .num_items = n_items,
> +        .items_ptr = to_user_pointer(items),
> +    };
> +    return __i915_query(fd, &q);
> 
> ... &q always points to good memory?

Yes, in this case struct drm_i915_query is already checked and copied by
drm_ioctl() onto the stack/heap.

Sensible suggestion for a test though, it's one that is rarely checked.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-03-09 14:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-09  9:31 [igt-dev] [PATCH i-g-t v8 1/3] include: bump drm uAPI headers Lionel Landwerlin
2018-03-09  9:31 ` [igt-dev] [PATCH i-g-t v8 2/3] intel_chipsets: store GT information in device info Lionel Landwerlin
2018-03-09 10:02   ` Chris Wilson
2018-03-09  9:31 ` [igt-dev] [PATCH i-g-t v8 3/3] tests: add i915 query tests Lionel Landwerlin
2018-03-09 13:23   ` Tvrtko Ursulin
2018-03-09 14:07     ` Lionel Landwerlin
2018-03-09 14:21       ` Tvrtko Ursulin
2018-03-09 14:27         ` Chris Wilson
2018-03-09  9:51 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v8,1/3] include: bump drm uAPI headers Patchwork
2018-03-09 11:05 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.