Quoting Lionel Landwerlin (2018-09-06 10:57:47) > On 05/09/2018 15:22, Tvrtko Ursulin wrote: > > From: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx> > > > > If some of the contexts submitting workloads to the GPU have been > > configured to shutdown slices/subslices, we might loose the NOA > > configurations written in the NOA muxes. > > > > One possible solution to this problem is to reprogram the NOA muxes > > when we switch to a new context. We initially tried this in the > > workaround batchbuffer but some concerns where raised about the cost > > of reprogramming at every context switch. This solution is also not > > without consequences from the userspace point of view. Reprogramming > > of the muxes can only happen once the powergating configuration has > > changed (which happens after context switch). This means for a window > > of time during the recording, counters recorded by the OA unit might > > be invalid. This requires userspace dealing with OA reports to discard > > the invalid values. > > > > Minimizing the reprogramming could be implemented by tracking of the > > last programmed configuration somewhere in GGTT and use MI_PREDICATE > > to discard some of the programming commands, but the command streamer > > would still have to parse all the MI_LRI instructions in the > > workaround batchbuffer. > > > > Another solution, which this change implements, is to simply disregard > > the user requested configuration for the period of time when i915/perf > > is active. There is no known issue with this apart from a performance > > penality for some media workloads that benefit from running on a > > partially powergated GPU. We already prevent RC6 from affecting the > > programming so it doesn't sound completely unreasonable to hold on > > powergating for the same reason. > > > > v2: Leave RPCS programming in intel_lrc.c (Lionel) > > > > v3: Update for s/union intel_sseu/struct intel_sseu/ (Lionel) > > More to_intel_context() (Tvrtko) > > s/dev_priv/i915/ (Tvrtko) > > > > Tvrtko Ursulin: > > > > v4: > > * Rebase for make_rpcs changes. > > > > v5: > > * Apply OA restriction from make_rpcs directly. > > > > v6: > > * Rebase for context image setup changes. > > > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_perf.c | 5 +++++ > > drivers/gpu/drm/i915/intel_lrc.c | 30 ++++++++++++++++++++---------- > > drivers/gpu/drm/i915/intel_lrc.h | 3 +++ > > 3 files changed, 28 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > > index ccb20230df2c..dd65b72bddd4 100644 > > --- a/drivers/gpu/drm/i915/i915_perf.c > > +++ b/drivers/gpu/drm/i915/i915_perf.c > > @@ -1677,6 +1677,11 @@ static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx, > > > > CTX_REG(reg_state, state_offset, flex_regs[i], value); > > } > > + > > + CTX_REG(reg_state, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, > > + gen8_make_rpcs(dev_priv, > > + &to_intel_context(ctx, > > + dev_priv->engine[RCS])->sseu)); > > > I think there is one issue I missed on the previous iterations of this > patch. > > This gen8_update_reg_state_unlocked() is called when the GPU is parked > on the kernel context. > > It's supposed to update all contexts, but I think we might not be able > to update the kernel context image while the GPU is using it. The kernel context is only ever taken in extremis (you are either parking or stalling userspace) so I don't care. > Context save might happen after we edited the image and that would > override the values we just put in there. > > > The OA config is emitted through context image edition in this function > but also through the ring buffer in > gen8_switch_to_updated_kernel_context() for the kernel context. > > Since we can't have a context modify its own RCPS value, we'll have to > resort to yet another context to do that for the kernel context. > > > I remember having a patch that created yet another kernel context (let's > call it rpcs edition context), which is used to reconfigure rpcs for > every context but itself and then have the kernel context reconfigure > this rpcs edition context. > > Or alternatively not do anything to it, because it's only going to run > to edit other contexts at a time when we don't care about power > configuration stability. Exactly. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
