[Intel-gfx] [PATCH 01/11] drm/i915/perf: Disable rc6 only while OA is enabled

[Lionel Landwerlin]

Hmm... nope, sorry.

We'll loose NOA configuration if you do that.
And you'll have to rerun the oa config BO prior to enabling.

-Lionel

On 09/10/2019 23:36, Chris Wilson wrote:
> Move rpm_get and forcewake_get into the perf enable (and corresponding
> the puts into the disable) so that we only prevent powermaangement while
> we OA is engaged.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_perf.c       | 49 ++++++++++++--------------
>   drivers/gpu/drm/i915/i915_perf_types.h | 12 +++----
>   2 files changed, 26 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 5a34cad7d824..6fa5c9dc38d3 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1348,9 +1348,6 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
>   
>   	free_oa_buffer(stream);
>   
> -	intel_uncore_forcewake_put(stream->gt->uncore, FORCEWAKE_ALL);
> -	intel_runtime_pm_put(stream->gt->uncore->rpm, stream->wakeref);
> -
>   	if (stream->ctx)
>   		oa_put_render_ctx_id(stream);
>   
> @@ -2192,21 +2189,6 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>   		goto err_config;
>   	}
>   
> -	/* PRM - observability performance counters:
> -	 *
> -	 *   OACONTROL, performance counter enable, note:
> -	 *
> -	 *   "When this bit is set, in order to have coherent counts,
> -	 *   RC6 power state and trunk clock gating must be disabled.
> -	 *   This can be achieved by programming MMIO registers as
> -	 *   0xA094=0 and 0xA090[31]=1"
> -	 *
> -	 *   In our case we are expecting that taking pm + FORCEWAKE
> -	 *   references will effectively disable RC6.
> -	 */
> -	stream->wakeref = intel_runtime_pm_get(stream->gt->uncore->rpm);
> -	intel_uncore_forcewake_get(stream->gt->uncore, FORCEWAKE_ALL);
> -
>   	ret = alloc_oa_buffer(stream);
>   	if (ret)
>   		goto err_oa_buf_alloc;
> @@ -2237,9 +2219,6 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>   err_oa_buf_alloc:
>   	put_oa_config(stream->oa_config);
>   
> -	intel_uncore_forcewake_put(stream->gt->uncore, FORCEWAKE_ALL);
> -	intel_runtime_pm_put(stream->gt->uncore->rpm, stream->wakeref);
> -
>   err_config:
>   	if (stream->ctx)
>   		oa_put_render_ctx_id(stream);
> @@ -2473,8 +2452,21 @@ static void i915_perf_enable_locked(struct i915_perf_stream *stream)
>   	if (stream->enabled)
>   		return;
>   
> -	/* Allow stream->ops->enable() to refer to this */
> -	stream->enabled = true;
> +	/*
> +	 * PRM - observability performance counters:
> +	 *
> +	 *   OACONTROL, performance counter enable, note:
> +	 *
> +	 *   "When this bit is set, in order to have coherent counts,
> +	 *   RC6 power state and trunk clock gating must be disabled.
> +	 *   This can be achieved by programming MMIO registers as
> +	 *   0xA094=0 and 0xA090[31]=1"
> +	 *
> +	 *   In our case we are expecting that taking pm + FORCEWAKE
> +	 *   references will effectively disable RC6.
> +	 */
> +	stream->enabled = intel_runtime_pm_get(stream->gt->uncore->rpm);
> +	intel_uncore_forcewake_get(stream->gt->uncore, FORCEWAKE_ALL);
>   
>   	if (stream->ops->enable)
>   		stream->ops->enable(stream);
> @@ -2496,14 +2488,17 @@ static void i915_perf_enable_locked(struct i915_perf_stream *stream)
>    */
>   static void i915_perf_disable_locked(struct i915_perf_stream *stream)
>   {
> -	if (!stream->enabled)
> -		return;
> +	intel_wakeref_t wakeref;
>   
> -	/* Allow stream->ops->disable() to refer to this */
> -	stream->enabled = false;
> +	wakeref = fetch_and_zero(&stream->enabled);
> +	if (!wakeref)
> +		return;
>   
>   	if (stream->ops->disable)
>   		stream->ops->disable(stream);
> +
> +	intel_uncore_forcewake_put(stream->gt->uncore, FORCEWAKE_ALL);
> +	intel_runtime_pm_put(stream->gt->uncore->rpm, wakeref);
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
> index 2d17059d32ee..edfd64732704 100644
> --- a/drivers/gpu/drm/i915/i915_perf_types.h
> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
> @@ -137,8 +137,11 @@ struct i915_perf_stream {
>   	/**
>   	 * @wakeref: As we keep the device awake while the perf stream is
>   	 * active, we track our runtime pm reference for later release.
> +	 * Also indicates whether the stream is currently enabled, considering
> +	 * whether the stream was opened in a disabled state and based
> +	 * on `I915_PERF_IOCTL_ENABLE` and `I915_PERF_IOCTL_DISABLE` calls.
>   	 */
> -	intel_wakeref_t wakeref;
> +	intel_wakeref_t enabled;
>   
>   	/**
>   	 * @sample_flags: Flags representing the `DRM_I915_PERF_PROP_SAMPLE_*`
> @@ -160,13 +163,6 @@ struct i915_perf_stream {
>   	 */
>   	struct i915_gem_context *ctx;
>   
> -	/**
> -	 * @enabled: Whether the stream is currently enabled, considering
> -	 * whether the stream was opened in a disabled state and based
> -	 * on `I915_PERF_IOCTL_ENABLE` and `I915_PERF_IOCTL_DISABLE` calls.
> -	 */
> -	bool enabled;
> -
>   	/**
>   	 * @ops: The callbacks providing the implementation of this specific
>   	 * type of configured stream.