[PATCH v3 0/3] Add a few tracepoints to panthor

[PATCH v3 0/3] 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 three 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 v3:
- Drop PWRFEATURES patch, as this register is no longer needed by this
  series.
- Eliminate the rt_on field from the gpu_power_status register, as per
  Steven Price's feedback.
- Make gpu_power_status tracepoint reg/unreg functions generic across
  hardware generations by wrapping a hw op in panthor_hw.c.
- Reimplement the <= v13 IRQ mask modification functions as the new hw
  ops functions. v14 can add its own ops in due time.
- Link to v2: https://lore.kernel.org/r/20251210-panthor-tracepoints-v2-0-ace2e29bad0f@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 (3):
      drm/panthor: Add panthor_*_irq_mask_set helper
      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    | 38 +++++++++++++-
 drivers/gpu/drm/panthor/panthor_gpu.h    |  2 +
 drivers/gpu/drm/panthor/panthor_hw.c     | 62 +++++++++++++++++++++++
 drivers/gpu/drm/panthor/panthor_hw.h     |  8 +++
 drivers/gpu/drm/panthor/panthor_trace.h  | 87 ++++++++++++++++++++++++++++++++
 7 files changed, 215 insertions(+), 2 deletions(-)
---
base-commit: f8da8e7cee8489b7d7d92d34adf1bd8be14cf527
change-id: 20251203-panthor-tracepoints-488af09d46e7

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

[PATCH v3 1/3] 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 v3 2/3] 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.

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.

The registration/unregistration functions for the tracepoint go through
a wrapper in panthor_hw.c, so that v14+ can implement the same
tracepoint by adding its hardware specific IRQ on/off callbacks to the
panthor_hw.ops member.

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

diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
index 057e167468d0..67572b607b55 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,12 @@ 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));
+
 	if (status & GPU_IRQ_FAULT) {
 		u32 fault_status = gpu_read(ptdev, GPU_FAULT_STATUS);
 		u64 address = gpu_read64(ptdev, GPU_FAULT_ADDR);
@@ -139,6 +154,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 +166,31 @@ 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;
 }
 
+int panthor_gpu_power_changed_on(struct panthor_device *ptdev)
+{
+	guard(pm_runtime_active)(ptdev->base.dev);
+
+	ptdev->gpu->irq_mask |= GPU_POWER_INTERRUPTS_MASK;
+	panthor_gpu_irq_mask_set(&ptdev->gpu->irq, ptdev->gpu->irq_mask);
+
+	return 0;
+}
+
+void panthor_gpu_power_changed_off(struct panthor_device *ptdev)
+{
+	guard(pm_runtime_active)(ptdev->base.dev);
+
+	ptdev->gpu->irq_mask &= ~GPU_POWER_INTERRUPTS_MASK;
+	panthor_gpu_irq_mask_set(&ptdev->gpu->irq, ptdev->gpu->irq_mask);
+}
+
 /**
  * panthor_gpu_block_power_off() - Power-off a specific block of the GPU
  * @ptdev: Device.
@@ -395,7 +429,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_gpu.h b/drivers/gpu/drm/panthor/panthor_gpu.h
index 12e66f48ced1..12c263a39928 100644
--- a/drivers/gpu/drm/panthor/panthor_gpu.h
+++ b/drivers/gpu/drm/panthor/panthor_gpu.h
@@ -51,5 +51,7 @@ int panthor_gpu_l2_power_on(struct panthor_device *ptdev);
 int panthor_gpu_flush_caches(struct panthor_device *ptdev,
 			     u32 l2, u32 lsc, u32 other);
 int panthor_gpu_soft_reset(struct panthor_device *ptdev);
+void panthor_gpu_power_changed_off(struct panthor_device *ptdev);
+int panthor_gpu_power_changed_on(struct panthor_device *ptdev);
 
 #endif
diff --git a/drivers/gpu/drm/panthor/panthor_hw.c b/drivers/gpu/drm/panthor/panthor_hw.c
index 87ebb7ae42c4..ae3320d0e251 100644
--- a/drivers/gpu/drm/panthor/panthor_hw.c
+++ b/drivers/gpu/drm/panthor/panthor_hw.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0 or MIT
 /* Copyright 2025 ARM Limited. All rights reserved. */
 
+#include <linux/platform_device.h>
+
 #include <drm/drm_print.h>
 
 #include "panthor_device.h"
@@ -29,6 +31,8 @@ static struct panthor_hw panthor_hw_arch_v10 = {
 		.soft_reset = panthor_gpu_soft_reset,
 		.l2_power_off = panthor_gpu_l2_power_off,
 		.l2_power_on = panthor_gpu_l2_power_on,
+		.power_changed_off = panthor_gpu_power_changed_off,
+		.power_changed_on = panthor_gpu_power_changed_on,
 	},
 };
 
