[PATCH 6/7] drm/amdgpu: recovery hw ring when gpu reset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2016å¹´06æ??29æ?¥ 17:03, Christian König wrote:
> Am 29.06.2016 um 10:09 schrieb Chunming Zhou:
>> Change-Id: I8e554d34c9e477ea255e0ed2a936397aa5f665e7
>> Signed-off-by: Chunming Zhou <David1.Zhou at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c  |  4 ++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c     | 23 
>> ++++++++++++++---------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c    | 22 ++++++++++++++++++++++
>>   5 files changed, 42 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 163429c8..03b0fe7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -763,6 +763,7 @@ void amdgpu_job_free(struct amdgpu_job *job);
>>   int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring 
>> *ring,
>>                 struct amd_sched_entity *entity, void *owner,
>>                 struct fence **f);
>> +void amdgpu_job_recovery(struct amd_gpu_scheduler *sched);
>>     struct amdgpu_ring {
>>       struct amdgpu_device        *adev;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 5c4691c..2c8e7f4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1994,13 +1994,14 @@ retry:
>>       }
>>       /* restore scratch */
>>       amdgpu_atombios_scratch_regs_restore(adev);
>> -    if (0) {
>> +    if (!r) {
>>           for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>>               struct amdgpu_ring *ring = adev->rings[i];
>>               if (!ring)
>>                   continue;
>> +            amdgpu_job_recovery(&ring->sched);
>>               kthread_unpark(ring->sched.thread);
>> -            amdgpu_ring_restore(ring, ring_sizes[i], ring_data[i]);
>> +            kfree(ring_data[i]);
>>               ring_sizes[i] = 0;
>>               ring_data[i] = NULL;
>>           }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> index 72bf9f8..8af9903 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> @@ -366,9 +366,9 @@ int amdgpu_fence_driver_init_ring(struct 
>> amdgpu_ring *ring,
>>       setup_timer(&ring->fence_drv.fallback_timer, 
>> amdgpu_fence_fallback,
>>               (unsigned long)ring);
>>   -    ring->fence_drv.num_fences_mask = num_hw_submission * 2 - 1;
>> +    ring->fence_drv.num_fences_mask = num_hw_submission * 4 - 1;
>>       spin_lock_init(&ring->fence_drv.lock);
>> -    ring->fence_drv.fences = kcalloc(num_hw_submission * 2, 
>> sizeof(void *),
>> +    ring->fence_drv.fences = kcalloc(num_hw_submission * 4, 
>> sizeof(void *),
>>                        GFP_KERNEL);
>>       if (!ring->fence_drv.fences)
>>           return -ENOMEM;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 34e3542..702bd9b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -193,14 +193,19 @@ int amdgpu_ib_schedule(struct amdgpu_ring 
>> *ring, unsigned num_ibs,
>>       if (ring->funcs->emit_hdp_invalidate)
>>           amdgpu_ring_emit_hdp_invalidate(ring);
>>   -    r = amdgpu_fence_emit(ring, &hwf);
>> -    if (r) {
>> -        dev_err(adev->dev, "failed to emit fence (%d)\n", r);
>> -        if (job && job->vm_id)
>> -            amdgpu_vm_reset_id(adev, job->vm_id);
>> -        amdgpu_ring_undo(ring);
>> -        return r;
>> -    }
>> +    if (!job || !job->fence) {
>> +        r = amdgpu_fence_emit(ring, &hwf);
>> +        if (r) {
>> +            dev_err(adev->dev, "failed to emit fence (%d)\n", r);
>> +            if (job && job->vm_id)
>> +                amdgpu_vm_reset_id(adev, job->vm_id);
>> +            amdgpu_ring_undo(ring);
>> +            return r;
>> +        }
>> +    } else
>> +        /* re-submit fence when gpu reset */
>> +        amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
>> +                       job->fence->seqno, AMDGPU_FENCE_FLAG_INT);
>
> You shouldn't resubmit the old hardware fence here, but rather 
> allocate a new fence number.
This will introduce new problem and let things complicate, E.g. old hw 
fence f1, f2 in hang engine, that means sched_f1, sched_f2 is callback 
from f1,f2, if allocating new fence number when re-submitting them, will 
have new f3,f4. When f3 is signalled, then that means f1,f2, 
sched_f1,sched_f2 are signalled, then userspace think cs of sched_f2 is 
completed, but not in fact, which is f4,sched_f4.

I don't know why we shouldn't/couldn't the old hw fence.

>
>>         /* wrap the last IB with fence */
>>       if (job && job->uf_bo) {
>> @@ -212,7 +217,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, 
>> unsigned num_ibs,
>>       }
>>         if (f)
>> -        *f = fence_get(hwf);
>> +        *f = (job && job->fence) ? job->fence : fence_get(hwf);
>>         if (patch_offset != ~0 && ring->funcs->patch_cond_exec)
>>           amdgpu_ring_patch_cond_exec(ring, patch_offset);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> index 83771c1..32fad1c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>> @@ -39,6 +39,28 @@ static void amdgpu_job_timedout(struct 
>> amd_sched_job *s_job)
>>       amdgpu_gpu_reset(job->adev);
>>   }
>>   +void amdgpu_job_recovery(struct amd_gpu_scheduler *sched)
>> +{
>> +    struct amd_sched_job *s_job, *tmp;
>> +
>> +    spin_lock(&sched->job_list_lock);
>> +    list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, 
>> node) {
>> +        struct amdgpu_job *job = to_amdgpu_job(s_job);
>> +        if (job->vm) {
>> +            struct amdgpu_vm_id *id = 
>> &job->adev->vm_manager.ids[job->vm_id];
>> +            job->vm_pd_addr = 
>> amdgpu_bo_gpu_offset(job->vm->page_directory);
>> +            id->pd_gpu_addr = job->vm_pd_addr;
>
> That is clearly not a good idea, the page directory could be moving 
> somewhere else when this is happening already.
Hmm, how about directly to use id->pd_gpu_addr? since pd BO must be 
there, id->pd_gpu_addr is saved that.

>
>> +        }
>> +        sched->ops->run_job(s_job);
>> +    }
>> +    s_job = list_first_entry_or_null(&sched->ring_mirror_list,
>> +                     struct amd_sched_job, node);
>> +    if (s_job)
>> +        schedule_delayed_work(&s_job->work_tdr, sched->timeout);
>> +
>> +    spin_unlock(&sched->job_list_lock);
>> +}
>
> The whole function belongs into the scheduler.
if vmid is processed, this indeed be sched function.

Thanks,
David Zhou
>
> Christian.
>
>> +
>>   int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>>                struct amdgpu_job **job, struct amdgpu_vm *vm)
>>   {
>



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux