On 23/09/2024 11:18, Boris Brezillon wrote:
> On Mon, 23 Sep 2024 10:07:14 +0100
> Steven Price <steven.price@xxxxxxx> 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
