All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling
@ 2018-11-30 17:44 Tvrtko Ursulin
  2018-11-30 17:44 ` [PATCH 1/7] drm/i915: Record GT workarounds in a list Tvrtko Ursulin
                   ` (10 more replies)
  0 siblings, 11 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 17:44 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

First two patches in this series fix losing of workarounds after engine reset
(https://bugzilla.freedesktop.org/show_bug.cgi?id=107945) which started
happening after 59b449d5c82a ("drm/i915: Split out functions for different kinds
of workarounds").

But since it was discovered to be unsafe to simply re-apply all of them, against
a possibly active GPU, and potentially from IRQ context, the approach taken was
to split GT workarounds and per-engine workarounds. Latter so far contain the
ones living in the 0x2xxx and 0xbxxx range, which were empirically shown to be
lost after RCS reset.

This way only a smaller set of affected workarounds can be applied after engine
resetm, which is done with irq safe read-modify-write cycle.

The series is structured like this so first two patches are as standalone as
possible so it is easy (easier) to backport them. The rest of the series
cleans up the whole workaround handling by moving all four classes of them to a
common framework.

v2:
 * One patch less due removing verification after engine reset.
 * See patch change logs.

Tvrtko Ursulin (7):
  drm/i915: Record GT workarounds in a list
  drm/i915: Introduce per-engine workarounds
  drm/i915: Verify GT workaround state after GPU init
  drm/i915/selftests: Add tests for GT and engine workaround
    verification
  drm/i915: Move register white-listing to the common workaround
    framework
  drm/i915: Fuse per-context workaround handling with the common
    framework
  drm/i915: Trim unused workaround list entries

 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/i915_debugfs.c           |  12 +-
 drivers/gpu/drm/i915/i915_drv.c               |   2 +
 drivers/gpu/drm/i915/i915_drv.h               |  17 +-
 drivers/gpu/drm/i915/i915_gem.c               |   5 +
 drivers/gpu/drm/i915/i915_gem_context.c       |   6 +-
 drivers/gpu/drm/i915/intel_engine_cs.c        |   4 +
 drivers/gpu/drm/i915/intel_lrc.c              |   5 +
 drivers/gpu/drm/i915/intel_ringbuffer.h       |   4 +
 drivers/gpu/drm/i915/intel_workarounds.c      | 879 +++++++++++-------
 drivers/gpu/drm/i915/intel_workarounds.h      |  31 +-
 drivers/gpu/drm/i915/selftests/igt_common.c   |  44 +
 drivers/gpu/drm/i915/selftests/igt_common.h   |  15 +
 .../gpu/drm/i915/selftests/intel_hangcheck.c  |  51 +-
 .../drm/i915/selftests/intel_workarounds.c    | 187 +++-
 15 files changed, 823 insertions(+), 440 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/selftests/igt_common.c
 create mode 100644 drivers/gpu/drm/i915/selftests/igt_common.h

-- 
2.19.1

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

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

* [PATCH 1/7] drm/i915: Record GT workarounds in a list
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
@ 2018-11-30 17:44 ` Tvrtko Ursulin
  2018-11-30 21:51   ` Chris Wilson
  2018-11-30 17:44 ` [PATCH 2/7] drm/i915: Introduce per-engine workarounds Tvrtko Ursulin
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 17:44 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

To enable later verification of GT workaround state at various stages of
driver lifetime, we record the list of applicable ones per platforms to a
list, from which they are also applied.

The added data structure is a simple array of register, mask and value
items, which is allocated on demand as workarounds are added to the list.

This is a temporary implementation which later in the series gets fused
with the existing per context workaround list handling. It is separated at
this stage since the following patch fixes a bug which needs to be as easy
to backport as possible.

Also, since in the following patch we will be adding a new class of
workarounds (per engine) which can be applied from interrupt context, we
straight away make the provision for safe read-modify-write cycle.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c          |   1 +
 drivers/gpu/drm/i915/i915_drv.h          |   2 +
 drivers/gpu/drm/i915/i915_gem.c          |   2 +
 drivers/gpu/drm/i915/intel_workarounds.c | 443 +++++++++++++++--------
 drivers/gpu/drm/i915/intel_workarounds.h |  22 ++
 5 files changed, 327 insertions(+), 143 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e39016713464..2f3dc1cf83a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1453,6 +1453,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 
 	intel_uncore_sanitize(dev_priv);
 
+	intel_gt_workarounds_init(dev_priv);
 	i915_gem_load_init_fences(dev_priv);
 
 	/* On the 945G/GM, the chipset reports the MSI capability on the
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 43ac6873a2bb..9ddbcc1f3554 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -69,6 +69,7 @@
 #include "intel_ringbuffer.h"
 #include "intel_uncore.h"
 #include "intel_wopcm.h"
+#include "intel_workarounds.h"
 #include "intel_uc.h"
 
 #include "i915_gem.h"
@@ -1652,6 +1653,7 @@ struct drm_i915_private {
 	int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
 
 	struct i915_workarounds workarounds;
+	struct i915_wa_list gt_wa_list;
 
 	struct i915_frontbuffer_tracking fb_tracking;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c55b1f75c980..18adb3dd1fcd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5706,6 +5706,8 @@ void i915_gem_fini(struct drm_i915_private *dev_priv)
 	i915_gem_contexts_fini(dev_priv);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
+	intel_wa_list_free(&dev_priv->gt_wa_list);
+
 	intel_cleanup_gt_powersave(dev_priv);
 
 	intel_uc_fini_misc(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index e5cd6c6c66c3..ff20ebf9e040 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -48,6 +48,18 @@
  * - Public functions to init or apply the given workaround type.
  */
 
+static void wa_init_start(struct i915_wa_list *wal, const char *name)
+{
+	wal->name = name;
+}
+
+static void wa_init_finish(struct i915_wa_list *wal)
+{
+	if (wal->count)
+		DRM_DEBUG_DRIVER("Initialized %u %s workarounds\n",
+				 wal->count, wal->name);
+}
+
 static void wa_add(struct drm_i915_private *i915,
 		   i915_reg_t reg, const u32 mask, const u32 val)
 {
@@ -575,28 +587,88 @@ int intel_ctx_workarounds_emit(struct i915_request *rq)
 	return 0;
 }
 
-static void bdw_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+static void
+wal_add(struct i915_wa_list *wal, const struct i915_wa *wa)
+{
+	const unsigned int grow = 4;
+
+	if (wal->__size == wal->count) {
+		struct i915_wa *list;
+
+		list = kcalloc(wal->__size + grow, sizeof(*wa), GFP_KERNEL);
+		if (!list) {
+			DRM_ERROR("No space for workaround init!\n");
+			return;
+		}
+
+		if (wal->list)
+			memcpy(list, wal->list, sizeof(*wa) * wal->count);
+
+		wal->list = list;
+		wal->__size += grow;
+	}
+
+	memcpy(&wal->list[wal->count], wa, sizeof(*wa));
+	wal->count++;
+}
+
+static void
+wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
 {
+	struct i915_wa wa = {
+		.reg = reg,
+		.mask = val,
+		.val = _MASKED_BIT_ENABLE(val)
+	};
+
+	wal_add(wal, &wa);
+}
+
+static void
+wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
+		   u32 val)
+{
+	struct i915_wa wa = {
+		.reg = reg,
+		.mask = mask,
+		.val = val
+	};
+
+	wal_add(wal, &wa);
 }
 
-static void chv_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+static void
+wa_write(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
 {
+	wa_write_masked_or(wal, reg, ~0, val);
 }
 
-static void gen9_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+static void
+wa_write_or(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
 {
+	wa_write_masked_or(wal, reg, val, val);
+}
+
+static void gen9_gt_workarounds_init(struct drm_i915_private *dev_priv)
+{
+	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
+
 	/* WaContextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk,cfl */
-	I915_WRITE(GEN9_CSFE_CHICKEN1_RCS,
-		   _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));
+	wa_masked_en(wal,
+		     GEN9_CSFE_CHICKEN1_RCS,
+		     GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE);
+
 
 	/* WaEnableLbsSlaRetryTimerDecrement:skl,bxt,kbl,glk,cfl */
-	I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) |
-		   GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE);
+	wa_write_or(wal,
+		    BDW_SCRATCH1,
+		    GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE);
 
 	/* WaDisableKillLogic:bxt,skl,kbl */
 	if (!IS_COFFEELAKE(dev_priv))
-		I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
-			   ECOCHK_DIS_TLB);
+		wa_write_or(wal,
+			    GAM_ECOCHK,
+			    ECOCHK_DIS_TLB);
 
 	if (HAS_LLC(dev_priv)) {
 		/* WaCompressedResourceSamplerPbeMediaNewHashMode:skl,kbl
@@ -604,89 +676,101 @@ static void gen9_gt_workarounds_apply(struct drm_i915_private *dev_priv)
 		 * Must match Display Engine. See
 		 * WaCompressedResourceDisplayNewHashMode.
 		 */
-		I915_WRITE(MMCD_MISC_CTRL,
-			   I915_READ(MMCD_MISC_CTRL) |
-			   MMCD_PCLA |
-			   MMCD_HOTSPOT_EN);
+		wa_write_or(wal,
+			    MMCD_MISC_CTRL,
+			    MMCD_PCLA | MMCD_HOTSPOT_EN);
 	}
 
 	/* WaDisableHDCInvalidation:skl,bxt,kbl,cfl */
-	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
-		   BDW_DISABLE_HDC_INVALIDATION);
+	wa_write_or(wal,
+		    GAM_ECOCHK,
+		    BDW_DISABLE_HDC_INVALIDATION);
 
 	/* WaProgramL3SqcReg1DefaultForPerf:bxt,glk */
-	if (IS_GEN9_LP(dev_priv)) {
-		u32 val = I915_READ(GEN8_L3SQCREG1);
-
-		val &= ~L3_PRIO_CREDITS_MASK;
-		val |= L3_GENERAL_PRIO_CREDITS(62) | L3_HIGH_PRIO_CREDITS(2);
-		I915_WRITE(GEN8_L3SQCREG1, val);
-	}
+	if (IS_GEN9_LP(dev_priv))
+		wa_write_masked_or(wal,
+				   GEN8_L3SQCREG1,
+				   L3_PRIO_CREDITS_MASK,
+				   L3_GENERAL_PRIO_CREDITS(62) |
+				   L3_HIGH_PRIO_CREDITS(2));
 
 	/* WaOCLCoherentLineFlush:skl,bxt,kbl,cfl */
-	I915_WRITE(GEN8_L3SQCREG4,
-		   I915_READ(GEN8_L3SQCREG4) | GEN8_LQSC_FLUSH_COHERENT_LINES);
+	wa_write_or(wal,
+		    GEN8_L3SQCREG4,
+		    GEN8_LQSC_FLUSH_COHERENT_LINES);
 
 	/* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] */
-	I915_WRITE(GEN7_FF_SLICE_CS_CHICKEN1,
-		   _MASKED_BIT_ENABLE(GEN9_FFSC_PERCTX_PREEMPT_CTRL));
+	wa_masked_en(wal,
+		     GEN7_FF_SLICE_CS_CHICKEN1,
+		     GEN9_FFSC_PERCTX_PREEMPT_CTRL);
 }
 
-static void skl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+static void skl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	gen9_gt_workarounds_apply(dev_priv);
+	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
+
+	gen9_gt_workarounds_init(dev_priv);
 
 	/* WaEnableGapsTsvCreditFix:skl */
-	I915_WRITE(GEN8_GARBCNTL,
-		   I915_READ(GEN8_GARBCNTL) | GEN9_GAPS_TSV_CREDIT_DISABLE);
+	wa_write_or(wal,
+		    GEN8_GARBCNTL,
+		    GEN9_GAPS_TSV_CREDIT_DISABLE);
 
 	/* WaDisableGafsUnitClkGating:skl */
-	I915_WRITE(GEN7_UCGCTL4,
-		   I915_READ(GEN7_UCGCTL4) | GEN8_EU_GAUNIT_CLOCK_GATE_DISABLE);
+	wa_write_or(wal,
+		    GEN7_UCGCTL4,
+		    GEN8_EU_GAUNIT_CLOCK_GATE_DISABLE);
 
 	/* WaInPlaceDecompressionHang:skl */
 	if (IS_SKL_REVID(dev_priv, SKL_REVID_H0, REVID_FOREVER))
-		I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
-			   I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
-			   GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+		wa_write_or(wal,
+			    GEN9_GAMT_ECO_REG_RW_IA,
+			    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
 }
 
-static void bxt_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+static void bxt_gt_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	gen9_gt_workarounds_apply(dev_priv);
+	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
+
+	gen9_gt_workarounds_init(dev_priv);
 
 	/* WaDisablePooledEuLoadBalancingFix:bxt */
-	I915_WRITE(FF_SLICE_CS_CHICKEN2,
-		   _MASKED_BIT_ENABLE(GEN9_POOLED_EU_LOAD_BALANCING_FIX_DISABLE));
+	wa_masked_en(wal,
+		     FF_SLICE_CS_CHICKEN2,
+		     GEN9_POOLED_EU_LOAD_BALANCING_FIX_DISABLE);
 
 	/* WaInPlaceDecompressionHang:bxt */
-	I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
-		   I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
-		   GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+	wa_write_or(wal,
+		    GEN9_GAMT_ECO_REG_RW_IA,
+		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
 }
 
-static void kbl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+static void kbl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	gen9_gt_workarounds_apply(dev_priv);
+	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
+
+	gen9_gt_workarounds_init(dev_priv);
 
 	/* WaEnableGapsTsvCreditFix:kbl */
-	I915_WRITE(GEN8_GARBCNTL,
-		   I915_READ(GEN8_GARBCNTL) | GEN9_GAPS_TSV_CREDIT_DISABLE);
+	wa_write_or(wal,
+		    GEN8_GARBCNTL,
+		    GEN9_GAPS_TSV_CREDIT_DISABLE);
 
 	/* WaDisableDynamicCreditSharing:kbl */
 	if (IS_KBL_REVID(dev_priv, 0, KBL_REVID_B0))
-		I915_WRITE(GAMT_CHKN_BIT_REG,
-			   I915_READ(GAMT_CHKN_BIT_REG) |
-			   GAMT_CHKN_DISABLE_DYNAMIC_CREDIT_SHARING);
+		wa_write_or(wal,
+			    GAMT_CHKN_BIT_REG,
+			    GAMT_CHKN_DISABLE_DYNAMIC_CREDIT_SHARING);
 
 	/* WaDisableGafsUnitClkGating:kbl */
-	I915_WRITE(GEN7_UCGCTL4,
-		   I915_READ(GEN7_UCGCTL4) | GEN8_EU_GAUNIT_CLOCK_GATE_DISABLE);
+	wa_write_or(wal,
+		    GEN7_UCGCTL4,
+		    GEN8_EU_GAUNIT_CLOCK_GATE_DISABLE);
 
 	/* WaInPlaceDecompressionHang:kbl */
-	I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
-		   I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
-		   GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+	wa_write_or(wal,
+		    GEN9_GAMT_ECO_REG_RW_IA,
+		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
 
 	/* WaKBLVECSSemaphoreWaitPoll:kbl */
 	if (IS_KBL_REVID(dev_priv, KBL_REVID_A0, KBL_REVID_E0)) {
@@ -697,38 +781,44 @@ static void kbl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
 			if (engine->id == RCS)
 				continue;
 
-			I915_WRITE(RING_SEMA_WAIT_POLL(engine->mmio_base), 1);
+			wa_write(wal,
+				 RING_SEMA_WAIT_POLL(engine->mmio_base),
+				 1);
 		}
 	}
 }
 
-static void glk_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+static void glk_gt_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	gen9_gt_workarounds_apply(dev_priv);
+	gen9_gt_workarounds_init(dev_priv);
 }
 
-static void cfl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+static void cfl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	gen9_gt_workarounds_apply(dev_priv);
+	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
+
+	gen9_gt_workarounds_init(dev_priv);
 
 	/* WaEnableGapsTsvCreditFix:cfl */
-	I915_WRITE(GEN8_GARBCNTL,
-		   I915_READ(GEN8_GARBCNTL) | GEN9_GAPS_TSV_CREDIT_DISABLE);
+	wa_write_or(wal,
+		    GEN8_GARBCNTL,
+		    GEN9_GAPS_TSV_CREDIT_DISABLE);
 
 	/* WaDisableGafsUnitClkGating:cfl */
-	I915_WRITE(GEN7_UCGCTL4,
-		   I915_READ(GEN7_UCGCTL4) | GEN8_EU_GAUNIT_CLOCK_GATE_DISABLE);
+	wa_write_or(wal,
+		    GEN7_UCGCTL4,
+		    GEN8_EU_GAUNIT_CLOCK_GATE_DISABLE);
 
 	/* WaInPlaceDecompressionHang:cfl */
-	I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
-		   I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
-		   GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+	wa_write_or(wal,
+		    GEN9_GAMT_ECO_REG_RW_IA,
+		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
 }
 
 static void wa_init_mcr(struct drm_i915_private *dev_priv)
 {
 	const struct sseu_dev_info *sseu = &(INTEL_INFO(dev_priv)->sseu);
-	u32 mcr;
+	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
 	u32 mcr_slice_subslice_mask;
 
 	/*
@@ -765,8 +855,6 @@ static void wa_init_mcr(struct drm_i915_private *dev_priv)
 		WARN_ON((enabled_mask & disabled_mask) != enabled_mask);
 	}
 
-	mcr = I915_READ(GEN8_MCR_SELECTOR);
-
 	if (INTEL_GEN(dev_priv) >= 11)
 		mcr_slice_subslice_mask = GEN11_MCR_SLICE_MASK |
 					  GEN11_MCR_SUBSLICE_MASK;
@@ -784,156 +872,225 @@ static void wa_init_mcr(struct drm_i915_private *dev_priv)
 	 * occasions, such as INSTDONE, where this value is dependent
 	 * on s/ss combo, the read should be done with read_subslice_reg.
 	 */
-	mcr &= ~mcr_slice_subslice_mask;
-	mcr |= intel_calculate_mcr_s_ss_select(dev_priv);
-	I915_WRITE(GEN8_MCR_SELECTOR, mcr);
+	wa_write_masked_or(wal,
+			   GEN8_MCR_SELECTOR,
+			   mcr_slice_subslice_mask,
+			   intel_calculate_mcr_s_ss_select(dev_priv));
 }
 
-static void cnl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+static void cnl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 {
+	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
+
 	wa_init_mcr(dev_priv);
 
 	/* WaDisableI2mCycleOnWRPort:cnl (pre-prod) */
 	if (IS_CNL_REVID(dev_priv, CNL_REVID_B0, CNL_REVID_B0))
-		I915_WRITE(GAMT_CHKN_BIT_REG,
-			   I915_READ(GAMT_CHKN_BIT_REG) |
-			   GAMT_CHKN_DISABLE_I2M_CYCLE_ON_WR_PORT);
+		wa_write_or(wal,
+			    GAMT_CHKN_BIT_REG,
+			    GAMT_CHKN_DISABLE_I2M_CYCLE_ON_WR_PORT);
 
 	/* WaInPlaceDecompressionHang:cnl */
-	I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
-		   I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
-		   GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+	wa_write_or(wal,
+		    GEN9_GAMT_ECO_REG_RW_IA,
+		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
 
 	/* WaEnablePreemptionGranularityControlByUMD:cnl */
-	I915_WRITE(GEN7_FF_SLICE_CS_CHICKEN1,
-		   _MASKED_BIT_ENABLE(GEN9_FFSC_PERCTX_PREEMPT_CTRL));
+	wa_masked_en(wal,
+		     GEN7_FF_SLICE_CS_CHICKEN1,
+		     GEN9_FFSC_PERCTX_PREEMPT_CTRL);
 }
 
-static void icl_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+static void icl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 {
+	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
+
 	wa_init_mcr(dev_priv);
 
 	/* This is not an Wa. Enable for better image quality */
-	I915_WRITE(_3D_CHICKEN3,
-		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE));
+	wa_masked_en(wal,
+		    _3D_CHICKEN3,
+		    _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE);
 
 	/* WaInPlaceDecompressionHang:icl */
-	I915_WRITE(GEN9_GAMT_ECO_REG_RW_IA,
-		   I915_READ(GEN9_GAMT_ECO_REG_RW_IA) |
-		   GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
+	wa_write_or(wal,
+		    GEN9_GAMT_ECO_REG_RW_IA,
+		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
 
 	/* WaPipelineFlushCoherentLines:icl */
-	I915_WRITE(GEN8_L3SQCREG4,
-		   I915_READ(GEN8_L3SQCREG4) |
-		   GEN8_LQSC_FLUSH_COHERENT_LINES);
+	wa_write_or(wal,
+		    GEN8_L3SQCREG4,
+		    GEN8_LQSC_FLUSH_COHERENT_LINES);
 
 	/* Wa_1405543622:icl
 	 * Formerly known as WaGAPZPriorityScheme
 	 */
-	I915_WRITE(GEN8_GARBCNTL,
-		   I915_READ(GEN8_GARBCNTL) |
-		   GEN11_ARBITRATION_PRIO_ORDER_MASK);
+	wa_write_or(wal,
+		    GEN8_GARBCNTL,
+		    GEN11_ARBITRATION_PRIO_ORDER_MASK);
 
 	/* Wa_1604223664:icl
 	 * Formerly known as WaL3BankAddressHashing
 	 */
-	I915_WRITE(GEN8_GARBCNTL,
-		   (I915_READ(GEN8_GARBCNTL) & ~GEN11_HASH_CTRL_EXCL_MASK) |
-		   GEN11_HASH_CTRL_EXCL_BIT0);
-	I915_WRITE(GEN11_GLBLINVL,
-		   (I915_READ(GEN11_GLBLINVL) & ~GEN11_BANK_HASH_ADDR_EXCL_MASK) |
-		   GEN11_BANK_HASH_ADDR_EXCL_BIT0);
+	wa_write_masked_or(wal,
+			   GEN8_GARBCNTL,
+			   GEN11_HASH_CTRL_EXCL_MASK,
+			   GEN11_HASH_CTRL_EXCL_BIT0);
+	wa_write_masked_or(wal,
+			   GEN11_GLBLINVL,
+			   GEN11_BANK_HASH_ADDR_EXCL_MASK,
+			   GEN11_BANK_HASH_ADDR_EXCL_BIT0);
 
 	/* WaModifyGamTlbPartitioning:icl */
-	I915_WRITE(GEN11_GACB_PERF_CTRL,
-		   (I915_READ(GEN11_GACB_PERF_CTRL) & ~GEN11_HASH_CTRL_MASK) |
-		   GEN11_HASH_CTRL_BIT0 | GEN11_HASH_CTRL_BIT4);
+	wa_write_masked_or(wal,
+			   GEN11_GACB_PERF_CTRL,
+			   GEN11_HASH_CTRL_MASK,
+			   GEN11_HASH_CTRL_BIT0 | GEN11_HASH_CTRL_BIT4);
 
 	/* Wa_1405733216:icl
 	 * Formerly known as WaDisableCleanEvicts
 	 */
-	I915_WRITE(GEN8_L3SQCREG4,
-		   I915_READ(GEN8_L3SQCREG4) |
-		   GEN11_LQSC_CLEAN_EVICT_DISABLE);
+	wa_write_or(wal,
+		    GEN8_L3SQCREG4,
+		    GEN11_LQSC_CLEAN_EVICT_DISABLE);
 
 	/* Wa_1405766107:icl
 	 * Formerly known as WaCL2SFHalfMaxAlloc
 	 */
-	I915_WRITE(GEN11_LSN_UNSLCVC,
-		   I915_READ(GEN11_LSN_UNSLCVC) |
-		   GEN11_LSN_UNSLCVC_GAFS_HALF_SF_MAXALLOC |
-		   GEN11_LSN_UNSLCVC_GAFS_HALF_CL2_MAXALLOC);
+	wa_write_or(wal,
+		    GEN11_LSN_UNSLCVC,
+		    GEN11_LSN_UNSLCVC_GAFS_HALF_SF_MAXALLOC |
+		    GEN11_LSN_UNSLCVC_GAFS_HALF_CL2_MAXALLOC);
 
 	/* Wa_220166154:icl
 	 * Formerly known as WaDisCtxReload
 	 */
-	I915_WRITE(GEN8_GAMW_ECO_DEV_RW_IA,
-		   I915_READ(GEN8_GAMW_ECO_DEV_RW_IA) |
-		   GAMW_ECO_DEV_CTX_RELOAD_DISABLE);
+	wa_write_or(wal,
+		    GEN8_GAMW_ECO_DEV_RW_IA,
+		    GAMW_ECO_DEV_CTX_RELOAD_DISABLE);
 
 	/* Wa_1405779004:icl (pre-prod) */
 	if (IS_ICL_REVID(dev_priv, ICL_REVID_A0, ICL_REVID_A0))
-		I915_WRITE(SLICE_UNIT_LEVEL_CLKGATE,
-			   I915_READ(SLICE_UNIT_LEVEL_CLKGATE) |
-			   MSCUNIT_CLKGATE_DIS);
+		wa_write_or(wal,
+			    SLICE_UNIT_LEVEL_CLKGATE,
+			    MSCUNIT_CLKGATE_DIS);
 
 	/* Wa_1406680159:icl */
-	I915_WRITE(SUBSLICE_UNIT_LEVEL_CLKGATE,
-		   I915_READ(SUBSLICE_UNIT_LEVEL_CLKGATE) |
-		   GWUNIT_CLKGATE_DIS);
+	wa_write_or(wal,
+		    SUBSLICE_UNIT_LEVEL_CLKGATE,
+		    GWUNIT_CLKGATE_DIS);
 
 	/* Wa_1406838659:icl (pre-prod) */
 	if (IS_ICL_REVID(dev_priv, ICL_REVID_A0, ICL_REVID_B0))
-		I915_WRITE(INF_UNIT_LEVEL_CLKGATE,
-			   I915_READ(INF_UNIT_LEVEL_CLKGATE) |
-			   CGPSF_CLKGATE_DIS);
+		wa_write_or(wal,
+			    INF_UNIT_LEVEL_CLKGATE,
+			    CGPSF_CLKGATE_DIS);
 
 	/* WaForwardProgressSoftReset:icl */
-	I915_WRITE(GEN10_SCRATCH_LNCF2,
-		   I915_READ(GEN10_SCRATCH_LNCF2) |
-		   PMFLUSHDONE_LNICRSDROP |
-		   PMFLUSH_GAPL3UNBLOCK |
-		   PMFLUSHDONE_LNEBLK);
+	wa_write_or(wal,
+		    GEN10_SCRATCH_LNCF2,
+		    PMFLUSHDONE_LNICRSDROP |
+		    PMFLUSH_GAPL3UNBLOCK |
+		    PMFLUSHDONE_LNEBLK);
 
 	/* Wa_1406463099:icl
 	 * Formerly known as WaGamTlbPendError
 	 */
-	I915_WRITE(GAMT_CHKN_BIT_REG,
-		   I915_READ(GAMT_CHKN_BIT_REG) |
-		   GAMT_CHKN_DISABLE_L3_COH_PIPE);
+	wa_write_or(wal,
+		    GAMT_CHKN_BIT_REG,
+		    GAMT_CHKN_DISABLE_L3_COH_PIPE);
 
 	/* Wa_1406609255:icl (pre-prod) */
 	if (IS_ICL_REVID(dev_priv, ICL_REVID_A0, ICL_REVID_B0))
-		I915_WRITE(GEN7_SARCHKMD,
-			   I915_READ(GEN7_SARCHKMD) |
-			   GEN7_DISABLE_DEMAND_PREFETCH |
-			   GEN7_DISABLE_SAMPLER_PREFETCH);
+		wa_write_or(wal,
+			    GEN7_SARCHKMD,
+			    GEN7_DISABLE_DEMAND_PREFETCH |
+			    GEN7_DISABLE_SAMPLER_PREFETCH);
 }
 
-void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+void intel_gt_workarounds_init(struct drm_i915_private *dev_priv)
 {
+	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
+
+	wa_init_start(wal, "GT");
+
 	if (INTEL_GEN(dev_priv) < 8)
 		return;
 	else if (IS_BROADWELL(dev_priv))
-		bdw_gt_workarounds_apply(dev_priv);
+		return;
 	else if (IS_CHERRYVIEW(dev_priv))
-		chv_gt_workarounds_apply(dev_priv);
+		return;
 	else if (IS_SKYLAKE(dev_priv))
-		skl_gt_workarounds_apply(dev_priv);
+		skl_gt_workarounds_init(dev_priv);
 	else if (IS_BROXTON(dev_priv))
-		bxt_gt_workarounds_apply(dev_priv);
+		bxt_gt_workarounds_init(dev_priv);
 	else if (IS_KABYLAKE(dev_priv))
-		kbl_gt_workarounds_apply(dev_priv);
+		kbl_gt_workarounds_init(dev_priv);
 	else if (IS_GEMINILAKE(dev_priv))
-		glk_gt_workarounds_apply(dev_priv);
+		glk_gt_workarounds_init(dev_priv);
 	else if (IS_COFFEELAKE(dev_priv))
-		cfl_gt_workarounds_apply(dev_priv);
+		cfl_gt_workarounds_init(dev_priv);
 	else if (IS_CANNONLAKE(dev_priv))
-		cnl_gt_workarounds_apply(dev_priv);
+		cnl_gt_workarounds_init(dev_priv);
 	else if (IS_ICELAKE(dev_priv))
-		icl_gt_workarounds_apply(dev_priv);
+		icl_gt_workarounds_init(dev_priv);
 	else
 		MISSING_CASE(INTEL_GEN(dev_priv));
+
+	wa_init_finish(wal);
+}
+
+static enum forcewake_domains
+wal_get_fw_for_rmw(struct drm_i915_private *dev_priv,
+		   const struct i915_wa_list *wal)
+{
+	enum forcewake_domains fw = 0;
+	struct i915_wa *wa;
+	unsigned int i;
+
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
+		fw |= intel_uncore_forcewake_for_reg(dev_priv,
+						     wa->reg,
+						     FW_REG_READ |
+						     FW_REG_WRITE);
+
+	return fw;
+}
+
+static void
+wa_list_apply(struct drm_i915_private *dev_priv, const struct i915_wa_list *wal)
+{
+	enum forcewake_domains fw;
+	unsigned long flags;
+	struct i915_wa *wa;
+	unsigned int i;
+
+	if (!wal->count)
+		return;
+
+	fw = wal_get_fw_for_rmw(dev_priv, wal);
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, flags);
+	intel_uncore_forcewake_get__locked(dev_priv, fw);
+
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
+		u32 val = I915_READ_FW(wa->reg);
+
+		val &= ~wa->mask;
+		val |= wa->val;
+
+		I915_WRITE_FW(wa->reg, val);
+	}
+
+	intel_uncore_forcewake_put__locked(dev_priv, fw);
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, flags);
+
+	DRM_DEBUG_DRIVER("Applied %u %s workarounds\n", wal->count, wal->name);
+}
+
+void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
+{
+	wa_list_apply(dev_priv, &dev_priv->gt_wa_list);
 }
 
 struct whitelist {
diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
index b11d0623e626..64aed9cf0200 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -7,9 +7,31 @@
 #ifndef _I915_WORKAROUNDS_H_
 #define _I915_WORKAROUNDS_H_
 
+#include <linux/slab.h>
+
+struct i915_wa {
+	i915_reg_t	  reg;
+	u32		  mask;
+	u32		  val;
+};
+
+struct i915_wa_list {
+	const char	*name;
+	unsigned int 	count;
+	struct i915_wa	*list;
+	unsigned int	__size;
+};
+
+static inline void intel_wa_list_free(struct i915_wa_list *wal)
+{
+	kfree(wal->list);
+	memset(wal, 0, sizeof(*wal));
+}
+
 int intel_ctx_workarounds_init(struct drm_i915_private *dev_priv);
 int intel_ctx_workarounds_emit(struct i915_request *rq);
 
+void intel_gt_workarounds_init(struct drm_i915_private *dev_priv);
 void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv);
 
 void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine);
-- 
2.19.1

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

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

* [PATCH 2/7] drm/i915: Introduce per-engine workarounds
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
  2018-11-30 17:44 ` [PATCH 1/7] drm/i915: Record GT workarounds in a list Tvrtko Ursulin
@ 2018-11-30 17:44 ` Tvrtko Ursulin
  2018-11-30 20:56   ` Chris Wilson
                     ` (2 more replies)
  2018-11-30 17:44 ` [PATCH 3/7] drm/i915: Verify GT workaround state after GPU init Tvrtko Ursulin
                   ` (8 subsequent siblings)
  10 siblings, 3 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 17:44 UTC (permalink / raw)
  To: Intel-gfx; +Cc: intel-gfx, Rodrigo Vivi

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We stopped re-applying the GT workarounds after engine reset since commit
59b449d5c82a ("drm/i915: Split out functions for different kinds of
workarounds").

Issue with this is that some of the GT workarounds live in the MMIO space
which gets lost during engine resets. So far the registers in 0x2xxx and
0xbxxx address range have been identified to be affected.

This losing of applied workarounds has obvious negative effects and can
even lead to hard system hangs (see the linked Bugzilla).

Rather than just restoring this re-application, because we have also
observed that it is not safe to just re-write all GT workarounds after
engine resets (GPU might be live and weird hardware states can happen),
we introduce a new class of per-engine workarounds and move only the
affected GT workarounds over.

Using the framework introduced in the previous patch, we therefore after
engine reset, re-apply only the workarounds living in the affected MMIO
address ranges.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=107945
Fixes: 59b449d5c82a ("drm/i915: Split out functions for different kinds of workarounds")
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/i915/intel_engine_cs.c   |   2 +
 drivers/gpu/drm/i915/intel_lrc.c         |   4 +
 drivers/gpu/drm/i915/intel_ringbuffer.h  |   2 +
 drivers/gpu/drm/i915/intel_workarounds.c | 249 +++++++++++++----------
 drivers/gpu/drm/i915/intel_workarounds.h |   3 +
 5 files changed, 148 insertions(+), 112 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 759c0fd58f8c..ef5d202e9d45 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -723,6 +723,8 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
 	__intel_context_unpin(i915->kernel_context, engine);
 
 	i915_timeline_fini(&engine->timeline);
+
+	intel_wa_list_free(&engine->wa_list);
 }
 
 u64 intel_engine_get_active_head(const struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 11f4e6148557..dfafc3f710d6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1617,6 +1617,8 @@ static bool unexpected_starting_state(struct intel_engine_cs *engine)
 
 static int gen8_init_common_ring(struct intel_engine_cs *engine)
 {
+	intel_engine_workarounds_apply(engine);
+
 	intel_mocs_init_engine(engine);
 
 	intel_engine_reset_breadcrumbs(engine);
@@ -2314,6 +2316,8 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
 			  ret);
 	}
 
+	intel_engine_workarounds_init(engine);
+
 	return 0;
 
 err_cleanup_common:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 8a2270b209b0..c5ff3d31cab7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -15,6 +15,7 @@
 #include "i915_selftest.h"
 #include "i915_timeline.h"
 #include "intel_gpu_commands.h"
+#include "intel_workarounds.h"
 
 struct drm_printer;
 struct i915_sched_attr;
@@ -451,6 +452,7 @@ struct intel_engine_cs {
 
 	struct intel_hw_status_page status_page;
 	struct i915_ctx_workarounds wa_ctx;
+	struct i915_wa_list wa_list;
 	struct i915_vma *scratch;
 
 	u32             irq_keep_mask; /* always keep these interrupts */
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index ff20ebf9e040..be63a2af3481 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -653,17 +653,6 @@ static void gen9_gt_workarounds_init(struct drm_i915_private *dev_priv)
 {
 	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
 
-	/* WaContextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk,cfl */
-	wa_masked_en(wal,
-		     GEN9_CSFE_CHICKEN1_RCS,
-		     GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE);
-
-
-	/* WaEnableLbsSlaRetryTimerDecrement:skl,bxt,kbl,glk,cfl */
-	wa_write_or(wal,
-		    BDW_SCRATCH1,
-		    GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE);
-
 	/* WaDisableKillLogic:bxt,skl,kbl */
 	if (!IS_COFFEELAKE(dev_priv))
 		wa_write_or(wal,
@@ -685,24 +674,6 @@ static void gen9_gt_workarounds_init(struct drm_i915_private *dev_priv)
 	wa_write_or(wal,
 		    GAM_ECOCHK,
 		    BDW_DISABLE_HDC_INVALIDATION);
-
-	/* WaProgramL3SqcReg1DefaultForPerf:bxt,glk */
-	if (IS_GEN9_LP(dev_priv))
-		wa_write_masked_or(wal,
-				   GEN8_L3SQCREG1,
-				   L3_PRIO_CREDITS_MASK,
-				   L3_GENERAL_PRIO_CREDITS(62) |
-				   L3_HIGH_PRIO_CREDITS(2));
-
-	/* WaOCLCoherentLineFlush:skl,bxt,kbl,cfl */
-	wa_write_or(wal,
-		    GEN8_L3SQCREG4,
-		    GEN8_LQSC_FLUSH_COHERENT_LINES);
-
-	/* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] */
-	wa_masked_en(wal,
-		     GEN7_FF_SLICE_CS_CHICKEN1,
-		     GEN9_FFSC_PERCTX_PREEMPT_CTRL);
 }
 
 static void skl_gt_workarounds_init(struct drm_i915_private *dev_priv)
@@ -711,11 +682,6 @@ static void skl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 
 	gen9_gt_workarounds_init(dev_priv);
 
-	/* WaEnableGapsTsvCreditFix:skl */
-	wa_write_or(wal,
-		    GEN8_GARBCNTL,
-		    GEN9_GAPS_TSV_CREDIT_DISABLE);
-
 	/* WaDisableGafsUnitClkGating:skl */
 	wa_write_or(wal,
 		    GEN7_UCGCTL4,
@@ -734,11 +700,6 @@ static void bxt_gt_workarounds_init(struct drm_i915_private *dev_priv)
 
 	gen9_gt_workarounds_init(dev_priv);
 
-	/* WaDisablePooledEuLoadBalancingFix:bxt */
-	wa_masked_en(wal,
-		     FF_SLICE_CS_CHICKEN2,
-		     GEN9_POOLED_EU_LOAD_BALANCING_FIX_DISABLE);
-
 	/* WaInPlaceDecompressionHang:bxt */
 	wa_write_or(wal,
 		    GEN9_GAMT_ECO_REG_RW_IA,
@@ -751,11 +712,6 @@ static void kbl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 
 	gen9_gt_workarounds_init(dev_priv);
 
-	/* WaEnableGapsTsvCreditFix:kbl */
-	wa_write_or(wal,
-		    GEN8_GARBCNTL,
-		    GEN9_GAPS_TSV_CREDIT_DISABLE);
-
 	/* WaDisableDynamicCreditSharing:kbl */
 	if (IS_KBL_REVID(dev_priv, 0, KBL_REVID_B0))
 		wa_write_or(wal,
@@ -771,21 +727,6 @@ static void kbl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 	wa_write_or(wal,
 		    GEN9_GAMT_ECO_REG_RW_IA,
 		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
-
-	/* WaKBLVECSSemaphoreWaitPoll:kbl */
-	if (IS_KBL_REVID(dev_priv, KBL_REVID_A0, KBL_REVID_E0)) {
-		struct intel_engine_cs *engine;
-		unsigned int tmp;
-
-		for_each_engine(engine, dev_priv, tmp) {
-			if (engine->id == RCS)
-				continue;
-
-			wa_write(wal,
-				 RING_SEMA_WAIT_POLL(engine->mmio_base),
-				 1);
-		}
-	}
 }
 
 static void glk_gt_workarounds_init(struct drm_i915_private *dev_priv)
@@ -799,11 +740,6 @@ static void cfl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 
 	gen9_gt_workarounds_init(dev_priv);
 
-	/* WaEnableGapsTsvCreditFix:cfl */
-	wa_write_or(wal,
-		    GEN8_GARBCNTL,
-		    GEN9_GAPS_TSV_CREDIT_DISABLE);
-
 	/* WaDisableGafsUnitClkGating:cfl */
 	wa_write_or(wal,
 		    GEN7_UCGCTL4,
@@ -894,11 +830,6 @@ static void cnl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 	wa_write_or(wal,
 		    GEN9_GAMT_ECO_REG_RW_IA,
 		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
-
-	/* WaEnablePreemptionGranularityControlByUMD:cnl */
-	wa_masked_en(wal,
-		     GEN7_FF_SLICE_CS_CHICKEN1,
-		     GEN9_FFSC_PERCTX_PREEMPT_CTRL);
 }
 
 static void icl_gt_workarounds_init(struct drm_i915_private *dev_priv)
@@ -907,53 +838,17 @@ static void icl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 
 	wa_init_mcr(dev_priv);
 
-	/* This is not an Wa. Enable for better image quality */
-	wa_masked_en(wal,
-		    _3D_CHICKEN3,
-		    _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE);
-
 	/* WaInPlaceDecompressionHang:icl */
 	wa_write_or(wal,
 		    GEN9_GAMT_ECO_REG_RW_IA,
 		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
 
-	/* WaPipelineFlushCoherentLines:icl */
-	wa_write_or(wal,
-		    GEN8_L3SQCREG4,
-		    GEN8_LQSC_FLUSH_COHERENT_LINES);
-
-	/* Wa_1405543622:icl
-	 * Formerly known as WaGAPZPriorityScheme
-	 */
-	wa_write_or(wal,
-		    GEN8_GARBCNTL,
-		    GEN11_ARBITRATION_PRIO_ORDER_MASK);
-
-	/* Wa_1604223664:icl
-	 * Formerly known as WaL3BankAddressHashing
-	 */
-	wa_write_masked_or(wal,
-			   GEN8_GARBCNTL,
-			   GEN11_HASH_CTRL_EXCL_MASK,
-			   GEN11_HASH_CTRL_EXCL_BIT0);
-	wa_write_masked_or(wal,
-			   GEN11_GLBLINVL,
-			   GEN11_BANK_HASH_ADDR_EXCL_MASK,
-			   GEN11_BANK_HASH_ADDR_EXCL_BIT0);
-
 	/* WaModifyGamTlbPartitioning:icl */
 	wa_write_masked_or(wal,
 			   GEN11_GACB_PERF_CTRL,
 			   GEN11_HASH_CTRL_MASK,
 			   GEN11_HASH_CTRL_BIT0 | GEN11_HASH_CTRL_BIT4);
 
-	/* Wa_1405733216:icl
-	 * Formerly known as WaDisableCleanEvicts
-	 */
-	wa_write_or(wal,
-		    GEN8_L3SQCREG4,
-		    GEN11_LQSC_CLEAN_EVICT_DISABLE);
-
 	/* Wa_1405766107:icl
 	 * Formerly known as WaCL2SFHalfMaxAlloc
 	 */
@@ -986,13 +881,6 @@ static void icl_gt_workarounds_init(struct drm_i915_private *dev_priv)
 			    INF_UNIT_LEVEL_CLKGATE,
 			    CGPSF_CLKGATE_DIS);
 
-	/* WaForwardProgressSoftReset:icl */
-	wa_write_or(wal,
-		    GEN10_SCRATCH_LNCF2,
-		    PMFLUSHDONE_LNICRSDROP |
-		    PMFLUSH_GAPL3UNBLOCK |
-		    PMFLUSHDONE_LNEBLK);
-
 	/* Wa_1406463099:icl
 	 * Formerly known as WaGamTlbPendError
 	 */
@@ -1242,6 +1130,143 @@ void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine)
 	whitelist_apply(engine, whitelist_build(engine, &w));
 }
 
+static void rcs_engine_wa_init(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *i915 = engine->i915;
+	struct i915_wa_list *wal = &engine->wa_list;
+
+	if (IS_GEN9(i915)) {
+		/* WaContextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk,cfl */
+		wa_masked_en(wal,
+			     GEN9_CSFE_CHICKEN1_RCS,
+			     GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE);
+
+		/* WaEnableLbsSlaRetryTimerDecrement:skl,bxt,kbl,glk,cfl */
+		wa_write_or(wal,
+			    BDW_SCRATCH1,
+			    GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE);
+
+		/* WaProgramL3SqcReg1DefaultForPerf:bxt,glk */
+		if (IS_GEN9_LP(i915))
+			wa_write_masked_or(wal,
+					   GEN8_L3SQCREG1,
+					   L3_PRIO_CREDITS_MASK,
+					   L3_GENERAL_PRIO_CREDITS(62) |
+					   L3_HIGH_PRIO_CREDITS(2));
+
+		/* WaOCLCoherentLineFlush:skl,bxt,kbl,cfl */
+		wa_write_or(wal,
+			    GEN8_L3SQCREG4,
+			    GEN8_LQSC_FLUSH_COHERENT_LINES);
+	}
+
+	if (IS_BROXTON(i915)) {
+		/* WaDisablePooledEuLoadBalancingFix:bxt */
+		wa_masked_en(wal,
+			     FF_SLICE_CS_CHICKEN2,
+			     GEN9_POOLED_EU_LOAD_BALANCING_FIX_DISABLE);
+	}
+
+	if (IS_SKYLAKE(i915) || IS_KABYLAKE(i915) || IS_COFFEELAKE(i915)) {
+		/* WaEnableGapsTsvCreditFix:skl,kbl,cfl */
+		wa_write_or(wal,
+			    GEN8_GARBCNTL,
+			    GEN9_GAPS_TSV_CREDIT_DISABLE);
+	}
+
+	if (IS_GEN9(i915) || IS_CANNONLAKE(i915)) {
+		/* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */
+		wa_masked_en(wal,
+			     GEN7_FF_SLICE_CS_CHICKEN1,
+			     GEN9_FFSC_PERCTX_PREEMPT_CTRL);
+	}
+
+	if (IS_ICELAKE(i915)) {
+		/* This is not an Wa. Enable for better image quality */
+		wa_masked_en(wal,
+			     _3D_CHICKEN3,
+			     _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE);
+
+		/* WaPipelineFlushCoherentLines:icl */
+		wa_write_or(wal,
+			    GEN8_L3SQCREG4,
+			    GEN8_LQSC_FLUSH_COHERENT_LINES);
+
+		/*
+		 * Wa_1405543622:icl
+		 * Formerly known as WaGAPZPriorityScheme
+		 */
+		wa_write_or(wal,
+			    GEN8_GARBCNTL,
+			    GEN11_ARBITRATION_PRIO_ORDER_MASK);
+
+		/*
+		 * Wa_1604223664:icl
+		 * Formerly known as WaL3BankAddressHashing
+		 */
+		wa_write_masked_or(wal,
+				   GEN8_GARBCNTL,
+				   GEN11_HASH_CTRL_EXCL_MASK,
+				   GEN11_HASH_CTRL_EXCL_BIT0);
+		wa_write_masked_or(wal,
+				   GEN11_GLBLINVL,
+				   GEN11_BANK_HASH_ADDR_EXCL_MASK,
+				   GEN11_BANK_HASH_ADDR_EXCL_BIT0);
+
+		/*
+		 * Wa_1405733216:icl
+		 * Formerly known as WaDisableCleanEvicts
+		 */
+		wa_write_or(wal,
+			    GEN8_L3SQCREG4,
+			    GEN11_LQSC_CLEAN_EVICT_DISABLE);
+
+		/* WaForwardProgressSoftReset:icl */
+		wa_write_or(wal,
+			    GEN10_SCRATCH_LNCF2,
+			    PMFLUSHDONE_LNICRSDROP |
+			    PMFLUSH_GAPL3UNBLOCK |
+			    PMFLUSHDONE_LNEBLK);
+	}
+}
+
+static void xcs_engine_wa_init(struct intel_engine_cs *engine)
+{
+	struct drm_i915_private *i915 = engine->i915;
+	struct i915_wa_list *wal = &engine->wa_list;
+
+	if (IS_KABYLAKE(i915)) {
+		/* WaKBLVECSSemaphoreWaitPoll:kbl */
+		if (IS_KBL_REVID(i915, KBL_REVID_A0, KBL_REVID_E0)) {
+			wa_write(wal,
+				 RING_SEMA_WAIT_POLL(engine->mmio_base),
+				 1);
+		}
+	}
+}
+
+void intel_engine_workarounds_init(struct intel_engine_cs *engine)
+{
+	struct i915_wa_list *wal = &engine->wa_list;
+
+	if (GEM_WARN_ON(INTEL_GEN(engine->i915) < 8))
+		return;
+
+	wa_init_start(wal, engine->name);
+
+	if (engine->id == RCS)
+		rcs_engine_wa_init(engine);
+	else
+		xcs_engine_wa_init(engine);
+
+	wa_init_finish(wal);
+}
+
+void intel_engine_workarounds_apply(struct intel_engine_cs *engine)
+{
+	wa_list_apply(engine->i915, &engine->wa_list);
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/intel_workarounds.c"
 #endif
diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
index 64aed9cf0200..2998767d51ca 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -36,4 +36,7 @@ void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv);
 
 void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine);
 
+void intel_engine_workarounds_init(struct intel_engine_cs *engine);
+void intel_engine_workarounds_apply(struct intel_engine_cs *engine);
+
 #endif
-- 
2.19.1

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

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

* [PATCH 3/7] drm/i915: Verify GT workaround state after GPU init
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
  2018-11-30 17:44 ` [PATCH 1/7] drm/i915: Record GT workarounds in a list Tvrtko Ursulin
  2018-11-30 17:44 ` [PATCH 2/7] drm/i915: Introduce per-engine workarounds Tvrtko Ursulin
@ 2018-11-30 17:44 ` Tvrtko Ursulin
  2018-11-30 21:55   ` Chris Wilson
  2018-11-30 17:44 ` [PATCH 4/7] drm/i915/selftests: Add tests for GT and engine workaround verification Tvrtko Ursulin
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 17:44 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Since we now have all the GT workarounds in a table, by adding a simple
shared helper function we can now verify that their values are still
applied after some interesting events in the lifetime of the driver.

Initially we only do this after GPU initialization.

v2:
 Chris Wilson:
 * Simplify verification by realizing it's a simple xor and and.
 * Remove verification from engine reset path.
 * Return bool straight away from the verify API.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c          |  1 +
 drivers/gpu/drm/i915/i915_gem.c          |  3 +++
 drivers/gpu/drm/i915/intel_workarounds.c | 34 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_workarounds.h |  2 ++
 4 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2f3dc1cf83a6..4883a20ed9ff 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -53,6 +53,7 @@
 #include "i915_vgpu.h"
 #include "intel_drv.h"
 #include "intel_uc.h"
+#include "intel_workarounds.h"
 
 static struct drm_driver driver;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 18adb3dd1fcd..1eff471d4366 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5334,7 +5334,10 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 		I915_WRITE(MI_PREDICATE_RESULT_2, IS_HSW_GT3(dev_priv) ?
 			   LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED);
 
+	/* Apply the GT workarounds... */
 	intel_gt_workarounds_apply(dev_priv);
+	/* ...and determine whether they are sticking. */
+	intel_gt_workarounds_verify(dev_priv, "init");
 
 	i915_gem_init_swizzling(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index be63a2af3481..d80ea8172222 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -981,6 +981,40 @@ void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
 	wa_list_apply(dev_priv, &dev_priv->gt_wa_list);
 }
 
+static bool
+wa_verify(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
+{
+	if ((cur ^ wa->val) & wa->mask) {
+		DRM_ERROR("%s workaround lost on %s! (%x=%x/%x, expected %x, mask=%x)\n",
+			  name, from, i915_mmio_reg_offset(wa->reg), cur,
+			  cur & wa->mask, wa->val, wa->mask);
+
+		return false;
+	}
+
+	return true;
+}
+
+static bool wa_list_verify(struct drm_i915_private *dev_priv,
+			   const struct i915_wa_list *wal,
+			   const char *from)
+{
+	struct i915_wa *wa;
+	unsigned int i;
+	bool ok = true;
+
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
+		ok &= wa_verify(wa, I915_READ(wa->reg), wal->name, from);
+
+	return ok;
+}
+
+bool intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
+				 const char *from)
+{
+	return wa_list_verify(dev_priv, &dev_priv->gt_wa_list, from);
+}
+
 struct whitelist {
 	i915_reg_t reg[RING_MAX_NONPRIV_SLOTS];
 	unsigned int count;
diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
index 2998767d51ca..f2a4e8c36027 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -33,6 +33,8 @@ int intel_ctx_workarounds_emit(struct i915_request *rq);
 
 void intel_gt_workarounds_init(struct drm_i915_private *dev_priv);
 void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv);
+bool intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
+				 const char *from);
 
 void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine);
 
