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

[Robert Bragg]

On Fri, Mar 24, 2017 at 12:52 AM, Robert Bragg <robert at sixbynine.org> wrote:
> 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.

*facepalm* - must have been on autopilot, sorry will fix.

>
>>
>>> +
>>> +                       /* 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