Re: [PATCH 11/11] drm/amdgpu: enable GTT PD/PT for raven

2018-08-23 Thread Christian König 

Am 23.08.2018 um 14:54 schrieb Huang Rui:

On Wed, Aug 22, 2018 at 11:44:04AM -0400, Andrey Grodzovsky wrote:


On 08/22/2018 11:05 AM, Christian König wrote:

Should work on Vega10 as well, but with an obvious performance hit.

Older APUs can be enabled as well, but will probably be more work.

Raven's VRAM is actually the system memory. May I know the benefit if we
switch the PD/PT BO from vram to gart?


We want to reduce VRAM usage as much as possible on APUs.

The end goal is that it should work with only 16MB (or was it 32MB?) 
stolen VRAM.


That is the recommended setting for newer OSes on that hardware.

Christian.




Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 ++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 928fdae0dab4..670a42729f88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -308,6 +308,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
list_move(_base->vm_status, >moved);
spin_unlock(>moved_lock);
} else {
+   amdgpu_ttm_alloc_gart(>tbo);

Looks like you forgot to check for return value here.


Yes, the same comment with me.

Thanks,
Ray


Andrey


list_move(_base->vm_status, >relocated);
}
}
@@ -396,6 +397,10 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
if (r)
goto error;
+   r = amdgpu_ttm_alloc_gart(>tbo);
+   if (r)
+   return r;
+
r = amdgpu_job_alloc_with_ib(adev, 64, );
if (r)
goto error;
@@ -461,7 +466,11 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
bp->size = amdgpu_vm_bo_size(adev, level);
bp->byte_align = AMDGPU_GPU_PAGE_SIZE;
bp->domain = AMDGPU_GEM_DOMAIN_VRAM;
-   bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
+   if (bp->size <= PAGE_SIZE && adev->asic_type == CHIP_RAVEN)
+   bp->domain |= AMDGPU_GEM_DOMAIN_GTT;
+   bp->domain = amdgpu_bo_get_preferred_pin_domain(adev, bp->domain);
+   bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
+   AMDGPU_GEM_CREATE_CPU_GTT_USWC;
if (vm->use_cpu_for_update)
bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
else

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 11/11] drm/amdgpu: enable GTT PD/PT for raven

2018-08-23 Thread Huang Rui 
On Wed, Aug 22, 2018 at 11:44:04AM -0400, Andrey Grodzovsky wrote:
> 
> 
> On 08/22/2018 11:05 AM, Christian König wrote:
> >Should work on Vega10 as well, but with an obvious performance hit.
> >
> >Older APUs can be enabled as well, but will probably be more work.

Raven's VRAM is actually the system memory. May I know the benefit if we
switch the PD/PT BO from vram to gart?

> >
> >Signed-off-by: Christian König 
> >---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 ++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> >b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >index 928fdae0dab4..670a42729f88 100644
> >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >@@ -308,6 +308,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device 
> >*adev, struct amdgpu_vm *vm,
> > list_move(_base->vm_status, >moved);
> > spin_unlock(>moved_lock);
> > } else {
> >+amdgpu_ttm_alloc_gart(>tbo);
> 
> Looks like you forgot to check for return value here.
> 

Yes, the same comment with me. 

Thanks,
Ray

> Andrey
> 
> > list_move(_base->vm_status, >relocated);
> > }
> > }
> >@@ -396,6 +397,10 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device 
> >*adev,
> > if (r)
> > goto error;
> >+r = amdgpu_ttm_alloc_gart(>tbo);
> >+if (r)
> >+return r;
> >+
> > r = amdgpu_job_alloc_with_ib(adev, 64, );
> > if (r)
> > goto error;
> >@@ -461,7 +466,11 @@ static void amdgpu_vm_bo_param(struct amdgpu_device 
> >*adev, struct amdgpu_vm *vm,
> > bp->size = amdgpu_vm_bo_size(adev, level);
> > bp->byte_align = AMDGPU_GPU_PAGE_SIZE;
> > bp->domain = AMDGPU_GEM_DOMAIN_VRAM;
> >-bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
> >+if (bp->size <= PAGE_SIZE && adev->asic_type == CHIP_RAVEN)
> >+bp->domain |= AMDGPU_GEM_DOMAIN_GTT;
> >+bp->domain = amdgpu_bo_get_preferred_pin_domain(adev, bp->domain);
> >+bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
> >+AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> > if (vm->use_cpu_for_update)
> > bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> > else
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 11/11] drm/amdgpu: enable GTT PD/PT for raven

2018-08-22 Thread Zhang, Jerry (Junwei) 

On 08/22/2018 11:05 PM, Christian König wrote:

