| Message ID | 20191212184359.16280-1-fernando.pacheco@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/guc: Provide mmio list to be saved/restored on engine reset | expand |
On 12/12/2019 18:43, Fernando Pacheco wrote: > The driver must provide GuC with a list of mmio registers > that should be saved/restored during a GuC-based engine reset. > We provide a minimal set of registers that should get things > working and extend as needed. > > v2: rebase and comment to explain why mmio list is kept sorted > > Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 25 +++-- > .../gpu/drm/i915/gt/intel_workarounds_types.h | 1 + > drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 96 ++++++++++++++++++- > 3 files changed, 114 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 195ccf7db272..866d4a7ba0ea 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -148,29 +148,37 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) > } > > static void wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > - u32 val, u32 read_mask) > + u32 val, u32 read_mask, bool masked_bits) > { > struct i915_wa wa = { > .reg = reg, > .mask = mask, > .val = val, > .read = read_mask, > + .masked_bits = masked_bits, > }; > > _wa_add(wal, &wa); > } > > +static void > +__wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > + u32 val, bool masked_bits) > +{ > + wa_add(wal, reg, mask, val, mask, masked_bits); > +} > + > static void > wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > u32 val) > { > - wa_add(wal, reg, mask, val, mask); > + __wa_write_masked_or(wal, reg, mask, val, false); > } > > static void > wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val) > { > - wa_write_masked_or(wal, reg, val, _MASKED_BIT_ENABLE(val)); > + __wa_write_masked_or(wal, reg, val, _MASKED_BIT_ENABLE(val), true); > } > > static void > @@ -186,13 +194,16 @@ wa_write_or(struct i915_wa_list *wal, i915_reg_t reg, u32 val) > } > > #define WA_SET_BIT_MASKED(addr, mask) \ > - wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_ENABLE(mask)) > + __wa_write_masked_or(wal, (addr), (mask), \ > + _MASKED_BIT_ENABLE(mask), true) > > #define WA_CLR_BIT_MASKED(addr, mask) \ > - wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_DISABLE(mask)) > + __wa_write_masked_or(wal, (addr), (mask), \ > + _MASKED_BIT_DISABLE(mask), true) > > #define WA_SET_FIELD_MASKED(addr, mask, value) \ > - wa_write_masked_or(wal, (addr), (mask), _MASKED_FIELD((mask), (value))) > + __wa_write_masked_or(wal, (addr), (mask), \ > + _MASKED_FIELD((mask), (value)), true) > > static void gen8_ctx_workarounds_init(struct intel_engine_cs *engine, > struct i915_wa_list *wal) > @@ -592,7 +603,7 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine, > */ > wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val, > IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 : > - FF_MODE2_TDS_TIMER_MASK); > + FF_MODE2_TDS_TIMER_MASK, false); > } > > static void > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h > index e27ab1b710b3..a43d5f968f2d 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h > @@ -16,6 +16,7 @@ struct i915_wa { > u32 mask; > u32 val; > u32 read; > + bool masked_bits; > }; > > struct i915_wa_list { > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > index 101728006ae9..3fea13fc2b1a 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c > @@ -3,6 +3,8 @@ > * Copyright © 2014-2019 Intel Corporation > */ > > +#include <linux/bsearch.h> > + > #include "gt/intel_gt.h" > #include "intel_guc_ads.h" > #include "intel_uc.h" > @@ -16,6 +18,9 @@ > * its internal state for sleep. > */ > > +static void guc_mmio_reg_state_init(struct guc_mmio_reg_state *reg_state, > + struct intel_engine_cs *engine); > + > static void guc_policy_init(struct guc_policy *policy) > { > policy->execution_quantum = POLICY_DEFAULT_EXECUTION_QUANTUM_US; > @@ -67,12 +72,19 @@ struct __guc_ads_blob { > > static void __guc_ads_init(struct intel_guc *guc) > { > - struct drm_i915_private *dev_priv = guc_to_gt(guc)->i915; > + struct intel_gt *gt = guc_to_gt(guc); > + struct drm_i915_private *dev_priv = gt->i915; > struct __guc_ads_blob *blob = guc->ads_blob; > const u32 skipped_size = LRC_PPHWSP_SZ * PAGE_SIZE + LR_HW_CONTEXT_SIZE; > + struct intel_engine_cs *engine; > + enum intel_engine_id id; > u32 base; > u8 engine_class; > > + /* GuC mmio save/restore list */ > + for_each_engine(engine, gt, id) > + guc_mmio_reg_state_init(&blob->reg_state, engine); > + > /* GuC scheduling policies */ > guc_policies_init(&blob->policies); > > @@ -170,3 +182,85 @@ void intel_guc_ads_reset(struct intel_guc *guc) > return; > __guc_ads_init(guc); > } > + > +static int guc_mmio_reg_cmp(const void *a, const void *b) > +{ > + const struct guc_mmio_reg *ra = a; > + const struct guc_mmio_reg *rb = b; > + > + return (int)ra->offset - (int)rb->offset; > +} > + > +static void guc_mmio_reg_add(struct guc_mmio_regset *regset, > + u32 offset, u32 flags) > +{ > + u32 count = regset->number_of_registers; > + struct guc_mmio_reg reg = { > + .offset = offset, > + .flags = flags, > + }; > + struct guc_mmio_reg *slot; > + > + GEM_BUG_ON(count >= GUC_REGSET_MAX_REGISTERS); > + > + /* > + * The mmio list is built using separate lists within the driver. > + * It's possible that at some point we may attempt to add the same > + * register more than once. Do not consider this an error; silently > + * move on if the register is already in the list. > + */ > + if (bsearch(®, regset->registers, count, > + sizeof(reg), guc_mmio_reg_cmp)) > + return; > + > + slot = ®set->registers[count]; > + regset->number_of_registers++; > + *slot = reg; > + > + while (slot-- > regset->registers) { > + GEM_BUG_ON(slot[0].offset == slot[1].offset); > + if (slot[1].offset > slot[0].offset) > + break; > + > + swap(slot[1], slot[0]); > + } > +} > + > +#define GUC_MMIO_REG_ADD(regset, reg, masked) \ > + guc_mmio_reg_add(regset, \ > + i915_mmio_reg_offset((reg)), \ > + (masked) ? GUC_REGSET_MASKED : 0) > + > +static void guc_mmio_regset_init(struct guc_mmio_regset *regset, > + struct intel_engine_cs *engine) > +{ > + const u32 base = engine->mmio_base; > + struct i915_wa_list *wal = &engine->wa_list; > + struct i915_wa *wa; > + unsigned int i; > + > + GUC_MMIO_REG_ADD(regset, RING_MODE_GEN7(base), true); > + GUC_MMIO_REG_ADD(regset, RING_HWS_PGA(base), false); > + GUC_MMIO_REG_ADD(regset, RING_IMR(base), false); > + > + for (i = 0, wa = wal->list; i < wal->count; i++, wa++) > + GUC_MMIO_REG_ADD(regset, wa->reg, wa->masked_bits); > + > + /* Be extra paranoid and include all whitelist registers. */ > + for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) > + GUC_MMIO_REG_ADD(regset, > + RING_FORCE_TO_NONPRIV(base, i), > + false); > +} > + > +static void guc_mmio_reg_state_init(struct guc_mmio_reg_state *reg_state, > + struct intel_engine_cs *engine) > +{ > + struct guc_mmio_regset *regset; > + > + GEM_BUG_ON(engine->class >= GUC_MAX_ENGINE_CLASSES); > + GEM_BUG_ON(engine->instance >= GUC_MAX_INSTANCES_PER_CLASS); > + regset = ®_state->engine_reg[engine->class][engine->instance]; > + > + guc_mmio_regset_init(regset, engine); > +} > So presumably the relevant GuC FW interface structures will remain the same from upstream to new interface? Assuming that, the code looks fine so: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> But it is dead code for now, right? Dead in the sense GuC engine reset path does not get exercised until more new patches land. Regards, Tvrtko
On 12/12/19 3:01 PM, Tvrtko Ursulin wrote: > > On 12/12/2019 18:43, Fernando Pacheco wrote: >> The driver must provide GuC with a list of mmio registers >> that should be saved/restored during a GuC-based engine reset. >> We provide a minimal set of registers that should get things >> working and extend as needed. >> >> v2: rebase and comment to explain why mmio list is kept sorted >> >> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> --- >> drivers/gpu/drm/i915/gt/intel_workarounds.c | 25 +++-- >> .../gpu/drm/i915/gt/intel_workarounds_types.h | 1 + >> drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 96 ++++++++++++++++++- >> 3 files changed, 114 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c >> b/drivers/gpu/drm/i915/gt/intel_workarounds.c >> index 195ccf7db272..866d4a7ba0ea 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c >> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c >> @@ -148,29 +148,37 @@ static void _wa_add(struct i915_wa_list *wal, >> const struct i915_wa *wa) >> } >> static void wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, >> - u32 val, u32 read_mask) >> + u32 val, u32 read_mask, bool masked_bits) >> { >> struct i915_wa wa = { >> .reg = reg, >> .mask = mask, >> .val = val, >> .read = read_mask, >> + .masked_bits = masked_bits, >> }; >> _wa_add(wal, &wa); >> } >> +static void >> +__wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, >> + u32 val, bool masked_bits) >> +{ >> + wa_add(wal, reg, mask, val, mask, masked_bits); >> +} >> + >> static void >> wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, >> u32 val) >> { >> - wa_add(wal, reg, mask, val, mask); >> + __wa_write_masked_or(wal, reg, mask, val, false); >> } >> static void >> wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val) >> { >> - wa_write_masked_or(wal, reg, val, _MASKED_BIT_ENABLE(val)); >> + __wa_write_masked_or(wal, reg, val, _MASKED_BIT_ENABLE(val), true); >> } >> static void >> @@ -186,13 +194,16 @@ wa_write_or(struct i915_wa_list *wal, i915_reg_t >> reg, u32 val) >> } >> #define WA_SET_BIT_MASKED(addr, mask) \ >> - wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_ENABLE(mask)) >> + __wa_write_masked_or(wal, (addr), (mask), \ >> + _MASKED_BIT_ENABLE(mask), true) >> #define WA_CLR_BIT_MASKED(addr, mask) \ >> - wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_DISABLE(mask)) >> + __wa_write_masked_or(wal, (addr), (mask), \ >> + _MASKED_BIT_DISABLE(mask), true) >> #define WA_SET_FIELD_MASKED(addr, mask, value) \ >> - wa_write_masked_or(wal, (addr), (mask), _MASKED_FIELD((mask), >> (value))) >> + __wa_write_masked_or(wal, (addr), (mask), \ >> + _MASKED_FIELD((mask), (value)), true) >> static void gen8_ctx_workarounds_init(struct intel_engine_cs *engine, >> struct i915_wa_list *wal) >> @@ -592,7 +603,7 @@ static void tgl_ctx_workarounds_init(struct >> intel_engine_cs *engine, >> */ >> wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val, >> IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 : >> - FF_MODE2_TDS_TIMER_MASK); >> + FF_MODE2_TDS_TIMER_MASK, false); >> } >> static void >> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h >> b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h >> index e27ab1b710b3..a43d5f968f2d 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h >> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h >> @@ -16,6 +16,7 @@ struct i915_wa { >> u32 mask; >> u32 val; >> u32 read; >> + bool masked_bits; >> }; >> struct i915_wa_list { >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c >> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c >> index 101728006ae9..3fea13fc2b1a 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c >> @@ -3,6 +3,8 @@ >> * Copyright © 2014-2019 Intel Corporation >> */ >> +#include <linux/bsearch.h> >> + >> #include "gt/intel_gt.h" >> #include "intel_guc_ads.h" >> #include "intel_uc.h" >> @@ -16,6 +18,9 @@ >> * its internal state for sleep. >> */ >> +static void guc_mmio_reg_state_init(struct guc_mmio_reg_state >> *reg_state, >> + struct intel_engine_cs *engine); >> + >> static void guc_policy_init(struct guc_policy *policy) >> { >> policy->execution_quantum = POLICY_DEFAULT_EXECUTION_QUANTUM_US; >> @@ -67,12 +72,19 @@ struct __guc_ads_blob { >> static void __guc_ads_init(struct intel_guc *guc) >> { >> - struct drm_i915_private *dev_priv = guc_to_gt(guc)->i915; >> + struct intel_gt *gt = guc_to_gt(guc); >> + struct drm_i915_private *dev_priv = gt->i915; >> struct __guc_ads_blob *blob = guc->ads_blob; >> const u32 skipped_size = LRC_PPHWSP_SZ * PAGE_SIZE + >> LR_HW_CONTEXT_SIZE; >> + struct intel_engine_cs *engine; >> + enum intel_engine_id id; >> u32 base; >> u8 engine_class; >> + /* GuC mmio save/restore list */ >> + for_each_engine(engine, gt, id) >> + guc_mmio_reg_state_init(&blob->reg_state, engine); >> + >> /* GuC scheduling policies */ >> guc_policies_init(&blob->policies); >> @@ -170,3 +182,85 @@ void intel_guc_ads_reset(struct intel_guc *guc) >> return; >> __guc_ads_init(guc); >> } >> + >> +static int guc_mmio_reg_cmp(const void *a, const void *b) >> +{ >> + const struct guc_mmio_reg *ra = a; >> + const struct guc_mmio_reg *rb = b; >> + >> + return (int)ra->offset - (int)rb->offset; >> +} >> + >> +static void guc_mmio_reg_add(struct guc_mmio_regset *regset, >> + u32 offset, u32 flags) >> +{ >> + u32 count = regset->number_of_registers; >> + struct guc_mmio_reg reg = { >> + .offset = offset, >> + .flags = flags, >> + }; >> + struct guc_mmio_reg *slot; >> + >> + GEM_BUG_ON(count >= GUC_REGSET_MAX_REGISTERS); >> + >> + /* >> + * The mmio list is built using separate lists within the driver. >> + * It's possible that at some point we may attempt to add the same >> + * register more than once. Do not consider this an error; silently >> + * move on if the register is already in the list. >> + */ >> + if (bsearch(®, regset->registers, count, >> + sizeof(reg), guc_mmio_reg_cmp)) >> + return; >> + >> + slot = ®set->registers[count]; >> + regset->number_of_registers++; >> + *slot = reg; >> + >> + while (slot-- > regset->registers) { >> + GEM_BUG_ON(slot[0].offset == slot[1].offset); >> + if (slot[1].offset > slot[0].offset) >> + break; >> + >> + swap(slot[1], slot[0]); >> + } >> +} >> + >> +#define GUC_MMIO_REG_ADD(regset, reg, masked) \ >> + guc_mmio_reg_add(regset, \ >> + i915_mmio_reg_offset((reg)), \ >> + (masked) ? GUC_REGSET_MASKED : 0) >> + >> +static void guc_mmio_regset_init(struct guc_mmio_regset *regset, >> + struct intel_engine_cs *engine) >> +{ >> + const u32 base = engine->mmio_base; >> + struct i915_wa_list *wal = &engine->wa_list; >> + struct i915_wa *wa; >> + unsigned int i; >> + >> + GUC_MMIO_REG_ADD(regset, RING_MODE_GEN7(base), true); >> + GUC_MMIO_REG_ADD(regset, RING_HWS_PGA(base), false); >> + GUC_MMIO_REG_ADD(regset, RING_IMR(base), false); >> + >> + for (i = 0, wa = wal->list; i < wal->count; i++, wa++) >> + GUC_MMIO_REG_ADD(regset, wa->reg, wa->masked_bits); >> + >> + /* Be extra paranoid and include all whitelist registers. */ >> + for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) >> + GUC_MMIO_REG_ADD(regset, >> + RING_FORCE_TO_NONPRIV(base, i), >> + false); >> +} >> + >> +static void guc_mmio_reg_state_init(struct guc_mmio_reg_state >> *reg_state, >> + struct intel_engine_cs *engine) >> +{ >> + struct guc_mmio_regset *regset; >> + >> + GEM_BUG_ON(engine->class >= GUC_MAX_ENGINE_CLASSES); >> + GEM_BUG_ON(engine->instance >= GUC_MAX_INSTANCES_PER_CLASS); >> + regset = ®_state->engine_reg[engine->class][engine->instance]; >> + >> + guc_mmio_regset_init(regset, engine); >> +} >> > > So presumably the relevant GuC FW interface structures will remain the > same from upstream to new interface? The structures are the same in the latest GuC drop. I've heard that there is a discussion ongoing about potentially making the list size dynamic at some point (instead of always statically allocating 64 entries for all possible engines), but even with that the bulk of this patch should still be ok. Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> I believe we need a selftest or some other logic in the driver to make sure the register list stays up to date with what we program during engine init, but we can add that as a follow up. Daniele > > Assuming that, the code looks fine so: > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > But it is dead code for now, right? Dead in the sense GuC engine reset > path does not get exercised until more new patches land. > > Regards, > > Tvrtko > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 195ccf7db272..866d4a7ba0ea 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -148,29 +148,37 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) } static void wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, - u32 val, u32 read_mask) + u32 val, u32 read_mask, bool masked_bits) { struct i915_wa wa = { .reg = reg, .mask = mask, .val = val, .read = read_mask, + .masked_bits = masked_bits, }; _wa_add(wal, &wa); } +static void +__wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, + u32 val, bool masked_bits) +{ + wa_add(wal, reg, mask, val, mask, masked_bits); +} + static void wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val) { - wa_add(wal, reg, mask, val, mask); + __wa_write_masked_or(wal, reg, mask, val, false); } static void wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val) { - wa_write_masked_or(wal, reg, val, _MASKED_BIT_ENABLE(val)); + __wa_write_masked_or(wal, reg, val, _MASKED_BIT_ENABLE(val), true); } static void @@ -186,13 +194,16 @@ wa_write_or(struct i915_wa_list *wal, i915_reg_t reg, u32 val) } #define WA_SET_BIT_MASKED(addr, mask) \ - wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_ENABLE(mask)) + __wa_write_masked_or(wal, (addr), (mask), \ + _MASKED_BIT_ENABLE(mask), true) #define WA_CLR_BIT_MASKED(addr, mask) \ - wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_DISABLE(mask)) + __wa_write_masked_or(wal, (addr), (mask), \ + _MASKED_BIT_DISABLE(mask), true) #define WA_SET_FIELD_MASKED(addr, mask, value) \ - wa_write_masked_or(wal, (addr), (mask), _MASKED_FIELD((mask), (value))) + __wa_write_masked_or(wal, (addr), (mask), \ + _MASKED_FIELD((mask), (value)), true) static void gen8_ctx_workarounds_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) @@ -592,7 +603,7 @@ static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine, */ wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val, IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 : - FF_MODE2_TDS_TIMER_MASK); + FF_MODE2_TDS_TIMER_MASK, false); } static void diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h index e27ab1b710b3..a43d5f968f2d 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds_types.h +++ b/drivers/gpu/drm/i915/gt/intel_workarounds_types.h @@ -16,6 +16,7 @@ struct i915_wa { u32 mask; u32 val; u32 read; + bool masked_bits; }; struct i915_wa_list { diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c index 101728006ae9..3fea13fc2b1a 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c @@ -3,6 +3,8 @@ * Copyright © 2014-2019 Intel Corporation */ +#include <linux/bsearch.h> + #include "gt/intel_gt.h" #include "intel_guc_ads.h" #include "intel_uc.h" @@ -16,6 +18,9 @@ * its internal state for sleep. */ +static void guc_mmio_reg_state_init(struct guc_mmio_reg_state *reg_state, + struct intel_engine_cs *engine); + static void guc_policy_init(struct guc_policy *policy) { policy->execution_quantum = POLICY_DEFAULT_EXECUTION_QUANTUM_US; @@ -67,12 +72,19 @@ struct __guc_ads_blob { static void __guc_ads_init(struct intel_guc *guc) { - struct drm_i915_private *dev_priv = guc_to_gt(guc)->i915; + struct intel_gt *gt = guc_to_gt(guc); + struct drm_i915_private *dev_priv = gt->i915; struct __guc_ads_blob *blob = guc->ads_blob; const u32 skipped_size = LRC_PPHWSP_SZ * PAGE_SIZE + LR_HW_CONTEXT_SIZE; + struct intel_engine_cs *engine; + enum intel_engine_id id; u32 base; u8 engine_class; + /* GuC mmio save/restore list */ + for_each_engine(engine, gt, id) + guc_mmio_reg_state_init(&blob->reg_state, engine); + /* GuC scheduling policies */ guc_policies_init(&blob->policies); @@ -170,3 +182,85 @@ void intel_guc_ads_reset(struct intel_guc *guc) return; __guc_ads_init(guc); } + +static int guc_mmio_reg_cmp(const void *a, const void *b) +{ + const struct guc_mmio_reg *ra = a; + const struct guc_mmio_reg *rb = b; + + return (int)ra->offset - (int)rb->offset; +} + +static void guc_mmio_reg_add(struct guc_mmio_regset *regset, + u32 offset, u32 flags) +{ + u32 count = regset->number_of_registers; + struct guc_mmio_reg reg = { + .offset = offset, + .flags = flags, + }; + struct guc_mmio_reg *slot; + + GEM_BUG_ON(count >= GUC_REGSET_MAX_REGISTERS); + + /* + * The mmio list is built using separate lists within the driver. + * It's possible that at some point we may attempt to add the same + * register more than once. Do not consider this an error; silently + * move on if the register is already in the list. + */ + if (bsearch(®, regset->registers, count, + sizeof(reg), guc_mmio_reg_cmp)) + return; + + slot = ®set->registers[count]; + regset->number_of_registers++; + *slot = reg; + + while (slot-- > regset->registers) { + GEM_BUG_ON(slot[0].offset == slot[1].offset); + if (slot[1].offset > slot[0].offset) + break; + + swap(slot[1], slot[0]); + } +} + +#define GUC_MMIO_REG_ADD(regset, reg, masked) \ + guc_mmio_reg_add(regset, \ + i915_mmio_reg_offset((reg)), \ + (masked) ? GUC_REGSET_MASKED : 0) + +static void guc_mmio_regset_init(struct guc_mmio_regset *regset, + struct intel_engine_cs *engine) +{ + const u32 base = engine->mmio_base; + struct i915_wa_list *wal = &engine->wa_list; + struct i915_wa *wa; + unsigned int i; + + GUC_MMIO_REG_ADD(regset, RING_MODE_GEN7(base), true); + GUC_MMIO_REG_ADD(regset, RING_HWS_PGA(base), false); + GUC_MMIO_REG_ADD(regset, RING_IMR(base), false); + + for (i = 0, wa = wal->list; i < wal->count; i++, wa++) + GUC_MMIO_REG_ADD(regset, wa->reg, wa->masked_bits); + + /* Be extra paranoid and include all whitelist registers. */ + for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) + GUC_MMIO_REG_ADD(regset, + RING_FORCE_TO_NONPRIV(base, i), + false); +} + +static void guc_mmio_reg_state_init(struct guc_mmio_reg_state *reg_state, + struct intel_engine_cs *engine) +{ + struct guc_mmio_regset *regset; + + GEM_BUG_ON(engine->class >= GUC_MAX_ENGINE_CLASSES); + GEM_BUG_ON(engine->instance >= GUC_MAX_INSTANCES_PER_CLASS); + regset = ®_state->engine_reg[engine->class][engine->instance]; + + guc_mmio_regset_init(regset, engine); +}
The driver must provide GuC with a list of mmio registers that should be saved/restored during a GuC-based engine reset. We provide a minimal set of registers that should get things working and extend as needed. v2: rebase and comment to explain why mmio list is kept sorted Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 25 +++-- .../gpu/drm/i915/gt/intel_workarounds_types.h | 1 + drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 96 ++++++++++++++++++- 3 files changed, 114 insertions(+), 8 deletions(-)