All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define
@ 2018-11-07 15:16 Tomasz Lis
  2018-11-07 15:16 ` [PATCH v6 2/2] drm/i915/icl: Define MOCS table for Icelake Tomasz Lis
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Tomasz Lis @ 2018-11-07 15:16 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lucas De Marchi

The MOCS tables are going to be very similar across platforms.

To reduce the amount of copied code, this patch rips the common part and
puts it into a definition valid for all gen9 platforms.

v2: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
    to MOCS_ENTRIES. (Joonas)

Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> (v1)
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_mocs.c | 86 ++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 77e9871..8d08a7b 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -96,71 +96,57 @@ struct drm_i915_mocs_table {
  *       may only be updated incrementally by adding entries at the
  *       end.
  */
-static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
-	[I915_MOCS_UNCACHED] = {
-	  /* 0x00000009 */
-	  .control_value = LE_CACHEABILITY(LE_UC) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
-
-	  /* 0x0010 */
-	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
-	},
-	[I915_MOCS_PTE] = {
-	  /* 0x00000038 */
-	  .control_value = LE_CACHEABILITY(LE_PAGETABLE) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
-	  /* 0x0030 */
-	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+
+#define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf) \
+	(LE_CACHEABILITY(lecc) | LE_TGT_CACHE(tc) | \
+	LE_LRUM(lrum) | LE_AOM(daom) | LE_RSC(ersc) | LE_SCC(scc) | \
+	LE_PFM(pfm) | LE_SCF(scf))
+
+#define MOCS_L3CC_VALUE(esc, scc, l3cc) \
+	(L3_ESC(esc) | L3_SCC(scc) | L3_CACHEABILITY(l3cc))
+
+#define GEN9_MOCS_ENTRIES \
+	[I915_MOCS_UNCACHED] = { \
+	  /* 0x00000009 */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC_ELLC, \
+					      0, 0, 0, 0, 0, 0), \
+	  /* 0x0010 */ \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	}, \
+	[I915_MOCS_PTE] = { \
+	  /* 0x00000038 */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_PAGETABLE, LE_TC_LLC_ELLC, \
+					      3, 0, 0, 0, 0, 0), \
+	  /* 0x0030 */ \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
 	},
+
+static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
+	GEN9_MOCS_ENTRIES
 	[I915_MOCS_CACHED] = {
 	  /* 0x0000003b */
-	  .control_value = LE_CACHEABILITY(LE_WB) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC_ELLC,
+					      3, 0, 0, 0, 0, 0),
 	  /* 0x0030 */
-	  .l3cc_value =   L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB),
 	},
 };
 
 /* NOTE: the LE_TGT_CACHE is not used on Broxton */
 static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
-	[I915_MOCS_UNCACHED] = {
-	  /* 0x00000009 */
-	  .control_value = LE_CACHEABILITY(LE_UC) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
-
-	  /* 0x0010 */
-	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
-	},
-	[I915_MOCS_PTE] = {
-	  /* 0x00000038 */
-	  .control_value = LE_CACHEABILITY(LE_PAGETABLE) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
-
-	  /* 0x0030 */
-	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
-	},
+	GEN9_MOCS_ENTRIES
 	[I915_MOCS_CACHED] = {
 	  /* 0x00000039 */
-	  .control_value = LE_CACHEABILITY(LE_UC) |
-			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
-			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
-			   LE_PFM(0) | LE_SCF(0),
-
+	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC_ELLC,
+					      3, 0, 0, 0, 0, 0),
 	  /* 0x0030 */
-	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB),
 	},
 };
 
+#undef MOCS_CONTROL_VALUE
+#undef MOCS_L3CC_VALUE
+
 /**
  * get_mocs_settings()
  * @dev_priv:	i915 device.
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v6 2/2] drm/i915/icl: Define MOCS table for Icelake
  2018-11-07 15:16 [PATCH v6 1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define Tomasz Lis
@ 2018-11-07 15:16 ` Tomasz Lis
  2018-12-10 15:17   ` Tvrtko Ursulin
  2018-11-07 16:06 ` ✓ Fi.CI.BAT: success for series starting with [v6,1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Tomasz Lis @ 2018-11-07 15:16 UTC (permalink / raw)
  To: intel-gfx
  Cc: Adam Cetnerowski, Piotr Rozenfeld, Lucas De Marchi, Anuj Phogat,
	Mika Kuoppala

The table has been unified across OSes to minimize virtualization overhead.

The MOCS table is now published as part of bspec, and versioned. Entries
are supposed to never be modified, but new ones can be added. Adding
entries increases table version. The patch includes version 1 entries.

Meaning of each entry is now explained in bspec, and user mode clients
are expected to know what each entry means. The 3 entries used for previous
platforms are still compatible with their legacy definitions, but that is
not guaranteed to be true for future platforms.

v2: Fixed SCC values, improved commit comment (Daniele)
v3: Improved MOCS table comment (Daniele)
v4: Moved new entries below gen9 ones. Put common entries into
    definition to be used in multiple arrays. (Lucas)
v5: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
    to MOCS_ENTRIES. Switched LE_CoS to upper case. (Joonas)
v6: Removed definitions of reserved entries. (Michal)
    Increased limit of entries sent to the hardware on gen11+.

BSpec: 34007
BSpec: 560
Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> (v4)
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhi A Wang <zhi.a.wang@intel.com>
Cc: Anuj Phogat <anuj.phogat@intel.com>
Cc: Adam Cetnerowski <adam.cetnerowski@intel.com>
Cc: Piotr Rozenfeld <piotr.rozenfeld@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_mocs.c | 222 +++++++++++++++++++++++++++++++++-----
 1 file changed, 197 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 8d08a7b..4eb05c6 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -44,6 +44,8 @@ struct drm_i915_mocs_table {
 #define LE_SCC(value)		((value) << 8)
 #define LE_PFM(value)		((value) << 11)
 #define LE_SCF(value)		((value) << 14)
+#define LE_COS(value)		((value) << 15)
+#define LE_SSE(value)		((value) << 17)
 
 /* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per word */
 #define L3_ESC(value)		((value) << 0)
@@ -52,6 +54,10 @@ struct drm_i915_mocs_table {
 
 /* Helper defines */
 #define GEN9_NUM_MOCS_ENTRIES	62  /* 62 out of 64 - 63 & 64 are reserved. */
+#define GEN11_NUM_MOCS_ENTRIES	64  /* 63-64 are reserved, but configured. */
+
+#define NUM_MOCS_ENTRIES(i915) \
+	(INTEL_GEN(i915) < 11 ? GEN9_NUM_MOCS_ENTRIES : GEN11_NUM_MOCS_ENTRIES)
 
 /* (e)LLC caching options */
 #define LE_PAGETABLE		0
@@ -80,21 +86,21 @@ struct drm_i915_mocs_table {
  * LNCFCMOCS0 - LNCFCMOCS32 registers.
  *
  * These tables are intended to be kept reasonably consistent across
- * platforms. However some of the fields are not applicable to all of
- * them.
+ * HW platforms, and for ICL+, be identical across OSes. To achieve
+ * that, for Icelake and above, list of entries is published as part
+ * of bspec.
  *
  * Entries not part of the following tables are undefined as far as
- * userspace is concerned and shouldn't be relied upon.  For the time
- * being they will be implicitly initialized to the strictest caching
- * configuration (uncached) to guarantee forwards compatibility with
- * userspace programs written against more recent kernels providing
- * additional MOCS entries.
+ * userspace is concerned and shouldn't be relied upon.
+ *
+ * The last two entries are reserved by the hardware. For ICL+ they
+ * should be initialized according to bspec and never used, for older
+ * platforms they should never be written to.
  *
- * NOTE: These tables MUST start with being uncached and the length
- *       MUST be less than 63 as the last two registers are reserved
- *       by the hardware.  These tables are part of the kernel ABI and
- *       may only be updated incrementally by adding entries at the
- *       end.
+ * NOTE: These tables are part of bspec and defined as part of hardware
+ *       interface for ICL+. For older platforms, they are part of kernel
+ *       ABI. It is expected that existing entries will remain constant
+ *       and the tables will only be updated by adding new entries.
  */
 
 #define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf) \
@@ -147,6 +153,167 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
 #undef MOCS_CONTROL_VALUE
 #undef MOCS_L3CC_VALUE
 
+#define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf, cos, sse) \
+	(LE_CACHEABILITY(lecc) | LE_TGT_CACHE(tc) | \
+	LE_LRUM(lrum) | LE_AOM(daom) | LE_RSC(ersc) | LE_SCC(scc) | \
+	LE_PFM(pfm) | LE_SCF(scf) | LE_COS(cos) | LE_SSE(sse))
+
+#define MOCS_L3CC_VALUE(esc, scc, l3cc) \
+	(L3_ESC(esc) | L3_SCC(scc) | L3_CACHEABILITY(l3cc))
+
+#define GEN11_MOCS_ENTRIES \
+	[0] = { \
+	  /* Base - Uncached (Deprecated) */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC, \
+					      0, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	}, \
+	[1] = { \
+	  /* Base - L3 + LeCC:PAT (Deprecated) */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_PAGETABLE, LE_TC_LLC, \
+					      0, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[2] = { \
+	  /* Base - L3 + LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[3] = { \
+	  /* Base - Uncached */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC, \
+					      0, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	}, \
+	[4] = { \
+	  /* Base - L3 */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC, \
+					      0, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[5] = { \
+	  /* Base - LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	}, \
+	[6] = { \
+	  /* Age 0 - LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      1, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	}, \
+	[7] = { \
+	  /* Age 0 - L3 + LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      1, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[8] = { \
+	  /* Age: Don't Chg. - LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      2, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	}, \
+	[9] = { \
+	  /* Age: Don't Chg. - L3 + LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      2, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[10] = { \
+	  /* No AOM - LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 1, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	}, \
+	[11] = { \
+	  /* No AOM - L3 + LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 1, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[12] = { \
+	  /* No AOM; Age 0 - LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      1, 1, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	}, \
+	[13] = { \
+	  /* No AOM; Age 0 - L3 + LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      1, 1, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[14] = { \
+	  /* No AOM; Age:DC - LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      2, 1, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	}, \
+	[15] = { \
+	  /* No AOM; Age:DC - L3 + LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      2, 1, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[18] = { \
+	  /* Self-Snoop - L3 + LLC */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 0, 0, 0, 0, 0, 0, 3), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[19] = { \
+	  /* Skip Caching - L3 + LLC(12.5%) */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 0, 0, 7, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[20] = { \
+	  /* Skip Caching - L3 + LLC(25%) */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 0, 0, 3, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[21] = { \
+	  /* Skip Caching - L3 + LLC(50%) */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 0, 0, 1, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[22] = { \
+	  /* Skip Caching - L3 + LLC(75%) */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 0, 1, 3, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[23] = { \
+	  /* Skip Caching - L3 + LLC(87.5%) */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 0, 1, 7, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
+	}, \
+	[62] = { \
+	  /* HW Reserved - SW program but never use */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	}, \
+	[63] = { \
+	  /* HW Reserved - SW program but never use */ \
+	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
+					      3, 0, 0, 0, 0, 0, 0, 0), \
+	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
+	},
+
+static const struct drm_i915_mocs_entry icelake_mocs_table[] = {
+	GEN11_MOCS_ENTRIES
+};
+
+#undef MOCS_CONTROL_VALUE
+#undef MOCS_L3CC_VALUE
+
 /**
  * get_mocs_settings()
  * @dev_priv:	i915 device.
@@ -164,8 +331,11 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
 {
 	bool result = false;
 
-	if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv) ||
-	    IS_ICELAKE(dev_priv)) {
+	if (IS_ICELAKE(dev_priv)) {
+		table->size  = ARRAY_SIZE(icelake_mocs_table);
+		table->table = icelake_mocs_table;
+		result = true;
+	} else if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv)) {
 		table->size  = ARRAY_SIZE(skylake_mocs_table);
 		table->table = skylake_mocs_table;
 		result = true;
@@ -228,7 +398,7 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
 	if (!get_mocs_settings(dev_priv, &table))
 		return;
 
-	GEM_BUG_ON(table.size > GEN9_NUM_MOCS_ENTRIES);
+	GEM_BUG_ON(table.size > NUM_MOCS_ENTRIES(dev_priv));
 
 	for (index = 0; index < table.size; index++)
 		I915_WRITE(mocs_register(engine->id, index),
@@ -242,7 +412,7 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
 	 * Entry 0 in the table is uncached - so we are just writing
 	 * that value to all the used entries.
 	 */
-	for (; index < GEN9_NUM_MOCS_ENTRIES; index++)
+	for (; index < NUM_MOCS_ENTRIES(dev_priv); index++)
 		I915_WRITE(mocs_register(engine->id, index),
 			   table.table[0].control_value);
 }
@@ -260,18 +430,19 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
 static int emit_mocs_control_table(struct i915_request *rq,
 				   const struct drm_i915_mocs_table *table)
 {
+	struct drm_i915_private *i915 = rq->i915;
 	enum intel_engine_id engine = rq->engine->id;
 	unsigned int index;
 	u32 *cs;
 
-	if (WARN_ON(table->size > GEN9_NUM_MOCS_ENTRIES))
+	if (WARN_ON(table->size > NUM_MOCS_ENTRIES(i915)))
 		return -ENODEV;
 
-	cs = intel_ring_begin(rq, 2 + 2 * GEN9_NUM_MOCS_ENTRIES);
+	cs = intel_ring_begin(rq, 2 + 2 * NUM_MOCS_ENTRIES(i915));
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES);
+	*cs++ = MI_LOAD_REGISTER_IMM(NUM_MOCS_ENTRIES(i915));
 
 	for (index = 0; index < table->size; index++) {
 		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
@@ -286,7 +457,7 @@ static int emit_mocs_control_table(struct i915_request *rq,
 	 * Entry 0 in the table is uncached - so we are just writing
 	 * that value to all the used entries.
 	 */
-	for (; index < GEN9_NUM_MOCS_ENTRIES; index++) {
+	for (; index < NUM_MOCS_ENTRIES(i915); index++) {
 		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
 		*cs++ = table->table[0].control_value;
 	}
@@ -319,17 +490,18 @@ static inline u32 l3cc_combine(const struct drm_i915_mocs_table *table,
 static int emit_mocs_l3cc_table(struct i915_request *rq,
 				const struct drm_i915_mocs_table *table)
 {
+	struct drm_i915_private *i915 = rq->i915;
 	unsigned int i;
 	u32 *cs;
 
-	if (WARN_ON(table->size > GEN9_NUM_MOCS_ENTRIES))
+	if (WARN_ON(table->size > NUM_MOCS_ENTRIES(i915)))
 		return -ENODEV;
 
-	cs = intel_ring_begin(rq, 2 + GEN9_NUM_MOCS_ENTRIES);
+	cs = intel_ring_begin(rq, 2 + NUM_MOCS_ENTRIES(i915));
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES / 2);
+	*cs++ = MI_LOAD_REGISTER_IMM(NUM_MOCS_ENTRIES(i915) / 2);
 
 	for (i = 0; i < table->size/2; i++) {
 		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
@@ -348,7 +520,7 @@ static int emit_mocs_l3cc_table(struct i915_request *rq,
 	 * this will be uncached. Leave the last pair uninitialised as
 	 * they are reserved by the hardware.
 	 */
-	for (; i < GEN9_NUM_MOCS_ENTRIES / 2; i++) {
+	for (; i < NUM_MOCS_ENTRIES(i915) / 2; i++) {
 		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
 		*cs++ = l3cc_combine(table, 0, 0);
 	}
@@ -395,7 +567,7 @@ void intel_mocs_init_l3cc_table(struct drm_i915_private *dev_priv)
 	 * this will be uncached. Leave the last pair as initialised as
 	 * they are reserved by the hardware.
 	 */
-	for (; i < (GEN9_NUM_MOCS_ENTRIES / 2); i++)
+	for (; i < (NUM_MOCS_ENTRIES(dev_priv) / 2); i++)
 		I915_WRITE(GEN9_LNCFCMOCS(i), l3cc_combine(&table, 0, 0));
 }
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for series starting with [v6,1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define
  2018-11-07 15:16 [PATCH v6 1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define Tomasz Lis
  2018-11-07 15:16 ` [PATCH v6 2/2] drm/i915/icl: Define MOCS table for Icelake Tomasz Lis
@ 2018-11-07 16:06 ` Patchwork
  2018-11-08  1:42 ` ✓ Fi.CI.IGT: " Patchwork
  2018-12-10 15:06 ` [PATCH v6 1/2] " Tvrtko Ursulin
  3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-11-07 16:06 UTC (permalink / raw)
  To: Tomasz Lis; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v6,1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define
URL   : https://patchwork.freedesktop.org/series/52165/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5097 -> Patchwork_10753 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52165/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10753 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-cfl-8109u:       PASS -> DMESG-WARN (fdo#107345)

    igt@kms_frontbuffer_tracking@basic:
      fi-icl-u2:          PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s3:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    
    ==== Warnings ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-cfl-8109u:       DMESG-WARN (fdo#107345) -> INCOMPLETE (fdo#108126, fdo#106070)

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070
  fdo#107345 https://bugs.freedesktop.org/show_bug.cgi?id=107345
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#108126 https://bugs.freedesktop.org/show_bug.cgi?id=108126


== Participating hosts (52 -> 46) ==

  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_5097 -> Patchwork_10753

  CI_DRM_5097: c20dfc4f015dfd41246beb7d72338aa50543a5ef @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4711: cc41f4c921e56c62c85ec5349c47022ae9b5f008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10753: 6a470e249bc3a98312c2c3bfa0d7b8cc7baa1f38 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6a470e249bc3 drm/i915/icl: Define MOCS table for Icelake
06a685031088 drm/i915/skl: Rework MOCS tables to keep common part in a define

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for series starting with [v6,1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define
  2018-11-07 15:16 [PATCH v6 1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define Tomasz Lis
  2018-11-07 15:16 ` [PATCH v6 2/2] drm/i915/icl: Define MOCS table for Icelake Tomasz Lis
  2018-11-07 16:06 ` ✓ Fi.CI.BAT: success for series starting with [v6,1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define Patchwork
@ 2018-11-08  1:42 ` Patchwork
  2018-12-10 15:06 ` [PATCH v6 1/2] " Tvrtko Ursulin
  3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-11-08  1:42 UTC (permalink / raw)
  To: Tomasz Lis; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v6,1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define
URL   : https://patchwork.freedesktop.org/series/52165/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5097_full -> Patchwork_10753_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10753_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10753_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10753_full:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10753_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-hsw:          PASS -> INCOMPLETE (fdo#106886, fdo#103540)
      shard-glk:          PASS -> INCOMPLETE (fdo#106886, fdo#103359, k.org#198133)

    igt@gem_cpu_reloc@full:
      shard-skl:          PASS -> INCOMPLETE (fdo#108073)

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956) +1

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
      shard-apl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_cursor_crc@cursor-128x128-random:
      shard-skl:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-128x42-sliding:
      shard-apl:          NOTRUN -> FAIL (fdo#103232) +1

    igt@kms_cursor_crc@cursor-64x64-random:
      shard-apl:          PASS -> FAIL (fdo#103232)

    igt@kms_draw_crc@draw-method-rgb565-blt-xtiled:
      shard-skl:          PASS -> FAIL (fdo#103184) +1

    igt@kms_flip@bo-too-big:
      shard-kbl:          PASS -> DMESG-WARN (fdo#105602, fdo#103558) +7

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
      shard-apl:          PASS -> FAIL (fdo#103167) +3

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-msflip-blt:
      shard-skl:          PASS -> FAIL (fdo#105682)

    igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-blt:
      shard-skl:          PASS -> FAIL (fdo#103167) +1

    igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
      shard-skl:          PASS -> FAIL (fdo#108145, fdo#107815)

    igt@kms_plane_alpha_blend@pipe-b-alpha-transparant-fb:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +1

    igt@kms_properties@connector-properties-atomic:
      shard-skl:          NOTRUN -> FAIL (fdo#108642)

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    igt@kms_universal_plane@universal-plane-pipe-a-functional:
      shard-glk:          PASS -> FAIL (fdo#103166)

    igt@perf@blocking:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    
    ==== Possible fixes ====

    igt@gem_ctx_isolation@vecs0-s3:
      shard-skl:          INCOMPLETE (fdo#107773, fdo#104108) -> PASS

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
      shard-skl:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_color@pipe-a-ctm-max:
      shard-apl:          FAIL (fdo#108147) -> PASS

    igt@kms_cursor_crc@cursor-256x256-random:
      shard-apl:          FAIL (fdo#103232) -> PASS +6

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-apl:          FAIL (fdo#103232, fdo#103191) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-skl:          FAIL (fdo#105363) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-render:
      shard-skl:          FAIL (fdo#105682) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
      shard-apl:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-glk:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-pwrite:
      shard-skl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS +1

    igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
      shard-skl:          FAIL (fdo#107815) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
      shard-glk:          FAIL (fdo#103166) -> PASS

    igt@kms_vblank@pipe-a-accuracy-idle:
      shard-skl:          FAIL (fdo#102583) -> PASS

    igt@pm_rpm@dpms-lpsp:
      shard-skl:          INCOMPLETE (fdo#107807) -> PASS

    
    ==== Warnings ====

    igt@gem_userptr_blits@stress-mm-invalidate-close-overlap:
      shard-apl:          DMESG-WARN -> INCOMPLETE (fdo#103927)

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108147 https://bugs.freedesktop.org/show_bug.cgi?id=108147
  fdo#108642 https://bugs.freedesktop.org/show_bug.cgi?id=108642
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5097 -> Patchwork_10753

  CI_DRM_5097: c20dfc4f015dfd41246beb7d72338aa50543a5ef @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4711: cc41f4c921e56c62c85ec5349c47022ae9b5f008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10753: 6a470e249bc3a98312c2c3bfa0d7b8cc7baa1f38 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

* Re: [PATCH v6 1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define
  2018-11-07 15:16 [PATCH v6 1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define Tomasz Lis
                   ` (2 preceding siblings ...)
  2018-11-08  1:42 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-12-10 15:06 ` Tvrtko Ursulin
  3 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2018-12-10 15:06 UTC (permalink / raw)
  To: Tomasz Lis, intel-gfx; +Cc: Lucas De Marchi


On 07/11/2018 15:16, Tomasz Lis wrote:
> The MOCS tables are going to be very similar across platforms.
> 
> To reduce the amount of copied code, this patch rips the common part and
> puts it into a definition valid for all gen9 platforms.
> 
> v2: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
>      to MOCS_ENTRIES. (Joonas)
> 
> Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
> Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> (v1)

R-b needs to be upgraded to v2 before merge.

> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_mocs.c | 86 ++++++++++++++++-----------------------
>   1 file changed, 36 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 77e9871..8d08a7b 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -96,71 +96,57 @@ struct drm_i915_mocs_table {
>    *       may only be updated incrementally by adding entries at the
>    *       end.
>    */
> -static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
> -	[I915_MOCS_UNCACHED] = {
> -	  /* 0x00000009 */
> -	  .control_value = LE_CACHEABILITY(LE_UC) |
> -			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> -			   LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
> -			   LE_PFM(0) | LE_SCF(0),
> -
> -	  /* 0x0010 */
> -	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
> -	},
> -	[I915_MOCS_PTE] = {
> -	  /* 0x00000038 */
> -	  .control_value = LE_CACHEABILITY(LE_PAGETABLE) |
> -			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> -			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
> -			   LE_PFM(0) | LE_SCF(0),
> -	  /* 0x0030 */
> -	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +
> +#define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf) \
> +	(LE_CACHEABILITY(lecc) | LE_TGT_CACHE(tc) | \
> +	LE_LRUM(lrum) | LE_AOM(daom) | LE_RSC(ersc) | LE_SCC(scc) | \
> +	LE_PFM(pfm) | LE_SCF(scf))
> +
> +#define MOCS_L3CC_VALUE(esc, scc, l3cc) \
> +	(L3_ESC(esc) | L3_SCC(scc) | L3_CACHEABILITY(l3cc))

These two macros do not seem more readable than the previous code, since 
one has to reference the macro to remind himself what is what. But never 
mind, I am only here because Tomasz copied me on a ping email. So only a 
reminder to upgrade the r-b.

Regards,

Tvrtko

> +
> +#define GEN9_MOCS_ENTRIES \
> +	[I915_MOCS_UNCACHED] = { \
> +	  /* 0x00000009 */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC_ELLC, \
> +					      0, 0, 0, 0, 0, 0), \
> +	  /* 0x0010 */ \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[I915_MOCS_PTE] = { \
> +	  /* 0x00000038 */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_PAGETABLE, LE_TC_LLC_ELLC, \
> +					      3, 0, 0, 0, 0, 0), \
> +	  /* 0x0030 */ \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
>   	},
> +
> +static const struct drm_i915_mocs_entry skylake_mocs_table[] = {
> +	GEN9_MOCS_ENTRIES
>   	[I915_MOCS_CACHED] = {
>   	  /* 0x0000003b */
> -	  .control_value = LE_CACHEABILITY(LE_WB) |
> -			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> -			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
> -			   LE_PFM(0) | LE_SCF(0),
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC_ELLC,
> +					      3, 0, 0, 0, 0, 0),
>   	  /* 0x0030 */
> -	  .l3cc_value =   L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB),
>   	},
>   };
>   
>   /* NOTE: the LE_TGT_CACHE is not used on Broxton */
>   static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
> -	[I915_MOCS_UNCACHED] = {
> -	  /* 0x00000009 */
> -	  .control_value = LE_CACHEABILITY(LE_UC) |
> -			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> -			   LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
> -			   LE_PFM(0) | LE_SCF(0),
> -
> -	  /* 0x0010 */
> -	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC),
> -	},
> -	[I915_MOCS_PTE] = {
> -	  /* 0x00000038 */
> -	  .control_value = LE_CACHEABILITY(LE_PAGETABLE) |
> -			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> -			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
> -			   LE_PFM(0) | LE_SCF(0),
> -
> -	  /* 0x0030 */
> -	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> -	},
> +	GEN9_MOCS_ENTRIES
>   	[I915_MOCS_CACHED] = {
>   	  /* 0x00000039 */
> -	  .control_value = LE_CACHEABILITY(LE_UC) |
> -			   LE_TGT_CACHE(LE_TC_LLC_ELLC) |
> -			   LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) |
> -			   LE_PFM(0) | LE_SCF(0),
> -
> +	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC_ELLC,
> +					      3, 0, 0, 0, 0, 0),
>   	  /* 0x0030 */
> -	  .l3cc_value =    L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB),
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB),
>   	},
>   };
>   
> +#undef MOCS_CONTROL_VALUE
> +#undef MOCS_L3CC_VALUE
> +
>   /**
>    * get_mocs_settings()
>    * @dev_priv:	i915 device.
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v6 2/2] drm/i915/icl: Define MOCS table for Icelake
  2018-11-07 15:16 ` [PATCH v6 2/2] drm/i915/icl: Define MOCS table for Icelake Tomasz Lis
@ 2018-12-10 15:17   ` Tvrtko Ursulin
  2018-12-13 13:46     ` Joonas Lahtinen
  0 siblings, 1 reply; 8+ messages in thread
From: Tvrtko Ursulin @ 2018-12-10 15:17 UTC (permalink / raw)
  To: Tomasz Lis, intel-gfx
  Cc: Adam Cetnerowski, Mika Kuoppala, Anuj Phogat, Lucas De Marchi,
	Piotr Rozenfeld


On 07/11/2018 15:16, Tomasz Lis wrote:
> The table has been unified across OSes to minimize virtualization overhead.
> 
> The MOCS table is now published as part of bspec, and versioned. Entries
> are supposed to never be modified, but new ones can be added. Adding
> entries increases table version. The patch includes version 1 entries.
> 
> Meaning of each entry is now explained in bspec, and user mode clients
> are expected to know what each entry means. The 3 entries used for previous
> platforms are still compatible with their legacy definitions, but that is
> not guaranteed to be true for future platforms.
> 
> v2: Fixed SCC values, improved commit comment (Daniele)
> v3: Improved MOCS table comment (Daniele)
> v4: Moved new entries below gen9 ones. Put common entries into
>      definition to be used in multiple arrays. (Lucas)
> v5: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
>      to MOCS_ENTRIES. Switched LE_CoS to upper case. (Joonas)
> v6: Removed definitions of reserved entries. (Michal)
>      Increased limit of entries sent to the hardware on gen11+.
> 
> BSpec: 34007
> BSpec: 560
> Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> (v4)

R-b upgrade needed here as well.

> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi A Wang <zhi.a.wang@intel.com>
> Cc: Anuj Phogat <anuj.phogat@intel.com>
> Cc: Adam Cetnerowski <adam.cetnerowski@intel.com>
> Cc: Piotr Rozenfeld <piotr.rozenfeld@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_mocs.c | 222 +++++++++++++++++++++++++++++++++-----
>   1 file changed, 197 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 8d08a7b..4eb05c6 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -44,6 +44,8 @@ struct drm_i915_mocs_table {
>   #define LE_SCC(value)		((value) << 8)
>   #define LE_PFM(value)		((value) << 11)
>   #define LE_SCF(value)		((value) << 14)
> +#define LE_COS(value)		((value) << 15)
> +#define LE_SSE(value)		((value) << 17)
>   
>   /* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per word */
>   #define L3_ESC(value)		((value) << 0)
> @@ -52,6 +54,10 @@ struct drm_i915_mocs_table {
>   
>   /* Helper defines */
>   #define GEN9_NUM_MOCS_ENTRIES	62  /* 62 out of 64 - 63 & 64 are reserved. */
> +#define GEN11_NUM_MOCS_ENTRIES	64  /* 63-64 are reserved, but configured. */
> +
> +#define NUM_MOCS_ENTRIES(i915) \
> +	(INTEL_GEN(i915) < 11 ? GEN9_NUM_MOCS_ENTRIES : GEN11_NUM_MOCS_ENTRIES)

I have a suggestion to make this a bit more elegant by avoiding a 
sprinkle of conditionals throughout the code - since I count ten 
non-debug call sites of this macros.

Since the MOCS code seems nicely driven from struct drm_i915_mocs_table, 
the suggestion is to store both the used and maximum valid number of 
entries per platform in that structure.

It's all nicely consolidated in get_mocs_settings, where all the sanity 
checks could be moved as well. Other bits of the code would then just 
trust the table.

>   
>   /* (e)LLC caching options */
>   #define LE_PAGETABLE		0
> @@ -80,21 +86,21 @@ struct drm_i915_mocs_table {
>    * LNCFCMOCS0 - LNCFCMOCS32 registers.
>    *
>    * These tables are intended to be kept reasonably consistent across
> - * platforms. However some of the fields are not applicable to all of
> - * them.
> + * HW platforms, and for ICL+, be identical across OSes. To achieve
> + * that, for Icelake and above, list of entries is published as part
> + * of bspec.
>    *
>    * Entries not part of the following tables are undefined as far as
> - * userspace is concerned and shouldn't be relied upon.  For the time
> - * being they will be implicitly initialized to the strictest caching
> - * configuration (uncached) to guarantee forwards compatibility with
> - * userspace programs written against more recent kernels providing
> - * additional MOCS entries.
> + * userspace is concerned and shouldn't be relied upon.
> + *
> + * The last two entries are reserved by the hardware. For ICL+ they
> + * should be initialized according to bspec and never used, for older
> + * platforms they should never be written to.
>    *
> - * NOTE: These tables MUST start with being uncached and the length
> - *       MUST be less than 63 as the last two registers are reserved
> - *       by the hardware.  These tables are part of the kernel ABI and
> - *       may only be updated incrementally by adding entries at the
> - *       end.
> + * NOTE: These tables are part of bspec and defined as part of hardware
> + *       interface for ICL+. For older platforms, they are part of kernel
> + *       ABI. It is expected that existing entries will remain constant
> + *       and the tables will only be updated by adding new entries.
>    */
>   
>   #define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf) \
> @@ -147,6 +153,167 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
>   #undef MOCS_CONTROL_VALUE
>   #undef MOCS_L3CC_VALUE
>   
> +#define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf, cos, sse) \
> +	(LE_CACHEABILITY(lecc) | LE_TGT_CACHE(tc) | \
> +	LE_LRUM(lrum) | LE_AOM(daom) | LE_RSC(ersc) | LE_SCC(scc) | \
> +	LE_PFM(pfm) | LE_SCF(scf) | LE_COS(cos) | LE_SSE(sse))
> +
> +#define MOCS_L3CC_VALUE(esc, scc, l3cc) \
> +	(L3_ESC(esc) | L3_SCC(scc) | L3_CACHEABILITY(l3cc))
> +
> +#define GEN11_MOCS_ENTRIES \
> +	[0] = { \
> +	  /* Base - Uncached (Deprecated) */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC, \
> +					      0, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[1] = { \
> +	  /* Base - L3 + LeCC:PAT (Deprecated) */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_PAGETABLE, LE_TC_LLC, \
> +					      0, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[2] = { \
> +	  /* Base - L3 + LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[3] = { \
> +	  /* Base - Uncached */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC, \
> +					      0, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[4] = { \
> +	  /* Base - L3 */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC, \
> +					      0, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[5] = { \
> +	  /* Base - LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[6] = { \
> +	  /* Age 0 - LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      1, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[7] = { \
> +	  /* Age 0 - L3 + LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      1, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[8] = { \
> +	  /* Age: Don't Chg. - LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      2, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[9] = { \
> +	  /* Age: Don't Chg. - L3 + LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      2, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[10] = { \
> +	  /* No AOM - LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 1, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[11] = { \
> +	  /* No AOM - L3 + LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 1, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[12] = { \
> +	  /* No AOM; Age 0 - LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      1, 1, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[13] = { \
> +	  /* No AOM; Age 0 - L3 + LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      1, 1, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[14] = { \
> +	  /* No AOM; Age:DC - LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      2, 1, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[15] = { \
> +	  /* No AOM; Age:DC - L3 + LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      2, 1, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[18] = { \
> +	  /* Self-Snoop - L3 + LLC */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 0, 0, 0, 0, 0, 0, 3), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[19] = { \
> +	  /* Skip Caching - L3 + LLC(12.5%) */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 0, 0, 7, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[20] = { \
> +	  /* Skip Caching - L3 + LLC(25%) */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 0, 0, 3, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[21] = { \
> +	  /* Skip Caching - L3 + LLC(50%) */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 0, 0, 1, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[22] = { \
> +	  /* Skip Caching - L3 + LLC(75%) */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 0, 1, 3, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[23] = { \
> +	  /* Skip Caching - L3 + LLC(87.5%) */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 0, 1, 7, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_WB), \
> +	}, \
> +	[62] = { \
> +	  /* HW Reserved - SW program but never use */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	}, \
> +	[63] = { \
> +	  /* HW Reserved - SW program but never use */ \
> +	  .control_value = MOCS_CONTROL_VALUE(LE_WB, LE_TC_LLC, \
> +					      3, 0, 0, 0, 0, 0, 0, 0), \
> +	  .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
> +	},
> +
> +static const struct drm_i915_mocs_entry icelake_mocs_table[] = {
> +	GEN11_MOCS_ENTRIES
> +};
> +
> +#undef MOCS_CONTROL_VALUE
> +#undef MOCS_L3CC_VALUE
> +
>   /**
>    * get_mocs_settings()
>    * @dev_priv:	i915 device.
> @@ -164,8 +331,11 @@ static bool get_mocs_settings(struct drm_i915_private *dev_priv,
>   {
>   	bool result = false;
>   
> -	if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv) ||
> -	    IS_ICELAKE(dev_priv)) {
> +	if (IS_ICELAKE(dev_priv)) {
> +		table->size  = ARRAY_SIZE(icelake_mocs_table);
> +		table->table = icelake_mocs_table;
> +		result = true;
> +	} else if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv)) {
>   		table->size  = ARRAY_SIZE(skylake_mocs_table);
>   		table->table = skylake_mocs_table;
>   		result = true;

So initialize table max here as well.

> @@ -228,7 +398,7 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
>   	if (!get_mocs_settings(dev_priv, &table))
>   		return;
>   
> -	GEM_BUG_ON(table.size > GEN9_NUM_MOCS_ENTRIES);
> +	GEM_BUG_ON(table.size > NUM_MOCS_ENTRIES(dev_priv));

Then this can go.

>   
>   	for (index = 0; index < table.size; index++)
>   		I915_WRITE(mocs_register(engine->id, index),
> @@ -242,7 +412,7 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
>   	 * Entry 0 in the table is uncached - so we are just writing
>   	 * that value to all the used entries.
>   	 */
> -	for (; index < GEN9_NUM_MOCS_ENTRIES; index++)
> +	for (; index < NUM_MOCS_ENTRIES(dev_priv); index++)
This can use table->max (or something).

>   		I915_WRITE(mocs_register(engine->id, index),
>   			   table.table[0].control_value);
>   }
> @@ -260,18 +430,19 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
>   static int emit_mocs_control_table(struct i915_request *rq,
>   				   const struct drm_i915_mocs_table *table)
>   {
> +	struct drm_i915_private *i915 = rq->i915;
>   	enum intel_engine_id engine = rq->engine->id;
>   	unsigned int index;
>   	u32 *cs;
>   
> -	if (WARN_ON(table->size > GEN9_NUM_MOCS_ENTRIES))
> +	if (WARN_ON(table->size > NUM_MOCS_ENTRIES(i915)))
>   		return -ENODEV;

This can then go as well. Or at least it can be GEM_WARN_ON since it 
cannot leave development in the broken state.

>   
> -	cs = intel_ring_begin(rq, 2 + 2 * GEN9_NUM_MOCS_ENTRIES);
> +	cs = intel_ring_begin(rq, 2 + 2 * NUM_MOCS_ENTRIES(i915));
>   	if (IS_ERR(cs))
>   		return PTR_ERR(cs);
>   
> -	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES);
> +	*cs++ = MI_LOAD_REGISTER_IMM(NUM_MOCS_ENTRIES(i915));

These two can be table->max as well then.

Etc to the end.

See what you think.

Regards,

Tvrtko

>   
>   	for (index = 0; index < table->size; index++) {
>   		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
> @@ -286,7 +457,7 @@ static int emit_mocs_control_table(struct i915_request *rq,
>   	 * Entry 0 in the table is uncached - so we are just writing
>   	 * that value to all the used entries.
>   	 */
> -	for (; index < GEN9_NUM_MOCS_ENTRIES; index++) {
> +	for (; index < NUM_MOCS_ENTRIES(i915); index++) {
>   		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
>   		*cs++ = table->table[0].control_value;
>   	}
> @@ -319,17 +490,18 @@ static inline u32 l3cc_combine(const struct drm_i915_mocs_table *table,
>   static int emit_mocs_l3cc_table(struct i915_request *rq,
>   				const struct drm_i915_mocs_table *table)
>   {
> +	struct drm_i915_private *i915 = rq->i915;
>   	unsigned int i;
>   	u32 *cs;
>   
> -	if (WARN_ON(table->size > GEN9_NUM_MOCS_ENTRIES))
> +	if (WARN_ON(table->size > NUM_MOCS_ENTRIES(i915)))
>   		return -ENODEV;
>   
> -	cs = intel_ring_begin(rq, 2 + GEN9_NUM_MOCS_ENTRIES);
> +	cs = intel_ring_begin(rq, 2 + NUM_MOCS_ENTRIES(i915));
>   	if (IS_ERR(cs))
>   		return PTR_ERR(cs);
>   
> -	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES / 2);
> +	*cs++ = MI_LOAD_REGISTER_IMM(NUM_MOCS_ENTRIES(i915) / 2);
>   
>   	for (i = 0; i < table->size/2; i++) {
>   		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
> @@ -348,7 +520,7 @@ static int emit_mocs_l3cc_table(struct i915_request *rq,
>   	 * this will be uncached. Leave the last pair uninitialised as
>   	 * they are reserved by the hardware.
>   	 */
> -	for (; i < GEN9_NUM_MOCS_ENTRIES / 2; i++) {
> +	for (; i < NUM_MOCS_ENTRIES(i915) / 2; i++) {
>   		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i));
>   		*cs++ = l3cc_combine(table, 0, 0);
>   	}
> @@ -395,7 +567,7 @@ void intel_mocs_init_l3cc_table(struct drm_i915_private *dev_priv)
>   	 * this will be uncached. Leave the last pair as initialised as
>   	 * they are reserved by the hardware.
>   	 */
> -	for (; i < (GEN9_NUM_MOCS_ENTRIES / 2); i++)
> +	for (; i < (NUM_MOCS_ENTRIES(dev_priv) / 2); i++)
>   		I915_WRITE(GEN9_LNCFCMOCS(i), l3cc_combine(&table, 0, 0));
>   }
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v6 2/2] drm/i915/icl: Define MOCS table for Icelake
  2018-12-10 15:17   ` Tvrtko Ursulin
@ 2018-12-13 13:46     ` Joonas Lahtinen
  2018-12-13 19:45       ` Lucas De Marchi
  0 siblings, 1 reply; 8+ messages in thread
From: Joonas Lahtinen @ 2018-12-13 13:46 UTC (permalink / raw)
  To: Tomasz Lis, Tvrtko Ursulin, intel-gfx
  Cc: Adam Cetnerowski, Piotr Rozenfeld, Anuj Phogat, Lucas De Marchi,
	Mika Kuoppala

Quoting Tvrtko Ursulin (2018-12-10 17:17:29)
> 
> On 07/11/2018 15:16, Tomasz Lis wrote:
> > The table has been unified across OSes to minimize virtualization overhead.
> > 
> > The MOCS table is now published as part of bspec, and versioned. Entries
> > are supposed to never be modified, but new ones can be added. Adding
> > entries increases table version. The patch includes version 1 entries.
> > 
> > Meaning of each entry is now explained in bspec, and user mode clients
> > are expected to know what each entry means. The 3 entries used for previous
> > platforms are still compatible with their legacy definitions, but that is
> > not guaranteed to be true for future platforms.
> > 
> > v2: Fixed SCC values, improved commit comment (Daniele)
> > v3: Improved MOCS table comment (Daniele)
> > v4: Moved new entries below gen9 ones. Put common entries into
> >      definition to be used in multiple arrays. (Lucas)
> > v5: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
> >      to MOCS_ENTRIES. Switched LE_CoS to upper case. (Joonas)
> > v6: Removed definitions of reserved entries. (Michal)
> >      Increased limit of entries sent to the hardware on gen11+.
> > 
> > BSpec: 34007
> > BSpec: 560
> > Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
> > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> (v4)
> 
> R-b upgrade needed here as well.
> 
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: Zhi A Wang <zhi.a.wang@intel.com>
> > Cc: Anuj Phogat <anuj.phogat@intel.com>
> > Cc: Adam Cetnerowski <adam.cetnerowski@intel.com>
> > Cc: Piotr Rozenfeld <piotr.rozenfeld@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_mocs.c | 222 +++++++++++++++++++++++++++++++++-----
> >   1 file changed, 197 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> > index 8d08a7b..4eb05c6 100644
> > --- a/drivers/gpu/drm/i915/intel_mocs.c
> > +++ b/drivers/gpu/drm/i915/intel_mocs.c
> > @@ -44,6 +44,8 @@ struct drm_i915_mocs_table {
> >   #define LE_SCC(value)               ((value) << 8)
> >   #define LE_PFM(value)               ((value) << 11)
> >   #define LE_SCF(value)               ((value) << 14)
> > +#define LE_COS(value)                ((value) << 15)
> > +#define LE_SSE(value)                ((value) << 17)
> >   
> >   /* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per word */
> >   #define L3_ESC(value)               ((value) << 0)
> > @@ -52,6 +54,10 @@ struct drm_i915_mocs_table {
> >   
> >   /* Helper defines */
> >   #define GEN9_NUM_MOCS_ENTRIES       62  /* 62 out of 64 - 63 & 64 are reserved. */
> > +#define GEN11_NUM_MOCS_ENTRIES       64  /* 63-64 are reserved, but configured. */
> > +
> > +#define NUM_MOCS_ENTRIES(i915) \
> > +     (INTEL_GEN(i915) < 11 ? GEN9_NUM_MOCS_ENTRIES : GEN11_NUM_MOCS_ENTRIES)
> 
> I have a suggestion to make this a bit more elegant by avoiding a 
> sprinkle of conditionals throughout the code - since I count ten 
> non-debug call sites of this macros.
> 
> Since the MOCS code seems nicely driven from struct drm_i915_mocs_table, 
> the suggestion is to store both the used and maximum valid number of 
> entries per platform in that structure.
> 
> It's all nicely consolidated in get_mocs_settings, where all the sanity 
> checks could be moved as well. Other bits of the code would then just 
> trust the table.
> 
> >   
> >   /* (e)LLC caching options */
> >   #define LE_PAGETABLE                0
> > @@ -80,21 +86,21 @@ struct drm_i915_mocs_table {
> >    * LNCFCMOCS0 - LNCFCMOCS32 registers.
> >    *
> >    * These tables are intended to be kept reasonably consistent across
> > - * platforms. However some of the fields are not applicable to all of
> > - * them.
> > + * HW platforms, and for ICL+, be identical across OSes. To achieve
> > + * that, for Icelake and above, list of entries is published as part
> > + * of bspec.
> >    *
> >    * Entries not part of the following tables are undefined as far as
> > - * userspace is concerned and shouldn't be relied upon.  For the time
> > - * being they will be implicitly initialized to the strictest caching
> > - * configuration (uncached) to guarantee forwards compatibility with
> > - * userspace programs written against more recent kernels providing
> > - * additional MOCS entries.
> > + * userspace is concerned and shouldn't be relied upon.
> > + *
> > + * The last two entries are reserved by the hardware. For ICL+ they
> > + * should be initialized according to bspec and never used, for older
> > + * platforms they should never be written to.
> >    *
> > - * NOTE: These tables MUST start with being uncached and the length
> > - *       MUST be less than 63 as the last two registers are reserved
> > - *       by the hardware.  These tables are part of the kernel ABI and
> > - *       may only be updated incrementally by adding entries at the
> > - *       end.
> > + * NOTE: These tables are part of bspec and defined as part of hardware
> > + *       interface for ICL+. For older platforms, they are part of kernel
> > + *       ABI. It is expected that existing entries will remain constant
> > + *       and the tables will only be updated by adding new entries.
> >    */
> >   
> >   #define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf) \
> > @@ -147,6 +153,167 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
> >   #undef MOCS_CONTROL_VALUE
> >   #undef MOCS_L3CC_VALUE
> >   
> > +#define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf, cos, sse) \
> > +     (LE_CACHEABILITY(lecc) | LE_TGT_CACHE(tc) | \
> > +     LE_LRUM(lrum) | LE_AOM(daom) | LE_RSC(ersc) | LE_SCC(scc) | \
> > +     LE_PFM(pfm) | LE_SCF(scf) | LE_COS(cos) | LE_SSE(sse))
> > +
> > +#define MOCS_L3CC_VALUE(esc, scc, l3cc) \
> > +     (L3_ESC(esc) | L3_SCC(scc) | L3_CACHEABILITY(l3cc))
> > +
> > +#define GEN11_MOCS_ENTRIES \
> > +     [0] = { \
> > +       /* Base - Uncached (Deprecated) */ \
> > +       .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC, \
> > +                                           0, 0, 0, 0, 0, 0, 0, 0), \
> > +       .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \

These indents don't follow the guidelines. Make sure checkpatch
doesn't complain.

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v6 2/2] drm/i915/icl: Define MOCS table for Icelake
  2018-12-13 13:46     ` Joonas Lahtinen
@ 2018-12-13 19:45       ` Lucas De Marchi
  0 siblings, 0 replies; 8+ messages in thread
From: Lucas De Marchi @ 2018-12-13 19:45 UTC (permalink / raw)
  To: Joonas Lahtinen
  Cc: adam.cetnerowski, intel-gfx, Lucas De Marchi, piotr.rozenfeld,
	anuj.phogat, Mika Kuoppala

On Thu, Dec 13, 2018 at 5:46 AM Joonas Lahtinen
<joonas.lahtinen@linux.intel.com> wrote:
>
> Quoting Tvrtko Ursulin (2018-12-10 17:17:29)
> >
> > On 07/11/2018 15:16, Tomasz Lis wrote:
> > > The table has been unified across OSes to minimize virtualization overhead.
> > >
> > > The MOCS table is now published as part of bspec, and versioned. Entries
> > > are supposed to never be modified, but new ones can be added. Adding
> > > entries increases table version. The patch includes version 1 entries.
> > >
> > > Meaning of each entry is now explained in bspec, and user mode clients
> > > are expected to know what each entry means. The 3 entries used for previous
> > > platforms are still compatible with their legacy definitions, but that is
> > > not guaranteed to be true for future platforms.
> > >
> > > v2: Fixed SCC values, improved commit comment (Daniele)
> > > v3: Improved MOCS table comment (Daniele)
> > > v4: Moved new entries below gen9 ones. Put common entries into
> > >      definition to be used in multiple arrays. (Lucas)
> > > v5: Made defines for or-ing flags. Renamed macros from MOCS_TABLE
> > >      to MOCS_ENTRIES. Switched LE_CoS to upper case. (Joonas)
> > > v6: Removed definitions of reserved entries. (Michal)
> > >      Increased limit of entries sent to the hardware on gen11+.
> > >
> > > BSpec: 34007
> > > BSpec: 560
> > > Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
> > > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> (v4)
> >
> > R-b upgrade needed here as well.
> >
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > Cc: Zhi A Wang <zhi.a.wang@intel.com>
> > > Cc: Anuj Phogat <anuj.phogat@intel.com>
> > > Cc: Adam Cetnerowski <adam.cetnerowski@intel.com>
> > > Cc: Piotr Rozenfeld <piotr.rozenfeld@intel.com>
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/intel_mocs.c | 222 +++++++++++++++++++++++++++++++++-----
> > >   1 file changed, 197 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> > > index 8d08a7b..4eb05c6 100644
> > > --- a/drivers/gpu/drm/i915/intel_mocs.c
> > > +++ b/drivers/gpu/drm/i915/intel_mocs.c
> > > @@ -44,6 +44,8 @@ struct drm_i915_mocs_table {
> > >   #define LE_SCC(value)               ((value) << 8)
> > >   #define LE_PFM(value)               ((value) << 11)
> > >   #define LE_SCF(value)               ((value) << 14)
> > > +#define LE_COS(value)                ((value) << 15)
> > > +#define LE_SSE(value)                ((value) << 17)
> > >
> > >   /* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per word */
> > >   #define L3_ESC(value)               ((value) << 0)
> > > @@ -52,6 +54,10 @@ struct drm_i915_mocs_table {
> > >
> > >   /* Helper defines */
> > >   #define GEN9_NUM_MOCS_ENTRIES       62  /* 62 out of 64 - 63 & 64 are reserved. */
> > > +#define GEN11_NUM_MOCS_ENTRIES       64  /* 63-64 are reserved, but configured. */
> > > +
> > > +#define NUM_MOCS_ENTRIES(i915) \
> > > +     (INTEL_GEN(i915) < 11 ? GEN9_NUM_MOCS_ENTRIES : GEN11_NUM_MOCS_ENTRIES)
> >
> > I have a suggestion to make this a bit more elegant by avoiding a
> > sprinkle of conditionals throughout the code - since I count ten
> > non-debug call sites of this macros.
> >
> > Since the MOCS code seems nicely driven from struct drm_i915_mocs_table,
> > the suggestion is to store both the used and maximum valid number of
> > entries per platform in that structure.
> >
> > It's all nicely consolidated in get_mocs_settings, where all the sanity
> > checks could be moved as well. Other bits of the code would then just
> > trust the table.
> >
> > >
> > >   /* (e)LLC caching options */
> > >   #define LE_PAGETABLE                0
> > > @@ -80,21 +86,21 @@ struct drm_i915_mocs_table {
> > >    * LNCFCMOCS0 - LNCFCMOCS32 registers.
> > >    *
> > >    * These tables are intended to be kept reasonably consistent across
> > > - * platforms. However some of the fields are not applicable to all of
> > > - * them.
> > > + * HW platforms, and for ICL+, be identical across OSes. To achieve
> > > + * that, for Icelake and above, list of entries is published as part
> > > + * of bspec.
> > >    *
> > >    * Entries not part of the following tables are undefined as far as
> > > - * userspace is concerned and shouldn't be relied upon.  For the time
> > > - * being they will be implicitly initialized to the strictest caching
> > > - * configuration (uncached) to guarantee forwards compatibility with
> > > - * userspace programs written against more recent kernels providing
> > > - * additional MOCS entries.
> > > + * userspace is concerned and shouldn't be relied upon.
> > > + *
> > > + * The last two entries are reserved by the hardware. For ICL+ they
> > > + * should be initialized according to bspec and never used, for older
> > > + * platforms they should never be written to.
> > >    *
> > > - * NOTE: These tables MUST start with being uncached and the length
> > > - *       MUST be less than 63 as the last two registers are reserved
> > > - *       by the hardware.  These tables are part of the kernel ABI and
> > > - *       may only be updated incrementally by adding entries at the
> > > - *       end.
> > > + * NOTE: These tables are part of bspec and defined as part of hardware
> > > + *       interface for ICL+. For older platforms, they are part of kernel
> > > + *       ABI. It is expected that existing entries will remain constant
> > > + *       and the tables will only be updated by adding new entries.
> > >    */
> > >
> > >   #define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf) \
> > > @@ -147,6 +153,167 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = {
> > >   #undef MOCS_CONTROL_VALUE
> > >   #undef MOCS_L3CC_VALUE
> > >
> > > +#define MOCS_CONTROL_VALUE(lecc, tc, lrum, daom, ersc, scc, pfm, scf, cos, sse) \
> > > +     (LE_CACHEABILITY(lecc) | LE_TGT_CACHE(tc) | \
> > > +     LE_LRUM(lrum) | LE_AOM(daom) | LE_RSC(ersc) | LE_SCC(scc) | \
> > > +     LE_PFM(pfm) | LE_SCF(scf) | LE_COS(cos) | LE_SSE(sse))
> > > +
> > > +#define MOCS_L3CC_VALUE(esc, scc, l3cc) \
> > > +     (L3_ESC(esc) | L3_SCC(scc) | L3_CACHEABILITY(l3cc))
> > > +
> > > +#define GEN11_MOCS_ENTRIES \
> > > +     [0] = { \
> > > +       /* Base - Uncached (Deprecated) */ \
> > > +       .control_value = MOCS_CONTROL_VALUE(LE_UC, LE_TC_LLC, \
> > > +                                           0, 0, 0, 0, 0, 0, 0, 0), \
> > > +       .l3cc_value = MOCS_L3CC_VALUE(0, 0, L3_UC), \
>
> These indents don't follow the guidelines. Make sure checkpatch
> doesn't complain.

yes, it passes.

$ dim checkpatch  drm-tip/drm-tip..HEAD  drm-intel
e02fab02e635 drm/i915/skl: Rework MOCS tables to keep common part in a define
261119c69063 drm/i915/icl: Define MOCS table for Icelake

This being inside a macro is probably what makes checkpatch to ignore
the indentation.

Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-12-13 19:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-07 15:16 [PATCH v6 1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define Tomasz Lis
2018-11-07 15:16 ` [PATCH v6 2/2] drm/i915/icl: Define MOCS table for Icelake Tomasz Lis
2018-12-10 15:17   ` Tvrtko Ursulin
2018-12-13 13:46     ` Joonas Lahtinen
2018-12-13 19:45       ` Lucas De Marchi
2018-11-07 16:06 ` ✓ Fi.CI.BAT: success for series starting with [v6,1/2] drm/i915/skl: Rework MOCS tables to keep common part in a define Patchwork
2018-11-08  1:42 ` ✓ Fi.CI.IGT: " Patchwork
2018-12-10 15:06 ` [PATCH v6 1/2] " Tvrtko Ursulin

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.