-- 
2.19.1

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

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

* [PATCH 4/7] drm/i915/selftests: Add tests for GT and engine workaround verification
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2018-11-30 17:44 ` [PATCH 3/7] drm/i915: Verify GT workaround state after GPU init Tvrtko Ursulin
@ 2018-11-30 17:44 ` Tvrtko Ursulin
  2018-11-30 21:04   ` Chris Wilson
  2018-11-30 17:44 ` [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework Tvrtko Ursulin
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 17:44 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Two simple selftests which test that both GT and engine workarounds are
not lost after either a full GPU reset, or after the per-engine ones.

(Including checks that one engine reset is not affecting workarounds not
belonging to itself.)

v2:
 * Rebase for series refactoring.
 * Add spinner for actual engine reset!
 * Add idle reset test as well. (Chris Wilson)
 * Share existing global_reset_lock. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/intel_workarounds.c      |   6 +
 drivers/gpu/drm/i915/selftests/igt_common.c   |  44 ++++++
 drivers/gpu/drm/i915/selftests/igt_common.h   |  15 ++
 .../gpu/drm/i915/selftests/intel_hangcheck.c  |  51 ++----
 .../drm/i915/selftests/intel_workarounds.c    | 147 +++++++++++++++++-
 6 files changed, 217 insertions(+), 47 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/selftests/igt_common.c
 create mode 100644 drivers/gpu/drm/i915/selftests/igt_common.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 50a8fa8fce64..ceeb21f8aa0c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -166,6 +166,7 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \
 	selftests/i915_random.o \
 	selftests/i915_selftest.o \
 	selftests/igt_flush_test.o \
+	selftests/igt_common.o \
 	selftests/igt_spinner.o
 
 # virtual gpu code
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index d80ea8172222..5a4d70e02b63 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -1303,4 +1303,10 @@ void intel_engine_workarounds_apply(struct intel_engine_cs *engine)
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/intel_workarounds.c"
+
+bool intel_engine_workarounds_verify(struct intel_engine_cs *engine,
+				     const char *from)
+{
+	return wa_list_verify(engine->i915, &engine->wa_list, from);
+}
 #endif
diff --git a/drivers/gpu/drm/i915/selftests/igt_common.c b/drivers/gpu/drm/i915/selftests/igt_common.c
new file mode 100644
index 000000000000..ad00fdf895be
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/igt_common.c
@@ -0,0 +1,44 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2018 Intel Corporation
+ */
+
+#include "igt_common.h"
+
+#include "../i915_drv.h"
+#include "../intel_ringbuffer.h"
+
+void igt_global_reset_lock(struct drm_i915_private *i915)
+{
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+
+	pr_debug("%s: current gpu_error=%08lx\n",
+		 __func__, i915->gpu_error.flags);
+
+	while (test_and_set_bit(I915_RESET_BACKOFF, &i915->gpu_error.flags))
+		wait_event(i915->gpu_error.reset_queue,
+			   !test_bit(I915_RESET_BACKOFF,
+				     &i915->gpu_error.flags));
+
+	for_each_engine(engine, i915, id) {
+		while (test_and_set_bit(I915_RESET_ENGINE + id,
+					&i915->gpu_error.flags))
+			wait_on_bit(&i915->gpu_error.flags,
+				    I915_RESET_ENGINE + id,
+				    TASK_UNINTERRUPTIBLE);
+	}
+}
+
+void igt_global_reset_unlock(struct drm_i915_private *i915)
+{
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+
+	for_each_engine(engine, i915, id)
+		clear_bit(I915_RESET_ENGINE + id, &i915->gpu_error.flags);
+
+	clear_bit(I915_RESET_BACKOFF, &i915->gpu_error.flags);
+	wake_up_all(&i915->gpu_error.reset_queue);
+}
diff --git a/drivers/gpu/drm/i915/selftests/igt_common.h b/drivers/gpu/drm/i915/selftests/igt_common.h
new file mode 100644
index 000000000000..3df12ed10200
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/igt_common.h
@@ -0,0 +1,15 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2018 Intel Corporation
+ */
+
+#ifndef __I915_SELFTESTS_IGT_COMMON_H__
+#define __I915_SELFTESTS_IGT_COMMON_H__
+
+#include "../i915_drv.h"
+
+void igt_global_reset_lock(struct drm_i915_private *i915);
+void igt_global_reset_unlock(struct drm_i915_private *i915);
+
+#endif
diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
index defe671130ab..af31e4a4979d 100644
--- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
@@ -25,6 +25,7 @@
 #include <linux/kthread.h>
 
 #include "../i915_selftest.h"
+#include "igt_common.h"
 #include "i915_random.h"
 #include "igt_flush_test.h"
 #include "igt_wedge_me.h"
@@ -348,40 +349,6 @@ static int igt_hang_sanitycheck(void *arg)
 	return err;
 }
 
