On 23/09/2024 11:18, Boris Brezillon wrote:
On Mon, 23 Sep 2024 10:07:14 +0100 Steven Price steven.price@arm.com wrote:
+static struct dma_fence * +queue_run_job(struct drm_sched_job *sched_job) +{
- struct panthor_job *job = container_of(sched_job, struct panthor_job, base);
- struct panthor_group *group = job->group;
- struct panthor_queue *queue = group->queues[job->queue_idx];
- struct panthor_device *ptdev = group->ptdev;
- struct panthor_scheduler *sched = ptdev->scheduler;
- struct panthor_job_ringbuf_instrs instrs;
instrs isn't initialised...
- struct panthor_job_cs_params cs_params;
- struct dma_fence *done_fence;
- int ret;
/* Stream size is zero, nothing to do except making sure all previously * submitted jobs are done before we signal the @@ -2900,17 +3062,23 @@ queue_run_job(struct drm_sched_job *sched_job) queue->fence_ctx.id, atomic64_inc_return(&queue->fence_ctx.seqno));
- memcpy(queue->ringbuf->kmap + ringbuf_insert,
call_instrs, sizeof(call_instrs));
- job->profiling.slot = queue->profiling.seqno++;
- if (queue->profiling.seqno == queue->profiling.slot_count)
queue->profiling.seqno = 0;- job->ringbuf.start = queue->iface.input->insert;
- get_job_cs_params(job, &cs_params);
- prepare_job_instrs(&cs_params, &instrs);
...but it's passed into prepare_job_instrs() which depends on instrs.count (same bug as was in calc_job_credits()) - sorry I didn't spot it last review.
Hm, can't we initialize instr::count to zero in prepare_job_instrs() instead?
Indeed that would probably be better! I hadn't noticed there were two places in the previous review.
Steve