* [PATCH 1/8] drm/i915: Record GT workarounds in a list
2018-11-30 11:31 [PATCH 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
@ 2018-11-30 11:31 ` Tvrtko Ursulin
2018-11-30 11:31 ` [PATCH 2/8] drm/i915: Introduce per-engine workarounds Tvrtko Ursulin
` (7 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 11:31 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] 24+ messages in thread* [PATCH 2/8] drm/i915: Introduce per-engine workarounds
2018-11-30 11:31 [PATCH 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
2018-11-30 11:31 ` [PATCH 1/8] drm/i915: Record GT workarounds in a list Tvrtko Ursulin
@ 2018-11-30 11:31 ` Tvrtko Ursulin
2018-11-30 11:31 ` [PATCH 3/8] drm/i915: Verify GT workaround state at runtime Tvrtko Ursulin
` (6 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 11:31 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] 24+ messages in thread* [PATCH 3/8] drm/i915: Verify GT workaround state at runtime
2018-11-30 11:31 [PATCH 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
2018-11-30 11:31 ` [PATCH 1/8] drm/i915: Record GT workarounds in a list Tvrtko Ursulin
2018-11-30 11:31 ` [PATCH 2/8] drm/i915: Introduce per-engine workarounds Tvrtko Ursulin
@ 2018-11-30 11:31 ` Tvrtko Ursulin
2018-11-30 11:38 ` Chris Wilson
2018-11-30 11:54 ` Chris Wilson
2018-11-30 11:31 ` [PATCH 4/8] drm/i915: Verify engine " Tvrtko Ursulin
` (5 subsequent siblings)
8 siblings, 2 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 11:31 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.
At this stage these are the driver initialization and engine reset.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 4 +++
drivers/gpu/drm/i915/i915_gem.c | 3 ++
drivers/gpu/drm/i915/intel_workarounds.c | 46 ++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_workarounds.h | 2 ++
4 files changed, 55 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2f3dc1cf83a6..14d019c9455b 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;
@@ -2362,6 +2363,9 @@ int i915_reset_engine(struct intel_engine_cs *engine, const char *msg)
goto out;
}
+ /* Catch GT workarounds affected by engine reset. */
+ intel_gt_workarounds_verify(engine->i915, engine->name);
+
/*
* The request that caused the hang is stuck on elsp, we know the
* active request and can drop it, adjust head to skip the offending
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..a5c0d206b2a4 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -981,6 +981,52 @@ void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
wa_list_apply(dev_priv, &dev_priv->gt_wa_list);
}
+static void
+wa_fail(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
+{
+ 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);
+}
+
+static void
+wa_verify_bits(const struct i915_wa *wa, u32 cur, const char *name,
+ const char *from)
+{
+ u32 bits = wa->mask;
+ u32 cur_ = cur;
+ u32 val_ = wa->val;
+
+ while (bits) {
+ if ((bits & 1) && ((cur_ & 1) != (val_ & 1))) {
+ wa_fail(wa, cur, name, from);
+ break;
+ }
+
+ bits >>= 1;
+ cur_ >>= 1;
+ val_ >>= 1;
+ }
+}
+
+static void 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;
+
+ for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
+ wa_verify_bits(wa, I915_READ(wa->reg), wal->name, from);
+}
+
+void intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
+ const char *from)
+{
+ 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..845c18dc110d 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);
+void 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] 24+ messages in thread* Re: [PATCH 3/8] drm/i915: Verify GT workaround state at runtime
2018-11-30 11:31 ` [PATCH 3/8] drm/i915: Verify GT workaround state at runtime Tvrtko Ursulin
@ 2018-11-30 11:38 ` Chris Wilson
2018-11-30 12:02 ` Tvrtko Ursulin
2018-11-30 11:54 ` Chris Wilson
1 sibling, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2018-11-30 11:38 UTC (permalink / raw)
To: Intel-gfx, Tvrtko Ursulin
Quoting Tvrtko Ursulin (2018-11-30 11:31:56)
> 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.
>
> At this stage these are the driver initialization and engine reset.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 4 +++
> drivers/gpu/drm/i915/i915_gem.c | 3 ++
> drivers/gpu/drm/i915/intel_workarounds.c | 46 ++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_workarounds.h | 2 ++
> 4 files changed, 55 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2f3dc1cf83a6..14d019c9455b 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;
>
> @@ -2362,6 +2363,9 @@ int i915_reset_engine(struct intel_engine_cs *engine, const char *msg)
> goto out;
> }
>
> + /* Catch GT workarounds affected by engine reset. */
> + intel_gt_workarounds_verify(engine->i915, engine->name);
I'd rather, quite strongly, not have this inside i915_reset_engine()
itself. i915_reset_engine() is [designed to be] called from atomic
context. Adding I915_READ() here is questionable. My preference is to
have all this inside selftests/intel_workarounds where we can cross
check the gt/all-engines after device reset and other engine reset.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/8] drm/i915: Verify GT workaround state at runtime
2018-11-30 11:38 ` Chris Wilson
@ 2018-11-30 12:02 ` Tvrtko Ursulin
2018-11-30 14:36 ` Chris Wilson
0 siblings, 1 reply; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 12:02 UTC (permalink / raw)
To: Chris Wilson, Intel-gfx
On 30/11/2018 11:38, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-11-30 11:31:56)
>> 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.
>>
>> At this stage these are the driver initialization and engine reset.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 4 +++
>> drivers/gpu/drm/i915/i915_gem.c | 3 ++
>> drivers/gpu/drm/i915/intel_workarounds.c | 46 ++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_workarounds.h | 2 ++
>> 4 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 2f3dc1cf83a6..14d019c9455b 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;
>>
>> @@ -2362,6 +2363,9 @@ int i915_reset_engine(struct intel_engine_cs *engine, const char *msg)
>> goto out;
>> }
>>
>> + /* Catch GT workarounds affected by engine reset. */
>> + intel_gt_workarounds_verify(engine->i915, engine->name);
>
> I'd rather, quite strongly, not have this inside i915_reset_engine()
> itself. i915_reset_engine() is [designed to be] called from atomic
> context. Adding I915_READ() here is questionable. My preference is to
> have all this inside selftests/intel_workarounds where we can cross
> check the gt/all-engines after device reset and other engine reset.
I wasn't sure myself how much value checking it here adds, but regarding
I915_READs, they are happening during engine->init_hw anyway. So I don't
think a few more would harm. But yes, as said, since I was unsure myself
I am happy to drop this hunk.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/8] drm/i915: Verify GT workaround state at runtime
2018-11-30 12:02 ` Tvrtko Ursulin
@ 2018-11-30 14:36 ` Chris Wilson
0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2018-11-30 14:36 UTC (permalink / raw)
To: Intel-gfx, Tvrtko Ursulin
Quoting Tvrtko Ursulin (2018-11-30 12:02:56)
>
> On 30/11/2018 11:38, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-11-30 11:31:56)
> >> 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.
> >>
> >> At this stage these are the driver initialization and engine reset.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.c | 4 +++
> >> drivers/gpu/drm/i915/i915_gem.c | 3 ++
> >> drivers/gpu/drm/i915/intel_workarounds.c | 46 ++++++++++++++++++++++++
> >> drivers/gpu/drm/i915/intel_workarounds.h | 2 ++
> >> 4 files changed, 55 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index 2f3dc1cf83a6..14d019c9455b 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;
> >>
> >> @@ -2362,6 +2363,9 @@ int i915_reset_engine(struct intel_engine_cs *engine, const char *msg)
> >> goto out;
> >> }
> >>
> >> + /* Catch GT workarounds affected by engine reset. */
> >> + intel_gt_workarounds_verify(engine->i915, engine->name);
> >
> > I'd rather, quite strongly, not have this inside i915_reset_engine()
> > itself. i915_reset_engine() is [designed to be] called from atomic
> > context. Adding I915_READ() here is questionable. My preference is to
> > have all this inside selftests/intel_workarounds where we can cross
> > check the gt/all-engines after device reset and other engine reset.
>
> I wasn't sure myself how much value checking it here adds, but regarding
> I915_READs, they are happening during engine->init_hw anyway. So I don't
> think a few more would harm. But yes, as said, since I was unsure myself
> I am happy to drop this hunk.
Adding the I915_READ isn't the end-of-the-world, I am more wary of the
slipperly slope and suddenly finding ourselves with a per-engine reset
that is no longer suitable for fast resets, possibly from hardirq
context. (Although that's more likely to be softirq really.)
In this case, I think we can satisfy ourselves with a comprehensive yet
fast test suite. And there's no harm in having the checks on idle for
runtime consistency checking (we can even inject SRM before idling for
completeness).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/8] drm/i915: Verify GT workaround state at runtime
2018-11-30 11:31 ` [PATCH 3/8] drm/i915: Verify GT workaround state at runtime Tvrtko Ursulin
2018-11-30 11:38 ` Chris Wilson
@ 2018-11-30 11:54 ` Chris Wilson
2018-11-30 15:21 ` Tvrtko Ursulin
1 sibling, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2018-11-30 11:54 UTC (permalink / raw)
To: Intel-gfx, Tvrtko Ursulin
Quoting Tvrtko Ursulin (2018-11-30 11:31:56)
> 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.
>
> At this stage these are the driver initialization and engine reset.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 4 +++
> drivers/gpu/drm/i915/i915_gem.c | 3 ++
> drivers/gpu/drm/i915/intel_workarounds.c | 46 ++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_workarounds.h | 2 ++
> 4 files changed, 55 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 2f3dc1cf83a6..14d019c9455b 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;
>
> @@ -2362,6 +2363,9 @@ int i915_reset_engine(struct intel_engine_cs *engine, const char *msg)
> goto out;
> }
>
> + /* Catch GT workarounds affected by engine reset. */
> + intel_gt_workarounds_verify(engine->i915, engine->name);
> +
> /*
> * The request that caused the hang is stuck on elsp, we know the
> * active request and can drop it, adjust head to skip the offending
> 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..a5c0d206b2a4 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -981,6 +981,52 @@ void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> wa_list_apply(dev_priv, &dev_priv->gt_wa_list);
> }
>
> +static void
> +wa_fail(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
> +{
> + 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);
> +}
> +
> +static void
> +wa_verify_bits(const struct i915_wa *wa, u32 cur, const char *name,
> + const char *from)
> +{
> + u32 bits = wa->mask;
> + u32 cur_ = cur;
> + u32 val_ = wa->val;
Make sure wa->mask is set to ~0u for whole registers, which it must be
for the wa_fail.
if ((cur ^ wa->val) & wa->mask) {
wa_fail(wa, cur, name, whom);
return false; /* make it bool now to save churn later? */
}
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 3/8] drm/i915: Verify GT workaround state at runtime
2018-11-30 11:54 ` Chris Wilson
@ 2018-11-30 15:21 ` Tvrtko Ursulin
0 siblings, 0 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 15:21 UTC (permalink / raw)
To: Chris Wilson, Intel-gfx
On 30/11/2018 11:54, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-11-30 11:31:56)
>> 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.
>>
>> At this stage these are the driver initialization and engine reset.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 4 +++
>> drivers/gpu/drm/i915/i915_gem.c | 3 ++
>> drivers/gpu/drm/i915/intel_workarounds.c | 46 ++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_workarounds.h | 2 ++
>> 4 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 2f3dc1cf83a6..14d019c9455b 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;
>>
>> @@ -2362,6 +2363,9 @@ int i915_reset_engine(struct intel_engine_cs *engine, const char *msg)
>> goto out;
>> }
>>
>> + /* Catch GT workarounds affected by engine reset. */
>> + intel_gt_workarounds_verify(engine->i915, engine->name);
>> +
>> /*
>> * The request that caused the hang is stuck on elsp, we know the
>> * active request and can drop it, adjust head to skip the offending
>> 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..a5c0d206b2a4 100644
>> --- a/drivers/gpu/drm/i915/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
>> @@ -981,6 +981,52 @@ void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
>> wa_list_apply(dev_priv, &dev_priv->gt_wa_list);
>> }
>>
>> +static void
>> +wa_fail(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
>> +{
>> + 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);
>> +}
>> +
>> +static void
>> +wa_verify_bits(const struct i915_wa *wa, u32 cur, const char *name,
>> + const char *from)
>> +{
>> + u32 bits = wa->mask;
>> + u32 cur_ = cur;
>> + u32 val_ = wa->val;
>
> Make sure wa->mask is set to ~0u for whole registers, which it must be
> for the wa_fail.
It is (wa_write), unless you spotted a miss?
> if ((cur ^ wa->val) & wa->mask) {
> wa_fail(wa, cur, name, whom);
Not sure if I feel like laughing or crying at my implementation. :) I
was cleaning up the lot when I realized it all reduces to simplicity,
but obviously missed this one..
> return false; /* make it bool now to save churn later? */
> }
Okay.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/8] drm/i915: Verify engine workaround state at runtime
2018-11-30 11:31 [PATCH 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
` (2 preceding siblings ...)
2018-11-30 11:31 ` [PATCH 3/8] drm/i915: Verify GT workaround state at runtime Tvrtko Ursulin
@ 2018-11-30 11:31 ` Tvrtko Ursulin
2018-11-30 11:31 ` [PATCH 5/8] drm/i915/selftests: Add tests for GT and engine workaround verification Tvrtko Ursulin
` (4 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 11:31 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Analogue to the previous patch we add at runtime verification that after
engine reset all respective workarounds have been correctly applied.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 3 +++
drivers/gpu/drm/i915/intel_workarounds.c | 6 ++++++
drivers/gpu/drm/i915/intel_workarounds.h | 2 ++
3 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 14d019c9455b..a5add317a06b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2382,6 +2382,9 @@ int i915_reset_engine(struct intel_engine_cs *engine, const char *msg)
if (ret)
goto out;
+ /* Catch engine workarounds not restored by engine re-init. */
+ intel_engine_workarounds_verify(engine, engine->name);
+
out:
intel_engine_cancel_stop_cs(engine);
i915_gem_reset_finish_engine(engine);
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index a5c0d206b2a4..2d17d8a36a57 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -1313,6 +1313,12 @@ void intel_engine_workarounds_apply(struct intel_engine_cs *engine)
wa_list_apply(engine->i915, &engine->wa_list);
}
+void intel_engine_workarounds_verify(struct intel_engine_cs *engine,
+ const char *from)
+{
+ wa_list_verify(engine->i915, &engine->wa_list, from);
+}
+
#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 845c18dc110d..f72cfda32d68 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -40,5 +40,7 @@ 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);
+void intel_engine_workarounds_verify(struct intel_engine_cs *engine,
+ const char *from);
#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] 24+ messages in thread* [PATCH 5/8] drm/i915/selftests: Add tests for GT and engine workaround verification
2018-11-30 11:31 [PATCH 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
` (3 preceding siblings ...)
2018-11-30 11:31 ` [PATCH 4/8] drm/i915: Verify engine " Tvrtko Ursulin
@ 2018-11-30 11:31 ` Tvrtko Ursulin
2018-11-30 11:43 ` Chris Wilson
2018-11-30 11:31 ` [PATCH 6/8] drm/i915: Move register white-listing to the common workaround framework Tvrtko Ursulin
` (3 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 11:31 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.)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_workarounds.c | 21 +++--
drivers/gpu/drm/i915/intel_workarounds.h | 4 +-
.../drm/i915/selftests/intel_workarounds.c | 90 +++++++++++++++++++
3 files changed, 105 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 2d17d8a36a57..a21a21855e6a 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -990,7 +990,7 @@ wa_fail(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
cur, cur & wa->mask, wa->val, wa->mask);
}
-static void
+static bool
wa_verify_bits(const struct i915_wa *wa, u32 cur, const char *name,
const char *from)
{
@@ -1001,30 +1001,35 @@ wa_verify_bits(const struct i915_wa *wa, u32 cur, const char *name,
while (bits) {
if ((bits & 1) && ((cur_ & 1) != (val_ & 1))) {
wa_fail(wa, cur, name, from);
- break;
+ return false;
}
bits >>= 1;
cur_ >>= 1;
val_ >>= 1;
}
+
+ return true;
}
-static void wa_list_verify(struct drm_i915_private *dev_priv,
+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 res = true;
for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
- wa_verify_bits(wa, I915_READ(wa->reg), wal->name, from);
+ res &= wa_verify_bits(wa, I915_READ(wa->reg), wal->name, from);
+
+ return res;
}
-void intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
+bool intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
const char *from)
{
- wa_list_verify(dev_priv, &dev_priv->gt_wa_list, from);
+ return wa_list_verify(dev_priv, &dev_priv->gt_wa_list, from);
}
struct whitelist {
@@ -1313,10 +1318,10 @@ void intel_engine_workarounds_apply(struct intel_engine_cs *engine)
wa_list_apply(engine->i915, &engine->wa_list);
}
-void intel_engine_workarounds_verify(struct intel_engine_cs *engine,
+bool intel_engine_workarounds_verify(struct intel_engine_cs *engine,
const char *from)
{
- wa_list_verify(engine->i915, &engine->wa_list, from);
+ return wa_list_verify(engine->i915, &engine->wa_list, from);
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
index f72cfda32d68..8f664d8b9e08 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/intel_workarounds.h
@@ -33,14 +33,14 @@ 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_gt_workarounds_verify(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);
void intel_engine_workarounds_init(struct intel_engine_cs *engine);
void intel_engine_workarounds_apply(struct intel_engine_cs *engine);
-void intel_engine_workarounds_verify(struct intel_engine_cs *engine,
+bool intel_engine_workarounds_verify(struct intel_engine_cs *engine,
const char *from);
#endif
diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index 80396b3592f5..c009eb2af7fc 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -327,10 +327,100 @@ static int live_reset_whitelist(void *arg)
return err;
}
+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");
+
+ set_bit(I915_RESET_BACKOFF, &error->flags);
+ set_bit(I915_RESET_HANDOFF, &error->flags);
+
+ ok = verify_gt_engine_wa(i915, "before reset");
+ if (!ok)
+ goto out;
+
+ intel_runtime_pm_get(i915);
+ i915_reset(i915, ~0, "live_workarounds");
+ intel_runtime_pm_put(i915);
+
+ ok = verify_gt_engine_wa(i915, "after reset");
+
+out:
+ clear_bit(I915_RESET_BACKOFF, &error->flags);
+
+ return ok ? 0 : -ESRCH;
+}
+
+static int
+live_engine_reset_gt_engine_workarounds(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ struct i915_gpu_error *error = &i915->gpu_error;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+ bool ok;
+
+ if (!intel_has_reset_engine(i915))
+ return 0;
+
+ for_each_engine(engine, i915, id) {
+ pr_info("Verifying after %s reset...\n", engine->name);
+
+ set_bit(I915_RESET_BACKOFF, &error->flags);
+ set_bit(I915_RESET_ENGINE + engine->id, &error->flags);
+
+ ok = verify_gt_engine_wa(i915, "before reset");
+ if (!ok)
+ goto out;
+
+ intel_runtime_pm_get(i915);
+ i915_reset_engine(engine, "live_workarounds");
+ intel_runtime_pm_put(i915);
+
+ ok = verify_gt_engine_wa(i915, "after reset");
+ if (!ok)
+ goto out;
+
+ clear_bit(I915_RESET_ENGINE + engine->id, &error->flags);
+ clear_bit(I915_RESET_BACKOFF, &error->flags);
+ }
+
+out:
+ if (engine) {
+ clear_bit(I915_RESET_ENGINE + engine->id, &error->flags);
+ clear_bit(I915_RESET_BACKOFF, &error->flags);
+ }
+
+ return ok ? 0 : -ESRCH;
+}
+
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] 24+ messages in thread* Re: [PATCH 5/8] drm/i915/selftests: Add tests for GT and engine workaround verification
2018-11-30 11:31 ` [PATCH 5/8] drm/i915/selftests: Add tests for GT and engine workaround verification Tvrtko Ursulin
@ 2018-11-30 11:43 ` Chris Wilson
2018-11-30 15:15 ` Tvrtko Ursulin
0 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2018-11-30 11:43 UTC (permalink / raw)
To: Intel-gfx, Tvrtko Ursulin
Quoting Tvrtko Ursulin (2018-11-30 11:31:58)
> 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.)
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/intel_workarounds.c | 21 +++--
> drivers/gpu/drm/i915/intel_workarounds.h | 4 +-
> .../drm/i915/selftests/intel_workarounds.c | 90 +++++++++++++++++++
> 3 files changed, 105 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index 2d17d8a36a57..a21a21855e6a 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -990,7 +990,7 @@ wa_fail(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
> cur, cur & wa->mask, wa->val, wa->mask);
> }
>
> -static void
> +static bool
> wa_verify_bits(const struct i915_wa *wa, u32 cur, const char *name,
> const char *from)
> {
> @@ -1001,30 +1001,35 @@ wa_verify_bits(const struct i915_wa *wa, u32 cur, const char *name,
> while (bits) {
> if ((bits & 1) && ((cur_ & 1) != (val_ & 1))) {
> wa_fail(wa, cur, name, from);
> - break;
> + return false;
> }
>
> bits >>= 1;
> cur_ >>= 1;
> val_ >>= 1;
> }
> +
> + return true;
> }
>
> -static void wa_list_verify(struct drm_i915_private *dev_priv,
> +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 res = true;
>
> for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
> - wa_verify_bits(wa, I915_READ(wa->reg), wal->name, from);
> + res &= wa_verify_bits(wa, I915_READ(wa->reg), wal->name, from);
> +
> + return res;
> }
>
> -void intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
> +bool intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
> const char *from)
> {
> - wa_list_verify(dev_priv, &dev_priv->gt_wa_list, from);
> + return wa_list_verify(dev_priv, &dev_priv->gt_wa_list, from);
> }
>
> struct whitelist {
> @@ -1313,10 +1318,10 @@ void intel_engine_workarounds_apply(struct intel_engine_cs *engine)
> wa_list_apply(engine->i915, &engine->wa_list);
> }
>
> -void intel_engine_workarounds_verify(struct intel_engine_cs *engine,
> +bool intel_engine_workarounds_verify(struct intel_engine_cs *engine,
> const char *from)
> {
> - wa_list_verify(engine->i915, &engine->wa_list, from);
> + return wa_list_verify(engine->i915, &engine->wa_list, from);
> }
>
> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
> index f72cfda32d68..8f664d8b9e08 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.h
> +++ b/drivers/gpu/drm/i915/intel_workarounds.h
> @@ -33,14 +33,14 @@ 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_gt_workarounds_verify(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);
>
> void intel_engine_workarounds_init(struct intel_engine_cs *engine);
> void intel_engine_workarounds_apply(struct intel_engine_cs *engine);
> -void intel_engine_workarounds_verify(struct intel_engine_cs *engine,
> +bool intel_engine_workarounds_verify(struct intel_engine_cs *engine,
> const char *from);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> index 80396b3592f5..c009eb2af7fc 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> @@ -327,10 +327,100 @@ static int live_reset_whitelist(void *arg)
> return err;
> }
>
> +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);
Ok, ok.
> +
> + 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");
> +
> + set_bit(I915_RESET_BACKOFF, &error->flags);
> + set_bit(I915_RESET_HANDOFF, &error->flags);
> +
> + ok = verify_gt_engine_wa(i915, "before reset");
> + if (!ok)
> + goto out;
> +
> + intel_runtime_pm_get(i915);
Move RESET_HANDOFF here to avoid any repetition of earlier problems we
had with the reset being applied too early.
> + i915_reset(i915, ~0, "live_workarounds");
> + intel_runtime_pm_put(i915);
> +
> + ok = verify_gt_engine_wa(i915, "after reset");
> +
> +out:
> + clear_bit(I915_RESET_BACKOFF, &error->flags);
> +
> + return ok ? 0 : -ESRCH;
> +}
> +
> +static int
> +live_engine_reset_gt_engine_workarounds(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + struct i915_gpu_error *error = &i915->gpu_error;
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> + bool ok;
> +
> + if (!intel_has_reset_engine(i915))
> + return 0;
May be easier to take global_reset_lock/unlock from
selftests/intel_hanghceck.
> +
> + for_each_engine(engine, i915, id) {
> + pr_info("Verifying after %s reset...\n", engine->name);
> +
> + set_bit(I915_RESET_BACKOFF, &error->flags);
> + set_bit(I915_RESET_ENGINE + engine->id, &error->flags);
> +
> + ok = verify_gt_engine_wa(i915, "before reset");
> + if (!ok)
> + goto out;
> +
> + intel_runtime_pm_get(i915);
> + i915_reset_engine(engine, "live_workarounds");
> + intel_runtime_pm_put(i915);
Once idle, and once with a spinner?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 5/8] drm/i915/selftests: Add tests for GT and engine workaround verification
2018-11-30 11:43 ` Chris Wilson
@ 2018-11-30 15:15 ` Tvrtko Ursulin
2018-11-30 15:21 ` Chris Wilson
0 siblings, 1 reply; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 15:15 UTC (permalink / raw)
To: Chris Wilson, Intel-gfx
On 30/11/2018 11:43, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-11-30 11:31:58)
>> 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.)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_workarounds.c | 21 +++--
>> drivers/gpu/drm/i915/intel_workarounds.h | 4 +-
>> .../drm/i915/selftests/intel_workarounds.c | 90 +++++++++++++++++++
>> 3 files changed, 105 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
>> index 2d17d8a36a57..a21a21855e6a 100644
>> --- a/drivers/gpu/drm/i915/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
>> @@ -990,7 +990,7 @@ wa_fail(const struct i915_wa *wa, u32 cur, const char *name, const char *from)
>> cur, cur & wa->mask, wa->val, wa->mask);
>> }
>>
>> -static void
>> +static bool
>> wa_verify_bits(const struct i915_wa *wa, u32 cur, const char *name,
>> const char *from)
>> {
>> @@ -1001,30 +1001,35 @@ wa_verify_bits(const struct i915_wa *wa, u32 cur, const char *name,
>> while (bits) {
>> if ((bits & 1) && ((cur_ & 1) != (val_ & 1))) {
>> wa_fail(wa, cur, name, from);
>> - break;
>> + return false;
>> }
>>
>> bits >>= 1;
>> cur_ >>= 1;
>> val_ >>= 1;
>> }
>> +
>> + return true;
>> }
>>
>> -static void wa_list_verify(struct drm_i915_private *dev_priv,
>> +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 res = true;
>>
>> for (i = 0, wa = wal->list; i < wal->count; i++, wa++)
>> - wa_verify_bits(wa, I915_READ(wa->reg), wal->name, from);
>> + res &= wa_verify_bits(wa, I915_READ(wa->reg), wal->name, from);
>> +
>> + return res;
>> }
>>
>> -void intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
>> +bool intel_gt_workarounds_verify(struct drm_i915_private *dev_priv,
>> const char *from)
>> {
>> - wa_list_verify(dev_priv, &dev_priv->gt_wa_list, from);
>> + return wa_list_verify(dev_priv, &dev_priv->gt_wa_list, from);
>> }
>>
>> struct whitelist {
>> @@ -1313,10 +1318,10 @@ void intel_engine_workarounds_apply(struct intel_engine_cs *engine)
>> wa_list_apply(engine->i915, &engine->wa_list);
>> }
>>
>> -void intel_engine_workarounds_verify(struct intel_engine_cs *engine,
>> +bool intel_engine_workarounds_verify(struct intel_engine_cs *engine,
>> const char *from)
>> {
>> - wa_list_verify(engine->i915, &engine->wa_list, from);
>> + return wa_list_verify(engine->i915, &engine->wa_list, from);
>> }
>>
>> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>> diff --git a/drivers/gpu/drm/i915/intel_workarounds.h b/drivers/gpu/drm/i915/intel_workarounds.h
>> index f72cfda32d68..8f664d8b9e08 100644
>> --- a/drivers/gpu/drm/i915/intel_workarounds.h
>> +++ b/drivers/gpu/drm/i915/intel_workarounds.h
>> @@ -33,14 +33,14 @@ 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_gt_workarounds_verify(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);
>>
>> void intel_engine_workarounds_init(struct intel_engine_cs *engine);
>> void intel_engine_workarounds_apply(struct intel_engine_cs *engine);
>> -void intel_engine_workarounds_verify(struct intel_engine_cs *engine,
>> +bool intel_engine_workarounds_verify(struct intel_engine_cs *engine,
>> const char *from);
>>
>> #endif
>> diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
>> index 80396b3592f5..c009eb2af7fc 100644
>> --- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
>> @@ -327,10 +327,100 @@ static int live_reset_whitelist(void *arg)
>> return err;
>> }
>>
>> +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);
>
> Ok, ok.
>
>> +
>> + 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");
>> +
>> + set_bit(I915_RESET_BACKOFF, &error->flags);
>> + set_bit(I915_RESET_HANDOFF, &error->flags);
>> +
>> + ok = verify_gt_engine_wa(i915, "before reset");
>> + if (!ok)
>> + goto out;
>> +
>> + intel_runtime_pm_get(i915);
>
> Move RESET_HANDOFF here to avoid any repetition of earlier problems we
> had with the reset being applied too early.
There are not requests in this test but OK, I guess it makes it more
future proof if one day something gets added.
>
>> + i915_reset(i915, ~0, "live_workarounds");
>> + intel_runtime_pm_put(i915);
>> +
>> + ok = verify_gt_engine_wa(i915, "after reset");
>> +
>> +out:
>> + clear_bit(I915_RESET_BACKOFF, &error->flags);
>> +
>> + return ok ? 0 : -ESRCH;
>> +}
>> +
>> +static int
>> +live_engine_reset_gt_engine_workarounds(void *arg)
>> +{
>> + struct drm_i915_private *i915 = arg;
>> + struct i915_gpu_error *error = &i915->gpu_error;
>> + struct intel_engine_cs *engine;
>> + enum intel_engine_id id;
>> + bool ok;
>> +
>> + if (!intel_has_reset_engine(i915))
>> + return 0;
>
> May be easier to take global_reset_lock/unlock from
> selftests/intel_hanghceck.
I looked inside and did not find anything with this name so I don't know
what you mean?
>> +
>> + for_each_engine(engine, i915, id) {
>> + pr_info("Verifying after %s reset...\n", engine->name);
>> +
>> + set_bit(I915_RESET_BACKOFF, &error->flags);
>> + set_bit(I915_RESET_ENGINE + engine->id, &error->flags);
>> +
>> + ok = verify_gt_engine_wa(i915, "before reset");
>> + if (!ok)
>> + goto out;
>> +
>> + intel_runtime_pm_get(i915);
>> + i915_reset_engine(engine, "live_workarounds");
>> + intel_runtime_pm_put(i915);
>
> Once idle, and once with a spinner?
If idle then engine reset does nothing, so maybe I am again not
following you.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 5/8] drm/i915/selftests: Add tests for GT and engine workaround verification
2018-11-30 15:15 ` Tvrtko Ursulin
@ 2018-11-30 15:21 ` Chris Wilson
0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2018-11-30 15:21 UTC (permalink / raw)
To: Intel-gfx, Tvrtko Ursulin
Quoting Tvrtko Ursulin (2018-11-30 15:15:28)
>
> On 30/11/2018 11:43, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-11-30 11:31:58)
> >> 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.)
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >> +static int
> >> +live_engine_reset_gt_engine_workarounds(void *arg)
> >> +{
> >> + struct drm_i915_private *i915 = arg;
> >> + struct i915_gpu_error *error = &i915->gpu_error;
> >> + struct intel_engine_cs *engine;
> >> + enum intel_engine_id id;
> >> + bool ok;
> >> +
> >> + if (!intel_has_reset_engine(i915))
> >> + return 0;
> >
> > May be easier to take global_reset_lock/unlock from
> > selftests/intel_hanghceck.
>
> I looked inside and did not find anything with this name so I don't know
> what you mean?
This function
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);
}
}
in selftests/intel_hangcheck.c
> >> +
> >> + for_each_engine(engine, i915, id) {
> >> + pr_info("Verifying after %s reset...\n", engine->name);
> >> +
> >> + set_bit(I915_RESET_BACKOFF, &error->flags);
> >> + set_bit(I915_RESET_ENGINE + engine->id, &error->flags);
> >> +
> >> + ok = verify_gt_engine_wa(i915, "before reset");
> >> + if (!ok)
> >> + goto out;
> >> +
> >> + intel_runtime_pm_get(i915);
> >> + i915_reset_engine(engine, "live_workarounds");
> >> + intel_runtime_pm_put(i915);
> >
> > Once idle, and once with a spinner?
>
> If idle then engine reset does nothing, so maybe I am again not
> following you.
The engine is idle here, so no reset, right?
Also worth verifying in case we change the implementation and there is
variation in HW between resetting from different states.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 6/8] drm/i915: Move register white-listing to the common workaround framework
2018-11-30 11:31 [PATCH 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
` (4 preceding siblings ...)
2018-11-30 11:31 ` [PATCH 5/8] drm/i915/selftests: Add tests for GT and engine workaround verification Tvrtko Ursulin
@ 2018-11-30 11:31 ` Tvrtko Ursulin
2018-11-30 11:45 ` Chris Wilson
2018-11-30 11:32 ` [PATCH 7/8] drm/i915: Fuse per-context workaround handling with the common framework Tvrtko Ursulin
` (2 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 11:31 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.
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 a21a21855e6a..21964209d970 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -1032,29 +1032,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_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);
@@ -1066,7 +1057,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);
@@ -1074,12 +1065,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);
@@ -1087,7 +1078,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);
@@ -1095,18 +1086,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);
@@ -1115,22 +1106,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))
@@ -1148,37 +1138,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 8f664d8b9e08..0540d00953ec 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 c009eb2af7fc..4c0635dbc5fd 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -92,17 +92,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",
@@ -110,8 +116,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;
@@ -139,11 +144,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);
@@ -215,7 +220,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;
@@ -225,7 +229,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);
@@ -237,7 +241,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;
@@ -261,7 +265,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);
@@ -274,7 +278,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);
@@ -291,15 +295,11 @@ 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;
/* 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;
set_bit(I915_RESET_BACKOFF, &error->flags);
@@ -307,7 +307,7 @@ static int live_reset_whitelist(void *arg)
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] 24+ messages in thread* Re: [PATCH 6/8] drm/i915: Move register white-listing to the common workaround framework
2018-11-30 11:31 ` [PATCH 6/8] drm/i915: Move register white-listing to the common workaround framework Tvrtko Ursulin
@ 2018-11-30 11:45 ` Chris Wilson
2018-11-30 15:16 ` Tvrtko Ursulin
0 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2018-11-30 11:45 UTC (permalink / raw)
To: Intel-gfx, Tvrtko Ursulin
Quoting Tvrtko Ursulin (2018-11-30 11:31:59)
> -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_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
We only need this code for pre-merge testing of patches as the build
list is static.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 6/8] drm/i915: Move register white-listing to the common workaround framework
2018-11-30 11:45 ` Chris Wilson
@ 2018-11-30 15:16 ` Tvrtko Ursulin
0 siblings, 0 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 15:16 UTC (permalink / raw)
To: Chris Wilson, Intel-gfx
On 30/11/2018 11:45, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-11-30 11:31:59)
>> -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_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
>
> We only need this code for pre-merge testing of patches as the build
> list is static.
I interpret this as change to GEM_DEBUG_WARN_ON - makes sense.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 7/8] drm/i915: Fuse per-context workaround handling with the common framework
2018-11-30 11:31 [PATCH 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
` (5 preceding siblings ...)
2018-11-30 11:31 ` [PATCH 6/8] drm/i915: Move register white-listing to the common workaround framework Tvrtko Ursulin
@ 2018-11-30 11:32 ` Tvrtko Ursulin
2018-11-30 11:47 ` Chris Wilson
2018-11-30 11:32 ` [PATCH 8/8] drm/i915: Trim unused workaround list entries Tvrtko Ursulin
2018-11-30 12:06 ` ✗ Fi.CI.BAT: failure for Restore workarounds after engine reset and unify their handling Patchwork
8 siblings, 1 reply; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 11:32 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.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 11 +-
drivers/gpu/drm/i915/i915_drv.h | 16 +-
drivers/gpu/drm/i915/i915_gem.c | 1 +
drivers/gpu/drm/i915/i915_gem_context.c | 6 +-
drivers/gpu/drm/i915/intel_workarounds.c | 267 ++++++++++-------------
drivers/gpu/drm/i915/intel_workarounds.h | 3 +-
6 files changed, 128 insertions(+), 176 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 129b9a6f8309..c65cbb990d02 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3375,13 +3375,14 @@ 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;
+ const struct i915_wa_list *wal = &node_to_i915(m->private)->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..274e5c8b6c9a 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,7 @@ struct drm_i915_private {
int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
- struct i915_workarounds workarounds;
+ struct i915_wa_list wa_list;
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 1eff471d4366..0fcbc4ed6973 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5709,6 +5709,7 @@ 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->wa_list);
intel_wa_list_free(&dev_priv->gt_wa_list);
intel_cleanup_gt_powersave(dev_priv);
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_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index 21964209d970..de2bddbc64b4 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->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->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->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->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->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->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->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->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->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->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->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->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->i915->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
@@ -1042,7 +1009,7 @@ whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
if (GEM_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 0540d00953ec..bd8629ec4d3a 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] 24+ messages in thread* Re: [PATCH 7/8] drm/i915: Fuse per-context workaround handling with the common framework
2018-11-30 11:32 ` [PATCH 7/8] drm/i915: Fuse per-context workaround handling with the common framework Tvrtko Ursulin
@ 2018-11-30 11:47 ` Chris Wilson
2018-11-30 15:17 ` Tvrtko Ursulin
0 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2018-11-30 11:47 UTC (permalink / raw)
To: Intel-gfx, Tvrtko Ursulin
Quoting Tvrtko Ursulin (2018-11-30 11:32:00)
> 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.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> @@ -1652,7 +1638,7 @@ struct drm_i915_private {
>
> int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
>
> - struct i915_workarounds workarounds;
> + struct i915_wa_list wa_list;
> struct i915_wa_list gt_wa_list;
ctx_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] 24+ messages in thread* Re: [PATCH 7/8] drm/i915: Fuse per-context workaround handling with the common framework
2018-11-30 11:47 ` Chris Wilson
@ 2018-11-30 15:17 ` Tvrtko Ursulin
0 siblings, 0 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 15:17 UTC (permalink / raw)
To: Chris Wilson, Intel-gfx
On 30/11/2018 11:47, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-11-30 11:32:00)
>> 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.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> @@ -1652,7 +1638,7 @@ struct drm_i915_private {
>>
>> int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
>>
>> - struct i915_workarounds workarounds;
>> + struct i915_wa_list wa_list;
>> struct i915_wa_list gt_wa_list;
>
> ctx_wa_list ?
Can do, will do.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 8/8] drm/i915: Trim unused workaround list entries
2018-11-30 11:31 [PATCH 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
` (6 preceding siblings ...)
2018-11-30 11:32 ` [PATCH 7/8] drm/i915: Fuse per-context workaround handling with the common framework Tvrtko Ursulin
@ 2018-11-30 11:32 ` Tvrtko Ursulin
2018-11-30 11:49 ` Chris Wilson
2018-11-30 12:06 ` ✗ Fi.CI.BAT: failure for Restore workarounds after engine reset and unify their handling Patchwork
8 siblings, 1 reply; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 11:32 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.
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 de2bddbc64b4..abfe4b530c23 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 *wa =
+ kcalloc(wal->count, sizeof(*wa), GFP_KERNEL);
+
+ if (wa) {
+ memcpy(wa, wal->list, sizeof(*wa) * wal->count);
+ kfree(wal->list);
+ wal->list = wa;
+ 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] 24+ messages in thread* Re: [PATCH 8/8] drm/i915: Trim unused workaround list entries
2018-11-30 11:32 ` [PATCH 8/8] drm/i915: Trim unused workaround list entries Tvrtko Ursulin
@ 2018-11-30 11:49 ` Chris Wilson
2018-11-30 15:18 ` Tvrtko Ursulin
0 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2018-11-30 11:49 UTC (permalink / raw)
To: Intel-gfx, Tvrtko Ursulin
Quoting Tvrtko Ursulin (2018-11-30 11:32:01)
> 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.
>
> 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 de2bddbc64b4..abfe4b530c23 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 *wa =
> + kcalloc(wal->count, sizeof(*wa), GFP_KERNEL);
kcalloc followed by memcpy, what have you done with Tvrtko?
kmemdup ?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 8/8] drm/i915: Trim unused workaround list entries
2018-11-30 11:49 ` Chris Wilson
@ 2018-11-30 15:18 ` Tvrtko Ursulin
0 siblings, 0 replies; 24+ messages in thread
From: Tvrtko Ursulin @ 2018-11-30 15:18 UTC (permalink / raw)
To: Chris Wilson, Intel-gfx
On 30/11/2018 11:49, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-11-30 11:32:01)
>> 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.
>>
>> 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 de2bddbc64b4..abfe4b530c23 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 *wa =
>> + kcalloc(wal->count, sizeof(*wa), GFP_KERNEL);
>
> kcalloc followed by memcpy, what have you done with Tvrtko?
> kmemdup ?
Oh well, the richer the API the easier to forget. :)
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* ✗ Fi.CI.BAT: failure for Restore workarounds after engine reset and unify their handling
2018-11-30 11:31 [PATCH 0/8] Restore workarounds after engine reset and unify their handling Tvrtko Ursulin
` (7 preceding siblings ...)
2018-11-30 11:32 ` [PATCH 8/8] drm/i915: Trim unused workaround list entries Tvrtko Ursulin
@ 2018-11-30 12:06 ` Patchwork
8 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2018-11-30 12:06 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: Restore workarounds after engine reset and unify their handling
URL : https://patchwork.freedesktop.org/series/53313/
State : failure
== Summary ==
Applying: drm/i915: Record GT workarounds in a list
Applying: drm/i915: Introduce per-engine workarounds
Applying: drm/i915: Verify GT workaround state at runtime
Applying: drm/i915: Verify engine workaround state at runtime
Applying: drm/i915/selftests: Add tests for GT and engine workaround verification
Applying: drm/i915: Move register white-listing to the common workaround framework
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/selftests/intel_workarounds.c).
error: could not build fake ancestor
Patch failed at 0006 drm/i915: Move register white-listing to the common workaround framework
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread