dri-devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling
@ 2024-05-12 22:23 Maíra Canal
  2024-05-12 22:23 ` [PATCH v2 1/6] drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1 Maíra Canal
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Maíra Canal @ 2024-05-12 22:23 UTC (permalink / raw
  To: Melissa Wen, Iago Toral, Jose Maria Casanova Crespo,
	Juan A . Suárez, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter
  Cc: dri-devel, kernel-dev, Maíra Canal

This series has the intention to address two issues with Performance Counters
on V3D:

1. Update the number of Performance Counters for V3D 7.1 
	
V3D 7.1 has 93 performance counters, while V3D 4.2 has only 87. Although the
series [1] enabled support for V3D 7.1, it didn’t replace the maximum number of
performance counters. This led to errors in user space as the Vulkan driver
updated the maximum number of performance counters, but the kernel didn’t. 
    
Currently, the user space can request values for performance counters that
are greater than 87 and the kernel will return an error instead of the values.
That’s why `dEQP-VK.query_pool.performance_query.*` currently fails on Mesa
CI [2]. This series intends to fix the `dEQP-VK.query_pool.performance_query.*`
fail.
    
2. Make the kernel able to provide the Performance Counter descriptions
    
Although all the management of the Performance Monitors is done through IOCTLs,
which means that the code is in the kernel, the performance counter descriptions
are in Mesa. This means two things: (#1) only Mesa has access to the descriptions
and (#2) we can have inconsistencies between the information provided by Mesa
and the kernel, as seen in the first issue addressed by this series.
	
To minimize the risk of inconsistencies, this series proposes to use the kernel
as a “single source of truth”. Therefore, if there are any changes to the
performance monitors, all the changes must be done only in the kernel. This
means that all information about the maximum number of performance counters and
all the descriptions will now be retrieved from the kernel. 

This series is coupled with a Mesa series [3] that enabled the use of the new
IOCTL. I appreciate any feedback from both the kernel and Mesa implementations.

[1] https://lore.kernel.org/dri-devel/20231031073859.25298-1-itoral@igalia.com/
[2] https://gitlab.freedesktop.org/mesa/mesa/-/commit/ea1f09a5f21839f4f3b93610b58507c4bd9b9b81
[3] https://gitlab.freedesktop.org/mairacanal/mesa/-/tree/v3dv/fix-perfcnt

Best Regards,
- Maíra Canal

---

v1 -> v2: https://lore.kernel.org/dri-devel/20240508143306.2435304-2-mcanal@igalia.com/T/

* [5/6] s/DRM_V3D_PARAM_V3D_MAX_PERF_COUNTERS/DRM_V3D_PARAM_MAX_PERF_COUNTERS (Iago Toral)
* [6/6] Include a reference to the new DRM_V3D_PARAM_MAX_PERF_COUNTERS param (Iago Toral)
* Add Iago's R-b (Iago Toral)

Maíra Canal (6):
  drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1
  drm/v3d: Different V3D versions can have different number of perfcnt
  drm/v3d: Create a new V3D parameter for the maximum number of perfcnt
  drm/v3d: Create new IOCTL to expose performance counters information
  drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM
  drm/v3d: Deprecate the use of the Performance Counters enum

 drivers/gpu/drm/v3d/v3d_drv.c                 |  11 +
 drivers/gpu/drm/v3d/v3d_drv.h                 |  14 +-
 drivers/gpu/drm/v3d/v3d_perfmon.c             |  36 ++-
 .../gpu/drm/v3d/v3d_performance_counters.h    | 208 ++++++++++++++++++
 drivers/gpu/drm/v3d/v3d_sched.c               |   2 +-
 include/uapi/drm/v3d_drm.h                    |  48 ++++
 6 files changed, 316 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/v3d/v3d_performance_counters.h

-- 
2.44.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/6] drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1
  2024-05-12 22:23 [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
@ 2024-05-12 22:23 ` Maíra Canal
  2024-05-12 22:23 ` [PATCH v2 2/6] drm/v3d: Different V3D versions can have different number of perfcnt Maíra Canal
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2024-05-12 22:23 UTC (permalink / raw
  To: Melissa Wen, Iago Toral, Jose Maria Casanova Crespo,
	Juan A . Suárez, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter
  Cc: dri-devel, kernel-dev, Maíra Canal

Add name, category and description for each one of the 93 performance
counters available on V3D.

Note that V3D 4.2 has 87 performance counters, while V3D 7.1 has 93.
Therefore, there are two performance counters arrays. The index of the
performance counter for each V3D version is represented by its position
on the array.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_drv.h                 |   2 +
 .../gpu/drm/v3d/v3d_performance_counters.h    | 208 ++++++++++++++++++
 2 files changed, 210 insertions(+)
 create mode 100644 drivers/gpu/drm/v3d/v3d_performance_counters.h

diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index a2c516fe6d79..671375a3bb66 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -11,6 +11,8 @@
 #include <drm/drm_gem_shmem_helper.h>
 #include <drm/gpu_scheduler.h>
 
+#include "v3d_performance_counters.h"
+
 #include "uapi/drm/v3d_drm.h"
 
 struct clk;
diff --git a/drivers/gpu/drm/v3d/v3d_performance_counters.h b/drivers/gpu/drm/v3d/v3d_performance_counters.h
new file mode 100644
index 000000000000..72822205ebdc
--- /dev/null
+++ b/drivers/gpu/drm/v3d/v3d_performance_counters.h
@@ -0,0 +1,208 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2024 Raspberry Pi
+ */
+#ifndef V3D_PERFORMANCE_COUNTERS_H
+#define V3D_PERFORMANCE_COUNTERS_H
+
+/* Holds a description of a given performance counter. The index of performance
+ * counter is given by the array on v3d_performance_counter.h
+ */
+struct v3d_perf_counter_desc {
+	/* Category of the counter */
+	char category[32];
+
+	/* Name of the counter */
+	char name[64];
+
+	/* Description of the counter */
+	char description[256];
+};
+
+static const struct v3d_perf_counter_desc v3d_v71_performance_counters[] = {
+	{"CORE", "cycle-count", "[CORE] Cycle counter"},
+	{"CORE", "core-active", "[CORE] Bin/Render/Compute active cycles"},
+	{"CLE", "CLE-bin-thread-active-cycles", "[CLE] Bin thread active cycles"},
+	{"CLE", "CLE-render-thread-active-cycles", "[CLE] Render thread active cycles"},
+	{"CORE", "compute-active-cycles", "[CORE] Compute active cycles"},
+	{"FEP", "FEP-valid-primitives-no-rendered-pixels", "[FEP] Valid primitives that result in no rendered pixels, for all rendered tiles"},
+	{"FEP", "FEP-valid-primitives-rendered-pixels", "[FEP] Valid primitives for all rendered tiles (primitives may be counted in more than one tile)"},
+	{"FEP", "FEP-clipped-quads", "[FEP] Early-Z/Near/Far clipped quads"},
+	{"FEP", "FEP-valid-quads", "[FEP] Valid quads"},
+	{"TLB", "TLB-quads-not-passing-stencil-test", "[TLB] Quads with no pixels passing the stencil test"},
+	{"TLB", "TLB-quads-not-passing-z-and-stencil-test", "[TLB] Quads with no pixels passing the Z and stencil tests"},
+	{"TLB", "TLB-quads-passing-z-and-stencil-test", "[TLB] Quads with any pixels passing the Z and stencil tests"},
+	{"TLB", "TLB-quads-written-to-color-buffer", "[TLB] Quads with valid pixels written to colour buffer"},
+	{"TLB", "TLB-partial-quads-written-to-color-buffer", "[TLB] Partial quads written to the colour buffer"},
+	{"PTB", "PTB-primitives-need-clipping", "[PTB] Primitives that need clipping"},
+	{"PTB", "PTB-primitives-discarded-outside-viewport", "[PTB] Primitives discarded by being outside the viewport"},
+	{"PTB", "PTB-primitives-binned", "[PTB] Total primitives binned"},
+	{"PTB", "PTB-primitives-discarded-reversed", "[PTB] Primitives that are discarded because they are reversed"},
+	{"QPU", "QPU-total-instr-cache-hit", "[QPU] Total instruction cache hits for all slices"},
+	{"QPU", "QPU-total-instr-cache-miss", "[QPU] Total instruction cache misses for all slices"},
+	{"QPU", "QPU-total-uniform-cache-hit", "[QPU] Total uniforms cache hits for all slices"},
+	{"QPU", "QPU-total-uniform-cache-miss", "[QPU] Total uniforms cache misses for all slices"},
+	{"TMU", "TMU-active-cycles", "[TMU] Active cycles"},
+	{"TMU", "TMU-stalled-cycles", "[TMU] Stalled cycles"},
+	{"TMU", "TMU-total-text-quads-access", "[TMU] Total texture cache accesses"},
+	{"TMU", "TMU-cache-x4-active-cycles", "[TMU] Cache active cycles for x4 access"},
+	{"TMU", "TMU-cache-x4-stalled-cycles", "[TMU] Cache stalled cycles for x4 access"},
+	{"TMU", "TMU-total-text-quads-x4-access", "[TMU] Total texture cache x4 access"},
+	{"L2T", "L2T-total-cache-hit", "[L2T] Total Level 2 cache hits"},
+	{"L2T", "L2T-total-cache-miss", "[L2T] Total Level 2 cache misses"},
+	{"L2T", "L2T-local", "[L2T] Local mode access"},
+	{"L2T", "L2T-writeback", "[L2T] Writeback"},
+	{"L2T", "L2T-zero", "[L2T] Zero"},
+	{"L2T", "L2T-merge", "[L2T] Merge"},
+	{"L2T", "L2T-fill", "[L2T] Fill"},
+	{"L2T", "L2T-stalls-no-wid", "[L2T] Stalls because no WID available"},
+	{"L2T", "L2T-stalls-no-rid", "[L2T] Stalls because no RID available"},
+	{"L2T", "L2T-stalls-queue-full", "[L2T] Stalls because internal queue full"},
+	{"L2T", "L2T-stalls-wrightback", "[L2T] Stalls because writeback in flight"},
+	{"L2T", "L2T-stalls-mem", "[L2T] Stalls because AXI blocks read"},
+	{"L2T", "L2T-stalls-fill", "[L2T] Stalls because fill pending for victim cache-line"},
+	{"L2T", "L2T-hitq", "[L2T] Sent request via hit queue"},
+	{"L2T", "L2T-hitq-full", "[L2T] Sent request via main queue because hit queue is full"},
+	{"L2T", "L2T-stalls-read-data", "[L2T] Stalls because waiting for data from SDRAM"},
+	{"L2T", "L2T-TMU-read-hits", "[L2T] TMU read hits"},
+	{"L2T", "L2T-TMU-read-miss", "[L2T] TMU read misses"},
+	{"L2T", "L2T-VCD-read-hits", "[L2T] VCD read hits"},
+	{"L2T", "L2T-VCD-read-miss", "[L2T] VCD read misses"},
+	{"L2T", "L2T-SLC-read-hits", "[L2T] SLC read hits (all slices)"},
+	{"L2T", "L2T-SLC-read-miss", "[L2T] SLC read misses (all slices)"},
+	{"AXI", "AXI-writes-seen-watch-0", "[AXI] Writes seen by watch 0"},
+	{"AXI", "AXI-reads-seen-watch-0", "[AXI] Reads seen by watch 0"},
+	{"AXI", "AXI-writes-stalled-seen-watch-0", "[AXI] Write stalls seen by watch 0"},
+	{"AXI", "AXI-reads-stalled-seen-watch-0", "[AXI] Read stalls seen by watch 0"},
+	{"AXI", "AXI-write-bytes-seen-watch-0", "[AXI] Total bytes written seen by watch 0"},
+	{"AXI", "AXI-read-bytes-seen-watch-0", "[AXI] Total bytes read seen by watch 0"},
+	{"AXI", "AXI-writes-seen-watch-1", "[AXI] Writes seen by watch 1"},
+	{"AXI", "AXI-reads-seen-watch-1", "[AXI] Reads seen by watch 1"},
+	{"AXI", "AXI-writes-stalled-seen-watch-1", "[AXI] Write stalls seen by watch 1"},
+	{"AXI", "AXI-reads-stalled-seen-watch-1", "[AXI] Read stalls seen by watch 1"},
+	{"AXI", "AXI-write-bytes-seen-watch-1", "[AXI] Total bytes written seen by watch 1"},
+	{"AXI", "AXI-read-bytes-seen-watch-1", "[AXI] Total bytes read seen by watch 1"},
+	{"CORE", "core-memory-writes", "[CORE] Total memory writes"},
+	{"L2T", "L2T-memory-writes", "[L2T] Total memory writes"},
+	{"PTB", "PTB-memory-writes", "[PTB] Total memory writes"},
+	{"TLB", "TLB-memory-writes", "[TLB] Total memory writes"},
+	{"CORE", "core-memory-reads", "[CORE] Total memory reads"},
+	{"L2T", "L2T-memory-reads", "[L2T] Total memory reads"},
+	{"PTB", "PTB-memory-reads", "[PTB] Total memory reads"},
+	{"PSE", "PSE-memory-reads", "[PSE] Total memory reads"},
+	{"TLB", "TLB-memory-reads", "[TLB] Total memory reads"},
+	{"PTB", "PTB-memory-words-writes", "[PTB] Total memory words written"},
+	{"TLB", "TLB-memory-words-writes", "[TLB] Total memory words written"},
+	{"PSE", "PSE-memory-words-reads", "[PSE] Total memory words read"},
+	{"TLB", "TLB-memory-words-reads", "[TLB] Total memory words read"},
+	{"AXI", "AXI-read-trans", "[AXI] Read transaction count"},
+	{"AXI", "AXI-write-trans", "[AXI] Write transaction count"},
+	{"AXI", "AXI-read-wait-cycles", "[AXI] Read total wait cycles"},
+	{"AXI", "AXI-write-wait-cycles", "[AXI] Write total wait cycles"},
+	{"AXI", "AXI-max-outstanding-reads", "[AXI] Maximum outstanding read transactions"},
+	{"AXI", "AXI-max-outstanding-writes", "[AXI] Maximum outstanding write transactions"},
+	{"QPU", "QPU-wait-bubble", "[QPU] Pipeline bubble in qcycles due all threads waiting"},
+	{"QPU", "QPU-ic-miss-bubble", "[QPU] Pipeline bubble in qcycles due instruction-cache miss"},
+	{"QPU", "QPU-active", "[QPU] Executed shader instruction"},
+	{"QPU", "QPU-total-active-clk-cycles-fragment-shading", "[QPU] Total active clock cycles for all QPUs doing fragment shading (counts only when QPU is not stalled)"},
+	{"QPU", "QPU-stalls", "[QPU] Stalled qcycles executing shader instruction"},
+	{"QPU", "QPU-total-clk-cycles-waiting-fragment-shading", "[QPU] Total stalled clock cycles for all QPUs doing fragment shading"},
+	{"QPU", "QPU-stalls-TMU", "[QPU] Stalled qcycles waiting for TMU"},
+	{"QPU", "QPU-stalls-TLB", "[QPU] Stalled qcycles waiting for TLB"},
+	{"QPU", "QPU-stalls-VPM", "[QPU] Stalled qcycles waiting for VPM"},
+	{"QPU", "QPU-stalls-uniforms", "[QPU] Stalled qcycles waiting for uniforms"},
+	{"QPU", "QPU-stalls-SFU", "[QPU] Stalled qcycles waiting for SFU"},
+	{"QPU", "QPU-stalls-other", "[QPU] Stalled qcycles waiting for any other reason (vary/W/Z)"},
+};
+
+static const struct v3d_perf_counter_desc v3d_v42_performance_counters[] = {
+	{"FEP", "FEP-valid-primitives-no-rendered-pixels", "[FEP] Valid primitives that result in no rendered pixels, for all rendered tiles"},
+	{"FEP", "FEP-valid-primitives-rendered-pixels", "[FEP] Valid primitives for all rendered tiles (primitives may be counted in more than one tile)"},
+	{"FEP", "FEP-clipped-quads", "[FEP] Early-Z/Near/Far clipped quads"},
+	{"FEP", "FEP-valid-quads", "[FEP] Valid quads"},
+	{"TLB", "TLB-quads-not-passing-stencil-test", "[TLB] Quads with no pixels passing the stencil test"},
+	{"TLB", "TLB-quads-not-passing-z-and-stencil-test", "[TLB] Quads with no pixels passing the Z and stencil tests"},
+	{"TLB", "TLB-quads-passing-z-and-stencil-test", "[TLB] Quads with any pixels passing the Z and stencil tests"},
+	{"TLB", "TLB-quads-with-zero-coverage", "[TLB] Quads with all pixels having zero coverage"},
+	{"TLB", "TLB-quads-with-non-zero-coverage", "[TLB] Quads with any pixels having non-zero coverage"},
+	{"TLB", "TLB-quads-written-to-color-buffer", "[TLB] Quads with valid pixels written to colour buffer"},
+	{"PTB", "PTB-primitives-discarded-outside-viewport", "[PTB] Primitives discarded by being outside the viewport"},
+	{"PTB", "PTB-primitives-need-clipping", "[PTB] Primitives that need clipping"},
+	{"PTB", "PTB-primitives-discarded-reversed", "[PTB] Primitives that are discarded because they are reversed"},
+	{"QPU", "QPU-total-idle-clk-cycles", "[QPU] Total idle clock cycles for all QPUs"},
+	{"QPU", "QPU-total-active-clk-cycles-vertex-coord-shading", "[QPU] Total active clock cycles for all QPUs doing vertex/coordinate/user shading (counts only when QPU is not stalled)"},
+	{"QPU", "QPU-total-active-clk-cycles-fragment-shading", "[QPU] Total active clock cycles for all QPUs doing fragment shading (counts only when QPU is not stalled)"},
+	{"QPU", "QPU-total-clk-cycles-executing-valid-instr", "[QPU] Total clock cycles for all QPUs executing valid instructions"},
+	{"QPU", "QPU-total-clk-cycles-waiting-TMU", "[QPU] Total clock cycles for all QPUs stalled waiting for TMUs only (counter won't increment if QPU also stalling for another reason)"},
+	{"QPU", "QPU-total-clk-cycles-waiting-scoreboard", "[QPU] Total clock cycles for all QPUs stalled waiting for Scoreboard only (counter won't increment if QPU also stalling for another reason)"},
+	{"QPU", "QPU-total-clk-cycles-waiting-varyings", "[QPU] Total clock cycles for all QPUs stalled waiting for Varyings only (counter won't increment if QPU also stalling for another reason)"},
+	{"QPU", "QPU-total-instr-cache-hit", "[QPU] Total instruction cache hits for all slices"},
+	{"QPU", "QPU-total-instr-cache-miss", "[QPU] Total instruction cache misses for all slices"},
+	{"QPU", "QPU-total-uniform-cache-hit", "[QPU] Total uniforms cache hits for all slices"},
+	{"QPU", "QPU-total-uniform-cache-miss", "[QPU] Total uniforms cache misses for all slices"},
+	{"TMU", "TMU-total-text-quads-access", "[TMU] Total texture cache accesses"},
+	{"TMU", "TMU-total-text-cache-miss", "[TMU] Total texture cache misses (number of fetches from memory/L2cache)"},
+	{"VPM", "VPM-total-clk-cycles-VDW-stalled", "[VPM] Total clock cycles VDW is stalled waiting for VPM access"},
+	{"VPM", "VPM-total-clk-cycles-VCD-stalled", "[VPM] Total clock cycles VCD is stalled waiting for VPM access"},
+	{"CLE", "CLE-bin-thread-active-cycles", "[CLE] Bin thread active cycles"},
+	{"CLE", "CLE-render-thread-active-cycles", "[CLE] Render thread active cycles"},
+	{"L2T", "L2T-total-cache-hit", "[L2T] Total Level 2 cache hits"},
+	{"L2T", "L2T-total-cache-miss", "[L2T] Total Level 2 cache misses"},
+	{"CORE", "cycle-count", "[CORE] Cycle counter"},
+	{"QPU", "QPU-total-clk-cycles-waiting-vertex-coord-shading", "[QPU] Total stalled clock cycles for all QPUs doing vertex/coordinate/user shading"},
+	{"QPU", "QPU-total-clk-cycles-waiting-fragment-shading", "[QPU] Total stalled clock cycles for all QPUs doing fragment shading"},
+	{"PTB", "PTB-primitives-binned", "[PTB] Total primitives binned"},
+	{"AXI", "AXI-writes-seen-watch-0", "[AXI] Writes seen by watch 0"},
+	{"AXI", "AXI-reads-seen-watch-0", "[AXI] Reads seen by watch 0"},
+	{"AXI", "AXI-writes-stalled-seen-watch-0", "[AXI] Write stalls seen by watch 0"},
+	{"AXI", "AXI-reads-stalled-seen-watch-0", "[AXI] Read stalls seen by watch 0"},
+	{"AXI", "AXI-write-bytes-seen-watch-0", "[AXI] Total bytes written seen by watch 0"},
+	{"AXI", "AXI-read-bytes-seen-watch-0", "[AXI] Total bytes read seen by watch 0"},
+	{"AXI", "AXI-writes-seen-watch-1", "[AXI] Writes seen by watch 1"},
+	{"AXI", "AXI-reads-seen-watch-1", "[AXI] Reads seen by watch 1"},
+	{"AXI", "AXI-writes-stalled-seen-watch-1", "[AXI] Write stalls seen by watch 1"},
+	{"AXI", "AXI-reads-stalled-seen-watch-1", "[AXI] Read stalls seen by watch 1"},
+	{"AXI", "AXI-write-bytes-seen-watch-1", "[AXI] Total bytes written seen by watch 1"},
+	{"AXI", "AXI-read-bytes-seen-watch-1", "[AXI] Total bytes read seen by watch 1"},
+	{"TLB", "TLB-partial-quads-written-to-color-buffer", "[TLB] Partial quads written to the colour buffer"},
+	{"TMU", "TMU-total-config-access", "[TMU] Total config accesses"},
+	{"L2T", "L2T-no-id-stalled", "[L2T] No ID stall"},
+	{"L2T", "L2T-command-queue-stalled", "[L2T] Command queue full stall"},
+	{"L2T", "L2T-TMU-writes", "[L2T] TMU write accesses"},
+	{"TMU", "TMU-active-cycles", "[TMU] Active cycles"},
+	{"TMU", "TMU-stalled-cycles", "[TMU] Stalled cycles"},
+	{"CLE", "CLE-thread-active-cycles", "[CLE] Bin or render thread active cycles"},
+	{"L2T", "L2T-TMU-reads", "[L2T] TMU read accesses"},
+	{"L2T", "L2T-CLE-reads", "[L2T] CLE read accesses"},
+	{"L2T", "L2T-VCD-reads", "[L2T] VCD read accesses"},
+	{"L2T", "L2T-TMU-config-reads", "[L2T] TMU CFG read accesses"},
+	{"L2T", "L2T-SLC0-reads", "[L2T] SLC0 read accesses"},
+	{"L2T", "L2T-SLC1-reads", "[L2T] SLC1 read accesses"},
+	{"L2T", "L2T-SLC2-reads", "[L2T] SLC2 read accesses"},
+	{"L2T", "L2T-TMU-write-miss", "[L2T] TMU write misses"},
+	{"L2T", "L2T-TMU-read-miss", "[L2T] TMU read misses"},
+	{"L2T", "L2T-CLE-read-miss", "[L2T] CLE read misses"},
+	{"L2T", "L2T-VCD-read-miss", "[L2T] VCD read misses"},
+	{"L2T", "L2T-TMU-config-read-miss", "[L2T] TMU CFG read misses"},
+	{"L2T", "L2T-SLC0-read-miss", "[L2T] SLC0 read misses"},
+	{"L2T", "L2T-SLC1-read-miss", "[L2T] SLC1 read misses"},
+	{"L2T", "L2T-SLC2-read-miss", "[L2T] SLC2 read misses"},
+	{"CORE", "core-memory-writes", "[CORE] Total memory writes"},
+	{"L2T", "L2T-memory-writes", "[L2T] Total memory writes"},
+	{"PTB", "PTB-memory-writes", "[PTB] Total memory writes"},
+	{"TLB", "TLB-memory-writes", "[TLB] Total memory writes"},
+	{"CORE", "core-memory-reads", "[CORE] Total memory reads"},
+	{"L2T", "L2T-memory-reads", "[L2T] Total memory reads"},
+	{"PTB", "PTB-memory-reads", "[PTB] Total memory reads"},
+	{"PSE", "PSE-memory-reads", "[PSE] Total memory reads"},
+	{"TLB", "TLB-memory-reads", "[TLB] Total memory reads"},
+	{"GMP", "GMP-memory-reads", "[GMP] Total memory reads"},
+	{"PTB", "PTB-memory-words-writes", "[PTB] Total memory words written"},
+	{"TLB", "TLB-memory-words-writes", "[TLB] Total memory words written"},
+	{"PSE", "PSE-memory-words-reads", "[PSE] Total memory words read"},
+	{"TLB", "TLB-memory-words-reads", "[TLB] Total memory words read"},
+	{"TMU", "TMU-MRU-hits", "[TMU] Total MRU hits"},
+	{"CORE", "compute-active-cycles", "[CORE] Compute active cycles"},
+};
+
+#endif
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/6] drm/v3d: Different V3D versions can have different number of perfcnt
  2024-05-12 22:23 [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
  2024-05-12 22:23 ` [PATCH v2 1/6] drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1 Maíra Canal
