On 02/07/2021 11:52, Boris Brezillon wrote:
> 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).
Actually I'm not sure it technically lockdep - have you got
CONFIG_DEBUG_ATOMIC_SLEEP set?
Steve
