Re: [PATCH 6/6] drm/amdgpu: explicit give BO type to amdgpu_bo_create
On 2018-03-07 04:16 AM, Christian König wrote:
> Am 06.03.2018 um 22:29 schrieb Felix Kuehling:
>> NAK.
>>
>> For KFD we need the ability to create a BO from an SG list that doesn't
>> come from another BO. We use this for mapping pages from the doorbell
>> aperture into GPUVM for GPU self-dispatch.
> You can still do this, see amdgpu_gem_prime_import_sg_table.
>
> Just set the BO type to SG and then setting the SG table after
> creating it:
>> ret = amdgpu_bo_create(adev, attach->dmabuf->size, PAGE_SIZE,
>> AMDGPU_GEM_DOMAIN_CPU, 0, ttm_bo_type_sg,
>> resv, &bo);
>> if (ret)
>> goto error;
>> bo->tbo.sg = sg;
>> bo->tbo.ttm->sg = sg;
>
> Then validate the result into the GTT domain to actually use the SG
> table.
OK, I'll try implementing that before we merge your change.
>
> BTW: How do you create an SG table for the doorbell?
KFD manages the doorbell aperture and assigns one or two pages of
doorbells to each process. KFD knows the bus address of the doorbells
and passes it to amdgpu, which creates the SG like this:
static struct sg_table *create_doorbell_sg(uint64_t addr, uint32_t size)
{
struct sg_table *sg = kmalloc(sizeof(*sg), GFP_KERNEL);
if (!sg)
return NULL;
if (sg_alloc_table(sg, 1, GFP_KERNEL)) {
kfree(sg);
return NULL;
}
sg->sgl->dma_address = addr;
sg->sgl->length = size;
#ifdef CONFIG_NEED_SG_DMA_LENGTH
sg->sgl->dma_length = size;
#endif
return sg;
}
Regards,
Felix
>
> Regards,
> Christian.
>
>>
>> If you remove this now, I'll need to add it back in some form in a month
>> or two when I get to that part of upstreaming KFD.
>>
>> There may be other ways to implement this. Currently we need to create a
>> BO for anything we want to map into a GPUVM page table, because the
>> amdgpu_vm code is based around BOs. If there was a way to map physical
>> addresses into GPUVM without creating a buffer object, that would
>> work too.
>>
>> Regards,
>> Felix
>>
>>
>> On 2018-03-06 09:43 AM, Christian König wrote:
>>> Drop the "kernel" and sg parameter and give the BO type to create
>>> explicit to amdgpu_bo_create instead of figuring it out from the
>>> parameters.
>>>
>>> Signed-off-by: Christian König
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 +--
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 8 ++---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 7 ++--
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++--
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 46
>>> +++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 11 +++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 7 ++--
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 11 +++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++---
>>> 11 files changed, 58 insertions(+), 64 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index 292c7e72820c..b1116b773516 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -441,7 +441,7 @@ struct amdgpu_sa_bo {
>>> void amdgpu_gem_force_release(struct amdgpu_device *adev);
>>> int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned
>>> long size,
>>> int alignment, u32 initial_domain,
>>> - u64 flags, bool kernel,
>>> + u64 flags, enum ttm_bo_type type,
>>> struct reservation_object *resv,
>>> struct drm_gem_object **obj);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> index 450426dbed92..7f096ed6e83d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> @@ -215,8 +215,9 @@ int alloc_gtt_mem(struct kgd_dev *kgd, size_t size,
>>> if ((*mem) == NULL)
>>> return -ENOMEM;
>>> - r = amdgpu_bo_create(adev, size, PAGE_SIZE, true,
>>> AMDGPU_GEM_DOMAIN_GTT,
>>> - AMDGPU_GEM_CREATE_CPU_GTT_USWC, NULL, NULL,
>>> &(*mem)->bo);
>>> + r = amdgpu_bo_create(adev, size, PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
>>> + AMDGPU_GEM_CREATE_CPU_GTT_USWC, ttm_bo_type_kernel,
>>> + NULL, &(*mem)->bo);
>>> if (r) {
>>> dev_err(adev->dev,
>>> "failed to allocate BO for amdkfd (%d)\n", r);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
>>> index 2fb299afc12b..02b849be083b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
>>> @@ -80,8 +80,8 @@ stati
Re: [PATCH 6/6] drm/amdgpu: explicit give BO type to amdgpu_bo_create
Am 06.03.2018 um 22:29 schrieb Felix Kuehling:
NAK.
For KFD we need the ability to create a BO from an SG list that doesn't
come from another BO. We use this for mapping pages from the doorbell
aperture into GPUVM for GPU self-dispatch.
You can still do this, see amdgpu_gem_prime_import_sg_table.
Just set the BO type to SG and then setting the SG table after creating it:
ret = amdgpu_bo_create(adev, attach->dmabuf->size, PAGE_SIZE,
AMDGPU_GEM_DOMAIN_CPU, 0, ttm_bo_type_sg,
resv, &bo);
if (ret)
goto error;
bo->tbo.sg = sg;
bo->tbo.ttm->sg = sg;
Then validate the result into the GTT domain to actually use the SG table.
BTW: How do you create an SG table for the doorbell?
Regards,
Christian.
If you remove this now, I'll need to add it back in some form in a month
or two when I get to that part of upstreaming KFD.
There may be other ways to implement this. Currently we need to create a
BO for anything we want to map into a GPUVM page table, because the
amdgpu_vm code is based around BOs. If there was a way to map physical
addresses into GPUVM without creating a buffer object, that would work too.
Regards,
Felix
On 2018-03-06 09:43 AM, Christian König wrote:
Drop the "kernel" and sg parameter and give the BO type to create
explicit to amdgpu_bo_create instead of figuring it out from the
parameters.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c| 5 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 8 ++---
drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 7 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 46 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_object.h| 11 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 7 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 11 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 8 ++---
11 files changed, 58 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 292c7e72820c..b1116b773516 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -441,7 +441,7 @@ struct amdgpu_sa_bo {
void amdgpu_gem_force_release(struct amdgpu_device *adev);
int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
int alignment, u32 initial_domain,
-u64 flags, bool kernel,
+u64 flags, enum ttm_bo_type type,
struct reservation_object *resv,
struct drm_gem_object **obj);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 450426dbed92..7f096ed6e83d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -215,8 +215,9 @@ int alloc_gtt_mem(struct kgd_dev *kgd, size_t size,
if ((*mem) == NULL)
return -ENOMEM;
- r = amdgpu_bo_create(adev, size, PAGE_SIZE, true, AMDGPU_GEM_DOMAIN_GTT,
-AMDGPU_GEM_CREATE_CPU_GTT_USWC, NULL, NULL,
&(*mem)->bo);
+ r = amdgpu_bo_create(adev, size, PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
+AMDGPU_GEM_CREATE_CPU_GTT_USWC, ttm_bo_type_kernel,
+NULL, &(*mem)->bo);
if (r) {
dev_err(adev->dev,
"failed to allocate BO for amdkfd (%d)\n", r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
index 2fb299afc12b..02b849be083b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -80,8 +80,8 @@ static void amdgpu_benchmark_move(struct amdgpu_device *adev,
unsigned size,
int time;
n = AMDGPU_BENCHMARK_ITERATIONS;
- r = amdgpu_bo_create(adev, size, PAGE_SIZE, true, sdomain, 0, NULL,
-NULL, &sobj);
+ r = amdgpu_bo_create(adev, size, PAGE_SIZE,sdomain, 0,
+ttm_bo_type_kernel, NULL, &sobj);
if (r) {
goto out_cleanup;
}
@@ -93,8 +93,8 @@ static void amdgpu_benchmark_move(struct amdgpu_device *adev,
unsigned size,
if (r) {
goto out_cleanup;
}
- r = amdgpu_bo_create(adev, size, PAGE_SIZE, true, ddomain, 0, NULL,
-NULL, &dobj);
+ r = amdgpu_bo_create(adev, size, PAGE_SIZE, ddomain, 0,
+ttm_bo_type_kernel, NULL, &dobj);
if (r) {
goto out_cleanup;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
Re: [PATCH 6/6] drm/amdgpu: explicit give BO type to amdgpu_bo_create
NAK.
For KFD we need the ability to create a BO from an SG list that doesn't
come from another BO. We use this for mapping pages from the doorbell
aperture into GPUVM for GPU self-dispatch.
If you remove this now, I'll need to add it back in some form in a month
or two when I get to that part of upstreaming KFD.
There may be other ways to implement this. Currently we need to create a
BO for anything we want to map into a GPUVM page table, because the
amdgpu_vm code is based around BOs. If there was a way to map physical
addresses into GPUVM without creating a buffer object, that would work too.
Regards,
Felix
On 2018-03-06 09:43 AM, Christian König wrote:
> Drop the "kernel" and sg parameter and give the BO type to create
> explicit to amdgpu_bo_create instead of figuring it out from the
> parameters.
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c| 5 +--
> drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 8 ++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 7 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 46
> +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h| 11 +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 7 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 11 +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 8 ++---
> 11 files changed, 58 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 292c7e72820c..b1116b773516 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -441,7 +441,7 @@ struct amdgpu_sa_bo {
> void amdgpu_gem_force_release(struct amdgpu_device *adev);
> int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>int alignment, u32 initial_domain,
> - u64 flags, bool kernel,
> + u64 flags, enum ttm_bo_type type,
>struct reservation_object *resv,
>struct drm_gem_object **obj);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 450426dbed92..7f096ed6e83d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -215,8 +215,9 @@ int alloc_gtt_mem(struct kgd_dev *kgd, size_t size,
> if ((*mem) == NULL)
> return -ENOMEM;
>
> - r = amdgpu_bo_create(adev, size, PAGE_SIZE, true, AMDGPU_GEM_DOMAIN_GTT,
> - AMDGPU_GEM_CREATE_CPU_GTT_USWC, NULL, NULL,
> &(*mem)->bo);
> + r = amdgpu_bo_create(adev, size, PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
> + AMDGPU_GEM_CREATE_CPU_GTT_USWC, ttm_bo_type_kernel,
> + NULL, &(*mem)->bo);
> if (r) {
> dev_err(adev->dev,
> "failed to allocate BO for amdkfd (%d)\n", r);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> index 2fb299afc12b..02b849be083b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> @@ -80,8 +80,8 @@ static void amdgpu_benchmark_move(struct amdgpu_device
> *adev, unsigned size,
> int time;
>
> n = AMDGPU_BENCHMARK_ITERATIONS;
> - r = amdgpu_bo_create(adev, size, PAGE_SIZE, true, sdomain, 0, NULL,
> - NULL, &sobj);
> + r = amdgpu_bo_create(adev, size, PAGE_SIZE,sdomain, 0,
> + ttm_bo_type_kernel, NULL, &sobj);
> if (r) {
> goto out_cleanup;
> }
> @@ -93,8 +93,8 @@ static void amdgpu_benchmark_move(struct amdgpu_device
> *adev, unsigned size,
> if (r) {
> goto out_cleanup;
> }
> - r = amdgpu_bo_create(adev, size, PAGE_SIZE, true, ddomain, 0, NULL,
> - NULL, &dobj);
> + r = amdgpu_bo_create(adev, size, PAGE_SIZE, ddomain, 0,
> + ttm_bo_type_kernel, NULL, &dobj);
> if (r) {
> goto out_cleanup;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index dc8d9f3216fa..cf0f186c6092 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -113,11 +113,12 @@ int amdgpu_gart_table_vram_alloc(struct amdgpu_device
> *adev)
> int r;
>
> if (adev->gart.robj == NULL) {
> - r = amdgpu_bo_create(adev, adev->gart.table_size,
> - PAGE_SIZE, true, AMDGPU_GEM_DOMAIN_VRAM,
> + r = amdgpu_bo_create(adev, adev->gart.table_size,