@ 2024-05-12 22:23 ` Maíra Canal
  2024-05-12 22:23 ` [PATCH v2 3/6] drm/v3d: Create a new V3D parameter for the maximum " Maíra Canal
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2024-05-12 22:23 UTC (permalink / raw
  To: Melissa Wen, Iago Toral, Jose Maria Casanova Crespo,
	Juan A . Suárez, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter
  Cc: dri-devel, kernel-dev, Maíra Canal

Currently, even though V3D 7.1 has 93 performance counters, it is not
possible to create counters bigger than 87, as
`v3d_perfmon_create_ioctl()` understands that counters bigger than 87
are invalid.

Therefore, create a device variable to expose the maximum
number of counters for a given V3D version and make
`v3d_perfmon_create_ioctl()` check this variable.

This commit fixes CTS failures in the performance queries tests
`dEQP-VK.query_pool.performance_query.*` [1]

Link: https://gitlab.freedesktop.org/mesa/mesa/-/commit/ea1f09a5f21839f4f3b93610b58507c4bd9b9b81 [1]
Fixes: 6fd9487147c4 ("drm/v3d: add brcm,2712-v3d as a compatible V3D device")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_drv.c     | 7 +++++++
 drivers/gpu/drm/v3d/v3d_drv.h     | 5 +++++
 drivers/gpu/drm/v3d/v3d_perfmon.c | 3 ++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 28b7ddce7747..6b9dd26df9fe 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -294,6 +294,13 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
 	v3d->cores = V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_NCORES);
 	WARN_ON(v3d->cores > 1); /* multicore not yet implemented */
 
