[igt-dev] [PATCH i-g-t v3 06/12] lib/intel_pat: add helpers for common pat_index modes

[Niranjana Vishwanathapura]

On Mon, Oct 16, 2023 at 03:14:44PM +0100, Matthew Auld wrote:
>For now just add uc, wt and wb for every platform. The wb mode should
>always be at least 1way coherent, if messing around with system memory.
>Also make non-matching platforms throw an error rather than trying to
>inherit the modes from previous platforms since they will likely be
>different.
>
>Signed-off-by: Matthew Auld <matthew.auld at intel.com>
>Cc: José Roberto de Souza <jose.souza at intel.com>
>Cc: Pallavi Mishra <pallavi.mishra at intel.com>
>---
> lib/intel_pat.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++
> lib/intel_pat.h | 19 ++++++++++++
> lib/meson.build |  1 +
> 3 files changed, 97 insertions(+)
> create mode 100644 lib/intel_pat.c
> create mode 100644 lib/intel_pat.h
>
>diff --git a/lib/intel_pat.c b/lib/intel_pat.c
>new file mode 100644
>index 000000000..2b892ee52
>--- /dev/null
>+++ b/lib/intel_pat.c
>@@ -0,0 +1,77 @@
>+// SPDX-License-Identifier: MIT
>+/*
>+ * Copyright © 2023 Intel Corporation
>+ */
>+
>+#include "intel_pat.h"
>+
>+#include "igt.h"
>+
>+struct intel_pat_cache {
>+	uint8_t uc; /* UC + COH_NONE */
>+	uint8_t wt; /* WT + COH_NONE */
>+	uint8_t wb; /* WB + COH_AT_LEAST_1WAY */
>+
>+	uint8_t max_index;
>+};
>+
>+static void intel_get_pat_idx(int fd, struct intel_pat_cache *pat)
>+{
>+	uint16_t dev_id = intel_get_drm_devid(fd);
>+
>+	if (intel_get_device_info(dev_id)->graphics_ver == 20) {
>+		pat->uc = 3;
>+		pat->wt = 15; /* Compressed + WB-transient */
>+		pat->wb = 2;
>+		pat->max_index = 31;
>+	} else if (IS_METEORLAKE(dev_id)) {
>+		pat->uc = 2;
>+		pat->wt = 1;
>+		pat->wb = 3;
>+		pat->max_index = 3;
>+	} else if (IS_PONTEVECCHIO(dev_id)) {
>+		pat->uc = 0;
>+		pat->wt = 2;
>+		pat->wb = 3;
>+		pat->max_index = 7;
>+	} else if (intel_graphics_ver(dev_id) <= IP_VER(12, 60)) {
>+		pat->uc = 3;
>+		pat->wt = 2;
>+		pat->wb = 0;
>+		pat->max_index = 3;
>+	} else {
>+		igt_critical("Platform is missing PAT settings for uc/wt/wb\n");
>+	}
>+}
>+
>+uint8_t intel_get_max_pat_index(int fd)
>+{
>+	struct intel_pat_cache pat = {};
>+
>+	intel_get_pat_idx(fd, &pat);
>+	return pat.max_index;
>+}
>+
>+uint8_t intel_get_pat_idx_uc(int fd)
>+{
>+	struct intel_pat_cache pat = {};
>+
>+	intel_get_pat_idx(fd, &pat);
>+	return pat.uc;
>+}
>+
>+uint8_t intel_get_pat_idx_wt(int fd)
>+{
>+	struct intel_pat_cache pat = {};
>+
>+	intel_get_pat_idx(fd, &pat);
>+	return pat.wt;
>+}
>+
>+uint8_t intel_get_pat_idx_wb(int fd)
>+{
>+	struct intel_pat_cache pat = {};
>+
>+	intel_get_pat_idx(fd, &pat);
>+	return pat.wb;
>+}
>diff --git a/lib/intel_pat.h b/lib/intel_pat.h
>new file mode 100644
>index 000000000..c24dbc275
>--- /dev/null
>+++ b/lib/intel_pat.h
>@@ -0,0 +1,19 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2023 Intel Corporation
>+ */
>+
>+#ifndef INTEL_PAT_H
>+#define INTEL_PAT_H
>+
>+#include <stdint.h>
>+
>+#define DEFAULT_PAT_INDEX ((uint8_t)-1) /* igt-core can pick 1way or better */

In the uapi pat_index is u16.
But here and elsewhere in tests, we are using uint8_t.
Why not use uint16_t?

Given pat_index 0 is valid, it forces user to always specify pat_index in
vm_bind call (hence this DEFAULT_PAT_INDEX). Not sure if it is worth
considering making pat_index a extension so that by not including this
extension, XeKMD will use the default value? I am fine either way and 
I understand uapi is already reviewed, but just thought of bringint it up.

Niranjana

>+
>+uint8_t intel_get_max_pat_index(int fd);
>+
>+uint8_t intel_get_pat_idx_uc(int fd);
>+uint8_t intel_get_pat_idx_wt(int fd);
>+uint8_t intel_get_pat_idx_wb(int fd);
>+
>+#endif /* INTEL_PAT_H */
>diff --git a/lib/meson.build b/lib/meson.build
>index a7bccafc3..48466a2e9 100644
>--- a/lib/meson.build
>+++ b/lib/meson.build
>@@ -64,6 +64,7 @@ lib_sources = [
> 	'intel_device_info.c',
> 	'intel_mmio.c',
> 	'intel_mocs.c',
>+	'intel_pat.c',
> 	'ioctl_wrappers.c',
> 	'media_spin.c',
> 	'media_fill.c',
>-- 
>2.41.0
>