[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <42d3b0c0-bad6-4ac2-b755-6cbfc5ec5524@gmail.com>
Date: Thu, 9 Nov 2023 20:25:04 -0500
From: Luben Tuikov <ltuikov89@...il.com>
To: Danilo Krummrich <dakr@...hat.com>, airlied@...il.com,
daniel@...ll.ch, matthew.brost@...el.com,
boris.brezillon@...labora.com, christian.koenig@....com,
faith@...strand.net
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH drm-misc-next v6] drm/sched: implement dynamic job-flow
control
On 2023-11-09 19:57, Luben Tuikov wrote:
> On 2023-11-09 19:16, Danilo Krummrich wrote:
[snip]
>> @@ -667,6 +771,8 @@ EXPORT_SYMBOL(drm_sched_resubmit_jobs);
>> * drm_sched_job_init - init a scheduler job
>> * @job: scheduler job to init
>> * @entity: scheduler entity to use
>> + * @credits: the number of credits this job contributes to the schedulers
>> + * credit limit
>> * @owner: job owner for debugging
>> *
>> * Refer to drm_sched_entity_push_job() documentation
>> @@ -684,7 +790,7 @@ EXPORT_SYMBOL(drm_sched_resubmit_jobs);
>> */
>> int drm_sched_job_init(struct drm_sched_job *job,
>> struct drm_sched_entity *entity,
>> - void *owner)
>> + u32 credits, void *owner)
>> {
>> if (!entity->rq) {
>> /* This will most likely be followed by missing frames
>> @@ -695,7 +801,11 @@ int drm_sched_job_init(struct drm_sched_job *job,
>> return -ENOENT;
>> }
>>
>> + if (unlikely(!credits))
>> + return -EINVAL;
>> +
>
> This will most likely result in bad user experience (read: blank screen),
> and debugging this would be really hard without something to go by
> in the kernel log.
>
> (This was exactly the case with amdgpu when 56e449603f0ac5
> ("drm/sched: Convert the GPU scheduler to variable number of run-queues")
> was being worked on and merged. Without the drm_err() on missing rq in
> the lines immediately before the hunk above returning -ENOENT, there
> was no indication why setting up an fb was failing very early on (blank screen).)
>
> So it is best to print a "[drm] *ERROR* "-equivalent string in the logs,
> so that we can make a note of this, without relying on drivers, old and new, logging
> that drm_sched_job_init() failed.
If you add _exactly_ this,
if (unlikely(!credits)) {
pr_err("*ERROR* %s: credits cannot be 0!\n", __func__)
return -EINVAL;
}
You can add my,
Reviewed-by: Luben Tuikov <ltuikov89@...il.com>
and push it.
--
Regards,
Luben
Download attachment "OpenPGP_0x4C15479431A334AF.asc" of type "application/pgp-keys" (665 bytes)
Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)
Powered by blists - more mailing lists