+	if (v3d->ver >= 71)
+		v3d->max_counters = ARRAY_SIZE(v3d_v71_performance_counters);
+	else if (v3d->ver >= 42)
+		v3d->max_counters = ARRAY_SIZE(v3d_v42_performance_counters);
+	else
+		v3d->max_counters = 0;
+
 	v3d->reset = devm_reset_control_get_exclusive(dev, NULL);
 	if (IS_ERR(v3d->reset)) {
 		ret = PTR_ERR(v3d->reset);
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 671375a3bb66..bd1e38f7d10a 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -104,6 +104,11 @@ struct v3d_dev {
 	int ver;
 	bool single_irq_line;
 
+	/* Different revisions of V3D have different total number of performance
+	 * counters
+	 */
+	unsigned int max_counters;
+
 	void __iomem *hub_regs;
 	void __iomem *core_regs[3];
 	void __iomem *bridge_regs;
diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
index e1be7368b87d..f268d9466c0f 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -123,6 +123,7 @@ int v3d_perfmon_create_ioctl(struct drm_device *dev, void *data,
 {
 	struct v3d_file_priv *v3d_priv = file_priv->driver_priv;
 	struct drm_v3d_perfmon_create *req = data;
+	struct v3d_dev *v3d = v3d_priv->v3d;
 	struct v3d_perfmon *perfmon;
 	unsigned int i;
 	int ret;
@@ -134,7 +135,7 @@ int v3d_perfmon_create_ioctl(struct drm_device *dev, void *data,
 
 	/* Make sure all counters are valid. */
 	for (i = 0; i < req->ncounters; i++) {
-		if (req->counters[i] >= V3D_PERFCNT_NUM)
+		if (req->counters[i] >= v3d->max_counters)
 			return -EINVAL;
 	}
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/6] drm/v3d: Create a new V3D parameter for the maximum number of perfcnt
  2024-05-12 22:23 [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
  2024-05-12 22:23 ` [PATCH v2 1/6] drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1 Maíra Canal
  2024-05-12 22:23 ` [PATCH v2 2/6] drm/v3d: Different V3D versions can have different number of perfcnt Maíra Canal
@ 2024-05-12 22:23 ` Maíra Canal
  2024-05-12 22:23 ` [PATCH v2 4/6] drm/v3d: Create new IOCTL to expose performance counters information Maíra Canal
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2024-05-12 22:23 UTC (permalink / raw
  To: Melissa Wen, Iago Toral, Jose Maria Casanova Crespo,
	Juan A . Suárez, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter
  Cc: dri-devel, kernel-dev, Maíra Canal

The maximum number of performance counters can change from version to
version and it's important for userspace to know this value, as it needs
to use the counters for performance queries. Therefore, expose the
maximum number of performance counters to userspace as a parameter.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_drv.c | 3 +++
 include/uapi/drm/v3d_drm.h    | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 6b9dd26df9fe..d2c1d5053132 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -94,6 +94,9 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data,
 	case DRM_V3D_PARAM_SUPPORTS_CPU_QUEUE:
 		args->value = 1;
 		return 0;
+	case DRM_V3D_PARAM_MAX_PERF_COUNTERS:
+		args->value = v3d->max_counters;
+		return 0;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", args->param);
 		return -EINVAL;
diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h
index dce1835eced4..215b01bb69c3 100644
--- a/include/uapi/drm/v3d_drm.h
+++ b/include/uapi/drm/v3d_drm.h
@@ -286,6 +286,7 @@ enum drm_v3d_param {
 	DRM_V3D_PARAM_SUPPORTS_PERFMON,
 	DRM_V3D_PARAM_SUPPORTS_MULTISYNC_EXT,
 	DRM_V3D_PARAM_SUPPORTS_CPU_QUEUE,
+	DRM_V3D_PARAM_MAX_PERF_COUNTERS,
 };
 
 struct drm_v3d_get_param {
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/6] drm/v3d: Create new IOCTL to expose performance counters information
  2024-05-12 22:23 [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
                   ` (2 preceding siblings ...)
  2024-05-12 22:23 ` [PATCH v2 3/6] drm/v3d: Create a new V3D parameter for the maximum " Maíra Canal
@ 2024-05-12 22:23 ` Maíra Canal
  2024-05-12 22:23 ` [PATCH v2 5/6] drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM Maíra Canal
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2024-05-12 22:23 UTC (permalink / raw
  To: Melissa Wen, Iago Toral, Jose Maria Casanova Crespo,
	Juan A . Suárez, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter
  Cc: dri-devel, kernel-dev, Maíra Canal

Userspace usually needs some information about the performance counters
available. Although we could replicate this information in the kernel
and user-space, let's use the kernel as the "single source of truth" to
avoid issues in the future (e.g. list of performance counters is updated
in user-space, but not in the kernel, generating invalid requests).

Therefore, create a new IOCTL to expose the performance counters
information, that is name, category, and description.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_drv.c     |  1 +
 drivers/gpu/drm/v3d/v3d_drv.h     |  2 ++
 drivers/gpu/drm/v3d/v3d_perfmon.c | 33 +++++++++++++++++++++++++++
 include/uapi/drm/v3d_drm.h        | 37 +++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index d2c1d5053132..f7477488b1cc 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -211,6 +211,7 @@ static const struct drm_ioctl_desc v3d_drm_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(V3D_PERFMON_DESTROY, v3d_perfmon_destroy_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(V3D_PERFMON_GET_VALUES, v3d_perfmon_get_values_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(V3D_SUBMIT_CPU, v3d_submit_cpu_ioctl, DRM_RENDER_ALLOW | DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(V3D_PERFMON_GET_COUNTER, v3d_perfmon_get_counter_ioctl, DRM_RENDER_ALLOW),
 };
 
 static const struct drm_driver v3d_drm_driver = {
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index bd1e38f7d10a..44cfddedebde 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -582,6 +582,8 @@ int v3d_perfmon_destroy_ioctl(struct drm_device *dev, void *data,
 			      struct drm_file *file_priv);
 int v3d_perfmon_get_values_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv);
+int v3d_perfmon_get_counter_ioctl(struct drm_device *dev, void *data,
+				  struct drm_file *file_priv);
 
 /* v3d_sysfs.c */
 int v3d_sysfs_init(struct device *dev);
diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
index f268d9466c0f..73e2bb8bdb7f 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -217,3 +217,36 @@ int v3d_perfmon_get_values_ioctl(struct drm_device *dev, void *data,
 
 	return ret;
 }
+
+int v3d_perfmon_get_counter_ioctl(struct drm_device *dev, void *data,
+				  struct drm_file *file_priv)
+{
+	struct drm_v3d_perfmon_get_counter *req = data;
+	struct v3d_dev *v3d = to_v3d_dev(dev);
+	const struct v3d_perf_counter_desc *counter;
+
+	for (int i = 0; i < ARRAY_SIZE(req->reserved); i++) {
+		if (req->reserved[i] != 0)
+			return -EINVAL;
+	}
+
+	/* Make sure that the counter ID is valid */
+	if (req->counter >= v3d->max_counters)
+		return -EINVAL;
+
+	if (v3d->ver >= 71) {
+		WARN_ON(v3d->max_counters != ARRAY_SIZE(v3d_v71_performance_counters));
+		counter = &v3d_v71_performance_counters[req->counter];
+	} else if (v3d->ver >= 42) {
+		WARN_ON(v3d->max_counters != ARRAY_SIZE(v3d_v42_performance_counters));
+		counter = &v3d_v42_performance_counters[req->counter];
+	} else {
+		return -EOPNOTSUPP;
+	}
+
+	strscpy(req->name, counter->name, sizeof(req->name));
+	strscpy(req->category, counter->category, sizeof(req->category));
+	strscpy(req->description, counter->description, sizeof(req->description));
+
+	return 0;
+}
diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h
index 215b01bb69c3..0860ddb3d0b6 100644
--- a/include/uapi/drm/v3d_drm.h
+++ b/include/uapi/drm/v3d_drm.h
@@ -42,6 +42,7 @@ extern "C" {
 #define DRM_V3D_PERFMON_DESTROY                   0x09
 #define DRM_V3D_PERFMON_GET_VALUES                0x0a
 #define DRM_V3D_SUBMIT_CPU                        0x0b
+#define DRM_V3D_PERFMON_GET_COUNTER               0x0c
 
 #define DRM_IOCTL_V3D_SUBMIT_CL           DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_SUBMIT_CL, struct drm_v3d_submit_cl)
 #define DRM_IOCTL_V3D_WAIT_BO             DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_WAIT_BO, struct drm_v3d_wait_bo)
@@ -58,6 +59,8 @@ extern "C" {
 #define DRM_IOCTL_V3D_PERFMON_GET_VALUES  DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_PERFMON_GET_VALUES, \
 						   struct drm_v3d_perfmon_get_values)
 #define DRM_IOCTL_V3D_SUBMIT_CPU          DRM_IOW(DRM_COMMAND_BASE + DRM_V3D_SUBMIT_CPU, struct drm_v3d_submit_cpu)
+#define DRM_IOCTL_V3D_PERFMON_GET_COUNTER DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_PERFMON_GET_COUNTER, \
+						   struct drm_v3d_perfmon_get_counter)
 
 #define DRM_V3D_SUBMIT_CL_FLUSH_CACHE             0x01
 #define DRM_V3D_SUBMIT_EXTENSION		  0x02
@@ -718,6 +721,40 @@ struct drm_v3d_perfmon_get_values {
 	__u64 values_ptr;
 };
 
+#define DRM_V3D_PERFCNT_MAX_NAME 64
+#define DRM_V3D_PERFCNT_MAX_CATEGORY 32
+#define DRM_V3D_PERFCNT_MAX_DESCRIPTION 256
+
+/**
+ * struct drm_v3d_perfmon_get_counter - ioctl to get the description of a
+ * performance counter
+ *
+ * As userspace needs to retrieve information about the performance counters
+ * available, this IOCTL allows users to get information about a performance
+ * counter (name, category and description).
+ */
+struct drm_v3d_perfmon_get_counter {
+	/*
+	 * Counter ID
+	 *
+	 * Must be smaller than the maximum number of performance counters, which
+	 * can be retrieve through DRM_V3D_PARAM_MAX_PERF_COUNTERS.
+	 */
+	__u8 counter;
+
+	/* Name of the counter */
+	__u8 name[DRM_V3D_PERFCNT_MAX_NAME];
+
+	/* Category of the counter */
+	__u8 category[DRM_V3D_PERFCNT_MAX_CATEGORY];
+
+	/* Description of the counter */
+	__u8 description[DRM_V3D_PERFCNT_MAX_DESCRIPTION];
+
+	/* mbz */
+	__u8 reserved[7];
+};
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 5/6] drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM
  2024-05-12 22:23 [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
                   ` (3 preceding siblings ...)
  2024-05-12 22:23 ` [PATCH v2 4/6] drm/v3d: Create new IOCTL to expose performance counters information Maíra Canal
@ 2024-05-12 22:23 ` Maíra Canal
  2024-05-12 22:23 ` [PATCH v2 6/6] drm/v3d: Deprecate the use of the Performance Counters enum Maíra Canal
  2024-05-20 20:29 ` [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
  6 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2024-05-12 22:23 UTC (permalink / raw
  To: Melissa Wen, Iago Toral, Jose Maria Casanova Crespo,
	Juan A . Suárez, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter
  Cc: dri-devel, kernel-dev, Maíra Canal

V3D_PERFCNT_NUM represents the maximum number of performance counters
for V3D 4.2, but not for V3D 7.1. This means that, if we use
V3D_PERFCNT_NUM, we might go out-of-bounds on V3D 7.1.

Therefore, use the number of performance counters on V3D 7.1 as the
maximum number of counters. This will allow us to create arrays on the
stack with reasonable size. Note that userspace must use the value
provided by DRM_V3D_PARAM_MAX_PERF_COUNTERS.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_drv.h   | 5 ++++-
 drivers/gpu/drm/v3d/v3d_sched.c | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 44cfddedebde..556cbb400ba0 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -351,8 +351,11 @@ struct v3d_timestamp_query {
 	struct drm_syncobj *syncobj;
 };
 
+/* Maximum number of performance counters supported by any version of V3D */
+#define V3D_MAX_COUNTERS ARRAY_SIZE(v3d_v71_performance_counters)
+
 /* Number of perfmons required to handle all supported performance counters */
-#define V3D_MAX_PERFMONS DIV_ROUND_UP(V3D_PERFCNT_NUM, \
+#define V3D_MAX_PERFMONS DIV_ROUND_UP(V3D_MAX_COUNTERS, \
 				      DRM_V3D_MAX_PERF_COUNTERS)
 
 struct v3d_performance_query {
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 7cd8c335cd9b..03df37a3acf5 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -490,7 +490,7 @@ v3d_write_performance_query_result(struct v3d_cpu_job *job, void *data, u32 quer
 	struct v3d_file_priv *v3d_priv = job->base.file->driver_priv;
 	struct v3d_dev *v3d = job->base.v3d;
 	struct v3d_perfmon *perfmon;
-	u64 counter_values[V3D_PERFCNT_NUM];
+	u64 counter_values[V3D_MAX_COUNTERS];
 
 	for (int i = 0; i < performance_query->nperfmons; i++) {
 		perfmon = v3d_perfmon_find(v3d_priv,
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 6/6] drm/v3d: Deprecate the use of the Performance Counters enum
  2024-05-12 22:23 [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
                   ` (4 preceding siblings ...)
  2024-05-12 22:23 ` [PATCH v2 5/6] drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM Maíra Canal
@ 2024-05-12 22:23 ` Maíra Canal
  2024-05-20 20:29 ` [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
  6 siblings, 0 replies; 11+ messages in thread
From: Maíra Canal @ 2024-05-12 22:23 UTC (permalink / raw
  To: Melissa Wen, Iago Toral, Jose Maria Casanova Crespo,
	Juan A . Suárez, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter
  Cc: dri-devel, kernel-dev, Maíra Canal

The Performance Counters enum used to identify the index of each
performance counter and provide the total number of performance
counters (V3D_PERFCNT_NUM). But, this enum is only valid for V3D 4.2,
not for V3D 7.1.

As we implemented a new flexible structure to retrieve performance
counters information, we can deprecate this enum.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
---
 include/uapi/drm/v3d_drm.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h
index 0860ddb3d0b6..87fc5bb0a61e 100644
--- a/include/uapi/drm/v3d_drm.h
+++ b/include/uapi/drm/v3d_drm.h
@@ -603,6 +603,16 @@ struct drm_v3d_submit_cpu {
 	__u64 extensions;
 };
 
+/* The performance counters index represented by this enum are deprecated and
+ * must no longer be used. These counters are only valid for V3D 4.2.
+ *
+ * In order to check for performance counter information,
+ * use DRM_IOCTL_V3D_PERFMON_GET_COUNTER.
+ *
+ * Don't use V3D_PERFCNT_NUM to retrieve the maximum number of performance
+ * counters. You should use DRM_IOCTL_V3D_GET_PARAM with the following
+ * parameter: DRM_V3D_PARAM_MAX_PERF_COUNTERS.
+ */
 enum {
 	V3D_PERFCNT_FEP_VALID_PRIMTS_NO_PIXELS,
 	V3D_PERFCNT_FEP_VALID_PRIMS,
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling
  2024-05-12 22:23 [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
                   ` (5 preceding siblings ...)
  2024-05-12 22:23 ` [PATCH v2 6/6] drm/v3d: Deprecate the use of the Performance Counters enum Maíra Canal
@ 2024-05-20 20:29 ` Maíra Canal
  2024-05-21 11:07   ` Jani Nikula
  6 siblings, 1 reply; 11+ messages in thread
From: Maíra Canal @ 2024-05-20 20:29 UTC (permalink / raw
  To: Melissa Wen, Iago Toral, Jose Maria Casanova Crespo,
	Juan A . Suárez, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter
  Cc: dri-devel, kernel-dev

On 5/12/24 19:23, Maíra Canal wrote:>
> Maíra Canal (6):
>    drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1
>    drm/v3d: Different V3D versions can have different number of perfcnt
>    drm/v3d: Create a new V3D parameter for the maximum number of perfcnt
>    drm/v3d: Create new IOCTL to expose performance counters information
>    drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM
>    drm/v3d: Deprecate the use of the Performance Counters enum >
>   drivers/gpu/drm/v3d/v3d_drv.c                 |  11 +
>   drivers/gpu/drm/v3d/v3d_drv.h                 |  14 +-
>   drivers/gpu/drm/v3d/v3d_perfmon.c             |  36 ++-
>   .../gpu/drm/v3d/v3d_performance_counters.h    | 208 ++++++++++++++++++
>   drivers/gpu/drm/v3d/v3d_sched.c               |   2 +-
>   include/uapi/drm/v3d_drm.h                    |  48 ++++
>   6 files changed, 316 insertions(+), 3 deletions(-)
>   create mode 100644 drivers/gpu/drm/v3d/v3d_performance_counters.h
> 

Applied to drm-misc/drm-misc-next!

Best Regards,
- Maíra

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling
  2024-05-20 20:29 ` [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
@ 2024-05-21 11:07   ` Jani Nikula
  2024-05-21 11:45     ` Maíra Canal
  0 siblings, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2024-05-21 11:07 UTC (permalink / raw
  To: Maíra Canal, Melissa Wen, Iago Toral,
	Jose Maria Casanova Crespo, Juan A . Suárez,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Daniel Vetter
  Cc: dri-devel, kernel-dev

On Mon, 20 May 2024, Maíra Canal <mcanal@igalia.com> wrote:
> On 5/12/24 19:23, Maíra Canal wrote:>
>> Maíra Canal (6):
>>    drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1
>>    drm/v3d: Different V3D versions can have different number of perfcnt
>>    drm/v3d: Create a new V3D parameter for the maximum number of perfcnt
>>    drm/v3d: Create new IOCTL to expose performance counters information
>>    drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM
>>    drm/v3d: Deprecate the use of the Performance Counters enum >
>>   drivers/gpu/drm/v3d/v3d_drv.c                 |  11 +
>>   drivers/gpu/drm/v3d/v3d_drv.h                 |  14 +-
>>   drivers/gpu/drm/v3d/v3d_perfmon.c             |  36 ++-
>>   .../gpu/drm/v3d/v3d_performance_counters.h    | 208 ++++++++++++++++++
>>   drivers/gpu/drm/v3d/v3d_sched.c               |   2 +-
>>   include/uapi/drm/v3d_drm.h                    |  48 ++++
>>   6 files changed, 316 insertions(+), 3 deletions(-)
>>   create mode 100644 drivers/gpu/drm/v3d/v3d_performance_counters.h
>> 
>
> Applied to drm-misc/drm-misc-next!

What compiler do you use? I'm hitting the same as kernel test robot [1]
with arm-linux-gnueabihf-gcc 12.2.0.

In general, I don't think it's a great idea to put arrays in headers,
and then include it everywhere via v3d_drv.h. You're not just relying on
the compiler to optimize it away in compilation units where its not
referenced (likely to happen), but also for the linker to deduplicate
rodata (possible, but I'm not sure that it will happen).

I think you need to move the arrays to a .c file, and then either a) add
interfaces to access the arrays, or b) declare the arrays and make them
global. For the latter you also need to figure out how to expose the
size.

BR,
Jani.


[1] https://lore.kernel.org/r/202405211137.hueFkLKG-lkp@intel.com


-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling
  2024-05-21 11:07   ` Jani Nikula
@ 2024-05-21 11:45     ` Maíra Canal
  2024-05-29  7:30       ` Jani Nikula
  0 siblings, 1 reply; 11+ messages in thread
From: Maíra Canal @ 2024-05-21 11:45 UTC (permalink / raw
  To: Jani Nikula, Melissa Wen, Iago Toral, Jose Maria Casanova Crespo,
	Juan A . Suárez, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter
  Cc: dri-devel, kernel-dev

Hi Jani,

On 5/21/24 08:07, Jani Nikula wrote:
> On Mon, 20 May 2024, Maíra Canal <mcanal@igalia.com> wrote:
>> On 5/12/24 19:23, Maíra Canal wrote:>
>>> Maíra Canal (6):
>>>     drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1
>>>     drm/v3d: Different V3D versions can have different number of perfcnt
>>>     drm/v3d: Create a new V3D parameter for the maximum number of perfcnt
>>>     drm/v3d: Create new IOCTL to expose performance counters information
>>>     drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM
>>>     drm/v3d: Deprecate the use of the Performance Counters enum >
>>>    drivers/gpu/drm/v3d/v3d_drv.c                 |  11 +
>>>    drivers/gpu/drm/v3d/v3d_drv.h                 |  14 +-
>>>    drivers/gpu/drm/v3d/v3d_perfmon.c             |  36 ++-
>>>    .../gpu/drm/v3d/v3d_performance_counters.h    | 208 ++++++++++++++++++
>>>    drivers/gpu/drm/v3d/v3d_sched.c               |   2 +-
>>>    include/uapi/drm/v3d_drm.h                    |  48 ++++
>>>    6 files changed, 316 insertions(+), 3 deletions(-)
>>>    create mode 100644 drivers/gpu/drm/v3d/v3d_performance_counters.h
>>>
>>
>> Applied to drm-misc/drm-misc-next!
> 
> What compiler do you use? I'm hitting the same as kernel test robot [1]
> with arm-linux-gnueabihf-gcc 12.2.0.

I use clang version 17.0.6.

> 
> In general, I don't think it's a great idea to put arrays in headers,
> and then include it everywhere via v3d_drv.h. You're not just relying on
> the compiler to optimize it away in compilation units where its not
> referenced (likely to happen), but also for the linker to deduplicate
> rodata (possible, but I'm not sure that it will happen).
> 
> I think you need to move the arrays to a .c file, and then either a) add
> interfaces to access the arrays, or b) declare the arrays and make them
> global. For the latter you also need to figure out how to expose the
> size.

I'll write a patch to fix it. Sorry for the disturbance, I didn't notice
it with clang.

Best Regards,
- Maíra

> 
> BR,
> Jani.
> 
> 
> [1] https://lore.kernel.org/r/202405211137.hueFkLKG-lkp@intel.com
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling
  2024-05-21 11:45     ` Maíra Canal
@ 2024-05-29  7:30       ` Jani Nikula
  0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2024-05-29  7:30 UTC (permalink / raw
  To: Maíra Canal, Melissa Wen, Iago Toral,
	Jose Maria Casanova Crespo, Juan A . Suárez,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Daniel Vetter
  Cc: dri-devel, kernel-dev

On Tue, 21 May 2024, Maíra Canal <mcanal@igalia.com> wrote:
> Hi Jani,
>
> On 5/21/24 08:07, Jani Nikula wrote:
>> On Mon, 20 May 2024, Maíra Canal <mcanal@igalia.com> wrote:
>>> On 5/12/24 19:23, Maíra Canal wrote:>
>>>> Maíra Canal (6):
>>>>     drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1
>>>>     drm/v3d: Different V3D versions can have different number of perfcnt
>>>>     drm/v3d: Create a new V3D parameter for the maximum number of perfcnt
>>>>     drm/v3d: Create new IOCTL to expose performance counters information
>>>>     drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM
>>>>     drm/v3d: Deprecate the use of the Performance Counters enum >
>>>>    drivers/gpu/drm/v3d/v3d_drv.c                 |  11 +
>>>>    drivers/gpu/drm/v3d/v3d_drv.h                 |  14 +-
>>>>    drivers/gpu/drm/v3d/v3d_perfmon.c             |  36 ++-
>>>>    .../gpu/drm/v3d/v3d_performance_counters.h    | 208 ++++++++++++++++++
>>>>    drivers/gpu/drm/v3d/v3d_sched.c               |   2 +-
>>>>    include/uapi/drm/v3d_drm.h                    |  48 ++++
>>>>    6 files changed, 316 insertions(+), 3 deletions(-)
>>>>    create mode 100644 drivers/gpu/drm/v3d/v3d_performance_counters.h
>>>>
>>>
>>> Applied to drm-misc/drm-misc-next!
>> 
>> What compiler do you use? I'm hitting the same as kernel test robot [1]
>> with arm-linux-gnueabihf-gcc 12.2.0.
>
> I use clang version 17.0.6.
>
>> 
>> In general, I don't think it's a great idea to put arrays in headers,
>> and then include it everywhere via v3d_drv.h. You're not just relying on
>> the compiler to optimize it away in compilation units where its not
>> referenced (likely to happen), but also for the linker to deduplicate
>> rodata (possible, but I'm not sure that it will happen).
>> 
>> I think you need to move the arrays to a .c file, and then either a) add
>> interfaces to access the arrays, or b) declare the arrays and make them
>> global. For the latter you also need to figure out how to expose the
>> size.
>
> I'll write a patch to fix it. Sorry for the disturbance, I didn't notice
> it with clang.

Another report [1].


BR,
Jani.

[1] https://lore.kernel.org/all/20240529122955.4cc16889@canb.auug.org.au



-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-05-29  7:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-12 22:23 [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
2024-05-12 22:23 ` [PATCH v2 1/6] drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1 Maíra Canal
2024-05-12 22:23 ` [PATCH v2 2/6] drm/v3d: Different V3D versions can have different number of perfcnt Maíra Canal
2024-05-12 22:23 ` [PATCH v2 3/6] drm/v3d: Create a new V3D parameter for the maximum " Maíra Canal
2024-05-12 22:23 ` [PATCH v2 4/6] drm/v3d: Create new IOCTL to expose performance counters information Maíra Canal
2024-05-12 22:23 ` [PATCH v2 5/6] drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM Maíra Canal
2024-05-12 22:23 ` [PATCH v2 6/6] drm/v3d: Deprecate the use of the Performance Counters enum Maíra Canal
2024-05-20 20:29 ` [PATCH v2 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
2024-05-21 11:07   ` Jani Nikula
2024-05-21 11:45     ` Maíra Canal
2024-05-29  7:30       ` Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).