[Intel-gfx] [PATCH v8 6/6] drm/i915/perf: Implement I915_PERF_ADD/REMOVE_CONFIG interface

[Lionel Landwerlin]

On 31/07/17 18:36, Matthew Auld wrote:
> On 28 July 2017 at 18:10, Lionel Landwerlin
> <lionel.g.landwerlin at intel.com> wrote:
>> The motivation behind this new interface is expose at runtime the
>> creation of new OA configs which can be used as part of the i915 perf
>> open interface. This will enable the kernel to learn new configs which
>> may be experimental, or otherwise not part of the core set currently
>> available through the i915 perf interface.
>>
>> v2: Drop DRM_ERROR for userspace errors (Matthew)
>>      Add padding to userspace structure (Matthew)
>>      s/guid/uuid/ (Matthew)
>>
>> v3: Use u32 instead of int to iterate through registers (Matthew)
>>
>> v4: Lock access to dynamic config list (Lionel)
>>
>> v5: by Matthew:
>>      Fix uninitialized error values
>>      Fix incorrect unwiding when opening perf stream
>>      Use kmalloc_array() to store register
>>      Use uuid_is_valid() to valid config uuids
>>      Declare ioctls as write only
>>      Check padding members are set to 0
>>      by Lionel:
>>      Return ENOENT rather than EINVAL when trying to remove non
>>      existing config
>>
>> v6: by Chris:
>>      Use ref counts for OA configs
>>      Store UUID in drm_i915_perf_oa_config rather then using pointer
>>      Shuffle fields of drm_i915_perf_oa_config to avoid padding
>>
>> v7: by Chris
>>      Rename uapi pointers fields to end with '_ptr'
>>
>> v8: by Andrzej, Marek, Sebastian
>>      Update register whitelisting
>>      by Lionel
>>      Add more register names for documentation
>>      Allow configuration programming in non-paranoid mode
>>      Add support for value filter for a couple of registers already
>>      programmed in other part of the kernel
>>
>> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> Signed-off-by: Andrzej Datczuk <andrzej.datczuk at intel.com>
>> ---
>>   Documentation/gpu/i915.rst       |   4 +
>>   drivers/gpu/drm/i915/i915_drv.c  |   2 +
>>   drivers/gpu/drm/i915/i915_drv.h  |  47 ++++
>>   drivers/gpu/drm/i915/i915_perf.c | 468 +++++++++++++++++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/i915_reg.h  |  70 +++++-
>>   include/uapi/drm/i915_drm.h      |  20 ++
>>   6 files changed, 594 insertions(+), 17 deletions(-)
>>
>> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
>> index 9c7ed3e3f1e9..46875c2bcc31 100644
>> --- a/Documentation/gpu/i915.rst
>> +++ b/Documentation/gpu/i915.rst
>> @@ -417,6 +417,10 @@ integrate with drm/i915 and to handle the `DRM_I915_PERF_OPEN` ioctl.
>>      :functions: i915_perf_open_ioctl
>>   .. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
>>      :functions: i915_perf_release
>> +.. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
>> +   :functions: i915_perf_add_config_ioctl
>> +.. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
>> +   :functions: i915_perf_remove_config_ioctl
> While you're here, fancy fixing the kernel doc for
> i915_perf_ioctl_locked and gen7_append_oa_reports as a follow up
> patch?
>
> Also I don't think I can r-b this one since it has my s-o-b, so we'll
> have to rope someone else in to giving it an r-b.
>

Should I add all of the utility functions too? (like append_oa_status, 
oa_buffer_check_unlocked, append_oa_sample, etc...)

I feel like gen7_*/gen8_* are kind of internal things, not sure how that 
should surface in the documentation...