[PATCH 0/2] Improve whitelist selftest for read-only registers

[PATCH 0/2] Improve whitelist selftest for read-only registers

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Follow up patch to earlier whitelist updates. This series adds some
extra sanity checking to the driver and improves the self-test.

John Harrison (2):
  drm/i915: Add test for invalid flag bits in whitelist entries
  drm/i915: Implement read-only support in whitelist selftest

 drivers/gpu/drm/i915/gt/intel_workarounds.c   | 29 ++++++++--
 .../gpu/drm/i915/gt/selftest_workarounds.c    | 57 +++++++++++++------
 drivers/gpu/drm/i915/i915_reg.h               | 12 +++-
 3 files changed, 73 insertions(+), 25 deletions(-)

-- 
2.21.0.5.gaeb582a983

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

[PATCH 1/2] drm/i915: Add test for invalid flag bits in whitelist entries

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

As per review feedback by Tvrtko, added a check that no invalid bits
are being set in the whitelist flags fields.

Also updated the read/write access definitions to make it clearer that
they are an enum field not a set of single bit flags.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c   | 29 +++++++++++++++----
 .../gpu/drm/i915/gt/selftest_workarounds.c    | 14 ++++++---
 drivers/gpu/drm/i915/i915_reg.h               | 12 ++++++--
 3 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index a908d829d6bd..9b401833aceb 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1011,6 +1011,20 @@ bool intel_gt_verify_workarounds(struct intel_gt *gt, const char *from)
 	return wa_list_verify(gt->uncore, &gt->i915->gt_wa_list, from);
 }
 
+static inline bool is_nonpriv_flags_valid(u32 flags)
+{
+	/* Check only valid flag bits are set */
+	if (flags & ~RING_FORCE_TO_NONPRIV_MASK_VALID)
+		return false;
+
+	/* NB: Only 3 out of 4 enum values are valid for access field */
+	if ((flags & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
+	    RING_FORCE_TO_NONPRIV_ACCESS_INVALID)
+		return false;
+
+	return true;
+}
+
 static void
 whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
 {
@@ -1021,6 +1035,9 @@ whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
 	if (GEM_DEBUG_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
 		return;
 
+	if (GEM_DEBUG_WARN_ON(!is_nonpriv_flags_valid(flags)))
+		return;
+
 	wa.reg.reg |= flags;
 	_wa_add(wal, &wa);
 }
@@ -1028,7 +1045,7 @@ whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
 static void
 whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
 {
-	whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_RW);
+	whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW);
 }
 
 static void gen9_whitelist_build(struct i915_wa_list *w)
@@ -1109,7 +1126,7 @@ static void cfl_whitelist_build(struct intel_engine_cs *engine)
 	 *   - PS_DEPTH_COUNT_UDW
 	 */
 	whitelist_reg_ext(w, PS_INVOCATION_COUNT,
-			  RING_FORCE_TO_NONPRIV_RD |
+			  RING_FORCE_TO_NONPRIV_ACCESS_RD |
 			  RING_FORCE_TO_NONPRIV_RANGE_4);
 }
 
@@ -1149,20 +1166,20 @@ static void icl_whitelist_build(struct intel_engine_cs *engine)
 		 *   - PS_DEPTH_COUNT_UDW
 		 */
 		whitelist_reg_ext(w, PS_INVOCATION_COUNT,
-				  RING_FORCE_TO_NONPRIV_RD |
+				  RING_FORCE_TO_NONPRIV_ACCESS_RD |
 				  RING_FORCE_TO_NONPRIV_RANGE_4);
 		break;
 
 	case VIDEO_DECODE_CLASS:
 		/* hucStatusRegOffset */
 		whitelist_reg_ext(w, _MMIO(0x2000 + engine->mmio_base),
-				  RING_FORCE_TO_NONPRIV_RD);
+				  RING_FORCE_TO_NONPRIV_ACCESS_RD);
 		/* hucUKernelHdrInfoRegOffset */
 		whitelist_reg_ext(w, _MMIO(0x2014 + engine->mmio_base),
-				  RING_FORCE_TO_NONPRIV_RD);
+				  RING_FORCE_TO_NONPRIV_ACCESS_RD);
 		/* hucStatus2RegOffset */
 		whitelist_reg_ext(w, _MMIO(0x23B0 + engine->mmio_base),
-				  RING_FORCE_TO_NONPRIV_RD);
+				  RING_FORCE_TO_NONPRIV_ACCESS_RD);
 		break;
 
 	default:
diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index b933d831eeb1..f8151d5946c8 100644
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -394,6 +394,10 @@ static bool wo_register(struct intel_engine_cs *engine, u32 reg)
 	enum intel_platform platform = INTEL_INFO(engine->i915)->platform;
 	int i;
 
