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