@@ -53,6 +57,64 @@ static struct panthor_hw_entry panthor_hw_match[] = {
 	},
 };
 
+static int panthor_hw_set_power_tracing(struct device *dev, void *data)
+{
+	struct panthor_device *ptdev = dev_get_drvdata(dev);
+
+	if (!ptdev)
+		return -ENODEV;
+
+	if (!ptdev->hw)
+		return 0;
+
+	if (data) {
+		if (ptdev->hw->ops.power_changed_on)
+			return ptdev->hw->ops.power_changed_on(ptdev);
+	} else {
+		if (ptdev->hw->ops.power_changed_off)
+			ptdev->hw->ops.power_changed_off(ptdev);
+	}
+
+	return 0;
+}
+
+int panthor_hw_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_hw_set_power_tracing);
+
+	return ret;
+}
+
+void panthor_hw_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_hw_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));
+}
+
 static char *get_gpu_model_name(struct panthor_device *ptdev)
 {
 	const u32 gpu_id = ptdev->gpu_info.gpu_id;
diff --git a/drivers/gpu/drm/panthor/panthor_hw.h b/drivers/gpu/drm/panthor/panthor_hw.h
index 56c68c1e9c26..2c28aea82841 100644
--- a/drivers/gpu/drm/panthor/panthor_hw.h
+++ b/drivers/gpu/drm/panthor/panthor_hw.h
@@ -19,6 +19,12 @@ struct panthor_hw_ops {
 
 	/** @l2_power_on: L2 power on function pointer */
 	int (*l2_power_on)(struct panthor_device *ptdev);
+
+	/** @power_changed_on: Start listening to power change IRQs */
+	int (*power_changed_on)(struct panthor_device *ptdev);
+
+	/** @power_changed_off: Stop listening to power change IRQs */
+	void (*power_changed_off)(struct panthor_device *ptdev);
 };
 
 /**
@@ -32,6 +38,8 @@ struct panthor_hw {
 };
 
 int panthor_hw_init(struct panthor_device *ptdev);
+int panthor_hw_power_status_register(void);
+void panthor_hw_power_status_unregister(void);
 
 static inline int panthor_hw_soft_reset(struct panthor_device *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..2b59d7f156b6
--- /dev/null
+++ b/drivers/gpu/drm/panthor/panthor_trace.h
@@ -0,0 +1,59 @@
+/* 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_hw_power_status_register(void);
+void panthor_hw_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
+ */
+TRACE_EVENT_FN(gpu_power_status,
+	TP_PROTO(const struct device *dev, u64 shader_bitmap, u64 tiler_bitmap,
+		 u64 l2_bitmap),
+	TP_ARGS(dev, shader_bitmap, tiler_bitmap, l2_bitmap),
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(dev))
+		__field(u64, shader_bitmap)
+		__field(u64, tiler_bitmap)
+		__field(u64, l2_bitmap)
+	),
+	TP_fast_assign(
+		__assign_str(dev_name);
+		__entry->shader_bitmap	= shader_bitmap;
+		__entry->tiler_bitmap	= tiler_bitmap;
+		__entry->l2_bitmap	= l2_bitmap;
+	),
+	TP_printk("%s: shader_bitmap=0x%llx tiler_bitmap=0x%llx l2_bitmap=0x%llx",
+		  __get_str(dev_name), __entry->shader_bitmap, __entry->tiler_bitmap,
+		  __entry->l2_bitmap
+	),
+	panthor_hw_power_status_register, panthor_hw_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

Re: [PATCH v3 2/3] drm/panthor: Add tracepoint for hardware utilisation changes

[Karunika Choo]

On 11/12/2025 16:15, 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.
> 
> 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.
> 
> The registration/unregistration functions for the tracepoint go through
> a wrapper in panthor_hw.c, so that v14+ can implement the same
> tracepoint by adding its hardware specific IRQ on/off callbacks to the
> panthor_hw.ops member.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_gpu.c   | 38 ++++++++++++++++++--
>  drivers/gpu/drm/panthor/panthor_gpu.h   |  2 ++
>  drivers/gpu/drm/panthor/panthor_hw.c    | 62 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/panthor/panthor_hw.h    |  8 +++++
>  drivers/gpu/drm/panthor/panthor_trace.h | 59 +++++++++++++++++++++++++++++++
>  5 files changed, 167 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 057e167468d0..67572b607b55 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,12 @@ 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));
> +
>  	if (status & GPU_IRQ_FAULT) {
>  		u32 fault_status = gpu_read(ptdev, GPU_FAULT_STATUS);
>  		u64 address = gpu_read64(ptdev, GPU_FAULT_ADDR);
> @@ -139,6 +154,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 +166,31 @@ 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;
>  }
>  
> +int panthor_gpu_power_changed_on(struct panthor_device *ptdev)
> +{
> +	guard(pm_runtime_active)(ptdev->base.dev);
> +
> +	ptdev->gpu->irq_mask |= GPU_POWER_INTERRUPTS_MASK;
> +	panthor_gpu_irq_mask_set(&ptdev->gpu->irq, ptdev->gpu->irq_mask);
> +
> +	return 0;
> +}
> +
> +void panthor_gpu_power_changed_off(struct panthor_device *ptdev)
> +{
> +	guard(pm_runtime_active)(ptdev->base.dev);
> +
> +	ptdev->gpu->irq_mask &= ~GPU_POWER_INTERRUPTS_MASK;
> +	panthor_gpu_irq_mask_set(&ptdev->gpu->irq, ptdev->gpu->irq_mask);
> +}
> +
>  /**
>   * panthor_gpu_block_power_off() - Power-off a specific block of the GPU
>   * @ptdev: Device.
> @@ -395,7 +429,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_gpu.h b/drivers/gpu/drm/panthor/panthor_gpu.h
> index 12e66f48ced1..12c263a39928 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.h
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.h
> @@ -51,5 +51,7 @@ int panthor_gpu_l2_power_on(struct panthor_device *ptdev);
>  int panthor_gpu_flush_caches(struct panthor_device *ptdev,
>  			     u32 l2, u32 lsc, u32 other);
>  int panthor_gpu_soft_reset(struct panthor_device *ptdev);
> +void panthor_gpu_power_changed_off(struct panthor_device *ptdev);
> +int panthor_gpu_power_changed_on(struct panthor_device *ptdev);
>  
>  #endif
> diff --git a/drivers/gpu/drm/panthor/panthor_hw.c b/drivers/gpu/drm/panthor/panthor_hw.c
> index 87ebb7ae42c4..ae3320d0e251 100644
> --- a/drivers/gpu/drm/panthor/panthor_hw.c
> +++ b/drivers/gpu/drm/panthor/panthor_hw.c
> @@ -1,6 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0 or MIT
>  /* Copyright 2025 ARM Limited. All rights reserved. */
>  
> +#include <linux/platform_device.h>
> +
>  #include <drm/drm_print.h>
>  
>  #include "panthor_device.h"
> @@ -29,6 +31,8 @@ static struct panthor_hw panthor_hw_arch_v10 = {
>  		.soft_reset = panthor_gpu_soft_reset,
>  		.l2_power_off = panthor_gpu_l2_power_off,
>  		.l2_power_on = panthor_gpu_l2_power_on,
> +		.power_changed_off = panthor_gpu_power_changed_off,
> +		.power_changed_on = panthor_gpu_power_changed_on,
>  	},
>  };
>  
> @@ -53,6 +57,64 @@ static struct panthor_hw_entry panthor_hw_match[] = {
>  	},
>  };
>  
> +static int panthor_hw_set_power_tracing(struct device *dev, void *data)
> +{
> +	struct panthor_device *ptdev = dev_get_drvdata(dev);
> +
> +	if (!ptdev)
> +		return -ENODEV;
> +
> +	if (!ptdev->hw)
> +		return 0;
> +
> +	if (data) {
> +		if (ptdev->hw->ops.power_changed_on)
> +			return ptdev->hw->ops.power_changed_on(ptdev);
> +	} else {
> +		if (ptdev->hw->ops.power_changed_off)
> +			ptdev->hw->ops.power_changed_off(ptdev);
> +	}
> +
> +	return 0;
> +}
> +
> +int panthor_hw_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_hw_set_power_tracing);
> +
> +	return ret;
> +}
> +
> +void panthor_hw_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_hw_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));
> +}
> +
>  static char *get_gpu_model_name(struct panthor_device *ptdev)
>  {
>  	const u32 gpu_id = ptdev->gpu_info.gpu_id;
> diff --git a/drivers/gpu/drm/panthor/panthor_hw.h b/drivers/gpu/drm/panthor/panthor_hw.h
> index 56c68c1e9c26..2c28aea82841 100644
> --- a/drivers/gpu/drm/panthor/panthor_hw.h
> +++ b/drivers/gpu/drm/panthor/panthor_hw.h
> @@ -19,6 +19,12 @@ struct panthor_hw_ops {
>  
>  	/** @l2_power_on: L2 power on function pointer */
>  	int (*l2_power_on)(struct panthor_device *ptdev);
> +
> +	/** @power_changed_on: Start listening to power change IRQs */
> +	int (*power_changed_on)(struct panthor_device *ptdev);
> +
> +	/** @power_changed_off: Stop listening to power change IRQs */
> +	void (*power_changed_off)(struct panthor_device *ptdev);
>  };
>  
>  /**
> @@ -32,6 +38,8 @@ struct panthor_hw {
>  };
>  
>  int panthor_hw_init(struct panthor_device *ptdev);
> +int panthor_hw_power_status_register(void);
> +void panthor_hw_power_status_unregister(void);
>  
>  static inline int panthor_hw_soft_reset(struct panthor_device *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..2b59d7f156b6
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/panthor_trace.h
> @@ -0,0 +1,59 @@
> +/* 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_hw_power_status_register(void);
> +void panthor_hw_power_status_unregister(void);

Hello, not sure if I'm missing something but, would doing

    #include "panthor_hw.h" 

address the need to redeclare panthor_hw_power_status_* in this file?
The change looks good otherwise.

Reviewed-by: Karunika Choo <karunika.choo@arm.com>

> +
> +/**
> + * 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
> + */
> +TRACE_EVENT_FN(gpu_power_status,
> +	TP_PROTO(const struct device *dev, u64 shader_bitmap, u64 tiler_bitmap,
> +		 u64 l2_bitmap),
> +	TP_ARGS(dev, shader_bitmap, tiler_bitmap, l2_bitmap),
> +	TP_STRUCT__entry(
> +		__string(dev_name, dev_name(dev))
> +		__field(u64, shader_bitmap)
> +		__field(u64, tiler_bitmap)
> +		__field(u64, l2_bitmap)
> +	),
> +	TP_fast_assign(
> +		__assign_str(dev_name);
> +		__entry->shader_bitmap	= shader_bitmap;
> +		__entry->tiler_bitmap	= tiler_bitmap;
> +		__entry->l2_bitmap	= l2_bitmap;
> +	),
> +	TP_printk("%s: shader_bitmap=0x%llx tiler_bitmap=0x%llx l2_bitmap=0x%llx",
> +		  __get_str(dev_name), __entry->shader_bitmap, __entry->tiler_bitmap,
> +		  __entry->l2_bitmap
> +	),
> +	panthor_hw_power_status_register, panthor_hw_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 v3 2/3] drm/panthor: Add tracepoint for hardware utilisation changes

[Nicolas Frattaroli]

On Thursday, 11 December 2025 20:37:09 Central European Standard Time Karunika Choo wrote:
> On 11/12/2025 16:15, 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.
> > 
> > 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.
> > 
> > The registration/unregistration functions for the tracepoint go through
> > a wrapper in panthor_hw.c, so that v14+ can implement the same
> > tracepoint by adding its hardware specific IRQ on/off callbacks to the
> > panthor_hw.ops member.
> > 
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  drivers/gpu/drm/panthor/panthor_gpu.c   | 38 ++++++++++++++++++--
> >  drivers/gpu/drm/panthor/panthor_gpu.h   |  2 ++
> >  drivers/gpu/drm/panthor/panthor_hw.c    | 62 +++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/panthor/panthor_hw.h    |  8 +++++
> >  drivers/gpu/drm/panthor/panthor_trace.h | 59 +++++++++++++++++++++++++++++++
> >  5 files changed, 167 insertions(+), 2 deletions(-)
> > 
> > [...]
> >
> > diff --git a/drivers/gpu/drm/panthor/panthor_trace.h b/drivers/gpu/drm/panthor/panthor_trace.h
> > new file mode 100644
> > index 000000000000..2b59d7f156b6
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panthor/panthor_trace.h
> > @@ -0,0 +1,59 @@
> > +/* 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_hw_power_status_register(void);
> > +void panthor_hw_power_status_unregister(void);
> 
> Hello, not sure if I'm missing something but, would doing
> 
>     #include "panthor_hw.h" 
> 
> address the need to redeclare panthor_hw_power_status_* in this file?
> The change looks good otherwise.

It would, but only in that it now pulls in a whole bunch of header
definitions that the trace header does not want or need, all for
two function prototypes. Since the function signature of the reg/unreg
functions are fixed aside from the name, I don't see any harm in
this particular instance of duplicating it.

Similarly, panthor_hw.c probably doesn't want the special magic
tracepoint stuff from panthor_trace.h, but it does need to implement
those two functions, so it needs to have a prototype somewhere.

Maybe someone else has stronger opinions on this, I'm fine with
including panthor_hw.h as well (it's what I had it do originally
as well), but I suspect many more experienced kernel devs are
wary of overeager header inclusions, because it leads to really
slow compilation quite quickly.

Kind regards,
Nicolas Frattaroli

> 
> Reviewed-by: Karunika Choo <karunika.choo@arm.com>
> 
> > [...]

Re: [PATCH v3 2/3] drm/panthor: Add tracepoint for hardware utilisation changes

[Boris Brezillon]

On Thu, 11 Dec 2025 21:15:43 +0100
Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote:

> On Thursday, 11 December 2025 20:37:09 Central European Standard Time Karunika Choo wrote:
> > On 11/12/2025 16:15, 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.
> > > 
> > > 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.
> > > 
> > > The registration/unregistration functions for the tracepoint go through
> > > a wrapper in panthor_hw.c, so that v14+ can implement the same
> > > tracepoint by adding its hardware specific IRQ on/off callbacks to the
> > > panthor_hw.ops member.
> > > 
> > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > ---
> > >  drivers/gpu/drm/panthor/panthor_gpu.c   | 38 ++++++++++++++++++--
> > >  drivers/gpu/drm/panthor/panthor_gpu.h   |  2 ++
> > >  drivers/gpu/drm/panthor/panthor_hw.c    | 62 +++++++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/panthor/panthor_hw.h    |  8 +++++
> > >  drivers/gpu/drm/panthor/panthor_trace.h | 59 +++++++++++++++++++++++++++++++
> > >  5 files changed, 167 insertions(+), 2 deletions(-)
> > > 
> > > [...]
> > >
> > > diff --git a/drivers/gpu/drm/panthor/panthor_trace.h b/drivers/gpu/drm/panthor/panthor_trace.h
> > > new file mode 100644
> > > index 000000000000..2b59d7f156b6
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/panthor/panthor_trace.h
> > > @@ -0,0 +1,59 @@
> > > +/* 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_hw_power_status_register(void);
> > > +void panthor_hw_power_status_unregister(void);  
> > 
> > Hello, not sure if I'm missing something but, would doing
> > 
> >     #include "panthor_hw.h" 
> > 
> > address the need to redeclare panthor_hw_power_status_* in this file?
> > The change looks good otherwise.  
> 
> It would, but only in that it now pulls in a whole bunch of header
> definitions that the trace header does not want or need, all for
> two function prototypes. Since the function signature of the reg/unreg
> functions are fixed aside from the name, I don't see any harm in
> this particular instance of duplicating it.
> 
> Similarly, panthor_hw.c probably doesn't want the special magic
> tracepoint stuff from panthor_trace.h, but it does need to implement
> those two functions, so it needs to have a prototype somewhere.
> 
> Maybe someone else has stronger opinions on this, I'm fine with
> including panthor_hw.h as well (it's what I had it do originally
> as well), but I suspect many more experienced kernel devs are
> wary of overeager header inclusions, because it leads to really
> slow compilation quite quickly.

In general I agree that including only headers you really need is a
good practice (to improve compilation time), but that's also an extra
maintenance burden when things are declared in multiple places, so I'd
go for the panthor_hw.h inclusion here, I think.

Re: [PATCH v3 2/3] drm/panthor: Add tracepoint for hardware utilisation changes

[Nicolas Frattaroli]

On Friday, 12 December 2025 10:29:21 Central European Standard Time Boris Brezillon wrote:
> On Thu, 11 Dec 2025 21:15:43 +0100
> Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote:
> 
> > On Thursday, 11 December 2025 20:37:09 Central European Standard Time Karunika Choo wrote:
> > > On 11/12/2025 16:15, 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.
> > > > 
> > > > 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.
> > > > 
> > > > The registration/unregistration functions for the tracepoint go through
> > > > a wrapper in panthor_hw.c, so that v14+ can implement the same
> > > > tracepoint by adding its hardware specific IRQ on/off callbacks to the
> > > > panthor_hw.ops member.
> > > > 
> > > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > > ---
> > > >  drivers/gpu/drm/panthor/panthor_gpu.c   | 38 ++++++++++++++++++--
> > > >  drivers/gpu/drm/panthor/panthor_gpu.h   |  2 ++
> > > >  drivers/gpu/drm/panthor/panthor_hw.c    | 62 +++++++++++++++++++++++++++++++++
> > > >  drivers/gpu/drm/panthor/panthor_hw.h    |  8 +++++
> > > >  drivers/gpu/drm/panthor/panthor_trace.h | 59 +++++++++++++++++++++++++++++++
> > > >  5 files changed, 167 insertions(+), 2 deletions(-)
> > > > 
> > > > [...]
> > > >
> > > > diff --git a/drivers/gpu/drm/panthor/panthor_trace.h b/drivers/gpu/drm/panthor/panthor_trace.h
> > > > new file mode 100644
> > > > index 000000000000..2b59d7f156b6
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/panthor/panthor_trace.h
> > > > @@ -0,0 +1,59 @@
> > > > +/* 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_hw_power_status_register(void);
> > > > +void panthor_hw_power_status_unregister(void);  
> > > 
> > > Hello, not sure if I'm missing something but, would doing
> > > 
> > >     #include "panthor_hw.h" 
> > > 
> > > address the need to redeclare panthor_hw_power_status_* in this file?
> > > The change looks good otherwise.  
> > 
> > It would, but only in that it now pulls in a whole bunch of header
> > definitions that the trace header does not want or need, all for
> > two function prototypes. Since the function signature of the reg/unreg
> > functions are fixed aside from the name, I don't see any harm in
> > this particular instance of duplicating it.
> > 
> > Similarly, panthor_hw.c probably doesn't want the special magic
> > tracepoint stuff from panthor_trace.h, but it does need to implement
> > those two functions, so it needs to have a prototype somewhere.
> > 
> > Maybe someone else has stronger opinions on this, I'm fine with
> > including panthor_hw.h as well (it's what I had it do originally
> > as well), but I suspect many more experienced kernel devs are
> > wary of overeager header inclusions, because it leads to really
> > slow compilation quite quickly.
> 
> In general I agree that including only headers you really need is a
> good practice (to improve compilation time), but that's also an extra
> maintenance burden when things are declared in multiple places, so I'd
> go for the panthor_hw.h inclusion here, I think.
> 

Alright, will do that in v4. Feel free to leave any more feedback
on anything in the series (e.g. the other tracepoint) so I don't
send too many tiny revisions.

Re: [PATCH v3 2/3] drm/panthor: Add tracepoint for hardware utilisation changes

[Lukas Zapolskas]

Hello Nicolas,


On 11/12/2025 16:15, 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.
> 
> 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.
> 
> The registration/unregistration functions for the tracepoint go through
> a wrapper in panthor_hw.c, so that v14+ can implement the same
> tracepoint by adding its hardware specific IRQ on/off callbacks to the
> panthor_hw.ops member.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_gpu.c   | 38 ++++++++++++++++++--
>  drivers/gpu/drm/panthor/panthor_gpu.h   |  2 ++
>  drivers/gpu/drm/panthor/panthor_hw.c    | 62 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/panthor/panthor_hw.h    |  8 +++++
>  drivers/gpu/drm/panthor/panthor_trace.h | 59 +++++++++++++++++++++++++++++++
>  5 files changed, 167 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index 057e167468d0..67572b607b55 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,12 @@ 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));
> +
>  	if (status & GPU_IRQ_FAULT) {
>  		u32 fault_status = gpu_read(ptdev, GPU_FAULT_STATUS);
>  		u64 address = gpu_read64(ptdev, GPU_FAULT_ADDR);
> @@ -139,6 +154,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 +166,31 @@ 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;
>  }
>  
> +int panthor_gpu_power_changed_on(struct panthor_device *ptdev)
> +{
> +	guard(pm_runtime_active)(ptdev->base.dev);
> +
> +	ptdev->gpu->irq_mask |= GPU_POWER_INTERRUPTS_MASK;
> +	panthor_gpu_irq_mask_set(&ptdev->gpu->irq, ptdev->gpu->irq_mask);
> +
> +	return 0;
> +}
> +
> +void panthor_gpu_power_changed_off(struct panthor_device *ptdev)
> +{
> +	guard(pm_runtime_active)(ptdev->base.dev);
> +
> +	ptdev->gpu->irq_mask &= ~GPU_POWER_INTERRUPTS_MASK;
> +	panthor_gpu_irq_mask_set(&ptdev->gpu->irq, ptdev->gpu->irq_mask);
> +}
> +
>  /**
>   * panthor_gpu_block_power_off() - Power-off a specific block of the GPU
>   * @ptdev: Device.
> @@ -395,7 +429,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_gpu.h b/drivers/gpu/drm/panthor/panthor_gpu.h
> index 12e66f48ced1..12c263a39928 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.h
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.h
> @@ -51,5 +51,7 @@ int panthor_gpu_l2_power_on(struct panthor_device *ptdev);
>  int panthor_gpu_flush_caches(struct panthor_device *ptdev,
>  			     u32 l2, u32 lsc, u32 other);
>  int panthor_gpu_soft_reset(struct panthor_device *ptdev);
> +void panthor_gpu_power_changed_off(struct panthor_device *ptdev);
> +int panthor_gpu_power_changed_on(struct panthor_device *ptdev);
>  
>  #endif
> diff --git a/drivers/gpu/drm/panthor/panthor_hw.c b/drivers/gpu/drm/panthor/panthor_hw.c
> index 87ebb7ae42c4..ae3320d0e251 100644
> --- a/drivers/gpu/drm/panthor/panthor_hw.c
> +++ b/drivers/gpu/drm/panthor/panthor_hw.c
> @@ -1,6 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0 or MIT
>  /* Copyright 2025 ARM Limited. All rights reserved. */
>  
> +#include <linux/platform_device.h>
> +
>  #include <drm/drm_print.h>
>  
>  #include "panthor_device.h"
> @@ -29,6 +31,8 @@ static struct panthor_hw panthor_hw_arch_v10 = {
>  		.soft_reset = panthor_gpu_soft_reset,
>  		.l2_power_off = panthor_gpu_l2_power_off,
>  		.l2_power_on = panthor_gpu_l2_power_on,
> +		.power_changed_off = panthor_gpu_power_changed_off,
> +		.power_changed_on = panthor_gpu_power_changed_on,
>  	},
>  };
>  
> @@ -53,6 +57,64 @@ static struct panthor_hw_entry panthor_hw_match[] = {
>  	},
>  };
>  
> +static int panthor_hw_set_power_tracing(struct device *dev, void *data)
> +{
> +	struct panthor_device *ptdev = dev_get_drvdata(dev);
> +
> +	if (!ptdev)
> +		return -ENODEV;
> +
> +	if (!ptdev->hw)
> +		return 0;
> +
> +	if (data) {
> +		if (ptdev->hw->ops.power_changed_on)
> +			return ptdev->hw->ops.power_changed_on(ptdev);
> +	} else {
> +		if (ptdev->hw->ops.power_changed_off)
> +			ptdev->hw->ops.power_changed_off(ptdev);
> +	}
> +
> +	return 0;
> +}
> +
> +int panthor_hw_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_hw_set_power_tracing);
> +
> +	return ret;
> +}
> +
> +void panthor_hw_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_hw_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));
> +}
> +
>  static char *get_gpu_model_name(struct panthor_device *ptdev)
>  {
>  	const u32 gpu_id = ptdev->gpu_info.gpu_id;
> diff --git a/drivers/gpu/drm/panthor/panthor_hw.h b/drivers/gpu/drm/panthor/panthor_hw.h
> index 56c68c1e9c26..2c28aea82841 100644
> --- a/drivers/gpu/drm/panthor/panthor_hw.h
> +++ b/drivers/gpu/drm/panthor/panthor_hw.h
> @@ -19,6 +19,12 @@ struct panthor_hw_ops {
>  
>  	/** @l2_power_on: L2 power on function pointer */
>  	int (*l2_power_on)(struct panthor_device *ptdev);
> +
> +	/** @power_changed_on: Start listening to power change IRQs */
> +	int (*power_changed_on)(struct panthor_device *ptdev);
> +
> +	/** @power_changed_off: Stop listening to power change IRQs */
> +	void (*power_changed_off)(struct panthor_device *ptdev);
>  };
>  
>  /**
> @@ -32,6 +38,8 @@ struct panthor_hw {
>  };
>  
>  int panthor_hw_init(struct panthor_device *ptdev);
> +int panthor_hw_power_status_register(void);
> +void panthor_hw_power_status_unregister(void);
>  
>  static inline int panthor_hw_soft_reset(struct panthor_device *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..2b59d7f156b6
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/panthor_trace.h
> @@ -0,0 +1,59 @@
> +/* 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_hw_power_status_register(void);
> +void panthor_hw_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
> + */
> +TRACE_EVENT_FN(gpu_power_status,
> +	TP_PROTO(const struct device *dev, u64 shader_bitmap, u64 tiler_bitmap,
> +		 u64 l2_bitmap),
> +	TP_ARGS(dev, shader_bitmap, tiler_bitmap, l2_bitmap),
> +	TP_STRUCT__entry(
> +		__string(dev_name, dev_name(dev))
> +		__field(u64, shader_bitmap)
> +		__field(u64, tiler_bitmap)
> +		__field(u64, l2_bitmap)
> +	),
> +	TP_fast_assign(
> +		__assign_str(dev_name);
> +		__entry->shader_bitmap	= shader_bitmap;
> +		__entry->tiler_bitmap	= tiler_bitmap;
> +		__entry->l2_bitmap	= l2_bitmap;
> +	),
> +	TP_printk("%s: shader_bitmap=0x%llx tiler_bitmap=0x%llx l2_bitmap=0x%llx",
> +		  __get_str(dev_name), __entry->shader_bitmap, __entry->tiler_bitmap,
> +		  __entry->l2_bitmap
> +	),
> +	panthor_hw_power_status_register, panthor_hw_power_status_unregister
> +);

What is the expectation of stability for this tracepoint? Because I worry about future architectures 
that add different hardware blocks: we would have to either extend this tracepoint, or deprecate it
and make another one that is very similar. Do you have any sort of userspace tooling that is consuming
this or is this more for local debugging? 

> +
> +#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>
> 

Kind regards,
Lukas

Re: [PATCH v3 2/3] drm/panthor: Add tracepoint for hardware utilisation changes

[Nicolas Frattaroli]

On Monday, 15 December 2025 18:21:52 Central European Standard Time Lukas Zapolskas wrote:
> Hello Nicolas,
> 
> 
> On 11/12/2025 16:15, 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.
> > 
> > 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.
> > 
> > The registration/unregistration functions for the tracepoint go through
> > a wrapper in panthor_hw.c, so that v14+ can implement the same
> > tracepoint by adding its hardware specific IRQ on/off callbacks to the
> > panthor_hw.ops member.
> > 
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  drivers/gpu/drm/panthor/panthor_gpu.c   | 38 ++++++++++++++++++--
> >  drivers/gpu/drm/panthor/panthor_gpu.h   |  2 ++
> >  drivers/gpu/drm/panthor/panthor_hw.c    | 62 +++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/panthor/panthor_hw.h    |  8 +++++
> >  drivers/gpu/drm/panthor/panthor_trace.h | 59 +++++++++++++++++++++++++++++++
> >  5 files changed, 167 insertions(+), 2 deletions(-)
> > 
> > [... snip ...]
> > +/**
> > + * 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
> > + */
> > +TRACE_EVENT_FN(gpu_power_status,
> > +	TP_PROTO(const struct device *dev, u64 shader_bitmap, u64 tiler_bitmap,
> > +		 u64 l2_bitmap),
> > +	TP_ARGS(dev, shader_bitmap, tiler_bitmap, l2_bitmap),
> > +	TP_STRUCT__entry(
> > +		__string(dev_name, dev_name(dev))
> > +		__field(u64, shader_bitmap)
> > +		__field(u64, tiler_bitmap)
> > +		__field(u64, l2_bitmap)
> > +	),
> > +	TP_fast_assign(
> > +		__assign_str(dev_name);
> > +		__entry->shader_bitmap	= shader_bitmap;
> > +		__entry->tiler_bitmap	= tiler_bitmap;
> > +		__entry->l2_bitmap	= l2_bitmap;
> > +	),
> > +	TP_printk("%s: shader_bitmap=0x%llx tiler_bitmap=0x%llx l2_bitmap=0x%llx",
> > +		  __get_str(dev_name), __entry->shader_bitmap, __entry->tiler_bitmap,
> > +		  __entry->l2_bitmap
> > +	),
> > +	panthor_hw_power_status_register, panthor_hw_power_status_unregister
> > +);
> 
> What is the expectation of stability for this tracepoint? Because I worry about future architectures 
> that add different hardware blocks: we would have to either extend this tracepoint, or deprecate it
> and make another one that is very similar.

There is no problem with extending this tracepoint in the future.
Linux tracepoints automatically have a machine-readable description
of their data layout in tracefs, in the "format" file. Adding new
fields to the tracepoint will not interfere with any tooling that
uses the format description to parse the data. The tracepoint's ABI
is self-describing in that sense.

> Do you have any sort of userspace tooling that is consuming
> this or is this more for local debugging? 
 
The specific use-case is Perfetto. This tracepoint can be consumed
by Perfetto with no special parsing added to Perfetto itself, as
it can consume event tracepoints like this based on their "format"
description.

Perfetto can do manual parsing for some tracepoints to integrate
them into the timeline differently, but for instantaneous events
like this, that is not needed.

Kind regards,
Nicolas Frattaroli

> > +
> > +#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>
> > 
> 
> Kind regards,
> Lukas
> 
> 

[PATCH v3 3/3] 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 2b59d7f156b6..1efd07861590 100644
--- a/drivers/gpu/drm/panthor/panthor_trace.h
+++ b/drivers/gpu/drm/panthor/panthor_trace.h
@@ -49,6 +49,34 @@ TRACE_EVENT_FN(gpu_power_status,
 	panthor_hw_power_status_register, panthor_hw_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