regression with d6c650c0a8f6f671e49553725e1db541376d95f2

[Christian König]

Yeah, that change is actually incorrect and should be reverted.

What we really need to do is remove dropping sched_job->s_fence from 
amd_sched_process_job() into amd_sched_job_finish() directly before the 
call to free_job().

Regards,
Christian.

Am 13.10.2017 um 09:24 schrieb Liu, Monk:
> commit d6c650c0a8f6f671e49553725e1db541376d95f2
> Author: Nicolai Hähnle <nicolai.haehnle at amd.com>
> @@ -611,6 +611,10 @@ static int amd_sched_main(void *param)
>
>                 fence = sched->ops->run_job(sched_job);
>                 amd_sched_fence_scheduled(s_fence);
> +
> +               /* amd_sched_process_job drops the job's reference of 
> the fence. */
> +               sched_job->s_fence = NULL;
> +
>                 if (fence) {
>                         s_fence->parent = dma_fence_get(fence);
>                         r = dma_fence_add_callback(fence, &s_fence->cb,
>
> Hi Nicolai
>
>
> with this patch, you will break "amdgpu_sched_hw_job_reset()"routine:
>
> voidamd_sched_hw_job_reset(structamd_gpu_scheduler *sched)
> {
> structamd_sched_job *s_job;
> spin_lock(&sched->job_list_lock);
> list_for_each_entry_reverse(s_job, &sched->ring_mirror_list, node) {
> if(s_job->s_fence->parent&&
> fence_remove_callback(s_job->s_fence->parent,
> &s_job->s_fence->cb)) {
> fence_put(s_job->s_fence->parent);
>             s_job->s_fence->parent=NULL;
> atomic_dec(&sched->hw_rq_count);
>         }
>     }
> spin_unlock(&sched->job_list_lock);
> }
>
>
> see that without sched_job->s_fence, you cannot remove the callback 
> from its hw fence,
>
>
> any idea??
>
>
> BR Monk
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20171013/d7608823/attachment.html>