-static void global_reset_lock(struct drm_i915_private *i915)
-{
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-
-	pr_debug("%s: current gpu_error=%08lx\n",
-		 __func__, i915->gpu_error.flags);
-
-	while (test_and_set_bit(I915_RESET_BACKOFF, &i915->gpu_error.flags))
-		wait_event(i915->gpu_error.reset_queue,
-			   !test_bit(I915_RESET_BACKOFF,
-				     &i915->gpu_error.flags));
-
-	for_each_engine(engine, i915, id) {
-		while (test_and_set_bit(I915_RESET_ENGINE + id,
-					&i915->gpu_error.flags))
-			wait_on_bit(&i915->gpu_error.flags,
-				    I915_RESET_ENGINE + id,
-				    TASK_UNINTERRUPTIBLE);
-	}
-}
-
-static void global_reset_unlock(struct drm_i915_private *i915)
-{
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-
-	for_each_engine(engine, i915, id)
-		clear_bit(I915_RESET_ENGINE + id, &i915->gpu_error.flags);
-
-	clear_bit(I915_RESET_BACKOFF, &i915->gpu_error.flags);
-	wake_up_all(&i915->gpu_error.reset_queue);
-}
-
 static int igt_global_reset(void *arg)
 {
 	struct drm_i915_private *i915 = arg;
@@ -390,7 +357,7 @@ static int igt_global_reset(void *arg)
 
 	/* Check that we can issue a global GPU reset */
 
-	global_reset_lock(i915);
+	igt_global_reset_lock(i915);
 	set_bit(I915_RESET_HANDOFF, &i915->gpu_error.flags);
 
 	mutex_lock(&i915->drm.struct_mutex);
@@ -405,7 +372,7 @@ static int igt_global_reset(void *arg)
 	mutex_unlock(&i915->drm.struct_mutex);
 
 	GEM_BUG_ON(test_bit(I915_RESET_HANDOFF, &i915->gpu_error.flags));
-	global_reset_unlock(i915);
+	igt_global_reset_unlock(i915);
 
 	if (i915_terminally_wedged(&i915->gpu_error))
 		err = -EIO;
@@ -936,7 +903,7 @@ static int igt_reset_wait(void *arg)
 
 	/* Check that we detect a stuck waiter and issue a reset */
 
-	global_reset_lock(i915);
+	igt_global_reset_lock(i915);
 
 	mutex_lock(&i915->drm.struct_mutex);
 	err = hang_init(&h, i915);
@@ -988,7 +955,7 @@ static int igt_reset_wait(void *arg)
 	hang_fini(&h);
 unlock:
 	mutex_unlock(&i915->drm.struct_mutex);
-	global_reset_unlock(i915);
+	igt_global_reset_unlock(i915);
 
 	if (i915_terminally_wedged(&i915->gpu_error))
 		return -EIO;
@@ -1066,7 +1033,7 @@ static int __igt_reset_evict_vma(struct drm_i915_private *i915,
 
 	/* Check that we can recover an unbind stuck on a hanging request */
 
-	global_reset_lock(i915);
+	igt_global_reset_lock(i915);
 
 	mutex_lock(&i915->drm.struct_mutex);
 	err = hang_init(&h, i915);
@@ -1186,7 +1153,7 @@ static int __igt_reset_evict_vma(struct drm_i915_private *i915,
 	hang_fini(&h);
 unlock:
 	mutex_unlock(&i915->drm.struct_mutex);
-	global_reset_unlock(i915);
+	igt_global_reset_unlock(i915);
 
 	if (i915_terminally_wedged(&i915->gpu_error))
 		return -EIO;
@@ -1266,7 +1233,7 @@ static int igt_reset_queue(void *arg)
 
 	/* Check that we replay pending requests following a hang */
 
-	global_reset_lock(i915);
+	igt_global_reset_lock(i915);
 
 	mutex_lock(&i915->drm.struct_mutex);
 	err = hang_init(&h, i915);
@@ -1397,7 +1364,7 @@ static int igt_reset_queue(void *arg)
 	hang_fini(&h);
 unlock:
 	mutex_unlock(&i915->drm.struct_mutex);
-	global_reset_unlock(i915);
+	igt_global_reset_unlock(i915);
 
 	if (i915_terminally_wedged(&i915->gpu_error))
 		return -EIO;
diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index 80396b3592f5..194a3c30554d 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -6,6 +6,8 @@
 
 #include "../i915_selftest.h"
 
+#include "igt_common.h"
+#include "igt_flush_test.h"
 #include "igt_spinner.h"
 #include "igt_wedge_me.h"
 #include "mock_context.h"
@@ -290,7 +292,6 @@ static int live_reset_whitelist(void *arg)
 {
 	struct drm_i915_private *i915 = arg;
 	struct intel_engine_cs *engine = i915->engine[RCS];
-	struct i915_gpu_error *error = &i915->gpu_error;
 	struct whitelist w;
 	int err = 0;
 
@@ -302,8 +303,7 @@ static int live_reset_whitelist(void *arg)
 	if (!whitelist_build(engine, &w))
 		return 0;
 
-	set_bit(I915_RESET_BACKOFF, &error->flags);
-	set_bit(I915_RESET_ENGINE + engine->id, &error->flags);
+	igt_global_reset_lock(i915);
 
 	if (intel_has_reset_engine(i915)) {
 		err = check_whitelist_across_reset(engine,
@@ -322,15 +322,152 @@ static int live_reset_whitelist(void *arg)
 	}
 
 out:
-	clear_bit(I915_RESET_ENGINE + engine->id, &error->flags);
-	clear_bit(I915_RESET_BACKOFF, &error->flags);
+	igt_global_reset_unlock(i915);
 	return err;
 }
 
+bool intel_engine_workarounds_verify(struct intel_engine_cs *engine,
+				     const char *from);
+
+static bool verify_gt_engine_wa(struct drm_i915_private *i915, const char *str)
+{
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+	bool ok = true;
+
+	ok &= intel_gt_workarounds_verify(i915, str);
+
+	for_each_engine(engine, i915, id)
+		ok &= intel_engine_workarounds_verify(engine, str);
+
+	return ok;
+}
+
+static int
+live_gpu_reset_gt_engine_workarounds(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct i915_gpu_error *error = &i915->gpu_error;
+	bool ok;
+
+	if (!intel_has_gpu_reset(i915))
+		return 0;
+
+	pr_info("Verifying after GPU reset...\n");
+
+	igt_global_reset_lock(i915);
+
+	ok = verify_gt_engine_wa(i915, "before reset");
+	if (!ok)
+		goto out;
+
+	intel_runtime_pm_get(i915);
+	set_bit(I915_RESET_HANDOFF, &error->flags);
+	i915_reset(i915, ALL_ENGINES, "live_workarounds");
+	intel_runtime_pm_put(i915);
+
+	ok = verify_gt_engine_wa(i915, "after reset");
+
+out:
+	igt_global_reset_unlock(i915);
+
+	return ok ? 0 : -ESRCH;
+}
+
+static int
+live_engine_reset_gt_engine_workarounds(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct intel_engine_cs *engine;
+	struct i915_gem_context *ctx;
+	struct igt_spinner spin;
+	enum intel_engine_id id;
+	struct i915_request *rq;
+	int ret = 0;
+
+	if (!intel_has_reset_engine(i915))
+		return 0;
+
+	ctx = kernel_context(i915);
+	if (IS_ERR(ctx))
+		return PTR_ERR(ctx);
+
+	for_each_engine(engine, i915, id) {
+		bool ok;
+
+		pr_info("Verifying after %s reset...\n", engine->name);
+
+		igt_global_reset_lock(i915);
+
+		ok = verify_gt_engine_wa(i915, "before reset");
+		if (!ok) {
+			ret = -ESRCH;
+			goto err;
+		}
+
+		intel_runtime_pm_get(i915);
+		i915_reset_engine(engine, "live_workarounds");
+		intel_runtime_pm_put(i915);
+
+		ok = verify_gt_engine_wa(i915, "after idle reset");
+		if (!ok) {
+			ret = -ESRCH;
+			goto err;
+		}
+
+		ret = igt_spinner_init(&spin, i915);
+		if (ret)
+			goto err;
+
+		intel_runtime_pm_get(i915);
+
+		rq = igt_spinner_create_request(&spin, ctx, engine, MI_NOOP);
+		if (IS_ERR(rq)) {
+			ret = PTR_ERR(rq);
+			igt_spinner_fini(&spin);
+			goto err;
+		}
+
+		i915_request_add(rq);
+
+		if (!igt_wait_for_spinner(&spin, rq)) {
+			pr_err("Spinner failed to start\n");
+			igt_spinner_fini(&spin);
+			ret = -ETIMEDOUT;
+			goto err;
+		}
+
+		i915_reset_engine(engine, "live_workarounds");
+
+		intel_runtime_pm_put(i915);
+
+		igt_spinner_end(&spin);
+		igt_spinner_fini(&spin);
+
+		ok = verify_gt_engine_wa(i915, "after busy reset");
+		if (!ok) {
+			ret = -ESRCH;
+			goto err;
+		}
+
+		igt_global_reset_unlock(i915);
+	}
+
+err:
+	igt_global_reset_unlock(i915);
+	kernel_context_close(ctx);
+
+	igt_flush_test(i915, I915_WAIT_LOCKED);
+
+	return ret;
+}
+
 int intel_workarounds_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
 		SUBTEST(live_reset_whitelist),
+		SUBTEST(live_gpu_reset_gt_engine_workarounds),
+		SUBTEST(live_engine_reset_gt_engine_workarounds),
 	};
 	int err;
 
-- 
2.19.1

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

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

* [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2018-11-30 17:44 ` [PATCH 4/7] drm/i915/selftests: Add tests for GT and engine workaround verification Tvrtko Ursulin
@ 2018-11-30 17:44 ` Tvrtko Ursulin
  2018-11-30 21:08   ` Chris Wilson
  2018-11-30 17:44 ` [PATCH 6/7] drm/i915: Fuse per-context workaround handling with the common framework Tvrtko Ursulin
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 17:44 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Instead of having a separate list of white-listed registers we can
trivially move this to the common workarounds framework.

This brings us one step closer to the goal of driving all workaround
classes using the same code.

v2:
 * Use GEM_DEBUG_WARN_ON for the sanity check. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c        |  1 +
 drivers/gpu/drm/i915/intel_lrc.c              |  1 +
 drivers/gpu/drm/i915/intel_ringbuffer.h       |  1 +
 drivers/gpu/drm/i915/intel_workarounds.c      | 83 ++++++++-----------
 drivers/gpu/drm/i915/intel_workarounds.h      |  1 +
 .../drm/i915/selftests/intel_workarounds.c    | 40 ++++-----
 6 files changed, 57 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index ef5d202e9d45..496462d77ebc 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -725,6 +725,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
 	i915_timeline_fini(&engine->timeline);
 
 	intel_wa_list_free(&engine->wa_list);
+	intel_wa_list_free(&engine->whitelist);
 }
 
 u64 intel_engine_get_active_head(const struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index dfafc3f710d6..4eead104cd9c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2316,6 +2316,7 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
 			  ret);
 	}
 
+	intel_whitelist_workarounds_init(engine);
 	intel_engine_workarounds_init(engine);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c5ff3d31cab7..91a750e90dc4 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -453,6 +453,7 @@ struct intel_engine_cs {
 	struct intel_hw_status_page status_page;
 	struct i915_ctx_workarounds wa_ctx;
 	struct i915_wa_list wa_list;
+	struct i915_wa_list whitelist;
 	struct i915_vma *scratch;
 
 	u32             irq_keep_mask; /* always keep these interrupts */
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 5a4d70e02b63..9bd044b8e545 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -1015,29 +1015,20 @@ bool intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
 	return wa_list_verify(dev_priv, &dev_priv->gt_wa_list, from);
 }
 
-struct whitelist {
-	i915_reg_t reg[RING_MAX_NONPRIV_SLOTS];
-	unsigned int count;
-	u32 nopid;
-};
-
-static void whitelist_reg(struct whitelist *w, i915_reg_t reg)
+static void
+whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
 {
-	if (GEM_DEBUG_WARN_ON(w->count >= RING_MAX_NONPRIV_SLOTS))
-		return;
-
-	w->reg[w->count++] = reg;
-}
+	struct i915_wa wa = {
+		.reg = reg
+	};
 
-static void bdw_whitelist_build(struct whitelist *w)
-{
-}
+	if (GEM_DEBUG_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
+		return;
 
-static void chv_whitelist_build(struct whitelist *w)
-{
+	wal_add(wal, &wa);
 }
 
-static void gen9_whitelist_build(struct whitelist *w)
+static void gen9_whitelist_build(struct i915_wa_list *w)
 {
 	/* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */
 	whitelist_reg(w, GEN9_CTX_PREEMPT_REG);
@@ -1049,7 +1040,7 @@ static void gen9_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN8_HDC_CHICKEN1);
 }
 
-static void skl_whitelist_build(struct whitelist *w)
+static void skl_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 
@@ -1057,12 +1048,12 @@ static void skl_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN8_L3SQCREG4);
 }
 
