| Message ID | 20161107194957.3385-10-robert@sixbynine.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <robert@sixbynine.org> > --- > 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. > + > +/* 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); 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. > + > + /* 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, > + }, > {} > }; >
On Tue, Nov 8, 2016 at 6:19 AM, sourab gupta <sourab.gupta@intel.com> wrote: > 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 <robert@sixbynine.org> > > --- > > 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. > 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. > > > + > > +/* 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); > 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. > 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. > > > + > > + /* 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, > > + }, > > {} > > }; > > > > >
On Tue, 2016-11-08 at 03:47 -0800, Robert Bragg wrote: > > > On Tue, Nov 8, 2016 at 6:19 AM, sourab gupta <sourab.gupta@intel.com> > wrote: > 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 <robert@sixbynine.org> > > --- > > 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. > > > 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. > Ok. Seems fine then. > > > > + > > +/* 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); > > 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. > > > 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. > Seems reasonable. > > > > + > > + /* 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, > > + }, > > {} > > }; > > > The patch looks good to me. Reviewed-by: Sourab Gupta <sourab.gupta@intel.com> > > > >
On 7 November 2016 at 19:49, Robert Bragg <robert@sixbynine.org> 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 <robert@sixbynine.org> > --- > 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; > + > +/* 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); > + > + /* 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", This line is getting a little too long. Reviewed-by: Matthew Auld <matthew.auld@intel.com>
On Wed, Nov 9, 2016 at 7:52 PM, Matthew Auld <matthew.william.auld@gmail.com > wrote: > On 7 November 2016 at 19:49, Robert Bragg <robert@sixbynine.org> 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 <robert@sixbynine.org> > > --- > > 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; > > + > > +/* 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); > > + > > + /* 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", > This line is getting a little too long. > yeah, though the coding style has the exception for string constants so they remain grepable which can be useful too. Could maybe remove the bracketed naming of the corresponding sysctl parameter, though it does seem like pertinent/useful info if someone were to hit this limit. > > Reviewed-by: Matthew Auld <matthew.auld@intel.com> > thanks
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; + +/* 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); + + /* 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, + }, {} };
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 <robert@sixbynine.org> --- drivers/gpu/drm/i915/i915_perf.c | 61 ++++++++++++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 11 deletions(-)