On Thu, 12 Mar 2020 16:05:02 -0700, Umesh Nerlige Ramappa wrote:
>
> From: Lionel Landwerlin <lionel.g.landwer...@intel.com>
>
> This new parameter let's the application choose how often the OA
> buffer should be checked on the CPU side for data availability. Longer
> polling period tend to reduce CPU overhead if the application does not
> care about somewhat real time data collection.
Lionely already has comments on this so skipping those.
> diff --git a/drivers/gpu/drm/i915/i915_perf.c
> b/drivers/gpu/drm/i915/i915_perf.c
> index 21a63644846f..ca139ac31b11 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -262,11 +262,11 @@
> */
> #define OA_TAIL_MARGIN_NSEC 100000ULL
>
> -/* frequency for checking whether the OA unit has written new reports to the
> - * circular OA buffer...
> +/* The default frequency for checking whether the OA unit has written new
> + * reports to the circular OA buffer...
> */
> -#define POLL_FREQUENCY 200
> -#define POLL_PERIOD (NSEC_PER_SEC / POLL_FREQUENCY)
> +#define DEFAULT_POLL_FREQUENCY 200
> +#define DEFAULT_POLL_PERIOD (NSEC_PER_SEC / DEFAULT_POLL_FREQUENCY)
nit, but I mostly like to have "units" as part of the name, so I'd recommed
calling this DEFAULT_POLL_PERIOD_NS.
> @@ -349,6 +349,8 @@ static const struct i915_oa_format
> gen12_oa_formats[I915_OA_FORMAT_MAX] = {
> * @oa_periodic: Whether to enable periodic OA unit sampling
> * @oa_period_exponent: The OA unit sampling period is derived from this
> * @engine: The engine (typically rcs0) being monitored by the OA unit
> + * @poll_oa_period: The period at which the CPU will check for OA data
> + * availability
> *
> * As read_properties_unlocked() enumerates and validates the properties
> given
> * to open a stream of metrics the configuration is built up in the structure
> @@ -368,6 +370,7 @@ struct perf_open_properties {
> int oa_period_exponent;
>
> struct intel_engine_cs *engine;
> + u64 poll_oa_period;
poll_oa_period_ns?
> @@ -2642,9 +2645,9 @@ static void i915_oa_stream_enable(struct
> i915_perf_stream *stream)
>
> stream->perf->ops.oa_enable(stream);
>
> - if (stream->periodic)
> + if (stream->periodic && stream->poll_oa_period)
poll_oa_period check is not required, it's always present.
> @@ -3617,6 +3625,14 @@ static int read_properties_unlocked(struct i915_perf
> *perf,
> case DRM_I915_PERF_PROP_HOLD_PREEMPTION:
> props->hold_preemption = !!value;
> break;
> + case DRM_I915_PERF_PROP_POLL_OA_DELAY:
> + if (value < 100000 /* 100us */) {
So no maximum? That's probably ok, reap what you sow. Have we tested 100
us, seems low, anyway.
> diff --git a/drivers/gpu/drm/i915/i915_perf_types.h
> b/drivers/gpu/drm/i915/i915_perf_types.h
> index 9ee7c58e70d5..01559ead22e2 100644
> --- a/drivers/gpu/drm/i915/i915_perf_types.h
> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
> @@ -304,6 +304,12 @@ struct i915_perf_stream {
> * reprogrammed.
> */
> struct i915_vma *noa_wait;
> +
> + /**
> + * @poll_oa_period: The period in nanoseconds at which the OA
> + * buffer should be checked for available data.
> + */
> + u64 poll_oa_period;
poll_oa_period_ns?
> };
>
> /**
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 2813e579b480..dd511e7f795d 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1969,6 +1969,19 @@ enum drm_i915_perf_property_id {
> */
> DRM_I915_PERF_PROP_HOLD_PREEMPTION,
>
> + /**
> + * This optional parameter specifies the timer interval in nanoseconds
> + * at which the i915 driver will check the OA buffer for available data.
> + * Minimum allowed value is 100 microseconds. A default value is used by
> + * the driver if this parameter is not specified. Note that a large
> + * value may reduce cpu consumption during OA perf captures, but it
> + * would also potentially result in OA buffer overwrite as the captures
> + * reach end of the OA buffer.
> + *
> + * This property is available in perf revision 4.
> + */
> + DRM_I915_PERF_PROP_POLL_OA_DELAY,
Is DRM_I915_PERF_PROP_POLL_OA_PERIOD_NS a better name? At least PERIOD
instead of DELAY.
Thanks!
--
Ashutosh
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