-static void bxt_whitelist_build(struct whitelist *w)
+static void bxt_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 }
 
-static void kbl_whitelist_build(struct whitelist *w)
+static void kbl_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 
@@ -1070,7 +1061,7 @@ static void kbl_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN8_L3SQCREG4);
 }
 
-static void glk_whitelist_build(struct whitelist *w)
+static void glk_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 
@@ -1078,18 +1069,18 @@ static void glk_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN9_SLICE_COMMON_ECO_CHICKEN1);
 }
 
-static void cfl_whitelist_build(struct whitelist *w)
+static void cfl_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 }
 
-static void cnl_whitelist_build(struct whitelist *w)
+static void cnl_whitelist_build(struct i915_wa_list *w)
 {
 	/* WaEnablePreemptionGranularityControlByUMD:cnl */
 	whitelist_reg(w, GEN8_CS_CHICKEN1);
 }
 
-static void icl_whitelist_build(struct whitelist *w)
+static void icl_whitelist_build(struct i915_wa_list *w)
 {
 	/* WaAllowUMDToModifyHalfSliceChicken7:icl */
 	whitelist_reg(w, GEN9_HALF_SLICE_CHICKEN7);
@@ -1098,22 +1089,21 @@ static void icl_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN10_SAMPLER_MODE);
 }
 
-static struct whitelist *whitelist_build(struct intel_engine_cs *engine,
-					 struct whitelist *w)
+void intel_whitelist_workarounds_init(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *i915 = engine->i915;
+	struct i915_wa_list *w = &engine->whitelist;
 
 	GEM_BUG_ON(engine->id != RCS);
 
-	w->count = 0;
-	w->nopid = i915_mmio_reg_offset(RING_NOPID(engine->mmio_base));
+	wa_init_start(w, "whitelist");
 
 	if (INTEL_GEN(i915) < 8)
-		return NULL;
+		return;
 	else if (IS_BROADWELL(i915))
-		bdw_whitelist_build(w);
+		return;
 	else if (IS_CHERRYVIEW(i915))
-		chv_whitelist_build(w);
+		return;
 	else if (IS_SKYLAKE(i915))
 		skl_whitelist_build(w);
 	else if (IS_BROXTON(i915))
@@ -1131,37 +1121,30 @@ static struct whitelist *whitelist_build(struct intel_engine_cs *engine,
 	else
 		MISSING_CASE(INTEL_GEN(i915));
 
-	return w;
+	wa_init_finish(w);
 }
 
-static void whitelist_apply(struct intel_engine_cs *engine,
-			    const struct whitelist *w)
+void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	const struct i915_wa_list *wal = &engine->whitelist;
 	const u32 base = engine->mmio_base;
+	struct i915_wa *wa;
 	unsigned int i;
 
-	if (!w)
+	if (!wal->count)
 		return;
 
-	intel_uncore_forcewake_get(engine->i915, FORCEWAKE_ALL);
-
-	for (i = 0; i < w->count; i++)
-		I915_WRITE_FW(RING_FORCE_TO_NONPRIV(base, i),
-			      i915_mmio_reg_offset(w->reg[i]));
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
+		I915_WRITE(RING_FORCE_TO_NONPRIV(base, i),
+			   i915_mmio_reg_offset(wa->reg));
 
 	/* And clear the rest just in case of garbage */
 	for (; i < RING_MAX_NONPRIV_SLOTS; i++)
-		I915_WRITE_FW(RING_FORCE_TO_NONPRIV(base, i), w->nopid);
+		I915_WRITE(RING_FORCE_TO_NONPRIV(base, i),
+			   i915_mmio_reg_offset(RING_NOPID(base)));
 
-	intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL);
-}
-
-void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine)
-{
-	struct whitelist w;
-
-	whitelist_apply(engine, whitelist_build(engine, &w));
+	DRM_DEBUG_DRIVER("Applied %u %s workarounds\n", wal->count, wal->name);
 }
 
 static void rcs_engine_wa_init(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
index f2a4e8c36027..e6da31f0ceb6 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -36,6 +36,7 @@ void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv);
 bool intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
 				 const char *from);
 
+void intel_whitelist_workarounds_init(struct intel_engine_cs *engine);
 void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine);
 
 void intel_engine_workarounds_init(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index 194a3c30554d..ab69cbb8869e 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -94,17 +94,23 @@ read_nonprivs(struct i915_gem_context *ctx, struct intel_engine_cs *engine)
 	return ERR_PTR(err);
 }
 
-static u32 get_whitelist_reg(const struct whitelist *w, unsigned int i)
+static u32
+get_whitelist_reg(const struct intel_engine_cs *engine, unsigned int i)
 {
-	return i < w->count ? i915_mmio_reg_offset(w->reg[i]) : w->nopid;
+	i915_reg_t reg = i < engine->whitelist.count ?
+			 engine->whitelist.list[i].reg :
+			 RING_NOPID(engine->mmio_base);
+
+	return i915_mmio_reg_offset(reg);
 }
 
-static void print_results(const struct whitelist *w, const u32 *results)
+static void
+print_results(const struct intel_engine_cs *engine, const u32 *results)
 {
 	unsigned int i;
 
 	for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) {
-		u32 expected = get_whitelist_reg(w, i);
+		u32 expected = get_whitelist_reg(engine, i);
 		u32 actual = results[i];
 
 		pr_info("RING_NONPRIV[%d]: expected 0x%08x, found 0x%08x\n",
@@ -112,8 +118,7 @@ static void print_results(const struct whitelist *w, const u32 *results)
 	}
 }
 
