* [Intel-gfx] [Patch v3 1/2] drm/i915: Move reg_in_range_table @ 2023-11-29 20:51 Matt Atwood 2023-11-29 20:51 ` [Intel-gfx] [Patch v3 2/2] drm/i915: Introduce Wa_1401127433 Matt Atwood ` (4 more replies) 0 siblings, 5 replies; 8+ messages in thread From: Matt Atwood @ 2023-11-29 20:51 UTC (permalink / raw) To: intel-gfx Function reg_in_range_table is useful in more places, move function to intel_uncore.h to make available to more places. Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> --- drivers/gpu/drm/i915/i915_perf.c | 13 +------------ drivers/gpu/drm/i915/intel_uncore.h | 12 ++++++++++++ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 7b1c8de2f9cb3..5e5dc73621379 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -219,6 +219,7 @@ #include "i915_perf.h" #include "i915_perf_oa_regs.h" #include "i915_reg.h" +#include "intel_uncore.h" /* HW requires this to be a power of two, between 128k and 16M, though driver * is currently generally designed assuming the largest 16M size is used such @@ -4324,18 +4325,6 @@ static bool gen8_is_valid_flex_addr(struct i915_perf *perf, u32 addr) return false; } -static bool reg_in_range_table(u32 addr, const struct i915_range *table) -{ - while (table->start || table->end) { - if (addr >= table->start && addr <= table->end) - return true; - - table++; - } - - return false; -} - #define REG_EQUAL(addr, mmio) \ ((addr) == i915_mmio_reg_offset(mmio)) diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h index f419c311a0dea..1e85eaec1fc5a 100644 --- a/drivers/gpu/drm/i915/intel_uncore.h +++ b/drivers/gpu/drm/i915/intel_uncore.h @@ -496,6 +496,18 @@ static inline int intel_uncore_write_and_verify(struct intel_uncore *uncore, return (reg_val & mask) != expected_val ? -EINVAL : 0; } +static inline bool reg_in_range_table(u32 addr, const struct i915_range *table) +{ + while (table->start || table->end) { + if (addr >= table->start && addr <= table->end) + return true; + + table++; + } + + return false; +} + static inline void __iomem *intel_uncore_regs(struct intel_uncore *uncore) { return uncore->regs; -- 2.40.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Intel-gfx] [Patch v3 2/2] drm/i915: Introduce Wa_1401127433 2023-11-29 20:51 [Intel-gfx] [Patch v3 1/2] drm/i915: Move reg_in_range_table Matt Atwood @ 2023-11-29 20:51 ` Matt Atwood 2023-12-01 22:05 ` Matt Roper 2023-11-30 6:58 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/2] drm/i915: Move reg_in_range_table Patchwork ` (3 subsequent siblings) 4 siblings, 1 reply; 8+ messages in thread From: Matt Atwood @ 2023-11-29 20:51 UTC (permalink / raw) To: intel-gfx Wa_14011274333 applies to gen 12.0->12.55. There are regions of registers that restore to default values on resume from rc6. The hardware has a built in register (CTX_WA_PTR), that can point to a region of memory full of commands to restore non default values on rc6 resume. Based off patch by Tilak Tangudu. v2: Use correct lineage number, more generically apply workarounds for all registers impacted, move workaround to gt/intel_workarounds.c (MattR) v3: Correctly use intel_engine_cs, intel_engine_regs, use reg_in_range_table() for wa registers impacted search, move the majority of functional changes into intel_workarounds.c, free up pinned memory on engine tear down, be more verbose in commit message and wa comments, use generic function for platforms applied to. (MattR) Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 + drivers/gpu/drm/i915/gt/intel_engine_regs.h | 4 + drivers/gpu/drm/i915/gt/intel_engine_types.h | 3 + drivers/gpu/drm/i915/gt/intel_workarounds.c | 114 +++++++++++++++++++ drivers/gpu/drm/i915/gt/intel_workarounds.h | 1 + 5 files changed, 124 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 40687806d22a6..d3628462bfb3e 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -1573,6 +1573,8 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine) intel_wa_list_free(&engine->ctx_wa_list); intel_wa_list_free(&engine->wa_list); intel_wa_list_free(&engine->whitelist); + + intel_ctx_wa_bb_fini(engine); } /** diff --git a/drivers/gpu/drm/i915/gt/intel_engine_regs.h b/drivers/gpu/drm/i915/gt/intel_engine_regs.h index a8eac59e37793..becb466f5910d 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_regs.h @@ -251,6 +251,10 @@ #define GEN11_VECS_SFC_USAGE(base) _MMIO((base) + 0x2014) #define GEN11_VECS_SFC_USAGE_BIT (1 << 0) +#define CTX_WA_PTR(base) _MMIO((base) + 0x2058) +#define CTX_WA_PTR_ADDR_MASK REG_GENMASK(31, 12) +#define CTX_WA_VALID REG_BIT(0) + #define RING_HWS_PGA_GEN6(base) _MMIO((base) + 0x2080) #define GEN12_HCP_SFC_LOCK_STATUS(base) _MMIO((base) + 0x2914) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index 960e6be2042fe..95ff3e1adf11e 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -388,6 +388,9 @@ struct intel_engine_cs { u32 context_size; u32 mmio_base; + u32 *ctx_wa_bb; + struct i915_vma *vma; + struct intel_engine_tlb_inv tlb_inv; /* diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 9bc0654efdc0c..1d33555039725 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -5,6 +5,8 @@ #include "i915_drv.h" #include "i915_reg.h" +#include "i915_perf.h" +#include "gem/i915_gem_internal.h" #include "intel_context.h" #include "intel_engine_pm.h" #include "intel_engine_regs.h" @@ -14,6 +16,7 @@ #include "intel_gt_print.h" #include "intel_gt_regs.h" #include "intel_ring.h" +#include "intel_uncore.h" #include "intel_workarounds.h" /** @@ -2985,6 +2988,105 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li } } +static const struct i915_range tdl_power_ctx[] = { + { .start = 0xe100, .end = 0xe100 }, + { .start = 0xe180, .end = 0xe194 }, + {}, +}; + +static void +emit_ctx_wa_bb(struct intel_engine_cs *engine, struct i915_wa_list *wal) +{ + const struct i915_wa *wa; + struct intel_uncore *uncore = engine->gt->uncore; + int i, count = 0; + u32 *cs = engine->ctx_wa_bb; + + *cs++ = MI_LOAD_REGISTER_IMM(1); + for (i = 0, wa = wal->list; i < wal->count; i++, wa++) { + if (reg_in_range_table(i915_mmio_reg_offset(wa->mcr_reg), tdl_power_ctx)) { + *cs++ = i915_mmio_reg_offset(wa->mcr_reg); + *cs++ = wa->set; + count++; + } + } + *cs++ = MI_BATCH_BUFFER_END; + + if (count != 0) { + i915_gem_object_flush_map(engine->vma->obj); + intel_uncore_write(uncore, CTX_WA_PTR(engine->mmio_base), + REG_FIELD_PREP(CTX_WA_PTR_ADDR_MASK, + i915_vma_offset(engine->vma) & GENMASK(31, 12)) | + CTX_WA_VALID); + } +} + +static int ctx_wa_bb_init(struct intel_engine_cs *engine) +{ + struct drm_i915_private *i915 = engine->i915; + struct intel_gt *gt = engine->gt; + struct drm_i915_gem_object *obj; + struct i915_vma *vma; + void *batch; + struct i915_gem_ww_ctx ww; + int err; + + obj = i915_gem_object_create_internal(i915, PAGE_SIZE); + if (IS_ERR(obj)) + return PTR_ERR(obj); + + vma = i915_vma_instance(obj, >->ggtt->vm, NULL); + if (IS_ERR(vma)) { + err = PTR_ERR(vma); + goto err; + } + + engine->vma = vma; + i915_gem_ww_ctx_init(&ww, true); + +retry: + err = i915_gem_object_lock(obj, &ww); + if (!err) + err = i915_ggtt_pin(engine->vma, &ww, 0, PIN_HIGH); + if (err) + goto err_ww_fini; + + batch = i915_gem_object_pin_map(obj, I915_MAP_WB); + if (IS_ERR(batch)) { + err = PTR_ERR(batch); + goto err_unpin; + } + engine->ctx_wa_bb = batch; + + return 0; + +err_unpin: + i915_vma_unpin(engine->vma); + +err_ww_fini: + if (err == -EDEADLK) { + err = i915_gem_ww_ctx_backoff(&ww); + if (!err) + goto retry; + } + + i915_gem_ww_ctx_fini(&ww); + i915_vma_put(engine->vma); + +err: + i915_gem_object_put(obj); + return err; +} + +void intel_ctx_wa_bb_fini(struct intel_engine_cs *engine) +{ + i915_vma_unpin_and_release(&engine->vma, I915_VMA_RELEASE_MAP); +} + +#define NEEDS_CTX_WABB(engine) ( \ + IS_GFX_GT_IP_RANGE(engine->gt, IP_VER(12, 0), IP_VER(12, 55)) && \ + engine->class == RENDER_CLASS) + static void engine_init_workarounds(struct intel_engine_cs *engine, struct i915_wa_list *wal) { @@ -3007,6 +3109,18 @@ engine_init_workarounds(struct intel_engine_cs *engine, struct i915_wa_list *wal rcs_engine_wa_init(engine, wal); else xcs_engine_wa_init(engine, wal); + + /* Wa_14011274333 + * After the workaround list has been populated some platforms have + * regions of addresses that do not retain their values after resuming + * from rc6. For these platforms, add all workarounds in these regions + * to a batch buffer, this batch buffer is stored in memory and + * executes on every rc6 resume. + */ + if (NEEDS_CTX_WABB(engine)) { + ctx_wa_bb_init(engine); + emit_ctx_wa_bb(engine, wal); + } } void intel_engine_init_workarounds(struct intel_engine_cs *engine) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h index 9beaab77c7f0b..fe8946b0c7b67 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h @@ -36,4 +36,5 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine); int intel_engine_verify_workarounds(struct intel_engine_cs *engine, const char *from); +void intel_ctx_wa_bb_fini(struct intel_engine_cs *engine); #endif -- 2.40.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [Patch v3 2/2] drm/i915: Introduce Wa_1401127433 2023-11-29 20:51 ` [Intel-gfx] [Patch v3 2/2] drm/i915: Introduce Wa_1401127433 Matt Atwood @ 2023-12-01 22:05 ` Matt Roper 0 siblings, 0 replies; 8+ messages in thread From: Matt Roper @ 2023-12-01 22:05 UTC (permalink / raw) To: Matt Atwood; +Cc: intel-gfx You're missing a digit in the patch subject. On Wed, Nov 29, 2023 at 12:51:22PM -0800, Matt Atwood wrote: > Wa_14011274333 applies to gen 12.0->12.55. There are regions of > registers that restore to default values on resume from rc6. The It would be more clear to write this sentence as "The TDL power context is not properly restored on RC6 exit, causing registers in the context to revert to their hardware default values on RC6 exit." > hardware has a built in register (CTX_WA_PTR), that can point to a > region of memory full of commands to restore non default values on rc6 > resume. > > Based off patch by Tilak Tangudu. > > v2: Use correct lineage number, more generically apply workarounds for > all registers impacted, move workaround to gt/intel_workarounds.c > (MattR) > > v3: Correctly use intel_engine_cs, intel_engine_regs, use > reg_in_range_table() for wa registers impacted search, move the majority > of functional changes into intel_workarounds.c, free up pinned memory on > engine tear down, be more verbose in commit message and wa comments, use > generic function for platforms applied to. (MattR) > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 + > drivers/gpu/drm/i915/gt/intel_engine_regs.h | 4 + > drivers/gpu/drm/i915/gt/intel_engine_types.h | 3 + > drivers/gpu/drm/i915/gt/intel_workarounds.c | 114 +++++++++++++++++++ > drivers/gpu/drm/i915/gt/intel_workarounds.h | 1 + > 5 files changed, 124 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > index 40687806d22a6..d3628462bfb3e 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > @@ -1573,6 +1573,8 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine) > intel_wa_list_free(&engine->ctx_wa_list); > intel_wa_list_free(&engine->wa_list); > intel_wa_list_free(&engine->whitelist); > + > + intel_ctx_wa_bb_fini(engine); > } > > /** > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_regs.h b/drivers/gpu/drm/i915/gt/intel_engine_regs.h > index a8eac59e37793..becb466f5910d 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_regs.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine_regs.h > @@ -251,6 +251,10 @@ > #define GEN11_VECS_SFC_USAGE(base) _MMIO((base) + 0x2014) > #define GEN11_VECS_SFC_USAGE_BIT (1 << 0) > > +#define CTX_WA_PTR(base) _MMIO((base) + 0x2058) This isn't the right offset; you've accidentally pre-added the RCS base. The definition should be (base + 0x58). > +#define CTX_WA_PTR_ADDR_MASK REG_GENMASK(31, 12) > +#define CTX_WA_VALID REG_BIT(0) > + > #define RING_HWS_PGA_GEN6(base) _MMIO((base) + 0x2080) > > #define GEN12_HCP_SFC_LOCK_STATUS(base) _MMIO((base) + 0x2914) > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h > index 960e6be2042fe..95ff3e1adf11e 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h > @@ -388,6 +388,9 @@ struct intel_engine_cs { > u32 context_size; > u32 mmio_base; > > + u32 *ctx_wa_bb; > + struct i915_vma *vma; You should pick a more descriptive name here. Otherwise nobody will know what this is a vma for. > + > struct intel_engine_tlb_inv tlb_inv; > > /* > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 9bc0654efdc0c..1d33555039725 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -5,6 +5,8 @@ > > #include "i915_drv.h" > #include "i915_reg.h" > +#include "i915_perf.h" > +#include "gem/i915_gem_internal.h" > #include "intel_context.h" > #include "intel_engine_pm.h" > #include "intel_engine_regs.h" > @@ -14,6 +16,7 @@ > #include "intel_gt_print.h" > #include "intel_gt_regs.h" > #include "intel_ring.h" > +#include "intel_uncore.h" > #include "intel_workarounds.h" > > /** > @@ -2985,6 +2988,105 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li > } > } > > +static const struct i915_range tdl_power_ctx[] = { > + { .start = 0xe100, .end = 0xe100 }, > + { .start = 0xe180, .end = 0xe194 }, > + {}, > +}; > + > +static void > +emit_ctx_wa_bb(struct intel_engine_cs *engine, struct i915_wa_list *wal) > +{ > + const struct i915_wa *wa; > + struct intel_uncore *uncore = engine->gt->uncore; > + int i, count = 0; > + u32 *cs = engine->ctx_wa_bb; > + > + *cs++ = MI_LOAD_REGISTER_IMM(1); I think you meant to come back and patch up the length of the instruction after emitting however many registers we find, right? It looks like you forgot that part. > + for (i = 0, wa = wal->list; i < wal->count; i++, wa++) { > + if (reg_in_range_table(i915_mmio_reg_offset(wa->mcr_reg), tdl_power_ctx)) { > + *cs++ = i915_mmio_reg_offset(wa->mcr_reg); > + *cs++ = wa->set; Part of the reason this is so easy to handle is because all of the registers in the TDL power context are masked, so we don't have to worry about clobbering unrelated bits when re-programming the workaround. You might want to add a comment noting that so that it doesn't look like we're just reprogramming the whole register here. > + count++; > + } > + } > + *cs++ = MI_BATCH_BUFFER_END; > + > + if (count != 0) { > + i915_gem_object_flush_map(engine->vma->obj); > + intel_uncore_write(uncore, CTX_WA_PTR(engine->mmio_base), > + REG_FIELD_PREP(CTX_WA_PTR_ADDR_MASK, > + i915_vma_offset(engine->vma) & GENMASK(31, 12)) | > + CTX_WA_VALID); CTX_WA_PTR gets cleared on render domain reset, so you need to make sure it gets re-applied properly as well. You should probably do this actual register write in intel_engine_apply_workarounds() (to make sure it's re-applied on the non-GuC platforms) and also add this register to the guc ADS regset so that the GuC will save/restore it around resets on ADL-P. If it turned out you didn't actually need the batchbuffer, you could release its object here and clear the vma reference in the engine. The presence/absence of that pointer can be used as the condition to determine whether the register should be written in intel_engine_apply_workarounds(). > + } > +} > + > +static int ctx_wa_bb_init(struct intel_engine_cs *engine) > +{ > + struct drm_i915_private *i915 = engine->i915; > + struct intel_gt *gt = engine->gt; > + struct drm_i915_gem_object *obj; > + struct i915_vma *vma; > + void *batch; > + struct i915_gem_ww_ctx ww; > + int err; > + > + obj = i915_gem_object_create_internal(i915, PAGE_SIZE); > + if (IS_ERR(obj)) > + return PTR_ERR(obj); > + > + vma = i915_vma_instance(obj, >->ggtt->vm, NULL); > + if (IS_ERR(vma)) { > + err = PTR_ERR(vma); > + goto err; > + } > + > + engine->vma = vma; > + i915_gem_ww_ctx_init(&ww, true); > + > +retry: > + err = i915_gem_object_lock(obj, &ww); > + if (!err) > + err = i915_ggtt_pin(engine->vma, &ww, 0, PIN_HIGH); > + if (err) > + goto err_ww_fini; > + > + batch = i915_gem_object_pin_map(obj, I915_MAP_WB); > + if (IS_ERR(batch)) { > + err = PTR_ERR(batch); > + goto err_unpin; > + } > + engine->ctx_wa_bb = batch; > + > + return 0; Aren't we still holding locks in the wait-wound context? > + > +err_unpin: > + i915_vma_unpin(engine->vma); > + > +err_ww_fini: > + if (err == -EDEADLK) { > + err = i915_gem_ww_ctx_backoff(&ww); > + if (!err) > + goto retry; > + } > + > + i915_gem_ww_ctx_fini(&ww); > + i915_vma_put(engine->vma); > + > +err: > + i915_gem_object_put(obj); > + return err; > +} > + > +void intel_ctx_wa_bb_fini(struct intel_engine_cs *engine) > +{ > + i915_vma_unpin_and_release(&engine->vma, I915_VMA_RELEASE_MAP); You might want to clear the vma pointer as well to allow this function to be used in more places, as noted above. > +} > + > +#define NEEDS_CTX_WABB(engine) ( \ It might be better to put the name of the workaround in the macro here. There are lots of potential reasons why we might want to execute a batch on RC6 exit; this workaround is just one specific reason, but there may be others in the future. > + IS_GFX_GT_IP_RANGE(engine->gt, IP_VER(12, 0), IP_VER(12, 55)) && \ This isn't needed on 12.55. It was a pre-production-only workaround for DG2-G10, and it never applied to DG2-G11 or DG2-G12. So 12.00 - 12.10 would be the proper range. Note that if you did need to program this on DG2, then you would have also needed to take Wa_18018699957 into account, which involves switching to use BB_PER_CTX_PTR instead of CTX_WA_PTR. > + engine->class == RENDER_CLASS) > + > static void > engine_init_workarounds(struct intel_engine_cs *engine, struct i915_wa_list *wal) > { > @@ -3007,6 +3109,18 @@ engine_init_workarounds(struct intel_engine_cs *engine, struct i915_wa_list *wal > rcs_engine_wa_init(engine, wal); > else > xcs_engine_wa_init(engine, wal); > + > + /* Wa_14011274333 > + * After the workaround list has been populated some platforms have > + * regions of addresses that do not retain their values after resuming > + * from rc6. For these platforms, add all workarounds in these regions > + * to a batch buffer, this batch buffer is stored in memory and > + * executes on every rc6 resume. > + */ > + if (NEEDS_CTX_WABB(engine)) { > + ctx_wa_bb_init(engine); This function returns errors, but you're not handling them here, so failure will be silently ignored. Matt > + emit_ctx_wa_bb(engine, wal); > + } > } > > void intel_engine_init_workarounds(struct intel_engine_cs *engine) > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h > index 9beaab77c7f0b..fe8946b0c7b67 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h > @@ -36,4 +36,5 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine); > int intel_engine_verify_workarounds(struct intel_engine_cs *engine, > const char *from); > > +void intel_ctx_wa_bb_fini(struct intel_engine_cs *engine); > #endif > -- > 2.40.1 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/2] drm/i915: Move reg_in_range_table 2023-11-29 20:51 [Intel-gfx] [Patch v3 1/2] drm/i915: Move reg_in_range_table Matt Atwood 2023-11-29 20:51 ` [Intel-gfx] [Patch v3 2/2] drm/i915: Introduce Wa_1401127433 Matt Atwood @ 2023-11-30 6:58 ` Patchwork 2023-11-30 6:58 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork ` (2 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Patchwork @ 2023-11-30 6:58 UTC (permalink / raw) To: Matt Atwood; +Cc: intel-gfx == Series Details == Series: series starting with [v3,1/2] drm/i915: Move reg_in_range_table URL : https://patchwork.freedesktop.org/series/127082/ State : warning == Summary == Error: dim checkpatch failed 553cf478868e drm/i915: Move reg_in_range_table 349983425e84 drm/i915: Introduce Wa_1401127433 -:188: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'engine' - possible side-effects? #188: FILE: drivers/gpu/drm/i915/gt/intel_workarounds.c:3078: +#define NEEDS_CTX_WABB(engine) ( \ + IS_GFX_GT_IP_RANGE(engine->gt, IP_VER(12, 0), IP_VER(12, 55)) && \ + engine->class == RENDER_CLASS) -:188: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'engine' may be better as '(engine)' to avoid precedence issues #188: FILE: drivers/gpu/drm/i915/gt/intel_workarounds.c:3078: +#define NEEDS_CTX_WABB(engine) ( \ + IS_GFX_GT_IP_RANGE(engine->gt, IP_VER(12, 0), IP_VER(12, 55)) && \ + engine->class == RENDER_CLASS) total: 0 errors, 0 warnings, 2 checks, 170 lines checked ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v3,1/2] drm/i915: Move reg_in_range_table 2023-11-29 20:51 [Intel-gfx] [Patch v3 1/2] drm/i915: Move reg_in_range_table Matt Atwood 2023-11-29 20:51 ` [Intel-gfx] [Patch v3 2/2] drm/i915: Introduce Wa_1401127433 Matt Atwood 2023-11-30 6:58 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/2] drm/i915: Move reg_in_range_table Patchwork @ 2023-11-30 6:58 ` Patchwork 2023-11-30 7:17 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork 2023-11-30 20:00 ` [Intel-gfx] [Patch v3 1/2] " Rodrigo Vivi 4 siblings, 0 replies; 8+ messages in thread From: Patchwork @ 2023-11-30 6:58 UTC (permalink / raw) To: Matt Atwood; +Cc: intel-gfx == Series Details == Series: series starting with [v3,1/2] drm/i915: Move reg_in_range_table URL : https://patchwork.freedesktop.org/series/127082/ 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] 8+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v3,1/2] drm/i915: Move reg_in_range_table 2023-11-29 20:51 [Intel-gfx] [Patch v3 1/2] drm/i915: Move reg_in_range_table Matt Atwood ` (2 preceding siblings ...) 2023-11-30 6:58 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork @ 2023-11-30 7:17 ` Patchwork 2023-11-30 20:00 ` [Intel-gfx] [Patch v3 1/2] " Rodrigo Vivi 4 siblings, 0 replies; 8+ messages in thread From: Patchwork @ 2023-11-30 7:17 UTC (permalink / raw) To: Matt Atwood; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 7458 bytes --] == Series Details == Series: series starting with [v3,1/2] drm/i915: Move reg_in_range_table URL : https://patchwork.freedesktop.org/series/127082/ State : failure == Summary == CI Bug Log - changes from CI_DRM_13951 -> Patchwork_127082v1 ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_127082v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_127082v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/index.html Participating hosts (36 -> 37) ------------------------------ Additional (3): bat-dg2-8 bat-kbl-2 bat-dg2-9 Missing (2): fi-snb-2520m bat-dg1-5 Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_127082v1: ### IGT changes ### #### Possible regressions #### * igt@i915_module_load@load: - bat-rpls-1: [PASS][1] -> [ABORT][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-rpls-1/igt@i915_module_load@load.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-rpls-1/igt@i915_module_load@load.html - bat-adlp-6: [PASS][3] -> [ABORT][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-adlp-6/igt@i915_module_load@load.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-adlp-6/igt@i915_module_load@load.html - fi-rkl-11600: [PASS][5] -> [ABORT][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/fi-rkl-11600/igt@i915_module_load@load.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/fi-rkl-11600/igt@i915_module_load@load.html - bat-adls-5: [PASS][7] -> [ABORT][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-adls-5/igt@i915_module_load@load.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-adls-5/igt@i915_module_load@load.html - bat-dg1-7: [PASS][9] -> [ABORT][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-dg1-7/igt@i915_module_load@load.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-dg1-7/igt@i915_module_load@load.html - bat-adlp-9: [PASS][11] -> [ABORT][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-adlp-9/igt@i915_module_load@load.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-adlp-9/igt@i915_module_load@load.html - bat-dg2-11: [PASS][13] -> [ABORT][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-dg2-11/igt@i915_module_load@load.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-dg2-11/igt@i915_module_load@load.html - bat-adln-1: [PASS][15] -> [ABORT][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-adln-1/igt@i915_module_load@load.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-adln-1/igt@i915_module_load@load.html - bat-adlm-1: [PASS][17] -> [ABORT][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-adlm-1/igt@i915_module_load@load.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-adlm-1/igt@i915_module_load@load.html - bat-rplp-1: [PASS][19] -> [ABORT][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-rplp-1/igt@i915_module_load@load.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-rplp-1/igt@i915_module_load@load.html - fi-tgl-1115g4: [PASS][21] -> [ABORT][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/fi-tgl-1115g4/igt@i915_module_load@load.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/fi-tgl-1115g4/igt@i915_module_load@load.html - bat-atsm-1: [PASS][23] -> [ABORT][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-atsm-1/igt@i915_module_load@load.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-atsm-1/igt@i915_module_load@load.html - bat-dg2-9: NOTRUN -> [ABORT][25] [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-dg2-9/igt@i915_module_load@load.html - bat-adlp-11: [PASS][26] -> [ABORT][27] [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-adlp-11/igt@i915_module_load@load.html [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-adlp-11/igt@i915_module_load@load.html - bat-dg2-8: NOTRUN -> [ABORT][28] [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-dg2-8/igt@i915_module_load@load.html #### Suppressed #### The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@i915_module_load@load: - {bat-dg2-13}: [PASS][29] -> [ABORT][30] [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-dg2-13/igt@i915_module_load@load.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-dg2-13/igt@i915_module_load@load.html - {bat-dg2-14}: [PASS][31] -> [ABORT][32] [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13951/bat-dg2-14/igt@i915_module_load@load.html [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-dg2-14/igt@i915_module_load@load.html Known issues ------------ Here are the changes found in Patchwork_127082v1 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@fbdev@info: - bat-kbl-2: NOTRUN -> [SKIP][33] ([fdo#109271] / [i915#1849]) [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-kbl-2/igt@fbdev@info.html * igt@gem_lmem_swapping@parallel-random-engines: - bat-kbl-2: NOTRUN -> [SKIP][34] ([fdo#109271]) +20 other tests skip [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-kbl-2/igt@gem_lmem_swapping@parallel-random-engines.html * igt@kms_pipe_crc_basic@read-crc-frame-sequence: - bat-kbl-2: NOTRUN -> [SKIP][35] ([fdo#109271] / [i915#1845]) +14 other tests skip [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/bat-kbl-2/igt@kms_pipe_crc_basic@read-crc-frame-sequence.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 [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845 [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849 Build changes ------------- * Linux: CI_DRM_13951 -> Patchwork_127082v1 CI-20190529: 20190529 CI_DRM_13951: 7bd342323d5bd405b02fd21cd78f157f363278ac @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7610: 7610 Patchwork_127082v1: 7bd342323d5bd405b02fd21cd78f157f363278ac @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 9c7f0e14fd6e drm/i915: Introduce Wa_1401127433 ba3630268a1c drm/i915: Move reg_in_range_table == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127082v1/index.html [-- Attachment #2: Type: text/html, Size: 8583 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [Patch v3 1/2] drm/i915: Move reg_in_range_table 2023-11-29 20:51 [Intel-gfx] [Patch v3 1/2] drm/i915: Move reg_in_range_table Matt Atwood ` (3 preceding siblings ...) 2023-11-30 7:17 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork @ 2023-11-30 20:00 ` Rodrigo Vivi 2023-12-01 22:07 ` Matt Roper 4 siblings, 1 reply; 8+ messages in thread From: Rodrigo Vivi @ 2023-11-30 20:00 UTC (permalink / raw) To: Matt Atwood; +Cc: intel-gfx On Wed, Nov 29, 2023 at 12:51:21PM -0800, Matt Atwood wrote: > Function reg_in_range_table is useful in more places, move function to > intel_uncore.h to make available to more places. > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > --- > drivers/gpu/drm/i915/i915_perf.c | 13 +------------ > drivers/gpu/drm/i915/intel_uncore.h | 12 ++++++++++++ > 2 files changed, 13 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index 7b1c8de2f9cb3..5e5dc73621379 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -219,6 +219,7 @@ > #include "i915_perf.h" > #include "i915_perf_oa_regs.h" > #include "i915_reg.h" > +#include "intel_uncore.h" > > /* HW requires this to be a power of two, between 128k and 16M, though driver > * is currently generally designed assuming the largest 16M size is used such > @@ -4324,18 +4325,6 @@ static bool gen8_is_valid_flex_addr(struct i915_perf *perf, u32 addr) > return false; > } > > -static bool reg_in_range_table(u32 addr, const struct i915_range *table) > -{ > - while (table->start || table->end) { > - if (addr >= table->start && addr <= table->end) > - return true; > - > - table++; > - } > - > - return false; > -} > - > #define REG_EQUAL(addr, mmio) \ > ((addr) == i915_mmio_reg_offset(mmio)) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h > index f419c311a0dea..1e85eaec1fc5a 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.h > +++ b/drivers/gpu/drm/i915/intel_uncore.h > @@ -496,6 +496,18 @@ static inline int intel_uncore_write_and_verify(struct intel_uncore *uncore, > return (reg_val & mask) != expected_val ? -EINVAL : 0; > } > > +static inline bool reg_in_range_table(u32 addr, const struct i915_range *table) exported functions should carry the name of the component that is exposing it. something like intel_uncore_reg_in_range() but also, based on your second patch, this doesn't seem to be the right thing to do. although I hate the i915_utils.h, that sounds like a better place for a function like this. or on a second thought... maybe just duplicate the logic that is inside this function if only one extra call... or maybe duplicate this function along the other table definition instead of having the table in one place and using the helper from another place. > +{ > + while (table->start || table->end) { > + if (addr >= table->start && addr <= table->end) > + return true; > + > + table++; > + } > + > + return false; > +} > + > static inline void __iomem *intel_uncore_regs(struct intel_uncore *uncore) > { > return uncore->regs; > -- > 2.40.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [Patch v3 1/2] drm/i915: Move reg_in_range_table 2023-11-30 20:00 ` [Intel-gfx] [Patch v3 1/2] " Rodrigo Vivi @ 2023-12-01 22:07 ` Matt Roper 0 siblings, 0 replies; 8+ messages in thread From: Matt Roper @ 2023-12-01 22:07 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: intel-gfx On Thu, Nov 30, 2023 at 03:00:59PM -0500, Rodrigo Vivi wrote: > On Wed, Nov 29, 2023 at 12:51:21PM -0800, Matt Atwood wrote: > > Function reg_in_range_table is useful in more places, move function to > > intel_uncore.h to make available to more places. > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > > --- > > drivers/gpu/drm/i915/i915_perf.c | 13 +------------ > > drivers/gpu/drm/i915/intel_uncore.h | 12 ++++++++++++ > > 2 files changed, 13 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > > index 7b1c8de2f9cb3..5e5dc73621379 100644 > > --- a/drivers/gpu/drm/i915/i915_perf.c > > +++ b/drivers/gpu/drm/i915/i915_perf.c > > @@ -219,6 +219,7 @@ > > #include "i915_perf.h" > > #include "i915_perf_oa_regs.h" > > #include "i915_reg.h" > > +#include "intel_uncore.h" > > > > /* HW requires this to be a power of two, between 128k and 16M, though driver > > * is currently generally designed assuming the largest 16M size is used such > > @@ -4324,18 +4325,6 @@ static bool gen8_is_valid_flex_addr(struct i915_perf *perf, u32 addr) > > return false; > > } > > > > -static bool reg_in_range_table(u32 addr, const struct i915_range *table) > > -{ > > - while (table->start || table->end) { > > - if (addr >= table->start && addr <= table->end) > > - return true; > > - > > - table++; > > - } > > - > > - return false; > > -} > > - > > #define REG_EQUAL(addr, mmio) \ > > ((addr) == i915_mmio_reg_offset(mmio)) > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h > > index f419c311a0dea..1e85eaec1fc5a 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.h > > +++ b/drivers/gpu/drm/i915/intel_uncore.h > > @@ -496,6 +496,18 @@ static inline int intel_uncore_write_and_verify(struct intel_uncore *uncore, > > return (reg_val & mask) != expected_val ? -EINVAL : 0; > > } > > > > +static inline bool reg_in_range_table(u32 addr, const struct i915_range *table) > > exported functions should carry the name of the component that is exposing it. > > something like intel_uncore_reg_in_range() > > but also, based on your second patch, this doesn't seem to be the right thing to do. > > although I hate the i915_utils.h, that sounds like a better place for a function like this. If the function leaves intel_uncore, we should probably move i915_range to wherever it ends up as well since that's defined in this header at the moment. Today intel_uncore is where pretty much all of the general register handling is, even though "uncore" is a bit of a misnomer for most of what we're actually doing. This function probably doesn't need to be an inline either. Just a normal function in a .c would probably be best. Matt > > or on a second thought... maybe just duplicate the logic that is inside this > function if only one extra call... or maybe duplicate this function along > the other table definition instead of having the table in one place and using > the helper from another place. > > > +{ > > + while (table->start || table->end) { > > + if (addr >= table->start && addr <= table->end) > > + return true; > > + > > + table++; > > + } > > + > > + return false; > > +} > > + > > static inline void __iomem *intel_uncore_regs(struct intel_uncore *uncore) > > { > > return uncore->regs; > > -- > > 2.40.1 > > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-12-01 22:07 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-11-29 20:51 [Intel-gfx] [Patch v3 1/2] drm/i915: Move reg_in_range_table Matt Atwood 2023-11-29 20:51 ` [Intel-gfx] [Patch v3 2/2] drm/i915: Introduce Wa_1401127433 Matt Atwood 2023-12-01 22:05 ` Matt Roper 2023-11-30 6:58 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/2] drm/i915: Move reg_in_range_table Patchwork 2023-11-30 6:58 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork 2023-11-30 7:17 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork 2023-11-30 20:00 ` [Intel-gfx] [Patch v3 1/2] " Rodrigo Vivi 2023-12-01 22:07 ` Matt Roper
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.