[Mesa-dev] [PATCH 2/4] panfrost: Expose perfcounters through the pipe_query interface

[Boris Brezillon]

On Thu, 4 Apr 2019 08:49:28 -0700
Alyssa Rosenzweig <alyssa at rosenzweig.io> wrote:

> > +	if (!screen->driver->create_perfcnt_query)
> > +		return NULL;  
> 
> Redundant -- it'll always be set since there's only DRM driver now. We
> should probably drop the indirection wholesale, too, but that's for a
> different patch. I guess if the DRM implementation isn't until next
> patch, it's still probably cleanest to just squash the two and hope for
> the best.

Will drop the indirect calls, and move the code to perfcnt.c (unless
you want to keep it in panfrost_drm.c).

> 
> > +	    screen->driver->destroy_perfcnt_query)  
> 
> """
> >          /* We need to flush out the jobs to actually run the counter, TODO
> >           * check wait, TODO wallpaper after if needed */
> >  
> >          panfrost_flush(pipe, NULL, PIPE_FLUSH_END_OF_FRAME);  
> 
> Does this flush apply to performance counters too?

We need to at least flush all jobs that have the perfmon pointed by
this query attached to them.

> 
> > +static int panfrost_get_query_group_info(struct pipe_screen *screen,
> > +					 unsigned index,
> > +					 struct pipe_driver_query_group_info *info)
> > +{
> > +	struct panfrost_screen *pscreen = pan_screen(screen);
> > +
> > +	if (!info)
> > +		return 1;  
> 
> Erm, semantically, what does this mean?

It returns the number of perf counter groups. I could declare 4 groups
(one per block), but I'm not convinced it's really useful here. The
main interest of counter groups is when you have a large amount
of counters inside a counter block but only a limited number of counters
can be enabled simultaneously. Declaring groups in that case helps the
user take a decision about which one he wants to enable, or force him
to replay several times the same operation each time with different
counters enabled (that's what apitrace does IIRC).