Re: [PATCH 7/7] drm/i915/perf: add flushing ioctl

[Lionel Landwerlin <lionel.g.landwerlin@intel.com>]

On 22/01/2019 16:25, Joonas Lahtinen wrote:
> Quoting Lionel Landwerlin (2019-01-16 17:36:22)
>> With the currently available parameters for the i915-perf stream,
>> there are still situations that are not well covered :
>>
>> If an application opens the stream with polling disable or at very low
>> frequency and OA interrupt enabled, no data will be available even
>> though somewhere between nothing and half of the OA buffer worth of
>> data might have landed in memory.
>>
>> To solve this issue we have a new flush ioctl on the perf stream that
>> forces the i915-perf driver to look at the state of the buffer when
>> called and makes any data available through both poll() & read() type
>> syscalls.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Link to userspace changes?
>
> Regards, Joonas


Hey Joonas,


I'm about to make the changes in gputop for the high frequency sampling 
use case.


One thing I would like to know is whether these new features should be 
reported through a flag.

So far we haven't added any new option to the i915/perf driver since the 
initial upstreaming series.


The way I'm currently detecting newly added parameters is by using 
trying to open the stream with a value that I know will report ENOENT 
rather than EINVAL when the feature is not available :

https://github.com/djdeath/intel-gpu-tools/blob/wip/djdeath/oa-interrupts/tests/perf.c#L4345

Is there a better way to do this?


Thanks,


-Lionel


>
>> ---
>>   drivers/gpu/drm/i915/i915_perf.c | 17 +++++++++++++++++
>>   include/uapi/drm/i915_drm.h      | 19 +++++++++++++++++++
>>   2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index da721fce2543..6c98ffa2135e 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -2433,6 +2433,20 @@ static void i915_perf_disable_locked(struct i915_perf_stream *stream)
>>                  stream->ops->disable(stream);
>>   }
>>   
>> +/**
>> + * i915_perf_flush_data - handle `I915_PERF_IOCTL_FLUSH_DATA` ioctl
>> + * @stream: An enabled i915 perf stream
>> + *
>> + * The intention is to flush all the data available for reading from the OA
>> + * buffer
>> + */
>> +static void i915_perf_flush_data(struct i915_perf_stream *stream)
>> +{
>> +       struct drm_i915_private *dev_priv = stream->dev_priv;
>> +
>> +       dev_priv->perf.oa.pollin = oa_buffer_check(stream->dev_priv, true);
>> +}
>> +
>>   /**
>>    * i915_perf_ioctl - support ioctl() usage with i915 perf stream FDs
>>    * @stream: An i915 perf stream
>> @@ -2456,6 +2470,9 @@ static long i915_perf_ioctl_locked(struct i915_perf_stream *stream,
>>          case I915_PERF_IOCTL_DISABLE:
>>                  i915_perf_disable_locked(stream);
>>                  return 0;
>> +       case I915_PERF_IOCTL_FLUSH_DATA:
>> +               i915_perf_flush_data(stream);
>> +               return 0;
>>          }
>>   
>>          return -EINVAL;
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index b6db5e544a35..0f0d20080572 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -1594,6 +1594,25 @@ struct drm_i915_perf_open_param {
>>    */
>>   #define I915_PERF_IOCTL_DISABLE        _IO('i', 0x1)
>>   
>> +/**
>> + * Actively check the availability of data from a stream.
>> + *
>> + * A stream data availability can be driven by two types of events :
>> + *
>> + *   - if enabled, the kernel's hrtimer checking the amount of available data
>> + *     in the OA buffer through head/tail registers.
>> + *
>> + *   - if enabled, the OA unit's interrupt mechanism
>> + *
>> + * The kernel hrtimer incur a cost of running callback at fixed time
>> + * intervals, while the OA interrupt might only happen rarely. In the
>> + * situation where the application has disabled the kernel's hrtimer and only
>> + * uses the OA interrupt to know about available data, the application can
>> + * request an active check of the available OA data through this ioctl. This
>> + * will make any data in the OA buffer available with either poll() or read().
>> + */
>> +#define I915_PERF_IOCTL_FLUSH_DATA _IO('i', 0x2)
>> +
>>   /**
>>    * Common to all i915 perf records
>>    */
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx