[Mesa-dev] [PATCH 1/3] st/mesa: Make an enum for pipeline statistics query result indices.

[Ilia Mirkin]

On Sat, Dec 15, 2018 at 4:12 PM Kenneth Graunke <kenneth at whitecape.org> wrote:
>
> On Saturday, December 15, 2018 9:10:46 AM PST Ilia Mirkin wrote:
> > On Sat, Dec 15, 2018 at 4:45 AM Kenneth Graunke <kenneth at whitecape.org> wrote:
> > > Gallium handles pipeline statistics queries as a single query
> > > (PIPE_QUERY_PIPELINE_STATISTICS) which returns a struct with 11 values.
> > > Sometimes it's useful to refer to each of those values individually,
> > > rather than as a group.  To avoid hardcoding numbers, we define a new
> > > enum for each value.  Here, the name and enum value correspond to the
> > > index in the struct pipe_query_data_pipeline_statistics result.
> >
> > This later-in-the-series desire to be able to get just one value back
> > from Gallium is an API change which would break any existing d3d1x
> > state trackers. I realize you're not changing any drivers, but in my
> > mind, it's preferable not to have ambiguous APIs where some drivers do
> > one thing, others do another. For NVIDIA, we have to fetch the
> > counters individually too, but we just do them all. It's ~free to do
> > so, and we only do one set of synchronization for all of them.
>
> Yes, I suppose you're right in that the existing interface is designed
> to return all 11 counters, and I'm essesntially implementing it wrong
> because I didn't understand it.  It seemed like more of an inconsistency
> in get_query_results vs get_query_results_resource, where one of those
> knew what value to fetch, and the other did not.

I implemented the direct-write-to-resource logic, and there was no
use-case for fetching them all, and you had to support a GL API where
you had to be able to place each value into a precise location. I
didn't want to alter the other API at the time as I didn't feel it
hurt too much.

>
> While it may not be that expensive to return 11 values, it isn't free.
> The ARB_pipeline_statistics_query extension is designed to help port
> D3D1x apps to OpenGL, but it provides GL-style single-value queries
> rather than a single query that returns a struct.  So, if a D3D1x
> translator naively tries to fetch all 11 counters, it would have to
> do 11 different GL queries...each of which would map to Gallium's
> return-all-11 interface...resulting in 121 counter reads.

Yep. Not ideal. (Which was never my argument anyways -- having a way
to return a single value would definitely be better.)

>
> > Anyways, I'd rather not have this ambiguous thing of "you could return
> > some or all" situation. We should be more definitive. I'd recommend
> > adding a PIPE_QUERY_PIPELINE_STATISTICS_ONE to differentiate [or
> > PIPE_QUERY_PIPELINE_STATISTIC if you want to be clever and cause lots
> > of typos], along with a CAP for support.
>
> I'd like to have an interface that better serves the in-tree state
> trackers.  st/mesa wants 1 value, but it seems st/nine wants 2.  So
> the single interface isn't ideal for nine, either, I guess.

Historically we've tried to avoid creating breakage for VMware where
it wasn't necessary.

>
> If people would prefer that we add a new query type and capability bit,
> I can do that, but I probably won't bother implementing the all-11 mode
> in my driver, then.

I think that's fine. It's already behind some PIPE_CAP. The state
tracker could then do like

if ONE: get_one();
if ALL: get_all(); select one();

Cheers,

  -ilia