On Thu, 4 Apr 2019 08:49:28 -0700
Alyssa Rosenzweig <[email protected]> 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).
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