-static int check_whitelist(const struct whitelist *w,
-			   struct i915_gem_context *ctx,
+static int check_whitelist(struct i915_gem_context *ctx,
 			   struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_object *results;
@@ -141,11 +146,11 @@ static int check_whitelist(const struct whitelist *w,
 	}
 
 	for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) {
-		u32 expected = get_whitelist_reg(w, i);
+		u32 expected = get_whitelist_reg(engine, i);
 		u32 actual = vaddr[i];
 
 		if (expected != actual) {
-			print_results(w, vaddr);
+			print_results(engine, vaddr);
 			pr_err("Invalid RING_NONPRIV[%d], expected 0x%08x, found 0x%08x\n",
 			       i, expected, actual);
 
@@ -217,7 +222,6 @@ switch_to_scratch_context(struct intel_engine_cs *engine,
 
 static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 					int (*reset)(struct intel_engine_cs *),
-					const struct whitelist *w,
 					const char *name)
 {
 	struct drm_i915_private *i915 = engine->i915;
@@ -227,7 +231,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 	int err;
 
 	pr_info("Checking %d whitelisted registers (RING_NONPRIV) [%s]\n",
-		w->count, name);
+		engine->whitelist.count, name);
 
 	if (want_spin) {
 		err = igt_spinner_init(&spin, i915);
@@ -239,7 +243,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 
-	err = check_whitelist(w, ctx, engine);
+	err = check_whitelist(ctx, engine);
 	if (err) {
 		pr_err("Invalid whitelist *before* %s reset!\n", name);
 		goto out;
@@ -263,7 +267,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 		goto out;
 	}
 
-	err = check_whitelist(w, ctx, engine);
+	err = check_whitelist(ctx, engine);
 	if (err) {
 		pr_err("Whitelist not preserved in context across %s reset!\n",
 		       name);
@@ -276,7 +280,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 
-	err = check_whitelist(w, ctx, engine);
+	err = check_whitelist(ctx, engine);
 	if (err) {
 		pr_err("Invalid whitelist *after* %s reset in fresh context!\n",
 		       name);
@@ -292,22 +296,18 @@ static int live_reset_whitelist(void *arg)
 {
 	struct drm_i915_private *i915 = arg;
 	struct intel_engine_cs *engine = i915->engine[RCS];
-	struct whitelist w;
 	int err = 0;
 
 	/* If we reset the gpu, we should not lose the RING_NONPRIV */
 
-	if (!engine)
-		return 0;
-
-	if (!whitelist_build(engine, &w))
+	if (!engine || engine->whitelist.count == 0)
 		return 0;
 
 	igt_global_reset_lock(i915);
 
 	if (intel_has_reset_engine(i915)) {
 		err = check_whitelist_across_reset(engine,
-						   do_engine_reset, &w,
+						   do_engine_reset,
 						   "engine");
 		if (err)
 			goto out;
@@ -315,7 +315,7 @@ static int live_reset_whitelist(void *arg)
 
 	if (intel_has_gpu_reset(i915)) {
 		err = check_whitelist_across_reset(engine,
-						   do_device_reset, &w,
+						   do_device_reset,
 						   "device");
 		if (err)
 			goto out;
-- 
2.19.1

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

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

* [PATCH 6/7] drm/i915: Fuse per-context workaround handling with the common framework
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
                   ` (4 preceding siblings ...)
  2018-11-30 17:44 ` [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework Tvrtko Ursulin
@ 2018-11-30 17:44 ` Tvrtko Ursulin
  2018-11-30 21:22   ` Chris Wilson
  2018-11-30 17:44 ` [PATCH 7/7] drm/i915: Trim unused workaround list entries Tvrtko Ursulin
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 17:44 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Convert the per context workaround handling code to run against the newly
introduced common workaround framework and fuse the two to use the
existing smarter list add helper, the one which does the sorted insert and
merges registers where possible.

This completes migration of all four classes of workarounds onto the
common framework.

Existing macros are kept untouched for smaller code churn.

v2:
 * Rename to list name ctx_wa_list and move from dev_priv to engine.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c      |  12 +-
 drivers/gpu/drm/i915/i915_drv.h          |  15 --
 drivers/gpu/drm/i915/i915_gem_context.c  |   6 +-
 drivers/gpu/drm/i915/intel_engine_cs.c   |   1 +
 drivers/gpu/drm/i915/intel_ringbuffer.h  |   1 +
 drivers/gpu/drm/i915/intel_workarounds.c | 267 ++++++++++-------------
 drivers/gpu/drm/i915/intel_workarounds.h |   3 +-
 7 files changed, 129 insertions(+), 176 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 129b9a6f8309..38dcee1ca062 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3375,13 +3375,15 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 
 static int i915_wa_registers(struct seq_file *m, void *unused)
 {
-	struct i915_workarounds *wa = &node_to_i915(m->private)->workarounds;
-	int i;
+	struct drm_i915_private *i915 = node_to_i915(m->private);
+	const struct i915_wa_list *wal = &i915->engine[RCS]->ctx_wa_list;
+	struct i915_wa *wa;
+	unsigned int i;
 
-	seq_printf(m, "Workarounds applied: %d\n", wa->count);
-	for (i = 0; i < wa->count; ++i)
+	seq_printf(m, "Workarounds applied: %u\n", wal->count);
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
 		seq_printf(m, "0x%X: 0x%08X, mask: 0x%08X\n",
-			   wa->reg[i].addr, wa->reg[i].value, wa->reg[i].mask);
+			   i915_mmio_reg_offset(wa->reg), wa->val, wa->mask);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9ddbcc1f3554..443e08f4736a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1189,20 +1189,6 @@ struct i915_frontbuffer_tracking {
 	unsigned flip_bits;
 };
 
-struct i915_wa_reg {
-	u32 addr;
-	u32 value;
-	/* bitmask representing WA bits */
-	u32 mask;
-};
-
-#define I915_MAX_WA_REGS 16
-
-struct i915_workarounds {
-	struct i915_wa_reg reg[I915_MAX_WA_REGS];
-	u32 count;
-};
-
 struct i915_virtual_gpu {
 	bool active;
 	u32 caps;
@@ -1652,7 +1638,6 @@ struct drm_i915_private {
 
 	int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
 
-	struct i915_workarounds workarounds;
 	struct i915_wa_list gt_wa_list;
 
 	struct i915_frontbuffer_tracking fb_tracking;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index b97963db0287..aae7e13c7420 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -535,16 +535,12 @@ static bool needs_preempt_context(struct drm_i915_private *i915)
 int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
 {
 	struct i915_gem_context *ctx;
-	int ret;
 
 	/* Reassure ourselves we are only called once */
 	GEM_BUG_ON(dev_priv->kernel_context);
 	GEM_BUG_ON(dev_priv->preempt_context);
 
-	ret = intel_ctx_workarounds_init(dev_priv);
-	if (ret)
-		return ret;
-
+	intel_ctx_workarounds_init(dev_priv);
 	init_contexts(dev_priv);
 
 	/* lowest priority; idle task */
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 496462d77ebc..6b427bc52f78 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -724,6 +724,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
 
 	i915_timeline_fini(&engine->timeline);
 
+	intel_wa_list_free(&engine->ctx_wa_list);
 	intel_wa_list_free(&engine->wa_list);
 	intel_wa_list_free(&engine->whitelist);
 }
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 91a750e90dc4..8f985c35ec92 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -452,6 +452,7 @@ struct intel_engine_cs {
 
 	struct intel_hw_status_page status_page;
 	struct i915_ctx_workarounds wa_ctx;
+	struct i915_wa_list ctx_wa_list;
 	struct i915_wa_list wa_list;
 	struct i915_wa_list whitelist;
 	struct i915_vma *scratch;
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 9bd044b8e545..3e6b388ea022 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -57,61 +57,85 @@ static void wa_init_finish(struct i915_wa_list *wal)
 {
 	if (wal->count)
 		DRM_DEBUG_DRIVER("Initialized %u %s workarounds\n",
-				 wal->count, wal->name);
+				 wal->wa_count, wal->name);
 }
 
-static void wa_add(struct drm_i915_private *i915,
-		   i915_reg_t reg, const u32 mask, const u32 val)
+static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
 {
-	struct i915_workarounds *wa = &i915->workarounds;
-	unsigned int start = 0, end = wa->count;
-	unsigned int addr = i915_mmio_reg_offset(reg);
-	struct i915_wa_reg *r;
+	unsigned int addr = i915_mmio_reg_offset(wa->reg);
+	unsigned int start = 0, end = wal->count;
+	const unsigned int grow = 16;
+	struct i915_wa *wa_;
+
+	if (wal->__size == wal->count) {
+		struct i915_wa *list;
+
+		list = kcalloc(wal->__size + grow, sizeof(*wa), GFP_KERNEL);
+		if (!list) {
+			DRM_ERROR("No space for workaround init!\n");
+			return;
+		}
+
+		if (wal->list)
+			memcpy(list, wal->list, sizeof(*wa) * wal->count);
+
+		wal->list = list;
+		wal->__size += grow;
+	}
 
 	while (start < end) {
 		unsigned int mid = start + (end - start) / 2;
 
-		if (wa->reg[mid].addr < addr) {
+		if (i915_mmio_reg_offset(wal->list[mid].reg) < addr) {
 			start = mid + 1;
-		} else if (wa->reg[mid].addr > addr) {
+		} else if (i915_mmio_reg_offset(wal->list[mid].reg) > addr) {
 			end = mid;
 		} else {
-			r = &wa->reg[mid];
+			wa_ = &wal->list[mid];
 
-			if ((mask & ~r->mask) == 0) {
+			if ((wa->mask & ~wa_->mask) == 0) {
 				DRM_ERROR("Discarding overwritten w/a for reg %04x (mask: %08x, value: %08x)\n",
-					  addr, r->mask, r->value);
+					  i915_mmio_reg_offset(wa_->reg),
+					  wa_->mask, wa_->val);
 
-				r->value &= ~mask;
+				wa_->val &= ~wa->mask;
 			}
 
-			r->value |= val;
-			r->mask  |= mask;
+			wal->wa_count++;
+			wa_->val |= wa->val;
+			wa_->mask |= wa->mask;
 			return;
 		}
 	}
 
-	if (WARN_ON_ONCE(wa->count >= I915_MAX_WA_REGS)) {
-		DRM_ERROR("Dropping w/a for reg %04x (mask: %08x, value: %08x)\n",
-			  addr, mask, val);
-		return;
-	}
+	wal->wa_count++;
+	wa_ = &wal->list[wal->count++];
+	*wa_ = *wa;
 
-	r = &wa->reg[wa->count++];
-	r->addr  = addr;
-	r->value = val;
-	r->mask  = mask;
-
-	while (r-- > wa->reg) {
-		GEM_BUG_ON(r[0].addr == r[1].addr);
-		if (r[1].addr > r[0].addr)
+	while (wa_-- > wal->list) {
+		GEM_BUG_ON(i915_mmio_reg_offset(wa_[0].reg) ==
+			   i915_mmio_reg_offset(wa_[1].reg));
+		if (i915_mmio_reg_offset(wa_[1].reg) >
+		    i915_mmio_reg_offset(wa_[0].reg))
 			break;
 
-		swap(r[1], r[0]);
+		swap(wa_[1], wa_[0]);
 	}
 }
 
-#define WA_REG(addr, mask, val) wa_add(dev_priv, (addr), (mask), (val))
+static void
+__wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val)
+{
+	struct i915_wa wa = {
+		.reg = reg,
+		.mask = mask,
+		.val = val
+	};
+
+	_wa_add(wal, &wa);
+}
+
+#define WA_REG(addr, mask, val) __wa_add(wal, (addr), (mask), (val))
 
 #define WA_SET_BIT_MASKED(addr, mask) \
 	WA_REG(addr, (mask), _MASKED_BIT_ENABLE(mask))
@@ -122,8 +146,10 @@ static void wa_add(struct drm_i915_private *i915,
 #define WA_SET_FIELD_MASKED(addr, mask, value) \
 	WA_REG(addr, (mask), _MASKED_FIELD(mask, value))
 
-static int gen8_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void gen8_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
+
 	WA_SET_BIT_MASKED(INSTPM, INSTPM_FORCE_ORDERING);
 
 	/* WaDisableAsyncFlipPerfMode:bdw,chv */
@@ -167,17 +193,13 @@ static int gen8_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 	WA_SET_FIELD_MASKED(GEN7_GT_MODE,
 			    GEN6_WIZ_HASHING_MASK,
 			    GEN6_WIZ_HASHING_16x4);
-
-	return 0;
 }
 
-static int bdw_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void bdw_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	int ret;
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
 
-	ret = gen8_ctx_workarounds_init(dev_priv);
-	if (ret)
-		return ret;
+	gen8_ctx_workarounds_init(dev_priv);
 
 	/* WaDisableThreadStallDopClockGating:bdw (pre-production) */
 	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, STALL_DOP_GATING_DISABLE);
@@ -198,29 +220,25 @@ static int bdw_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 			  HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT |
 			  /* WaDisableFenceDestinationToSLM:bdw (pre-prod) */
 			  (IS_BDW_GT3(dev_priv) ? HDC_FENCE_DEST_SLM_DISABLE : 0));
-
-	return 0;
 }
 
-static int chv_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void chv_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	int ret;
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
 
-	ret = gen8_ctx_workarounds_init(dev_priv);
-	if (ret)
-		return ret;
+	gen8_ctx_workarounds_init(dev_priv);
 
 	/* WaDisableThreadStallDopClockGating:chv */
 	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, STALL_DOP_GATING_DISABLE);
 
 	/* Improve HiZ throughput on CHV. */
 	WA_SET_BIT_MASKED(HIZ_CHICKEN, CHV_HZ_8X8_MODE_IN_1X);
-
-	return 0;
 }
 
-static int gen9_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void gen9_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
+
 	if (HAS_LLC(dev_priv)) {
 		/* WaCompressedResourceSamplerPbeMediaNewHashMode:skl,kbl
 		 *
@@ -314,12 +332,11 @@ static int gen9_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 	/* WaClearHIZ_WM_CHICKEN3:bxt,glk */
 	if (IS_GEN9_LP(dev_priv))
 		WA_SET_BIT_MASKED(GEN9_WM_CHICKEN3, GEN9_FACTOR_IN_CLR_VAL_HIZ);
-
-	return 0;
 }
 
-static int skl_tune_iz_hashing(struct drm_i915_private *dev_priv)
+static void skl_tune_iz_hashing(struct drm_i915_private *dev_priv)
 {
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
 	u8 vals[3] = { 0, 0, 0 };
 	unsigned int i;
 
@@ -344,7 +361,7 @@ static int skl_tune_iz_hashing(struct drm_i915_private *dev_priv)
 	}
 
 	if (vals[0] == 0 && vals[1] == 0 && vals[2] == 0)
-		return 0;
+		return;
 
 	/* Tune IZ hashing. See intel_device_info_runtime_init() */
 	WA_SET_FIELD_MASKED(GEN7_GT_MODE,
@@ -354,28 +371,19 @@ static int skl_tune_iz_hashing(struct drm_i915_private *dev_priv)
 			    GEN9_IZ_HASHING(2, vals[2]) |
 			    GEN9_IZ_HASHING(1, vals[1]) |
 			    GEN9_IZ_HASHING(0, vals[0]));
-
-	return 0;
 }
 
-static int skl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void skl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	int ret;
-
-	ret = gen9_ctx_workarounds_init(dev_priv);
-	if (ret)
-		return ret;
-
-	return skl_tune_iz_hashing(dev_priv);
+	gen9_ctx_workarounds_init(dev_priv);
+	skl_tune_iz_hashing(dev_priv);
 }
 
-static int bxt_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void bxt_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	int ret;
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
 
-	ret = gen9_ctx_workarounds_init(dev_priv);
-	if (ret)
-		return ret;
+	gen9_ctx_workarounds_init(dev_priv);
 
 	/* WaDisableThreadStallDopClockGating:bxt */
 	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
@@ -384,17 +392,13 @@ static int bxt_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 	/* WaToEnableHwFixForPushConstHWBug:bxt */
 	WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,
 			  GEN8_SBE_DISABLE_REPLAY_BUF_OPTIMIZATION);
-
-	return 0;
 }
 
-static int kbl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void kbl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	int ret;
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
 
-	ret = gen9_ctx_workarounds_init(dev_priv);
-	if (ret)
-		return ret;
+	gen9_ctx_workarounds_init(dev_priv);
 
 	/* WaToEnableHwFixForPushConstHWBug:kbl */
 	if (IS_KBL_REVID(dev_priv, KBL_REVID_C0, REVID_FOREVER))
@@ -404,32 +408,24 @@ static int kbl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 	/* WaDisableSbeCacheDispatchPortSharing:kbl */
 	WA_SET_BIT_MASKED(GEN7_HALF_SLICE_CHICKEN1,
 			  GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
-
-	return 0;
 }
 
-static int glk_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void glk_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	int ret;
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
 
-	ret = gen9_ctx_workarounds_init(dev_priv);
-	if (ret)
-		return ret;
+	gen9_ctx_workarounds_init(dev_priv);
 
 	/* WaToEnableHwFixForPushConstHWBug:glk */
 	WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,
 			  GEN8_SBE_DISABLE_REPLAY_BUF_OPTIMIZATION);
-
-	return 0;
 }
 
-static int cfl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void cfl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	int ret;
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
 
-	ret = gen9_ctx_workarounds_init(dev_priv);
-	if (ret)
-		return ret;
+	gen9_ctx_workarounds_init(dev_priv);
 
 	/* WaToEnableHwFixForPushConstHWBug:cfl */
 	WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,
@@ -438,12 +434,12 @@ static int cfl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 	/* WaDisableSbeCacheDispatchPortSharing:cfl */
 	WA_SET_BIT_MASKED(GEN7_HALF_SLICE_CHICKEN1,
 			  GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
-
-	return 0;
 }
 
-static int cnl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void cnl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
+
 	/* WaForceContextSaveRestoreNonCoherent:cnl */
 	WA_SET_BIT_MASKED(CNL_HDC_CHICKEN0,
 			  HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT);
@@ -477,12 +473,12 @@ static int cnl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 
 	/* WaDisableEarlyEOT:cnl */
 	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN, DISABLE_EARLY_EOT);
-
-	return 0;
 }
 
-static int icl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+static void icl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
+
 	/* Wa_1604370585:icl (pre-prod)
 	 * Formerly known as WaPushConstantDereferenceHoldDisable
 	 */
@@ -514,67 +510,63 @@ static int icl_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 	if (IS_ICL_REVID(dev_priv, ICL_REVID_A0, ICL_REVID_A0))
 		WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3,
 				  GEN11_BLEND_EMB_FIX_DISABLE_IN_RCC);
-
-	return 0;
 }
 
-int intel_ctx_workarounds_init(struct drm_i915_private *dev_priv)
+void intel_ctx_workarounds_init(struct drm_i915_private *dev_priv)
 {
-	int err = 0;
+	struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
 
-	dev_priv->workarounds.count = 0;
+	wa_init_start(wal, "context");
 
 	if (INTEL_GEN(dev_priv) < 8)
-		err = 0;
+		return;
 	else if (IS_BROADWELL(dev_priv))
-		err = bdw_ctx_workarounds_init(dev_priv);
+		bdw_ctx_workarounds_init(dev_priv);
 	else if (IS_CHERRYVIEW(dev_priv))
-		err = chv_ctx_workarounds_init(dev_priv);
+		chv_ctx_workarounds_init(dev_priv);
 	else if (IS_SKYLAKE(dev_priv))
-		err = skl_ctx_workarounds_init(dev_priv);
+		skl_ctx_workarounds_init(dev_priv);
 	else if (IS_BROXTON(dev_priv))
-		err = bxt_ctx_workarounds_init(dev_priv);
+		bxt_ctx_workarounds_init(dev_priv);
 	else if (IS_KABYLAKE(dev_priv))
-		err = kbl_ctx_workarounds_init(dev_priv);
+		kbl_ctx_workarounds_init(dev_priv);
 	else if (IS_GEMINILAKE(dev_priv))
-		err = glk_ctx_workarounds_init(dev_priv);
+		glk_ctx_workarounds_init(dev_priv);
 	else if (IS_COFFEELAKE(dev_priv))
-		err = cfl_ctx_workarounds_init(dev_priv);
+		cfl_ctx_workarounds_init(dev_priv);
 	else if (IS_CANNONLAKE(dev_priv))
-		err = cnl_ctx_workarounds_init(dev_priv);
+		cnl_ctx_workarounds_init(dev_priv);
 	else if (IS_ICELAKE(dev_priv))
-		err = icl_ctx_workarounds_init(dev_priv);
+		icl_ctx_workarounds_init(dev_priv);
 	else
 		MISSING_CASE(INTEL_GEN(dev_priv));
-	if (err)
-		return err;
 
-	DRM_DEBUG_DRIVER("Number of context specific w/a: %d\n",
-			 dev_priv->workarounds.count);
-	return 0;
+	wa_init_finish(wal);
 }
 
 int intel_ctx_workarounds_emit(struct i915_request *rq)
 {
-	struct i915_workarounds *w = &rq->i915->workarounds;
+	struct i915_wa_list *wal = &rq->engine->ctx_wa_list;
+	struct i915_wa *wa;
+	unsigned int i;
 	u32 *cs;
-	int ret, i;
+	int ret;
 
-	if (w->count == 0)
+	if (wal->count == 0)
 		return 0;
 
 	ret = rq->engine->emit_flush(rq, EMIT_BARRIER);
 	if (ret)
 		return ret;
 
-	cs = intel_ring_begin(rq, (w->count * 2 + 2));
+	cs = intel_ring_begin(rq, (wal->count * 2 + 2));
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(w->count);
-	for (i = 0; i < w->count; i++) {
-		*cs++ = w->reg[i].addr;
-		*cs++ = w->reg[i].value;
+	*cs++ = MI_LOAD_REGISTER_IMM(wal->count);
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
+		*cs++ = i915_mmio_reg_offset(wa->reg);
+		*cs++ = wa->val;
 	}
 	*cs++ = MI_NOOP;
 
@@ -587,31 +579,6 @@ int intel_ctx_workarounds_emit(struct i915_request *rq)
 	return 0;
 }
 
-static void
-wal_add(struct i915_wa_list *wal, const struct i915_wa *wa)
-{
-	const unsigned int grow = 4;
-
-	if (wal->__size == wal->count) {
-		struct i915_wa *list;
-
-		list = kcalloc(wal->__size + grow, sizeof(*wa), GFP_KERNEL);
-		if (!list) {
-			DRM_ERROR("No space for workaround init!\n");
-			return;
-		}
-
-		if (wal->list)
-			memcpy(list, wal->list, sizeof(*wa) * wal->count);
-
-		wal->list = list;
-		wal->__size += grow;
-	}
-
-	memcpy(&wal->list[wal->count], wa, sizeof(*wa));
-	wal->count++;
-}
-
 static void
 wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
 {
@@ -621,7 +588,7 @@ wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
 		.val = _MASKED_BIT_ENABLE(val)
 	};
 
-	wal_add(wal, &wa);
+	_wa_add(wal, &wa);
 }
 
 static void
@@ -634,7 +601,7 @@ wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
 		.val = val
 	};
 
-	wal_add(wal, &wa);
+	_wa_add(wal, &wa);
 }
 
 static void
@@ -1025,7 +992,7 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
 	if (GEM_DEBUG_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
 		return;
 
-	wal_add(wal, &wa);
+	_wa_add(wal, &wa);
 }
 
 static void gen9_whitelist_build(struct i915_wa_list *w)
diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
index e6da31f0ceb6..2129d6e9fcc9 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -18,6 +18,7 @@ struct i915_wa {
 struct i915_wa_list {
 	const char	*name;
 	unsigned int 	count;
+	unsigned int	wa_count;
 	struct i915_wa	*list;
 	unsigned int	__size;
 };
@@ -28,7 +29,7 @@ static inline void intel_wa_list_free(struct i915_wa_list *wal)
 	memset(wal, 0, sizeof(*wal));
 }
 
-int intel_ctx_workarounds_init(struct drm_i915_private *dev_priv);
+void intel_ctx_workarounds_init(struct drm_i915_private *dev_priv);
 int intel_ctx_workarounds_emit(struct i915_request *rq);
 
 void intel_gt_workarounds_init(struct drm_i915_private *dev_priv);
-- 
2.19.1

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

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

* [PATCH 7/7] drm/i915: Trim unused workaround list entries
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
                   ` (5 preceding siblings ...)
  2018-11-30 17:44 ` [PATCH 6/7] drm/i915: Fuse per-context workaround handling with the common framework Tvrtko Ursulin
@ 2018-11-30 17:44 ` Tvrtko Ursulin
  2018-11-30 21:58   ` Chris Wilson
  2018-11-30 17:53 ` ✗ Fi.CI.CHECKPATCH: warning for Restore workarounds after engine reset and unify their handling (rev2) Patchwork
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 17:44 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

The new workaround list allocator grows the list in chunks so will end up
with some unused space. Trim it when the initialization phase is done to
free up a tiny bit of slab.

v2:
 * Simplify with kmemdup. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_workarounds.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 3e6b388ea022..9d876d554e57 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -55,6 +55,19 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name)
 
 static void wa_init_finish(struct i915_wa_list *wal)
 {
+	/* Trim unused entries. */
+	if (wal->count < wal->__size) {
+		struct i915_wa *list = kmemdup(wal->list,
+					       wal->count * sizeof(*list),
+					       GFP_KERNEL);
+
+		if (list) {
+			kfree(wal->list);
+			wal->list = list;
+			wal->__size = wal->count;
+		}
+	}
+
 	if (wal->count)
 		DRM_DEBUG_DRIVER("Initialized %u %s workarounds\n",
 				 wal->wa_count, wal->name);
-- 
2.19.1

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

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

* ✗ Fi.CI.CHECKPATCH: warning for Restore workarounds after engine reset and unify their handling (rev2)
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
                   ` (6 preceding siblings ...)
  2018-11-30 17:44 ` [PATCH 7/7] drm/i915: Trim unused workaround list entries Tvrtko Ursulin
@ 2018-11-30 17:53 ` Patchwork
  2018-11-30 17:56 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-11-30 17:53 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: Restore workarounds after engine reset and unify their handling (rev2)
URL   : https://patchwork.freedesktop.org/series/53313/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
1125245b3d4b drm/i915: Record GT workarounds in a list
-:460: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#460: FILE: drivers/gpu/drm/i915/intel_workarounds.c:912:
+	wa_masked_en(wal,
+		    _3D_CHICKEN3,

-:722: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#722: FILE: drivers/gpu/drm/i915/intel_workarounds.h:20:
+^Iunsigned int ^Icount;$

total: 0 errors, 1 warnings, 1 checks, 685 lines checked
42b374d1ba00 drm/i915: Introduce per-engine workarounds
-:10: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 59b449d5c82a ("drm/i915: Split out functions for different kinds of workarounds")'
#10: 
59b449d5c82a ("drm/i915: Split out functions for different kinds of

total: 1 errors, 0 warnings, 0 checks, 371 lines checked
e9a04e2832a9 drm/i915: Verify GT workaround state after GPU init
6549917a0bd0 drm/i915/selftests: Add tests for GT and engine workaround verification
-:49: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#49: 
new file mode 100644

-:54: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#54: FILE: drivers/gpu/drm/i915/selftests/igt_common.c:1:
+/*

-:104: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#104: FILE: drivers/gpu/drm/i915/selftests/igt_common.h:1:
+/*

total: 0 errors, 3 warnings, 0 checks, 365 lines checked
a3beaa49924a drm/i915: Move register white-listing to the common workaround framework
f4ea75c6a6cd drm/i915: Fuse per-context workaround handling with the common framework
b7af01a81fb8 drm/i915: Trim unused workaround list entries

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

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

* ✗ Fi.CI.SPARSE: warning for Restore workarounds after engine reset and unify their handling (rev2)
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
                   ` (7 preceding siblings ...)
  2018-11-30 17:53 ` ✗ Fi.CI.CHECKPATCH: warning for Restore workarounds after engine reset and unify their handling (rev2) Patchwork
@ 2018-11-30 17:56 ` Patchwork
  2018-11-30 18:24 ` ✓ Fi.CI.BAT: success " Patchwork
  2018-12-01 13:36 ` ✗ Fi.CI.IGT: failure " Patchwork
  10 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-11-30 17:56 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: Restore workarounds after engine reset and unify their handling (rev2)
URL   : https://patchwork.freedesktop.org/series/53313/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Record GT workarounds in a list
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3570:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3572:16: warning: expression using sizeof(void)
+./include/linux/slab.h:665:13: error: undefined identifier '__builtin_mul_overflow'
+./include/linux/slab.h:665:13: warning: call with no type!

Commit: drm/i915: Introduce per-engine workarounds
Okay!

Commit: drm/i915: Verify GT workaround state after GPU init
Okay!

Commit: drm/i915/selftests: Add tests for GT and engine workaround verification
+./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)

Commit: drm/i915: Move register white-listing to the common workaround framework
Okay!

Commit: drm/i915: Fuse per-context workaround handling with the common framework
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3572:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3557:16: warning: expression using sizeof(void)

Commit: drm/i915: Trim unused workaround list entries
Okay!

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

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

* ✓ Fi.CI.BAT: success for Restore workarounds after engine reset and unify their handling (rev2)
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
                   ` (8 preceding siblings ...)
  2018-11-30 17:56 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-11-30 18:24 ` Patchwork
  2018-12-01 13:36 ` ✗ Fi.CI.IGT: failure " Patchwork
  10 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-11-30 18:24 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: Restore workarounds after engine reset and unify their handling (rev2)
URL   : https://patchwork.freedesktop.org/series/53313/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5235 -> Patchwork_10981
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53313/revisions/2/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       PASS -> DMESG-WARN [fdo#102614]

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - fi-byt-clapper:     NOTRUN -> FAIL [fdo#107362]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-kefka:       FAIL [fdo#108656] -> PASS

  * igt@gem_exec_suspend@basic-s3:
    - fi-byt-clapper:     INCOMPLETE [fdo#102657] -> PASS

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] -> PASS

  * igt@i915_selftest@live_hangcheck:
    - fi-kbl-7560u:       INCOMPLETE [fdo#108044] -> PASS

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108044]: https://bugs.freedesktop.org/show_bug.cgi?id=108044
  [fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656


Participating hosts (50 -> 44)
------------------------------

  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_5235 -> Patchwork_10981

  CI_DRM_5235: 5e327aec3fa3fc277b40818e06785743cf16b1ad @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10981: b7af01a81fb8c4c250ba60f24eb7b0272bcf7d3f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

b7af01a81fb8 drm/i915: Trim unused workaround list entries
f4ea75c6a6cd drm/i915: Fuse per-context workaround handling with the common framework
a3beaa49924a drm/i915: Move register white-listing to the common workaround framework
6549917a0bd0 drm/i915/selftests: Add tests for GT and engine workaround verification
e9a04e2832a9 drm/i915: Verify GT workaround state after GPU init
42b374d1ba00 drm/i915: Introduce per-engine workarounds
1125245b3d4b drm/i915: Record GT workarounds in a list

== Logs ==

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

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

* Re: [PATCH 2/7] drm/i915: Introduce per-engine workarounds
  2018-11-30 17:44 ` [PATCH 2/7] drm/i915: Introduce per-engine workarounds Tvrtko Ursulin
@ 2018-11-30 20:56   ` Chris Wilson
  2018-11-30 21:53   ` Chris Wilson
  2018-11-30 22:13   ` Rodrigo Vivi
  2 siblings, 0 replies; 27+ messages in thread
From: Chris Wilson @ 2018-11-30 20:56 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin; +Cc: intel-gfx, Rodrigo Vivi

Quoting Tvrtko Ursulin (2018-11-30 17:44:07)
> +static void xcs_engine_wa_init(struct intel_engine_cs *engine)
> +{
> +       struct drm_i915_private *i915 = engine->i915;
> +       struct i915_wa_list *wal = &engine->wa_list;
> +
> +       if (IS_KABYLAKE(i915)) {
> +               /* WaKBLVECSSemaphoreWaitPoll:kbl */
> +               if (IS_KBL_REVID(i915, KBL_REVID_A0, KBL_REVID_E0)) {

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

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

* Re: [PATCH 4/7] drm/i915/selftests: Add tests for GT and engine workaround verification
  2018-11-30 17:44 ` [PATCH 4/7] drm/i915/selftests: Add tests for GT and engine workaround verification Tvrtko Ursulin
@ 2018-11-30 21:04   ` Chris Wilson
  2018-11-30 22:01     ` Chris Wilson
  0 siblings, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2018-11-30 21:04 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2018-11-30 17:44:09)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Two simple selftests which test that both GT and engine workarounds are
> not lost after either a full GPU reset, or after the per-engine ones.
> 
> (Including checks that one engine reset is not affecting workarounds not
> belonging to itself.)
> 
> v2:
>  * Rebase for series refactoring.
>  * Add spinner for actual engine reset!
>  * Add idle reset test as well. (Chris Wilson)
>  * Share existing global_reset_lock. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |   1 +
>  drivers/gpu/drm/i915/intel_workarounds.c      |   6 +
>  drivers/gpu/drm/i915/selftests/igt_common.c   |  44 ++++++
>  drivers/gpu/drm/i915/selftests/igt_common.h   |  15 ++
>  .../gpu/drm/i915/selftests/intel_hangcheck.c  |  51 ++----
>  .../drm/i915/selftests/intel_workarounds.c    | 147 +++++++++++++++++-
>  6 files changed, 217 insertions(+), 47 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/selftests/igt_common.c
>  create mode 100644 drivers/gpu/drm/i915/selftests/igt_common.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 50a8fa8fce64..ceeb21f8aa0c 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -166,6 +166,7 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \
>         selftests/i915_random.o \
>         selftests/i915_selftest.o \
>         selftests/igt_flush_test.o \
> +       selftests/igt_common.o \

I was anticipating igt_reset.c.

> diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> index 80396b3592f5..194a3c30554d 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> @@ -6,6 +6,8 @@
>  
>  #include "../i915_selftest.h"
>  
> +#include "igt_common.h"
> +#include "igt_flush_test.h"
>  #include "igt_spinner.h"
>  #include "igt_wedge_me.h"
>  #include "mock_context.h"
> @@ -290,7 +292,6 @@ static int live_reset_whitelist(void *arg)
>  {
>         struct drm_i915_private *i915 = arg;
>         struct intel_engine_cs *engine = i915->engine[RCS];
> -       struct i915_gpu_error *error = &i915->gpu_error;
>         struct whitelist w;
>         int err = 0;
>  
> @@ -302,8 +303,7 @@ static int live_reset_whitelist(void *arg)
>         if (!whitelist_build(engine, &w))
>                 return 0;
>  
> -       set_bit(I915_RESET_BACKOFF, &error->flags);
> -       set_bit(I915_RESET_ENGINE + engine->id, &error->flags);
> +       igt_global_reset_lock(i915);
>  
>         if (intel_has_reset_engine(i915)) {
>                 err = check_whitelist_across_reset(engine,
> @@ -322,15 +322,152 @@ static int live_reset_whitelist(void *arg)
>         }
>  
>  out:
> -       clear_bit(I915_RESET_ENGINE + engine->id, &error->flags);
> -       clear_bit(I915_RESET_BACKOFF, &error->flags);
> +       igt_global_reset_unlock(i915);

Yup, makes sense.

>         return err;
>  }
>  
> +bool intel_engine_workarounds_verify(struct intel_engine_cs *engine,
> +                                    const char *from);
> +
> +static bool verify_gt_engine_wa(struct drm_i915_private *i915, const char *str)
> +{
> +       struct intel_engine_cs *engine;
> +       enum intel_engine_id id;
> +       bool ok = true;
> +
> +       ok &= intel_gt_workarounds_verify(i915, str);
> +
> +       for_each_engine(engine, i915, id)
> +               ok &= intel_engine_workarounds_verify(engine, str);
> +
> +       return ok;
> +}
> +
> +static int
> +live_gpu_reset_gt_engine_workarounds(void *arg)
> +{
> +       struct drm_i915_private *i915 = arg;
> +       struct i915_gpu_error *error = &i915->gpu_error;
> +       bool ok;
> +
> +       if (!intel_has_gpu_reset(i915))
> +               return 0;
> +
> +       pr_info("Verifying after GPU reset...\n");

Such noise! ;)

> +
> +       igt_global_reset_lock(i915);
> +
> +       ok = verify_gt_engine_wa(i915, "before reset");
> +       if (!ok)
> +               goto out;
> +
> +       intel_runtime_pm_get(i915);
> +       set_bit(I915_RESET_HANDOFF, &error->flags);
> +       i915_reset(i915, ALL_ENGINES, "live_workarounds");
> +       intel_runtime_pm_put(i915);
> +
> +       ok = verify_gt_engine_wa(i915, "after reset");
> +
> +out:
> +       igt_global_reset_unlock(i915);
> +
> +       return ok ? 0 : -ESRCH;
> +}
> +
> +static int
> +live_engine_reset_gt_engine_workarounds(void *arg)
> +{
> +       struct drm_i915_private *i915 = arg;
> +       struct intel_engine_cs *engine;
> +       struct i915_gem_context *ctx;
> +       struct igt_spinner spin;
> +       enum intel_engine_id id;
> +       struct i915_request *rq;
> +       int ret = 0;
> +
> +       if (!intel_has_reset_engine(i915))
> +               return 0;
> +
> +       ctx = kernel_context(i915);
> +       if (IS_ERR(ctx))
> +               return PTR_ERR(ctx);
> +
> +       for_each_engine(engine, i915, id) {
> +               bool ok;
> +
> +               pr_info("Verifying after %s reset...\n", engine->name);
> +
> +               igt_global_reset_lock(i915);

The key thing with it being global, is that you can take it globally :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework
  2018-11-30 17:44 ` [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework Tvrtko Ursulin
@ 2018-11-30 21:08   ` Chris Wilson
  2018-11-30 22:03     ` Chris Wilson
  0 siblings, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2018-11-30 21:08 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2018-11-30 17:44:10)
>  u64 intel_engine_get_active_head(const struct intel_engine_cs *engine)
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index dfafc3f710d6..4eead104cd9c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2316,6 +2316,7 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
>                           ret);
>         }
>  
> +       intel_whitelist_workarounds_init(engine);
>         intel_engine_workarounds_init(engine);

So to play devil's advocate:

intel_engine_init_whitelist(engine);
intel_engine_init_workarounds(engine);

object_verb_subject
	object = intel_engine
	verb = init
	subject / subcluase.

I guess there's one more to see how it fits into the pattern...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 6/7] drm/i915: Fuse per-context workaround handling with the common framework
  2018-11-30 17:44 ` [PATCH 6/7] drm/i915: Fuse per-context workaround handling with the common framework Tvrtko Ursulin
@ 2018-11-30 21:22   ` Chris Wilson
  2018-12-03 10:34     ` Tvrtko Ursulin
  0 siblings, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2018-11-30 21:22 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2018-11-30 17:44:11)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 91a750e90dc4..8f985c35ec92 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -452,6 +452,7 @@ struct intel_engine_cs {
>  
>         struct intel_hw_status_page status_page;
>         struct i915_ctx_workarounds wa_ctx;
> +       struct i915_wa_list ctx_wa_list;
>         struct i915_wa_list wa_list;
>         struct i915_wa_list whitelist;

Hmm. I think I would suggest we use

ctx_wa_list
mmio_wa_list ???
whitelist

> -int intel_ctx_workarounds_init(struct drm_i915_private *dev_priv)
> +void intel_ctx_workarounds_init(struct drm_i915_private *dev_priv)
>  {
> -       int err = 0;
> +       struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;

And here,

intel_engine_init_ctx_wa(intel_engine_cs *engine)

or something to match the other engine wa_list.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/7] drm/i915: Record GT workarounds in a list
  2018-11-30 17:44 ` [PATCH 1/7] drm/i915: Record GT workarounds in a list Tvrtko Ursulin
@ 2018-11-30 21:51   ` Chris Wilson
  0 siblings, 0 replies; 27+ messages in thread
From: Chris Wilson @ 2018-11-30 21:51 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2018-11-30 17:44:06)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> To enable later verification of GT workaround state at various stages of
> driver lifetime, we record the list of applicable ones per platforms to a
> list, from which they are also applied.
> 
> The added data structure is a simple array of register, mask and value
> items, which is allocated on demand as workarounds are added to the list.
> 
> This is a temporary implementation which later in the series gets fused
> with the existing per context workaround list handling. It is separated at
> this stage since the following patch fixes a bug which needs to be as easy
> to backport as possible.
> 
> Also, since in the following patch we will be adding a new class of
> workarounds (per engine) which can be applied from interrupt context, we
> straight away make the provision for safe read-modify-write cycle.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c          |   1 +
>  drivers/gpu/drm/i915/i915_drv.h          |   2 +
>  drivers/gpu/drm/i915/i915_gem.c          |   2 +
>  drivers/gpu/drm/i915/intel_workarounds.c | 443 +++++++++++++++--------
>  drivers/gpu/drm/i915/intel_workarounds.h |  22 ++
>  5 files changed, 327 insertions(+), 143 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e39016713464..2f3dc1cf83a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1453,6 +1453,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  
>         intel_uncore_sanitize(dev_priv);
>  
> +       intel_gt_workarounds_init(dev_priv);
>         i915_gem_load_init_fences(dev_priv);
>  
>         /* On the 945G/GM, the chipset reports the MSI capability on the
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 43ac6873a2bb..9ddbcc1f3554 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -69,6 +69,7 @@
>  #include "intel_ringbuffer.h"
>  #include "intel_uncore.h"
>  #include "intel_wopcm.h"
> +#include "intel_workarounds.h"
>  #include "intel_uc.h"
>  
>  #include "i915_gem.h"
> @@ -1652,6 +1653,7 @@ struct drm_i915_private {
>         int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
>  
>         struct i915_workarounds workarounds;
> +       struct i915_wa_list gt_wa_list;
>  
>         struct i915_frontbuffer_tracking fb_tracking;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c55b1f75c980..18adb3dd1fcd 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5706,6 +5706,8 @@ void i915_gem_fini(struct drm_i915_private *dev_priv)
>         i915_gem_contexts_fini(dev_priv);
>         mutex_unlock(&dev_priv->drm.struct_mutex);
>  
> +       intel_wa_list_free(&dev_priv->gt_wa_list);
> +
>         intel_cleanup_gt_powersave(dev_priv);
>  
>         intel_uc_fini_misc(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index e5cd6c6c66c3..ff20ebf9e040 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -48,6 +48,18 @@
>   * - Public functions to init or apply the given workaround type.
>   */
>  
> +static void wa_init_start(struct i915_wa_list *wal, const char *name)
> +{
> +       wal->name = name;
> +}
> +
> +static void wa_init_finish(struct i915_wa_list *wal)
> +{
> +       if (wal->count)
> +               DRM_DEBUG_DRIVER("Initialized %u %s workarounds\n",
> +                                wal->count, wal->name);
> +}
> +
>  static void wa_add(struct drm_i915_private *i915,
>                    i915_reg_t reg, const u32 mask, const u32 val)
>  {
> @@ -575,28 +587,88 @@ int intel_ctx_workarounds_emit(struct i915_request *rq)
>         return 0;
>  }
>  
> -static void bdw_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> +static void
> +wal_add(struct i915_wa_list *wal, const struct i915_wa *wa)
> +{
> +       const unsigned int grow = 4;
> +
> +       if (wal->__size == wal->count) {
> +               struct i915_wa *list;
> +
> +               list = kcalloc(wal->__size + grow, sizeof(*wa), GFP_KERNEL);
> +               if (!list) {
> +                       DRM_ERROR("No space for workaround init!\n");
> +                       return;
> +               }
> +
> +               if (wal->list)
> +                       memcpy(list, wal->list, sizeof(*wa) * wal->count);
> +
> +               wal->list = list;
> +               wal->__size += grow;
> +       }
> +
> +       memcpy(&wal->list[wal->count], wa, sizeof(*wa));
> +       wal->count++;
> +}
> +
> +static void
> +wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
>  {
> +       struct i915_wa wa = {
> +               .reg = reg,
> +               .mask = val,
> +               .val = _MASKED_BIT_ENABLE(val)
> +       };
> +
> +       wal_add(wal, &wa);
> +}
> +
> +static void
> +wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
> +                  u32 val)
> +{
> +       struct i915_wa wa = {
> +               .reg = reg,
> +               .mask = mask,
> +               .val = val
> +       };
> +
> +       wal_add(wal, &wa);
>  }
>  
> -static void chv_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> +static void
> +wa_write(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
>  {
> +       wa_write_masked_or(wal, reg, ~0, val);
>  }
>  
> -static void gen9_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> +static void
> +wa_write_or(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
>  {
> +       wa_write_masked_or(wal, reg, val, val);
> +}
> +
> +static void gen9_gt_workarounds_init(struct drm_i915_private *dev_priv)

Sneaky observation is that we now don't need dev_priv throughout the
init path, so a bonus s/dev_priv/i915/.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/7] drm/i915: Introduce per-engine workarounds
  2018-11-30 17:44 ` [PATCH 2/7] drm/i915: Introduce per-engine workarounds Tvrtko Ursulin
  2018-11-30 20:56   ` Chris Wilson
@ 2018-11-30 21:53   ` Chris Wilson
  2018-11-30 22:13   ` Rodrigo Vivi
  2 siblings, 0 replies; 27+ messages in thread
From: Chris Wilson @ 2018-11-30 21:53 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin; +Cc: intel-gfx, Rodrigo Vivi

Quoting Tvrtko Ursulin (2018-11-30 17:44:07)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> We stopped re-applying the GT workarounds after engine reset since commit
> 59b449d5c82a ("drm/i915: Split out functions for different kinds of
> workarounds").
> 
> Issue with this is that some of the GT workarounds live in the MMIO space
> which gets lost during engine resets. So far the registers in 0x2xxx and
> 0xbxxx address range have been identified to be affected.
> 
> This losing of applied workarounds has obvious negative effects and can
> even lead to hard system hangs (see the linked Bugzilla).
> 
> Rather than just restoring this re-application, because we have also
> observed that it is not safe to just re-write all GT workarounds after
> engine resets (GPU might be live and weird hardware states can happen),
> we introduce a new class of per-engine workarounds and move only the
> affected GT workarounds over.
> 
> Using the framework introduced in the previous patch, we therefore after
> engine reset, re-apply only the workarounds living in the affected MMIO
> address ranges.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=107945
> Fixes: 59b449d5c82a ("drm/i915: Split out functions for different kinds of workarounds")
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: intel-gfx@lists.freedesktop.org

Give or take the naming arguments,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/7] drm/i915: Verify GT workaround state after GPU init
  2018-11-30 17:44 ` [PATCH 3/7] drm/i915: Verify GT workaround state after GPU init Tvrtko Ursulin
@ 2018-11-30 21:55   ` Chris Wilson
  0 siblings, 0 replies; 27+ messages in thread
From: Chris Wilson @ 2018-11-30 21:55 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2018-11-30 17:44:08)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Since we now have all the GT workarounds in a table, by adding a simple
> shared helper function we can now verify that their values are still
> applied after some interesting events in the lifetime of the driver.
> 
> Initially we only do this after GPU initialization.
> 
> v2:
>  Chris Wilson:
>  * Simplify verification by realizing it's a simple xor and and.
>  * Remove verification from engine reset path.
>  * Return bool straight away from the verify API.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c          |  1 +
>  drivers/gpu/drm/i915/i915_gem.c          |  3 +++
>  drivers/gpu/drm/i915/intel_workarounds.c | 34 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_workarounds.h |  2 ++
>  4 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2f3dc1cf83a6..4883a20ed9ff 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -53,6 +53,7 @@
>  #include "i915_vgpu.h"
>  #include "intel_drv.h"
>  #include "intel_uc.h"
> +#include "intel_workarounds.h"
>  
>  static struct drm_driver driver;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 18adb3dd1fcd..1eff471d4366 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5334,7 +5334,10 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
>                 I915_WRITE(MI_PREDICATE_RESULT_2, IS_HSW_GT3(dev_priv) ?
>                            LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED);

What's that, a few more mmio that should be part of gt_wa? :)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 7/7] drm/i915: Trim unused workaround list entries
  2018-11-30 17:44 ` [PATCH 7/7] drm/i915: Trim unused workaround list entries Tvrtko Ursulin
@ 2018-11-30 21:58   ` Chris Wilson
  2018-12-03 10:39     ` Tvrtko Ursulin
  0 siblings, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2018-11-30 21:58 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2018-11-30 17:44:12)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> The new workaround list allocator grows the list in chunks so will end up
> with some unused space. Trim it when the initialization phase is done to
> free up a tiny bit of slab.
> 
> v2:
>  * Simplify with kmemdup. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_workarounds.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index 3e6b388ea022..9d876d554e57 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -55,6 +55,19 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name)
>  
>  static void wa_init_finish(struct i915_wa_list *wal)
>  {
> +       /* Trim unused entries. */
> +       if (wal->count < wal->__size) {
> +               struct i915_wa *list = kmemdup(wal->list,
> +                                              wal->count * sizeof(*list),
> +                                              GFP_KERNEL);
> +
> +               if (list) {
> +                       kfree(wal->list);
> +                       wal->list = list;
> +                       wal->__size = wal->count;

Hmm. We could kill __size entirely if you reallocated on 
is_power_of_two(wal->count).

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 4/7] drm/i915/selftests: Add tests for GT and engine workaround verification
  2018-11-30 21:04   ` Chris Wilson
@ 2018-11-30 22:01     ` Chris Wilson
  0 siblings, 0 replies; 27+ messages in thread
From: Chris Wilson @ 2018-11-30 22:01 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Chris Wilson (2018-11-30 21:04:46)
> Quoting Tvrtko Ursulin (2018-11-30 17:44:09)
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > Two simple selftests which test that both GT and engine workarounds are
> > not lost after either a full GPU reset, or after the per-engine ones.
> > 
> > (Including checks that one engine reset is not affecting workarounds not
> > belonging to itself.)
> > 
> > v2:
> >  * Rebase for series refactoring.
> >  * Add spinner for actual engine reset!
> >  * Add idle reset test as well. (Chris Wilson)
> >  * Share existing global_reset_lock. (Chris Wilson)
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> > +       for_each_engine(engine, i915, id) {
> > +               bool ok;
> > +
> > +               pr_info("Verifying after %s reset...\n", engine->name);
> > +
> > +               igt_global_reset_lock(i915);
> 
> The key thing with it being global, is that you can take it globally :)

I think that tidies up the loop innards to make it worthwhile, so with
that
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework
  2018-11-30 21:08   ` Chris Wilson
@ 2018-11-30 22:03     ` Chris Wilson
  0 siblings, 0 replies; 27+ messages in thread
From: Chris Wilson @ 2018-11-30 22:03 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Chris Wilson (2018-11-30 21:08:08)
> Quoting Tvrtko Ursulin (2018-11-30 17:44:10)
> >  u64 intel_engine_get_active_head(const struct intel_engine_cs *engine)
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index dfafc3f710d6..4eead104cd9c 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -2316,6 +2316,7 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
> >                           ret);
> >         }
> >  
> > +       intel_whitelist_workarounds_init(engine);
> >         intel_engine_workarounds_init(engine);
> 
> So to play devil's advocate:
> 
> intel_engine_init_whitelist(engine);
> intel_engine_init_workarounds(engine);
> 
> object_verb_subject
>         object = intel_engine
>         verb = init
>         subject / subcluase.
> 
> I guess there's one more to see how it fits into the pattern...

The guts are fine though,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/7] drm/i915: Introduce per-engine workarounds
  2018-11-30 17:44 ` [PATCH 2/7] drm/i915: Introduce per-engine workarounds Tvrtko Ursulin
  2018-11-30 20:56   ` Chris Wilson
  2018-11-30 21:53   ` Chris Wilson
@ 2018-11-30 22:13   ` Rodrigo Vivi
  2 siblings, 0 replies; 27+ messages in thread
From: Rodrigo Vivi @ 2018-11-30 22:13 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Fri, Nov 30, 2018 at 05:44:07PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> We stopped re-applying the GT workarounds after engine reset since commit
> 59b449d5c82a ("drm/i915: Split out functions for different kinds of
> workarounds").
> 
> Issue with this is that some of the GT workarounds live in the MMIO space
> which gets lost during engine resets. So far the registers in 0x2xxx and
> 0xbxxx address range have been identified to be affected.
> 
> This losing of applied workarounds has obvious negative effects and can
> even lead to hard system hangs (see the linked Bugzilla).
> 
> Rather than just restoring this re-application, because we have also
> observed that it is not safe to just re-write all GT workarounds after
> engine resets (GPU might be live and weird hardware states can happen),
> we introduce a new class of per-engine workarounds and move only the
> affected GT workarounds over.
> 
> Using the framework introduced in the previous patch, we therefore after
> engine reset, re-apply only the workarounds living in the affected MMIO
> address ranges.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=107945
> Fixes: 59b449d5c82a ("drm/i915: Split out functions for different kinds of workarounds")
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c   |   2 +
>  drivers/gpu/drm/i915/intel_lrc.c         |   4 +
>  drivers/gpu/drm/i915/intel_ringbuffer.h  |   2 +
>  drivers/gpu/drm/i915/intel_workarounds.c | 249 +++++++++++++----------
>  drivers/gpu/drm/i915/intel_workarounds.h |   3 +
>  5 files changed, 148 insertions(+), 112 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 759c0fd58f8c..ef5d202e9d45 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -723,6 +723,8 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
>  	__intel_context_unpin(i915->kernel_context, engine);
>  
>  	i915_timeline_fini(&engine->timeline);
> +
> +	intel_wa_list_free(&engine->wa_list);
>  }
>  
>  u64 intel_engine_get_active_head(const struct intel_engine_cs *engine)
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 11f4e6148557..dfafc3f710d6 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1617,6 +1617,8 @@ static bool unexpected_starting_state(struct intel_engine_cs *engine)
>  
>  static int gen8_init_common_ring(struct intel_engine_cs *engine)
>  {
> +	intel_engine_workarounds_apply(engine);
> +
>  	intel_mocs_init_engine(engine);
>  
>  	intel_engine_reset_breadcrumbs(engine);
> @@ -2314,6 +2316,8 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
>  			  ret);
>  	}
>  
> +	intel_engine_workarounds_init(engine);
> +
>  	return 0;
>  
>  err_cleanup_common:
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 8a2270b209b0..c5ff3d31cab7 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -15,6 +15,7 @@
>  #include "i915_selftest.h"
>  #include "i915_timeline.h"
>  #include "intel_gpu_commands.h"
> +#include "intel_workarounds.h"
>  
>  struct drm_printer;
>  struct i915_sched_attr;
> @@ -451,6 +452,7 @@ struct intel_engine_cs {
>  
>  	struct intel_hw_status_page status_page;
>  	struct i915_ctx_workarounds wa_ctx;
> +	struct i915_wa_list wa_list;
>  	struct i915_vma *scratch;
>  
>  	u32             irq_keep_mask; /* always keep these interrupts */
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index ff20ebf9e040..be63a2af3481 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -653,17 +653,6 @@ static void gen9_gt_workarounds_init(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_wa_list *wal = &dev_priv->gt_wa_list;
>  
> -	/* WaContextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk,cfl */
> -	wa_masked_en(wal,
> -		     GEN9_CSFE_CHICKEN1_RCS,
> -		     GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE);
> -
> -
> -	/* WaEnableLbsSlaRetryTimerDecrement:skl,bxt,kbl,glk,cfl */
> -	wa_write_or(wal,
> -		    BDW_SCRATCH1,
> -		    GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE);
> -
>  	/* WaDisableKillLogic:bxt,skl,kbl */
>  	if (!IS_COFFEELAKE(dev_priv))
>  		wa_write_or(wal,
> @@ -685,24 +674,6 @@ static void gen9_gt_workarounds_init(struct drm_i915_private *dev_priv)
>  	wa_write_or(wal,
>  		    GAM_ECOCHK,
>  		    BDW_DISABLE_HDC_INVALIDATION);
> -
> -	/* WaProgramL3SqcReg1DefaultForPerf:bxt,glk */
> -	if (IS_GEN9_LP(dev_priv))
> -		wa_write_masked_or(wal,
> -				   GEN8_L3SQCREG1,
> -				   L3_PRIO_CREDITS_MASK,
> -				   L3_GENERAL_PRIO_CREDITS(62) |
> -				   L3_HIGH_PRIO_CREDITS(2));
> -
> -	/* WaOCLCoherentLineFlush:skl,bxt,kbl,cfl */
> -	wa_write_or(wal,
> -		    GEN8_L3SQCREG4,
> -		    GEN8_LQSC_FLUSH_COHERENT_LINES);
> -
> -	/* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] */
> -	wa_masked_en(wal,
> -		     GEN7_FF_SLICE_CS_CHICKEN1,
> -		     GEN9_FFSC_PERCTX_PREEMPT_CTRL);
>  }
>  
>  static void skl_gt_workarounds_init(struct drm_i915_private *dev_priv)
> @@ -711,11 +682,6 @@ static void skl_gt_workarounds_init(struct drm_i915_private *dev_priv)
>  
>  	gen9_gt_workarounds_init(dev_priv);
>  
> -	/* WaEnableGapsTsvCreditFix:skl */
> -	wa_write_or(wal,
> -		    GEN8_GARBCNTL,
> -		    GEN9_GAPS_TSV_CREDIT_DISABLE);
> -
>  	/* WaDisableGafsUnitClkGating:skl */
>  	wa_write_or(wal,
>  		    GEN7_UCGCTL4,
> @@ -734,11 +700,6 @@ static void bxt_gt_workarounds_init(struct drm_i915_private *dev_priv)
>  
>  	gen9_gt_workarounds_init(dev_priv);
>  
> -	/* WaDisablePooledEuLoadBalancingFix:bxt */
> -	wa_masked_en(wal,
> -		     FF_SLICE_CS_CHICKEN2,
> -		     GEN9_POOLED_EU_LOAD_BALANCING_FIX_DISABLE);
> -
>  	/* WaInPlaceDecompressionHang:bxt */
>  	wa_write_or(wal,
>  		    GEN9_GAMT_ECO_REG_RW_IA,
> @@ -751,11 +712,6 @@ static void kbl_gt_workarounds_init(struct drm_i915_private *dev_priv)
>  
>  	gen9_gt_workarounds_init(dev_priv);
>  
> -	/* WaEnableGapsTsvCreditFix:kbl */
> -	wa_write_or(wal,
> -		    GEN8_GARBCNTL,
> -		    GEN9_GAPS_TSV_CREDIT_DISABLE);
> -
>  	/* WaDisableDynamicCreditSharing:kbl */
>  	if (IS_KBL_REVID(dev_priv, 0, KBL_REVID_B0))
>  		wa_write_or(wal,
> @@ -771,21 +727,6 @@ static void kbl_gt_workarounds_init(struct drm_i915_private *dev_priv)
>  	wa_write_or(wal,
>  		    GEN9_GAMT_ECO_REG_RW_IA,
>  		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> -
> -	/* WaKBLVECSSemaphoreWaitPoll:kbl */
> -	if (IS_KBL_REVID(dev_priv, KBL_REVID_A0, KBL_REVID_E0)) {
> -		struct intel_engine_cs *engine;
> -		unsigned int tmp;
> -
> -		for_each_engine(engine, dev_priv, tmp) {
> -			if (engine->id == RCS)
> -				continue;
> -
> -			wa_write(wal,
> -				 RING_SEMA_WAIT_POLL(engine->mmio_base),
> -				 1);
> -		}
> -	}
>  }
>  
>  static void glk_gt_workarounds_init(struct drm_i915_private *dev_priv)
> @@ -799,11 +740,6 @@ static void cfl_gt_workarounds_init(struct drm_i915_private *dev_priv)
>  
>  	gen9_gt_workarounds_init(dev_priv);
>  
> -	/* WaEnableGapsTsvCreditFix:cfl */
> -	wa_write_or(wal,
> -		    GEN8_GARBCNTL,
> -		    GEN9_GAPS_TSV_CREDIT_DISABLE);
> -
>  	/* WaDisableGafsUnitClkGating:cfl */
>  	wa_write_or(wal,
>  		    GEN7_UCGCTL4,
> @@ -894,11 +830,6 @@ static void cnl_gt_workarounds_init(struct drm_i915_private *dev_priv)
>  	wa_write_or(wal,
>  		    GEN9_GAMT_ECO_REG_RW_IA,
>  		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
> -
> -	/* WaEnablePreemptionGranularityControlByUMD:cnl */
> -	wa_masked_en(wal,
> -		     GEN7_FF_SLICE_CS_CHICKEN1,
> -		     GEN9_FFSC_PERCTX_PREEMPT_CTRL);
>  }
>  
>  static void icl_gt_workarounds_init(struct drm_i915_private *dev_priv)
> @@ -907,53 +838,17 @@ static void icl_gt_workarounds_init(struct drm_i915_private *dev_priv)
>  
>  	wa_init_mcr(dev_priv);
>  
> -	/* This is not an Wa. Enable for better image quality */
> -	wa_masked_en(wal,
> -		    _3D_CHICKEN3,
> -		    _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE);
> -
>  	/* WaInPlaceDecompressionHang:icl */
>  	wa_write_or(wal,
>  		    GEN9_GAMT_ECO_REG_RW_IA,
>  		    GAMT_ECO_ENABLE_IN_PLACE_DECOMPRESS);
>  
> -	/* WaPipelineFlushCoherentLines:icl */
> -	wa_write_or(wal,
> -		    GEN8_L3SQCREG4,
> -		    GEN8_LQSC_FLUSH_COHERENT_LINES);
> -
> -	/* Wa_1405543622:icl
> -	 * Formerly known as WaGAPZPriorityScheme
> -	 */
> -	wa_write_or(wal,
> -		    GEN8_GARBCNTL,
> -		    GEN11_ARBITRATION_PRIO_ORDER_MASK);
> -
> -	/* Wa_1604223664:icl
> -	 * Formerly known as WaL3BankAddressHashing
> -	 */
> -	wa_write_masked_or(wal,
> -			   GEN8_GARBCNTL,
> -			   GEN11_HASH_CTRL_EXCL_MASK,
> -			   GEN11_HASH_CTRL_EXCL_BIT0);
> -	wa_write_masked_or(wal,
> -			   GEN11_GLBLINVL,
> -			   GEN11_BANK_HASH_ADDR_EXCL_MASK,
> -			   GEN11_BANK_HASH_ADDR_EXCL_BIT0);
> -
>  	/* WaModifyGamTlbPartitioning:icl */
>  	wa_write_masked_or(wal,
>  			   GEN11_GACB_PERF_CTRL,
>  			   GEN11_HASH_CTRL_MASK,
>  			   GEN11_HASH_CTRL_BIT0 | GEN11_HASH_CTRL_BIT4);
>  
> -	/* Wa_1405733216:icl
> -	 * Formerly known as WaDisableCleanEvicts
> -	 */
> -	wa_write_or(wal,
> -		    GEN8_L3SQCREG4,
> -		    GEN11_LQSC_CLEAN_EVICT_DISABLE);
> -
>  	/* Wa_1405766107:icl
>  	 * Formerly known as WaCL2SFHalfMaxAlloc
>  	 */
> @@ -986,13 +881,6 @@ static void icl_gt_workarounds_init(struct drm_i915_private *dev_priv)
>  			    INF_UNIT_LEVEL_CLKGATE,
>  			    CGPSF_CLKGATE_DIS);
>  
> -	/* WaForwardProgressSoftReset:icl */
> -	wa_write_or(wal,
> -		    GEN10_SCRATCH_LNCF2,
> -		    PMFLUSHDONE_LNICRSDROP |
> -		    PMFLUSH_GAPL3UNBLOCK |
> -		    PMFLUSHDONE_LNEBLK);
> -
>  	/* Wa_1406463099:icl
>  	 * Formerly known as WaGamTlbPendError
>  	 */
> @@ -1242,6 +1130,143 @@ void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine)
>  	whitelist_apply(engine, whitelist_build(engine, &w));
>  }
>  
> +static void rcs_engine_wa_init(struct intel_engine_cs *engine)
> +{
> +	struct drm_i915_private *i915 = engine->i915;
> +	struct i915_wa_list *wal = &engine->wa_list;
> +
> +	if (IS_GEN9(i915)) {
> +		/* WaContextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk,cfl */
> +		wa_masked_en(wal,
> +			     GEN9_CSFE_CHICKEN1_RCS,
> +			     GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE);
> +
> +		/* WaEnableLbsSlaRetryTimerDecrement:skl,bxt,kbl,glk,cfl */
> +		wa_write_or(wal,
> +			    BDW_SCRATCH1,
> +			    GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE);
> +
> +		/* WaProgramL3SqcReg1DefaultForPerf:bxt,glk */
> +		if (IS_GEN9_LP(i915))
> +			wa_write_masked_or(wal,
> +					   GEN8_L3SQCREG1,
> +					   L3_PRIO_CREDITS_MASK,
> +					   L3_GENERAL_PRIO_CREDITS(62) |
> +					   L3_HIGH_PRIO_CREDITS(2));
> +
> +		/* WaOCLCoherentLineFlush:skl,bxt,kbl,cfl */
> +		wa_write_or(wal,
> +			    GEN8_L3SQCREG4,
> +			    GEN8_LQSC_FLUSH_COHERENT_LINES);
> +	}
> +
> +	if (IS_BROXTON(i915)) {
> +		/* WaDisablePooledEuLoadBalancingFix:bxt */
> +		wa_masked_en(wal,
> +			     FF_SLICE_CS_CHICKEN2,
> +			     GEN9_POOLED_EU_LOAD_BALANCING_FIX_DISABLE);
> +	}
> +
> +	if (IS_SKYLAKE(i915) || IS_KABYLAKE(i915) || IS_COFFEELAKE(i915)) {
> +		/* WaEnableGapsTsvCreditFix:skl,kbl,cfl */
> +		wa_write_or(wal,
> +			    GEN8_GARBCNTL,
> +			    GEN9_GAPS_TSV_CREDIT_DISABLE);
> +	}
> +
> +	if (IS_GEN9(i915) || IS_CANNONLAKE(i915)) {
> +		/* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */
> +		wa_masked_en(wal,
> +			     GEN7_FF_SLICE_CS_CHICKEN1,
> +			     GEN9_FFSC_PERCTX_PREEMPT_CTRL);
> +	}
> +
> +	if (IS_ICELAKE(i915)) {

Could we make newer platform coming first?

anyway:

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> +		/* This is not an Wa. Enable for better image quality */
> +		wa_masked_en(wal,
> +			     _3D_CHICKEN3,
> +			     _3D_CHICKEN3_AA_LINE_QUALITY_FIX_ENABLE);
> +
> +		/* WaPipelineFlushCoherentLines:icl */
> +		wa_write_or(wal,
> +			    GEN8_L3SQCREG4,
> +			    GEN8_LQSC_FLUSH_COHERENT_LINES);
> +
> +		/*
> +		 * Wa_1405543622:icl
> +		 * Formerly known as WaGAPZPriorityScheme
> +		 */
> +		wa_write_or(wal,
> +			    GEN8_GARBCNTL,
> +			    GEN11_ARBITRATION_PRIO_ORDER_MASK);
> +
> +		/*
> +		 * Wa_1604223664:icl
> +		 * Formerly known as WaL3BankAddressHashing
> +		 */
> +		wa_write_masked_or(wal,
> +				   GEN8_GARBCNTL,
> +				   GEN11_HASH_CTRL_EXCL_MASK,
> +				   GEN11_HASH_CTRL_EXCL_BIT0);
> +		wa_write_masked_or(wal,
> +				   GEN11_GLBLINVL,
> +				   GEN11_BANK_HASH_ADDR_EXCL_MASK,
> +				   GEN11_BANK_HASH_ADDR_EXCL_BIT0);
> +
> +		/*
> +		 * Wa_1405733216:icl
> +		 * Formerly known as WaDisableCleanEvicts
> +		 */
> +		wa_write_or(wal,
> +			    GEN8_L3SQCREG4,
> +			    GEN11_LQSC_CLEAN_EVICT_DISABLE);
> +
> +		/* WaForwardProgressSoftReset:icl */
> +		wa_write_or(wal,
> +			    GEN10_SCRATCH_LNCF2,
> +			    PMFLUSHDONE_LNICRSDROP |
> +			    PMFLUSH_GAPL3UNBLOCK |
> +			    PMFLUSHDONE_LNEBLK);
> +	}
> +}
> +
> +static void xcs_engine_wa_init(struct intel_engine_cs *engine)
> +{
> +	struct drm_i915_private *i915 = engine->i915;
> +	struct i915_wa_list *wal = &engine->wa_list;
> +
> +	if (IS_KABYLAKE(i915)) {
> +		/* WaKBLVECSSemaphoreWaitPoll:kbl */
> +		if (IS_KBL_REVID(i915, KBL_REVID_A0, KBL_REVID_E0)) {
> +			wa_write(wal,
> +				 RING_SEMA_WAIT_POLL(engine->mmio_base),
> +				 1);
> +		}
> +	}
> +}
> +
> +void intel_engine_workarounds_init(struct intel_engine_cs *engine)
> +{
> +	struct i915_wa_list *wal = &engine->wa_list;
> +
> +	if (GEM_WARN_ON(INTEL_GEN(engine->i915) < 8))
> +		return;
> +
> +	wa_init_start(wal, engine->name);
> +
> +	if (engine->id == RCS)
> +		rcs_engine_wa_init(engine);
> +	else
> +		xcs_engine_wa_init(engine);
> +
> +	wa_init_finish(wal);
> +}
> +
> +void intel_engine_workarounds_apply(struct intel_engine_cs *engine)
> +{
> +	wa_list_apply(engine->i915, &engine->wa_list);
> +}
> +
>  #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>  #include "selftests/intel_workarounds.c"
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
> index 64aed9cf0200..2998767d51ca 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.h
> +++ b/drivers/gpu/drm/i915/intel_workarounds.h
> @@ -36,4 +36,7 @@ void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv);
>  
>  void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine);
>  
> +void intel_engine_workarounds_init(struct intel_engine_cs *engine);
> +void intel_engine_workarounds_apply(struct intel_engine_cs *engine);
> +
>  #endif
> -- 
> 2.19.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.IGT: failure for Restore workarounds after engine reset and unify their handling (rev2)
  2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
                   ` (9 preceding siblings ...)
  2018-11-30 18:24 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-12-01 13:36 ` Patchwork
  10 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-12-01 13:36 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: Restore workarounds after engine reset and unify their handling (rev2)
URL   : https://patchwork.freedesktop.org/series/53313/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5235_full -> Patchwork_10981_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_10981_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10981_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_10981_full:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_workarounds:
    - {shard-iclb}:       PASS -> DMESG-FAIL

  * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-ytiled:
    - shard-skl:          PASS -> FAIL

  
#### Warnings ####

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-snb:          PASS -> SKIP

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_reuse@contexts:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@gem_ppgtt@blt-vs-render-ctx0:
    - shard-skl:          PASS -> TIMEOUT [fdo#108039]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#107725]

  * igt@kms_chv_cursor_fail@pipe-a-128x128-bottom-edge:
    - shard-skl:          PASS -> FAIL [fdo#104671]

  * igt@kms_cursor_crc@cursor-256x256-offscreen:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x85-offscreen:
    - shard-skl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-untiled:
    - {shard-iclb}:       PASS -> WARN [fdo#108336] +6

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

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - {shard-iclb}:       PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-render:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-blt:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167] / [fdo#105682]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-pwrite:
    - {shard-iclb}:       PASS -> DMESG-FAIL [fdo#107724] +6

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-skl:          NOTRUN -> FAIL [fdo#105682] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-tilingchange:
    - shard-skl:          PASS -> FAIL [fdo#105682] +1

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-cpu:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-blt:
    - shard-skl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - {shard-iclb}:       PASS -> INCOMPLETE [fdo#107713]

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - {shard-iclb}:       PASS -> FAIL [fdo#103166]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +3

  * igt@kms_plane_lowres@pipe-c-tiling-y:
    - {shard-iclb}:       PASS -> DMESG-WARN [fdo#107724] / [fdo#108336] +21

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_scaling@pipe-b-plane-scaling:
    - {shard-iclb}:       PASS -> DMESG-WARN [fdo#107724] +36

  * igt@kms_rotation_crc@primary-rotation-180:
    - shard-skl:          PASS -> FAIL [fdo#103925] / [fdo#107815]

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@kms_universal_plane@universal-plane-pipe-a-functional:
    - {shard-iclb}:       PASS -> DMESG-FAIL [fdo#103166] / [fdo#107724]

  * igt@perf@short-reads:
    - shard-skl:          PASS -> FAIL [fdo#103183]

  * igt@pm_rpm@pm-caching:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807]

  * igt@pm_rpm@system-suspend-modeset:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773] / [fdo#107807]

  * igt@pm_rps@waitboost:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#102250] / [fdo#108059]

  
#### Possible fixes ####

  * igt@gem_userptr_blits@readonly-unsync:
    - shard-kbl:          TIMEOUT -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-glk:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-kbl:          FAIL [fdo#100368] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - {shard-iclb}:       DMESG-FAIL [fdo#107724] -> PASS +6

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-pwrite:
    - {shard-iclb}:       FAIL [fdo#103167] -> PASS +2

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - {shard-iclb}:       DMESG-FAIL [fdo#103166] / [fdo#107724] -> PASS

  * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-glk:          FAIL [fdo#103166] -> PASS

  * igt@kms_rmfb@rmfb-ioctl:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] -> PASS +9

  * igt@pm_rpm@cursor:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS +1

  * igt@pm_rpm@gem-execbuf-stress-pc8:
    - {shard-iclb}:       INCOMPLETE [fdo#107713] / [fdo#108840] -> SKIP

  
#### Warnings ####

  * igt@kms_ccs@pipe-b-crc-primary-rotation-180:
    - {shard-iclb}:       FAIL [fdo#107725] -> DMESG-WARN [fdo#107724] / [fdo#108336]

  * igt@kms_cursor_crc@cursor-128x128-sliding:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x21-random:
    - {shard-iclb}:       FAIL [fdo#103232] -> DMESG-WARN [fdo#107724] / [fdo#108336] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-pwrite:
    - {shard-iclb}:       FAIL [fdo#103167] -> DMESG-FAIL [fdo#107724]

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - {shard-iclb}:       FAIL [fdo#103166] -> DMESG-WARN [fdo#107724] / [fdo#108336]

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

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102250]: https://bugs.freedesktop.org/show_bug.cgi?id=102250
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103183]: https://bugs.freedesktop.org/show_bug.cgi?id=103183
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103925]: https://bugs.freedesktop.org/show_bug.cgi?id=103925
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [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#108039]: https://bugs.freedesktop.org/show_bug.cgi?id=108039
  [fdo#108059]: https://bugs.freedesktop.org/show_bug.cgi?id=108059
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


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

    * Linux: CI_DRM_5235 -> Patchwork_10981

  CI_DRM_5235: 5e327aec3fa3fc277b40818e06785743cf16b1ad @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10981: b7af01a81fb8c4c250ba60f24eb7b0272bcf7d3f @ 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_10981/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 6/7] drm/i915: Fuse per-context workaround handling with the common framework
  2018-11-30 21:22   ` Chris Wilson
@ 2018-12-03 10:34     ` Tvrtko Ursulin
  0 siblings, 0 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-12-03 10:34 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx


On 30/11/2018 21:22, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-11-30 17:44:11)
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
>> index 91a750e90dc4..8f985c35ec92 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>> @@ -452,6 +452,7 @@ struct intel_engine_cs {
>>   
>>          struct intel_hw_status_page status_page;
>>          struct i915_ctx_workarounds wa_ctx;
>> +       struct i915_wa_list ctx_wa_list;
>>          struct i915_wa_list wa_list;
>>          struct i915_wa_list whitelist;
> 
> Hmm. I think I would suggest we use
> 
> ctx_wa_list
> mmio_wa_list ???
> whitelist

It is implied workarounds are about mmio one way or the other via struct 
i915_wa itself so not sure.

>> -int intel_ctx_workarounds_init(struct drm_i915_private *dev_priv)
>> +void intel_ctx_workarounds_init(struct drm_i915_private *dev_priv)
>>   {
>> -       int err = 0;
>> +       struct i915_wa_list *wal = &dev_priv->engine[RCS]->ctx_wa_list;
> 
> And here,
> 
> intel_engine_init_ctx_wa(intel_engine_cs *engine)
> 
> or something to match the other engine wa_list.

I don't have any better ideas so shout quickly or else I am changing it 
to intel_engine_init_ctx_wa.

Regards,

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

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

* Re: [PATCH 7/7] drm/i915: Trim unused workaround list entries
  2018-11-30 21:58   ` Chris Wilson
@ 2018-12-03 10:39     ` Tvrtko Ursulin
  0 siblings, 0 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-12-03 10:39 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx


On 30/11/2018 21:58, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-11-30 17:44:12)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> The new workaround list allocator grows the list in chunks so will end up
>> with some unused space. Trim it when the initialization phase is done to
>> free up a tiny bit of slab.
>>
>> v2:
>>   * Simplify with kmemdup. (Chris Wilson)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_workarounds.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
>> index 3e6b388ea022..9d876d554e57 100644
>> --- a/drivers/gpu/drm/i915/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
>> @@ -55,6 +55,19 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name)
>>   
>>   static void wa_init_finish(struct i915_wa_list *wal)
>>   {
>> +       /* Trim unused entries. */
>> +       if (wal->count < wal->__size) {
>> +               struct i915_wa *list = kmemdup(wal->list,
>> +                                              wal->count * sizeof(*list),
>> +                                              GFP_KERNEL);
>> +
>> +               if (list) {
>> +                       kfree(wal->list);
>> +                       wal->list = list;
>> +                       wal->__size = wal->count;
> 
> Hmm. We could kill __size entirely if you reallocated on
> is_power_of_two(wal->count).

You mean allocate to next power of two is is_power_of_two(wal->count) ? 
In that case I could also allocate to next ALIGN(wal->count, CHUNK_SIZE) 
if IS_ALIGNED which looks more appropriate for this particular use case.

Can't decide whether it is worth it though. Why not..

Regards,

Tvrtko

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework
  2018-12-03 12:50 [PATCH v4 0/7] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
@ 2018-12-03 12:50 ` Tvrtko Ursulin
  0 siblings, 0 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-12-03 12:50 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Instead of having a separate list of white-listed registers we can
trivially move this to the common workarounds framework.

This brings us one step closer to the goal of driving all workaround
classes using the same code.

v2:
 * Use GEM_DEBUG_WARN_ON for the sanity check. (Chris Wilson)

v3:
 * API rename. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_engine_cs.c        |  1 +
 drivers/gpu/drm/i915/intel_lrc.c              |  5 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h       |  1 +
 drivers/gpu/drm/i915/intel_workarounds.c      | 83 ++++++++-----------
 drivers/gpu/drm/i915/intel_workarounds.h      |  3 +-
 .../drm/i915/selftests/intel_workarounds.c    | 40 ++++-----
 6 files changed, 60 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index ef5d202e9d45..496462d77ebc 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -725,6 +725,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
 	i915_timeline_fini(&engine->timeline);
 
 	intel_wa_list_free(&engine->wa_list);
+	intel_wa_list_free(&engine->whitelist);
 }
 
 u64 intel_engine_get_active_head(const struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 7a7787cbafee..af61a3cf9cb8 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1643,7 +1643,7 @@ static int gen8_init_render_ring(struct intel_engine_cs *engine)
 	if (ret)
 		return ret;
 
-	intel_whitelist_workarounds_apply(engine);
+	intel_engine_apply_whitelist(engine);
 
 	/* We need to disable the AsyncFlip performance optimisations in order
 	 * to use MI_WAIT_FOR_EVENT within the CS. It should already be
@@ -1666,7 +1666,7 @@ static int gen9_init_render_ring(struct intel_engine_cs *engine)
 	if (ret)
 		return ret;
 
-	intel_whitelist_workarounds_apply(engine);
+	intel_engine_apply_whitelist(engine);
 
 	return 0;
 }
@@ -2316,6 +2316,7 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
 			  ret);
 	}
 
+	intel_engine_init_whitelist(engine);
 	intel_engine_init_workarounds(engine);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c5ff3d31cab7..91a750e90dc4 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -453,6 +453,7 @@ struct intel_engine_cs {
 	struct intel_hw_status_page status_page;
 	struct i915_ctx_workarounds wa_ctx;
 	struct i915_wa_list wa_list;
+	struct i915_wa_list whitelist;
 	struct i915_vma *scratch;
 
 	u32             irq_keep_mask; /* always keep these interrupts */
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index bb49a5504c9a..56907311349e 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -1012,29 +1012,20 @@ bool intel_gt_verify_workarounds(struct drm_i915_private *dev_priv,
 	return wa_list_verify(dev_priv, &dev_priv->gt_wa_list, from);
 }
 
-struct whitelist {
-	i915_reg_t reg[RING_MAX_NONPRIV_SLOTS];
-	unsigned int count;
-	u32 nopid;
-};
-
-static void whitelist_reg(struct whitelist *w, i915_reg_t reg)
+static void
+whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
 {
-	if (GEM_DEBUG_WARN_ON(w->count >= RING_MAX_NONPRIV_SLOTS))
-		return;
-
-	w->reg[w->count++] = reg;
-}
+	struct i915_wa wa = {
+		.reg = reg
+	};
 
-static void bdw_whitelist_build(struct whitelist *w)
-{
-}
+	if (GEM_DEBUG_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
+		return;
 
-static void chv_whitelist_build(struct whitelist *w)
-{
+	wal_add(wal, &wa);
 }
 
-static void gen9_whitelist_build(struct whitelist *w)
+static void gen9_whitelist_build(struct i915_wa_list *w)
 {
 	/* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */
 	whitelist_reg(w, GEN9_CTX_PREEMPT_REG);
@@ -1046,7 +1037,7 @@ static void gen9_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN8_HDC_CHICKEN1);
 }
 
-static void skl_whitelist_build(struct whitelist *w)
+static void skl_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 
@@ -1054,12 +1045,12 @@ static void skl_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN8_L3SQCREG4);
 }
 
-static void bxt_whitelist_build(struct whitelist *w)
+static void bxt_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 }
 
-static void kbl_whitelist_build(struct whitelist *w)
+static void kbl_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 
@@ -1067,7 +1058,7 @@ static void kbl_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN8_L3SQCREG4);
 }
 
-static void glk_whitelist_build(struct whitelist *w)
+static void glk_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 
@@ -1075,18 +1066,18 @@ static void glk_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN9_SLICE_COMMON_ECO_CHICKEN1);
 }
 
-static void cfl_whitelist_build(struct whitelist *w)
+static void cfl_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 }
 
