[Intel-gfx] [PATCH v2 3/5] drm/i915: Add 'render basic' Gen8+ OA unit configs

[Robert Bragg]

On Thu, Mar 23, 2017 at 8:48 PM, Matthew Auld
<matthew.william.auld at gmail.com> wrote:
> On 23 March 2017 at 20:18, Robert Bragg <robert at sixbynine.org> wrote:
>> Adds a static OA unit, MUX, B Counter + Flex EU configurations for basic
>> render metrics on Broadwell, Cherryview, Skylake and Broxton. These are
>> auto generated from an XML description of metric sets, currently
>> maintained in gputop, ref:
>>
>>  https://github.com/rib/gputop
>>  > gputop-data/oa-*.xml
>>  > scripts/i915-perf-kernelgen.py
>>
>>  $ make -C gputop-data -f Makefile.xml WHITELIST=RenderBasic
>>
>> Signed-off-by: Robert Bragg <robert at sixbynine.org>
>
> <SNIP>
>
>> +
>> +int i915_oa_select_metric_set_bdw(struct drm_i915_private *dev_priv)
>> +{
>> +       dev_priv->perf.oa.mux_regs = NULL;
>> +       dev_priv->perf.oa.mux_regs_len = 0;
>> +       dev_priv->perf.oa.b_counter_regs = NULL;
>> +       dev_priv->perf.oa.b_counter_regs_len = 0;
>> +       dev_priv->perf.oa.flex_regs = NULL;
>> +       dev_priv->perf.oa.flex_regs_len = 0;
>> +
>> +       switch (dev_priv->perf.oa.metrics_set) {
>> +       case METRIC_SET_ID_RENDER_BASIC:
>> +               dev_priv->perf.oa.mux_regs =
>> +                       get_render_basic_mux_config(dev_priv,
>> +                                                   &dev_priv->perf.oa.mux_regs_len);
>> +               if (!dev_priv->perf.oa.mux_regs) {
>> +                       DRM_DEBUG_DRIVER("No suitable MUX config for \"RENDER_BASIC\" metric set");
> You forgot to update your script ;)

Hmm, that's odd; I didn't:
https://github.com/rib/gputop/commit/95bc05957d488e2004ef2e7b5ba5b33d7dd559dd

So need to double check what's up here.

>
>> +
>> +                       /* EINVAL because *_register_sysfs already checked this
>> +                        * and so it wouldn't have been advertised so userspace and
>> +                        * so shouldn't have been requested
> s/so userspace/to userspace/ ?

Ah yup.

>
> Otherwise assuming the configs are indeed correct:
> Reviewed-by: Matthew Auld <matthew.auld at intel.com>

Thanks