[Mesa-dev] [PATCH v2 1/7] nouveau: implement the nvif hardware performance counters interface
Martin Peres
martin.peres at free.fr
Wed Jul 22 13:29:38 PDT 2015
On 01/07/15 01:01, Samuel Pitoiset wrote:
> This commit implements the base interface for hardware performance
> counters that will be shared between nv50 and nvc0 drivers.
>
> TODO: Bump libdrm version of mesa when nvif will be merged.
>
> Changes since v2:
> - remove double-query thing for domains, signals and sources
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
> src/gallium/drivers/nouveau/Makefile.sources | 2 +
> src/gallium/drivers/nouveau/nouveau_perfmon.c | 290 ++++++++++++++++++++++++++
> src/gallium/drivers/nouveau/nouveau_perfmon.h | 58 ++++++
> src/gallium/drivers/nouveau/nouveau_screen.c | 5 +
> src/gallium/drivers/nouveau/nouveau_screen.h | 1 +
> 5 files changed, 356 insertions(+)
> create mode 100644 src/gallium/drivers/nouveau/nouveau_perfmon.c
> create mode 100644 src/gallium/drivers/nouveau/nouveau_perfmon.h
>
> diff --git a/src/gallium/drivers/nouveau/Makefile.sources b/src/gallium/drivers/nouveau/Makefile.sources
> index 3fae3bc..3da0bdc 100644
> --- a/src/gallium/drivers/nouveau/Makefile.sources
> +++ b/src/gallium/drivers/nouveau/Makefile.sources
> @@ -10,6 +10,8 @@ C_SOURCES := \
> nouveau_heap.h \
> nouveau_mm.c \
> nouveau_mm.h \
> + nouveau_perfmon.c \
> + nouveau_perfmon.h \
> nouveau_screen.c \
> nouveau_screen.h \
> nouveau_statebuf.h \
> diff --git a/src/gallium/drivers/nouveau/nouveau_perfmon.c b/src/gallium/drivers/nouveau/nouveau_perfmon.c
> new file mode 100644
> index 0000000..e1d4546
> --- /dev/null
> +++ b/src/gallium/drivers/nouveau/nouveau_perfmon.c
> @@ -0,0 +1,290 @@
> +/*
> + * Copyright 2015 Samuel Pitoiset
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <errno.h>
> +
> +#include "util/u_memory.h"
> +
> +#include "nouveau_debug.h"
> +#include "nouveau_winsys.h"
> +#include "nouveau_perfmon.h"
> +
> +static int
> +nouveau_perfmon_query_sources(struct nouveau_perfmon *pm,
> + struct nouveau_perfmon_dom *dom,
> + struct nouveau_perfmon_sig *sig)
> +{
> + struct nvif_perfmon_query_source_v0 args = {};
> +
> + args.iter = 1;
Why start iterating from 1 and not 0?
> + args.domain = dom->id;
> + args.signal = sig->signal;
> + do {
> + struct nouveau_perfmon_src *src;
> + int ret;
> +
> + ret = nouveau_object_mthd(pm->object, NVIF_PERFMON_V0_QUERY_SOURCE,
> + &args, sizeof(args));
> + if (ret)
> + return ret;
You do not check what happens if you do not expose any source for this
signal. A test on args.iter != 0xffff with a return if not the case
would be nice!
> +
> + src = CALLOC_STRUCT(nouveau_perfmon_src);
> + if (!src)
> + return -ENOMEM;
> +
> +#if 0
> + debug_printf("id = %d\n", args.source);
> + debug_printf("name = %s\n", args.name);
> + debug_printf("mask = %08x\n", args.mask);
> + debug_printf("\n");
> +#endif
> +
> + src->id = args.source;
> + strncpy(src->name, args.name, sizeof(src->name));
> + list_addtail(&src->head, &sig->sources);
> + } while (args.iter != 0xff);
> +
> + return 0;
> +}
> +
> +static int
> +nouveau_perfmon_query_signals(struct nouveau_perfmon *pm,
> + struct nouveau_perfmon_dom *dom)
> +{
> + struct nvif_perfmon_query_signal_v0 args = {};
> +
> + args.iter = 1;
> + args.domain = dom->id;
> + do {
> + struct nouveau_perfmon_sig *sig;
> + int ret;
> +
> + ret = nouveau_object_mthd(pm->object, NVIF_PERFMON_V0_QUERY_SIGNAL,
> + &args, sizeof(args));
> + if (ret)
> + return ret;
You do not check what happens if you do not expose any source for this
signal. A test on args.iter != 0xffff with a return if not the case
would be nice!
> +
> + sig = CALLOC_STRUCT(nouveau_perfmon_sig);
> + if (!sig)
> + return -ENOMEM;
> + list_inithead(&sig->sources);
> +
> +#if 0
> + debug_printf("name = %s\n", args.name);
> + debug_printf("signal = 0x%02x\n", args.signal);
> + debug_printf("source_nr = %d\n", args.source_nr);
> + debug_printf("\n");
> +#endif
> +
> + sig->signal = args.signal;
> + strncpy(sig->name, args.name, sizeof(sig->name));
> + list_addtail(&sig->head, &dom->signals);
> +
> + /* Query all available sources for this signal. */
> + if (args.source_nr > 0) {
> + ret = nouveau_perfmon_query_sources(pm, dom, sig);
> + if (ret)
> + return ret;
> + }
> + } while (args.iter != 0xffff);
> +
> + return 0;
> +}
> +
> +static int
> +nouveau_perfmon_query_domains(struct nouveau_perfmon *pm)
> +{
> + struct nvif_perfmon_query_domain_v0 args = {};
> + int ret;
> +
> + /* Before querying domains, check if the device exposes some of them. */
> + ret = nouveau_object_mthd(pm->object, NVIF_PERFMON_V0_QUERY_DOMAIN,
> + &args, sizeof(args));
> + if (ret)
> + return ret;
Why did you unrol this test? Delete the block above and move the block
under this comment right before the allocation of dom.
> +
> + if (args.iter == 0xff) {
> + /* No domains are exposed by the device. */
> + return 0;
> + }
> +
> + args.iter = 1;
> + do {
> + struct nouveau_perfmon_dom *dom;
> + int ret;
> +
> + ret = nouveau_object_mthd(pm->object, NVIF_PERFMON_V0_QUERY_DOMAIN,
> + &args, sizeof(args));
> + if (ret)
> + return ret;
> +
> + dom = CALLOC_STRUCT(nouveau_perfmon_dom);
> + if (!dom)
> + return -ENOMEM;
> + list_inithead(&dom->signals);
> +
> +#if 0
> + debug_printf("id = %d\n", args.id);
> + debug_printf("name = %s\n", args.name);
> + debug_printf("counter_nr = %d\n", args.counter_nr);
> + debug_printf("signal_nr = %d\n", args.signal_nr);
> + debug_printf("\n");
> +#endif
> +
> + dom->id = args.id;
> + dom->max_active_cntr = args.counter_nr;
> + strncpy(dom->name, args.name, sizeof(dom->name));
> + list_addtail(&dom->head, &pm->domains);
> +
> + /* Query all available signals for this domain. */
> + if (args.signal_nr > 0) {
> + ret = nouveau_perfmon_query_signals(pm, dom);
> + if (ret)
> + return ret;
> + }
> + } while (args.iter != 0xff);
> +
> + return 0;
> +}
> +
> +static void
> +nouveau_perfmon_free_sources(struct nouveau_perfmon_sig *sig)
> +{
> + struct nouveau_perfmon_src *src, *next;
> +
> + LIST_FOR_EACH_ENTRY_SAFE(src, next, &sig->sources, head) {
> + list_del(&src->head);
> + free(src);
> + }
> +}
> +
> +static void
> +nouveau_perfmon_free_signals(struct nouveau_perfmon_dom *dom)
> +{
> + struct nouveau_perfmon_sig *sig, *next;
> +
> + LIST_FOR_EACH_ENTRY_SAFE(sig, next, &dom->signals, head) {
> + nouveau_perfmon_free_sources(sig);
> + list_del(&sig->head);
> + free(sig);
> + }
> +}
> +
> +static void
> +nouveau_perfmon_free_domains(struct nouveau_perfmon *pm)
> +{
> + struct nouveau_perfmon_dom *dom, *next;
> +
> + LIST_FOR_EACH_ENTRY_SAFE(dom, next, &pm->domains, head) {
> + nouveau_perfmon_free_signals(dom);
> + list_del(&dom->head);
> + free(dom);
Free(dom), nice one :D
> + }
> +}
> +
> +struct nouveau_perfmon *
> +nouveau_perfmon_create(struct nouveau_device *dev)
> +{
> + struct nouveau_perfmon *pm = NULL;
> + int ret;
> +
> + pm = CALLOC_STRUCT(nouveau_perfmon);
> + if (!pm) {
> + NOUVEAU_ERR("Failed to allocate pm struct!\n");
> + return NULL;
> + }
> + list_inithead(&pm->domains);
> +
> + /* init handle for perfdom objects */
> + pm->handle = 0xbeef9751;
> +
> + ret = nouveau_object_new(&dev->object, 0xbeef9750,
Shouldn't those handles be in libdrm?
> + NVIF_IOCTL_NEW_V0_PERFMON, NULL, 0, &pm->object);
> + if (ret)
> + goto fail;
> +
> + /* Query all available domains, signals and sources for this device. */
> + ret = nouveau_perfmon_query_domains(pm);
> + if (ret)
> + goto fail;
> +
> + return pm;
> +
> +fail:
> + nouveau_perfmon_destroy(pm);
> + return NULL;
> +}
> +
> +void
> +nouveau_perfmon_destroy(struct nouveau_perfmon *pm)
> +{
> + if (!pm)
> + return;
> +
> + nouveau_perfmon_free_domains(pm);
> + nouveau_object_del(&pm->object);
> + FREE(pm);
> +}
> +
> +struct nouveau_perfmon_dom *
> +nouveau_perfmon_get_dom_by_id(struct nouveau_perfmon *pm, uint8_t dom_id)
> +{
> + struct nouveau_perfmon_dom *dom;
> +
> + if (pm) {
> + LIST_FOR_EACH_ENTRY(dom, &pm->domains, head) {
> + if (dom->id == dom_id)
> + return dom;
> + }
> + }
> + return NULL;
> +}
> +
> +struct nouveau_perfmon_sig *
> +nouveau_perfmon_get_sig_by_name(struct nouveau_perfmon_dom *dom,
> + const char *name)
> +{
> + struct nouveau_perfmon_sig *sig;
> +
> + if (dom) {
> + LIST_FOR_EACH_ENTRY(sig, &dom->signals, head) {
> + if (!strcmp(sig->name, name))
> + return sig;
> + }
> + }
> + return NULL;
> +}
> +
> +struct nouveau_perfmon_src *
> +nouveau_perfmon_get_src_by_name(struct nouveau_perfmon_sig *sig,
> + const char *name)
> +{
> + struct nouveau_perfmon_src *src;
> +
> + if (sig) {
> + LIST_FOR_EACH_ENTRY(src, &sig->sources, head) {
> + if (!strcmp(src->name, name))
> + return src;
> + }
> + }
> + return NULL;
> +}
> diff --git a/src/gallium/drivers/nouveau/nouveau_perfmon.h b/src/gallium/drivers/nouveau/nouveau_perfmon.h
> new file mode 100644
> index 0000000..c18addc
> --- /dev/null
> +++ b/src/gallium/drivers/nouveau/nouveau_perfmon.h
> @@ -0,0 +1,58 @@
> +#ifndef __NOUVEAU_PERFMON_H__
> +#define __NOUVEAU_PERFMON_H__
> +
> +#include <drm.h>
> +#include <nouveau_class.h>
> +#include <nouveau_ioctl.h>
> +
> +#include "util/list.h"
> +
> +struct nouveau_perfmon
> +{
> + struct nouveau_object *object;
> + struct list_head domains;
> + uint64_t handle;
> +};
> +
> +struct nouveau_perfmon_dom
> +{
> + struct list_head head;
> + struct list_head signals;
> + uint8_t id;
> + char name[64];
> + uint8_t max_active_cntr;
> +};
> +
> +struct nouveau_perfmon_sig
> +{
> + struct list_head head;
> + struct list_head sources;
> + uint8_t signal;
> + char name[64];
> +};
> +
> +struct nouveau_perfmon_src
> +{
> + struct list_head head;
> + uint8_t id;
> + char name[64];
> +};
> +
> +struct nouveau_perfmon *
> +nouveau_perfmon_create(struct nouveau_device *dev);
> +
> +void
> +nouveau_perfmon_destroy(struct nouveau_perfmon *pm);
> +
> +struct nouveau_perfmon_dom *
> +nouveau_perfmon_get_dom_by_id(struct nouveau_perfmon *pm, uint8_t dom_id);
> +
> +struct nouveau_perfmon_sig *
> +nouveau_perfmon_get_sig_by_name(struct nouveau_perfmon_dom *dom,
> + const char *name);
> +
> +struct nouveau_perfmon_src *
> +nouveau_perfmon_get_src_by_name(struct nouveau_perfmon_sig *sig,
> + const char *name);
> +
> +#endif /* __NOUVEAU_PERFMON_H__ */
> diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c b/src/gallium/drivers/nouveau/nouveau_screen.c
> index c6e5074..3c14a77 100644
> --- a/src/gallium/drivers/nouveau/nouveau_screen.c
> +++ b/src/gallium/drivers/nouveau/nouveau_screen.c
> @@ -21,6 +21,7 @@
> #include "nouveau_fence.h"
> #include "nouveau_mm.h"
> #include "nouveau_buffer.h"
> +#include "nouveau_perfmon.h"
>
> /* XXX this should go away */
> #include "state_tracker/drm_driver.h"
> @@ -226,6 +227,8 @@ nouveau_screen_init(struct nouveau_screen *screen, struct nouveau_device *dev)
> NOUVEAU_BO_GART | NOUVEAU_BO_MAP,
> &mm_config);
> screen->mm_VRAM = nouveau_mm_create(dev, NOUVEAU_BO_VRAM, &mm_config);
> +
> + screen->perfmon = nouveau_perfmon_create(dev);
> return 0;
> }
>
> @@ -235,6 +238,8 @@ nouveau_screen_fini(struct nouveau_screen *screen)
> nouveau_mm_destroy(screen->mm_GART);
> nouveau_mm_destroy(screen->mm_VRAM);
>
> + nouveau_perfmon_destroy(screen->perfmon);
> +
> nouveau_pushbuf_del(&screen->pushbuf);
>
> nouveau_client_del(&screen->client);
> diff --git a/src/gallium/drivers/nouveau/nouveau_screen.h b/src/gallium/drivers/nouveau/nouveau_screen.h
> index 30041b2..fd7ecdb 100644
> --- a/src/gallium/drivers/nouveau/nouveau_screen.h
> +++ b/src/gallium/drivers/nouveau/nouveau_screen.h
> @@ -21,6 +21,7 @@ struct nouveau_screen {
> struct nouveau_object *channel;
> struct nouveau_client *client;
> struct nouveau_pushbuf *pushbuf;
> + struct nouveau_perfmon *perfmon;
>
> int refcount;
>
With the checks in the allocations added, this is:
Reviewed-by: Martin Peres <martin.peres at free.fr>
More information about the mesa-dev
mailing list