-static void cnl_whitelist_build(struct whitelist *w)
+static void cnl_whitelist_build(struct i915_wa_list *w)
 {
 	/* WaEnablePreemptionGranularityControlByUMD:cnl */
 	whitelist_reg(w, GEN8_CS_CHICKEN1);
 }
 
-static void icl_whitelist_build(struct whitelist *w)
+static void icl_whitelist_build(struct i915_wa_list *w)
 {
 	/* WaAllowUMDToModifyHalfSliceChicken7:icl */
 	whitelist_reg(w, GEN9_HALF_SLICE_CHICKEN7);
@@ -1095,22 +1086,21 @@ static void icl_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN10_SAMPLER_MODE);
 }
 
-static struct whitelist *whitelist_build(struct intel_engine_cs *engine,
-					 struct whitelist *w)
+void intel_engine_init_whitelist(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *i915 = engine->i915;
+	struct i915_wa_list *w = &engine->whitelist;
 
 	GEM_BUG_ON(engine->id != RCS);
 
-	w->count = 0;
-	w->nopid = i915_mmio_reg_offset(RING_NOPID(engine->mmio_base));
+	wa_init_start(w, "whitelist");
 
 	if (INTEL_GEN(i915) < 8)
-		return NULL;
+		return;
 	else if (IS_BROADWELL(i915))
-		bdw_whitelist_build(w);
+		return;
 	else if (IS_CHERRYVIEW(i915))
-		chv_whitelist_build(w);
+		return;
 	else if (IS_SKYLAKE(i915))
 		skl_whitelist_build(w);
 	else if (IS_BROXTON(i915))
@@ -1128,37 +1118,30 @@ static struct whitelist *whitelist_build(struct intel_engine_cs *engine,
 	else
 		MISSING_CASE(INTEL_GEN(i915));
 
-	return w;
+	wa_init_finish(w);
 }
 
-static void whitelist_apply(struct intel_engine_cs *engine,
-			    const struct whitelist *w)
+void intel_engine_apply_whitelist(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	const struct i915_wa_list *wal = &engine->whitelist;
 	const u32 base = engine->mmio_base;
+	struct i915_wa *wa;
 	unsigned int i;
 
-	if (!w)
+	if (!wal->count)
 		return;
 
-	intel_uncore_forcewake_get(engine->i915, FORCEWAKE_ALL);
-
-	for (i = 0; i < w->count; i++)
-		I915_WRITE_FW(RING_FORCE_TO_NONPRIV(base, i),
-			      i915_mmio_reg_offset(w->reg[i]));
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
+		I915_WRITE(RING_FORCE_TO_NONPRIV(base, i),
+			   i915_mmio_reg_offset(wa->reg));
 
 	/* And clear the rest just in case of garbage */
 	for (; i < RING_MAX_NONPRIV_SLOTS; i++)
-		I915_WRITE_FW(RING_FORCE_TO_NONPRIV(base, i), w->nopid);
-
-	intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL);
-}
+		I915_WRITE(RING_FORCE_TO_NONPRIV(base, i),
+			   i915_mmio_reg_offset(RING_NOPID(base)));
 
