On 04/10/2021 13:24, Boris Brezillon wrote:
On Mon, 4 Oct 2021 12:30:42 +0100 Steven Price steven.price@arm.com wrote:
[...]
It took me a while to convince myself that the reference counting for the PM reference is correct. Before panfrost_job_hw_submit() always returned with an extra reference, but now we have a case which doesn't. AFAICT this is probably fine because we dereference on the path where the hardware has completed the job (which obviously won't happen here). But I'm still a bit uneasy whether the reference counts are always correct.
I think it is. We only decrement the PM count in the interrupt handler path, and as you pointed out, we won't reach that path here. But if that helps, I can move this if to `panfrost_job_run()`:
/* Nothing to execute, signal the fence directly. */ if (job->requirements & PANFROST_JD_REQ_DEP_ONLY) dma_fence_signal_locked(job->done_fence); else panfrost_job_hw_submit(job, slot);
I think that would make it a bit more readable - really panfrost_job_hw_submit() needs a bit of TLC, I did post a patch ages ago[1] but it didn't get any feedback and then I forgot about it. Things have moved on so it would need a little bit of rework.
Thanks,
Steve
[1] https://lore.kernel.org/dri-devel/20210512152419.30003-1-steven.price@arm.co...