<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, Nov 8, 2016 at 6:19 AM, sourab gupta <<a href="mailto:sourab.gupta@intel.com" target="_blank">sourab.gupta@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, 2016-11-07 at 11:49 -0800, Robert Bragg wrote: > The maximum OA sampling frequency is now configurable via a > dev.i915.oa_max_sample_rate sysctl parameter. > > Following the precedent set by perf's similar > kernel.perf_event_max_sample_rate the default maximum rate is 100000Hz > > Signed-off-by: Robert Bragg <<a href="mailto:robert@sixbynine.org">robert@sixbynine.org</a>> > --- > drivers/gpu/drm/i915/i915_perf.c | 61 ++++++++++++++++++++++++++++++++-------- > 1 file changed, 50 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index e51c1d8..1a87fe9 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -82,6 +82,21 @@ static u32 i915_perf_stream_paranoid = true; > #define INVALID_CTX_ID 0xffffffff > > > +/* For sysctl proc_dointvec_minmax of i915_oa_max_sample_rate > + * > + * 160ns is the smallest sampling period we can theoretically program the OA > + * unit with on Haswell, corresponding to 6.25MHz. > + */ > +static int oa_sample_rate_hard_limit = 6250000; There's no check for 'oa_sample_rate_hard_limit' anywhere below. </blockquote><div> </div><div>It's in the struct ctl_table oa_table[] declaration of the "oa_max_sample_rate" paramater, assigned to .extra2 which is referenced by the proc_dointvec_minmax validation handler for the parameter. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div><div class="h5"> > + > +/* Theoretically we can program the OA unit to sample every 160ns but don't > + * allow that by default unless root... > + * > + * The default threshold of 100000Hz is based on perf's similar > + * kernel.perf_event_max_sample_rate sysctl parameter. > + */ > +static u32 i915_oa_max_sample_rate = 100000; > + > /* XXX: beware if future OA HW adds new report formats that the current > * code assumes all reports have a power-of-two size and ~(size - 1) can > * be used as a mask to align the OA tail pointer. > @@ -1314,6 +1329,7 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, > } > > for (i = 0; i < n_props; i++) { > + u64 oa_period, oa_freq_hz; > u64 id, value; > int ret; > > @@ -1359,21 +1375,35 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, > return -EINVAL; > } > > - /* NB: The exponent represents a period as follows: > - * > - * 80ns * 2^(period_exponent + 1) > - * > - * Theoretically we can program the OA unit to sample > + /* Theoretically we can program the OA unit to sample > * every 160ns but don't allow that by default unless > * root. > * > - * Referring to perf's > - * kernel.perf_event_max_sample_rate for a precedent > - * (100000 by default); with an OA exponent of 6 we get > - * a period of 10.240 microseconds -just under 100000Hz > + * On Haswell the period is derived from the exponent > + * as: > + * > + * period = 80ns * 2^(exponent + 1) > + */ > + BUILD_BUG_ON(sizeof(oa_period) != 8); > + oa_period = 80ull * (2ull << value); </div></div>I assume now that there'll be a platform specific check for 80ull, while programming oa_period, for subquent Gen8+ platforms, which should be fine. </blockquote><div> </div><div>Yeah, this code will need adapting for gen9+. I guess we'll change it to work in terms of ((2<<exp) * NSEC_PER_SEC) / timestamp_frequency. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5"> > + > + /* This check is primarily to ensure that oa_period <= > + * UINT32_MAX (before passing to do_div which only > + * accepts a u32 denominator), but we can also skip > + * checking anything < 1Hz which implicitly can't be > + * limited via an integer oa_max_sample_rate. > */ > - if (value < 6 && !capable(CAP_SYS_ADMIN)) { > - DRM_ERROR("Minimum OA sampling exponent is 6 without root privileges\n"); > + if (oa_period <= NSEC_PER_SEC) { > + u64 tmp = NSEC_PER_SEC; > + do_div(tmp, oa_period); > + oa_freq_hz = tmp; > + } else > + oa_freq_hz = 0; > + > + if (oa_freq_hz > i915_oa_max_sample_rate && > + !capable(CAP_SYS_ADMIN)) { > + DRM_ERROR("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without root privileges\n", > + i915_oa_max_sample_rate); > return -EACCES; > } > > @@ -1481,6 +1511,15 @@ static struct ctl_table oa_table[] = { > .extra1 = &zero, > .extra2 = &one, > }, > + { > + .procname = "oa_max_sample_rate", > + .data = &i915_oa_max_sample_rate, > + .maxlen = sizeof(i915_oa_max_sample_rate), > + .mode = 0644, > + .proc_handler = proc_dointvec_minmax, > + .extra1 = &zero, > + .extra2 = &oa_sample_rate_hard_limit, > + }, > {} > }; > </div></div></blockquote></div> </div></div>