-void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine)
-{
-	struct whitelist w;
-
-	whitelist_apply(engine, whitelist_build(engine, &w));
+	DRM_DEBUG_DRIVER("Applied %u %s workarounds\n", wal->count, wal->name);
 }
 
 static void rcs_engine_wa_init(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
index 8822e6035f8d..3f99bfcb4a03 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -35,7 +35,8 @@ void intel_gt_apply_workarounds(struct drm_i915_private *dev_priv);
 bool intel_gt_verify_workarounds(struct drm_i915_private *dev_priv,
 				 const char *from);
 
-void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine);
+void intel_engine_init_whitelist(struct intel_engine_cs *engine);
+void intel_engine_apply_whitelist(struct intel_engine_cs *engine);
 
 void intel_engine_init_workarounds(struct intel_engine_cs *engine);
 void intel_engine_apply_workarounds(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index d76a048c3954..67017d5175b8 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -94,17 +94,23 @@ read_nonprivs(struct i915_gem_context *ctx, struct intel_engine_cs *engine)
 	return ERR_PTR(err);
 }
 
-static u32 get_whitelist_reg(const struct whitelist *w, unsigned int i)
+static u32
+get_whitelist_reg(const struct intel_engine_cs *engine, unsigned int i)
 {
-	return i < w->count ? i915_mmio_reg_offset(w->reg[i]) : w->nopid;
+	i915_reg_t reg = i < engine->whitelist.count ?
+			 engine->whitelist.list[i].reg :
+			 RING_NOPID(engine->mmio_base);
+
+	return i915_mmio_reg_offset(reg);
 }
 
