[PATCH v2 4/7] drm/panfrost: Add the ability to create submit queues

[Boris Brezillon]

On Fri, 2 Jul 2021 11:58:34 +0100
Steven Price <steven.price at arm.com> wrote:

> On 02/07/2021 11:52, Boris Brezillon wrote:
> > On Fri, 2 Jul 2021 11:08:58 +0100
> > Steven Price <steven.price at arm.com> 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?

Nope, I was missing that one :-/.