[Intel-gfx] [PATCH 4/5] drm/i915: add a new perf configuration execbuf parameter

[Chris Wilson]

Quoting Lionel Landwerlin (2019-05-21 18:19:50)
> On 21/05/2019 18:07, Chris Wilson wrote:
> > Quoting Lionel Landwerlin (2019-05-21 15:08:54)
> >> +       if (eb->oa_config &&
> >> +           eb->oa_config != eb->i915->perf.oa.exclusive_stream->oa_config) {
> > But the oa_config is not ordered with respect to requests, and the
> > registers changed here are not context saved and so may be changed by a
> > third party before execution. The first party must presumably dropped
> > the perf_fd before then, so maybe you don't care? Hmm, doesn't even take
> > a third party as the perf_fd owner may change their mind for different
> > contexts and be surprised when the wrong set is used.
> 
> 
> The OA config batch should be ordered with regard to the MI_BBS we're 
> doing just below right?

But you only emit if the oa_config has changed. Ergo, it may have
changed between submission and execution.

> It's written before in the ring buffer.
> 
> 
> That essentially all we need so that as the perf queries run in the 
> batch supplied by the application runs with the configuration needed.
> 
> If the application shares the perf FD and someone else runs another 
> configuration, it's the application fault.
> 
> It needs to hold the perf FD for as long as it's doing perf queries and 
> not allow anybody else to interact with the OA configuration.

If one app is trying to use different configs on different contexts
(which seems reasonable if it is trying to sample different stats?) then
it can be caught out. The order of execution is not the same as the
order of submission (unless we enforce it by e.g. defining the
perf.oa_config as a barrier).


Another way would be to unconditionally emit the BB_START for the
oa_vma, and instead do the early exit with a MI_CONDITIONAL_BB_END by
comparing against a value stashed in the engine hwsp. You could do a
predicated BB_START instead, but that looks to be more of a nuisance.
-Chris