-static void print_results(const struct whitelist *w, const u32 *results)
+static void
+print_results(const struct intel_engine_cs *engine, const u32 *results)
 {
 	unsigned int i;
 
 	for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) {
-		u32 expected = get_whitelist_reg(w, i);
+		u32 expected = get_whitelist_reg(engine, i);
 		u32 actual = results[i];
 
 		pr_info("RING_NONPRIV[%d]: expected 0x%08x, found 0x%08x\n",
@@ -112,8 +118,7 @@ static void print_results(const struct whitelist *w, const u32 *results)
 	}
 }
 
-static int check_whitelist(const struct whitelist *w,
-			   struct i915_gem_context *ctx,
+static int check_whitelist(struct i915_gem_context *ctx,
 			   struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_object *results;
@@ -141,11 +146,11 @@ static int check_whitelist(const struct whitelist *w,
 	}
 
 	for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) {
-		u32 expected = get_whitelist_reg(w, i);
+		u32 expected = get_whitelist_reg(engine, i);
 		u32 actual = vaddr[i];
 
 		if (expected != actual) {
-			print_results(w, vaddr);
+			print_results(engine, vaddr);
 			pr_err("Invalid RING_NONPRIV[%d], expected 0x%08x, found 0x%08x\n",
 			       i, expected, actual);
 
@@ -217,7 +222,6 @@ switch_to_scratch_context(struct intel_engine_cs *engine,
 
 static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 					int (*reset)(struct intel_engine_cs *),
-					const struct whitelist *w,
 					const char *name)
 {
 	struct drm_i915_private *i915 = engine->i915;
@@ -227,7 +231,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 	int err;
 
 	pr_info("Checking %d whitelisted registers (RING_NONPRIV) [%s]\n",
-		w->count, name);
+		engine->whitelist.count, name);
 
 	if (want_spin) {
 		err = igt_spinner_init(&spin, i915);
@@ -239,7 +243,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 
-	err = check_whitelist(w, ctx, engine);
+	err = check_whitelist(ctx, engine);
 	if (err) {
 		pr_err("Invalid whitelist *before* %s reset!\n", name);
 		goto out;
@@ -263,7 +267,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 		goto out;
 	}
 
-	err = check_whitelist(w, ctx, engine);
+	err = check_whitelist(ctx, engine);
 	if (err) {
 		pr_err("Whitelist not preserved in context across %s reset!\n",
 		       name);
@@ -276,7 +280,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 
-	err = check_whitelist(w, ctx, engine);
+	err = check_whitelist(ctx, engine);
 	if (err) {
 		pr_err("Invalid whitelist *after* %s reset in fresh context!\n",
 		       name);
@@ -292,22 +296,18 @@ static int live_reset_whitelist(void *arg)
 {
 	struct drm_i915_private *i915 = arg;
 	struct intel_engine_cs *engine = i915->engine[RCS];
-	struct whitelist w;
 	int err = 0;
 
 	/* If we reset the gpu, we should not lose the RING_NONPRIV */
 
-	if (!engine)
-		return 0;
-
-	if (!whitelist_build(engine, &w))
+	if (!engine || engine->whitelist.count == 0)
 		return 0;
 
 	igt_global_reset_lock(i915);
 
 	if (intel_has_reset_engine(i915)) {
 		err = check_whitelist_across_reset(engine,
-						   do_engine_reset, &w,
+						   do_engine_reset,
 						   "engine");
 		if (err)
 			goto out;
@@ -315,7 +315,7 @@ static int live_reset_whitelist(void *arg)
 
 	if (intel_has_gpu_reset(i915)) {
 		err = check_whitelist_across_reset(engine,
-						   do_device_reset, &w,
+						   do_device_reset,
 						   "device");
 		if (err)
 			goto out;
-- 
2.19.1

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

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

* [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework
  2018-12-03 11:46 [PATCH v3 0/7] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
@ 2018-12-03 11:46 ` Tvrtko Ursulin
  0 siblings, 0 replies; 27+ messages in thread
From: Tvrtko Ursulin @ 2018-12-03 11:46 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Instead of having a separate list of white-listed registers we can
trivially move this to the common workarounds framework.

This brings us one step closer to the goal of driving all workaround
classes using the same code.

v2:
 * Use GEM_DEBUG_WARN_ON for the sanity check. (Chris Wilson)

v3:
 * API rename. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_engine_cs.c        |  1 +
 drivers/gpu/drm/i915/intel_lrc.c              |  5 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h       |  1 +
 drivers/gpu/drm/i915/intel_workarounds.c      | 83 ++++++++-----------
 drivers/gpu/drm/i915/intel_workarounds.h      |  3 +-
 .../drm/i915/selftests/intel_workarounds.c    | 40 ++++-----
 6 files changed, 60 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index ef5d202e9d45..496462d77ebc 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -725,6 +725,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
 	i915_timeline_fini(&engine->timeline);
 
 	intel_wa_list_free(&engine->wa_list);
+	intel_wa_list_free(&engine->whitelist);
 }
 
 u64 intel_engine_get_active_head(const struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 7a7787cbafee..af61a3cf9cb8 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1643,7 +1643,7 @@ static int gen8_init_render_ring(struct intel_engine_cs *engine)
 	if (ret)
 		return ret;
 
-	intel_whitelist_workarounds_apply(engine);
+	intel_engine_apply_whitelist(engine);
 
 	/* We need to disable the AsyncFlip performance optimisations in order
 	 * to use MI_WAIT_FOR_EVENT within the CS. It should already be
@@ -1666,7 +1666,7 @@ static int gen9_init_render_ring(struct intel_engine_cs *engine)
 	if (ret)
 		return ret;
 
-	intel_whitelist_workarounds_apply(engine);
+	intel_engine_apply_whitelist(engine);
 
 	return 0;
 }
@@ -2316,6 +2316,7 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
 			  ret);
 	}
 
+	intel_engine_init_whitelist(engine);
 	intel_engine_init_workarounds(engine);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c5ff3d31cab7..91a750e90dc4 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -453,6 +453,7 @@ struct intel_engine_cs {
 	struct intel_hw_status_page status_page;
 	struct i915_ctx_workarounds wa_ctx;
 	struct i915_wa_list wa_list;
+	struct i915_wa_list whitelist;
 	struct i915_vma *scratch;
 
 	u32             irq_keep_mask; /* always keep these interrupts */
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index e7b0c6b98d94..0350513bf7f7 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -1010,29 +1010,20 @@ bool intel_gt_verify_workarounds(struct drm_i915_private *dev_priv,
 	return wa_list_verify(dev_priv, &dev_priv->gt_wa_list, from);
 }
 
-struct whitelist {
-	i915_reg_t reg[RING_MAX_NONPRIV_SLOTS];
-	unsigned int count;
-	u32 nopid;
-};
-
-static void whitelist_reg(struct whitelist *w, i915_reg_t reg)
+static void
+whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
 {
-	if (GEM_DEBUG_WARN_ON(w->count >= RING_MAX_NONPRIV_SLOTS))
-		return;
-
-	w->reg[w->count++] = reg;
-}
+	struct i915_wa wa = {
+		.reg = reg
+	};
 
-static void bdw_whitelist_build(struct whitelist *w)
-{
-}
+	if (GEM_DEBUG_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
+		return;
 
-static void chv_whitelist_build(struct whitelist *w)
-{
+	wal_add(wal, &wa);
 }
 
-static void gen9_whitelist_build(struct whitelist *w)
+static void gen9_whitelist_build(struct i915_wa_list *w)
 {
 	/* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */
 	whitelist_reg(w, GEN9_CTX_PREEMPT_REG);
@@ -1044,7 +1035,7 @@ static void gen9_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN8_HDC_CHICKEN1);
 }
 
-static void skl_whitelist_build(struct whitelist *w)
+static void skl_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 
@@ -1052,12 +1043,12 @@ static void skl_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN8_L3SQCREG4);
 }
 
-static void bxt_whitelist_build(struct whitelist *w)
+static void bxt_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 }
 
-static void kbl_whitelist_build(struct whitelist *w)
+static void kbl_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 
@@ -1065,7 +1056,7 @@ static void kbl_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN8_L3SQCREG4);
 }
 
-static void glk_whitelist_build(struct whitelist *w)
+static void glk_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 
@@ -1073,18 +1064,18 @@ static void glk_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN9_SLICE_COMMON_ECO_CHICKEN1);
 }
 
-static void cfl_whitelist_build(struct whitelist *w)
+static void cfl_whitelist_build(struct i915_wa_list *w)
 {
 	gen9_whitelist_build(w);
 }
 
-static void cnl_whitelist_build(struct whitelist *w)
+static void cnl_whitelist_build(struct i915_wa_list *w)
 {
 	/* WaEnablePreemptionGranularityControlByUMD:cnl */
 	whitelist_reg(w, GEN8_CS_CHICKEN1);
 }
 
-static void icl_whitelist_build(struct whitelist *w)
+static void icl_whitelist_build(struct i915_wa_list *w)
 {
 	/* WaAllowUMDToModifyHalfSliceChicken7:icl */
 	whitelist_reg(w, GEN9_HALF_SLICE_CHICKEN7);
@@ -1093,22 +1084,21 @@ static void icl_whitelist_build(struct whitelist *w)
 	whitelist_reg(w, GEN10_SAMPLER_MODE);
 }
 
-static struct whitelist *whitelist_build(struct intel_engine_cs *engine,
-					 struct whitelist *w)
+void intel_engine_init_whitelist(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *i915 = engine->i915;
+	struct i915_wa_list *w = &engine->whitelist;
 
 	GEM_BUG_ON(engine->id != RCS);
 
-	w->count = 0;
-	w->nopid = i915_mmio_reg_offset(RING_NOPID(engine->mmio_base));
+	wa_init_start(w, "whitelist");
 
 	if (INTEL_GEN(i915) < 8)
-		return NULL;
+		return;
 	else if (IS_BROADWELL(i915))
-		bdw_whitelist_build(w);
+		return;
 	else if (IS_CHERRYVIEW(i915))
-		chv_whitelist_build(w);
+		return;
 	else if (IS_SKYLAKE(i915))
 		skl_whitelist_build(w);
 	else if (IS_BROXTON(i915))
@@ -1126,37 +1116,30 @@ static struct whitelist *whitelist_build(struct intel_engine_cs *engine,
 	else
 		MISSING_CASE(INTEL_GEN(i915));
 
-	return w;
+	wa_init_finish(w);
 }
 
-static void whitelist_apply(struct intel_engine_cs *engine,
-			    const struct whitelist *w)
+void intel_engine_apply_whitelist(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	const struct i915_wa_list *wal = &engine->whitelist;
 	const u32 base = engine->mmio_base;
+	struct i915_wa *wa;
 	unsigned int i;
 
-	if (!w)
+	if (!wal->count)
 		return;
 
-	intel_uncore_forcewake_get(engine->i915, FORCEWAKE_ALL);
-
-	for (i = 0; i < w->count; i++)
-		I915_WRITE_FW(RING_FORCE_TO_NONPRIV(base, i),
-			      i915_mmio_reg_offset(w->reg[i]));
+	for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
+		I915_WRITE(RING_FORCE_TO_NONPRIV(base, i),
+			   i915_mmio_reg_offset(wa->reg));
 
 	/* And clear the rest just in case of garbage */
 	for (; i < RING_MAX_NONPRIV_SLOTS; i++)
-		I915_WRITE_FW(RING_FORCE_TO_NONPRIV(base, i), w->nopid);
-
-	intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL);
-}
+		I915_WRITE(RING_FORCE_TO_NONPRIV(base, i),
+			   i915_mmio_reg_offset(RING_NOPID(base)));
 
-void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine)
-{
-	struct whitelist w;
-
-	whitelist_apply(engine, whitelist_build(engine, &w));
+	DRM_DEBUG_DRIVER("Applied %u %s workarounds\n", wal->count, wal->name);
 }
 
 static void rcs_engine_wa_init(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
index 2506db0380f1..bc6aa503f6c6 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -35,7 +35,8 @@ void intel_gt_apply_workarounds(struct drm_i915_private *dev_priv);
 bool intel_gt_verify_workarounds(struct drm_i915_private *dev_priv,
 				 const char *from);
 
-void intel_whitelist_workarounds_apply(struct intel_engine_cs *engine);
+void intel_engine_init_whitelist(struct intel_engine_cs *engine);
+void intel_engine_apply_whitelist(struct intel_engine_cs *engine);
 
 void intel_engine_init_workarounds(struct intel_engine_cs *engine);
 void intel_engine_apply_workarounds(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index 0c118d18f825..7b37bbce2308 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -94,17 +94,23 @@ read_nonprivs(struct i915_gem_context *ctx, struct intel_engine_cs *engine)
 	return ERR_PTR(err);
 }
 
-static u32 get_whitelist_reg(const struct whitelist *w, unsigned int i)
+static u32
+get_whitelist_reg(const struct intel_engine_cs *engine, unsigned int i)
 {
-	return i < w->count ? i915_mmio_reg_offset(w->reg[i]) : w->nopid;
+	i915_reg_t reg = i < engine->whitelist.count ?
+			 engine->whitelist.list[i].reg :
+			 RING_NOPID(engine->mmio_base);
+
+	return i915_mmio_reg_offset(reg);
 }
 
-static void print_results(const struct whitelist *w, const u32 *results)
+static void
+print_results(const struct intel_engine_cs *engine, const u32 *results)
 {
 	unsigned int i;
 
 	for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) {
-		u32 expected = get_whitelist_reg(w, i);
+		u32 expected = get_whitelist_reg(engine, i);
 		u32 actual = results[i];
 
 		pr_info("RING_NONPRIV[%d]: expected 0x%08x, found 0x%08x\n",
@@ -112,8 +118,7 @@ static void print_results(const struct whitelist *w, const u32 *results)
 	}
 }
 
-static int check_whitelist(const struct whitelist *w,
-			   struct i915_gem_context *ctx,
+static int check_whitelist(struct i915_gem_context *ctx,
 			   struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_object *results;
@@ -141,11 +146,11 @@ static int check_whitelist(const struct whitelist *w,
 	}
 
 	for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) {
-		u32 expected = get_whitelist_reg(w, i);
+		u32 expected = get_whitelist_reg(engine, i);
 		u32 actual = vaddr[i];
 
 		if (expected != actual) {
-			print_results(w, vaddr);
+			print_results(engine, vaddr);
 			pr_err("Invalid RING_NONPRIV[%d], expected 0x%08x, found 0x%08x\n",
 			       i, expected, actual);
 
@@ -217,7 +222,6 @@ switch_to_scratch_context(struct intel_engine_cs *engine,
 
 static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 					int (*reset)(struct intel_engine_cs *),
-					const struct whitelist *w,
 					const char *name)
 {
 	struct drm_i915_private *i915 = engine->i915;
@@ -227,7 +231,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 	int err;
 
 	pr_info("Checking %d whitelisted registers (RING_NONPRIV) [%s]\n",
-		w->count, name);
+		engine->whitelist.count, name);
 
 	if (want_spin) {
 		err = igt_spinner_init(&spin, i915);
@@ -239,7 +243,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 
-	err = check_whitelist(w, ctx, engine);
+	err = check_whitelist(ctx, engine);
 	if (err) {
 		pr_err("Invalid whitelist *before* %s reset!\n", name);
 		goto out;
@@ -263,7 +267,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 		goto out;
 	}
 
-	err = check_whitelist(w, ctx, engine);
+	err = check_whitelist(ctx, engine);
 	if (err) {
 		pr_err("Whitelist not preserved in context across %s reset!\n",
 		       name);
@@ -276,7 +280,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 
-	err = check_whitelist(w, ctx, engine);
+	err = check_whitelist(ctx, engine);
 	if (err) {
 		pr_err("Invalid whitelist *after* %s reset in fresh context!\n",
 		       name);
@@ -292,22 +296,18 @@ static int live_reset_whitelist(void *arg)
 {
 	struct drm_i915_private *i915 = arg;
 	struct intel_engine_cs *engine = i915->engine[RCS];
-	struct whitelist w;
 	int err = 0;
 
 	/* If we reset the gpu, we should not lose the RING_NONPRIV */
 
-	if (!engine)
-		return 0;
-
-	if (!whitelist_build(engine, &w))
+	if (!engine || engine->whitelist.count == 0)
 		return 0;
 
 	igt_global_reset_lock(i915);
 
 	if (intel_has_reset_engine(i915)) {
 		err = check_whitelist_across_reset(engine,
-						   do_engine_reset, &w,
+						   do_engine_reset,
 						   "engine");
 		if (err)
 			goto out;
@@ -315,7 +315,7 @@ static int live_reset_whitelist(void *arg)
 
 	if (intel_has_gpu_reset(i915)) {
 		err = check_whitelist_across_reset(engine,
-						   do_device_reset, &w,
+						   do_device_reset,
 						   "device");
 		if (err)
 			goto out;
-- 
2.19.1

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

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

end of thread, other threads:[~2018-12-03 12:50 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-30 17:44 [PATCH v2 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
2018-11-30 17:44 ` [PATCH 1/7] drm/i915: Record GT workarounds in a list Tvrtko Ursulin
2018-11-30 21:51   ` Chris Wilson
2018-11-30 17:44 ` [PATCH 2/7] drm/i915: Introduce per-engine workarounds Tvrtko Ursulin
2018-11-30 20:56   ` Chris Wilson
2018-11-30 21:53   ` Chris Wilson
2018-11-30 22:13   ` Rodrigo Vivi
2018-11-30 17:44 ` [PATCH 3/7] drm/i915: Verify GT workaround state after GPU init Tvrtko Ursulin
2018-11-30 21:55   ` Chris Wilson
2018-11-30 17:44 ` [PATCH 4/7] drm/i915/selftests: Add tests for GT and engine workaround verification Tvrtko Ursulin
2018-11-30 21:04   ` Chris Wilson
2018-11-30 22:01     ` Chris Wilson
2018-11-30 17:44 ` [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework Tvrtko Ursulin
2018-11-30 21:08   ` Chris Wilson
2018-11-30 22:03     ` Chris Wilson
2018-11-30 17:44 ` [PATCH 6/7] drm/i915: Fuse per-context workaround handling with the common framework Tvrtko Ursulin
2018-11-30 21:22   ` Chris Wilson
2018-12-03 10:34     ` Tvrtko Ursulin
2018-11-30 17:44 ` [PATCH 7/7] drm/i915: Trim unused workaround list entries Tvrtko Ursulin
2018-11-30 21:58   ` Chris Wilson
2018-12-03 10:39     ` Tvrtko Ursulin
2018-11-30 17:53 ` ✗ Fi.CI.CHECKPATCH: warning for Restore workarounds after engine reset and unify their handling (rev2) Patchwork
2018-11-30 17:56 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-11-30 18:24 ` ✓ Fi.CI.BAT: success " Patchwork
2018-12-01 13:36 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-12-03 11:46 [PATCH v3 0/7] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
2018-12-03 11:46 ` [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework Tvrtko Ursulin
2018-12-03 12:50 [PATCH v4 0/7] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
2018-12-03 12:50 ` [PATCH 5/7] drm/i915: Move register white-listing to the common workaround framework 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.