[Intel-gfx] [PATCH v3 8/9] drm/i915/perf: Add engine class instance parameters to perf

[Umesh Nerlige Ramappa]

On Wed, Mar 01, 2023 at 01:59:10PM -0800, Dixit, Ashutosh wrote:
>On Mon, 27 Feb 2023 18:23:28 -0800, Umesh Nerlige Ramappa wrote:
>>
>
>Hi Umesh,
>
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index 8df261c5ab9b..8ce20004a9dd 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -2758,6 +2758,28 @@ enum drm_i915_perf_property_id {
>>	 */
>>	DRM_I915_PERF_PROP_POLL_OA_PERIOD,
>>
>> +	/**
>> +	 * In platforms with multiple OA buffers, the engine class instance must
>
>I'd say "one engine class instance".
>
>> +	 * be passed to open a stream to a OA unit corresponding to the engine.
>> +	 * Multiple engines may be mapped to the same OA unit.
>
>Maybe (at the risk of over-stating) something like "Multiple engines may be
>mapped to the same OA unit. The OA unit is identified by class:instance of
>any engine mapped to it".
>
>But it's ok to not change anything here.
>
>> +	 *
>> +	 * In addition to the class:instance, if a gem context is also passed, then
>> +	 * 1) the report headers of OA reports from any contexts that do not
>> +	 *    match this specific engine context are squashed.
>
>This one I don't understand: we seem to be mixing gem contexts and engine
>context here. Also, afais there are no changes related to this in this
>series. This context squashing has always been happening, so why add this
>comment to DRM_I915_PERF_PROP_OA_ENGINE_CLASS (or to this patch)? If we
>want to clarify something maybe this comment should be added to
>DRM_I915_PERF_PROP_CTX_HANDLE? But otherwise I think we should just drop
>this comment, at least from this patch?
>
>> +	 * 2) if the engine supports MI_REPORT_PERF_COUNT, this specific engine
>> +	 *    context is configured for this command.

I think I will drop 1 and 2 since they are not specifically related to 
the class:instance parameters.

Thanks,
Umesh

>> +	 *
>> +	 * This property is available in perf revision 6.
>> +	 */
>> +	DRM_I915_PERF_PROP_OA_ENGINE_CLASS,
>> +
>> +	/**
>> +	 * This parameter specifies the engine instance.
>> +	 *
>> +	 * This property is available in perf revision 6.
>> +	 */
>> +	DRM_I915_PERF_PROP_OA_ENGINE_INSTANCE,
>> +
>>	DRM_I915_PERF_PROP_MAX /* non-ABI */
>>  };
>
>Thanks.
>--
>Ashutosh