+	if ((reg & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
+	     RING_FORCE_TO_NONPRIV_ACCESS_WR)
+		return true;
+
 	for (i = 0; i < ARRAY_SIZE(wo_registers); i++) {
 		if (wo_registers[i].platform == platform &&
 		    wo_registers[i].reg == reg)
@@ -405,7 +409,8 @@ static bool wo_register(struct intel_engine_cs *engine, u32 reg)
 
 static bool ro_register(u32 reg)
 {
-	if (reg & RING_FORCE_TO_NONPRIV_RD)
+	if ((reg & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
+	     RING_FORCE_TO_NONPRIV_ACCESS_RD)
 		return true;
 
 	return false;
@@ -757,8 +762,8 @@ static int read_whitelisted_registers(struct i915_gem_context *ctx,
 		u64 offset = results->node.start + sizeof(u32) * i;
 		u32 reg = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
 
-		/* Clear RD only and WR only flags */
-		reg &= ~(RING_FORCE_TO_NONPRIV_RD | RING_FORCE_TO_NONPRIV_WR);
+		/* Clear access permission field */
+		reg &= ~RING_FORCE_TO_NONPRIV_ACCESS_MASK;
 
 		*cs++ = srm;
 		*cs++ = reg;
@@ -928,7 +933,8 @@ check_whitelisted_registers(struct intel_engine_cs *engine,
 	for (i = 0; i < engine->whitelist.count; i++) {
 		const struct i915_wa *wa = &engine->whitelist.list[i];
 
-		if (i915_mmio_reg_offset(wa->reg) & RING_FORCE_TO_NONPRIV_RD)
+		if (i915_mmio_reg_offset(wa->reg) &
+		    RING_FORCE_TO_NONPRIV_ACCESS_RD)
 			continue;
 
 		if (!fn(engine, a[i], b[i], wa->reg))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c814cc1b3ae5..0bd4bf0b4629 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2521,13 +2521,19 @@ enum i915_power_well_id {
 #define   RING_WAIT_SEMAPHORE	(1 << 10) /* gen6+ */
 
 #define RING_FORCE_TO_NONPRIV(base, i) _MMIO(((base) + 0x4D0) + (i) * 4)
-#define   RING_FORCE_TO_NONPRIV_RW		(0 << 28)    /* CFL+ & Gen11+ */
-#define   RING_FORCE_TO_NONPRIV_RD		(1 << 28)
-#define   RING_FORCE_TO_NONPRIV_WR		(2 << 28)
+#define   RING_FORCE_TO_NONPRIV_ACCESS_RW	(0 << 28)    /* CFL+ & Gen11+ */
+#define   RING_FORCE_TO_NONPRIV_ACCESS_RD	(1 << 28)
+#define   RING_FORCE_TO_NONPRIV_ACCESS_WR	(2 << 28)
+#define   RING_FORCE_TO_NONPRIV_ACCESS_INVALID	(3 << 28)
+#define   RING_FORCE_TO_NONPRIV_ACCESS_MASK	(3 << 28)
 #define   RING_FORCE_TO_NONPRIV_RANGE_1		(0 << 0)     /* CFL+ & Gen11+ */
 #define   RING_FORCE_TO_NONPRIV_RANGE_4		(1 << 0)
 #define   RING_FORCE_TO_NONPRIV_RANGE_16	(2 << 0)
 #define   RING_FORCE_TO_NONPRIV_RANGE_64	(3 << 0)
+#define   RING_FORCE_TO_NONPRIV_RANGE_MASK	(3 << 0)
+#define   RING_FORCE_TO_NONPRIV_MASK_VALID	\
+					(RING_FORCE_TO_NONPRIV_RANGE_MASK \
+					| RING_FORCE_TO_NONPRIV_ACCESS_MASK)
 #define   RING_MAX_NONPRIV_SLOTS  12
 
 #define GEN7_TLB_RD_ADDR	_MMIO(0x4700)
-- 
2.21.0.5.gaeb582a983

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

[PATCH 2/2] drm/i915: Implement read-only support in whitelist selftest

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Newer hardware supports extra feature in the whitelist registers. This
patch updates the selftest to test that entries marked as read only
are actually read only.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 .../gpu/drm/i915/gt/selftest_workarounds.c    | 43 +++++++++++++------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index f8151d5946c8..5cd2b17105ba 100644
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -482,12 +482,12 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
 		u32 srm, lrm, rsvd;
 		u32 expect;
 		int idx;
+		bool ro_reg;
 
 		if (wo_register(engine, reg))
 			continue;
 
-		if (ro_register(reg))
-			continue;
+		ro_reg = ro_register(reg);
 
 		srm = MI_STORE_REGISTER_MEM;
 		lrm = MI_LOAD_REGISTER_MEM;
@@ -588,24 +588,37 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
 		}
 
 		GEM_BUG_ON(values[ARRAY_SIZE(values) - 1] != 0xffffffff);
-		rsvd = results[ARRAY_SIZE(values)]; /* detect write masking */
-		if (!rsvd) {
-			pr_err("%s: Unable to write to whitelisted register %x\n",
-			       engine->name, reg);
-			err = -EINVAL;
-			goto out_unpin;
+		if (ro_reg) {
+			rsvd = 0xFFFFFFFF;
+		} else {
+			/* detect write masking */
+			rsvd = results[ARRAY_SIZE(values)];
+			if (!rsvd) {
+				pr_err("%s: Unable to write to whitelisted register %x\n",
+				       engine->name, reg);
+				err = -EINVAL;
+				goto out_unpin;
+			}
 		}
 
 		expect = results[0];
 		idx = 1;
 		for (v = 0; v < ARRAY_SIZE(values); v++) {
-			expect = reg_write(expect, values[v], rsvd);
+			if (ro_reg)
+				expect = results[0];
+			else
+				expect = reg_write(expect, values[v], rsvd);
+
 			if (results[idx] != expect)
 				err++;
 			idx++;
 		}
 		for (v = 0; v < ARRAY_SIZE(values); v++) {
-			expect = reg_write(expect, ~values[v], rsvd);
+			if (ro_reg)
+				expect = results[0];
+			else
+				expect = reg_write(expect, ~values[v], rsvd);
+
 			if (results[idx] != expect)
 				err++;
 			idx++;
@@ -622,7 +635,10 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
 			for (v = 0; v < ARRAY_SIZE(values); v++) {
 				u32 w = values[v];
 
-				expect = reg_write(expect, w, rsvd);
+				if (ro_reg)
+					expect = results[0];
+				else
+					expect = reg_write(expect, w, rsvd);
 				pr_info("Wrote %08x, read %08x, expect %08x\n",
 					w, results[idx], expect);
 				idx++;
@@ -630,7 +646,10 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
 			for (v = 0; v < ARRAY_SIZE(values); v++) {
 				u32 w = ~values[v];
 
-				expect = reg_write(expect, w, rsvd);
+				if (ro_reg)
+					expect = results[0];
+				else
+					expect = reg_write(expect, w, rsvd);
 				pr_info("Wrote %08x, read %08x, expect %08x\n",
 					w, results[idx], expect);
 				idx++;
-- 
2.21.0.5.gaeb582a983

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

✓ Fi.CI.BAT: success for Improve whitelist selftest for read-only registers

[Patchwork]

== Series Details ==

Series: Improve whitelist selftest for read-only registers
URL   : https://patchwork.freedesktop.org/series/63102/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6398 -> Patchwork_13499
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/

Possible new issues
-------------------

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@kms_chamelium@common-hpd-after-suspend:
    - {fi-icl-u4}:        [DMESG-WARN][1] ([fdo#102505]) -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/fi-icl-u4/igt@kms_chamelium@common-hpd-after-suspend.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/fi-icl-u4/igt@kms_chamelium@common-hpd-after-suspend.html

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [PASS][5] -> [FAIL][6] ([fdo#103167])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@vgem_basic@debugfs:
    - fi-icl-u3:          [PASS][7] -> [DMESG-WARN][8] ([fdo#107724])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/fi-icl-u3/igt@vgem_basic@debugfs.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/fi-icl-u3/igt@vgem_basic@debugfs.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-icl-dsi:         [INCOMPLETE][9] ([fdo#107713] / [fdo#109100]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/fi-icl-dsi/igt@gem_ctx_create@basic-files.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/fi-icl-dsi/igt@gem_ctx_create@basic-files.html

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-r:           [DMESG-WARN][11] ([fdo#111012]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/fi-kbl-r/igt@i915_pm_rpm@module-reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/fi-kbl-r/igt@i915_pm_rpm@module-reload.html
    - fi-hsw-peppy:       [DMESG-WARN][13] ([fdo#111012]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/fi-hsw-peppy/igt@i915_pm_rpm@module-reload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/fi-hsw-peppy/igt@i915_pm_rpm@module-reload.html

  * igt@prime_vgem@basic-read:
    - fi-icl-u3:          [DMESG-WARN][15] ([fdo#107724]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/fi-icl-u3/igt@prime_vgem@basic-read.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/fi-icl-u3/igt@prime_vgem@basic-read.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#111012]: https://bugs.freedesktop.org/show_bug.cgi?id=111012


Participating hosts (55 -> 47)
------------------------------

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6398 -> Patchwork_13499

  CI_DRM_6398: 9b9df28dc0ec04a7fb1a020d869ef0ea14be4d14 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5079: 873df2fa9e8f5fd02d4532b30ef2579f4fe4f27f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13499: 82ce470c711a50b727ab8c902ea66505f7dbdd41 @ git://anongit.freedesktop.org/gfx-ci/linux


== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2


== Linux commits ==

82ce470c711a drm/i915: Implement read-only support in whitelist selftest
b99af47de05a drm/i915: Add test for invalid flag bits in whitelist entries

== Logs ==

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

[PATCH 3/3] drm/i915: Add engine name to workaround debug print

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

There is a debug message in the workaround initialisation path that
reports how many entries were added of each type. However, whitelist
workarounds exist for multiple engines but the type name is just
'whitelist'. Tvrtko suggested adding the engine name to make the
message more useful.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c       | 15 ++++++++-------
 drivers/gpu/drm/i915/gt/intel_workarounds_types.h |  1 +
 drivers/gpu/drm/i915/gt/selftest_workarounds.c    | 13 +++----------
 3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 9b401833aceb..cf18bb962708 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -50,9 +50,10 @@
  * - Public functions to init or apply the given workaround type.
  */
 
-static void wa_init_start(struct i915_wa_list *wal, const char *name)
+static void wa_init_start(struct i915_wa_list *wal, const char *name, const char *engine_name)
 {
 	wal->name = name;
+	wal->engine_name = engine_name;
 }
 
 #define WA_LIST_CHUNK (1 << 4)
@@ -74,8 +75,8 @@ static void wa_init_finish(struct i915_wa_list *wal)
 	if (!wal->count)
 		return;
 
-	DRM_DEBUG_DRIVER("Initialized %u %s workarounds\n",
-			 wal->wa_count, wal->name);
+	DRM_DEBUG_DRIVER("Initialized %u %s workarounds on %s\n",
+			 wal->wa_count, wal->name, wal->engine_name);
 }
 
 static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
@@ -591,7 +592,7 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
 	if (engine->class != RENDER_CLASS)
 		return;
 
-	wa_init_start(wal, name);
+	wa_init_start(wal, name, engine->name);
 
 	if (IS_GEN(i915, 11))
 		icl_ctx_workarounds_init(engine, wal);
@@ -921,7 +922,7 @@ void intel_gt_init_workarounds(struct drm_i915_private *i915)
 {
 	struct i915_wa_list *wal = &i915->gt_wa_list;
 
-	wa_init_start(wal, "GT");
+	wa_init_start(wal, "GT", "global");
 	gt_init_workarounds(i915, wal);
 	wa_init_finish(wal);
 }
@@ -1192,7 +1193,7 @@ void intel_engine_init_whitelist(struct intel_engine_cs *engine)
 	struct drm_i915_private *i915 = engine->i915;
 	struct i915_wa_list *w = &engine->whitelist;
 
-	wa_init_start(w, "whitelist");
+	wa_init_start(w, "whitelist", engine->name);
 
 	if (IS_GEN(i915, 11))
 		icl_whitelist_build(engine);
@@ -1384,7 +1385,7 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine)
 	if (GEM_WARN_ON(INTEL_GEN(engine->i915) < 8))
 		return;
 
-	wa_init_start(wal, engine->name);
+	wa_init_start(wal, "engine", engine->name);
 	engine_init_workarounds(engine, wal);
 	wa_init_finish(wal);
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
index 42ac1fb99572..e27ab1b710b3 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
@@ -20,6 +20,7 @@ struct i915_wa {
 
 struct i915_wa_list {
 	const char	*name;
+	const char	*engine_name;
 	struct i915_wa	*list;
 	unsigned int	count;
 	unsigned int	wa_count;
diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index 5cd2b17105ba..ba95d9e28f8a 100644
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -25,11 +25,9 @@ static const struct wo_register {
 	{ INTEL_GEMINILAKE, 0x731c }
 };
 
-#define REF_NAME_MAX (INTEL_ENGINE_CS_MAX_NAME + 8)
 struct wa_lists {
 	struct i915_wa_list gt_wa_list;
 	struct {
-		char name[REF_NAME_MAX];
 		struct i915_wa_list wa_list;
 		struct i915_wa_list ctx_wa_list;
 	} engine[I915_NUM_ENGINES];
@@ -43,25 +41,20 @@ reference_lists_init(struct drm_i915_private *i915, struct wa_lists *lists)
 
 	memset(lists, 0, sizeof(*lists));
 
-	wa_init_start(&lists->gt_wa_list, "GT_REF");
+	wa_init_start(&lists->gt_wa_list, "GT_REF", "global");
 	gt_init_workarounds(i915, &lists->gt_wa_list);
 	wa_init_finish(&lists->gt_wa_list);
 
 	for_each_engine(engine, i915, id) {
 		struct i915_wa_list *wal = &lists->engine[id].wa_list;
-		char *name = lists->engine[id].name;
 
-		snprintf(name, REF_NAME_MAX, "%s_REF", engine->name);
-
-		wa_init_start(wal, name);
+		wa_init_start(wal, "REF", engine->name);
 		engine_init_workarounds(engine, wal);
 		wa_init_finish(wal);
 
-		snprintf(name, REF_NAME_MAX, "%s_CTX_REF", engine->name);
-
 		__intel_engine_init_ctx_wa(engine,
 					   &lists->engine[id].ctx_wa_list,
-					   name);
+					   "CTX_REF");
 	}
 }
 
-- 
2.21.0.5.gaeb582a983

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

Re: [PATCH 2/2] drm/i915: Implement read-only support in whitelist selftest

[Chris Wilson]

Quoting John.C.Harrison@Intel.com (2019-07-03 03:06:04)
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Newer hardware supports extra feature in the whitelist registers. This
> patch updates the selftest to test that entries marked as read only
> are actually read only.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  .../gpu/drm/i915/gt/selftest_workarounds.c    | 43 +++++++++++++------
>  1 file changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index f8151d5946c8..5cd2b17105ba 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -482,12 +482,12 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>                 u32 srm, lrm, rsvd;
>                 u32 expect;
>                 int idx;
> +               bool ro_reg;
>  
>                 if (wo_register(engine, reg))
>                         continue;
>  
> -               if (ro_register(reg))
> -                       continue;
> +               ro_reg = ro_register(reg);
>  
>                 srm = MI_STORE_REGISTER_MEM;
>                 lrm = MI_LOAD_REGISTER_MEM;
> @@ -588,24 +588,37 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>                 }
>  
>                 GEM_BUG_ON(values[ARRAY_SIZE(values) - 1] != 0xffffffff);
> -               rsvd = results[ARRAY_SIZE(values)]; /* detect write masking */
> -               if (!rsvd) {
> -                       pr_err("%s: Unable to write to whitelisted register %x\n",
> -                              engine->name, reg);
> -                       err = -EINVAL;
> -                       goto out_unpin;
> +               if (ro_reg) {
> +                       rsvd = 0xFFFFFFFF;

rsvd = 0;

reg_write() will then dtrt.

Does this not replace the skip placed in check_whitelisted_registers()?

We still need a way to verify that the register exists, as even writing
from a secure batch fails (not tried ring though). Do we load a spinner,
tweak via mmio?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Improve whitelist selftest for read-only registers

[Patchwork]

== Series Details ==

Series: Improve whitelist selftest for read-only registers
URL   : https://patchwork.freedesktop.org/series/63102/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6399 -> Patchwork_13501
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap_gtt@basic-write:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-icl-u3/igt@gem_mmap_gtt@basic-write.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/fi-icl-u3/igt@gem_mmap_gtt@basic-write.html

  
#### Possible fixes ####

  * igt@gem_basic@create-close:
    - fi-icl-u3:          [DMESG-WARN][3] ([fdo#107724]) -> [PASS][4] +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-icl-u3/igt@gem_basic@create-close.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/fi-icl-u3/igt@gem_basic@create-close.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [INCOMPLETE][5] ([fdo#107718]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@kms_busy@basic-flip-c:
    - fi-skl-6770hq:      [SKIP][7] ([fdo#109271] / [fdo#109278]) -> [PASS][8] +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-skl-6770hq/igt@kms_busy@basic-flip-c.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/fi-skl-6770hq/igt@kms_busy@basic-flip-c.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-icl-u2:          [FAIL][9] ([fdo#109635 ] / [fdo#110387]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6770hq:      [SKIP][11] ([fdo#109271]) -> [PASS][12] +23 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [FAIL][13] ([fdo#103167]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 
  [fdo#110387]: https://bugs.freedesktop.org/show_bug.cgi?id=110387


Participating hosts (54 -> 47)
------------------------------

  Additional (2): fi-hsw-peppy fi-pnv-d510 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6399 -> Patchwork_13501

  CI_DRM_6399: c15e6447c0c506c058671d59082fcd710e42d6d4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5079: 873df2fa9e8f5fd02d4532b30ef2579f4fe4f27f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13501: 03ff43603e1ee1d0c5b83b0d82524a7e1a46f77a @ git://anongit.freedesktop.org/gfx-ci/linux


== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2


== Linux commits ==

03ff43603e1e drm/i915: Add engine name to workaround debug print
9b17be220071 drm/i915: Implement read-only support in whitelist selftest
0bb808b31399 drm/i915: Add test for invalid flag bits in whitelist entries

== Logs ==

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

Re: [PATCH 1/2] drm/i915: Add test for invalid flag bits in whitelist entries

[Tvrtko Ursulin]

On 03/07/2019 03:06, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> As per review feedback by Tvrtko, added a check that no invalid bits
> are being set in the whitelist flags fields.
> 
> Also updated the read/write access definitions to make it clearer that
> they are an enum field not a set of single bit flags.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_workarounds.c   | 29 +++++++++++++++----
>   .../gpu/drm/i915/gt/selftest_workarounds.c    | 14 ++++++---
>   drivers/gpu/drm/i915/i915_reg.h               | 12 ++++++--
>   3 files changed, 42 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index a908d829d6bd..9b401833aceb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1011,6 +1011,20 @@ bool intel_gt_verify_workarounds(struct intel_gt *gt, const char *from)
>   	return wa_list_verify(gt->uncore, &gt->i915->gt_wa_list, from);
>   }
>   
> +static inline bool is_nonpriv_flags_valid(u32 flags)
> +{
> +	/* Check only valid flag bits are set */
> +	if (flags & ~RING_FORCE_TO_NONPRIV_MASK_VALID)
> +		return false;
> +
> +	/* NB: Only 3 out of 4 enum values are valid for access field */
> +	if ((flags & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
> +	    RING_FORCE_TO_NONPRIV_ACCESS_INVALID)
> +		return false;
> +
> +	return true;
> +}
> +
>   static void
>   whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
>   {
> @@ -1021,6 +1035,9 @@ whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
>   	if (GEM_DEBUG_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
>   		return;
>   
> +	if (GEM_DEBUG_WARN_ON(!is_nonpriv_flags_valid(flags)))
> +		return;
> +
>   	wa.reg.reg |= flags;
>   	_wa_add(wal, &wa);
>   }
> @@ -1028,7 +1045,7 @@ whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
>   static void
>   whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
>   {
> -	whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_RW);
> +	whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW);
>   }
>   
>   static void gen9_whitelist_build(struct i915_wa_list *w)
> @@ -1109,7 +1126,7 @@ static void cfl_whitelist_build(struct intel_engine_cs *engine)
>   	 *   - PS_DEPTH_COUNT_UDW
>   	 */
>   	whitelist_reg_ext(w, PS_INVOCATION_COUNT,
> -			  RING_FORCE_TO_NONPRIV_RD |
> +			  RING_FORCE_TO_NONPRIV_ACCESS_RD |
>   			  RING_FORCE_TO_NONPRIV_RANGE_4);
>   }
>   
> @@ -1149,20 +1166,20 @@ static void icl_whitelist_build(struct intel_engine_cs *engine)
>   		 *   - PS_DEPTH_COUNT_UDW
>   		 */
>   		whitelist_reg_ext(w, PS_INVOCATION_COUNT,
> -				  RING_FORCE_TO_NONPRIV_RD |
> +				  RING_FORCE_TO_NONPRIV_ACCESS_RD |
>   				  RING_FORCE_TO_NONPRIV_RANGE_4);
>   		break;
>   
>   	case VIDEO_DECODE_CLASS:
>   		/* hucStatusRegOffset */
>   		whitelist_reg_ext(w, _MMIO(0x2000 + engine->mmio_base),
> -				  RING_FORCE_TO_NONPRIV_RD);
> +				  RING_FORCE_TO_NONPRIV_ACCESS_RD);
>   		/* hucUKernelHdrInfoRegOffset */
>   		whitelist_reg_ext(w, _MMIO(0x2014 + engine->mmio_base),
> -				  RING_FORCE_TO_NONPRIV_RD);
> +				  RING_FORCE_TO_NONPRIV_ACCESS_RD);
>   		/* hucStatus2RegOffset */
>   		whitelist_reg_ext(w, _MMIO(0x23B0 + engine->mmio_base),
> -				  RING_FORCE_TO_NONPRIV_RD);
> +				  RING_FORCE_TO_NONPRIV_ACCESS_RD);
>   		break;
>   
>   	default:
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index b933d831eeb1..f8151d5946c8 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -394,6 +394,10 @@ static bool wo_register(struct intel_engine_cs *engine, u32 reg)
>   	enum intel_platform platform = INTEL_INFO(engine->i915)->platform;
>   	int i;
>   
> +	if ((reg & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
> +	     RING_FORCE_TO_NONPRIV_ACCESS_WR)
> +		return true;
> +
>   	for (i = 0; i < ARRAY_SIZE(wo_registers); i++) {
>   		if (wo_registers[i].platform == platform &&
>   		    wo_registers[i].reg == reg)
> @@ -405,7 +409,8 @@ static bool wo_register(struct intel_engine_cs *engine, u32 reg)
>   
>   static bool ro_register(u32 reg)
>   {
> -	if (reg & RING_FORCE_TO_NONPRIV_RD)
> +	if ((reg & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
> +	     RING_FORCE_TO_NONPRIV_ACCESS_RD)
>   		return true;
>   
>   	return false;
> @@ -757,8 +762,8 @@ static int read_whitelisted_registers(struct i915_gem_context *ctx,
>   		u64 offset = results->node.start + sizeof(u32) * i;
>   		u32 reg = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
>   
> -		/* Clear RD only and WR only flags */
> -		reg &= ~(RING_FORCE_TO_NONPRIV_RD | RING_FORCE_TO_NONPRIV_WR);
> +		/* Clear access permission field */
> +		reg &= ~RING_FORCE_TO_NONPRIV_ACCESS_MASK;
>   
>   		*cs++ = srm;
>   		*cs++ = reg;
> @@ -928,7 +933,8 @@ check_whitelisted_registers(struct intel_engine_cs *engine,
>   	for (i = 0; i < engine->whitelist.count; i++) {
>   		const struct i915_wa *wa = &engine->whitelist.list[i];
>   
> -		if (i915_mmio_reg_offset(wa->reg) & RING_FORCE_TO_NONPRIV_RD)
> +		if (i915_mmio_reg_offset(wa->reg) &
> +		    RING_FORCE_TO_NONPRIV_ACCESS_RD)
>   			continue;
>   
>   		if (!fn(engine, a[i], b[i], wa->reg))
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c814cc1b3ae5..0bd4bf0b4629 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2521,13 +2521,19 @@ enum i915_power_well_id {
>   #define   RING_WAIT_SEMAPHORE	(1 << 10) /* gen6+ */
>   
>   #define RING_FORCE_TO_NONPRIV(base, i) _MMIO(((base) + 0x4D0) + (i) * 4)
> -#define   RING_FORCE_TO_NONPRIV_RW		(0 << 28)    /* CFL+ & Gen11+ */
> -#define   RING_FORCE_TO_NONPRIV_RD		(1 << 28)
> -#define   RING_FORCE_TO_NONPRIV_WR		(2 << 28)
> +#define   RING_FORCE_TO_NONPRIV_ACCESS_RW	(0 << 28)    /* CFL+ & Gen11+ */
> +#define   RING_FORCE_TO_NONPRIV_ACCESS_RD	(1 << 28)
> +#define   RING_FORCE_TO_NONPRIV_ACCESS_WR	(2 << 28)
> +#define   RING_FORCE_TO_NONPRIV_ACCESS_INVALID	(3 << 28)
> +#define   RING_FORCE_TO_NONPRIV_ACCESS_MASK	(3 << 28)
>   #define   RING_FORCE_TO_NONPRIV_RANGE_1		(0 << 0)     /* CFL+ & Gen11+ */
>   #define   RING_FORCE_TO_NONPRIV_RANGE_4		(1 << 0)
>   #define   RING_FORCE_TO_NONPRIV_RANGE_16	(2 << 0)
>   #define   RING_FORCE_TO_NONPRIV_RANGE_64	(3 << 0)
> +#define   RING_FORCE_TO_NONPRIV_RANGE_MASK	(3 << 0)
> +#define   RING_FORCE_TO_NONPRIV_MASK_VALID	\
> +					(RING_FORCE_TO_NONPRIV_RANGE_MASK \
> +					| RING_FORCE_TO_NONPRIV_ACCESS_MASK)
>   #define   RING_MAX_NONPRIV_SLOTS  12
>   
>   #define GEN7_TLB_RD_ADDR	_MMIO(0x4700)
> 

Looks okay. the only nitpick I have is that "is flags" reads funny ("are 
flags"?), but maybe it is just because I am not a native speaker.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH 3/3] drm/i915: Add engine name to workaround debug print

[Tvrtko Ursulin]

On 03/07/2019 08:50, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> There is a debug message in the workaround initialisation path that
> reports how many entries were added of each type. However, whitelist
> workarounds exist for multiple engines but the type name is just
> 'whitelist'. Tvrtko suggested adding the engine name to make the
> message more useful.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_workarounds.c       | 15 ++++++++-------
>   drivers/gpu/drm/i915/gt/intel_workarounds_types.h |  1 +
>   drivers/gpu/drm/i915/gt/selftest_workarounds.c    | 13 +++----------
>   3 files changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 9b401833aceb..cf18bb962708 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -50,9 +50,10 @@
>    * - Public functions to init or apply the given workaround type.
>    */
>   
> -static void wa_init_start(struct i915_wa_list *wal, const char *name)
> +static void wa_init_start(struct i915_wa_list *wal, const char *name, const char *engine_name)
>   {
>   	wal->name = name;
> +	wal->engine_name = engine_name;
>   }
>   
>   #define WA_LIST_CHUNK (1 << 4)
> @@ -74,8 +75,8 @@ static void wa_init_finish(struct i915_wa_list *wal)
>   	if (!wal->count)
>   		return;
>   
> -	DRM_DEBUG_DRIVER("Initialized %u %s workarounds\n",
> -			 wal->wa_count, wal->name);
> +	DRM_DEBUG_DRIVER("Initialized %u %s workarounds on %s\n",
> +			 wal->wa_count, wal->name, wal->engine_name);
>   }
>   
>   static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
> @@ -591,7 +592,7 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs *engine,
>   	if (engine->class != RENDER_CLASS)
>   		return;
>   
> -	wa_init_start(wal, name);
> +	wa_init_start(wal, name, engine->name);
>   
>   	if (IS_GEN(i915, 11))
>   		icl_ctx_workarounds_init(engine, wal);
> @@ -921,7 +922,7 @@ void intel_gt_init_workarounds(struct drm_i915_private *i915)
>   {
>   	struct i915_wa_list *wal = &i915->gt_wa_list;
>   
> -	wa_init_start(wal, "GT");
> +	wa_init_start(wal, "GT", "global");
>   	gt_init_workarounds(i915, wal);
>   	wa_init_finish(wal);
>   }
> @@ -1192,7 +1193,7 @@ void intel_engine_init_whitelist(struct intel_engine_cs *engine)
>   	struct drm_i915_private *i915 = engine->i915;
>   	struct i915_wa_list *w = &engine->whitelist;
>   
> -	wa_init_start(w, "whitelist");
> +	wa_init_start(w, "whitelist", engine->name);
>   
>   	if (IS_GEN(i915, 11))
>   		icl_whitelist_build(engine);
> @@ -1384,7 +1385,7 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine)
>   	if (GEM_WARN_ON(INTEL_GEN(engine->i915) < 8))
>   		return;
>   
> -	wa_init_start(wal, engine->name);
> +	wa_init_start(wal, "engine", engine->name);
>   	engine_init_workarounds(engine, wal);
>   	wa_init_finish(wal);
>   }
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
> index 42ac1fb99572..e27ab1b710b3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h
> @@ -20,6 +20,7 @@ struct i915_wa {
>   
>   struct i915_wa_list {
>   	const char	*name;
> +	const char	*engine_name;
>   	struct i915_wa	*list;
>   	unsigned int	count;
>   	unsigned int	wa_count;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index 5cd2b17105ba..ba95d9e28f8a 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -25,11 +25,9 @@ static const struct wo_register {
>   	{ INTEL_GEMINILAKE, 0x731c }
>   };
>   
> -#define REF_NAME_MAX (INTEL_ENGINE_CS_MAX_NAME + 8)
>   struct wa_lists {
>   	struct i915_wa_list gt_wa_list;
>   	struct {
> -		char name[REF_NAME_MAX];
>   		struct i915_wa_list wa_list;
>   		struct i915_wa_list ctx_wa_list;
>   	} engine[I915_NUM_ENGINES];
> @@ -43,25 +41,20 @@ reference_lists_init(struct drm_i915_private *i915, struct wa_lists *lists)
>   
>   	memset(lists, 0, sizeof(*lists));
>   
> -	wa_init_start(&lists->gt_wa_list, "GT_REF");
> +	wa_init_start(&lists->gt_wa_list, "GT_REF", "global");
>   	gt_init_workarounds(i915, &lists->gt_wa_list);
>   	wa_init_finish(&lists->gt_wa_list);
>   
>   	for_each_engine(engine, i915, id) {
>   		struct i915_wa_list *wal = &lists->engine[id].wa_list;
> -		char *name = lists->engine[id].name;
>   
> -		snprintf(name, REF_NAME_MAX, "%s_REF", engine->name);
> -
> -		wa_init_start(wal, name);
> +		wa_init_start(wal, "REF", engine->name);
>   		engine_init_workarounds(engine, wal);
>   		wa_init_finish(wal);
>   
> -		snprintf(name, REF_NAME_MAX, "%s_CTX_REF", engine->name);
> -
>   		__intel_engine_init_ctx_wa(engine,
>   					   &lists->engine[id].ctx_wa_list,
> -					   name);
> +					   "CTX_REF");
>   	}
>   }
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH 2/2] drm/i915: Implement read-only support in whitelist selftest

[John Harrison]

On 7/3/2019 01:32, Chris Wilson wrote:
> Quoting John.C.Harrison@Intel.com (2019-07-03 03:06:04)
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> Newer hardware supports extra feature in the whitelist registers. This
>> patch updates the selftest to test that entries marked as read only
>> are actually read only.
>>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   .../gpu/drm/i915/gt/selftest_workarounds.c    | 43 +++++++++++++------
>>   1 file changed, 31 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
>> index f8151d5946c8..5cd2b17105ba 100644
>> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
>> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
>> @@ -482,12 +482,12 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>>                  u32 srm, lrm, rsvd;
>>                  u32 expect;
>>                  int idx;
>> +               bool ro_reg;
>>   
>>                  if (wo_register(engine, reg))
>>                          continue;
>>   
>> -               if (ro_register(reg))
>> -                       continue;
>> +               ro_reg = ro_register(reg);
>>   
>>                  srm = MI_STORE_REGISTER_MEM;
>>                  lrm = MI_LOAD_REGISTER_MEM;
>> @@ -588,24 +588,37 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>>                  }
>>   
>>                  GEM_BUG_ON(values[ARRAY_SIZE(values) - 1] != 0xffffffff);
>> -               rsvd = results[ARRAY_SIZE(values)]; /* detect write masking */
>> -               if (!rsvd) {
>> -                       pr_err("%s: Unable to write to whitelisted register %x\n",
>> -                              engine->name, reg);
>> -                       err = -EINVAL;
>> -                       goto out_unpin;
>> +               if (ro_reg) {
>> +                       rsvd = 0xFFFFFFFF;
> rsvd = 0;
>
> reg_write() will then dtrt.
It seemed too suspiciously broken to have the test claim a read-only 
register was successfully written to. This way makes it clear that the 
test expects read-only to always return the first value read.

> Does this not replace the skip placed in check_whitelisted_registers()?
The two versions of that test looks like they need to be able to set 
values. So they can't be run on read-only registers.

> We still need a way to verify that the register exists, as even writing
> from a secure batch fails (not tried ring though). Do we load a spinner,
> tweak via mmio?

I don't think there is a reliable, generic mechanism to test that you 
can actually read from a read only register. You need to know what 
content it should provide. Even the current test (that it always returns 
the same value) would break if the register changes dynamically (e.g. 
it's a hardware counter).

John.


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

✓ Fi.CI.IGT: success for Improve whitelist selftest for read-only registers

[Patchwork]

== Series Details ==

Series: Improve whitelist selftest for read-only registers
URL   : https://patchwork.freedesktop.org/series/63102/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6398_full -> Patchwork_13499_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@system-suspend-modeset:
    - shard-skl:          [PASS][1] -> [INCOMPLETE][2] ([fdo#104108] / [fdo#107807])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-skl9/igt@i915_pm_rpm@system-suspend-modeset.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-skl1/igt@i915_pm_rpm@system-suspend-modeset.html

  * igt@i915_selftest@mock_requests:
    - shard-skl:          [PASS][3] -> [INCOMPLETE][4] ([fdo#110550])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-skl5/igt@i915_selftest@mock_requests.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-skl5/igt@i915_selftest@mock_requests.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         [PASS][5] -> [FAIL][6] ([fdo#103167]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-apl6/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-apl3/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][9] -> [FAIL][10] ([fdo#108341])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-iclb2/igt@kms_psr@no_drrs.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_suspend:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#109441]) +2 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-iclb2/igt@kms_psr@psr2_suspend.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-iclb3/igt@kms_psr@psr2_suspend.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][13] -> [FAIL][14] ([fdo#99912])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-apl1/igt@kms_setmode@basic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-apl7/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-10ms:
    - shard-glk:          [DMESG-WARN][15] -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-glk5/igt@gem_eio@in-flight-10ms.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-glk2/igt@gem_eio@in-flight-10ms.html

  * igt@i915_pm_rps@min-max-config-idle:
    - shard-iclb:         [INCOMPLETE][17] ([fdo#107713]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-iclb1/igt@i915_pm_rps@min-max-config-idle.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-iclb6/igt@i915_pm_rps@min-max-config-idle.html

  * igt@i915_suspend@debugfs-reader:
    - shard-apl:          [DMESG-WARN][19] ([fdo#108566]) -> [PASS][20] +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-apl6/igt@i915_suspend@debugfs-reader.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-apl2/igt@i915_suspend@debugfs-reader.html

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][21] ([fdo#103355]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-atomic.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-hsw5/igt@kms_cursor_legacy@cursor-vs-flip-atomic.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [INCOMPLETE][23] ([fdo#104108]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-skl6/igt@kms_fbcon_fbt@psr-suspend.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-skl4/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][25] ([fdo#105363]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-glk3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-glk4/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-iclb:         [FAIL][27] ([fdo#103167]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][29] ([fdo#103166]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@perf@oa-exponents:
    - shard-glk:          [FAIL][31] ([fdo#105483]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6398/shard-glk6/igt@perf@oa-exponents.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13499/shard-glk6/igt@perf@oa-exponents.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105483]: https://bugs.freedesktop.org/show_bug.cgi?id=105483
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110550]: https://bugs.freedesktop.org/show_bug.cgi?id=110550
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_6398 -> Patchwork_13499

  CI_DRM_6398: 9b9df28dc0ec04a7fb1a020d869ef0ea14be4d14 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5079: 873df2fa9e8f5fd02d4532b30ef2579f4fe4f27f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13499: 82ce470c711a50b727ab8c902ea66505f7dbdd41 @ 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_13499/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Improve whitelist selftest for read-only registers

[Patchwork]

== Series Details ==

Series: Improve whitelist selftest for read-only registers
URL   : https://patchwork.freedesktop.org/series/63102/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6399_full -> Patchwork_13501_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@i2c:
    - shard-hsw:          [PASS][1] -> [FAIL][2] ([fdo#104097])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-hsw7/igt@i915_pm_rpm@i2c.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-hsw1/igt@i915_pm_rpm@i2c.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([fdo#108566]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-apl4/igt@i915_suspend@fence-restore-tiled2untiled.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-apl8/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-apl:          [PASS][5] -> [FAIL][6] ([fdo#103060])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-apl6/igt@kms_flip@modeset-vs-vblank-race.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-apl7/igt@kms_flip@modeset-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         [PASS][7] -> [FAIL][8] ([fdo#103167]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][9] -> [FAIL][10] ([fdo#103166]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_suspend:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#109441])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb2/igt@kms_psr@psr2_suspend.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-iclb5/igt@kms_psr@psr2_suspend.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][13] -> [FAIL][14] ([fdo#99912])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-apl6/igt@kms_setmode@basic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-apl7/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-iclb:         [INCOMPLETE][15] ([fdo#107713]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb3/igt@gem_eio@in-flight-suspend.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-iclb6/igt@gem_eio@in-flight-suspend.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][17] ([fdo#104108]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-skl10/igt@gem_softpin@noreloc-s3.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-skl3/igt@gem_softpin@noreloc-s3.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-skl:          [INCOMPLETE][19] ([fdo#110741]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-skl8/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-skl9/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][21] ([fdo#105363]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-glk8/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render:
    - shard-snb:          [SKIP][23] ([fdo#109271]) -> [PASS][24] +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-snb4/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-snb4/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         [FAIL][25] ([fdo#103167]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][27] ([fdo#108145]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [FAIL][29] ([fdo#108341]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb1/igt@kms_psr@no_drrs.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-iclb3/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [SKIP][31] ([fdo#109441]) -> [PASS][32] +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-iclb3/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          [DMESG-WARN][33] ([fdo#108566]) -> [PASS][34] +5 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6399/shard-apl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13501/shard-apl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110741]: https://bugs.freedesktop.org/show_bug.cgi?id=110741
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_6399 -> Patchwork_13501

  CI_DRM_6399: c15e6447c0c506c058671d59082fcd710e42d6d4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5079: 873df2fa9e8f5fd02d4532b30ef2579f4fe4f27f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13501: 03ff43603e1ee1d0c5b83b0d82524a7e1a46f77a @ 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_13501/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Implement read-only support in whitelist selftest

[Tvrtko Ursulin]

On 03/07/2019 03:06, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Newer hardware supports extra feature in the whitelist registers. This
> patch updates the selftest to test that entries marked as read only
> are actually read only.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I think I gave my r-b for this in the last round.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

P.S. I don't have a strong opinion on whether to have it like it is, or 
to do what Chris suggested and to cheat with rsvd = 0. Both are a bit 
difficult to figure out when reviewing. 0xffffffff solution is also 
misleading in a way that the value is only used in a log message for no 
real effect. So I guess this means slight preference to rsvd = 0 
solution after all.

> ---
>   .../gpu/drm/i915/gt/selftest_workarounds.c    | 43 +++++++++++++------
>   1 file changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index f8151d5946c8..5cd2b17105ba 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -482,12 +482,12 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>   		u32 srm, lrm, rsvd;
>   		u32 expect;
>   		int idx;
> +		bool ro_reg;
>   
>   		if (wo_register(engine, reg))
>   			continue;
>   
> -		if (ro_register(reg))
> -			continue;
> +		ro_reg = ro_register(reg);
>   
>   		srm = MI_STORE_REGISTER_MEM;
>   		lrm = MI_LOAD_REGISTER_MEM;
> @@ -588,24 +588,37 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>   		}
>   
>   		GEM_BUG_ON(values[ARRAY_SIZE(values) - 1] != 0xffffffff);
> -		rsvd = results[ARRAY_SIZE(values)]; /* detect write masking */
> -		if (!rsvd) {
> -			pr_err("%s: Unable to write to whitelisted register %x\n",
> -			       engine->name, reg);
> -			err = -EINVAL;
> -			goto out_unpin;
> +		if (ro_reg) {
> +			rsvd = 0xFFFFFFFF;
> +		} else {
> +			/* detect write masking */
> +			rsvd = results[ARRAY_SIZE(values)];
> +			if (!rsvd) {
> +				pr_err("%s: Unable to write to whitelisted register %x\n",
> +				       engine->name, reg);
> +				err = -EINVAL;
> +				goto out_unpin;
> +			}
>   		}
>   
>   		expect = results[0];
>   		idx = 1;
>   		for (v = 0; v < ARRAY_SIZE(values); v++) {
> -			expect = reg_write(expect, values[v], rsvd);
> +			if (ro_reg)
> +				expect = results[0];
> +			else
> +				expect = reg_write(expect, values[v], rsvd);
> +
>   			if (results[idx] != expect)
>   				err++;
>   			idx++;
>   		}
>   		for (v = 0; v < ARRAY_SIZE(values); v++) {
> -			expect = reg_write(expect, ~values[v], rsvd);
> +			if (ro_reg)
> +				expect = results[0];
> +			else
> +				expect = reg_write(expect, ~values[v], rsvd);
> +
>   			if (results[idx] != expect)
>   				err++;
>   			idx++;
> @@ -622,7 +635,10 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>   			for (v = 0; v < ARRAY_SIZE(values); v++) {
>   				u32 w = values[v];
>   
> -				expect = reg_write(expect, w, rsvd);
> +				if (ro_reg)
> +					expect = results[0];
> +				else
> +					expect = reg_write(expect, w, rsvd);
>   				pr_info("Wrote %08x, read %08x, expect %08x\n",
>   					w, results[idx], expect);
>   				idx++;
> @@ -630,7 +646,10 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>   			for (v = 0; v < ARRAY_SIZE(values); v++) {
>   				u32 w = ~values[v];
>   
> -				expect = reg_write(expect, w, rsvd);
> +				if (ro_reg)
> +					expect = results[0];
> +				else
> +					expect = reg_write(expect, w, rsvd);
>   				pr_info("Wrote %08x, read %08x, expect %08x\n",
>   					w, results[idx], expect);
>   				idx++;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Implement read-only support in whitelist selftest

[Tvrtko Ursulin]

On 03/07/2019 20:43, John Harrison wrote:
> On 7/3/2019 01:32, Chris Wilson wrote:
>> Quoting John.C.Harrison@Intel.com (2019-07-03 03:06:04)
>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>
>>> Newer hardware supports extra feature in the whitelist registers. This
>>> patch updates the selftest to test that entries marked as read only
>>> are actually read only.
>>>
>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>> CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>>   .../gpu/drm/i915/gt/selftest_workarounds.c    | 43 +++++++++++++------
>>>   1 file changed, 31 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c 
>>> b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
>>> index f8151d5946c8..5cd2b17105ba 100644
>>> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
>>> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
>>> @@ -482,12 +482,12 @@ static int check_dirty_whitelist(struct 
>>> i915_gem_context *ctx,
>>>                  u32 srm, lrm, rsvd;
>>>                  u32 expect;
>>>                  int idx;
>>> +               bool ro_reg;
>>>                  if (wo_register(engine, reg))
>>>                          continue;
>>> -               if (ro_register(reg))
>>> -                       continue;
>>> +               ro_reg = ro_register(reg);
>>>                  srm = MI_STORE_REGISTER_MEM;
>>>                  lrm = MI_LOAD_REGISTER_MEM;
>>> @@ -588,24 +588,37 @@ static int check_dirty_whitelist(struct 
>>> i915_gem_context *ctx,
>>>                  }
>>>                  GEM_BUG_ON(values[ARRAY_SIZE(values) - 1] != 
>>> 0xffffffff);
>>> -               rsvd = results[ARRAY_SIZE(values)]; /* detect write 
>>> masking */
>>> -               if (!rsvd) {
>>> -                       pr_err("%s: Unable to write to whitelisted 
>>> register %x\n",
>>> -                              engine->name, reg);
>>> -                       err = -EINVAL;
>>> -                       goto out_unpin;
>>> +               if (ro_reg) {
>>> +                       rsvd = 0xFFFFFFFF;
>> rsvd = 0;
>>
>> reg_write() will then dtrt.
> It seemed too suspiciously broken to have the test claim a read-only 
> register was successfully written to. This way makes it clear that the 
> test expects read-only to always return the first value read.

I suggest we go with this version if it is not too-disagreeable. Chris?

John can only hope it still applies.

Regards,

Tvrtko

>> Does this not replace the skip placed in check_whitelisted_registers()?
> The two versions of that test looks like they need to be able to set 
> values. So they can't be run on read-only registers.
> 
>> We still need a way to verify that the register exists, as even writing
>> from a secure batch fails (not tried ring though). Do we load a spinner,
>> tweak via mmio?
> 
> I don't think there is a reliable, generic mechanism to test that you 
> can actually read from a read only register. You need to know what 
> content it should provide. Even the current test (that it always returns 
> the same value) would break if the register changes dynamically (e.g. 
> it's a hardware counter).
> 
> John.
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx