On 04/10/2021 13:24, Boris Brezillon wrote: > On Mon, 4 Oct 2021 12:30:42 +0100 > Steven Price <steven.price@xxxxxxx> 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@xxxxxxx/
