[PATCH 3/3] drm/amdgpu: move static CSA address to top of address space

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

 



Thanks, can you give the patches a try as well?

I don't have access to SRIOV hardware to test them.

Christian.

Am 23.01.2018 um 14:44 schrieb Liu, Monk:
>
> looks this is HSA feature to me that CPU and GPU share the same 
> virtual address ....
>
>
> Ack-by: Monk Liu <monk.liu at amd.com>
>
> ------------------------------------------------------------------------
> *From:* Koenig, Christian
> *Sent:* Tuesday, January 23, 2018 8:51:21 PM
> *To:* Liu, Monk; He, Roger; amd-gfx at lists.freedesktop.org
> *Subject:* Re: [PATCH 3/3] drm/amdgpu: move static CSA address to top 
> of address space
> The ATC (or alternatively HMM) maps the CPU addresses into the GPU 
> address space.
>
> Now on Linux the lower range (0x0-0x7fffffffffff) is used for the 
> userspace address space on the CPU, so when we want to use that on the 
> GPU it must be free of other mappings.
>
> GFX8 and GFX7 hardware had a separate flag which address space to use 
> in the memory instructions, but on GFX9 it's all one big shared 
> address space and we need to make sure that we don't kick each others 
> feet.
>
> Had to move the addresses Mesa use as well because of this and older 
> VCE firmware unfortunately was buggy regarding this as well.
>
> Regards,
> Christian.
>
> Am 23.01.2018 um 13:43 schrieb Liu, Monk:
>>
>> OK I see, but why HMM/ATC/SVN want  low 8MB range ? why they cannot 
>> use high range address ?
>>
>>
>> ------------------------------------------------------------------------
>> *From:* Christian König <ckoenig.leichtzumerken at gmail.com> 
>> <mailto:ckoenig.leichtzumerken at gmail.com>
>> *Sent:* Tuesday, January 23, 2018 4:26:21 PM
>> *To:* Liu, Monk; He, Roger; amd-gfx at lists.freedesktop.org 
>> <mailto:amd-gfx at lists.freedesktop.org>
>> *Subject:* Re: [PATCH 3/3] drm/amdgpu: move static CSA address to top 
>> of address space
>>> Any bug introduced by original design ? I don't see why 8MB- 8KB as 
>>> the start vm address of CSA has any trouble compared with top range ?
>> On gfx9 we need the lower address range (0x0-0x7fffffffffff) for 
>> HMM/ATC/SVM. If we map the 8k CSA in there and hit it by accident 
>> then the GPU will not do what it is supposed to do :)
>>
>> I could limit the change to only gfx9, but I think the code will be 
>> simpler if we change both gfx8 and gfx9.
>>
>>> Besides, please note that you also need modify gfx8/9 source to 
>>> align emit_cs/de_meta with your change
>> That was the design flaw I was talking about. In other words I 
>> couldn't find the code where that address was actually used.
>>
>> We should use the macro define in those functions as well. Going to 
>> send fixed for this in a minute.
>>
>> Thanks,
>> Christian.
>>
>> Am 23.01.2018 um 04:23 schrieb Liu, Monk:
>>>
>>> anyway I prefer no change on that part unless there issues or bug 
>>> need to fix by the change.
>>>
>>> the CSA address is for use by CPG h/w not s/w, and since 8MB is a 
>>> reserved range for each VM I don't see
>>>
>>> it's a design flaw with it
>>>
>>> ------------------------------------------------------------------------
>>> *From:* Liu, Monk
>>> *Sent:* Tuesday, January 23, 2018 11:20:35 AM
>>> *To:* He, Roger; Christian König; amd-gfx at lists.freedesktop.org 
>>> <mailto:amd-gfx at lists.freedesktop.org>
>>> *Subject:* Re: [PATCH 3/3] drm/amdgpu: move static CSA address to 
>>> top of address space
>>>
>>> Any bug introduced by original design ? I don't see why 8MB- 8KB as 
>>> the start vm address of CSA has any trouble compared with top range 
>>> ? Besides, please note that you also need modify gfx8/9 source to 
>>> align emit_cs/de_meta with your change
>>>
>>>
>>>
>>> ------------------------------------------------------------------------
>>> *From:* He, Roger
>>> *Sent:* Tuesday, January 23, 2018 10:36:31 AM
>>> *To:* Christian König; Liu, Monk; amd-gfx at lists.freedesktop.org 
>>> <mailto:amd-gfx at lists.freedesktop.org>
>>> *Subject:* RE: [PATCH 3/3] drm/amdgpu: move static CSA address to 
>>> top of address space
>>>
>>>
>>> -----Original Message-----
>>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On 
>>> Behalf Of Christian K?nig
>>> Sent: Monday, January 22, 2018 6:44 PM
>>> To: Liu, Monk <Monk.Liu at amd.com> <mailto:Monk.Liu at amd.com>; 
>>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org>
>>> Subject: [PATCH 3/3] drm/amdgpu: move static CSA address to top of 
>>> address space
>>>
>>> There seems to be a design flaw with this since the address of the 
>>> static CSA is never exported anywhere.
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com> 
>>> <mailto:christian.koenig at amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 15 +++++++-------- 
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h |  6 ++++--
>>>  2 files changed, 11 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> index e7dfb7b44b4b..c7d24af03e3e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>>> @@ -55,11 +55,10 @@ void amdgpu_free_static_csa(struct amdgpu_device 
>>> *adev) {
>>>
>>>  /*
>>>   * amdgpu_map_static_csa should be called during amdgpu_vm_init
>>> - * it maps virtual address "AMDGPU_VA_RESERVED_SIZE - AMDGPU_CSA_SIZE"
>>> - * to this VM, and each command submission of GFX should use this 
>>> virtual
>>> - * address within META_DATA init package to support SRIOV gfx 
>>> preemption.
>>> + * it maps virtual address "AMDGPU_CSA_VADDR" to this VM, and each
>>> + command
>>> + * submission of GFX should use this virtual address within META_DATA
>>> + init
>>> + * package to support SRIOV gfx preemption.
>>>   */
>>> -
>>>  int amdgpu_map_static_csa(struct amdgpu_device *adev, struct 
>>> amdgpu_vm *vm,
>>>                            struct amdgpu_bo_va **bo_va)
>>>  {
>>> @@ -90,7 +89,7 @@ int amdgpu_map_static_csa(struct amdgpu_device 
>>> *adev, struct amdgpu_vm *vm,
>>>                  return -ENOMEM;
>>>          }
>>>
>>> -       r = amdgpu_vm_alloc_pts(adev, (*bo_va)->base.vm, 
>>> AMDGPU_CSA_VADDR,
>>> +       r = amdgpu_vm_alloc_pts(adev, (*bo_va)->base.vm,
>>> +AMDGPU_CSA_VADDR(adev),
>>> AMDGPU_CSA_SIZE);
>>>          if (r) {
>>>                  DRM_ERROR("failed to allocate pts for static CSA, 
>>> err=%d\n", r); @@ -99,9 +98,9 @@ int amdgpu_map_static_csa(struct 
>>> amdgpu_device *adev, struct amdgpu_vm *vm,
>>>                  return r;
>>>          }
>>>
>>> -       r = amdgpu_vm_bo_map(adev, *bo_va, AMDGPU_CSA_VADDR, 0, 
>>> AMDGPU_CSA_SIZE,
>>> -                            AMDGPU_PTE_READABLE | 
>>> AMDGPU_PTE_WRITEABLE |
>>> - AMDGPU_PTE_EXECUTABLE);
>>> +       r = amdgpu_vm_bo_map(adev, *bo_va, AMDGPU_CSA_VADDR(adev), 0,
>>> +                            AMDGPU_CSA_SIZE, AMDGPU_PTE_READABLE |
>>> + AMDGPU_PTE_WRITEABLE | AMDGPU_PTE_EXECUTABLE);
>>>
>>>          if (r) {
>>>                  DRM_ERROR("failed to do bo_map on static CSA, 
>>> err=%d\n", r); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> index 6a83425aa9ed..499362b55e45 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>>> @@ -251,8 +251,10 @@ struct amdgpu_virt {
>>>          uint32_t gim_feature;
>>>  };
>>>
>>> -#define AMDGPU_CSA_SIZE    (8 * 1024)
>>> -#define AMDGPU_CSA_VADDR (AMDGPU_VA_RESERVED_SIZE - AMDGPU_CSA_SIZE)
>>> +#define AMDGPU_CSA_SIZE                (8 * 1024)
>>> +#define AMDGPU_CSA_VADDR(adev) \
>>>         +       (((adev)->vm_manager.max_pfn << 
>>> AMDGPU_GPU_PAGE_SHIFT) + \
>>>         +        AMDGPU_VA_RESERVED_SIZE)
>>>
>>> If I  understand your intention correctly, it should be that:
>>> +       (((adev)->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT)  - \
>>> +        AMDGPU_VA_RESERVED_SIZE)
>>>
>>> Thanks
>>> Roger(Hongbo.He)
>>>
>>>  #define amdgpu_sriov_enabled(adev) \
>>>  ((adev)->virt.caps & AMDGPU_SRIOV_CAPS_ENABLE_IOV)
>>> --
>>> 2.14.1
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org>
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>

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


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

  Powered by Linux