On Fri, 2 Jul 2021 11:08:58 +0100 Steven Price <steven.price@xxxxxxx> wrote: > On 01/07/2021 10:12, Boris Brezillon wrote: > > Needed to keep VkQueues isolated from each other. > > One more comment I noticed when I tried this out: > > [...] > > +struct panfrost_submitqueue * > > +panfrost_submitqueue_create(struct panfrost_file_priv *ctx, > > + enum panfrost_submitqueue_priority priority, > > + u32 flags) > > +{ > > + struct panfrost_submitqueue *queue; > > + enum drm_sched_priority sched_prio; > > + int ret, i; > > + > > + if (flags || priority >= PANFROST_SUBMITQUEUE_PRIORITY_COUNT) > > + return ERR_PTR(-EINVAL); > > + > > + queue = kzalloc(sizeof(*queue), GFP_KERNEL); > > + if (!queue) > > + return ERR_PTR(-ENOMEM); > > + > > + queue->pfdev = ctx->pfdev; > > + sched_prio = to_sched_prio(priority); > > + for (i = 0; i < NUM_JOB_SLOTS; i++) { > > + struct drm_gpu_scheduler *sched; > > + > > + sched = panfrost_job_get_sched(ctx->pfdev, i); > > + ret = drm_sched_entity_init(&queue->sched_entity[i], > > + sched_prio, &sched, 1, NULL); > > + if (ret) > > + break; > > + } > > + > > + if (ret) { > > + for (i--; i >= 0; i--) > > + drm_sched_entity_destroy(&queue->sched_entity[i]); > > + > > + return ERR_PTR(ret); > > + } > > + > > + kref_init(&queue->refcount); > > + idr_lock(&ctx->queues); > > + ret = idr_alloc(&ctx->queues, queue, 0, INT_MAX, GFP_KERNEL); > > This makes lockdep complain. idr_lock() is a spinlock and GFP_KERNEL can > sleep. So either we need to bring our own mutex here or not use GFP_KERNEL. > Ouch! I wonder why I don't see that (I have lockdep enabled, and the igt tests should have exercised this path). > Steve