[Intel-gfx] [PATCH 6/7] drm/i915/perf: add interrupt enabling parameter

[Lionel Landwerlin]

On 04/03/2020 07:47, Dixit, Ashutosh wrote:
> On Tue, 03 Mar 2020 14:19:04 -0800, Umesh Nerlige Ramappa wrote:
>> From: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>>
>> This let's the application choose to be driven by the interrupt
>> mechanism of the HW. In conjuction with long periods for checks for
>> the availability of data on the CPU, this can reduce the CPU load when
>> doing capture of OA data.
>>
>> v2: Version the new parameter (Joonas)
>> v3: Rebase (Umesh)
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
> [snip]
>
>> +	/**
>> +	 * Specifying this property sets up the interrupt mechanism for the OA
>> +	 * buffer in i915. This option in conjuction with a long polling delay
>> +	 * for avaibility of OA data can reduce CPU load significantly if you
>> +	 * do not care about OA data being read as soon as it's available.
>> +	 *
>> +	 * This property is available in perf revision 5.
>> +	 */
>> +	DRM_I915_PERF_PROP_OA_ENABLE_INTERRUPT,
> What if we do not expose this parameter in the uapi at all and internally
> decide in i915 whether to leave the interrupt either always enabled or
> always disabled (and in that case always use the hrtimer)? This way we
> retain flexibility in i915 if hardware evolves in the future e.g. to use
> watermarks for the interrupt, without yielding control to userspace.
>
> Overall I feel we should avoid exposing unnecessary details of the internal
> implemenation to userspace, they would be neither interested in knowing
> internal details nor know how to properly use these parameters. Shouldn't
> the driver be able to make these kinds of decisions internally?
>
> At this point the only parameter which implicitly exposed to userspace is
> the hrtimer poll period, so perhaps all we need to do is to expose that in
> the uapi? Thoughts?


I guess I agree with you. I can't remember why I exposed it to userspace.

There might be one test that checks the stream reports LOST_BUFFER with 
no poll() wakeup, but I guess we could update it.


-Lionel