* [Intel-gfx] [PATCH 1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
@ 2022-12-07 17:36 Andrzej Hajda
2022-12-09 9:37 ` Tvrtko Ursulin
0 siblings, 1 reply; 11+ messages in thread
From: Andrzej Hajda @ 2022-12-07 17:36 UTC (permalink / raw)
To: intel-gfx
Cc: Chris Wilson, Matthew Auld, Andrzej Hajda, Rodrigo Vivi,
Daniel Vetter, Nirmoy Das
In case of Gen12.50 video and compute engines, TLB_INV registers are
masked - to modify one bit, corresponding bit in upper half of the register
must be enabled, otherwise nothing happens.
Fixes: 77fa9efc16a9 ("drm/i915/xehp: Create separate reg definitions for new MCR registers")
Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
This patch is simple enhancement of
04aa64375f48 ("drm/i915: fix TLB invalidation for Gen12 video and compute engines")
for Gen12.5 which is added in dev branches.
---
drivers/gpu/drm/i915/gt/intel_gt.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index d114347c004ee5..f0224b607aa4a7 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -1120,9 +1120,15 @@ static void mmio_invalidate_full(struct intel_gt *gt)
continue;
if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
+ u32 val = BIT(engine->instance);
+
+ if (engine->class == VIDEO_DECODE_CLASS ||
+ engine->class == VIDEO_ENHANCEMENT_CLASS ||
+ engine->class == COMPUTE_CLASS)
+ val = _MASKED_BIT_ENABLE(val);
intel_gt_mcr_multicast_write_fw(gt,
xehp_regs[engine->class],
- BIT(engine->instance));
+ val);
} else {
rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
if (!i915_mmio_reg_offset(rb.reg))
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
2022-12-07 17:36 [Intel-gfx] [PATCH 1/2] " Andrzej Hajda
@ 2022-12-09 9:37 ` Tvrtko Ursulin
0 siblings, 0 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2022-12-09 9:37 UTC (permalink / raw)
To: Andrzej Hajda, intel-gfx
Cc: Chris Wilson, Daniel Vetter, Matthew Auld, Rodrigo Vivi,
Nirmoy Das
On 07/12/2022 17:36, Andrzej Hajda wrote:
> In case of Gen12.50 video and compute engines, TLB_INV registers are
> masked - to modify one bit, corresponding bit in upper half of the register
> must be enabled, otherwise nothing happens.
>
> Fixes: 77fa9efc16a9 ("drm/i915/xehp: Create separate reg definitions for new MCR registers")
Just a note that target wasn't strictly the only one to blame, but it is
a good target to ensure proper backporting.
> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> ---
> This patch is simple enhancement of
> 04aa64375f48 ("drm/i915: fix TLB invalidation for Gen12 video and compute engines")
> for Gen12.5 which is added in dev branches.
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index d114347c004ee5..f0224b607aa4a7 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -1120,9 +1120,15 @@ static void mmio_invalidate_full(struct intel_gt *gt)
> continue;
>
> if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> + u32 val = BIT(engine->instance);
> +
> + if (engine->class == VIDEO_DECODE_CLASS ||
> + engine->class == VIDEO_ENHANCEMENT_CLASS ||
> + engine->class == COMPUTE_CLASS)
> + val = _MASKED_BIT_ENABLE(val);
> intel_gt_mcr_multicast_write_fw(gt,
> xehp_regs[engine->class],
> - BIT(engine->instance));
> + val);
> } else {
> rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
> if (!i915_mmio_reg_offset(rb.reg))
Triple checked against bspec.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-gfx] [PATCH 1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
@ 2022-12-13 12:39 Tvrtko Ursulin
2022-12-13 12:39 ` [Intel-gfx] [PATCH 2/2] drm/i915: Consolidate TLB invalidation flow Tvrtko Ursulin
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2022-12-13 12:39 UTC (permalink / raw)
To: Intel-gfx, dri-devel; +Cc: Andrzej Hajda
From: Andrzej Hajda <andrzej.hajda@intel.com>
In case of Gen12.50 video and compute engines, TLB_INV registers are
masked - to modify one bit, corresponding bit in upper half of the register
must be enabled, otherwise nothing happens.
Fixes: 77fa9efc16a9 ("drm/i915/xehp: Create separate reg definitions for new MCR registers")
Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/gt/intel_gt.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 63f95c5f3614..7eeee5a7cb33 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -1100,9 +1100,15 @@ static void mmio_invalidate_full(struct intel_gt *gt)
continue;
if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
+ u32 val = BIT(engine->instance);
+
+ if (engine->class == VIDEO_DECODE_CLASS ||
+ engine->class == VIDEO_ENHANCEMENT_CLASS ||
+ engine->class == COMPUTE_CLASS)
+ val = _MASKED_BIT_ENABLE(val);
intel_gt_mcr_multicast_write_fw(gt,
xehp_regs[engine->class],
- BIT(engine->instance));
+ val);
} else {
rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
if (!i915_mmio_reg_offset(rb.reg))
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Intel-gfx] [PATCH 2/2] drm/i915: Consolidate TLB invalidation flow
2022-12-13 12:39 [Intel-gfx] [PATCH 1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines Tvrtko Ursulin
@ 2022-12-13 12:39 ` Tvrtko Ursulin
2022-12-13 14:52 ` Andrzej Hajda
2022-12-13 13:24 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines Patchwork
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2022-12-13 12:39 UTC (permalink / raw)
To: Intel-gfx, dri-devel; +Cc: Andrzej Hajda
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
As the logic for selecting the register and corresponsing values grew, the
code become a bit unsightly. Consolidate by storing the required values at
engine init time in the engine itself, and by doing so minimise the amount
of invariant platform and engine checks during each and every TLB
invalidation.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
---
I think this looks nicer, but I don't really want to merge any flavour of
consolidation until we get IGT coverage for the issue upstreamed.
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 81 ++++++++++++
drivers/gpu/drm/i915/gt/intel_engine_types.h | 16 +++
drivers/gpu/drm/i915/gt/intel_gt.c | 126 ++++---------------
3 files changed, 120 insertions(+), 103 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 99c4b866addd..97cdd9853e38 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1143,6 +1143,84 @@ static int init_status_page(struct intel_engine_cs *engine)
return ret;
}
+static bool intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine)
+{
+ static const union intel_engine_tlb_inv_reg gen8_regs[] = {
+ [RENDER_CLASS].reg = GEN8_RTCR,
+ [VIDEO_DECODE_CLASS].reg = GEN8_M1TCR, /* , GEN8_M2TCR */
+ [VIDEO_ENHANCEMENT_CLASS].reg = GEN8_VTCR,
+ [COPY_ENGINE_CLASS].reg = GEN8_BTCR,
+ };
+ static const union intel_engine_tlb_inv_reg gen12_regs[] = {
+ [RENDER_CLASS].reg = GEN12_GFX_TLB_INV_CR,
+ [VIDEO_DECODE_CLASS].reg = GEN12_VD_TLB_INV_CR,
+ [VIDEO_ENHANCEMENT_CLASS].reg = GEN12_VE_TLB_INV_CR,
+ [COPY_ENGINE_CLASS].reg = GEN12_BLT_TLB_INV_CR,
+ [COMPUTE_CLASS].reg = GEN12_COMPCTX_TLB_INV_CR,
+ };
+ static const union intel_engine_tlb_inv_reg xehp_regs[] = {
+ [RENDER_CLASS].mcr_reg = XEHP_GFX_TLB_INV_CR,
+ [VIDEO_DECODE_CLASS].mcr_reg = XEHP_VD_TLB_INV_CR,
+ [VIDEO_ENHANCEMENT_CLASS].mcr_reg = XEHP_VE_TLB_INV_CR,
+ [COPY_ENGINE_CLASS].mcr_reg = XEHP_BLT_TLB_INV_CR,
+ [COMPUTE_CLASS].mcr_reg = XEHP_COMPCTX_TLB_INV_CR,
+ };
+ struct drm_i915_private *i915 = engine->i915;
+ const union intel_engine_tlb_inv_reg *regs;
+ union intel_engine_tlb_inv_reg reg;
+ unsigned int class = engine->class;
+ unsigned int num = 0;
+ u32 val;
+
+ if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
+ regs = xehp_regs;
+ num = ARRAY_SIZE(xehp_regs);
+ } else if (GRAPHICS_VER(i915) == 12) {
+ regs = gen12_regs;
+ num = ARRAY_SIZE(gen12_regs);
+ } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) {
+ regs = gen8_regs;
+ num = ARRAY_SIZE(gen8_regs);
+ } else if (GRAPHICS_VER(i915) < 8) {
+ return false;
+ }
+
+ if (drm_WARN_ONCE(&i915->drm, !num,
+ "Platform does not implement TLB invalidation!"))
+ return false;
+
+ if (drm_WARN_ON_ONCE(&i915->drm,
+ class >= num ||
+ (!regs[class].reg.reg &&
+ !regs[class].mcr_reg.reg)))
+ return false;
+
+ reg = regs[class];
+
+ if (GRAPHICS_VER(i915) == 8 && class == VIDEO_DECODE_CLASS) {
+ reg.reg.reg += 4 * engine->instance; /* GEN8_M2TCR */
+ val = 0;
+ } else {
+ val = engine->instance;
+ }
+
+ val = BIT(val);
+
+ engine->tlb_inv.mcr = regs == xehp_regs;
+ engine->tlb_inv.reg = reg;
+ engine->tlb_inv.done = val;
+
+ if (GRAPHICS_VER(i915) >= 12 &&
+ (engine->class == VIDEO_DECODE_CLASS ||
+ engine->class == VIDEO_ENHANCEMENT_CLASS ||
+ engine->class == COMPUTE_CLASS))
+ engine->tlb_inv.request = _MASKED_BIT_ENABLE(val);
+ else
+ engine->tlb_inv.request = val;
+
+ return true;
+}
+
static int engine_setup_common(struct intel_engine_cs *engine)
{
int err;
@@ -1182,6 +1260,9 @@ static int engine_setup_common(struct intel_engine_cs *engine)
intel_engine_init_whitelist(engine);
intel_engine_init_ctx_wa(engine);
+ if (intel_engine_init_tlb_invalidation(engine))
+ engine->flags |= I915_ENGINE_HAS_TLB_INVALIDATION;
+
if (GRAPHICS_VER(engine->i915) >= 12)
engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 4fd54fb8810f..8df4a09a6022 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -341,6 +341,19 @@ struct intel_engine_guc_stats {
u64 start_gt_clk;
};
+union intel_engine_tlb_inv_reg {
+ i915_reg_t reg;
+ i915_mcr_reg_t mcr_reg;
+};
+
+struct intel_engine_tlb_inv
+{
+ bool mcr;
+ union intel_engine_tlb_inv_reg reg;
+ u32 request;
+ u32 done;
+};
+
struct intel_engine_cs {
struct drm_i915_private *i915;
struct intel_gt *gt;
@@ -372,6 +385,8 @@ struct intel_engine_cs {
u32 context_size;
u32 mmio_base;
+ struct intel_engine_tlb_inv tlb_inv;
+
/*
* Some w/a require forcewake to be held (which prevents RC6) while
* a particular engine is active. If so, we set fw_domain to which
@@ -556,6 +571,7 @@ struct intel_engine_cs {
#define I915_ENGINE_HAS_EU_PRIORITY BIT(10)
#define I915_ENGINE_FIRST_RENDER_COMPUTE BIT(11)
#define I915_ENGINE_USES_WA_HOLD_CCS_SWITCHOUT BIT(12)
+#define I915_ENGINE_HAS_TLB_INVALIDATION BIT(13)
unsigned int flags;
/*
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 7eeee5a7cb33..df7afff16fd6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -983,36 +983,6 @@ void intel_gt_info_print(const struct intel_gt_info *info,
intel_sseu_dump(&info->sseu, p);
}
-struct reg_and_bit {
- union {
- i915_reg_t reg;
- i915_mcr_reg_t mcr_reg;
- };
- u32 bit;
-};
-
-static struct reg_and_bit
-get_reg_and_bit(const struct intel_engine_cs *engine, const bool gen8,
- const i915_reg_t *regs, const unsigned int num)
-{
- const unsigned int class = engine->class;
- struct reg_and_bit rb = { };
-
- if (drm_WARN_ON_ONCE(&engine->i915->drm,
- class >= num || !regs[class].reg))
- return rb;
-
- rb.reg = regs[class];
- if (gen8 && class == VIDEO_DECODE_CLASS)
- rb.reg.reg += 4 * engine->instance; /* GEN8_M2TCR */
- else
- rb.bit = engine->instance;
-
- rb.bit = BIT(rb.bit);
-
- return rb;
-}
-
/*
* HW architecture suggest typical invalidation time at 40us,
* with pessimistic cases up to 100us and a recommendation to
@@ -1026,14 +996,20 @@ get_reg_and_bit(const struct intel_engine_cs *engine, const bool gen8,
* but are now considered MCR registers. Since they exist within a GAM range,
* the primary instance of the register rolls up the status from each unit.
*/
-static int wait_for_invalidate(struct intel_gt *gt, struct reg_and_bit rb)
+static int wait_for_invalidate(struct intel_engine_cs *engine)
{
- if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 50))
- return intel_gt_mcr_wait_for_reg(gt, rb.mcr_reg, rb.bit, 0,
+ if (engine->tlb_inv.mcr)
+ return intel_gt_mcr_wait_for_reg(engine->gt,
+ engine->tlb_inv.reg.mcr_reg,
+ engine->tlb_inv.done,
+ 0,
TLB_INVAL_TIMEOUT_US,
TLB_INVAL_TIMEOUT_MS);
else
- return __intel_wait_for_register_fw(gt->uncore, rb.reg, rb.bit, 0,
+ return __intel_wait_for_register_fw(engine->gt->uncore,
+ engine->tlb_inv.reg.reg,
+ engine->tlb_inv.done,
+ 0,
TLB_INVAL_TIMEOUT_US,
TLB_INVAL_TIMEOUT_MS,
NULL);
@@ -1041,50 +1017,14 @@ static int wait_for_invalidate(struct intel_gt *gt, struct reg_and_bit rb)
static void mmio_invalidate_full(struct intel_gt *gt)
{
- static const i915_reg_t gen8_regs[] = {
- [RENDER_CLASS] = GEN8_RTCR,
- [VIDEO_DECODE_CLASS] = GEN8_M1TCR, /* , GEN8_M2TCR */
- [VIDEO_ENHANCEMENT_CLASS] = GEN8_VTCR,
- [COPY_ENGINE_CLASS] = GEN8_BTCR,
- };
- static const i915_reg_t gen12_regs[] = {
- [RENDER_CLASS] = GEN12_GFX_TLB_INV_CR,
- [VIDEO_DECODE_CLASS] = GEN12_VD_TLB_INV_CR,
- [VIDEO_ENHANCEMENT_CLASS] = GEN12_VE_TLB_INV_CR,
- [COPY_ENGINE_CLASS] = GEN12_BLT_TLB_INV_CR,
- [COMPUTE_CLASS] = GEN12_COMPCTX_TLB_INV_CR,
- };
- static const i915_mcr_reg_t xehp_regs[] = {
- [RENDER_CLASS] = XEHP_GFX_TLB_INV_CR,
- [VIDEO_DECODE_CLASS] = XEHP_VD_TLB_INV_CR,
- [VIDEO_ENHANCEMENT_CLASS] = XEHP_VE_TLB_INV_CR,
- [COPY_ENGINE_CLASS] = XEHP_BLT_TLB_INV_CR,
- [COMPUTE_CLASS] = XEHP_COMPCTX_TLB_INV_CR,
- };
struct drm_i915_private *i915 = gt->i915;
struct intel_uncore *uncore = gt->uncore;
struct intel_engine_cs *engine;
intel_engine_mask_t awake, tmp;
enum intel_engine_id id;
- const i915_reg_t *regs;
- unsigned int num = 0;
unsigned long flags;
- if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
- regs = NULL;
- num = ARRAY_SIZE(xehp_regs);
- } else if (GRAPHICS_VER(i915) == 12) {
- regs = gen12_regs;
- num = ARRAY_SIZE(gen12_regs);
- } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) {
- regs = gen8_regs;
- num = ARRAY_SIZE(gen8_regs);
- } else if (GRAPHICS_VER(i915) < 8) {
- return;
- }
-
- if (drm_WARN_ONCE(&i915->drm, !num,
- "Platform does not implement TLB invalidation!"))
+ if (GRAPHICS_VER(i915) < 8)
return;
intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
@@ -1094,33 +1034,22 @@ static void mmio_invalidate_full(struct intel_gt *gt)
awake = 0;
for_each_engine(engine, gt, id) {
- struct reg_and_bit rb;
-
if (!intel_engine_pm_is_awake(engine))
continue;
- if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
- u32 val = BIT(engine->instance);
+ if (drm_WARN_ON_ONCE(&i915->drm,
+ !(engine->flags & I915_ENGINE_HAS_TLB_INVALIDATION)))
+ continue;
- if (engine->class == VIDEO_DECODE_CLASS ||
- engine->class == VIDEO_ENHANCEMENT_CLASS ||
- engine->class == COMPUTE_CLASS)
- val = _MASKED_BIT_ENABLE(val);
+ if (engine->tlb_inv.mcr)
intel_gt_mcr_multicast_write_fw(gt,
- xehp_regs[engine->class],
- val);
- } else {
- rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
- if (!i915_mmio_reg_offset(rb.reg))
- continue;
-
- if (GRAPHICS_VER(i915) == 12 && (engine->class == VIDEO_DECODE_CLASS ||
- engine->class == VIDEO_ENHANCEMENT_CLASS ||
- engine->class == COMPUTE_CLASS))
- rb.bit = _MASKED_BIT_ENABLE(rb.bit);
-
- intel_uncore_write_fw(uncore, rb.reg, rb.bit);
- }
+ engine->tlb_inv.reg.mcr_reg,
+ engine->tlb_inv.request);
+ else
+ intel_uncore_write_fw(uncore,
+ engine->tlb_inv.reg.reg,
+ engine->tlb_inv.request);
+
awake |= engine->mask;
}
@@ -1139,16 +1068,7 @@ static void mmio_invalidate_full(struct intel_gt *gt)
intel_gt_mcr_unlock(gt, flags);
for_each_engine_masked(engine, gt, awake, tmp) {
- struct reg_and_bit rb;
-
- if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
- rb.mcr_reg = xehp_regs[engine->class];
- rb.bit = BIT(engine->instance);
- } else {
- rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
- }
-
- if (wait_for_invalidate(gt, rb))
+ if (wait_for_invalidate(engine))
drm_err_ratelimited(>->i915->drm,
"%s TLB invalidation did not complete in %ums!\n",
engine->name, TLB_INVAL_TIMEOUT_MS);
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
2022-12-13 12:39 [Intel-gfx] [PATCH 1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines Tvrtko Ursulin
2022-12-13 12:39 ` [Intel-gfx] [PATCH 2/2] drm/i915: Consolidate TLB invalidation flow Tvrtko Ursulin
@ 2022-12-13 13:24 ` Patchwork
2022-12-13 13:24 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2022-12-13 13:24 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
URL : https://patchwork.freedesktop.org/series/111895/
State : warning
== Summary ==
Error: dim checkpatch failed
63a3c046d3e0 drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
a48a6c2b60fa drm/i915: Consolidate TLB invalidation flow
-:128: ERROR:OPEN_BRACE: open brace '{' following struct go on the same line
#128: FILE: drivers/gpu/drm/i915/gt/intel_engine_types.h:350:
+struct intel_engine_tlb_inv
+{
total: 1 errors, 0 warnings, 0 checks, 299 lines checked
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
2022-12-13 12:39 [Intel-gfx] [PATCH 1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines Tvrtko Ursulin
2022-12-13 12:39 ` [Intel-gfx] [PATCH 2/2] drm/i915: Consolidate TLB invalidation flow Tvrtko Ursulin
2022-12-13 13:24 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines Patchwork
@ 2022-12-13 13:24 ` Patchwork
2022-12-13 13:46 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-12-14 18:35 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2022-12-13 13:24 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
URL : https://patchwork.freedesktop.org/series/111895/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
2022-12-13 12:39 [Intel-gfx] [PATCH 1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines Tvrtko Ursulin
` (2 preceding siblings ...)
2022-12-13 13:24 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2022-12-13 13:46 ` Patchwork
2022-12-14 18:35 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2022-12-13 13:46 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 3979 bytes --]
== Series Details ==
Series: series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
URL : https://patchwork.freedesktop.org/series/111895/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12501 -> Patchwork_111895v1
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/index.html
Participating hosts (38 -> 19)
------------------------------
Additional (1): fi-skl-guc
Missing (20): fi-kbl-soraka bat-dg1-6 bat-dg1-5 bat-adlp-6 fi-skl-6600u fi-bsw-n3050 bat-dg2-8 bat-adlm-1 bat-dg2-9 fi-bwr-2160 bat-adln-1 bat-atsm-1 bat-jsl-3 bat-rplp-1 bat-dg2-11 fi-bsw-nick bat-dg1-7 bat-kbl-2 bat-adlp-9 bat-adlp-4
New tests
---------
New tests have been introduced between CI_DRM_12501 and Patchwork_111895v1:
### New IGT tests (5) ###
* igt@fbdev@eof:
- Statuses : 17 pass(s) 2 skip(s)
- Exec time: [0.0] s
* igt@fbdev@nullptr:
- Statuses : 17 pass(s) 2 skip(s)
- Exec time: [0.0] s
* igt@fbdev@read:
- Statuses : 17 pass(s) 2 skip(s)
- Exec time: [0.0] s
* igt@fbdev@write:
- Statuses : 17 pass(s) 2 skip(s)
- Exec time: [0.0] s
* igt@gem_exec_suspend@basic-s0:
- Statuses :
- Exec time: [None] s
Known issues
------------
Here are the changes found in Patchwork_111895v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_lmem_swapping@basic:
- fi-skl-guc: NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#4613]) +3 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/fi-skl-guc/igt@gem_lmem_swapping@basic.html
* igt@i915_suspend@basic-s3-without-i915:
- fi-rkl-11600: [PASS][2] -> [INCOMPLETE][3] ([i915#4817])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
* igt@kms_chamelium@hdmi-crc-fast:
- fi-skl-guc: NOTRUN -> [SKIP][4] ([fdo#109271] / [fdo#111827]) +8 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/fi-skl-guc/igt@kms_chamelium@hdmi-crc-fast.html
* igt@kms_setmode@basic-clone-single-crtc:
- fi-skl-guc: NOTRUN -> [SKIP][5] ([fdo#109271]) +9 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/fi-skl-guc/igt@kms_setmode@basic-clone-single-crtc.html
#### Possible fixes ####
* igt@gem_exec_gttfill@basic:
- fi-pnv-d510: [FAIL][6] ([i915#7229]) -> [PASS][7]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/fi-pnv-d510/igt@gem_exec_gttfill@basic.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/fi-pnv-d510/igt@gem_exec_gttfill@basic.html
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817
[i915#7229]: https://gitlab.freedesktop.org/drm/intel/issues/7229
Build changes
-------------
* Linux: CI_DRM_12501 -> Patchwork_111895v1
CI-20190529: 20190529
CI_DRM_12501: 1b38b5a419ab3d838b6ac95d22f1fe057fc8889d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7091: b8015f920c9f469d3733854263cb878373c1df51 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_111895v1: 1b38b5a419ab3d838b6ac95d22f1fe057fc8889d @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
7f444672992b drm/i915: Consolidate TLB invalidation flow
2efdc15febdd drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/index.html
[-- Attachment #2: Type: text/html, Size: 4990 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] drm/i915: Consolidate TLB invalidation flow
2022-12-13 12:39 ` [Intel-gfx] [PATCH 2/2] drm/i915: Consolidate TLB invalidation flow Tvrtko Ursulin
@ 2022-12-13 14:52 ` Andrzej Hajda
2022-12-13 15:22 ` Tvrtko Ursulin
0 siblings, 1 reply; 11+ messages in thread
From: Andrzej Hajda @ 2022-12-13 14:52 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx, dri-devel
On 13.12.2022 13:39, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> As the logic for selecting the register and corresponsing values grew, the
> code become a bit unsightly. Consolidate by storing the required values at
> engine init time in the engine itself, and by doing so minimise the amount
> of invariant platform and engine checks during each and every TLB
> invalidation.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> ---
> I think this looks nicer, but I don't really want to merge any flavour of
> consolidation until we get IGT coverage for the issue upstreamed.
Yep, the important is to have 1st patch merged.
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 81 ++++++++++++
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 16 +++
> drivers/gpu/drm/i915/gt/intel_gt.c | 126 ++++---------------
> 3 files changed, 120 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 99c4b866addd..97cdd9853e38 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -1143,6 +1143,84 @@ static int init_status_page(struct intel_engine_cs *engine)
> return ret;
> }
>
> +static bool intel_engine_init_tlb_invalidation(struct intel_engine_cs *engine)
> +{
> + static const union intel_engine_tlb_inv_reg gen8_regs[] = {
> + [RENDER_CLASS].reg = GEN8_RTCR,
> + [VIDEO_DECODE_CLASS].reg = GEN8_M1TCR, /* , GEN8_M2TCR */
> + [VIDEO_ENHANCEMENT_CLASS].reg = GEN8_VTCR,
> + [COPY_ENGINE_CLASS].reg = GEN8_BTCR,
> + };
> + static const union intel_engine_tlb_inv_reg gen12_regs[] = {
> + [RENDER_CLASS].reg = GEN12_GFX_TLB_INV_CR,
> + [VIDEO_DECODE_CLASS].reg = GEN12_VD_TLB_INV_CR,
> + [VIDEO_ENHANCEMENT_CLASS].reg = GEN12_VE_TLB_INV_CR,
> + [COPY_ENGINE_CLASS].reg = GEN12_BLT_TLB_INV_CR,
> + [COMPUTE_CLASS].reg = GEN12_COMPCTX_TLB_INV_CR,
> + };
> + static const union intel_engine_tlb_inv_reg xehp_regs[] = {
> + [RENDER_CLASS].mcr_reg = XEHP_GFX_TLB_INV_CR,
> + [VIDEO_DECODE_CLASS].mcr_reg = XEHP_VD_TLB_INV_CR,
> + [VIDEO_ENHANCEMENT_CLASS].mcr_reg = XEHP_VE_TLB_INV_CR,
> + [COPY_ENGINE_CLASS].mcr_reg = XEHP_BLT_TLB_INV_CR,
> + [COMPUTE_CLASS].mcr_reg = XEHP_COMPCTX_TLB_INV_CR,
> + };
> + struct drm_i915_private *i915 = engine->i915;
> + const union intel_engine_tlb_inv_reg *regs;
> + union intel_engine_tlb_inv_reg reg;
> + unsigned int class = engine->class;
> + unsigned int num = 0;
> + u32 val;
> +
> + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> + regs = xehp_regs;
> + num = ARRAY_SIZE(xehp_regs);
> + } else if (GRAPHICS_VER(i915) == 12) {
> + regs = gen12_regs;
> + num = ARRAY_SIZE(gen12_regs);
> + } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) {
"GRAPHICS_VER(i915) <= 11" seems redundant.
> + regs = gen8_regs;
> + num = ARRAY_SIZE(gen8_regs);
> + } else if (GRAPHICS_VER(i915) < 8) {
ditto
> + return false;
> + }
> +
> + if (drm_WARN_ONCE(&i915->drm, !num,
> + "Platform does not implement TLB invalidation!"))
> + return false;
It never happens.
> +
> + if (drm_WARN_ON_ONCE(&i915->drm,
> + class >= num ||
> + (!regs[class].reg.reg &&
> + !regs[class].mcr_reg.reg)))
> + return false;
> +
> + reg = regs[class];
> +
> + if (GRAPHICS_VER(i915) == 8 && class == VIDEO_DECODE_CLASS) {
> + reg.reg.reg += 4 * engine->instance; /* GEN8_M2TCR */
> + val = 0;
> + } else {
> + val = engine->instance;
> + }
> +
> + val = BIT(val);
> +
> + engine->tlb_inv.mcr = regs == xehp_regs;
> + engine->tlb_inv.reg = reg;
> + engine->tlb_inv.done = val;
> +
> + if (GRAPHICS_VER(i915) >= 12 &&
> + (engine->class == VIDEO_DECODE_CLASS ||
> + engine->class == VIDEO_ENHANCEMENT_CLASS ||
> + engine->class == COMPUTE_CLASS))
> + engine->tlb_inv.request = _MASKED_BIT_ENABLE(val);
> + else
> + engine->tlb_inv.request = val;
> +
> + return true;
> +}
> +
> static int engine_setup_common(struct intel_engine_cs *engine)
> {
> int err;
> @@ -1182,6 +1260,9 @@ static int engine_setup_common(struct intel_engine_cs *engine)
> intel_engine_init_whitelist(engine);
> intel_engine_init_ctx_wa(engine);
>
> + if (intel_engine_init_tlb_invalidation(engine))
> + engine->flags |= I915_ENGINE_HAS_TLB_INVALIDATION;
> +
> if (GRAPHICS_VER(engine->i915) >= 12)
> engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 4fd54fb8810f..8df4a09a6022 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -341,6 +341,19 @@ struct intel_engine_guc_stats {
> u64 start_gt_clk;
> };
>
> +union intel_engine_tlb_inv_reg {
> + i915_reg_t reg;
> + i915_mcr_reg_t mcr_reg;
> +};
> +
> +struct intel_engine_tlb_inv
> +{
> + bool mcr;
> + union intel_engine_tlb_inv_reg reg;
> + u32 request;
> + u32 done;
> +};
> +
> struct intel_engine_cs {
> struct drm_i915_private *i915;
> struct intel_gt *gt;
> @@ -372,6 +385,8 @@ struct intel_engine_cs {
> u32 context_size;
> u32 mmio_base;
>
> + struct intel_engine_tlb_inv tlb_inv;
> +
> /*
> * Some w/a require forcewake to be held (which prevents RC6) while
> * a particular engine is active. If so, we set fw_domain to which
> @@ -556,6 +571,7 @@ struct intel_engine_cs {
> #define I915_ENGINE_HAS_EU_PRIORITY BIT(10)
> #define I915_ENGINE_FIRST_RENDER_COMPUTE BIT(11)
> #define I915_ENGINE_USES_WA_HOLD_CCS_SWITCHOUT BIT(12)
> +#define I915_ENGINE_HAS_TLB_INVALIDATION BIT(13)
> unsigned int flags;
>
> /*
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 7eeee5a7cb33..df7afff16fd6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -983,36 +983,6 @@ void intel_gt_info_print(const struct intel_gt_info *info,
> intel_sseu_dump(&info->sseu, p);
> }
>
> -struct reg_and_bit {
> - union {
> - i915_reg_t reg;
> - i915_mcr_reg_t mcr_reg;
> - };
> - u32 bit;
> -};
> -
> -static struct reg_and_bit
> -get_reg_and_bit(const struct intel_engine_cs *engine, const bool gen8,
> - const i915_reg_t *regs, const unsigned int num)
> -{
> - const unsigned int class = engine->class;
> - struct reg_and_bit rb = { };
> -
> - if (drm_WARN_ON_ONCE(&engine->i915->drm,
> - class >= num || !regs[class].reg))
> - return rb;
> -
> - rb.reg = regs[class];
> - if (gen8 && class == VIDEO_DECODE_CLASS)
> - rb.reg.reg += 4 * engine->instance; /* GEN8_M2TCR */
> - else
> - rb.bit = engine->instance;
> -
> - rb.bit = BIT(rb.bit);
> -
> - return rb;
> -}
> -
> /*
> * HW architecture suggest typical invalidation time at 40us,
> * with pessimistic cases up to 100us and a recommendation to
> @@ -1026,14 +996,20 @@ get_reg_and_bit(const struct intel_engine_cs *engine, const bool gen8,
> * but are now considered MCR registers. Since they exist within a GAM range,
> * the primary instance of the register rolls up the status from each unit.
> */
> -static int wait_for_invalidate(struct intel_gt *gt, struct reg_and_bit rb)
> +static int wait_for_invalidate(struct intel_engine_cs *engine)
> {
> - if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 50))
> - return intel_gt_mcr_wait_for_reg(gt, rb.mcr_reg, rb.bit, 0,
> + if (engine->tlb_inv.mcr)
> + return intel_gt_mcr_wait_for_reg(engine->gt,
> + engine->tlb_inv.reg.mcr_reg,
> + engine->tlb_inv.done,
> + 0,
> TLB_INVAL_TIMEOUT_US,
> TLB_INVAL_TIMEOUT_MS);
> else
> - return __intel_wait_for_register_fw(gt->uncore, rb.reg, rb.bit, 0,
> + return __intel_wait_for_register_fw(engine->gt->uncore,
> + engine->tlb_inv.reg.reg,
> + engine->tlb_inv.done,
> + 0,
> TLB_INVAL_TIMEOUT_US,
> TLB_INVAL_TIMEOUT_MS,
> NULL);
> @@ -1041,50 +1017,14 @@ static int wait_for_invalidate(struct intel_gt *gt, struct reg_and_bit rb)
>
> static void mmio_invalidate_full(struct intel_gt *gt)
> {
> - static const i915_reg_t gen8_regs[] = {
> - [RENDER_CLASS] = GEN8_RTCR,
> - [VIDEO_DECODE_CLASS] = GEN8_M1TCR, /* , GEN8_M2TCR */
> - [VIDEO_ENHANCEMENT_CLASS] = GEN8_VTCR,
> - [COPY_ENGINE_CLASS] = GEN8_BTCR,
> - };
> - static const i915_reg_t gen12_regs[] = {
> - [RENDER_CLASS] = GEN12_GFX_TLB_INV_CR,
> - [VIDEO_DECODE_CLASS] = GEN12_VD_TLB_INV_CR,
> - [VIDEO_ENHANCEMENT_CLASS] = GEN12_VE_TLB_INV_CR,
> - [COPY_ENGINE_CLASS] = GEN12_BLT_TLB_INV_CR,
> - [COMPUTE_CLASS] = GEN12_COMPCTX_TLB_INV_CR,
> - };
> - static const i915_mcr_reg_t xehp_regs[] = {
> - [RENDER_CLASS] = XEHP_GFX_TLB_INV_CR,
> - [VIDEO_DECODE_CLASS] = XEHP_VD_TLB_INV_CR,
> - [VIDEO_ENHANCEMENT_CLASS] = XEHP_VE_TLB_INV_CR,
> - [COPY_ENGINE_CLASS] = XEHP_BLT_TLB_INV_CR,
> - [COMPUTE_CLASS] = XEHP_COMPCTX_TLB_INV_CR,
> - };
> struct drm_i915_private *i915 = gt->i915;
> struct intel_uncore *uncore = gt->uncore;
> struct intel_engine_cs *engine;
> intel_engine_mask_t awake, tmp;
> enum intel_engine_id id;
> - const i915_reg_t *regs;
> - unsigned int num = 0;
> unsigned long flags;
>
> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> - regs = NULL;
> - num = ARRAY_SIZE(xehp_regs);
> - } else if (GRAPHICS_VER(i915) == 12) {
> - regs = gen12_regs;
> - num = ARRAY_SIZE(gen12_regs);
> - } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) {
> - regs = gen8_regs;
> - num = ARRAY_SIZE(gen8_regs);
> - } else if (GRAPHICS_VER(i915) < 8) {
> - return;
> - }
> -
> - if (drm_WARN_ONCE(&i915->drm, !num,
> - "Platform does not implement TLB invalidation!"))
> + if (GRAPHICS_VER(i915) < 8)
> return;
>
> intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
> @@ -1094,33 +1034,22 @@ static void mmio_invalidate_full(struct intel_gt *gt)
>
> awake = 0;
> for_each_engine(engine, gt, id) {
> - struct reg_and_bit rb;
> -
> if (!intel_engine_pm_is_awake(engine))
> continue;
>
> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> - u32 val = BIT(engine->instance);
> + if (drm_WARN_ON_ONCE(&i915->drm,
> + !(engine->flags & I915_ENGINE_HAS_TLB_INVALIDATION)))
> + continue;
Hmm, can this flag change dynamically? If not why not put it in
initialization phase.
>
> - if (engine->class == VIDEO_DECODE_CLASS ||
> - engine->class == VIDEO_ENHANCEMENT_CLASS ||
> - engine->class == COMPUTE_CLASS)
> - val = _MASKED_BIT_ENABLE(val);
> + if (engine->tlb_inv.mcr)
> intel_gt_mcr_multicast_write_fw(gt,
> - xehp_regs[engine->class],
> - val);
> - } else {
> - rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
> - if (!i915_mmio_reg_offset(rb.reg))
> - continue;
> -
> - if (GRAPHICS_VER(i915) == 12 && (engine->class == VIDEO_DECODE_CLASS ||
> - engine->class == VIDEO_ENHANCEMENT_CLASS ||
> - engine->class == COMPUTE_CLASS))
> - rb.bit = _MASKED_BIT_ENABLE(rb.bit);
> -
> - intel_uncore_write_fw(uncore, rb.reg, rb.bit);
> - }
> + engine->tlb_inv.reg.mcr_reg,
> + engine->tlb_inv.request);
> + else
> + intel_uncore_write_fw(uncore,
> + engine->tlb_inv.reg.reg,
> + engine->tlb_inv.request);
> +
> awake |= engine->mask;
> }
>
> @@ -1139,16 +1068,7 @@ static void mmio_invalidate_full(struct intel_gt *gt)
> intel_gt_mcr_unlock(gt, flags);
>
> for_each_engine_masked(engine, gt, awake, tmp) {
> - struct reg_and_bit rb;
> -
> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
> - rb.mcr_reg = xehp_regs[engine->class];
> - rb.bit = BIT(engine->instance);
> - } else {
> - rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
> - }
> -
> - if (wait_for_invalidate(gt, rb))
> + if (wait_for_invalidate(engine))
> drm_err_ratelimited(>->i915->drm,
> "%s TLB invalidation did not complete in %ums!\n",
> engine->name, TLB_INVAL_TIMEOUT_MS);
With minor comments addressed:
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Regards
Andrzej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] drm/i915: Consolidate TLB invalidation flow
2022-12-13 14:52 ` Andrzej Hajda
@ 2022-12-13 15:22 ` Tvrtko Ursulin
2022-12-14 9:02 ` Andrzej Hajda
0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2022-12-13 15:22 UTC (permalink / raw)
To: Andrzej Hajda, Intel-gfx, dri-devel
On 13/12/2022 14:52, Andrzej Hajda wrote:
> On 13.12.2022 13:39, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> As the logic for selecting the register and corresponsing values grew,
>> the
>> code become a bit unsightly. Consolidate by storing the required
>> values at
>> engine init time in the engine itself, and by doing so minimise the
>> amount
>> of invariant platform and engine checks during each and every TLB
>> invalidation.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>> ---
>> I think this looks nicer, but I don't really want to merge any flavour of
>> consolidation until we get IGT coverage for the issue upstreamed.
>
>
> Yep, the important is to have 1st patch merged.
Agreed.
1)
Can you send it standalone so it gets CI results and can get merged?
2)
Could you check internally where did gem_exec_tlb get stuck?
>> ---
>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 81 ++++++++++++
>> drivers/gpu/drm/i915/gt/intel_engine_types.h | 16 +++
>> drivers/gpu/drm/i915/gt/intel_gt.c | 126 ++++---------------
>> 3 files changed, 120 insertions(+), 103 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> index 99c4b866addd..97cdd9853e38 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> @@ -1143,6 +1143,84 @@ static int init_status_page(struct
>> intel_engine_cs *engine)
>> return ret;
>> }
>> +static bool intel_engine_init_tlb_invalidation(struct intel_engine_cs
>> *engine)
>> +{
>> + static const union intel_engine_tlb_inv_reg gen8_regs[] = {
>> + [RENDER_CLASS].reg = GEN8_RTCR,
>> + [VIDEO_DECODE_CLASS].reg = GEN8_M1TCR, /* , GEN8_M2TCR */
>> + [VIDEO_ENHANCEMENT_CLASS].reg = GEN8_VTCR,
>> + [COPY_ENGINE_CLASS].reg = GEN8_BTCR,
>> + };
>> + static const union intel_engine_tlb_inv_reg gen12_regs[] = {
>> + [RENDER_CLASS].reg = GEN12_GFX_TLB_INV_CR,
>> + [VIDEO_DECODE_CLASS].reg = GEN12_VD_TLB_INV_CR,
>> + [VIDEO_ENHANCEMENT_CLASS].reg = GEN12_VE_TLB_INV_CR,
>> + [COPY_ENGINE_CLASS].reg = GEN12_BLT_TLB_INV_CR,
>> + [COMPUTE_CLASS].reg = GEN12_COMPCTX_TLB_INV_CR,
>> + };
>> + static const union intel_engine_tlb_inv_reg xehp_regs[] = {
>> + [RENDER_CLASS].mcr_reg = XEHP_GFX_TLB_INV_CR,
>> + [VIDEO_DECODE_CLASS].mcr_reg = XEHP_VD_TLB_INV_CR,
>> + [VIDEO_ENHANCEMENT_CLASS].mcr_reg = XEHP_VE_TLB_INV_CR,
>> + [COPY_ENGINE_CLASS].mcr_reg = XEHP_BLT_TLB_INV_CR,
>> + [COMPUTE_CLASS].mcr_reg = XEHP_COMPCTX_TLB_INV_CR,
>> + };
>> + struct drm_i915_private *i915 = engine->i915;
>> + const union intel_engine_tlb_inv_reg *regs;
>> + union intel_engine_tlb_inv_reg reg;
>> + unsigned int class = engine->class;
>> + unsigned int num = 0;
>> + u32 val;
>> +
>> + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
>> + regs = xehp_regs;
>> + num = ARRAY_SIZE(xehp_regs);
>> + } else if (GRAPHICS_VER(i915) == 12) {
>> + regs = gen12_regs;
>> + num = ARRAY_SIZE(gen12_regs);
>> + } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) {
>
> "GRAPHICS_VER(i915) <= 11" seems redundant.
Code movement... Fixes: "patch which added >= 12,50" to clean just that
part up. Before it was deliberately made to hit the below warn on so
that any new platform added requires a human checking if the registers
are still same and compatible. (Instead of assuming future platforms
remain compatible - which is IMO too big risk to take.)
>
>> + regs = gen8_regs;
>> + num = ARRAY_SIZE(gen8_regs);
>> + } else if (GRAPHICS_VER(i915) < 8) {
>
> ditto
How? We want to do nothing before gen 8.
>> + return false;
>> + }
>> +
>> + if (drm_WARN_ONCE(&i915->drm, !num,
>> + "Platform does not implement TLB invalidation!"))
>> + return false;
>
> It never happens.
As above a) it's just code movement and b) it would be safer if it could
happen.
>> +
>> + if (drm_WARN_ON_ONCE(&i915->drm,
>> + class >= num ||
>> + (!regs[class].reg.reg &&
>> + !regs[class].mcr_reg.reg)))
>> + return false;
>> +
>> + reg = regs[class];
>> +
>> + if (GRAPHICS_VER(i915) == 8 && class == VIDEO_DECODE_CLASS) {
>> + reg.reg.reg += 4 * engine->instance; /* GEN8_M2TCR */
>> + val = 0;
>> + } else {
>> + val = engine->instance;
>> + }
>> +
>> + val = BIT(val);
>> +
>> + engine->tlb_inv.mcr = regs == xehp_regs;
>> + engine->tlb_inv.reg = reg;
>> + engine->tlb_inv.done = val;
>> +
>> + if (GRAPHICS_VER(i915) >= 12 &&
>> + (engine->class == VIDEO_DECODE_CLASS ||
>> + engine->class == VIDEO_ENHANCEMENT_CLASS ||
>> + engine->class == COMPUTE_CLASS))
>> + engine->tlb_inv.request = _MASKED_BIT_ENABLE(val);
>> + else
>> + engine->tlb_inv.request = val;
>> +
>> + return true;
>> +}
>> +
>> static int engine_setup_common(struct intel_engine_cs *engine)
>> {
>> int err;
>> @@ -1182,6 +1260,9 @@ static int engine_setup_common(struct
>> intel_engine_cs *engine)
>> intel_engine_init_whitelist(engine);
>> intel_engine_init_ctx_wa(engine);
>> + if (intel_engine_init_tlb_invalidation(engine))
>> + engine->flags |= I915_ENGINE_HAS_TLB_INVALIDATION;
>> +
>> if (GRAPHICS_VER(engine->i915) >= 12)
>> engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> index 4fd54fb8810f..8df4a09a6022 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>> @@ -341,6 +341,19 @@ struct intel_engine_guc_stats {
>> u64 start_gt_clk;
>> };
>> +union intel_engine_tlb_inv_reg {
>> + i915_reg_t reg;
>> + i915_mcr_reg_t mcr_reg;
>> +};
>> +
>> +struct intel_engine_tlb_inv
>> +{
>> + bool mcr;
>> + union intel_engine_tlb_inv_reg reg;
>> + u32 request;
>> + u32 done;
>> +};
>> +
>> struct intel_engine_cs {
>> struct drm_i915_private *i915;
>> struct intel_gt *gt;
>> @@ -372,6 +385,8 @@ struct intel_engine_cs {
>> u32 context_size;
>> u32 mmio_base;
>> + struct intel_engine_tlb_inv tlb_inv;
>> +
>> /*
>> * Some w/a require forcewake to be held (which prevents RC6) while
>> * a particular engine is active. If so, we set fw_domain to which
>> @@ -556,6 +571,7 @@ struct intel_engine_cs {
>> #define I915_ENGINE_HAS_EU_PRIORITY BIT(10)
>> #define I915_ENGINE_FIRST_RENDER_COMPUTE BIT(11)
>> #define I915_ENGINE_USES_WA_HOLD_CCS_SWITCHOUT BIT(12)
>> +#define I915_ENGINE_HAS_TLB_INVALIDATION BIT(13)
>> unsigned int flags;
>> /*
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
>> b/drivers/gpu/drm/i915/gt/intel_gt.c
>> index 7eeee5a7cb33..df7afff16fd6 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>> @@ -983,36 +983,6 @@ void intel_gt_info_print(const struct
>> intel_gt_info *info,
>> intel_sseu_dump(&info->sseu, p);
>> }
>> -struct reg_and_bit {
>> - union {
>> - i915_reg_t reg;
>> - i915_mcr_reg_t mcr_reg;
>> - };
>> - u32 bit;
>> -};
>> -
>> -static struct reg_and_bit
>> -get_reg_and_bit(const struct intel_engine_cs *engine, const bool gen8,
>> - const i915_reg_t *regs, const unsigned int num)
>> -{
>> - const unsigned int class = engine->class;
>> - struct reg_and_bit rb = { };
>> -
>> - if (drm_WARN_ON_ONCE(&engine->i915->drm,
>> - class >= num || !regs[class].reg))
>> - return rb;
>> -
>> - rb.reg = regs[class];
>> - if (gen8 && class == VIDEO_DECODE_CLASS)
>> - rb.reg.reg += 4 * engine->instance; /* GEN8_M2TCR */
>> - else
>> - rb.bit = engine->instance;
>> -
>> - rb.bit = BIT(rb.bit);
>> -
>> - return rb;
>> -}
>> -
>> /*
>> * HW architecture suggest typical invalidation time at 40us,
>> * with pessimistic cases up to 100us and a recommendation to
>> @@ -1026,14 +996,20 @@ get_reg_and_bit(const struct intel_engine_cs
>> *engine, const bool gen8,
>> * but are now considered MCR registers. Since they exist within a
>> GAM range,
>> * the primary instance of the register rolls up the status from
>> each unit.
>> */
>> -static int wait_for_invalidate(struct intel_gt *gt, struct
>> reg_and_bit rb)
>> +static int wait_for_invalidate(struct intel_engine_cs *engine)
>> {
>> - if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 50))
>> - return intel_gt_mcr_wait_for_reg(gt, rb.mcr_reg, rb.bit, 0,
>> + if (engine->tlb_inv.mcr)
>> + return intel_gt_mcr_wait_for_reg(engine->gt,
>> + engine->tlb_inv.reg.mcr_reg,
>> + engine->tlb_inv.done,
>> + 0,
>> TLB_INVAL_TIMEOUT_US,
>> TLB_INVAL_TIMEOUT_MS);
>> else
>> - return __intel_wait_for_register_fw(gt->uncore, rb.reg,
>> rb.bit, 0,
>> + return __intel_wait_for_register_fw(engine->gt->uncore,
>> + engine->tlb_inv.reg.reg,
>> + engine->tlb_inv.done,
>> + 0,
>> TLB_INVAL_TIMEOUT_US,
>> TLB_INVAL_TIMEOUT_MS,
>> NULL);
>> @@ -1041,50 +1017,14 @@ static int wait_for_invalidate(struct intel_gt
>> *gt, struct reg_and_bit rb)
>> static void mmio_invalidate_full(struct intel_gt *gt)
>> {
>> - static const i915_reg_t gen8_regs[] = {
>> - [RENDER_CLASS] = GEN8_RTCR,
>> - [VIDEO_DECODE_CLASS] = GEN8_M1TCR, /* , GEN8_M2TCR */
>> - [VIDEO_ENHANCEMENT_CLASS] = GEN8_VTCR,
>> - [COPY_ENGINE_CLASS] = GEN8_BTCR,
>> - };
>> - static const i915_reg_t gen12_regs[] = {
>> - [RENDER_CLASS] = GEN12_GFX_TLB_INV_CR,
>> - [VIDEO_DECODE_CLASS] = GEN12_VD_TLB_INV_CR,
>> - [VIDEO_ENHANCEMENT_CLASS] = GEN12_VE_TLB_INV_CR,
>> - [COPY_ENGINE_CLASS] = GEN12_BLT_TLB_INV_CR,
>> - [COMPUTE_CLASS] = GEN12_COMPCTX_TLB_INV_CR,
>> - };
>> - static const i915_mcr_reg_t xehp_regs[] = {
>> - [RENDER_CLASS] = XEHP_GFX_TLB_INV_CR,
>> - [VIDEO_DECODE_CLASS] = XEHP_VD_TLB_INV_CR,
>> - [VIDEO_ENHANCEMENT_CLASS] = XEHP_VE_TLB_INV_CR,
>> - [COPY_ENGINE_CLASS] = XEHP_BLT_TLB_INV_CR,
>> - [COMPUTE_CLASS] = XEHP_COMPCTX_TLB_INV_CR,
>> - };
>> struct drm_i915_private *i915 = gt->i915;
>> struct intel_uncore *uncore = gt->uncore;
>> struct intel_engine_cs *engine;
>> intel_engine_mask_t awake, tmp;
>> enum intel_engine_id id;
>> - const i915_reg_t *regs;
>> - unsigned int num = 0;
>> unsigned long flags;
>> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
>> - regs = NULL;
>> - num = ARRAY_SIZE(xehp_regs);
>> - } else if (GRAPHICS_VER(i915) == 12) {
>> - regs = gen12_regs;
>> - num = ARRAY_SIZE(gen12_regs);
>> - } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) {
>> - regs = gen8_regs;
>> - num = ARRAY_SIZE(gen8_regs);
>> - } else if (GRAPHICS_VER(i915) < 8) {
>> - return;
>> - }
>> -
>> - if (drm_WARN_ONCE(&i915->drm, !num,
>> - "Platform does not implement TLB invalidation!"))
>> + if (GRAPHICS_VER(i915) < 8)
>> return;
>> intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
>> @@ -1094,33 +1034,22 @@ static void mmio_invalidate_full(struct
>> intel_gt *gt)
>> awake = 0;
>> for_each_engine(engine, gt, id) {
>> - struct reg_and_bit rb;
>> -
>> if (!intel_engine_pm_is_awake(engine))
>> continue;
>> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
>> - u32 val = BIT(engine->instance);
>> + if (drm_WARN_ON_ONCE(&i915->drm,
>> + !(engine->flags &
>> I915_ENGINE_HAS_TLB_INVALIDATION)))
>> + continue;
>
> Hmm, can this flag change dynamically? If not why not put it in
> initialization phase.
Could do. In that case I couldn't have any asserts that engine->tlb_inv
is valid but perhaps that is okay. Could just fail engine probe from
there if register table is incomplete. Sounds better indeed, I'll change it.
>
>> - if (engine->class == VIDEO_DECODE_CLASS ||
>> - engine->class == VIDEO_ENHANCEMENT_CLASS ||
>> - engine->class == COMPUTE_CLASS)
>> - val = _MASKED_BIT_ENABLE(val);
>> + if (engine->tlb_inv.mcr)
>> intel_gt_mcr_multicast_write_fw(gt,
>> - xehp_regs[engine->class],
>> - val);
>> - } else {
>> - rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
>> - if (!i915_mmio_reg_offset(rb.reg))
>> - continue;
>> -
>> - if (GRAPHICS_VER(i915) == 12 && (engine->class ==
>> VIDEO_DECODE_CLASS ||
>> - engine->class == VIDEO_ENHANCEMENT_CLASS ||
>> - engine->class == COMPUTE_CLASS))
>> - rb.bit = _MASKED_BIT_ENABLE(rb.bit);
>> -
>> - intel_uncore_write_fw(uncore, rb.reg, rb.bit);
>> - }
>> + engine->tlb_inv.reg.mcr_reg,
>> + engine->tlb_inv.request);
>> + else
>> + intel_uncore_write_fw(uncore,
>> + engine->tlb_inv.reg.reg,
>> + engine->tlb_inv.request);
>> +
>> awake |= engine->mask;
>> }
>> @@ -1139,16 +1068,7 @@ static void mmio_invalidate_full(struct
>> intel_gt *gt)
>> intel_gt_mcr_unlock(gt, flags);
>> for_each_engine_masked(engine, gt, awake, tmp) {
>> - struct reg_and_bit rb;
>> -
>> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
>> - rb.mcr_reg = xehp_regs[engine->class];
>> - rb.bit = BIT(engine->instance);
>> - } else {
>> - rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
>> - }
>> -
>> - if (wait_for_invalidate(gt, rb))
>> + if (wait_for_invalidate(engine))
>> drm_err_ratelimited(>->i915->drm,
>> "%s TLB invalidation did not complete in
>> %ums!\n",
>> engine->name, TLB_INVAL_TIMEOUT_MS);
>
> With minor comments addressed:
>
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
I'll send v2 at some point, please stand by.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] drm/i915: Consolidate TLB invalidation flow
2022-12-13 15:22 ` Tvrtko Ursulin
@ 2022-12-14 9:02 ` Andrzej Hajda
0 siblings, 0 replies; 11+ messages in thread
From: Andrzej Hajda @ 2022-12-14 9:02 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx, dri-devel
On 13.12.2022 16:22, Tvrtko Ursulin wrote:
>
> On 13/12/2022 14:52, Andrzej Hajda wrote:
>> On 13.12.2022 13:39, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> As the logic for selecting the register and corresponsing values
>>> grew, the
>>> code become a bit unsightly. Consolidate by storing the required
>>> values at
>>> engine init time in the engine itself, and by doing so minimise the
>>> amount
>>> of invariant platform and engine checks during each and every TLB
>>> invalidation.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>>> ---
>>> I think this looks nicer, but I don't really want to merge any
>>> flavour of
>>> consolidation until we get IGT coverage for the issue upstreamed.
>>
>>
>> Yep, the important is to have 1st patch merged.
>
> Agreed.
>
> 1)
> Can you send it standalone so it gets CI results and can get merged?
Done.
>
> 2)
> Could you check internally where did gem_exec_tlb get stuck? >
>>> ---
>>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 81 ++++++++++++
>>> drivers/gpu/drm/i915/gt/intel_engine_types.h | 16 +++
>>> drivers/gpu/drm/i915/gt/intel_gt.c | 126 ++++---------------
>>> 3 files changed, 120 insertions(+), 103 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> index 99c4b866addd..97cdd9853e38 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> @@ -1143,6 +1143,84 @@ static int init_status_page(struct
>>> intel_engine_cs *engine)
>>> return ret;
>>> }
>>> +static bool intel_engine_init_tlb_invalidation(struct
>>> intel_engine_cs *engine)
>>> +{
>>> + static const union intel_engine_tlb_inv_reg gen8_regs[] = {
>>> + [RENDER_CLASS].reg = GEN8_RTCR,
>>> + [VIDEO_DECODE_CLASS].reg = GEN8_M1TCR, /* , GEN8_M2TCR */
>>> + [VIDEO_ENHANCEMENT_CLASS].reg = GEN8_VTCR,
>>> + [COPY_ENGINE_CLASS].reg = GEN8_BTCR,
>>> + };
>>> + static const union intel_engine_tlb_inv_reg gen12_regs[] = {
>>> + [RENDER_CLASS].reg = GEN12_GFX_TLB_INV_CR,
>>> + [VIDEO_DECODE_CLASS].reg = GEN12_VD_TLB_INV_CR,
>>> + [VIDEO_ENHANCEMENT_CLASS].reg = GEN12_VE_TLB_INV_CR,
>>> + [COPY_ENGINE_CLASS].reg = GEN12_BLT_TLB_INV_CR,
>>> + [COMPUTE_CLASS].reg = GEN12_COMPCTX_TLB_INV_CR,
>>> + };
>>> + static const union intel_engine_tlb_inv_reg xehp_regs[] = {
>>> + [RENDER_CLASS].mcr_reg = XEHP_GFX_TLB_INV_CR,
>>> + [VIDEO_DECODE_CLASS].mcr_reg = XEHP_VD_TLB_INV_CR,
>>> + [VIDEO_ENHANCEMENT_CLASS].mcr_reg = XEHP_VE_TLB_INV_CR,
>>> + [COPY_ENGINE_CLASS].mcr_reg = XEHP_BLT_TLB_INV_CR,
>>> + [COMPUTE_CLASS].mcr_reg = XEHP_COMPCTX_TLB_INV_CR,
>>> + };
>>> + struct drm_i915_private *i915 = engine->i915;
>>> + const union intel_engine_tlb_inv_reg *regs;
>>> + union intel_engine_tlb_inv_reg reg;
>>> + unsigned int class = engine->class;
>>> + unsigned int num = 0;
>>> + u32 val;
>>> +
>>> + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
>>> + regs = xehp_regs;
>>> + num = ARRAY_SIZE(xehp_regs);
>>> + } else if (GRAPHICS_VER(i915) == 12) {
>>> + regs = gen12_regs;
>>> + num = ARRAY_SIZE(gen12_regs);
>>> + } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) {
>>
>> "GRAPHICS_VER(i915) <= 11" seems redundant.
>
> Code movement... Fixes: "patch which added >= 12,50" to clean just that
> part up. Before it was deliberately made to hit the below warn on so
> that any new platform added requires a human checking if the registers
> are still same and compatible. (Instead of assuming future platforms
> remain compatible - which is IMO too big risk to take.)
>
>>
>>> + regs = gen8_regs;
>>> + num = ARRAY_SIZE(gen8_regs);
>>> + } else if (GRAPHICS_VER(i915) < 8) {
>>
>> ditto
>
> How? We want to do nothing before gen 8.
} else {
return false;
}
?
As I understand there are three 'rules' we want to follow:
1. start from the newest hw, end at the oldest.
2. warn for not yet supported platforms, lets assume it is ver >= 13.
3. cover all cases
Why not use >= consistently, till exhaustion:
if (ver >= 13) {
warn
return false;
} else if (ver >= 12.50) {
...
} else if (ver >= 12) {
...
} else if (ver >= 8) {
...
} else {
return false;
}
It looks cleaner, ordered, no redundant checks, no doubts, hopefuly
future-proof.
Regards
Andrzej
>
>>> + return false;
>>> + }
>>> +
>>> + if (drm_WARN_ONCE(&i915->drm, !num,
>>> + "Platform does not implement TLB invalidation!"))
>>> + return false;
>>
>> It never happens.
>
> As above a) it's just code movement and b) it would be safer if it could
> happen.
>
>>> +
>>> + if (drm_WARN_ON_ONCE(&i915->drm,
>>> + class >= num ||
>>> + (!regs[class].reg.reg &&
>>> + !regs[class].mcr_reg.reg)))
>>> + return false;
>>> +
>>> + reg = regs[class];
>>> +
>>> + if (GRAPHICS_VER(i915) == 8 && class == VIDEO_DECODE_CLASS) {
>>> + reg.reg.reg += 4 * engine->instance; /* GEN8_M2TCR */
>>> + val = 0;
>>> + } else {
>>> + val = engine->instance;
>>> + }
>>> +
>>> + val = BIT(val);
>>> +
>>> + engine->tlb_inv.mcr = regs == xehp_regs;
>>> + engine->tlb_inv.reg = reg;
>>> + engine->tlb_inv.done = val;
>>> +
>>> + if (GRAPHICS_VER(i915) >= 12 &&
>>> + (engine->class == VIDEO_DECODE_CLASS ||
>>> + engine->class == VIDEO_ENHANCEMENT_CLASS ||
>>> + engine->class == COMPUTE_CLASS))
>>> + engine->tlb_inv.request = _MASKED_BIT_ENABLE(val);
>>> + else
>>> + engine->tlb_inv.request = val;
>>> +
>>> + return true;
>>> +}
>>> +
>>> static int engine_setup_common(struct intel_engine_cs *engine)
>>> {
>>> int err;
>>> @@ -1182,6 +1260,9 @@ static int engine_setup_common(struct
>>> intel_engine_cs *engine)
>>> intel_engine_init_whitelist(engine);
>>> intel_engine_init_ctx_wa(engine);
>>> + if (intel_engine_init_tlb_invalidation(engine))
>>> + engine->flags |= I915_ENGINE_HAS_TLB_INVALIDATION;
>>> +
>>> if (GRAPHICS_VER(engine->i915) >= 12)
>>> engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> index 4fd54fb8810f..8df4a09a6022 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> @@ -341,6 +341,19 @@ struct intel_engine_guc_stats {
>>> u64 start_gt_clk;
>>> };
>>> +union intel_engine_tlb_inv_reg {
>>> + i915_reg_t reg;
>>> + i915_mcr_reg_t mcr_reg;
>>> +};
>>> +
>>> +struct intel_engine_tlb_inv
>>> +{
>>> + bool mcr;
>>> + union intel_engine_tlb_inv_reg reg;
>>> + u32 request;
>>> + u32 done;
>>> +};
>>> +
>>> struct intel_engine_cs {
>>> struct drm_i915_private *i915;
>>> struct intel_gt *gt;
>>> @@ -372,6 +385,8 @@ struct intel_engine_cs {
>>> u32 context_size;
>>> u32 mmio_base;
>>> + struct intel_engine_tlb_inv tlb_inv;
>>> +
>>> /*
>>> * Some w/a require forcewake to be held (which prevents RC6)
>>> while
>>> * a particular engine is active. If so, we set fw_domain to which
>>> @@ -556,6 +571,7 @@ struct intel_engine_cs {
>>> #define I915_ENGINE_HAS_EU_PRIORITY BIT(10)
>>> #define I915_ENGINE_FIRST_RENDER_COMPUTE BIT(11)
>>> #define I915_ENGINE_USES_WA_HOLD_CCS_SWITCHOUT BIT(12)
>>> +#define I915_ENGINE_HAS_TLB_INVALIDATION BIT(13)
>>> unsigned int flags;
>>> /*
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
>>> b/drivers/gpu/drm/i915/gt/intel_gt.c
>>> index 7eeee5a7cb33..df7afff16fd6 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>>> @@ -983,36 +983,6 @@ void intel_gt_info_print(const struct
>>> intel_gt_info *info,
>>> intel_sseu_dump(&info->sseu, p);
>>> }
>>> -struct reg_and_bit {
>>> - union {
>>> - i915_reg_t reg;
>>> - i915_mcr_reg_t mcr_reg;
>>> - };
>>> - u32 bit;
>>> -};
>>> -
>>> -static struct reg_and_bit
>>> -get_reg_and_bit(const struct intel_engine_cs *engine, const bool gen8,
>>> - const i915_reg_t *regs, const unsigned int num)
>>> -{
>>> - const unsigned int class = engine->class;
>>> - struct reg_and_bit rb = { };
>>> -
>>> - if (drm_WARN_ON_ONCE(&engine->i915->drm,
>>> - class >= num || !regs[class].reg))
>>> - return rb;
>>> -
>>> - rb.reg = regs[class];
>>> - if (gen8 && class == VIDEO_DECODE_CLASS)
>>> - rb.reg.reg += 4 * engine->instance; /* GEN8_M2TCR */
>>> - else
>>> - rb.bit = engine->instance;
>>> -
>>> - rb.bit = BIT(rb.bit);
>>> -
>>> - return rb;
>>> -}
>>> -
>>> /*
>>> * HW architecture suggest typical invalidation time at 40us,
>>> * with pessimistic cases up to 100us and a recommendation to
>>> @@ -1026,14 +996,20 @@ get_reg_and_bit(const struct intel_engine_cs
>>> *engine, const bool gen8,
>>> * but are now considered MCR registers. Since they exist within a
>>> GAM range,
>>> * the primary instance of the register rolls up the status from
>>> each unit.
>>> */
>>> -static int wait_for_invalidate(struct intel_gt *gt, struct
>>> reg_and_bit rb)
>>> +static int wait_for_invalidate(struct intel_engine_cs *engine)
>>> {
>>> - if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 50))
>>> - return intel_gt_mcr_wait_for_reg(gt, rb.mcr_reg, rb.bit, 0,
>>> + if (engine->tlb_inv.mcr)
>>> + return intel_gt_mcr_wait_for_reg(engine->gt,
>>> + engine->tlb_inv.reg.mcr_reg,
>>> + engine->tlb_inv.done,
>>> + 0,
>>> TLB_INVAL_TIMEOUT_US,
>>> TLB_INVAL_TIMEOUT_MS);
>>> else
>>> - return __intel_wait_for_register_fw(gt->uncore, rb.reg,
>>> rb.bit, 0,
>>> + return __intel_wait_for_register_fw(engine->gt->uncore,
>>> + engine->tlb_inv.reg.reg,
>>> + engine->tlb_inv.done,
>>> + 0,
>>> TLB_INVAL_TIMEOUT_US,
>>> TLB_INVAL_TIMEOUT_MS,
>>> NULL);
>>> @@ -1041,50 +1017,14 @@ static int wait_for_invalidate(struct
>>> intel_gt *gt, struct reg_and_bit rb)
>>> static void mmio_invalidate_full(struct intel_gt *gt)
>>> {
>>> - static const i915_reg_t gen8_regs[] = {
>>> - [RENDER_CLASS] = GEN8_RTCR,
>>> - [VIDEO_DECODE_CLASS] = GEN8_M1TCR, /* , GEN8_M2TCR */
>>> - [VIDEO_ENHANCEMENT_CLASS] = GEN8_VTCR,
>>> - [COPY_ENGINE_CLASS] = GEN8_BTCR,
>>> - };
>>> - static const i915_reg_t gen12_regs[] = {
>>> - [RENDER_CLASS] = GEN12_GFX_TLB_INV_CR,
>>> - [VIDEO_DECODE_CLASS] = GEN12_VD_TLB_INV_CR,
>>> - [VIDEO_ENHANCEMENT_CLASS] = GEN12_VE_TLB_INV_CR,
>>> - [COPY_ENGINE_CLASS] = GEN12_BLT_TLB_INV_CR,
>>> - [COMPUTE_CLASS] = GEN12_COMPCTX_TLB_INV_CR,
>>> - };
>>> - static const i915_mcr_reg_t xehp_regs[] = {
>>> - [RENDER_CLASS] = XEHP_GFX_TLB_INV_CR,
>>> - [VIDEO_DECODE_CLASS] = XEHP_VD_TLB_INV_CR,
>>> - [VIDEO_ENHANCEMENT_CLASS] = XEHP_VE_TLB_INV_CR,
>>> - [COPY_ENGINE_CLASS] = XEHP_BLT_TLB_INV_CR,
>>> - [COMPUTE_CLASS] = XEHP_COMPCTX_TLB_INV_CR,
>>> - };
>>> struct drm_i915_private *i915 = gt->i915;
>>> struct intel_uncore *uncore = gt->uncore;
>>> struct intel_engine_cs *engine;
>>> intel_engine_mask_t awake, tmp;
>>> enum intel_engine_id id;
>>> - const i915_reg_t *regs;
>>> - unsigned int num = 0;
>>> unsigned long flags;
>>> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
>>> - regs = NULL;
>>> - num = ARRAY_SIZE(xehp_regs);
>>> - } else if (GRAPHICS_VER(i915) == 12) {
>>> - regs = gen12_regs;
>>> - num = ARRAY_SIZE(gen12_regs);
>>> - } else if (GRAPHICS_VER(i915) >= 8 && GRAPHICS_VER(i915) <= 11) {
>>> - regs = gen8_regs;
>>> - num = ARRAY_SIZE(gen8_regs);
>>> - } else if (GRAPHICS_VER(i915) < 8) {
>>> - return;
>>> - }
>>> -
>>> - if (drm_WARN_ONCE(&i915->drm, !num,
>>> - "Platform does not implement TLB invalidation!"))
>>> + if (GRAPHICS_VER(i915) < 8)
>>> return;
>>> intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
>>> @@ -1094,33 +1034,22 @@ static void mmio_invalidate_full(struct
>>> intel_gt *gt)
>>> awake = 0;
>>> for_each_engine(engine, gt, id) {
>>> - struct reg_and_bit rb;
>>> -
>>> if (!intel_engine_pm_is_awake(engine))
>>> continue;
>>> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
>>> - u32 val = BIT(engine->instance);
>>> + if (drm_WARN_ON_ONCE(&i915->drm,
>>> + !(engine->flags &
>>> I915_ENGINE_HAS_TLB_INVALIDATION)))
>>> + continue;
>>
>> Hmm, can this flag change dynamically? If not why not put it in
>> initialization phase.
>
> Could do. In that case I couldn't have any asserts that engine->tlb_inv
> is valid but perhaps that is okay. Could just fail engine probe from
> there if register table is incomplete. Sounds better indeed, I'll change
> it.
>
>>
>>> - if (engine->class == VIDEO_DECODE_CLASS ||
>>> - engine->class == VIDEO_ENHANCEMENT_CLASS ||
>>> - engine->class == COMPUTE_CLASS)
>>> - val = _MASKED_BIT_ENABLE(val);
>>> + if (engine->tlb_inv.mcr)
>>> intel_gt_mcr_multicast_write_fw(gt,
>>> - xehp_regs[engine->class],
>>> - val);
>>> - } else {
>>> - rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
>>> - if (!i915_mmio_reg_offset(rb.reg))
>>> - continue;
>>> -
>>> - if (GRAPHICS_VER(i915) == 12 && (engine->class ==
>>> VIDEO_DECODE_CLASS ||
>>> - engine->class == VIDEO_ENHANCEMENT_CLASS ||
>>> - engine->class == COMPUTE_CLASS))
>>> - rb.bit = _MASKED_BIT_ENABLE(rb.bit);
>>> -
>>> - intel_uncore_write_fw(uncore, rb.reg, rb.bit);
>>> - }
>>> + engine->tlb_inv.reg.mcr_reg,
>>> + engine->tlb_inv.request);
>>> + else
>>> + intel_uncore_write_fw(uncore,
>>> + engine->tlb_inv.reg.reg,
>>> + engine->tlb_inv.request);
>>> +
>>> awake |= engine->mask;
>>> }
>>> @@ -1139,16 +1068,7 @@ static void mmio_invalidate_full(struct
>>> intel_gt *gt)
>>> intel_gt_mcr_unlock(gt, flags);
>>> for_each_engine_masked(engine, gt, awake, tmp) {
>>> - struct reg_and_bit rb;
>>> -
>>> - if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) {
>>> - rb.mcr_reg = xehp_regs[engine->class];
>>> - rb.bit = BIT(engine->instance);
>>> - } else {
>>> - rb = get_reg_and_bit(engine, regs == gen8_regs, regs, num);
>>> - }
>>> -
>>> - if (wait_for_invalidate(gt, rb))
>>> + if (wait_for_invalidate(engine))
>>> drm_err_ratelimited(>->i915->drm,
>>> "%s TLB invalidation did not complete in
>>> %ums!\n",
>>> engine->name, TLB_INVAL_TIMEOUT_MS);
>>
>> With minor comments addressed:
>>
>> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
>
> I'll send v2 at some point, please stand by.
>
> Regards,
>
> Tvrtko
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
2022-12-13 12:39 [Intel-gfx] [PATCH 1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines Tvrtko Ursulin
` (3 preceding siblings ...)
2022-12-13 13:46 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2022-12-14 18:35 ` Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2022-12-14 18:35 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 20158 bytes --]
== Series Details ==
Series: series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines
URL : https://patchwork.freedesktop.org/series/111895/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12501_full -> Patchwork_111895v1_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Participating hosts (14 -> 9)
------------------------------
Missing (5): shard-tglu-9 shard-tglu-10 shard-tglu shard-rkl shard-dg1
Known issues
------------
Here are the changes found in Patchwork_111895v1_full that come from known issues:
### CI changes ###
#### Issues hit ####
* boot:
- shard-apl: ([PASS][1], [PASS][2], [PASS][3], [PASS][4], [PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25]) -> ([PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [FAIL][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [PASS][50]) ([i915#4386])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl1/boot.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl1/boot.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl1/boot.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl1/boot.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl2/boot.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl2/boot.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl2/boot.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl2/boot.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl2/boot.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl2/boot.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl3/boot.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl3/boot.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl3/boot.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl3/boot.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl3/boot.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl6/boot.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl6/boot.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl6/boot.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl6/boot.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl6/boot.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl8/boot.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl8/boot.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl8/boot.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl8/boot.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl8/boot.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl8/boot.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl8/boot.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl8/boot.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl8/boot.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl6/boot.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl6/boot.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl6/boot.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl6/boot.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl6/boot.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl3/boot.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl3/boot.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl3/boot.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl3/boot.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl2/boot.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl2/boot.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl2/boot.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl2/boot.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl2/boot.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl2/boot.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl1/boot.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl1/boot.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl1/boot.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl1/boot.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl1/boot.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl8/boot.html
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_balancer@parallel-out-fence:
- shard-iclb: [PASS][51] -> [SKIP][52] ([i915#4525])
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb1/igt@gem_exec_balancer@parallel-out-fence.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb6/igt@gem_exec_balancer@parallel-out-fence.html
* igt@kms_ccs@pipe-d-random-ccs-data-y_tiled_gen12_rc_ccs:
- shard-skl: NOTRUN -> [SKIP][53] ([fdo#109271]) +3 similar issues
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-skl6/igt@kms_ccs@pipe-d-random-ccs-data-y_tiled_gen12_rc_ccs.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible@b-dp1:
- shard-apl: [PASS][54] -> [FAIL][55] ([i915#79])
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl1/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-dp1.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl8/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-dp1.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1:
- shard-skl: [PASS][56] -> [FAIL][57] ([i915#79]) +1 similar issue
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-skl1/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html
* igt@kms_flip@flip-vs-suspend@a-edp1:
- shard-skl: [PASS][58] -> [INCOMPLETE][59] ([i915#4839])
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-skl4/igt@kms_flip@flip-vs-suspend@a-edp1.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-skl1/igt@kms_flip@flip-vs-suspend@a-edp1.html
* igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1:
- shard-skl: [PASS][60] -> [FAIL][61] ([i915#2122]) +1 similar issue
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-skl1/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-skl1/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html
* igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling@pipe-a-default-mode:
- shard-iclb: [PASS][62] -> [SKIP][63] ([i915#3555])
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb3/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling@pipe-a-default-mode.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling@pipe-a-default-mode.html
* igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-16bpp-4tile-downscaling@pipe-a-valid-mode:
- shard-iclb: NOTRUN -> [SKIP][64] ([i915#2587] / [i915#2672]) +3 similar issues
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb1/igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-16bpp-4tile-downscaling@pipe-a-valid-mode.html
* igt@kms_flip_scaled_crc@flip-64bpp-linear-to-16bpp-linear-downscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][65] ([i915#3555])
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-linear-to-16bpp-linear-downscaling@pipe-a-default-mode.html
* igt@kms_flip_scaled_crc@flip-64bpp-yftile-to-32bpp-yftile-upscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][66] ([i915#2672]) +2 similar issues
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb3/igt@kms_flip_scaled_crc@flip-64bpp-yftile-to-32bpp-yftile-upscaling@pipe-a-default-mode.html
* igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilercccs-downscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][67] ([i915#2672] / [i915#3555])
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytilercccs-downscaling@pipe-a-default-mode.html
* igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-edp-1:
- shard-iclb: [PASS][68] -> [SKIP][69] ([i915#5176]) +2 similar issues
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb3/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-edp-1.html
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb2/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-edp-1.html
* igt@kms_psr@psr2_basic:
- shard-iclb: [PASS][70] -> [SKIP][71] ([fdo#109441]) +1 similar issue
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb2/igt@kms_psr@psr2_basic.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb1/igt@kms_psr@psr2_basic.html
* igt@kms_psr_stress_test@flip-primary-invalidate-overlay:
- shard-iclb: [PASS][72] -> [SKIP][73] ([i915#5519])
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb7/igt@kms_psr_stress_test@flip-primary-invalidate-overlay.html
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb7/igt@kms_psr_stress_test@flip-primary-invalidate-overlay.html
#### Possible fixes ####
* igt@gem_eio@in-flight-contexts-1us:
- shard-snb: [FAIL][74] ([i915#4409]) -> [PASS][75]
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-snb5/igt@gem_eio@in-flight-contexts-1us.html
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-snb7/igt@gem_eio@in-flight-contexts-1us.html
* igt@gem_eio@in-flight-contexts-immediate:
- shard-skl: [DMESG-WARN][76] ([i915#1982]) -> [PASS][77] +2 similar issues
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-skl6/igt@gem_eio@in-flight-contexts-immediate.html
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-skl1/igt@gem_eio@in-flight-contexts-immediate.html
* igt@gem_exec_balancer@parallel-keep-in-fence:
- shard-iclb: [SKIP][78] ([i915#4525]) -> [PASS][79] +1 similar issue
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb5/igt@gem_exec_balancer@parallel-keep-in-fence.html
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb2/igt@gem_exec_balancer@parallel-keep-in-fence.html
* igt@gem_exec_fair@basic-flow@rcs0:
- shard-tglb: [FAIL][80] ([i915#2842]) -> [PASS][81]
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-tglb3/igt@gem_exec_fair@basic-flow@rcs0.html
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-tglb2/igt@gem_exec_fair@basic-flow@rcs0.html
* igt@gem_exec_fair@basic-none-solo@rcs0:
- shard-apl: [FAIL][82] ([i915#2842]) -> [PASS][83]
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-apl3/igt@gem_exec_fair@basic-none-solo@rcs0.html
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-apl8/igt@gem_exec_fair@basic-none-solo@rcs0.html
* igt@i915_selftest@live@gt_heartbeat:
- shard-skl: [DMESG-FAIL][84] ([i915#5334]) -> [PASS][85]
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-skl1/igt@i915_selftest@live@gt_heartbeat.html
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-skl7/igt@i915_selftest@live@gt_heartbeat.html
* igt@kms_async_flips@alternate-sync-async-flip@pipe-b-edp-1:
- shard-skl: [FAIL][86] ([i915#2521]) -> [PASS][87] +2 similar issues
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-skl7/igt@kms_async_flips@alternate-sync-async-flip@pipe-b-edp-1.html
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-skl1/igt@kms_async_flips@alternate-sync-async-flip@pipe-b-edp-1.html
* igt@kms_flip@flip-vs-expired-vblank@a-edp1:
- shard-skl: [FAIL][88] ([i915#79]) -> [PASS][89] +2 similar issues
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-skl1/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-skl6/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
* igt@kms_flip@flip-vs-expired-vblank@d-edp1:
- shard-tglb: [FAIL][90] ([i915#79]) -> [PASS][91]
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-tglb1/igt@kms_flip@flip-vs-expired-vblank@d-edp1.html
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-tglb5/igt@kms_flip@flip-vs-expired-vblank@d-edp1.html
* igt@kms_flip@plain-flip-ts-check@a-edp1:
- shard-skl: [FAIL][92] ([i915#2122]) -> [PASS][93] +1 similar issue
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-skl6/igt@kms_flip@plain-flip-ts-check@a-edp1.html
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-skl1/igt@kms_flip@plain-flip-ts-check@a-edp1.html
* igt@kms_psr@psr2_primary_page_flip:
- shard-iclb: [SKIP][94] ([fdo#109441]) -> [PASS][95] +3 similar issues
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb1/igt@kms_psr@psr2_primary_page_flip.html
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html
* igt@kms_psr_stress_test@invalidate-primary-flip-overlay:
- shard-iclb: [SKIP][96] ([i915#5519]) -> [PASS][97]
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb5/igt@kms_psr_stress_test@invalidate-primary-flip-overlay.html
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb6/igt@kms_psr_stress_test@invalidate-primary-flip-overlay.html
#### Warnings ####
* igt@i915_pm_dc@dc3co-vpb-simulation:
- shard-iclb: [SKIP][98] ([i915#658]) -> [SKIP][99] ([i915#588])
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb3/igt@i915_pm_dc@dc3co-vpb-simulation.html
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
* igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-fully-sf:
- shard-iclb: [SKIP][100] ([i915#2920]) -> [SKIP][101] ([i915#658])
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb2/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-fully-sf.html
[101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb1/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-fully-sf.html
* igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area:
- shard-iclb: [SKIP][102] ([fdo#111068] / [i915#658]) -> [SKIP][103] ([i915#2920])
[102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb5/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html
[103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb2/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html
* igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb:
- shard-iclb: [SKIP][104] ([i915#658]) -> [SKIP][105] ([i915#2920])
[104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-iclb3/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html
[105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-iclb2/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html
* igt@runner@aborted:
- shard-skl: ([FAIL][106], [FAIL][107]) ([i915#3002] / [i915#4312] / [i915#6949]) -> [FAIL][108] ([i915#6949])
[106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-skl4/igt@runner@aborted.html
[107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12501/shard-skl4/igt@runner@aborted.html
[108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/shard-skl7/igt@runner@aborted.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
[i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
[i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
[i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
[i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4386]: https://gitlab.freedesktop.org/drm/intel/issues/4386
[i915#4409]: https://gitlab.freedesktop.org/drm/intel/issues/4409
[i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
[i915#4839]: https://gitlab.freedesktop.org/drm/intel/issues/4839
[i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#5519]: https://gitlab.freedesktop.org/drm/intel/issues/5519
[i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#6949]: https://gitlab.freedesktop.org/drm/intel/issues/6949
[i915#7688]: https://gitlab.freedesktop.org/drm/intel/issues/7688
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
Build changes
-------------
* Linux: CI_DRM_12501 -> Patchwork_111895v1
CI-20190529: 20190529
CI_DRM_12501: 1b38b5a419ab3d838b6ac95d22f1fe057fc8889d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7091: b8015f920c9f469d3733854263cb878373c1df51 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_111895v1: 1b38b5a419ab3d838b6ac95d22f1fe057fc8889d @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111895v1/index.html
[-- Attachment #2: Type: text/html, Size: 22863 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-12-14 18:36 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-13 12:39 [Intel-gfx] [PATCH 1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines Tvrtko Ursulin
2022-12-13 12:39 ` [Intel-gfx] [PATCH 2/2] drm/i915: Consolidate TLB invalidation flow Tvrtko Ursulin
2022-12-13 14:52 ` Andrzej Hajda
2022-12-13 15:22 ` Tvrtko Ursulin
2022-12-14 9:02 ` Andrzej Hajda
2022-12-13 13:24 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: fix TLB invalidation for Gen12.50 video and compute engines Patchwork
2022-12-13 13:24 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-12-13 13:46 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-12-14 18:35 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2022-12-07 17:36 [Intel-gfx] [PATCH 1/2] " Andrzej Hajda
2022-12-09 9:37 ` Tvrtko Ursulin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox