On Thu, 2024-11-14 at 11:01 +0100, Pierre-Eric Pelloux-Prayer wrote:
> This will be used in a later commit to trace the drm client_id in
> some of the gpu_scheduler trace events.
I think this commit message should detail what the patch is actual
intended to do.
Especially, it should mention that several drivers are being touched.
Some other comments below
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer
> <pierre-eric.pelloux-pra...@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 8 +++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ++-
> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
> drivers/gpu/drm/imagination/pvr_job.c | 2 +-
> drivers/gpu/drm/imagination/pvr_queue.c | 5 +++--
> drivers/gpu/drm/imagination/pvr_queue.h | 2 +-
> drivers/gpu/drm/lima/lima_gem.c | 2 +-
> drivers/gpu/drm/lima/lima_sched.c | 6 ++++--
> drivers/gpu/drm/lima/lima_sched.h | 3 ++-
> drivers/gpu/drm/msm/msm_gem_submit.c | 8 +++++---
> drivers/gpu/drm/nouveau/nouveau_sched.c | 3 ++-
> drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +-
> drivers/gpu/drm/scheduler/sched_fence.c | 4 +++-
> drivers/gpu/drm/scheduler/sched_main.c | 6 ++++--
> drivers/gpu/drm/v3d/v3d_submit.c | 2 +-
> drivers/gpu/drm/xe/xe_sched_job.c | 3 ++-
> include/drm/gpu_scheduler.h | 12 ++++++++++--
> 19 files changed, 51 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index b545940e512b..eede43701d51 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -681,7 +681,7 @@ int amdgpu_amdkfd_submit_ib(struct amdgpu_device
> *adev,
> goto err;
> }
>
> - ret = amdgpu_job_alloc(adev, NULL, NULL, NULL, 1, &job);
> + ret = amdgpu_job_alloc(adev, NULL, NULL, NULL, 1, &job, 0);
> if (ret)
> goto err;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 98aa4beee36a..a0a129405323 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -293,7 +293,8 @@ static int amdgpu_cs_pass1(struct
> amdgpu_cs_parser *p,
>
> for (i = 0; i < p->gang_size; ++i) {
> ret = amdgpu_job_alloc(p->adev, vm, p->entities[i],
> vm,
> - num_ibs[i], &p->jobs[i]);
> + num_ibs[i], &p->jobs[i],
> + p->filp->client_id);
> if (ret)
> goto free_all_kdata;
> p->jobs[i]->enforce_isolation = p->adev-
> >enforce_isolation[fpriv->xcp_id];
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index c774cd019a10..1dd8e940d1e9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -186,7 +186,8 @@ static enum drm_gpu_sched_stat
> amdgpu_job_timedout(struct drm_sched_job *s_job)
>
> int amdgpu_job_alloc(struct amdgpu_device *adev, struct amdgpu_vm
> *vm,
> struct drm_sched_entity *entity, void *owner,
> - unsigned int num_ibs, struct amdgpu_job **job)
> + unsigned int num_ibs, struct amdgpu_job **job,
> + uint64_t drm_client_id)
You can take the short cut and use u64 instead of uint64_t. Easier to
type for you and more congruent with the other code (in the kernel).
> {
> if (num_ibs == 0)
> return -EINVAL;
> @@ -209,7 +210,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev,
> struct amdgpu_vm *vm,
> if (!entity)
> return 0;
>
> - return drm_sched_job_init(&(*job)->base, entity, 1, owner);
> + return drm_sched_job_init(&(*job)->base, entity, 1, owner,
> + drm_client_id);
> }
>
> int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev,
> @@ -219,7 +221,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device
> *adev,
> {
> int r;
>
> - r = amdgpu_job_alloc(adev, NULL, entity, owner, 1, job);
> + r = amdgpu_job_alloc(adev, NULL, entity, owner, 1, job, 0);
> if (r)
> return r;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> index ce6b9ba967ff..41a03477ba5d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> @@ -90,7 +90,8 @@ static inline struct amdgpu_ring
> *amdgpu_job_ring(struct amdgpu_job *job)
>
> int amdgpu_job_alloc(struct amdgpu_device *adev, struct amdgpu_vm
> *vm,
> struct drm_sched_entity *entity, void *owner,
> - unsigned int num_ibs, struct amdgpu_job **job);
> + unsigned int num_ibs, struct amdgpu_job **job,
> + uint64_t drm_client_id);
same
> int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev,
> struct drm_sched_entity *entity, void
> *owner,
> size_t size, enum amdgpu_ib_pool_type
> pool_type,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 3d0f8d182506..70294ca6202f 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -535,7 +535,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device
> *dev, void *data,
>
> ret = drm_sched_job_init(&submit->sched_job,
> &ctx->sched_entity[args->pipe],
> - 1, submit->ctx);
> + 1, submit->ctx, file->client_id);
> if (ret)
> goto err_submit_put;
>
> diff --git a/drivers/gpu/drm/imagination/pvr_job.c
> b/drivers/gpu/drm/imagination/pvr_job.c
> index 618503a212a7..64152b57e8b1 100644
> --- a/drivers/gpu/drm/imagination/pvr_job.c
> +++ b/drivers/gpu/drm/imagination/pvr_job.c
> @@ -446,7 +446,7 @@ create_job(struct pvr_device *pvr_dev,
> if (err)
> goto err_put_job;
>
> - err = pvr_queue_job_init(job);
> + err = pvr_queue_job_init(job, pvr_file->file->client_id);
> if (err)
> goto err_put_job;
>
> diff --git a/drivers/gpu/drm/imagination/pvr_queue.c
> b/drivers/gpu/drm/imagination/pvr_queue.c
> index c4f08432882b..598180fca141 100644
> --- a/drivers/gpu/drm/imagination/pvr_queue.c
> +++ b/drivers/gpu/drm/imagination/pvr_queue.c
> @@ -1059,6 +1059,7 @@ static int pvr_queue_cleanup_fw_context(struct
> pvr_queue *queue)
> /**
> * pvr_queue_job_init() - Initialize queue related fields in a
> pvr_job object.
> * @job: The job to initialize.
> + * @drm_client_id: drm_file.client_id submitting the job
> *
> * Bind the job to a queue and allocate memory to guarantee
> pvr_queue_job_arm()
> * and pvr_queue_job_push() can't fail. We also make sure the
> context type is
> @@ -1068,7 +1069,7 @@ static int pvr_queue_cleanup_fw_context(struct
> pvr_queue *queue)
> * * 0 on success, or
> * * An error code if something failed.
> */
> -int pvr_queue_job_init(struct pvr_job *job)
> +int pvr_queue_job_init(struct pvr_job *job, uint64_t drm_client_id)
same
> {
> /* Fragment jobs need at least one native fence wait on the
> geometry job fence. */
> u32 min_native_dep_count = job->type ==
> DRM_PVR_JOB_TYPE_FRAGMENT ? 1 : 0;
> @@ -1085,7 +1086,7 @@ int pvr_queue_job_init(struct pvr_job *job)
> if (!pvr_cccb_cmdseq_can_fit(&queue->cccb,
> job_cmds_size(job, min_native_dep_count)))
> return -E2BIG;
>
> - err = drm_sched_job_init(&job->base, &queue->entity, 1,
> THIS_MODULE);
> + err = drm_sched_job_init(&job->base, &queue->entity, 1,
> THIS_MODULE, drm_client_id);
> if (err)
> return err;
>
> diff --git a/drivers/gpu/drm/imagination/pvr_queue.h
> b/drivers/gpu/drm/imagination/pvr_queue.h
> index e06ced69302f..bc556169b2cf 100644
> --- a/drivers/gpu/drm/imagination/pvr_queue.h
> +++ b/drivers/gpu/drm/imagination/pvr_queue.h
> @@ -139,7 +139,7 @@ struct pvr_queue {
>
> bool pvr_queue_fence_is_ufo_backed(struct dma_fence *f);
>
> -int pvr_queue_job_init(struct pvr_job *job);
> +int pvr_queue_job_init(struct pvr_job *job, uint64_t drm_client_id);
Same. Probably applies to the other patches, too.
>
> void pvr_queue_job_cleanup(struct pvr_job *job);
>
> diff --git a/drivers/gpu/drm/lima/lima_gem.c
> b/drivers/gpu/drm/lima/lima_gem.c
> index 9bb997dbb4b9..f46f961afc56 100644
> --- a/drivers/gpu/drm/lima/lima_gem.c
> +++ b/drivers/gpu/drm/lima/lima_gem.c
> @@ -341,7 +341,7 @@ int lima_gem_submit(struct drm_file *file, struct
> lima_submit *submit)
>
> err = lima_sched_task_init(
> submit->task, submit->ctx->context + submit->pipe,
> - bos, submit->nr_bos, vm);
> + bos, submit->nr_bos, vm, file->client_id);
> if (err)
> goto err_out1;
>
> diff --git a/drivers/gpu/drm/lima/lima_sched.c
> b/drivers/gpu/drm/lima/lima_sched.c
> index b40c90e97d7e..84599549661a 100644
> --- a/drivers/gpu/drm/lima/lima_sched.c
> +++ b/drivers/gpu/drm/lima/lima_sched.c
> @@ -113,7 +113,8 @@ static inline struct lima_sched_pipe
> *to_lima_pipe(struct drm_gpu_scheduler *sch
> int lima_sched_task_init(struct lima_sched_task *task,
> struct lima_sched_context *context,
> struct lima_bo **bos, int num_bos,
> - struct lima_vm *vm)
> + struct lima_vm *vm,
> + uint64_t drm_client_id)
> {
> int err, i;
>
> @@ -124,7 +125,8 @@ int lima_sched_task_init(struct lima_sched_task
> *task,
> for (i = 0; i < num_bos; i++)
> drm_gem_object_get(&bos[i]->base.base);
>
> - err = drm_sched_job_init(&task->base, &context->base, 1,
> vm);
> + err = drm_sched_job_init(&task->base, &context->base, 1, vm,
> + drm_client_id);
> if (err) {
> kfree(task->bos);
> return err;
> diff --git a/drivers/gpu/drm/lima/lima_sched.h
> b/drivers/gpu/drm/lima/lima_sched.h
> index 85b23ba901d5..4041468586bd 100644
> --- a/drivers/gpu/drm/lima/lima_sched.h
> +++ b/drivers/gpu/drm/lima/lima_sched.h
> @@ -88,7 +88,8 @@ struct lima_sched_pipe {
> int lima_sched_task_init(struct lima_sched_task *task,
> struct lima_sched_context *context,
> struct lima_bo **bos, int num_bos,
> - struct lima_vm *vm);
> + struct lima_vm *vm,
> + uint64_t drm_client_id);
> void lima_sched_task_fini(struct lima_sched_task *task);
>
> int lima_sched_context_init(struct lima_sched_pipe *pipe,
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index fba78193127d..ceeedd4186ef 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -30,7 +30,7 @@
> static struct msm_gem_submit *submit_create(struct drm_device *dev,
> struct msm_gpu *gpu,
> struct msm_gpu_submitqueue *queue, uint32_t nr_bos,
> - uint32_t nr_cmds)
> + uint32_t nr_cmds, uint64_t drm_client_id)
> {
> static atomic_t ident = ATOMIC_INIT(0);
> struct msm_gem_submit *submit;
> @@ -54,7 +54,8 @@ static struct msm_gem_submit *submit_create(struct
> drm_device *dev,
> return ERR_PTR(ret);
> }
>
> - ret = drm_sched_job_init(&submit->base, queue->entity, 1,
> queue);
> + ret = drm_sched_job_init(&submit->base, queue->entity, 1,
> queue,
> + drm_client_id);
> if (ret) {
> kfree(submit->hw_fence);
> kfree(submit);
> @@ -702,7 +703,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev,
> void *data,
> }
> }
>
> - submit = submit_create(dev, gpu, queue, args->nr_bos, args-
> >nr_cmds);
> + submit = submit_create(dev, gpu, queue, args->nr_bos, args-
> >nr_cmds,
> + file->client_id);
> if (IS_ERR(submit)) {
> ret = PTR_ERR(submit);
> goto out_post_unlock;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c
> b/drivers/gpu/drm/nouveau/nouveau_sched.c
> index 4412f2711fb5..ebc31adea39a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_sched.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_sched.c
> @@ -87,7 +87,8 @@ nouveau_job_init(struct nouveau_job *job,
> }
>
> ret = drm_sched_job_init(&job->base, &sched->entity,
> - args->credits, NULL);
> + args->credits, NULL,
> + job->file_priv->client_id);
> if (ret)
> goto err_free_chains;
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 04d615df5259..a8135bd75cae 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -312,7 +312,7 @@ static int panfrost_ioctl_submit(struct
> drm_device *dev, void *data,
>
> ret = drm_sched_job_init(&job->base,
> &file_priv->sched_entity[slot],
> - 1, NULL);
> + 1, NULL, file->client_id);
> if (ret)
> goto out_put_job;
>
> diff --git a/drivers/gpu/drm/scheduler/sched_fence.c
> b/drivers/gpu/drm/scheduler/sched_fence.c
> index 0f35f009b9d3..909b886cd379 100644
> --- a/drivers/gpu/drm/scheduler/sched_fence.c
> +++ b/drivers/gpu/drm/scheduler/sched_fence.c
> @@ -204,7 +204,8 @@ struct drm_sched_fence *to_drm_sched_fence(struct
> dma_fence *f)
> EXPORT_SYMBOL(to_drm_sched_fence);
>
> struct drm_sched_fence *drm_sched_fence_alloc(struct
> drm_sched_entity *entity,
> - void *owner)
> + void *owner,
> + uint64_t
> drm_client_id)
> {
> struct drm_sched_fence *fence = NULL;
>
> @@ -213,6 +214,7 @@ struct drm_sched_fence
> *drm_sched_fence_alloc(struct drm_sched_entity *entity,
> return NULL;
>
> fence->owner = owner;
> + fence->drm_client_id = drm_client_id;
> spin_lock_init(&fence->lock);
>
> return fence;
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> index 7ce25281c74c..28ac709750e9 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -776,6 +776,7 @@ EXPORT_SYMBOL(drm_sched_resubmit_jobs);
> * @credits: the number of credits this job contributes to the
> schedulers
> * credit limit
> * @owner: job owner for debugging
> + * @drm_client_id: drm_file.client_id of the owner
> *
> * Refer to drm_sched_entity_push_job() documentation
> * for locking considerations.
> @@ -796,7 +797,8 @@ EXPORT_SYMBOL(drm_sched_resubmit_jobs);
> */
> int drm_sched_job_init(struct drm_sched_job *job,
> struct drm_sched_entity *entity,
> - u32 credits, void *owner)
> + u32 credits, void *owner,
> + uint64_t drm_client_id)
> {
> if (!entity->rq) {
> /* This will most likely be followed by missing
> frames
> @@ -822,7 +824,7 @@ int drm_sched_job_init(struct drm_sched_job *job,
>
> job->entity = entity;
> job->credits = credits;
> - job->s_fence = drm_sched_fence_alloc(entity, owner);
> + job->s_fence = drm_sched_fence_alloc(entity, owner,
> drm_client_id);
> if (!job->s_fence)
> return -ENOMEM;
>
> diff --git a/drivers/gpu/drm/v3d/v3d_submit.c
> b/drivers/gpu/drm/v3d/v3d_submit.c
> index d607aa9c4ec2..a086da31f441 100644
> --- a/drivers/gpu/drm/v3d/v3d_submit.c
> +++ b/drivers/gpu/drm/v3d/v3d_submit.c
> @@ -168,7 +168,7 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file
> *file_priv,
> job->file = file_priv;
>
> ret = drm_sched_job_init(&job->base, &v3d_priv-
> >sched_entity[queue],
> - 1, v3d_priv);
> + 1, v3d_priv, file_priv->client_id);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/xe/xe_sched_job.c
> b/drivers/gpu/drm/xe/xe_sched_job.c
> index eeccc1c318ae..6617555e7a51 100644
> --- a/drivers/gpu/drm/xe/xe_sched_job.c
> +++ b/drivers/gpu/drm/xe/xe_sched_job.c
> @@ -113,7 +113,8 @@ struct xe_sched_job *xe_sched_job_create(struct
> xe_exec_queue *q,
> kref_init(&job->refcount);
> xe_exec_queue_get(job->q);
>
> - err = drm_sched_job_init(&job->drm, q->entity, 1, NULL);
> + err = drm_sched_job_init(&job->drm, q->entity, 1, NULL,
> + q->xef->drm->client_id);
> if (err)
> goto err_free;
>
> diff --git a/include/drm/gpu_scheduler.h
> b/include/drm/gpu_scheduler.h
> index 95e17504e46a..42c381449443 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -311,6 +311,13 @@ struct drm_sched_fence {
> * @owner: job owner for debugging
> */
> void *owner;
> +
> + /**
> + * @drm_client_id:
> + *
> + * The client_id of the drm_file who owned the job.
s/who owned/which owns
Thx,
P.
> + */
> + uint64_t drm_client_id;
> };
>
> struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
> @@ -563,7 +570,8 @@ int drm_sched_init(struct drm_gpu_scheduler
> *sched,
> void drm_sched_fini(struct drm_gpu_scheduler *sched);
> int drm_sched_job_init(struct drm_sched_job *job,
> struct drm_sched_entity *entity,
> - u32 credits, void *owner);
> + u32 credits, void *owner,
> + uint64_t drm_client_id);
> void drm_sched_job_arm(struct drm_sched_job *job);
> int drm_sched_job_add_dependency(struct drm_sched_job *job,
> struct dma_fence *fence);
> @@ -624,7 +632,7 @@ bool drm_sched_entity_is_ready(struct
> drm_sched_entity *entity);
> int drm_sched_entity_error(struct drm_sched_entity *entity);
>
> struct drm_sched_fence *drm_sched_fence_alloc(
> - struct drm_sched_entity *s_entity, void *owner);
> + struct drm_sched_entity *s_entity, void *owner, uint64_t
> drm_client_id);
> void drm_sched_fence_init(struct drm_sched_fence *fence,
> struct drm_sched_entity *entity);
> void drm_sched_fence_free(struct drm_sched_fence *fence);
