Re: [PATCH 0/8] Retry page fault handling for Vega10

[Oded Gabbay <oded.gabbay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Mon, Sep 11, 2017 at 10:29 PM, Deucher, Alexander
<Alexander.Deucher@amd.com> wrote:
>> -----Original Message-----
>> From: Kuehling, Felix
>> Sent: Wednesday, September 06, 2017 5:54 PM
>> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander; Oded Gabbay;
>> Koenig, Christian
>> Subject: Re: [PATCH 0/8] Retry page fault handling for Vega10
>>
>> I realized that the drm-next-4.15-wip branch isn't very useful for
>> testing this, because it has no display support for Vega10 and no KFD
>> support for Vega10. So you can't test graphics or compute on Vega10 with
>> this branch. On the other hand, I need to make changes in both KFD and
>> AMDGPU, so I tried to avoid an amd-internal branch. But it seems until
>> either DAL or KFD is upstream, it'll have to remain on an AMD-internal
>> branch (amd-staging-4.12 for now, to be changed soon). Christian, would
>> this enable any of work you were going to do?
>>
>> Alex, is this going to make your regular upstreaming more difficult? Or
>> are you OK with upstreaming KFD changes that have dependencies with
>> amdgpu changes? Oded, would you be OK with Alex upstreaming KFD
>> changes
>> along with amdgpu changes? Assuming they have your "Reviewed-by"?
>
> I'm fine to take the changes through my tree if Oded is ok with it.

+John,

If Alex is fine with it then I'm fine with it as well, as long as this
is a temporary solution until some point where you have some
convergence between your internal code and the upstream one.
And of course if you have amdkfd only changes then that can be
upstreamed through me directly.

Having said that, if you/John/Alex think that this is a more permanent
solution, then maybe a better plan is to first unify the drivers (as
was discussed many times) before starting to upstream changes. If the
90% of the changes are in both drivers, then there is really no point
of keeping amdkfd as a separate driver.

Oded


>
>>
>> I also haven't got any feedback from LKLM on the addition of the chash
>> data structure to kernel/lib. I'm considering adding it in
>> drivers/gpu/drm/amd/chash as an interim step. It can be moved to lib
>> later, if other components are interested in using it. Any objections?
>
> Works for me.
>
> Alex
>
>>
>> Regards,
>>   Felix
>>
>>
>> On 2017-08-29 06:25 PM, Felix Kuehling wrote:
>> > Rebased on the public drm-next-4.15-wip. Patch 8 from the WIP patch
>> > series did not apply at all, because upstream KFD doesn't support
>> > GPUVM yet.
>> >
>> > The "lib: Closed hash table ..." change is updated and the same as
>> > what I sent to LKML yesterday. Changes are mainly in the way the self
>> > test is hooked up, Kconfig options and some checkpatch fixes. If it
>> > takes too long to get accepted upstream, I could add it under
>> > drivers/gpu/drm/amd/chash in the interim.
>> >
>> > This is only compile tested on this branch. I can't do much more
>> > because the upstream KFD doesn't support Vega10 and GPUVM yet.
>> Someone
>> > will have to add PASID support for graphics on top of this.
>> >
>> > TODO:
>> > * Finish upstreaming KFD
>> > * Allocate PASIDs for graphics contexts
>> > * Setup VMID-PASID mapping during graphics command submission
>> > * Confirm that graphics page faults have the correct PASID in the IV
>> >
>> > Felix Kuehling (8):
>> >   drm/amdgpu: Fix error handling in amdgpu_vm_init
>> >   drm/amdgpu: Add PASID management
>> >   drm/radeon: Add PASID manager for KFD
>> >   drm/amdkfd: Separate doorbell allocation from PASID
>> >   drm/amdkfd: Use PASID manager from KGD
>> >   drm/amdgpu: Add prescreening stage in IH processing
>> >   lib: Closed hash table with low overhead
>> >   drm/amdgpu: Track pending retry faults in IH and VM (v2)
>> >
>> >  drivers/gpu/drm/Kconfig                           |   1 +
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h               |   2 +
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c |   2 +
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c |   2 +
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c            |  82 +++
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h            |  12 +
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c           |   2 +-
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c            |  84 ++-
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h            |  21 +-
>> >  drivers/gpu/drm/amd/amdgpu/cik_ih.c               |  14 +
>> >  drivers/gpu/drm/amd/amdgpu/cz_ih.c                |  14 +
>> >  drivers/gpu/drm/amd/amdgpu/iceland_ih.c           |  14 +
>> >  drivers/gpu/drm/amd/amdgpu/si_ih.c                |  14 +
>> >  drivers/gpu/drm/amd/amdgpu/tonga_ih.c             |  14 +
>> >  drivers/gpu/drm/amd/amdgpu/vega10_ih.c            |  90 ++++
>> >  drivers/gpu/drm/amd/amdkfd/kfd_device.c           |   7 -
>> >  drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c         |  50 +-
>> >  drivers/gpu/drm/amd/amdkfd/kfd_module.c           |   6 -
>> >  drivers/gpu/drm/amd/amdkfd/kfd_pasid.c            |  90 ++--
>> >  drivers/gpu/drm/amd/amdkfd/kfd_priv.h             |  10 +-
>> >  drivers/gpu/drm/amd/amdkfd/kfd_process.c          |   6 +
>> >  drivers/gpu/drm/amd/include/kgd_kfd_interface.h   |   6 +
>> >  drivers/gpu/drm/radeon/radeon_kfd.c               |  31 ++
>> >  include/linux/chash.h                             | 358 +++++++++++++
>> >  lib/Kconfig                                       |  24 +
>> >  lib/Makefile                                      |   2 +
>> >  lib/chash.c                                       | 622 ++++++++++++++++++++++
>> >  27 files changed, 1489 insertions(+), 91 deletions(-)
>> >  create mode 100644 include/linux/chash.h
>> >  create mode 100644 lib/chash.c
>> >
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx