| Message ID | 20190627080045.8814-2-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Vulkan performance query support | expand |
Quoting Lionel Landwerlin (2019-06-27 09:00:36) > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Fixes: 19f81df2859eb1 ("drm/i915/perf: Add OA unit support for Gen 8+") > --- > drivers/gpu/drm/i915/i915_perf.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index d28a5bf80bd7..909e22835e84 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -1838,6 +1838,29 @@ static int gen8_enable_metric_set(struct i915_perf_stream *stream) > > config_oa_regs(dev_priv, oa_config->mux_regs, oa_config->mux_regs_len); > > + /* It apparently takes a fairly long time for a new MUX > + * configuration to be be applied after these register writes. > + * This delay duration was derived empirically based on the > + * render_basic config but hopefully it covers the maximum > + * configuration latency. > + * > + * As a fallback, the checks in _append_oa_reports() to skip > + * invalid OA reports do also seem to work to discard reports > + * generated before this config has completed - albeit not > + * silently. If you know the initial batch of reports is invalid after changing the register, why not just silently discard them until you see X valid reports? No gratuitous sleeps required, still a magic number (albeit it may be a small number). -Chris
On 27/06/2019 11:07, Chris Wilson wrote: > Quoting Lionel Landwerlin (2019-06-27 09:00:36) >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> Fixes: 19f81df2859eb1 ("drm/i915/perf: Add OA unit support for Gen 8+") >> --- >> drivers/gpu/drm/i915/i915_perf.c | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c >> index d28a5bf80bd7..909e22835e84 100644 >> --- a/drivers/gpu/drm/i915/i915_perf.c >> +++ b/drivers/gpu/drm/i915/i915_perf.c >> @@ -1838,6 +1838,29 @@ static int gen8_enable_metric_set(struct i915_perf_stream *stream) >> >> config_oa_regs(dev_priv, oa_config->mux_regs, oa_config->mux_regs_len); >> >> + /* It apparently takes a fairly long time for a new MUX >> + * configuration to be be applied after these register writes. >> + * This delay duration was derived empirically based on the >> + * render_basic config but hopefully it covers the maximum >> + * configuration latency. >> + * >> + * As a fallback, the checks in _append_oa_reports() to skip >> + * invalid OA reports do also seem to work to discard reports >> + * generated before this config has completed - albeit not >> + * silently. > If you know the initial batch of reports is invalid after changing the > register, why not just silently discard them until you see X valid > reports? No gratuitous sleeps required, still a magic number (albeit it > may be a small number). > -Chris > That would be nice, but unfortunately we don't really have a criteria to tell whether the values written in the snapshots are right or wrong. That's because the B/C counters in the snapshots are programmable. -Lionel
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index d28a5bf80bd7..909e22835e84 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1838,6 +1838,29 @@ static int gen8_enable_metric_set(struct i915_perf_stream *stream) config_oa_regs(dev_priv, oa_config->mux_regs, oa_config->mux_regs_len); + /* It apparently takes a fairly long time for a new MUX + * configuration to be be applied after these register writes. + * This delay duration was derived empirically based on the + * render_basic config but hopefully it covers the maximum + * configuration latency. + * + * As a fallback, the checks in _append_oa_reports() to skip + * invalid OA reports do also seem to work to discard reports + * generated before this config has completed - albeit not + * silently. + * + * Unfortunately this is essentially a magic number, since we + * don't currently know of a reliable mechanism for predicting + * how long the MUX config will take to apply and besides + * seeing invalid reports we don't know of a reliable way to + * explicitly check that the MUX config has landed. + * + * It's even possible we've miss characterized the underlying + * problem - it just seems like the simplest explanation why + * a delay at this location would mitigate any invalid reports. + */ + usleep_range(15000, 20000); + config_oa_regs(dev_priv, oa_config->b_counter_regs, oa_config->b_counter_regs_len);
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Fixes: 19f81df2859eb1 ("drm/i915/perf: Add OA unit support for Gen 8+") --- drivers/gpu/drm/i915/i915_perf.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)