Quoting Tvrtko Ursulin (2020-11-26 18:58:20)
>
> On 26/11/2020 16:56, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2020-11-26 16:47:03)
> >> -static unsigned int config_enabled_bit(u64 config)
> >> +static unsigned int is_tracked_config(const u64 config)
> >> {
> >> - if (is_engine_config(config))
> >> + unsigned int val;
> >
> >> +/**
> >> + * Non-engine events that we need to track enabled-disabled transition and
> >> + * current state.
> >> + */
> >
> > I'm not understanding what is_tracked_config() actually means and how
> > that becomes config_enabled_bit().
> >
> > These look like the non-engine ones where we interact with HW during the
> > sample.
> >
> > How do the events we define a bit for here differ from the "untracked"
> > events?
>
> Tracked means i915 pmu needs to track enabled/disabled transitions and
> state.
>
> So far frequency and rc6 needs that, due sampling timer decisions and
> park/unpark handling respectively.
>
> Interrupts on the contrary don't need to do any of that. We can just
> read the count at any given time.
>
> Is_tracked_config() for convenience returns a "bit + 1", so 0 means
> untracked.
>
> Every tracked event needs a slot in pmu->enabled and pmu->enable_count.
> The rest don't. Before this patch I had too many bits/array elements
> reserved there.
So my confusion boils down to 'enabled' in config_enabled_bit.
To me that makes it seem like it is a state, as opposed to the bit to be
used to track that state. (I feel enabled <-> tracked is quite clunky.)
config_enable_bit() is better, but I think you can just drop the
'enabled' altogether to leave
static unsigned int config_bit(u64 config)
{
if (is_engine_config(config))
return engine_config_bit(config);
if (is_tracked_config(config))
return tracked_config_bit(config);
return -1;
}
static u64 config_mask(u64 config)
{
return BIT_ULL(config_bit(config));
}
static unsigned int event_bit(struct perf_event *event)
{
return config_bit(event->attr.config);
}
unsigned int bit = event_bit(event);
Or if that is too much,
config_sample_bit()
config_sample_mask()
event_sample_bit()
?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
