On Mon, 15 Dec 2025 17:14:49 +0000 Lukas Zapolskas <[email protected]> wrote:
> Added the panthor_perf system initialization and unplug code to allow > for the handling of userspace sessions to be added in follow-up > patches. > > Signed-off-by: Lukas Zapolskas <[email protected]> > Reviewed-by: Adrián Larumbe <[email protected]> > --- > drivers/gpu/drm/panthor/panthor_device.c | 2 + > drivers/gpu/drm/panthor/panthor_device.h | 5 +- > drivers/gpu/drm/panthor/panthor_perf.c | 63 +++++++++++++++++++++++- > drivers/gpu/drm/panthor/panthor_perf.h | 1 + > 4 files changed, 69 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_device.c > b/drivers/gpu/drm/panthor/panthor_device.c > index dc237da92340..3063ffbead45 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.c > +++ b/drivers/gpu/drm/panthor/panthor_device.c > @@ -120,6 +120,7 @@ void panthor_device_unplug(struct panthor_device *ptdev) > /* Now, try to cleanly shutdown the GPU before the device resources > * get reclaimed. > */ > + panthor_perf_unplug(ptdev); > panthor_sched_unplug(ptdev); > panthor_fw_unplug(ptdev); > panthor_mmu_unplug(ptdev); > @@ -323,6 +324,7 @@ int panthor_device_init(struct panthor_device *ptdev) > > err_disable_autosuspend: > pm_runtime_dont_use_autosuspend(ptdev->base.dev); > + panthor_perf_unplug(ptdev); > > err_unplug_sched: > panthor_sched_unplug(ptdev); > diff --git a/drivers/gpu/drm/panthor/panthor_device.h > b/drivers/gpu/drm/panthor/panthor_device.h > index 64b0048de6ac..e1a6250cecc8 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.h > +++ b/drivers/gpu/drm/panthor/panthor_device.h > @@ -28,8 +28,8 @@ struct panthor_hw; > struct panthor_job; > struct panthor_mmu; > struct panthor_fw; > -struct panthor_perfcnt; > struct panthor_pwr; > +struct panthor_perf; > struct panthor_vm; > struct panthor_vm_pool; > > @@ -160,6 +160,9 @@ struct panthor_device { > /** @devfreq: Device frequency scaling management data. */ > struct panthor_devfreq *devfreq; > > + /** @perf: Performance counter management data. */ > + struct panthor_perf *perf; > + > /** @unplug: Device unplug related fields. */ > struct { > /** @lock: Lock used to serialize unplug operations. */ > diff --git a/drivers/gpu/drm/panthor/panthor_perf.c > b/drivers/gpu/drm/panthor/panthor_perf.c > index 842d62826ac3..3a65d6d326e8 100644 > --- a/drivers/gpu/drm/panthor/panthor_perf.c > +++ b/drivers/gpu/drm/panthor/panthor_perf.c > @@ -4,6 +4,7 @@ > > #include <linux/bitops.h> > #include <drm/panthor_drm.h> > +#include <drm/drm_print.h> > > #include "panthor_device.h" > #include "panthor_fw.h" > @@ -22,6 +23,19 @@ > */ > #define PANTHOR_HW_COUNTER_SIZE (sizeof(u32)) > > +struct panthor_perf { > + /** @next_session: The ID of the next session. */ > + u32 next_session; > + > + /** @session_range: The number of sessions supported at a time. */ > + struct xa_limit session_range; > + > + /** > + * @sessions: Global map of sessions, accessed by their ID. > + */ > + struct xarray sessions; > +}; > + > struct panthor_perf_counter_block { > struct drm_panthor_perf_block_header header; > u64 counters[]; > @@ -76,14 +90,61 @@ static void panthor_perf_info_init(struct panthor_device > *const ptdev) > * panthor_perf_init - Initialize the performance counter subsystem. > * @ptdev: Panthor device > * > + * The performance counters require the FW interface to be available to > setup the > + * sampling ringbuffers, so this must be called only after FW is initialized. > + * > * Return: 0 on success, negative error code on failure. > */ > int panthor_perf_init(struct panthor_device *ptdev) > { > + struct panthor_perf *perf __free(kfree) = NULL; > + int ret = 0; > + > if (!ptdev) > return -EINVAL; > > panthor_perf_info_init(ptdev); > > - return 0; > + perf = kzalloc(sizeof(*perf), GFP_KERNEL); > + if (ZERO_OR_NULL_PTR(perf)) > + return -ENOMEM; > + > + xa_init_flags(&perf->sessions, XA_FLAGS_ALLOC); > + > + perf->session_range = (struct xa_limit) { > + .min = 0, > + .max = 1, > + }; > + > + drm_info(&ptdev->base, "Performance counter subsystem initialized"); > + > + ptdev->perf = no_free_ptr(perf); side-note: it's a bit of a personal opinion but those __free(kfree) annotated vars that are then assigned to something or returned make me nervous. AFAICT, there's no compile-time warning/error when you forget to wrap the assigned/returned value with no_free_ptr()/return_ptr(), which basically turn unlikely leaks (because the error path is rarely taken) into almost certain UAF situations. I know it's something you have to get right only once most of the time, but still, I find it quite dangerous, especially when early bailouts with valid pointers are added to an existing function. All this to say that, unless there's a very good reason to use it, or if there exist tools to prove correctness at compile time, I'd prefer to stay away from this pattern and stick to manual kfree()s. > + > + return ret; > +} > + > +/** > + * panthor_perf_unplug - Terminate the performance counter subsystem. > + * @ptdev: Panthor device. > + * > + * This function will terminate the performance counter control structures > and any remaining > + * sessions, after waiting for any pending interrupts. > + */ > +void panthor_perf_unplug(struct panthor_device *ptdev) > +{ > + struct panthor_perf *perf = ptdev->perf; > + > + if (!perf) > + return; > + > + if (!xa_empty(&perf->sessions)) { > + drm_err(&ptdev->base, > + "Performance counter sessions active when unplugging > the driver!"); > + } > + > + xa_destroy(&perf->sessions); > + > + kfree(ptdev->perf); > + > + ptdev->perf = NULL; > } > diff --git a/drivers/gpu/drm/panthor/panthor_perf.h > b/drivers/gpu/drm/panthor/panthor_perf.h > index 3c32c24c164c..e4805727b9e7 100644 > --- a/drivers/gpu/drm/panthor/panthor_perf.h > +++ b/drivers/gpu/drm/panthor/panthor_perf.h > @@ -10,6 +10,7 @@ > struct panthor_device; > > int panthor_perf_init(struct panthor_device *ptdev); > +void panthor_perf_unplug(struct panthor_device *ptdev); > > #endif /* __PANTHOR_PERF_H__ */ >