Should work on Vega10 as well, but with an obvious performance hit.

Older APUs can be enabled as well, but will probably be more work.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 ++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 928fdae0dab4..670a42729f88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -308,6 +308,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
list_move(_base->vm_status, >moved);
spin_unlock(>moved_lock);
} else {
+   amdgpu_ttm_alloc_gart(>tbo);
list_move(_base->vm_status, >relocated);
}
}
@@ -396,6 +397,10 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
if (r)
goto error;

+   r = amdgpu_ttm_alloc_gart(>tbo);
+   if (r)
+   return r;
+
r = amdgpu_job_alloc_with_ib(adev, 64, );
if (r)
goto error;
@@ -461,7 +466,11 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
bp->size = amdgpu_vm_bo_size(adev, level);
bp->byte_align = AMDGPU_GPU_PAGE_SIZE;
bp->domain = AMDGPU_GEM_DOMAIN_VRAM;
-   bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
+   if (bp->size <= PAGE_SIZE && adev->asic_type == CHIP_RAVEN)


Do we need bp->size <= PAGE_SIZE?
Seems it's always less 12 bit for raven?

Regards,
Jerry


+   bp->domain |= AMDGPU_GEM_DOMAIN_GTT;
+   bp->domain = amdgpu_bo_get_preferred_pin_domain(adev, bp->domain);
+   bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
+   AMDGPU_GEM_CREATE_CPU_GTT_USWC;
if (vm->use_cpu_for_update)
bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
else


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 11/11] drm/amdgpu: enable GTT PD/PT for raven

2018-08-22 Thread Andrey Grodzovsky 



On 08/22/2018 11:05 AM, Christian König wrote:

Should work on Vega10 as well, but with an obvious performance hit.

Older APUs can be enabled as well, but will probably be more work.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 ++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 928fdae0dab4..670a42729f88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -308,6 +308,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
list_move(_base->vm_status, >moved);
spin_unlock(>moved_lock);
} else {
+   amdgpu_ttm_alloc_gart(>tbo);


Looks like you forgot to check for return value here.

Andrey


list_move(_base->vm_status, >relocated);
}
}
@@ -396,6 +397,10 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
if (r)
goto error;
  
+	r = amdgpu_ttm_alloc_gart(>tbo);

+   if (r)
+   return r;
+
r = amdgpu_job_alloc_with_ib(adev, 64, );
if (r)
goto error;
@@ -461,7 +466,11 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
bp->size = amdgpu_vm_bo_size(adev, level);
bp->byte_align = AMDGPU_GPU_PAGE_SIZE;
bp->domain = AMDGPU_GEM_DOMAIN_VRAM;
-   bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
+   if (bp->size <= PAGE_SIZE && adev->asic_type == CHIP_RAVEN)
+   bp->domain |= AMDGPU_GEM_DOMAIN_GTT;
+   bp->domain = amdgpu_bo_get_preferred_pin_domain(adev, bp->domain);
+   bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
+   AMDGPU_GEM_CREATE_CPU_GTT_USWC;
if (vm->use_cpu_for_update)
bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
else


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 11/11] drm/amdgpu: enable GTT PD/PT for raven

2018-08-22 Thread Christian König 
Should work on Vega10 as well, but with an obvious performance hit.

Older APUs can be enabled as well, but will probably be more work.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 928fdae0dab4..670a42729f88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -308,6 +308,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
list_move(_base->vm_status, >moved);
spin_unlock(>moved_lock);
} else {
+   amdgpu_ttm_alloc_gart(>tbo);
list_move(_base->vm_status, >relocated);
}
}
@@ -396,6 +397,10 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
if (r)
goto error;
 
+   r = amdgpu_ttm_alloc_gart(>tbo);
+   if (r)
+   return r;
+
r = amdgpu_job_alloc_with_ib(adev, 64, );
if (r)
goto error;
@@ -461,7 +466,11 @@ static void amdgpu_vm_bo_param(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
bp->size = amdgpu_vm_bo_size(adev, level);
bp->byte_align = AMDGPU_GPU_PAGE_SIZE;
bp->domain = AMDGPU_GEM_DOMAIN_VRAM;
-   bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
+   if (bp->size <= PAGE_SIZE && adev->asic_type == CHIP_RAVEN)
+   bp->domain |= AMDGPU_GEM_DOMAIN_GTT;
+   bp->domain = amdgpu_bo_get_preferred_pin_domain(adev, bp->domain);
+   bp->flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
+   AMDGPU_GEM_CREATE_CPU_GTT_USWC;
if (vm->use_cpu_for_update)
bp->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
else
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx