[PATCH v2 0/4] Add a few tracepoints to panthor

[PATCH v2 0/4] Add a few tracepoints to panthor

[Nicolas Frattaroli]

This series adds two tracepoints to panthor.

The first tracepoint allows for inspecting the power status of the
hardware subdivisions, e.g. how many shader cores are powered on. This
is done by reading four hardware registers when a certain IRQ fires.

The second tracepoint instruments panthor's job IRQ handler. This is
more useful than the generic interrupt tracing functionality, as the
tracepoint has the events bit mask included, which indicates which
command stream group interfaces triggered the interrupt.

To test the tracepoints, the following can be used:

  :~# echo 1 > /sys/kernel/tracing/events/panthor/gpu_power_status/enable
  :~# echo 1 > /sys/kernel/tracing/events/panthor/gpu_job_irq/enable
  :~# echo 1 > /sys/kernel/tracing/tracing_on
  :~# cat /sys/kernel/tracing/trace_pipe

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v2:
- Only enable the GPU_IRQ_POWER_CHANGED_* IRQ mask bits when the
  tracepoint is enabled. Necessitates the new irq helper patch.
- Only enable the GPU_IRQ_POWER_CHANGED_* IRQ mask bits if the hardware
  architecture is <= v13, as v14 changes things.
- Use _READY instead of _PWRACTIVE registers, and rename the tracepoint
  accordingly.
- Also read the status of the ray tracing unit's power. This is a global
  flag for all shader cores, it seems. Necessitates the new register
  definition patch.
- Move the POWER_CHANGED_* check to earlier in the interrupt handler.
- Also listen to POWER_CHANGED, not just POWER_CHANGED_ALL, as this
  provides useful information with the _READY registers.
- Print the device name in both tracepoints, to disambiguate things on
  systems with multiple Mali GPUs.
- Document the gpu_power_status tracepoint, so the meaning of the fields
  is made clear.
- Link to v1: https://lore.kernel.org/r/20251203-panthor-tracepoints-v1-0-871c8917e084@collabora.com

---
Nicolas Frattaroli (4):
      drm/panthor: Add panthor_*_irq_mask_set helper
      drm/panthor: Add SHADER_PWRFEATURES register definition
      drm/panthor: Add tracepoint for hardware utilisation changes
      drm/panthor: Add gpu_job_irq tracepoint

 drivers/gpu/drm/panthor/panthor_device.h |  7 +++
 drivers/gpu/drm/panthor/panthor_fw.c     | 13 +++++
 drivers/gpu/drm/panthor/panthor_gpu.c    | 96 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/panthor/panthor_regs.h   |  2 +
 drivers/gpu/drm/panthor/panthor_trace.h  | 90 ++++++++++++++++++++++++++++++
 5 files changed, 206 insertions(+), 2 deletions(-)
---
base-commit: 8362fcabaa24e5d632c7d0fa35f74103e74e8850
change-id: 20251203-panthor-tracepoints-488af09d46e7

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v2 1/4] drm/panthor: Add panthor_*_irq_mask_set helper

[Nicolas Frattaroli]

Add a function to modify an IRQ's mask. If the IRQ is currently active,
it will write to the register, otherwise it will only set the struct
member.

There's no locking done to guarantee exclusion with the other two
functions that touch the IRQ mask, and it should only be called from a
context where the circumstances guarantee no concurrent access is
performed.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_device.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index f35e52b9546a..894d28b3eb02 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -470,6 +470,13 @@ static int panthor_request_ ## __name ## _irq(struct panthor_device *ptdev,			\
 					 panthor_ ## __name ## _irq_threaded_handler,		\
 					 IRQF_SHARED, KBUILD_MODNAME "-" # __name,		\
 					 pirq);							\
+}												\
+												\
+static inline void panthor_ ## __name ## _irq_mask_set(struct panthor_irq *pirq, u32 mask)	\
+{												\
+	pirq->mask = mask;									\
+	if (!atomic_read(&pirq->suspended))							\
+		gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, mask);			\
 }
 
 extern struct workqueue_struct *panthor_cleanup_wq;

-- 
2.52.0

[PATCH v2 2/4] drm/panthor: Add SHADER_PWRFEATURES register definition

[Nicolas Frattaroli]

Available starting with Mali v12, the SHADER_PWRFEATURES register is a
32-bit register that contains a single bit so far. This bit flag
indicates whether the RT cores should be powered up or not.

The name "SHADER_PWRFEATURES" (as opposed to "SHADER_POWER_FEATURES") is
taken directly from official hardware documentation.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_regs.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/panthor/panthor_regs.h b/drivers/gpu/drm/panthor/panthor_regs.h
index 08bf06c452d6..4c79cec8dc21 100644
--- a/drivers/gpu/drm/panthor/panthor_regs.h
+++ b/drivers/gpu/drm/panthor/panthor_regs.h
@@ -96,6 +96,8 @@
 #define L2_READY					0x160
 
 #define SHADER_PWRON					0x180
+#define SHADER_PWRFEATURES				0x188
+#define   SHADER_PWRFEATURES_RT				BIT(0)
 #define TILER_PWRON					0x190
 #define L2_PWRON					0x1A0
 

-- 
2.52.0

[PATCH v2 3/4] drm/panthor: Add tracepoint for hardware utilisation changes

[Nicolas Frattaroli]

Mali GPUs have three registers that indicate which parts of the hardware
are powered at any moment. These take the form of bitmaps. In the case
of SHADER_READY for example, a high bit indicates that the shader core
corresponding to that bit index is powered on. These bitmaps aren't
solely contiguous bits, as it's common to have holes in the sequence of
shader core indices, and the actual set of which cores are present is
defined by the "shader present" register.

When the GPU finishes a power state transition, it fires a
GPU_IRQ_POWER_CHANGED_ALL interrupt. After such an interrupt is
received, the _READY registers will contain new interesting data. During
power transitions, the GPU_IRQ_POWER_CHANGED interrupt will fire, and
the registers will likewise contain potentially changed data.

This is not to be confused with the PWR_IRQ_POWER_CHANGED_ALL interrupt,
which is something related to Mali v14+'s power control logic. The
_READY registers and corresponding interrupts are already available in
v9 and onwards.

Additionally, the SHADER_PWRFEATURES register may be of interest, which
contains a bit flag indicating whether raytracing functionality is
turned on, as the ray tracing unit's power can separately be toggled.
Reading this register on platforms from before it was added has no
unpleasant side-effects; it's officially specified to read as 0 in this
case.

Expose the data as a tracepoint to userspace. This allows users to debug
various scenarios and gather interesting information, such as: knowing
how much hardware is lit up at any given time, correlating graphics
corruption with a specific powered shader core, measuring when hardware
is allowed to go to a powered off state again, and so on.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_gpu.c   | 96 ++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/panthor/panthor_trace.h | 62 +++++++++++++++++++++
 2 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
index 057e167468d0..e86bca9ffd10 100644
--- a/drivers/gpu/drm/panthor/panthor_gpu.c
+++ b/drivers/gpu/drm/panthor/panthor_gpu.c
@@ -22,6 +22,9 @@
 #include "panthor_hw.h"
 #include "panthor_regs.h"
 
+#define CREATE_TRACE_POINTS
+#include "panthor_trace.h"
+
 /**
  * struct panthor_gpu - GPU block management data.
  */
@@ -29,6 +32,9 @@ struct panthor_gpu {
 	/** @irq: GPU irq. */
 	struct panthor_irq irq;
 
+	/** @irq_mask: GPU irq mask. */
+	u32 irq_mask;
+
 	/** @reqs_lock: Lock protecting access to pending_reqs. */
 	spinlock_t reqs_lock;
 
@@ -48,6 +54,9 @@ struct panthor_gpu {
 	 GPU_IRQ_RESET_COMPLETED | \
 	 GPU_IRQ_CLEAN_CACHES_COMPLETED)
 
+#define GPU_POWER_INTERRUPTS_MASK	\
+	(GPU_IRQ_POWER_CHANGED | GPU_IRQ_POWER_CHANGED_ALL)
+
 static void panthor_gpu_coherency_set(struct panthor_device *ptdev)
 {
 	gpu_write(ptdev, GPU_COHERENCY_PROTOCOL,
@@ -80,6 +89,14 @@ static void panthor_gpu_irq_handler(struct panthor_device *ptdev, u32 status)
 {
 	gpu_write(ptdev, GPU_INT_CLEAR, status);
 
+	if (tracepoint_enabled(gpu_power_status) && (status & GPU_POWER_INTERRUPTS_MASK))
+		trace_gpu_power_status(ptdev->base.dev,
+				       gpu_read64(ptdev, SHADER_READY),
+				       gpu_read64(ptdev, TILER_READY),
+				       gpu_read64(ptdev, L2_READY),
+				       gpu_read(ptdev, SHADER_PWRFEATURES) &
+				       SHADER_PWRFEATURES_RT);
+
 	if (status & GPU_IRQ_FAULT) {
 		u32 fault_status = gpu_read(ptdev, GPU_FAULT_STATUS);
 		u64 address = gpu_read64(ptdev, GPU_FAULT_ADDR);
@@ -139,6 +156,7 @@ int panthor_gpu_init(struct panthor_device *ptdev)
 	init_waitqueue_head(&gpu->reqs_acked);
 	mutex_init(&gpu->cache_flush_lock);
 	ptdev->gpu = gpu;
+	gpu->irq_mask = GPU_INTERRUPTS_MASK;
 
 	dma_set_max_seg_size(ptdev->base.dev, UINT_MAX);
 	pa_bits = GPU_MMU_FEATURES_PA_BITS(ptdev->gpu_info.mmu_features);
@@ -150,13 +168,87 @@ int panthor_gpu_init(struct panthor_device *ptdev)
 	if (irq < 0)
 		return irq;
 
-	ret = panthor_request_gpu_irq(ptdev, &ptdev->gpu->irq, irq, GPU_INTERRUPTS_MASK);
+	ret = panthor_request_gpu_irq(ptdev, &ptdev->gpu->irq, irq, gpu->irq_mask);
 	if (ret)
 		return ret;
 
 	return 0;
 }
 
+static int panthor_gpu_set_power_tracing(struct device *dev, void *data)
+{
+	struct panthor_device *ptdev = dev_get_drvdata(dev);
+	u32 mask = GPU_INTERRUPTS_MASK;
+
+	if (!ptdev || !ptdev->gpu)
+		return -ENODEV;
+
+	/* v14+ don't use this interrupt for this purpose, skip */
+	if (GPU_ARCH_MAJOR(ptdev->gpu_info.gpu_id) > 13)
+		return 0;
+
+	if (data)
+		mask |= GPU_POWER_INTERRUPTS_MASK;
+
+	guard(pm_runtime_active)(dev);
+
+	ptdev->gpu->irq_mask = mask;
+	panthor_gpu_irq_mask_set(&ptdev->gpu->irq, mask);
+
+	return 0;
+}
+
+/**
+ * panthor_gpu_power_status_register - reg callback for power_status tracepoint
+ *
+ * This function gets called when the gpu_power_status tracepoint is enabled.
+ * Its purpose is to modify the interrupt mask for the GPU interrupt.
+ *
+ * Returns 0 on success, negative errno otherwise.
+ */
+int panthor_gpu_power_status_register(void)
+{
+	struct device_driver *drv;
+	int ret;
+
+	drv = driver_find("panthor", &platform_bus_type);
+	if (!drv)
+		return -ENODEV;
+
+	ret = driver_for_each_device(drv, NULL, (void *)true,
+				     panthor_gpu_set_power_tracing);
+
+	return ret;
+}
+
+/**
+ * panthor_gpu_power_status_unregister - unreg callback for power_status tracepoint
+ *
+ * This function gets called when the gpu_power_status tracepoint is disabled.
+ * Its purpose is to modify the interrupt mask for the GPU interrupt.
+ */
+void panthor_gpu_power_status_unregister(void)
+{
+	struct device_driver *drv;
+	int ret;
+
+	drv = driver_find("panthor", &platform_bus_type);
+	if (!drv)
+		return;
+
+	ret = driver_for_each_device(drv, NULL, NULL, panthor_gpu_set_power_tracing);
+
+	/*
+	 * Ideally, it'd be possible to ask driver_for_each_device to hand us
+	 * another "start" to keep going after the failing device, but it
+	 * doesn't do that. Minor inconvenience in what is probably a bad day
+	 * on the computer already though.
+	 */
+	if (ret)
+		pr_warn("Couldn't mask power IRQ for at least one device: %pe\n",
+			ERR_PTR(ret));
+}
+
 /**
  * panthor_gpu_block_power_off() - Power-off a specific block of the GPU
  * @ptdev: Device.
@@ -395,7 +487,7 @@ void panthor_gpu_suspend(struct panthor_device *ptdev)
  */
 void panthor_gpu_resume(struct panthor_device *ptdev)
 {
-	panthor_gpu_irq_resume(&ptdev->gpu->irq, GPU_INTERRUPTS_MASK);
+	panthor_gpu_irq_resume(&ptdev->gpu->irq, ptdev->gpu->irq_mask);
 	panthor_hw_l2_power_on(ptdev);
 }
 
diff --git a/drivers/gpu/drm/panthor/panthor_trace.h b/drivers/gpu/drm/panthor/panthor_trace.h
new file mode 100644
index 000000000000..8258217066fc
--- /dev/null
+++ b/drivers/gpu/drm/panthor/panthor_trace.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 or MIT */
+/* Copyright 2025 Collabora ltd. */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM panthor
+
+#if !defined(__PANTHOR_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ)
+#define __PANTHOR_TRACE_H__
+
+#include <linux/tracepoint.h>
+#include <linux/types.h>
+
+int panthor_gpu_power_status_register(void);
+void panthor_gpu_power_status_unregister(void);
+
+/**
+ * gpu_power_status - called whenever parts of GPU hardware are turned on or off
+ * @dev: pointer to the &struct device, for printing the device name
+ * @shader_bitmap: bitmap where a high bit indicates the shader core at a given
+ *                 bit index is on, and a low bit indicates a shader core is
+ *                 either powered off or absent
+ * @tiler_bitmap: bitmap where a high bit indicates the tiler unit at a given
+ *                bit index is on, and a low bit indicates a tiler unit is
+ *                either powered off or absent
+ * @l2_bitmap: bitmap where a high bit indicates the L2 cache at a given bit
+ *             index is on, and a low bit indicates the L2 cache is either
+ *             powered off or absent
+ * @rt_on: 0 all the raytracing units are off, 1 if they're all powered on.
+ */
+TRACE_EVENT_FN(gpu_power_status,
+	TP_PROTO(const struct device *dev, u64 shader_bitmap, u64 tiler_bitmap,
+		 u64 l2_bitmap, bool rt_on),
+	TP_ARGS(dev, shader_bitmap, tiler_bitmap, l2_bitmap, rt_on),
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(dev))
+		__field(u64, shader_bitmap)
+		__field(u64, tiler_bitmap)
+		__field(u64, l2_bitmap)
+		__field(bool, rt_on)
+	),
+	TP_fast_assign(
+		__assign_str(dev_name);
+		__entry->shader_bitmap	= shader_bitmap;
+		__entry->tiler_bitmap	= tiler_bitmap;
+		__entry->l2_bitmap	= l2_bitmap;
+		__entry->rt_on		= rt_on;
+	),
+	TP_printk("%s: shader_bitmap=0x%llx tiler_bitmap=0x%llx l2_bitmap=0x%llx rt_on=%d",
+		  __get_str(dev_name), __entry->shader_bitmap, __entry->tiler_bitmap,
+		  __entry->l2_bitmap, __entry->rt_on
+	),
+	panthor_gpu_power_status_register, panthor_gpu_power_status_unregister
+);
+
+#endif /* __PANTHOR_TRACE_H__ */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE panthor_trace
+
+#include <trace/define_trace.h>

-- 
2.52.0

[PATCH v2 4/4] drm/panthor: Add gpu_job_irq tracepoint

[Nicolas Frattaroli]

Mali's CSF firmware triggers the job IRQ whenever there's new firmware
events for processing. While this can be a global event (BIT(31) of the
status register), it's usually an event relating to a command stream
group (the other bit indices).

Panthor throws these events onto a workqueue for processing outside the
IRQ handler. It's therefore useful to have an instrumented tracepoint
that goes beyond the generic IRQ tracepoint for this specific case, as
it can be augmented with additional data, namely the events bit mask.

This can then be used to debug problems relating to GPU jobs events not
being processed quickly enough. The duration_ns field can be used to
work backwards from when the tracepoint fires (at the end of the IRQ
handler) to figure out when the interrupt itself landed, providing not
just information on how long the work queueing took, but also when the
actual interrupt itself arrived.

With this information in hand, the IRQ handler itself being slow can be
excluded as a possible source of problems, and attention can be directed
to the workqueue processing instead.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_fw.c    | 13 +++++++++++++
 drivers/gpu/drm/panthor/panthor_trace.h | 28 ++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index 4beaa589ba66..1823d7ba5c78 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -26,6 +26,7 @@
 #include "panthor_mmu.h"
 #include "panthor_regs.h"
 #include "panthor_sched.h"
+#include "panthor_trace.h"
 
 #define CSF_FW_NAME "mali_csffw.bin"
 
@@ -1060,6 +1061,12 @@ static void panthor_fw_init_global_iface(struct panthor_device *ptdev)
 
 static void panthor_job_irq_handler(struct panthor_device *ptdev, u32 status)
 {
+	u32 duration;
+	u64 start;
+
+	if (tracepoint_enabled(gpu_job_irq))
+		start = ktime_get_ns();
+
 	gpu_write(ptdev, JOB_INT_CLEAR, status);
 
 	if (!ptdev->fw->booted && (status & JOB_INT_GLOBAL_IF))
@@ -1072,6 +1079,12 @@ static void panthor_job_irq_handler(struct panthor_device *ptdev, u32 status)
 		return;
 
 	panthor_sched_report_fw_events(ptdev, status);
+
+	if (tracepoint_enabled(gpu_job_irq)) {
+		if (check_sub_overflow(ktime_get_ns(), start, &duration))
+			duration = U32_MAX;
+		trace_gpu_job_irq(ptdev->base.dev, status, duration);
+	}
 }
 PANTHOR_IRQ_HANDLER(job, JOB, panthor_job_irq_handler);
 
diff --git a/drivers/gpu/drm/panthor/panthor_trace.h b/drivers/gpu/drm/panthor/panthor_trace.h
index 8258217066fc..c5ceda52a6b1 100644
--- a/drivers/gpu/drm/panthor/panthor_trace.h
+++ b/drivers/gpu/drm/panthor/panthor_trace.h
@@ -52,6 +52,34 @@ TRACE_EVENT_FN(gpu_power_status,
 	panthor_gpu_power_status_register, panthor_gpu_power_status_unregister
 );
 
+/**
+ * gpu_job_irq - called after a job interrupt from firmware completes
+ * @dev: pointer to the &struct device, for printing the device name
+ * @events: bitmask of BIT(CSG id) | BIT(31) for a global event
+ * @duration_ns: Nanoseconds between job IRQ handler entry and exit
+ *
+ * The panthor_job_irq_handler() function instrumented by this tracepoint exits
+ * once it has queued the firmware interrupts for processing, not when the
+ * firmware interrupts are fully processed. This tracepoint allows for debugging
+ * issues with delays in the workqueue's processing of events.
+ */
+TRACE_EVENT(gpu_job_irq,
+	TP_PROTO(const struct device *dev, u32 events, u32 duration_ns),
+	TP_ARGS(dev, events, duration_ns),
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(dev))
+		__field(u32, events)
+		__field(u32, duration_ns)
+	),
+	TP_fast_assign(
+		__assign_str(dev_name);
+		__entry->events		= events;
+		__entry->duration_ns	= duration_ns;
+	),
+	TP_printk("%s: events=0x%x duration_ns=%d", __get_str(dev_name),
+		  __entry->events, __entry->duration_ns)
+);
+
 #endif /* __PANTHOR_TRACE_H__ */
 
 #undef TRACE_INCLUDE_PATH

-- 
2.52.0

Re: [PATCH v2 1/4] drm/panthor: Add panthor_*_irq_mask_set helper

[Steven Price]

On 10/12/2025 14:30, Nicolas Frattaroli wrote:
> Add a function to modify an IRQ's mask. If the IRQ is currently active,
> it will write to the register, otherwise it will only set the struct
> member.
> 
> There's no locking done to guarantee exclusion with the other two
> functions that touch the IRQ mask, and it should only be called from a
> context where the circumstances guarantee no concurrent access is
> performed.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Usually it's best to add the helper at the same time as the first user
(especially if it's small like this), otherwise it's really difficult to
see whether the helper is the right shape.

The comment about no locking is worrying without the context to check if
this is reasonable.

Thanks,
Steve

> ---
>  drivers/gpu/drm/panthor/panthor_device.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index f35e52b9546a..894d28b3eb02 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -470,6 +470,13 @@ static int panthor_request_ ## __name ## _irq(struct panthor_device *ptdev,			\
>  					 panthor_ ## __name ## _irq_threaded_handler,		\
>  					 IRQF_SHARED, KBUILD_MODNAME "-" # __name,		\
>  					 pirq);							\
> +}												\
> +												\
> +static inline void panthor_ ## __name ## _irq_mask_set(struct panthor_irq *pirq, u32 mask)	\
> +{												\
> +	pirq->mask = mask;									\
> +	if (!atomic_read(&pirq->suspended))							\
> +		gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, mask);			\
>  }
>  
>  extern struct workqueue_struct *panthor_cleanup_wq;
> 

Re: [PATCH v2 3/4] drm/panthor: Add tracepoint for hardware utilisation changes

[Steven Price]

On 10/12/2025 14:30, Nicolas Frattaroli wrote:
> Mali GPUs have three registers that indicate which parts of the hardware
> are powered at any moment. These take the form of bitmaps. In the case
> of SHADER_READY for example, a high bit indicates that the shader core
> corresponding to that bit index is powered on. These bitmaps aren't
> solely contiguous bits, as it's common to have holes in the sequence of
> shader core indices, and the actual set of which cores are present is
> defined by the "shader present" register.
> 
> When the GPU finishes a power state transition, it fires a
> GPU_IRQ_POWER_CHANGED_ALL interrupt. After such an interrupt is
> received, the _READY registers will contain new interesting data. During
> power transitions, the GPU_IRQ_POWER_CHANGED interrupt will fire, and
> the registers will likewise contain potentially changed data.
> 
> This is not to be confused with the PWR_IRQ_POWER_CHANGED_ALL interrupt,
> which is something related to Mali v14+'s power control logic. The
> _READY registers and corresponding interrupts are already available in
> v9 and onwards.
> 
> Additionally, the SHADER_PWRFEATURES register may be of interest, which
> contains a bit flag indicating whether raytracing functionality is
> turned on, as the ray tracing unit's power can separately be toggled.
> Reading this register on platforms from before it was added has no
> unpleasant side-effects; it's officially specified to read as 0 in this
> case.

I'm confused by this addition, SHADER_PWRFEATURES is sampled (by the
hardware) on power up of a shader core. So the value of the register
isn't necessarily representative of the actual hardware state. In normal
operation it is controlled by the MCU and so probably has some
correlation to ray tracing happening, but I don't think there's any
gaurentee there. And on later GPUs this functionality has been moved
into the POWER_CONTROL block and I don't think there's anything equivalent.

Also in general I wouldn't rely on the "read as 0" parts - not
specifically because I expect HW bugs, but because later GPUs might
reuse those register positions for other things.

So this seems to only work on a small number of GPUs and even then it's
relying on a firmware behaviour which isn't gaurenteed.

It would be good if we can come up with a tracepoint which is both
useful and likely to work over a range of GPUs.

Thanks,
Steve

> Expose the data as a tracepoint to userspace. This allows users to debug
> various scenarios and gather interesting information, such as: knowing
> how much hardware is lit up at any given time, correlating graphics
> corruption with a specific powered shader core, measuring when hardware
> is allowed to go to a powered off state again, and so on.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_gpu.c   | 96 ++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/panthor/panthor_trace.h | 62 +++++++++++++++++++++
>  2 files changed, 156 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 057e167468d0..e86bca9ffd10 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -22,6 +22,9 @@
>  #include "panthor_hw.h"
>  #include "panthor_regs.h"
>  
> +#define CREATE_TRACE_POINTS
> +#include "panthor_trace.h"
> +
>  /**
>   * struct panthor_gpu - GPU block management data.
>   */
> @@ -29,6 +32,9 @@ struct panthor_gpu {
>  	/** @irq: GPU irq. */
>  	struct panthor_irq irq;
>  
> +	/** @irq_mask: GPU irq mask. */
> +	u32 irq_mask;
> +
>  	/** @reqs_lock: Lock protecting access to pending_reqs. */
>  	spinlock_t reqs_lock;
>  
> @@ -48,6 +54,9 @@ struct panthor_gpu {
>  	 GPU_IRQ_RESET_COMPLETED | \
>  	 GPU_IRQ_CLEAN_CACHES_COMPLETED)
>  
> +#define GPU_POWER_INTERRUPTS_MASK	\
> +	(GPU_IRQ_POWER_CHANGED | GPU_IRQ_POWER_CHANGED_ALL)
> +
>  static void panthor_gpu_coherency_set(struct panthor_device *ptdev)
>  {
>  	gpu_write(ptdev, GPU_COHERENCY_PROTOCOL,
> @@ -80,6 +89,14 @@ static void panthor_gpu_irq_handler(struct panthor_device *ptdev, u32 status)
>  {
>  	gpu_write(ptdev, GPU_INT_CLEAR, status);
>  
> +	if (tracepoint_enabled(gpu_power_status) && (status & GPU_POWER_INTERRUPTS_MASK))
> +		trace_gpu_power_status(ptdev->base.dev,
> +				       gpu_read64(ptdev, SHADER_READY),
> +				       gpu_read64(ptdev, TILER_READY),
> +				       gpu_read64(ptdev, L2_READY),
> +				       gpu_read(ptdev, SHADER_PWRFEATURES) &
> +				       SHADER_PWRFEATURES_RT);
> +
>  	if (status & GPU_IRQ_FAULT) {
>  		u32 fault_status = gpu_read(ptdev, GPU_FAULT_STATUS);
>  		u64 address = gpu_read64(ptdev, GPU_FAULT_ADDR);
> @@ -139,6 +156,7 @@ int panthor_gpu_init(struct panthor_device *ptdev)
>  	init_waitqueue_head(&gpu->reqs_acked);
>  	mutex_init(&gpu->cache_flush_lock);
>  	ptdev->gpu = gpu;
> +	gpu->irq_mask = GPU_INTERRUPTS_MASK;
>  
>  	dma_set_max_seg_size(ptdev->base.dev, UINT_MAX);
>  	pa_bits = GPU_MMU_FEATURES_PA_BITS(ptdev->gpu_info.mmu_features);
> @@ -150,13 +168,87 @@ int panthor_gpu_init(struct panthor_device *ptdev)
>  	if (irq < 0)
>  		return irq;
>  
> -	ret = panthor_request_gpu_irq(ptdev, &ptdev->gpu->irq, irq, GPU_INTERRUPTS_MASK);
> +	ret = panthor_request_gpu_irq(ptdev, &ptdev->gpu->irq, irq, gpu->irq_mask);
>  	if (ret)
>  		return ret;
>  
>  	return 0;
>  }
>  
> +static int panthor_gpu_set_power_tracing(struct device *dev, void *data)
> +{
> +	struct panthor_device *ptdev = dev_get_drvdata(dev);
> +	u32 mask = GPU_INTERRUPTS_MASK;
> +
> +	if (!ptdev || !ptdev->gpu)
> +		return -ENODEV;
> +
> +	/* v14+ don't use this interrupt for this purpose, skip */
> +	if (GPU_ARCH_MAJOR(ptdev->gpu_info.gpu_id) > 13)
> +		return 0;
> +
> +	if (data)
> +		mask |= GPU_POWER_INTERRUPTS_MASK;
> +
> +	guard(pm_runtime_active)(dev);
> +
> +	ptdev->gpu->irq_mask = mask;
> +	panthor_gpu_irq_mask_set(&ptdev->gpu->irq, mask);
> +
> +	return 0;
> +}
> +
> +/**
> + * panthor_gpu_power_status_register - reg callback for power_status tracepoint
> + *
> + * This function gets called when the gpu_power_status tracepoint is enabled.
> + * Its purpose is to modify the interrupt mask for the GPU interrupt.
> + *
> + * Returns 0 on success, negative errno otherwise.
> + */
> +int panthor_gpu_power_status_register(void)
> +{
> +	struct device_driver *drv;
> +	int ret;
> +
> +	drv = driver_find("panthor", &platform_bus_type);
> +	if (!drv)
> +		return -ENODEV;
> +
> +	ret = driver_for_each_device(drv, NULL, (void *)true,
> +				     panthor_gpu_set_power_tracing);
> +
> +	return ret;
> +}
> +
> +/**
> + * panthor_gpu_power_status_unregister - unreg callback for power_status tracepoint
> + *
> + * This function gets called when the gpu_power_status tracepoint is disabled.
> + * Its purpose is to modify the interrupt mask for the GPU interrupt.
> + */
> +void panthor_gpu_power_status_unregister(void)
> +{
> +	struct device_driver *drv;
> +	int ret;
> +
> +	drv = driver_find("panthor", &platform_bus_type);
> +	if (!drv)
> +		return;
> +
> +	ret = driver_for_each_device(drv, NULL, NULL, panthor_gpu_set_power_tracing);
> +
> +	/*
> +	 * Ideally, it'd be possible to ask driver_for_each_device to hand us
> +	 * another "start" to keep going after the failing device, but it
> +	 * doesn't do that. Minor inconvenience in what is probably a bad day
> +	 * on the computer already though.
> +	 */
> +	if (ret)
> +		pr_warn("Couldn't mask power IRQ for at least one device: %pe\n",
> +			ERR_PTR(ret));
> +}
> +
>  /**
>   * panthor_gpu_block_power_off() - Power-off a specific block of the GPU
>   * @ptdev: Device.
> @@ -395,7 +487,7 @@ void panthor_gpu_suspend(struct panthor_device *ptdev)
>   */
>  void panthor_gpu_resume(struct panthor_device *ptdev)
>  {
> -	panthor_gpu_irq_resume(&ptdev->gpu->irq, GPU_INTERRUPTS_MASK);
> +	panthor_gpu_irq_resume(&ptdev->gpu->irq, ptdev->gpu->irq_mask);
>  	panthor_hw_l2_power_on(ptdev);
>  }
>  
> diff --git a/drivers/gpu/drm/panthor/panthor_trace.h b/drivers/gpu/drm/panthor/panthor_trace.h
> new file mode 100644
> index 000000000000..8258217066fc
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/panthor_trace.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 or MIT */
> +/* Copyright 2025 Collabora ltd. */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM panthor
> +
> +#if !defined(__PANTHOR_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ)
> +#define __PANTHOR_TRACE_H__
> +
> +#include <linux/tracepoint.h>
> +#include <linux/types.h>
> +
> +int panthor_gpu_power_status_register(void);
> +void panthor_gpu_power_status_unregister(void);
> +
> +/**
> + * gpu_power_status - called whenever parts of GPU hardware are turned on or off
> + * @dev: pointer to the &struct device, for printing the device name
> + * @shader_bitmap: bitmap where a high bit indicates the shader core at a given
> + *                 bit index is on, and a low bit indicates a shader core is
> + *                 either powered off or absent
> + * @tiler_bitmap: bitmap where a high bit indicates the tiler unit at a given
> + *                bit index is on, and a low bit indicates a tiler unit is
> + *                either powered off or absent
> + * @l2_bitmap: bitmap where a high bit indicates the L2 cache at a given bit
> + *             index is on, and a low bit indicates the L2 cache is either
> + *             powered off or absent
> + * @rt_on: 0 all the raytracing units are off, 1 if they're all powered on.
> + */
> +TRACE_EVENT_FN(gpu_power_status,
> +	TP_PROTO(const struct device *dev, u64 shader_bitmap, u64 tiler_bitmap,
> +		 u64 l2_bitmap, bool rt_on),
> +	TP_ARGS(dev, shader_bitmap, tiler_bitmap, l2_bitmap, rt_on),
> +	TP_STRUCT__entry(
> +		__string(dev_name, dev_name(dev))
> +		__field(u64, shader_bitmap)
> +		__field(u64, tiler_bitmap)
> +		__field(u64, l2_bitmap)
> +		__field(bool, rt_on)
> +	),
> +	TP_fast_assign(
> +		__assign_str(dev_name);
> +		__entry->shader_bitmap	= shader_bitmap;
> +		__entry->tiler_bitmap	= tiler_bitmap;
> +		__entry->l2_bitmap	= l2_bitmap;
> +		__entry->rt_on		= rt_on;
> +	),
> +	TP_printk("%s: shader_bitmap=0x%llx tiler_bitmap=0x%llx l2_bitmap=0x%llx rt_on=%d",
> +		  __get_str(dev_name), __entry->shader_bitmap, __entry->tiler_bitmap,
> +		  __entry->l2_bitmap, __entry->rt_on
> +	),
> +	panthor_gpu_power_status_register, panthor_gpu_power_status_unregister
> +);
> +
> +#endif /* __PANTHOR_TRACE_H__ */
> +
> +#undef TRACE_INCLUDE_PATH
> +#define TRACE_INCLUDE_PATH .
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_FILE panthor_trace
> +
> +#include <trace/define_trace.h>
> 

Re: [PATCH v2 3/4] drm/panthor: Add tracepoint for hardware utilisation changes

[Nicolas Frattaroli]

On Wednesday, 10 December 2025 16:57:42 Central European Standard Time Steven Price wrote:
> On 10/12/2025 14:30, Nicolas Frattaroli wrote:
> > Mali GPUs have three registers that indicate which parts of the hardware
> > are powered at any moment. These take the form of bitmaps. In the case
> > of SHADER_READY for example, a high bit indicates that the shader core
> > corresponding to that bit index is powered on. These bitmaps aren't
> > solely contiguous bits, as it's common to have holes in the sequence of
> > shader core indices, and the actual set of which cores are present is
> > defined by the "shader present" register.
> > 
> > When the GPU finishes a power state transition, it fires a
> > GPU_IRQ_POWER_CHANGED_ALL interrupt. After such an interrupt is
> > received, the _READY registers will contain new interesting data. During
> > power transitions, the GPU_IRQ_POWER_CHANGED interrupt will fire, and
> > the registers will likewise contain potentially changed data.
> > 
> > This is not to be confused with the PWR_IRQ_POWER_CHANGED_ALL interrupt,
> > which is something related to Mali v14+'s power control logic. The
> > _READY registers and corresponding interrupts are already available in
> > v9 and onwards.
> > 
> > Additionally, the SHADER_PWRFEATURES register may be of interest, which
> > contains a bit flag indicating whether raytracing functionality is
> > turned on, as the ray tracing unit's power can separately be toggled.
> > Reading this register on platforms from before it was added has no
> > unpleasant side-effects; it's officially specified to read as 0 in this
> > case.
> 
> I'm confused by this addition, SHADER_PWRFEATURES is sampled (by the
> hardware) on power up of a shader core. So the value of the register
> isn't necessarily representative of the actual hardware state. In normal
> operation it is controlled by the MCU and so probably has some
> correlation to ray tracing happening, but I don't think there's any
> gaurentee there. And on later GPUs this functionality has been moved
> into the POWER_CONTROL block and I don't think there's anything equivalent.

I was afraid that would be the case based on the hw docs wording.
In that case yeah, I'll just drop it for now. My hope was that
SHADER_PWRFEATURES would contain whatever the current status of the
hardware is, aside from just being the place to write the desired
status as well.

> Also in general I wouldn't rely on the "read as 0" parts - not
> specifically because I expect HW bugs, but because later GPUs might
> reuse those register positions for other things.

That's fun. After I remove the SHADER_PWRFEATURES register, the
register reads that will be left can only happen if the
GPU_IRQ_POWER_CHANGED/GPU_IRQ_POWER_CHANGED_ALL interrupt status
bits are set. Unless Arm has plans to reuse those bit values in
future GPUs, and someone adds them to Panthor (shadowing the old
ones), and someone then enables the tracepoint, then these reads
should never happen on hardware that doesn't have the _READY regs.

> So this seems to only work on a small number of GPUs and even then it's
> relying on a firmware behaviour which isn't gaurenteed.
>
> It would be good if we can come up with a tracepoint which is both
> useful and likely to work over a range of GPUs.

If I understand correctly, the RTU power status isn't something
that can be read like SHADER_READY on v14+. In that case, I'll just
drop the rt_on field entirely, because on v13 it doesn't necessarily
do what I want it to do, and on v14+, it doesn't exist. It was an
added "oh this is simple enough" bonus, the main goal is to have the
tracepoints to track power events.

On that note, I'd love to implement the trace event call on v14+ as
well, because I'm fairly sure it can re-use the same tracepoint
definition. I might need to generalize the register and unregister
functions though, probably by moving them somewhere else, and then
having them call into a per-hw-version function pointer. With no
v14+ hardware to test it on, I won't implement that variant for now
however.

Should I generalize the tracepoint register/unregister in this way
right now even without the v14+ implementation? Might make it more
palatable.

Kind regards,
Nicolas Frattaroli
 
> Thanks,
> Steve

Re: [PATCH v2 3/4] drm/panthor: Add tracepoint for hardware utilisation changes

[Steven Price]

On 10/12/2025 16:27, Nicolas Frattaroli wrote:
> On Wednesday, 10 December 2025 16:57:42 Central European Standard Time Steven Price wrote:
>> On 10/12/2025 14:30, Nicolas Frattaroli wrote:
>>> Mali GPUs have three registers that indicate which parts of the hardware
>>> are powered at any moment. These take the form of bitmaps. In the case
>>> of SHADER_READY for example, a high bit indicates that the shader core
>>> corresponding to that bit index is powered on. These bitmaps aren't
>>> solely contiguous bits, as it's common to have holes in the sequence of
>>> shader core indices, and the actual set of which cores are present is
>>> defined by the "shader present" register.
>>>
>>> When the GPU finishes a power state transition, it fires a
>>> GPU_IRQ_POWER_CHANGED_ALL interrupt. After such an interrupt is
>>> received, the _READY registers will contain new interesting data. During
>>> power transitions, the GPU_IRQ_POWER_CHANGED interrupt will fire, and
>>> the registers will likewise contain potentially changed data.
>>>
>>> This is not to be confused with the PWR_IRQ_POWER_CHANGED_ALL interrupt,
>>> which is something related to Mali v14+'s power control logic. The
>>> _READY registers and corresponding interrupts are already available in
>>> v9 and onwards.
>>>
>>> Additionally, the SHADER_PWRFEATURES register may be of interest, which
>>> contains a bit flag indicating whether raytracing functionality is
>>> turned on, as the ray tracing unit's power can separately be toggled.
>>> Reading this register on platforms from before it was added has no
>>> unpleasant side-effects; it's officially specified to read as 0 in this
>>> case.
>>
>> I'm confused by this addition, SHADER_PWRFEATURES is sampled (by the
>> hardware) on power up of a shader core. So the value of the register
>> isn't necessarily representative of the actual hardware state. In normal
>> operation it is controlled by the MCU and so probably has some
>> correlation to ray tracing happening, but I don't think there's any
>> gaurentee there. And on later GPUs this functionality has been moved
>> into the POWER_CONTROL block and I don't think there's anything equivalent.
> 
> I was afraid that would be the case based on the hw docs wording.
> In that case yeah, I'll just drop it for now. My hope was that
> SHADER_PWRFEATURES would contain whatever the current status of the
> hardware is, aside from just being the place to write the desired
> status as well.
> 
>> Also in general I wouldn't rely on the "read as 0" parts - not
>> specifically because I expect HW bugs, but because later GPUs might
>> reuse those register positions for other things.
> 
> That's fun. After I remove the SHADER_PWRFEATURES register, the
> register reads that will be left can only happen if the
> GPU_IRQ_POWER_CHANGED/GPU_IRQ_POWER_CHANGED_ALL interrupt status
> bits are set. Unless Arm has plans to reuse those bit values in
> future GPUs, and someone adds them to Panthor (shadowing the old
> ones), and someone then enables the tracepoint, then these reads
> should never happen on hardware that doesn't have the _READY regs.

Yeah I think that sort of thing should be safe - the GPU shouldn't
produce IRQs for things we haven't unmasked - so we won't see them. If
(/when) in the future those bits get reused then we can deal with that
when Panthor is updated to use the new meaning of the bits. Arguably we
could drop GPU_IRQ_POWER_CHANGED from the bit mask (we're not enabling
it so it should never fire) - which is the one I can see proposed for
reuse ;)

>> So this seems to only work on a small number of GPUs and even then it's
>> relying on a firmware behaviour which isn't gaurenteed.
>>
>> It would be good if we can come up with a tracepoint which is both
>> useful and likely to work over a range of GPUs.
> 
> If I understand correctly, the RTU power status isn't something
> that can be read like SHADER_READY on v14+. In that case, I'll just
> drop the rt_on field entirely, because on v13 it doesn't necessarily
> do what I want it to do, and on v14+, it doesn't exist. It was an
> added "oh this is simple enough" bonus, the main goal is to have the
> tracepoints to track power events.

Yeah I don't believe that status is exposed when the MCU is handling the
power control.

> On that note, I'd love to implement the trace event call on v14+ as
> well, because I'm fairly sure it can re-use the same tracepoint
> definition. I might need to generalize the register and unregister
> functions though, probably by moving them somewhere else, and then
> having them call into a per-hw-version function pointer. With no
> v14+ hardware to test it on, I won't implement that variant for now
> however.
> 
> Should I generalize the tracepoint register/unregister in this way
> right now even without the v14+ implementation? Might make it more
> palatable.

It would be good to ensure that the tracepoint can be generalized - but
it's fine to only implement what you can test and leave it to others to
extend. I just don't want us to end up in a mess where every different
GPU version has a different set of tracepoints.

Thanks,
Steve

> Kind regards,
> Nicolas Frattaroli
>  
>> Thanks,
>> Steve
> 
> 
>