linux-kernel - Re: [PATCH] drm/panfrost: Name scheduler queues after their job slots

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ffzu4ekyatmcjekfngtjmdbrwuxc25s5mpzq7sqfgsjgjifiq6@gb3ilo3aecil>
Date: Thu, 9 Oct 2025 12:17:44 +0100
From: Adrián Larumbe <adrian.larumbe@...labora.com>
To: Steven Price <steven.price@....com>
Cc: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	Boris Brezillon <boris.brezillon@...labora.com>, kernel@...labora.com, Rob Herring <robh@...nel.org>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>
Subject: Re: [PATCH] drm/panfrost: Name scheduler queues after their job slots

On 08.10.2025 15:48, Steven Price wrote:
> On 02/10/2025 18:11, Adrián Larumbe wrote:
> > Drawing from commit d2624d90a0b7 ("drm/panthor: assign unique names to
> > queues"), give scheduler queues proper names that reflect the function
> > of their JM slot, so that this will be shown when gathering DRM
> > scheduler tracepoints.
> >
> > Signed-off-by: Adrián Larumbe <adrian.larumbe@...labora.com>
>
> Two minor things below, but with those fixed:
>
> Reviewed-by: Steven Price <steven.price@....com>
>
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_drv.c | 6 ------
> >  drivers/gpu/drm/panfrost/panfrost_job.c | 6 +++++-
> >  drivers/gpu/drm/panfrost/panfrost_job.h | 2 ++
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index 22350ce8a08f..d08c87bc63a2 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -668,12 +668,6 @@ static void panfrost_gpu_show_fdinfo(struct panfrost_device *pfdev,
> >  	 *   job spent on the GPU.
> >  	 */
> >
> > -	static const char * const engine_names[] = {
> > -		"fragment", "vertex-tiler", "compute-only"
> > -	};
> > -
> > -	BUILD_BUG_ON(ARRAY_SIZE(engine_names) != NUM_JOB_SLOTS);
>
> NIT: We could move this to panfrost_job.c and keep the BUILD_BUG_ON.
>
> > -
> >  	for (i = 0; i < NUM_JOB_SLOTS - 1; i++) {
> >  		if (pfdev->profile_mode) {
> >  			drm_printf(p, "drm-engine-%s:\t%llu ns\n",
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
> > index c47d14eabbae..0f0340ffee19 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> > @@ -28,6 +28,10 @@
> >  #define job_write(dev, reg, data) writel(data, dev->iomem + (reg))
> >  #define job_read(dev, reg) readl(dev->iomem + (reg))
> >
> > +const char * const engine_names[] = {
> > +	"fragment", "vertex-tiler-compute", "compute-only"
> > +};
> > +
> >  struct panfrost_queue_state {
> >  	struct drm_gpu_scheduler sched;
> >  	u64 fence_context;
> > @@ -846,7 +850,6 @@ int panfrost_job_init(struct panfrost_device *pfdev)
> >  		.num_rqs = DRM_SCHED_PRIORITY_COUNT,
> >  		.credit_limit = 2,
> >  		.timeout = msecs_to_jiffies(JOB_TIMEOUT_MS),
> > -		.name = "pan_js",
> >  		.dev = pfdev->dev,
> >  	};
> >  	struct panfrost_job_slot *js;
> > @@ -887,6 +890,7 @@ int panfrost_job_init(struct panfrost_device *pfdev)
> >
> >  	for (j = 0; j < NUM_JOB_SLOTS; j++) {
> >  		js->queue[j].fence_context = dma_fence_context_alloc(1);
> > +		args.name = engine_names[j];
> >
> >  		ret = drm_sched_init(&js->queue[j].sched, &args);
> >  		if (ret) {
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.h b/drivers/gpu/drm/panfrost/panfrost_job.h
> > index 5a30ff1503c6..52ff10b8d3d0 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_job.h
> > +++ b/drivers/gpu/drm/panfrost/panfrost_job.h
> > @@ -53,6 +53,8 @@ struct panfrost_jm_ctx {
> >  	struct drm_sched_entity slot_entity[NUM_JOB_SLOTS];
> >  };
> >
> > +extern const char * const engine_names[];
>
> NIT: Now this is no longer a local variable I think we should prefix it,
> e.g. panfrost_engine_names.

On top of this, I'm also keeping the original engine names to avoid
breaking UM uitlities which depend on fdinfo.

> > +
> >  int panfrost_jm_ctx_create(struct drm_file *file,
> >  			   struct drm_panfrost_jm_ctx_create *args);
> >  int panfrost_jm_ctx_destroy(struct drm_file *file, u32 handle);
> >
> > base-commit: 30531e9ca7cd4f8c5740babd35cdb465edf73a2d


Adrian Larumbe