[PATCH 2/2] drm/amdgpu: use amdgpu_bo_param for amdgpu_bo_create v2

[Chunming Zhou]

After that, we can easily add new parameter when need.

v2:
a) rebase.
b) Initialize struct amdgpu_bo_param, future new
member could only be used in some one case, but all member
should have its own initial value.

Change-Id: I6e80039c3801f163129ecc605d931483fdbc91db
Signed-off-by: Chunming Zhou 
Reviewed-by: Huang Rui  (v1)
Reviewed-by: Christian König  (v1)
Cc: christian.koe...@amd.com
Cc: felix.kuehl...@amd.com
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   | 12 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 11 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c| 15 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 17 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c  | 11 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c   | 58 
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h   |  6 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c| 12 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 18 +---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 15 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 26 ---
 11 files changed, 130 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 4d36203ffb11..887702c59488 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -217,13 +217,19 @@ int alloc_gtt_mem(struct kgd_dev *kgd, size_t size,
 {
struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
struct amdgpu_bo *bo = NULL;
+   struct amdgpu_bo_param bp;
int r;
uint64_t gpu_addr_tmp = 0;
void *cpu_ptr_tmp = NULL;
 
-   r = amdgpu_bo_create(adev, size, PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
-AMDGPU_GEM_CREATE_CPU_GTT_USWC, ttm_bo_type_kernel,
-NULL, );
+   memset(, 0, sizeof(bp));
+   bp.size = size;
+   bp.byte_align = PAGE_SIZE;
+   bp.domain = AMDGPU_GEM_DOMAIN_GTT;
+   bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC;
+   bp.type = ttm_bo_type_kernel;
+   bp.resv = NULL;
+   r = amdgpu_bo_create(adev, , );
if (r) {
dev_err(adev->dev,
"failed to allocate BO for amdkfd (%d)\n", r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 1d6e1479da38..c1b0cdb401dc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1004,6 +1004,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
struct amdgpu_device *adev = get_amdgpu_device(kgd);
struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
struct amdgpu_bo *bo;
+   struct amdgpu_bo_param bp;
int byte_align;
u32 alloc_domain;
u64 alloc_flags;
@@ -1069,8 +1070,14 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
pr_debug("\tcreate BO VA 0x%llx size 0x%llx domain %s\n",
va, size, domain_string(alloc_domain));
 
-   ret = amdgpu_bo_create(adev, size, byte_align,
-   alloc_domain, alloc_flags, ttm_bo_type_device, 
NULL, );
+   memset(, 0, sizeof(bp));
+   bp.size = size;
+   bp.byte_align = byte_align;
+   bp.domain = alloc_domain;
+   bp.flags = alloc_flags;
+   bp.type = ttm_bo_type_device;
+   bp.resv = NULL;
+   ret = amdgpu_bo_create(adev, , );
if (ret) {
pr_debug("Failed to create BO on domain %s. ret %d\n",
domain_string(alloc_domain), ret);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
index 02b849be083b..19cfff31f2e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -75,13 +75,20 @@ static void amdgpu_benchmark_move(struct amdgpu_device 
*adev, unsigned size,
 {
struct amdgpu_bo *dobj = NULL;
struct amdgpu_bo *sobj = NULL;
+   struct amdgpu_bo_param bp;
uint64_t saddr, daddr;
int r, n;
int time;
 
+   memset(, 0, sizeof(bp));
+   bp.size = size;
+   bp.byte_align = PAGE_SIZE;
+   bp.domain = sdomain;
+   bp.flags = 0;
+   bp.type = ttm_bo_type_kernel;
+   bp.resv = NULL;
n = AMDGPU_BENCHMARK_ITERATIONS;
-   r = amdgpu_bo_create(adev, size, PAGE_SIZE,sdomain, 0,
-ttm_bo_type_kernel, NULL, );
+   r = amdgpu_bo_create(adev, , );
if (r) {
goto out_cleanup;
}
@@ -93,8 +100,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, ddomain, 0,
-

[PATCH 1/2] drm/amdgpu: add amdgpu_bo_param

[Chunming Zhou]

amdgpu_bo_create has too many parameters, and used in
too many places. Collect them to one structure.

Change-Id: Ib2aa98ee37a70f3cb0d61eef1d336e89187554d5
Signed-off-by: Chunming Zhou 
Reviewed-by: Huang Rui 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 75 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  9 
 2 files changed, 51 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 24f582c696cc..b33a7fdea7f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -341,27 +341,25 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device 
*adev,
return false;
 }
 
-static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size,
-  int byte_align, u32 domain,
-  u64 flags, enum ttm_bo_type type,
-  struct reservation_object *resv,
+static int amdgpu_bo_do_create(struct amdgpu_device *adev,
+  struct amdgpu_bo_param *bp,
   struct amdgpu_bo **bo_ptr)
 {
struct ttm_operation_ctx ctx = {
-   .interruptible = (type != ttm_bo_type_kernel),
+   .interruptible = (bp->type != ttm_bo_type_kernel),
.no_wait_gpu = false,
-   .resv = resv,
+   .resv = bp->resv,
.flags = TTM_OPT_FLAG_ALLOW_RES_EVICT
};
struct amdgpu_bo *bo;
-   unsigned long page_align;
+   unsigned long page_align, size = bp->size;
size_t acc_size;
int r;
 
-   page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;
+   page_align = roundup(bp->byte_align, PAGE_SIZE) >> PAGE_SHIFT;
size = ALIGN(size, PAGE_SIZE);
 
-   if (!amdgpu_bo_validate_size(adev, size, domain))
+   if (!amdgpu_bo_validate_size(adev, size, bp->domain))
return -ENOMEM;
 
*bo_ptr = NULL;
@@ -375,18 +373,18 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
*adev, unsigned long size,
drm_gem_private_object_init(adev->ddev, >gem_base, size);
INIT_LIST_HEAD(>shadow_list);
INIT_LIST_HEAD(>va);
-   bo->preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
-AMDGPU_GEM_DOMAIN_GTT |
-AMDGPU_GEM_DOMAIN_CPU |
-AMDGPU_GEM_DOMAIN_GDS |
-AMDGPU_GEM_DOMAIN_GWS |
-AMDGPU_GEM_DOMAIN_OA);
+   bo->preferred_domains = bp->domain & (AMDGPU_GEM_DOMAIN_VRAM |
+ AMDGPU_GEM_DOMAIN_GTT |
+ AMDGPU_GEM_DOMAIN_CPU |
+ AMDGPU_GEM_DOMAIN_GDS |
+ AMDGPU_GEM_DOMAIN_GWS |
+ AMDGPU_GEM_DOMAIN_OA);
bo->allowed_domains = bo->preferred_domains;
-   if (type != ttm_bo_type_kernel &&
+   if (bp->type != ttm_bo_type_kernel &&
bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
 
-   bo->flags = flags;
+   bo->flags = bp->flags;
 
 #ifdef CONFIG_X86_32
/* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
@@ -417,11 +415,11 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
*adev, unsigned long size,
 #endif
 
bo->tbo.bdev = >mman.bdev;
-   amdgpu_ttm_placement_from_domain(bo, domain);
+   amdgpu_ttm_placement_from_domain(bo, bp->domain);
 
-   r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
+   r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp->type,
 >placement, page_align, , acc_size,
-NULL, resv, _ttm_bo_destroy);
+NULL, bp->resv, _ttm_bo_destroy);
if (unlikely(r != 0))
return r;
 
@@ -433,10 +431,10 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
*adev, unsigned long size,
else
amdgpu_cs_report_moved_bytes(adev, ctx.bytes_moved, 0);
 
-   if (type == ttm_bo_type_kernel)
+   if (bp->type == ttm_bo_type_kernel)
bo->tbo.priority = 1;
 
-   if (flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
+   if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
bo->tbo.mem.placement & TTM_PL_FLAG_VRAM) {
struct dma_fence *fence;
 
@@ -449,20 +447,20 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
*adev, unsigned long size,
bo->tbo.moving = dma_fence_get(fence);
dma_fence_put(fence);
}
- 

Re: [PATCH 2/2] drm/amdgpu: use amdgpu_bo_parm for amdgpu_bo_create

[Huang Rui]

On Mon, Apr 16, 2018 at 08:20:59PM +0800, Koenig, Christian wrote:
> Am 16.04.2018 um 13:43 schrieb Huang Rui:
> > On Mon, Apr 16, 2018 at 07:13:05PM +0800, Chunming Zhou wrote:
> >> after that, we can easily add new parameter when need.
> >>
> >> Change-Id: I6e80039c3801f163129ecc605d931483fdbc91db
> >> Signed-off-by: Chunming Zhou 
> >> Cc: christian.koe...@amd.com
> >> Cc: felix.kuehl...@amd.com
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   | 12 ++--
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 10 +--
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c| 15 +++---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 16 ++
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c  | 11 +--
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c   | 38 
> >> +++-
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h   |  6 ++--
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c| 12 ++--
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 16 ++
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 15 ++
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 24 ++-
> >>   11 files changed, 114 insertions(+), 61 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> >> index 4d36203ffb11..f90405e572f9 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> >> @@ -217,13 +217,19 @@ int alloc_gtt_mem(struct kgd_dev *kgd, size_t size,
> >>   {
> >>struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
> >>struct amdgpu_bo *bo = NULL;
> >> +  struct amdgpu_bo_param bp = {
> >> +  .size = size,
> >> +  .byte_align = PAGE_SIZE,
> >> +  .domain = AMDGPU_GEM_DOMAIN_GTT,
> >> +  .flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC,
> >> +  .type = ttm_bo_type_kernel,
> >> +  .resv = NULL
> >> +  };
> >>int r;
> >>uint64_t gpu_addr_tmp = 0;
> >>void *cpu_ptr_tmp = NULL;
> >>   
> >> -  r = amdgpu_bo_create(adev, size, PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
> >> -   AMDGPU_GEM_CREATE_CPU_GTT_USWC, ttm_bo_type_kernel,
> >> -   NULL, );
> >> +  r = amdgpu_bo_create(adev, , );
> >>if (r) {
> >>dev_err(adev->dev,
> >>"failed to allocate BO for amdkfd (%d)\n", r);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> >> index 1d6e1479da38..b7bd24c35b25 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> >> @@ -1004,6 +1004,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> >>struct amdgpu_device *adev = get_amdgpu_device(kgd);
> >>struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
> >>struct amdgpu_bo *bo;
> >> +  struct amdgpu_bo_param bp;
> >>int byte_align;
> >>u32 alloc_domain;
> >>u64 alloc_flags;
> >> @@ -1069,8 +1070,13 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> >>pr_debug("\tcreate BO VA 0x%llx size 0x%llx domain %s\n",
> >>va, size, domain_string(alloc_domain));
> >>   
> >> -  ret = amdgpu_bo_create(adev, size, byte_align,
> >> -  alloc_domain, alloc_flags, ttm_bo_type_device, 
> >> NULL, );
> >> +  bp.size = size;
> >> +  bp.byte_align = byte_align;
> >> +  bp.domain = alloc_domain;
> >> +  bp.flags = alloc_flags;
> >> +  bp.type = ttm_bo_type_device;
> >> +  bp.resv = NULL;
> >> +  ret = amdgpu_bo_create(adev, , );
> >>if (ret) {
> >>pr_debug("Failed to create BO on domain %s. ret %d\n",
> >>domain_string(alloc_domain), ret);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c 
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> >> index 02b849be083b..96bdb454bdf6 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> >> @@ -75,13 +75,20 @@ static void amdgpu_benchmark_move(struct amdgpu_device 
> >> *adev, unsigned size,
> >>   {
> >>struct amdgpu_bo *dobj = NULL;
> >>struct amdgpu_bo *sobj = NULL;
> >> +  struct amdgpu_bo_param bp = {
> >> +  .size = size,
> >> +  .byte_align = PAGE_SIZE,
> >> +  .domain = sdomain,
> >> +  .flags = 0,
> >> +  .type = ttm_bo_type_kernel,
> >> +  .resv = NULL
> >> +  };
> >>uint64_t saddr, daddr;
> >>int r, n;
> >>int time;
> >>   
> >>n = AMDGPU_BENCHMARK_ITERATIONS;
> >> -  r = amdgpu_bo_create(adev, size, PAGE_SIZE,sdomain, 0,
> >> -   ttm_bo_type_kernel, NULL, );
> >> +  r = amdgpu_bo_create(adev, , );
> >>if (r) {
> >>goto out_cleanup;
> >>}
> >> @@ -93,8 +100,8 @@ static void amdgpu_benchmark_move(struct 

Re: Gamma problem after 4096 LUT entries change

[Môshe van der Sterre]

On 04/16/2018 04:07 PM, Harry Wentland wrote:

On 2018-04-15 05:37 AM, Môshe van der Sterre wrote:

Hello Leo Li,

The current mainline has a gamma regression for me, running wayland on a Ryzen 
5 2400G (integrated gpu). Colors are much too dark after the following commit:
086247a4b2fb "drm/amd/display: Use 4096 lut entries"

 From this commit alone I cannot deduce how to properly address the problem. 
Reverting restores the correct colors. Unfortunately, I have no other amdgpu 
hardware to test if this is specific to the integrated gpu or not.

I'll gladly test possible fixes or debug a little further if you can point me 
in the right direction.


Can you give attached patch a spin. We'll probably merge it or a variant of it 
soon.

Harry

Thank you Harry. The patch works on my system.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: Don't program bypass on linear regamma LUT

[Leo Li]

On 2018-04-16 03:39 PM, Harry Wentland wrote:

Even though this is required for degamma since DCE HW only supports a
couple predefined LUTs we can just program the LUT directly for regamma.

This fixes dark screens which occurs when we program regamma to bypass
while degamma is using srgb LUT.

Signed-off-by: Harry Wentland 


Thanks Harry,

Reviewed-by: Leo Li 


---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 7 ---
  1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index ef5fad8c5aac..e3d90e918d1b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -139,13 +139,6 @@ int amdgpu_dm_set_regamma_lut(struct dm_crtc_state *crtc)
lut = (struct drm_color_lut *)blob->data;
lut_size = blob->length / sizeof(struct drm_color_lut);
  
-	if (__is_lut_linear(lut, lut_size)) {

-   /* Set to bypass if lut is set to linear */
-   stream->out_transfer_func->type = TF_TYPE_BYPASS;
-   stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
-   return 0;
-   }
-
gamma = dc_create_gamma();
if (!gamma)
return -ENOMEM;


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

[PATCH] drm/amd/display: Don't program bypass on linear regamma LUT

[Harry Wentland]

Even though this is required for degamma since DCE HW only supports a
couple predefined LUTs we can just program the LUT directly for regamma.

This fixes dark screens which occurs when we program regamma to bypass
while degamma is using srgb LUT.

Signed-off-by: Harry Wentland 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index ef5fad8c5aac..e3d90e918d1b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -139,13 +139,6 @@ int amdgpu_dm_set_regamma_lut(struct dm_crtc_state *crtc)
lut = (struct drm_color_lut *)blob->data;
lut_size = blob->length / sizeof(struct drm_color_lut);
 
-   if (__is_lut_linear(lut, lut_size)) {
-   /* Set to bypass if lut is set to linear */
-   stream->out_transfer_func->type = TF_TYPE_BYPASS;
-   stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
-   return 0;
-   }
-
gamma = dc_create_gamma();
if (!gamma)
return -ENOMEM;
-- 
2.17.0

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

Re: [PATCH 1/2] drm/amd/pp: Adding set_watermarks_for_clocks_ranges for SMU10

[Deucher, Alexander]

Series is:

Acked-by: Alex Deucher 


From: amd-gfx  on behalf of 
mikita.lip...@amd.com 
Sent: Monday, April 16, 2018 11:22:46 AM
To: amd-gfx@lists.freedesktop.org; Wentland, Harry; Zhu, Rex; Deucher, Alexander
Cc: Lipski, Mikita
Subject: [PATCH 1/2] drm/amd/pp: Adding set_watermarks_for_clocks_ranges for 
SMU10

From: Mikita Lipski 

The function is never implemented for raven on linux.
It follows similair implementation as on windows.

SMU still needs to notify SMC and copy WM table, which is added
here. But on other Asics such as Vega this step is not implemented.

Signed-off-by: Mikita Lipski 
Reviewed-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 13 +
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.h |  1 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
index 6ba3b1f..b712d16 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
@@ -992,6 +992,18 @@ static int smu10_read_sensor(struct pp_hwmgr *hwmgr, int 
idx,
 return ret;
 }

+static int smu10_set_watermarks_for_clocks_ranges(struct pp_hwmgr *hwmgr,
+   struct pp_wm_sets_with_clock_ranges_soc15 *wm_with_clock_ranges)
+{
+   struct smu10_hwmgr *data = hwmgr->backend;
+   Watermarks_t *table = &(data->water_marks_table);
+   int result = 0;
+
+   smu_set_watermarks_for_clocks_ranges(table,wm_with_clock_ranges);
+   smum_smc_table_manager(hwmgr, (uint8_t *)table, 
(uint16_t)SMU10_WMTABLE, false);
+   data->water_marks_exist = true;
+   return result;
+}
 static int smu10_set_mmhub_powergating_by_smu(struct pp_hwmgr *hwmgr)
 {
 return smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PowerGateMmHub);
@@ -1021,6 +1033,7 @@ static const struct pp_hwmgr_func smu10_hwmgr_funcs = {
 .get_current_shallow_sleep_clocks = 
smu10_get_current_shallow_sleep_clocks,
 .get_clock_by_type_with_latency = smu10_get_clock_by_type_with_latency,
 .get_clock_by_type_with_voltage = smu10_get_clock_by_type_with_voltage,
+   .set_watermarks_for_clocks_ranges = 
smu10_set_watermarks_for_clocks_ranges,
 .get_max_high_clocks = smu10_get_max_high_clocks,
 .read_sensor = smu10_read_sensor,
 .set_active_display_count = smu10_set_active_display_count,
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.h 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.h
index 175c3a5..f68b218 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.h
@@ -290,6 +290,7 @@ struct smu10_hwmgr {
 bool   vcn_dpg_mode;

 bool   gfx_off_controled_by_driver;
+   bool   water_marks_exist;
 Watermarks_t  water_marks_table;
 struct smu10_clock_voltage_information   clock_vol_info;
 DpmClocks_t   clock_table;
--
2.7.4

___
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

[PATCH 1/2] drm/amd/pp: Adding set_watermarks_for_clocks_ranges for SMU10

[mikita.lipski]

From: Mikita Lipski 

The function is never implemented for raven on linux.
It follows similair implementation as on windows.

SMU still needs to notify SMC and copy WM table, which is added
here. But on other Asics such as Vega this step is not implemented.

Signed-off-by: Mikita Lipski 
Reviewed-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 13 +
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.h |  1 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
index 6ba3b1f..b712d16 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
@@ -992,6 +992,18 @@ static int smu10_read_sensor(struct pp_hwmgr *hwmgr, int 
idx,
return ret;
 }
 
+static int smu10_set_watermarks_for_clocks_ranges(struct pp_hwmgr *hwmgr,
+   struct pp_wm_sets_with_clock_ranges_soc15 *wm_with_clock_ranges)
+{
+   struct smu10_hwmgr *data = hwmgr->backend;
+   Watermarks_t *table = &(data->water_marks_table);
+   int result = 0;
+
+   smu_set_watermarks_for_clocks_ranges(table,wm_with_clock_ranges);
+   smum_smc_table_manager(hwmgr, (uint8_t *)table, 
(uint16_t)SMU10_WMTABLE, false);
+   data->water_marks_exist = true;
+   return result;
+}
 static int smu10_set_mmhub_powergating_by_smu(struct pp_hwmgr *hwmgr)
 {
return smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PowerGateMmHub);
@@ -1021,6 +1033,7 @@ static const struct pp_hwmgr_func smu10_hwmgr_funcs = {
.get_current_shallow_sleep_clocks = 
smu10_get_current_shallow_sleep_clocks,
.get_clock_by_type_with_latency = smu10_get_clock_by_type_with_latency,
.get_clock_by_type_with_voltage = smu10_get_clock_by_type_with_voltage,
+   .set_watermarks_for_clocks_ranges = 
smu10_set_watermarks_for_clocks_ranges,
.get_max_high_clocks = smu10_get_max_high_clocks,
.read_sensor = smu10_read_sensor,
.set_active_display_count = smu10_set_active_display_count,
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.h 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.h
index 175c3a5..f68b218 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.h
@@ -290,6 +290,7 @@ struct smu10_hwmgr {
bool   vcn_dpg_mode;
 
bool   gfx_off_controled_by_driver;
+   bool   water_marks_exist;
Watermarks_t  water_marks_table;
struct smu10_clock_voltage_information   clock_vol_info;
DpmClocks_t   clock_table;
-- 
2.7.4

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

[PATCH 2/2] drm/amd/pp: Adding a function to store cc6 data in SMU10

[mikita.lipski]

From: Mikita Lipski 

Filling the smu10_store_cc6_data based on the implementation
of Windows Powerplay.

There is an uncertainty with one of the parameters passed to the function
pstate_switch_disable - is not a part of smu10 private data structure.
So in the function its just ignored.

Signed-off-by: Mikita Lipski 
Reviewed-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
index b712d16..0f25226 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
@@ -699,6 +699,16 @@ static int smu10_set_cpu_power_state(struct pp_hwmgr 
*hwmgr)
 static int smu10_store_cc6_data(struct pp_hwmgr *hwmgr, uint32_t 
separation_time,
bool cc6_disable, bool pstate_disable, bool 
pstate_switch_disable)
 {
+   struct smu10_hwmgr *data = (struct smu10_hwmgr *)(hwmgr->backend);
+
+   if (separation_time != data->separation_time ||
+   cc6_disable != data->cc6_disable ||
+   pstate_disable != data->pstate_disable) {
+   data->separation_time = separation_time;
+   data->cc6_disable = cc6_disable;
+   data->pstate_disable = pstate_disable;
+   data->cc6_setting_changed = true;
+   }
return 0;
 }
 
-- 
2.7.4

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

Re: Gamma problem after 4096 LUT entries change

[Harry Wentland]

On 2018-04-15 05:37 AM, Môshe van der Sterre wrote:
> Hello Leo Li,
> 
> The current mainline has a gamma regression for me, running wayland on a 
> Ryzen 5 2400G (integrated gpu). Colors are much too dark after the following 
> commit:
> 086247a4b2fb "drm/amd/display: Use 4096 lut entries"
> 
> From this commit alone I cannot deduce how to properly address the problem. 
> Reverting restores the correct colors. Unfortunately, I have no other amdgpu 
> hardware to test if this is specific to the integrated gpu or not.
> 
> I'll gladly test possible fixes or debug a little further if you can point me 
> in the right direction.
> 

Can you give attached patch a spin. We'll probably merge it or a variant of it 
soon.

Harry

> Thanks,
> Môshe van der Sterre
> 
> 
>From 9adc614f258ba021aa70eeed92ecb0613406cf09 Mon Sep 17 00:00:00 2001
From: Harry Wentland 
Date: Thu, 12 Apr 2018 16:37:09 -0400
Subject: [PATCH] drm/amd/display: Don't program bypass on linear regamma LUT

Even though this is required for degamma since DCE HW only supports a
couple predefined LUTs we can just program the LUT directly.

This fixes dark screens which occurs when we program regamma to bypass
while degamma is using srgb LUT.

Signed-off-by: Harry Wentland 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c  | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index ef5fad8c5aac..59e993ad1209 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -133,19 +133,15 @@ int amdgpu_dm_set_regamma_lut(struct dm_crtc_state *crtc)
 		/* By default, use the SRGB predefined curve.*/
 		stream->out_transfer_func->type = TF_TYPE_PREDEFINED;
 		stream->out_transfer_func->tf = TRANSFER_FUNCTION_SRGB;
+
+		DRM_DEBUG_KMS("Using pre-defined regamma\n");
+
 		return 0;
 	}
 
 	lut = (struct drm_color_lut *)blob->data;
 	lut_size = blob->length / sizeof(struct drm_color_lut);
 
-	if (__is_lut_linear(lut, lut_size)) {
-		/* Set to bypass if lut is set to linear */
-		stream->out_transfer_func->type = TF_TYPE_BYPASS;
-		stream->out_transfer_func->tf = TRANSFER_FUNCTION_LINEAR;
-		return 0;
-	}
-
 	gamma = dc_create_gamma();
 	if (!gamma)
 		return -ENOMEM;
@@ -161,6 +157,8 @@ int amdgpu_dm_set_regamma_lut(struct dm_crtc_state *crtc)
 		return -EINVAL;
 	}
 
+	DRM_DEBUG_KMS("Using regamma with %d entries\n", lut_size);
+
 	/* Convert drm_lut into dc_gamma */
 	__drm_lut_to_dc_gamma(lut, gamma, gamma->type == GAMMA_RGB_256);
 
-- 
2.17.0

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

Re: [PATCH 2/2] Revert "drm/amd/display: disable CRTCs with NULL FB on their primary plane (V2)"

[Harry Wentland]

On 2018-04-13 03:15 AM, Michel Dänzer wrote:
> On 2018-04-13 04:55 AM, S, Shirish wrote:
>> Hi Harry, Alex,
>>
>> The solution given while reviewing my patch was that "DC should support 
>> enabling a CRTC without a framebuffer."
> 
> I think it's weird that an enabled CRTC would end up having a NULL FB
> during normal operation in the first place. Any idea how that happens?
> 

I think we've only seen that with ChromeOS and in one case on Ubuntu. If I 
remember right it was when running GDM with Wayland and logging into X. I 
imagine our DDX driver never pulls this scenario on us.

Technically this would be a reasonable case for our HW, i.e. we could keep pipe 
running and blanked and just not scan-out a FB.

> 
>> Since the revert is a temporary workaround to address issue at hand and 
>> considering the bigger regression it will cause on ChromeOS(explained below),
>> I would strongly recommend that the revert should not be mainlined (to linus 
>> tree), until a proper fix for both the issues i.e., flickering and BUG hit 
>> on atomic is found.
>>
>> For the sake of everyone's understanding in the list below is a brief 
>> background.
>>
>> Mainline patch from intel folks, "846c7df drm/atomic: Try to preserve the 
>> crtc enabled state in drm_atomic_remove_fb, v2." 
>> introduces a slight behavioral change to rmfb. Instead of disabling a crtc 
>> when the primary plane is disabled, it now preserves it.
>>
>> This change leads to BUG hit while performing atomic commit on amd driver 
>> leading to reboot/system instability on ChromeOS which has enabled
>> drm atomic way of rendering, I also remember it causing some issue on other 
>> OS as well.
> 
> Can you provide more information about "some issue on other OS"?
> Otherwise I'm afraid this is a no-brainer, since nobody's using ChromeOS
> with an AMD GPU in the wild, whereas many people have been running into
> issues with these commits in the wild, especially since they're in the
> 4.16 release.
> 

I tend to agree with Michel here, there's been quite the fallout from that 
patch for people's daily drivers.

Harry

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

Re: [PATCH 4/8] dma-buf: add peer2peer flag

[Daniel Vetter]

On Mon, Apr 16, 2018 at 2:39 PM, Christoph Hellwig  wrote:
> On Tue, Apr 03, 2018 at 08:08:32PM +0200, Daniel Vetter wrote:
>> I did not mean you should dma_map_sg/page. I just meant that using
>> dma_map_resource to fill only the dma address part of the sg table seems
>> perfectly sufficient.
>
> But that is not how the interface work, especially facing sg_dma_len.
>
>> Assuming you get an sg table that's been mapping by calling dma_map_sg was
>> always a bit a case of bending the abstraction to avoid typing code. The
>> only thing an importer ever should have done is look at the dma addresses
>> in that sg table, nothing else.
>
> The scatterlist is not a very good abstraction unfortunately, but it
> it is spread all over the kernel.  And we do expect that anyone who
> gets passed a scatterlist can use sg_page() or sg_virt() (which calls
> sg_page()) on it.  Your changes would break that, and will cause major
> trouble because of that.
>
> If you want to expose p2p memory returned from dma_map_resource in
> dmabuf do not use scatterlists for this please, but with a new interface
> that explicitly passes a virtual address, a dma address and a length
> and make it very clear that virt_to_page will not work on the virtual
> address.

We've broken that assumption in i915 years ago. Not struct page backed
gpu memory is very real.

Of course we'll never feed such a strange sg table to a driver which
doesn't understand it, but allowing sg_page == NULL works perfectly
fine. At least for gpu drivers.

If that's not acceptable then I guess we could go over the entire tree
and frob all the gpu related code to switch over to a new struct
sg_table_might_not_be_struct_page_backed, including all the other
functions we added over the past few years to iterate over sg tables.
But seems slightly silly, given that sg tables seem to do exactly what
we need.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/8] dma-buf: add peer2peer flag

[Christoph Hellwig]

On Tue, Apr 03, 2018 at 08:08:32PM +0200, Daniel Vetter wrote:
> I did not mean you should dma_map_sg/page. I just meant that using
> dma_map_resource to fill only the dma address part of the sg table seems
> perfectly sufficient.

But that is not how the interface work, especially facing sg_dma_len.

> Assuming you get an sg table that's been mapping by calling dma_map_sg was
> always a bit a case of bending the abstraction to avoid typing code. The
> only thing an importer ever should have done is look at the dma addresses
> in that sg table, nothing else.

The scatterlist is not a very good abstraction unfortunately, but it
it is spread all over the kernel.  And we do expect that anyone who
gets passed a scatterlist can use sg_page() or sg_virt() (which calls
sg_page()) on it.  Your changes would break that, and will cause major
trouble because of that.

If you want to expose p2p memory returned from dma_map_resource in
dmabuf do not use scatterlists for this please, but with a new interface
that explicitly passes a virtual address, a dma address and a length
and make it very clear that virt_to_page will not work on the virtual
address.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: add amdgpu_bo_param

[Christian König]

Am 16.04.2018 um 13:13 schrieb Chunming Zhou:

Change-Id: Ib2aa98ee37a70f3cb0d61eef1d336e89187554d5
Signed-off-by: Chunming Zhou 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 81 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  9 
  2 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index a160ef0332d6..b557b63bb648 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -341,28 +341,26 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device 
*adev,
return false;
  }
  
-static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size,

-  int byte_align, u32 domain,
-  u64 flags, enum ttm_bo_type type,
-  struct reservation_object *resv,
+static int amdgpu_bo_do_create(struct amdgpu_device *adev,
+  struct amdgpu_bo_param *bp,
   struct amdgpu_bo **bo_ptr)
  {
struct ttm_operation_ctx ctx = {
-   .interruptible = (type != ttm_bo_type_kernel),
+   .interruptible = (bp->type != ttm_bo_type_kernel),
.no_wait_gpu = false,
-   .resv = resv,
+   .resv = bp->resv,
.flags = TTM_OPT_FLAG_ALLOW_RES_EVICT
};
struct amdgpu_bo *bo;
-   unsigned long page_align;
+   unsigned long page_align, size = bp->size;
size_t acc_size;
u32 domains, preferred_domains, allowed_domains;
int r;
  
-	page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;

+   page_align = roundup(bp->byte_align, PAGE_SIZE) >> PAGE_SHIFT;
size = ALIGN(size, PAGE_SIZE);
  
-	if (!amdgpu_bo_validate_size(adev, size, domain))

+   if (!amdgpu_bo_validate_size(adev, size, bp->domain))
return -ENOMEM;
  
  	*bo_ptr = NULL;

@@ -370,14 +368,14 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
*adev, unsigned long size,
acc_size = ttm_bo_dma_acc_size(>mman.bdev, size,
   sizeof(struct amdgpu_bo));
  
-	preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |

- AMDGPU_GEM_DOMAIN_GTT |
- AMDGPU_GEM_DOMAIN_CPU |
- AMDGPU_GEM_DOMAIN_GDS |
- AMDGPU_GEM_DOMAIN_GWS |
- AMDGPU_GEM_DOMAIN_OA);
+   preferred_domains = bp->domain & (AMDGPU_GEM_DOMAIN_VRAM |
+ AMDGPU_GEM_DOMAIN_GTT |
+ AMDGPU_GEM_DOMAIN_CPU |
+ AMDGPU_GEM_DOMAIN_GDS |
+ AMDGPU_GEM_DOMAIN_GWS |
+ AMDGPU_GEM_DOMAIN_OA);
allowed_domains = preferred_domains;
-   if (type != ttm_bo_type_kernel &&
+   if (bp->type != ttm_bo_type_kernel &&
allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
domains = preferred_domains;
@@ -391,7 +389,7 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, 
unsigned long size,
bo->preferred_domains = preferred_domains;
bo->allowed_domains = allowed_domains;
  
-	bo->flags = flags;

+   bo->flags = bp->flags;
  
  #ifdef CONFIG_X86_32

/* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
@@ -423,13 +421,13 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
*adev, unsigned long size,
  
  	bo->tbo.bdev = >mman.bdev;

amdgpu_ttm_placement_from_domain(bo, domains);
-   r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
+   r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp->type,
 >placement, page_align, , acc_size,
-NULL, resv, _ttm_bo_destroy);
-   if (unlikely(r && r != -ERESTARTSYS) && type == ttm_bo_type_device &&
-   !(flags & AMDGPU_GEM_CREATE_NO_FALLBACK)) {
-   if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
-   flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
+NULL, bp->resv, _ttm_bo_destroy);
+   if (unlikely(r && r != -ERESTARTSYS) && bp->type == ttm_bo_type_device 
&&
+   !(bp->flags & AMDGPU_GEM_CREATE_NO_FALLBACK)) {
+   if (bp->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
+   bp->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
goto retry;
} else if (domains != allowed_domains) {
domains = allowed_domains;
@@ -447,10 +445,10 @@ 

Re: [PATCH 2/2] drm/amdgpu: use amdgpu_bo_parm for amdgpu_bo_create

[Christian König]

Am 16.04.2018 um 13:43 schrieb Huang Rui:

On Mon, Apr 16, 2018 at 07:13:05PM +0800, Chunming Zhou wrote:

after that, we can easily add new parameter when need.

Change-Id: I6e80039c3801f163129ecc605d931483fdbc91db
Signed-off-by: Chunming Zhou 
Cc: christian.koe...@amd.com
Cc: felix.kuehl...@amd.com
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   | 12 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 10 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c| 15 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 16 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c  | 11 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c   | 38 +++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h   |  6 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c| 12 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 16 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 15 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 24 ++-
  11 files changed, 114 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 4d36203ffb11..f90405e572f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -217,13 +217,19 @@ int alloc_gtt_mem(struct kgd_dev *kgd, size_t size,
  {
struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
struct amdgpu_bo *bo = NULL;
+   struct amdgpu_bo_param bp = {
+   .size = size,
+   .byte_align = PAGE_SIZE,
+   .domain = AMDGPU_GEM_DOMAIN_GTT,
+   .flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC,
+   .type = ttm_bo_type_kernel,
+   .resv = NULL
+   };
int r;
uint64_t gpu_addr_tmp = 0;
void *cpu_ptr_tmp = NULL;
  
-	r = amdgpu_bo_create(adev, size, PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,

-AMDGPU_GEM_CREATE_CPU_GTT_USWC, ttm_bo_type_kernel,
-NULL, );
+   r = amdgpu_bo_create(adev, , );
if (r) {
dev_err(adev->dev,
"failed to allocate BO for amdkfd (%d)\n", r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 1d6e1479da38..b7bd24c35b25 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1004,6 +1004,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
struct amdgpu_device *adev = get_amdgpu_device(kgd);
struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
struct amdgpu_bo *bo;
+   struct amdgpu_bo_param bp;
int byte_align;
u32 alloc_domain;
u64 alloc_flags;
@@ -1069,8 +1070,13 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
pr_debug("\tcreate BO VA 0x%llx size 0x%llx domain %s\n",
va, size, domain_string(alloc_domain));
  
-	ret = amdgpu_bo_create(adev, size, byte_align,

-   alloc_domain, alloc_flags, ttm_bo_type_device, 
NULL, );
+   bp.size = size;
+   bp.byte_align = byte_align;
+   bp.domain = alloc_domain;
+   bp.flags = alloc_flags;
+   bp.type = ttm_bo_type_device;
+   bp.resv = NULL;
+   ret = amdgpu_bo_create(adev, , );
if (ret) {
pr_debug("Failed to create BO on domain %s. ret %d\n",
domain_string(alloc_domain), ret);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
index 02b849be083b..96bdb454bdf6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -75,13 +75,20 @@ static void amdgpu_benchmark_move(struct amdgpu_device 
*adev, unsigned size,
  {
struct amdgpu_bo *dobj = NULL;
struct amdgpu_bo *sobj = NULL;
+   struct amdgpu_bo_param bp = {
+   .size = size,
+   .byte_align = PAGE_SIZE,
+   .domain = sdomain,
+   .flags = 0,
+   .type = ttm_bo_type_kernel,
+   .resv = NULL
+   };
uint64_t saddr, daddr;
int r, n;
int time;
  
  	n = AMDGPU_BENCHMARK_ITERATIONS;

-   r = amdgpu_bo_create(adev, size, PAGE_SIZE,sdomain, 0,
-ttm_bo_type_kernel, NULL, );
+   r = amdgpu_bo_create(adev, , );
if (r) {
goto out_cleanup;
}
@@ -93,8 +100,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, ddomain, 0,
-ttm_bo_type_kernel, NULL, );
+   bp.domain = ddomain;
+   r = amdgpu_bo_create(adev, , );
if (r) {

Re: [PATCH 2/2] drm/amdgpu: use amdgpu_bo_parm for amdgpu_bo_create

[Huang Rui]

On Mon, Apr 16, 2018 at 07:13:05PM +0800, Chunming Zhou wrote:
> after that, we can easily add new parameter when need.
> 
> Change-Id: I6e80039c3801f163129ecc605d931483fdbc91db
> Signed-off-by: Chunming Zhou 
> Cc: christian.koe...@amd.com
> Cc: felix.kuehl...@amd.com
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   | 12 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 10 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c| 15 +++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 16 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c  | 11 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c   | 38 
> +++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h   |  6 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c| 12 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 16 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 15 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 24 ++-
>  11 files changed, 114 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 4d36203ffb11..f90405e572f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -217,13 +217,19 @@ int alloc_gtt_mem(struct kgd_dev *kgd, size_t size,
>  {
>   struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
>   struct amdgpu_bo *bo = NULL;
> + struct amdgpu_bo_param bp = {
> + .size = size,
> + .byte_align = PAGE_SIZE,
> + .domain = AMDGPU_GEM_DOMAIN_GTT,
> + .flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC,
> + .type = ttm_bo_type_kernel,
> + .resv = NULL
> + };
>   int r;
>   uint64_t gpu_addr_tmp = 0;
>   void *cpu_ptr_tmp = NULL;
>  
> - r = amdgpu_bo_create(adev, size, PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
> -  AMDGPU_GEM_CREATE_CPU_GTT_USWC, ttm_bo_type_kernel,
> -  NULL, );
> + r = amdgpu_bo_create(adev, , );
>   if (r) {
>   dev_err(adev->dev,
>   "failed to allocate BO for amdkfd (%d)\n", r);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 1d6e1479da38..b7bd24c35b25 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1004,6 +1004,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   struct amdgpu_device *adev = get_amdgpu_device(kgd);
>   struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
>   struct amdgpu_bo *bo;
> + struct amdgpu_bo_param bp;
>   int byte_align;
>   u32 alloc_domain;
>   u64 alloc_flags;
> @@ -1069,8 +1070,13 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
>   pr_debug("\tcreate BO VA 0x%llx size 0x%llx domain %s\n",
>   va, size, domain_string(alloc_domain));
>  
> - ret = amdgpu_bo_create(adev, size, byte_align,
> - alloc_domain, alloc_flags, ttm_bo_type_device, 
> NULL, );
> + bp.size = size;
> + bp.byte_align = byte_align;
> + bp.domain = alloc_domain;
> + bp.flags = alloc_flags;
> + bp.type = ttm_bo_type_device;
> + bp.resv = NULL;
> + ret = amdgpu_bo_create(adev, , );
>   if (ret) {
>   pr_debug("Failed to create BO on domain %s. ret %d\n",
>   domain_string(alloc_domain), ret);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> index 02b849be083b..96bdb454bdf6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> @@ -75,13 +75,20 @@ static void amdgpu_benchmark_move(struct amdgpu_device 
> *adev, unsigned size,
>  {
>   struct amdgpu_bo *dobj = NULL;
>   struct amdgpu_bo *sobj = NULL;
> + struct amdgpu_bo_param bp = {
> + .size = size,
> + .byte_align = PAGE_SIZE,
> + .domain = sdomain,
> + .flags = 0,
> + .type = ttm_bo_type_kernel,
> + .resv = NULL
> + };
>   uint64_t saddr, daddr;
>   int r, n;
>   int time;
>  
>   n = AMDGPU_BENCHMARK_ITERATIONS;
> - r = amdgpu_bo_create(adev, size, PAGE_SIZE,sdomain, 0,
> -  ttm_bo_type_kernel, NULL, );
> + r = amdgpu_bo_create(adev, , );
>   if (r) {
>   goto out_cleanup;
>   }
> @@ -93,8 +100,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, ddomain, 0,
> -  ttm_bo_type_kernel, NULL, );
> + bp.domain = ddomain;
> + r = 

Re: [PATCH 1/2] drm/amdgpu: add amdgpu_bo_param

[Huang Rui]

On Mon, Apr 16, 2018 at 07:13:04PM +0800, Chunming Zhou wrote:
> Change-Id: Ib2aa98ee37a70f3cb0d61eef1d336e89187554d5
> Signed-off-by: Chunming Zhou 

Reviewed-by: Huang Rui 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 81 
> +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  9 
>  2 files changed, 54 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index a160ef0332d6..b557b63bb648 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -341,28 +341,26 @@ static bool amdgpu_bo_validate_size(struct 
> amdgpu_device *adev,
>   return false;
>  }
>  
> -static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long 
> size,
> -int byte_align, u32 domain,
> -u64 flags, enum ttm_bo_type type,
> -struct reservation_object *resv,
> +static int amdgpu_bo_do_create(struct amdgpu_device *adev,
> +struct amdgpu_bo_param *bp,
>  struct amdgpu_bo **bo_ptr)
>  {
>   struct ttm_operation_ctx ctx = {
> - .interruptible = (type != ttm_bo_type_kernel),
> + .interruptible = (bp->type != ttm_bo_type_kernel),
>   .no_wait_gpu = false,
> - .resv = resv,
> + .resv = bp->resv,
>   .flags = TTM_OPT_FLAG_ALLOW_RES_EVICT
>   };
>   struct amdgpu_bo *bo;
> - unsigned long page_align;
> + unsigned long page_align, size = bp->size;
>   size_t acc_size;
>   u32 domains, preferred_domains, allowed_domains;
>   int r;
>  
> - page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;
> + page_align = roundup(bp->byte_align, PAGE_SIZE) >> PAGE_SHIFT;
>   size = ALIGN(size, PAGE_SIZE);
>  
> - if (!amdgpu_bo_validate_size(adev, size, domain))
> + if (!amdgpu_bo_validate_size(adev, size, bp->domain))
>   return -ENOMEM;
>  
>   *bo_ptr = NULL;
> @@ -370,14 +368,14 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
> *adev, unsigned long size,
>   acc_size = ttm_bo_dma_acc_size(>mman.bdev, size,
>  sizeof(struct amdgpu_bo));
>  
> - preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
> -   AMDGPU_GEM_DOMAIN_GTT |
> -   AMDGPU_GEM_DOMAIN_CPU |
> -   AMDGPU_GEM_DOMAIN_GDS |
> -   AMDGPU_GEM_DOMAIN_GWS |
> -   AMDGPU_GEM_DOMAIN_OA);
> + preferred_domains = bp->domain & (AMDGPU_GEM_DOMAIN_VRAM |
> +   AMDGPU_GEM_DOMAIN_GTT |
> +   AMDGPU_GEM_DOMAIN_CPU |
> +   AMDGPU_GEM_DOMAIN_GDS |
> +   AMDGPU_GEM_DOMAIN_GWS |
> +   AMDGPU_GEM_DOMAIN_OA);
>   allowed_domains = preferred_domains;
> - if (type != ttm_bo_type_kernel &&
> + if (bp->type != ttm_bo_type_kernel &&
>   allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
>   allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
>   domains = preferred_domains;
> @@ -391,7 +389,7 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
> *adev, unsigned long size,
>   bo->preferred_domains = preferred_domains;
>   bo->allowed_domains = allowed_domains;
>  
> - bo->flags = flags;
> + bo->flags = bp->flags;
>  
>  #ifdef CONFIG_X86_32
>   /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
> @@ -423,13 +421,13 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
> *adev, unsigned long size,
>  
>   bo->tbo.bdev = >mman.bdev;
>   amdgpu_ttm_placement_from_domain(bo, domains);
> - r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
> + r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp->type,
>>placement, page_align, , acc_size,
> -  NULL, resv, _ttm_bo_destroy);
> - if (unlikely(r && r != -ERESTARTSYS) && type == ttm_bo_type_device &&
> - !(flags & AMDGPU_GEM_CREATE_NO_FALLBACK)) {
> - if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
> - flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> +  NULL, bp->resv, _ttm_bo_destroy);
> + if (unlikely(r && r != -ERESTARTSYS) && bp->type == ttm_bo_type_device 
> &&
> + !(bp->flags & AMDGPU_GEM_CREATE_NO_FALLBACK)) {
> + if (bp->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
> + bp->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>   goto retry;
>   } else if (domains != 

[PATCH 1/2] drm/amdgpu: add amdgpu_bo_param

[Chunming Zhou]

Change-Id: Ib2aa98ee37a70f3cb0d61eef1d336e89187554d5
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 81 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  9 
 2 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index a160ef0332d6..b557b63bb648 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -341,28 +341,26 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device 
*adev,
return false;
 }
 
-static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size,
-  int byte_align, u32 domain,
-  u64 flags, enum ttm_bo_type type,
-  struct reservation_object *resv,
+static int amdgpu_bo_do_create(struct amdgpu_device *adev,
+  struct amdgpu_bo_param *bp,
   struct amdgpu_bo **bo_ptr)
 {
struct ttm_operation_ctx ctx = {
-   .interruptible = (type != ttm_bo_type_kernel),
+   .interruptible = (bp->type != ttm_bo_type_kernel),
.no_wait_gpu = false,
-   .resv = resv,
+   .resv = bp->resv,
.flags = TTM_OPT_FLAG_ALLOW_RES_EVICT
};
struct amdgpu_bo *bo;
-   unsigned long page_align;
+   unsigned long page_align, size = bp->size;
size_t acc_size;
u32 domains, preferred_domains, allowed_domains;
int r;
 
-   page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;
+   page_align = roundup(bp->byte_align, PAGE_SIZE) >> PAGE_SHIFT;
size = ALIGN(size, PAGE_SIZE);
 
-   if (!amdgpu_bo_validate_size(adev, size, domain))
+   if (!amdgpu_bo_validate_size(adev, size, bp->domain))
return -ENOMEM;
 
*bo_ptr = NULL;
@@ -370,14 +368,14 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
*adev, unsigned long size,
acc_size = ttm_bo_dma_acc_size(>mman.bdev, size,
   sizeof(struct amdgpu_bo));
 
-   preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
- AMDGPU_GEM_DOMAIN_GTT |
- AMDGPU_GEM_DOMAIN_CPU |
- AMDGPU_GEM_DOMAIN_GDS |
- AMDGPU_GEM_DOMAIN_GWS |
- AMDGPU_GEM_DOMAIN_OA);
+   preferred_domains = bp->domain & (AMDGPU_GEM_DOMAIN_VRAM |
+ AMDGPU_GEM_DOMAIN_GTT |
+ AMDGPU_GEM_DOMAIN_CPU |
+ AMDGPU_GEM_DOMAIN_GDS |
+ AMDGPU_GEM_DOMAIN_GWS |
+ AMDGPU_GEM_DOMAIN_OA);
allowed_domains = preferred_domains;
-   if (type != ttm_bo_type_kernel &&
+   if (bp->type != ttm_bo_type_kernel &&
allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
domains = preferred_domains;
@@ -391,7 +389,7 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, 
unsigned long size,
bo->preferred_domains = preferred_domains;
bo->allowed_domains = allowed_domains;
 
-   bo->flags = flags;
+   bo->flags = bp->flags;
 
 #ifdef CONFIG_X86_32
/* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
@@ -423,13 +421,13 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
*adev, unsigned long size,
 
bo->tbo.bdev = >mman.bdev;
amdgpu_ttm_placement_from_domain(bo, domains);
-   r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
+   r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, bp->type,
 >placement, page_align, , acc_size,
-NULL, resv, _ttm_bo_destroy);
-   if (unlikely(r && r != -ERESTARTSYS) && type == ttm_bo_type_device &&
-   !(flags & AMDGPU_GEM_CREATE_NO_FALLBACK)) {
-   if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
-   flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
+NULL, bp->resv, _ttm_bo_destroy);
+   if (unlikely(r && r != -ERESTARTSYS) && bp->type == ttm_bo_type_device 
&&
+   !(bp->flags & AMDGPU_GEM_CREATE_NO_FALLBACK)) {
+   if (bp->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
+   bp->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
goto retry;
} else if (domains != allowed_domains) {
domains = allowed_domains;
@@ -447,10 +445,10 @@ static int amdgpu_bo_do_create(struct amdgpu_device 
*adev, unsigned long size,
else
 

[PATCH 2/2] drm/amdgpu: use amdgpu_bo_parm for amdgpu_bo_create

[Chunming Zhou]

after that, we can easily add new parameter when need.

Change-Id: I6e80039c3801f163129ecc605d931483fdbc91db
Signed-off-by: Chunming Zhou 
Cc: christian.koe...@amd.com
Cc: felix.kuehl...@amd.com
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   | 12 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 10 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c| 15 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 16 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c  | 11 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c   | 38 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h   |  6 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c| 12 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 16 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 15 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 24 ++-
 11 files changed, 114 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 4d36203ffb11..f90405e572f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -217,13 +217,19 @@ int alloc_gtt_mem(struct kgd_dev *kgd, size_t size,
 {
struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
struct amdgpu_bo *bo = NULL;
+   struct amdgpu_bo_param bp = {
+   .size = size,
+   .byte_align = PAGE_SIZE,
+   .domain = AMDGPU_GEM_DOMAIN_GTT,
+   .flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC,
+   .type = ttm_bo_type_kernel,
+   .resv = NULL
+   };
int r;
uint64_t gpu_addr_tmp = 0;
void *cpu_ptr_tmp = NULL;
 
-   r = amdgpu_bo_create(adev, size, PAGE_SIZE, AMDGPU_GEM_DOMAIN_GTT,
-AMDGPU_GEM_CREATE_CPU_GTT_USWC, ttm_bo_type_kernel,
-NULL, );
+   r = amdgpu_bo_create(adev, , );
if (r) {
dev_err(adev->dev,
"failed to allocate BO for amdkfd (%d)\n", r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 1d6e1479da38..b7bd24c35b25 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1004,6 +1004,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
struct amdgpu_device *adev = get_amdgpu_device(kgd);
struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
struct amdgpu_bo *bo;
+   struct amdgpu_bo_param bp;
int byte_align;
u32 alloc_domain;
u64 alloc_flags;
@@ -1069,8 +1070,13 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
pr_debug("\tcreate BO VA 0x%llx size 0x%llx domain %s\n",
va, size, domain_string(alloc_domain));
 
-   ret = amdgpu_bo_create(adev, size, byte_align,
-   alloc_domain, alloc_flags, ttm_bo_type_device, 
NULL, );
+   bp.size = size;
+   bp.byte_align = byte_align;
+   bp.domain = alloc_domain;
+   bp.flags = alloc_flags;
+   bp.type = ttm_bo_type_device;
+   bp.resv = NULL;
+   ret = amdgpu_bo_create(adev, , );
if (ret) {
pr_debug("Failed to create BO on domain %s. ret %d\n",
domain_string(alloc_domain), ret);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
index 02b849be083b..96bdb454bdf6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -75,13 +75,20 @@ static void amdgpu_benchmark_move(struct amdgpu_device 
*adev, unsigned size,
 {
struct amdgpu_bo *dobj = NULL;
struct amdgpu_bo *sobj = NULL;
+   struct amdgpu_bo_param bp = {
+   .size = size,
+   .byte_align = PAGE_SIZE,
+   .domain = sdomain,
+   .flags = 0,
+   .type = ttm_bo_type_kernel,
+   .resv = NULL
+   };
uint64_t saddr, daddr;
int r, n;
int time;
 
n = AMDGPU_BENCHMARK_ITERATIONS;
-   r = amdgpu_bo_create(adev, size, PAGE_SIZE,sdomain, 0,
-ttm_bo_type_kernel, NULL, );
+   r = amdgpu_bo_create(adev, , );
if (r) {
goto out_cleanup;
}
@@ -93,8 +100,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, ddomain, 0,
-ttm_bo_type_kernel, NULL, );
+   bp.domain = ddomain;
+   r = amdgpu_bo_create(adev, , );
if (r) {
goto out_cleanup;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c 

Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling

[zhoucm1]

On 2018年04月16日 17:36, Christian König wrote:

Am 16.04.2018 um 11:21 schrieb zhoucm1:



On 2018年04月16日 17:04, zhoucm1 wrote:



On 2018年04月16日 16:57, Christian König wrote:

Am 11.04.2018 um 11:22 schrieb Christian König:

Am 11.04.2018 um 04:38 schrieb zhoucm1:



On 2018年04月10日 21:42, Christian König wrote:

When GEM needs to fallback to GTT for VRAM BOs we still want the
preferred domain to be untouched so that the BO has a cance to 
move back

to VRAM in the future.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 14 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 ++---
  2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c

index 46b9ea4e6103..9dc0a190413c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -47,6 +47,7 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

   struct reservation_object *resv,
   struct drm_gem_object **obj)
  {
+    uint32_t domain = initial_domain;
  struct amdgpu_bo *bo;
  int r;
  @@ -57,7 +58,7 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  }
    retry:
-    r = amdgpu_bo_create(adev, size, alignment, initial_domain,
+    r = amdgpu_bo_create(adev, size, alignment, domain,
   flags, type, resv, );
  if (r) {
  if (r != -ERESTARTSYS) {
@@ -66,8 +67,8 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  goto retry;
  }
  -    if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
-    initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
+    if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+    domain |= AMDGPU_GEM_DOMAIN_GTT;
  goto retry;
  }
  DRM_DEBUG("Failed to allocate GEM object (%ld, %d, 
%u, %d)\n",
@@ -75,6 +76,13 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  }
  return r;
  }
+
+    bo->preferred_domains = initial_domain;
+    bo->allowed_domains = initial_domain;
+    if (type != ttm_bo_type_kernel &&
+    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
+    bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
It's an ugly change back after bo creation! Do you think it's 
better than previous?


Well it's better than moving the fallback handling into the core 
functions and then adding a flag to disable it again because we 
don't want it in the core function.




Alternative way, you can add one parameter to amdgpu_bo_create, 
directly pass preferred_domains and allowed_domains to 
amdgpu_bo_create.


Then we would need three parameters, one where to create the BO 
now and two for preferred/allowed domains.


I think that in this case overwriting the placement from the 
initial value looks cleaner to me.


Ping? Any more opinions on how to proceed?

No, I keep my opinion on this. Michel already gave your RB I remember.

Regards,
David Zhou


I agree that neither solution is perfect, but updating the 
placement after we created the BO still sounds like the least 
painful to me.
And I'm going to have patch for amdgpu_bo_create paramters, collect 
many paramters to one param struct like done in vm.


Oh, good idea! That would be a good solution as well I think.

Then we can provide amdgpu_bo_do_create() with both the preferred and 
the allowed domain.


Do you want to keep working on this or should I?
you can push your reverting patch#1 and #2 first. we can re-do this 
patch#3 again after I pass my patches of collecting parameters for 
amdgpu_bo_create.


Regards,
David Zhou


Thanks,
Christian.



Regards,
David Zhou


Christian.



Regards,
Christian.



Regards,
David Zhou

+
  *obj = >gem_base;
    return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index 6d08cde8443c..854d18d5cff4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -375,17 +375,8 @@ static int amdgpu_bo_do_create(struct 
amdgpu_device *adev, unsigned long size,

  drm_gem_private_object_init(adev->ddev, >gem_base, size);
  INIT_LIST_HEAD(>shadow_list);
  INIT_LIST_HEAD(>va);
-    bo->preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
- AMDGPU_GEM_DOMAIN_GTT |
- AMDGPU_GEM_DOMAIN_CPU |
- AMDGPU_GEM_DOMAIN_GDS |
- AMDGPU_GEM_DOMAIN_GWS |
- AMDGPU_GEM_DOMAIN_OA);
-    bo->allowed_domains = bo->preferred_domains;
-    if (type != ttm_bo_type_kernel &&
-    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
-    bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
-
+    bo->preferred_domains = domain;
+    bo->allowed_domains 

Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling

[Christian König]

Am 16.04.2018 um 11:21 schrieb zhoucm1:



On 2018年04月16日 17:04, zhoucm1 wrote:



On 2018年04月16日 16:57, Christian König wrote:

Am 11.04.2018 um 11:22 schrieb Christian König:

Am 11.04.2018 um 04:38 schrieb zhoucm1:



On 2018年04月10日 21:42, Christian König wrote:

When GEM needs to fallback to GTT for VRAM BOs we still want the
preferred domain to be untouched so that the BO has a cance to 
move back

to VRAM in the future.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 14 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 ++---
  2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c

index 46b9ea4e6103..9dc0a190413c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -47,6 +47,7 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

   struct reservation_object *resv,
   struct drm_gem_object **obj)
  {
+    uint32_t domain = initial_domain;
  struct amdgpu_bo *bo;
  int r;
  @@ -57,7 +58,7 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  }
    retry:
-    r = amdgpu_bo_create(adev, size, alignment, initial_domain,
+    r = amdgpu_bo_create(adev, size, alignment, domain,
   flags, type, resv, );
  if (r) {
  if (r != -ERESTARTSYS) {
@@ -66,8 +67,8 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  goto retry;
  }
  -    if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
-    initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
+    if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+    domain |= AMDGPU_GEM_DOMAIN_GTT;
  goto retry;
  }
  DRM_DEBUG("Failed to allocate GEM object (%ld, %d, 
%u, %d)\n",
@@ -75,6 +76,13 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  }
  return r;
  }
+
+    bo->preferred_domains = initial_domain;
+    bo->allowed_domains = initial_domain;
+    if (type != ttm_bo_type_kernel &&
+    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
+    bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
It's an ugly change back after bo creation! Do you think it's 
better than previous?


Well it's better than moving the fallback handling into the core 
functions and then adding a flag to disable it again because we 
don't want it in the core function.




Alternative way, you can add one parameter to amdgpu_bo_create, 
directly pass preferred_domains and allowed_domains to 
amdgpu_bo_create.


Then we would need three parameters, one where to create the BO now 
and two for preferred/allowed domains.


I think that in this case overwriting the placement from the 
initial value looks cleaner to me.


Ping? Any more opinions on how to proceed?

No, I keep my opinion on this. Michel already gave your RB I remember.

Regards,
David Zhou


I agree that neither solution is perfect, but updating the placement 
after we created the BO still sounds like the least painful to me.
And I'm going to have patch for amdgpu_bo_create paramters, collect 
many paramters to one param struct like done in vm.


Oh, good idea! That would be a good solution as well I think.

Then we can provide amdgpu_bo_do_create() with both the preferred and 
the allowed domain.


Do you want to keep working on this or should I?

Thanks,
Christian.



Regards,
David Zhou


Christian.



Regards,
Christian.



Regards,
David Zhou

+
  *obj = >gem_base;
    return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index 6d08cde8443c..854d18d5cff4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -375,17 +375,8 @@ static int amdgpu_bo_do_create(struct 
amdgpu_device *adev, unsigned long size,

  drm_gem_private_object_init(adev->ddev, >gem_base, size);
  INIT_LIST_HEAD(>shadow_list);
  INIT_LIST_HEAD(>va);
-    bo->preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
- AMDGPU_GEM_DOMAIN_GTT |
- AMDGPU_GEM_DOMAIN_CPU |
- AMDGPU_GEM_DOMAIN_GDS |
- AMDGPU_GEM_DOMAIN_GWS |
- AMDGPU_GEM_DOMAIN_OA);
-    bo->allowed_domains = bo->preferred_domains;
-    if (type != ttm_bo_type_kernel &&
-    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
-    bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
-
+    bo->preferred_domains = domain;
+    bo->allowed_domains = domain;
  bo->flags = flags;
    #ifdef CONFIG_X86_32








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

Re: [PATCH] drm/amdgpu: defer test IBs on the rings at boot (V3)

[Zhang, Jerry (Junwei)]

On 04/16/2018 05:19 PM, Christian König wrote:

Am 16.04.2018 um 11:04 schrieb Zhang, Jerry (Junwei):

On 04/16/2018 03:17 PM, Shirish S wrote:

amdgpu_ib_ring_tests() runs test IB's on rings at boot
contributes to ~500 ms of amdgpu driver's boot time.

This patch defers it and ensures that its executed
in amdgpu_info_ioctl() if it wasn't scheduled.

V2: Use queue_delayed_work() & flush_delayed_work().
V3: removed usage of separate wq, ensure ib tests is
 run before enabling clockgating.

Signed-off-by: Shirish S 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 17 ++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|  3 +++
  2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1762eb4..f225840 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1656,6 +1656,10 @@ static int amdgpu_device_ip_late_set_cg_state(struct
amdgpu_device *adev)
  if (amdgpu_emu_mode == 1)
  return 0;

+r = amdgpu_ib_ring_tests(adev);
+if (r)
+DRM_ERROR("ib ring test failed (%d).\n", r);
+


Just confirm:
IIRC, ib may not run immediately via scheduler thread.
Is there any possible that the actual ib ring test interferes cg enablement?


No, the IB tests are run directly on the hardware ring and not through the
scheduler.

But even when we run them through the scheduler we wait for the to complete, so
that shouldn't be an issue.


Thanks for explanation.
That's fine.

Jerry



Christian.



Jerry


  for (i = 0; i < adev->num_ip_blocks; i++) {
  if (!adev->ip_blocks[i].status.valid)
  continue;
@@ -1706,8 +1710,8 @@ static int amdgpu_device_ip_late_init(struct
amdgpu_device *adev)
  }
  }

-mod_delayed_work(system_wq, >late_init_work,
-msecs_to_jiffies(AMDGPU_RESUME_MS));
+queue_delayed_work(system_wq, >late_init_work,
+   msecs_to_jiffies(AMDGPU_RESUME_MS));

  amdgpu_device_fill_reset_magic(adev);

@@ -2374,10 +2378,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
  goto failed;
  }

-r = amdgpu_ib_ring_tests(adev);
-if (r)
-DRM_ERROR("ib ring test failed (%d).\n", r);
-
  if (amdgpu_sriov_vf(adev))
  amdgpu_virt_init_data_exchange(adev);

@@ -2640,11 +2640,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool
resume, bool fbcon)

  amdgpu_fence_driver_resume(adev);

-if (resume) {
-r = amdgpu_ib_ring_tests(adev);
-if (r)
-DRM_ERROR("ib ring test failed (%d).\n", r);
-}

  r = amdgpu_device_ip_late_init(adev);
  if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 487d39e..83d7160 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -279,6 +279,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void
*data, struct drm_file
  if (!info->return_size || !info->return_pointer)
  return -EINVAL;

+/* Ensure IB tests are run on ring */
+flush_delayed_work(>late_init_work);
+
  switch (info->query) {
  case AMDGPU_INFO_ACCEL_WORKING:
  ui32 = adev->accel_working;




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

Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling

[zhoucm1]

On 2018年04月16日 17:04, zhoucm1 wrote:



On 2018年04月16日 16:57, Christian König wrote:

Am 11.04.2018 um 11:22 schrieb Christian König:

Am 11.04.2018 um 04:38 schrieb zhoucm1:



On 2018年04月10日 21:42, Christian König wrote:

When GEM needs to fallback to GTT for VRAM BOs we still want the
preferred domain to be untouched so that the BO has a cance to 
move back

to VRAM in the future.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 14 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 ++---
  2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c

index 46b9ea4e6103..9dc0a190413c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -47,6 +47,7 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

   struct reservation_object *resv,
   struct drm_gem_object **obj)
  {
+    uint32_t domain = initial_domain;
  struct amdgpu_bo *bo;
  int r;
  @@ -57,7 +58,7 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  }
    retry:
-    r = amdgpu_bo_create(adev, size, alignment, initial_domain,
+    r = amdgpu_bo_create(adev, size, alignment, domain,
   flags, type, resv, );
  if (r) {
  if (r != -ERESTARTSYS) {
@@ -66,8 +67,8 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  goto retry;
  }
  -    if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
-    initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
+    if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+    domain |= AMDGPU_GEM_DOMAIN_GTT;
  goto retry;
  }
  DRM_DEBUG("Failed to allocate GEM object (%ld, %d, 
%u, %d)\n",
@@ -75,6 +76,13 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  }
  return r;
  }
+
+    bo->preferred_domains = initial_domain;
+    bo->allowed_domains = initial_domain;
+    if (type != ttm_bo_type_kernel &&
+    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
+    bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
It's an ugly change back after bo creation! Do you think it's 
better than previous?


Well it's better than moving the fallback handling into the core 
functions and then adding a flag to disable it again because we 
don't want it in the core function.




Alternative way, you can add one parameter to amdgpu_bo_create, 
directly pass preferred_domains and allowed_domains to 
amdgpu_bo_create.


Then we would need three parameters, one where to create the BO now 
and two for preferred/allowed domains.


I think that in this case overwriting the placement from the initial 
value looks cleaner to me.


Ping? Any more opinions on how to proceed?

No, I keep my opinion on this. Michel already gave your RB I remember.

Regards,
David Zhou


I agree that neither solution is perfect, but updating the placement 
after we created the BO still sounds like the least painful to me.
And I'm going to have patch for amdgpu_bo_create paramters, collect many 
paramters to one param struct like done in vm.


Regards,
David Zhou


Christian.



Regards,
Christian.



Regards,
David Zhou

+
  *obj = >gem_base;
    return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index 6d08cde8443c..854d18d5cff4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -375,17 +375,8 @@ static int amdgpu_bo_do_create(struct 
amdgpu_device *adev, unsigned long size,

  drm_gem_private_object_init(adev->ddev, >gem_base, size);
  INIT_LIST_HEAD(>shadow_list);
  INIT_LIST_HEAD(>va);
-    bo->preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
- AMDGPU_GEM_DOMAIN_GTT |
- AMDGPU_GEM_DOMAIN_CPU |
- AMDGPU_GEM_DOMAIN_GDS |
- AMDGPU_GEM_DOMAIN_GWS |
- AMDGPU_GEM_DOMAIN_OA);
-    bo->allowed_domains = bo->preferred_domains;
-    if (type != ttm_bo_type_kernel &&
-    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
-    bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
-
+    bo->preferred_domains = domain;
+    bo->allowed_domains = domain;
  bo->flags = flags;
    #ifdef CONFIG_X86_32








___
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] drm/amdgpu: defer test IBs on the rings at boot (V3)

[Christian König]

Am 16.04.2018 um 11:04 schrieb Zhang, Jerry (Junwei):

On 04/16/2018 03:17 PM, Shirish S wrote:

amdgpu_ib_ring_tests() runs test IB's on rings at boot
contributes to ~500 ms of amdgpu driver's boot time.

This patch defers it and ensures that its executed
in amdgpu_info_ioctl() if it wasn't scheduled.

V2: Use queue_delayed_work() & flush_delayed_work().
V3: removed usage of separate wq, ensure ib tests is
 run before enabling clockgating.

Signed-off-by: Shirish S 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 17 ++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  3 +++
  2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

index 1762eb4..f225840 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1656,6 +1656,10 @@ static int 
amdgpu_device_ip_late_set_cg_state(struct amdgpu_device *adev)

  if (amdgpu_emu_mode == 1)
  return 0;

+    r = amdgpu_ib_ring_tests(adev);
+    if (r)
+    DRM_ERROR("ib ring test failed (%d).\n", r);
+


Just confirm:
IIRC, ib may not run immediately via scheduler thread.
Is there any possible that the actual ib ring test interferes cg 
enablement?


No, the IB tests are run directly on the hardware ring and not through 
the scheduler.


But even when we run them through the scheduler we wait for the to 
complete, so that shouldn't be an issue.


Christian.



Jerry


  for (i = 0; i < adev->num_ip_blocks; i++) {
  if (!adev->ip_blocks[i].status.valid)
  continue;
@@ -1706,8 +1710,8 @@ static int amdgpu_device_ip_late_init(struct 
amdgpu_device *adev)

  }
  }

-    mod_delayed_work(system_wq, >late_init_work,
-    msecs_to_jiffies(AMDGPU_RESUME_MS));
+    queue_delayed_work(system_wq, >late_init_work,
+   msecs_to_jiffies(AMDGPU_RESUME_MS));

  amdgpu_device_fill_reset_magic(adev);

@@ -2374,10 +2378,6 @@ int amdgpu_device_init(struct amdgpu_device 
*adev,

  goto failed;
  }

-    r = amdgpu_ib_ring_tests(adev);
-    if (r)
-    DRM_ERROR("ib ring test failed (%d).\n", r);
-
  if (amdgpu_sriov_vf(adev))
  amdgpu_virt_init_data_exchange(adev);

@@ -2640,11 +2640,6 @@ int amdgpu_device_resume(struct drm_device 
*dev, bool resume, bool fbcon)


  amdgpu_fence_driver_resume(adev);

-    if (resume) {
-    r = amdgpu_ib_ring_tests(adev);
-    if (r)
-    DRM_ERROR("ib ring test failed (%d).\n", r);
-    }

  r = amdgpu_device_ip_late_init(adev);
  if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

index 487d39e..83d7160 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -279,6 +279,9 @@ static int amdgpu_info_ioctl(struct drm_device 
*dev, void *data, struct drm_file

  if (!info->return_size || !info->return_pointer)
  return -EINVAL;

+    /* Ensure IB tests are run on ring */
+    flush_delayed_work(>late_init_work);
+
  switch (info->query) {
  case AMDGPU_INFO_ACCEL_WORKING:
  ui32 = adev->accel_working;



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

Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling

[zhoucm1]

On 2018年04月16日 16:57, Christian König wrote:

Am 11.04.2018 um 11:22 schrieb Christian König:

Am 11.04.2018 um 04:38 schrieb zhoucm1:



On 2018年04月10日 21:42, Christian König wrote:

When GEM needs to fallback to GTT for VRAM BOs we still want the
preferred domain to be untouched so that the BO has a cance to move 
back

to VRAM in the future.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 14 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 ++---
  2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c

index 46b9ea4e6103..9dc0a190413c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -47,6 +47,7 @@ int amdgpu_gem_object_create(struct amdgpu_device 
*adev, unsigned long size,

   struct reservation_object *resv,
   struct drm_gem_object **obj)
  {
+    uint32_t domain = initial_domain;
  struct amdgpu_bo *bo;
  int r;
  @@ -57,7 +58,7 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  }
    retry:
-    r = amdgpu_bo_create(adev, size, alignment, initial_domain,
+    r = amdgpu_bo_create(adev, size, alignment, domain,
   flags, type, resv, );
  if (r) {
  if (r != -ERESTARTSYS) {
@@ -66,8 +67,8 @@ int amdgpu_gem_object_create(struct amdgpu_device 
*adev, unsigned long size,

  goto retry;
  }
  -    if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
-    initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
+    if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+    domain |= AMDGPU_GEM_DOMAIN_GTT;
  goto retry;
  }
  DRM_DEBUG("Failed to allocate GEM object (%ld, %d, 
%u, %d)\n",
@@ -75,6 +76,13 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  }
  return r;
  }
+
+    bo->preferred_domains = initial_domain;
+    bo->allowed_domains = initial_domain;
+    if (type != ttm_bo_type_kernel &&
+    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
+    bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
It's an ugly change back after bo creation! Do you think it's better 
than previous?


Well it's better than moving the fallback handling into the core 
functions and then adding a flag to disable it again because we don't 
want it in the core function.




Alternative way, you can add one parameter to amdgpu_bo_create, 
directly pass preferred_domains and allowed_domains to 
amdgpu_bo_create.


Then we would need three parameters, one where to create the BO now 
and two for preferred/allowed domains.


I think that in this case overwriting the placement from the initial 
value looks cleaner to me.


Ping? Any more opinions on how to proceed?

No, I keep my opinion on this. Michel already gave your RB I remember.

Regards,
David Zhou


I agree that neither solution is perfect, but updating the placement 
after we created the BO still sounds like the least painful to me.


Christian.



Regards,
Christian.



Regards,
David Zhou

+
  *obj = >gem_base;
    return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index 6d08cde8443c..854d18d5cff4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -375,17 +375,8 @@ static int amdgpu_bo_do_create(struct 
amdgpu_device *adev, unsigned long size,

  drm_gem_private_object_init(adev->ddev, >gem_base, size);
  INIT_LIST_HEAD(>shadow_list);
  INIT_LIST_HEAD(>va);
-    bo->preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
- AMDGPU_GEM_DOMAIN_GTT |
- AMDGPU_GEM_DOMAIN_CPU |
- AMDGPU_GEM_DOMAIN_GDS |
- AMDGPU_GEM_DOMAIN_GWS |
- AMDGPU_GEM_DOMAIN_OA);
-    bo->allowed_domains = bo->preferred_domains;
-    if (type != ttm_bo_type_kernel &&
-    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
-    bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
-
+    bo->preferred_domains = domain;
+    bo->allowed_domains = domain;
  bo->flags = flags;
    #ifdef CONFIG_X86_32








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

Re: [PATCH] drm/amdgpu: defer test IBs on the rings at boot (V3)

[Zhang, Jerry (Junwei)]

On 04/16/2018 03:17 PM, Shirish S wrote:

amdgpu_ib_ring_tests() runs test IB's on rings at boot
contributes to ~500 ms of amdgpu driver's boot time.

This patch defers it and ensures that its executed
in amdgpu_info_ioctl() if it wasn't scheduled.

V2: Use queue_delayed_work() & flush_delayed_work().
V3: removed usage of separate wq, ensure ib tests is
 run before enabling clockgating.

Signed-off-by: Shirish S 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 17 ++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|  3 +++
  2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1762eb4..f225840 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1656,6 +1656,10 @@ static int amdgpu_device_ip_late_set_cg_state(struct 
amdgpu_device *adev)
if (amdgpu_emu_mode == 1)
return 0;

+   r = amdgpu_ib_ring_tests(adev);
+   if (r)
+   DRM_ERROR("ib ring test failed (%d).\n", r);
+


Just confirm:
IIRC, ib may not run immediately via scheduler thread.
Is there any possible that the actual ib ring test interferes cg enablement?

Jerry


for (i = 0; i < adev->num_ip_blocks; i++) {
if (!adev->ip_blocks[i].status.valid)
continue;
@@ -1706,8 +1710,8 @@ static int amdgpu_device_ip_late_init(struct 
amdgpu_device *adev)
}
}

-   mod_delayed_work(system_wq, >late_init_work,
-   msecs_to_jiffies(AMDGPU_RESUME_MS));
+   queue_delayed_work(system_wq, >late_init_work,
+  msecs_to_jiffies(AMDGPU_RESUME_MS));

amdgpu_device_fill_reset_magic(adev);

@@ -2374,10 +2378,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
goto failed;
}

-   r = amdgpu_ib_ring_tests(adev);
-   if (r)
-   DRM_ERROR("ib ring test failed (%d).\n", r);
-
if (amdgpu_sriov_vf(adev))
amdgpu_virt_init_data_exchange(adev);

@@ -2640,11 +2640,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool 
resume, bool fbcon)

amdgpu_fence_driver_resume(adev);

-   if (resume) {
-   r = amdgpu_ib_ring_tests(adev);
-   if (r)
-   DRM_ERROR("ib ring test failed (%d).\n", r);
-   }

r = amdgpu_device_ip_late_init(adev);
if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 487d39e..83d7160 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -279,6 +279,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
if (!info->return_size || !info->return_pointer)
return -EINVAL;

+   /* Ensure IB tests are run on ring */
+   flush_delayed_work(>late_init_work);
+
switch (info->query) {
case AMDGPU_INFO_ACCEL_WORKING:
ui32 = adev->accel_working;


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

Re: [PATCH 3/3] drm/amdgpu: set preferred_domain independent of fallback handling

[Christian König]

Am 11.04.2018 um 11:22 schrieb Christian König:

Am 11.04.2018 um 04:38 schrieb zhoucm1:



On 2018年04月10日 21:42, Christian König wrote:

When GEM needs to fallback to GTT for VRAM BOs we still want the
preferred domain to be untouched so that the BO has a cance to move 
back

to VRAM in the future.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 14 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 ++---
  2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c

index 46b9ea4e6103..9dc0a190413c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -47,6 +47,7 @@ int amdgpu_gem_object_create(struct amdgpu_device 
*adev, unsigned long size,

   struct reservation_object *resv,
   struct drm_gem_object **obj)
  {
+    uint32_t domain = initial_domain;
  struct amdgpu_bo *bo;
  int r;
  @@ -57,7 +58,7 @@ int amdgpu_gem_object_create(struct 
amdgpu_device *adev, unsigned long size,

  }
    retry:
-    r = amdgpu_bo_create(adev, size, alignment, initial_domain,
+    r = amdgpu_bo_create(adev, size, alignment, domain,
   flags, type, resv, );
  if (r) {
  if (r != -ERESTARTSYS) {
@@ -66,8 +67,8 @@ int amdgpu_gem_object_create(struct amdgpu_device 
*adev, unsigned long size,

  goto retry;
  }
  -    if (initial_domain == AMDGPU_GEM_DOMAIN_VRAM) {
-    initial_domain |= AMDGPU_GEM_DOMAIN_GTT;
+    if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
+    domain |= AMDGPU_GEM_DOMAIN_GTT;
  goto retry;
  }
  DRM_DEBUG("Failed to allocate GEM object (%ld, %d, %u, 
%d)\n",
@@ -75,6 +76,13 @@ int amdgpu_gem_object_create(struct amdgpu_device 
*adev, unsigned long size,

  }
  return r;
  }
+
+    bo->preferred_domains = initial_domain;
+    bo->allowed_domains = initial_domain;
+    if (type != ttm_bo_type_kernel &&
+    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
+    bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
It's an ugly change back after bo creation! Do you think it's better 
than previous?


Well it's better than moving the fallback handling into the core 
functions and then adding a flag to disable it again because we don't 
want it in the core function.




Alternative way, you can add one parameter to amdgpu_bo_create, 
directly pass preferred_domains and allowed_domains to amdgpu_bo_create.


Then we would need three parameters, one where to create the BO now 
and two for preferred/allowed domains.


I think that in this case overwriting the placement from the initial 
value looks cleaner to me.


Ping? Any more opinions on how to proceed?

I agree that neither solution is perfect, but updating the placement 
after we created the BO still sounds like the least painful to me.


Christian.



Regards,
Christian.



Regards,
David Zhou

+
  *obj = >gem_base;
    return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index 6d08cde8443c..854d18d5cff4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -375,17 +375,8 @@ static int amdgpu_bo_do_create(struct 
amdgpu_device *adev, unsigned long size,

  drm_gem_private_object_init(adev->ddev, >gem_base, size);
  INIT_LIST_HEAD(>shadow_list);
  INIT_LIST_HEAD(>va);
-    bo->preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
- AMDGPU_GEM_DOMAIN_GTT |
- AMDGPU_GEM_DOMAIN_CPU |
- AMDGPU_GEM_DOMAIN_GDS |
- AMDGPU_GEM_DOMAIN_GWS |
- AMDGPU_GEM_DOMAIN_OA);
-    bo->allowed_domains = bo->preferred_domains;
-    if (type != ttm_bo_type_kernel &&
-    bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
-    bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
-
+    bo->preferred_domains = domain;
+    bo->allowed_domains = domain;
  bo->flags = flags;
    #ifdef CONFIG_X86_32






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

Re: [RFC] Rewrite page fault interception in TTM drivers

[Christian König]

Am 15.04.2018 um 05:31 schrieb Matthew Wilcox:

Three^W Two of the TTM drivers intercept the pagefault handler in a rather
un-Linuxy and racy way.  If they really must intercept the fault call
(and it's not clear to me that they need to), I'd rather see them do it
like this.

The QXL driver seems least likely to need it; as the virtio driver has
exactly the same code in it, only commented out.


QXL and virtio can probably just be cleaned up. The code looks more like 
copy leftover from radeon to me.



The radeon driver takes its own lock ... maybe there's a way to avoid that 
since no other driver
needs to take its own lock at this point?


No, there isn't. Accessing the PCI BAR with the CPU while the driver is 
changing the memory clocks can shoot the whole system into nirvana.


That's why radeon intercepts the fault handler and so prevents all CPU 
access to BOs while the memory clocks are changing.


Anyway that still looks like a valid cleanup to me. I would just split 
it into cleaning up qxl/virtio by removing the code and then cleaning up 
the TTM/Radeon interaction separately.


Regards,
Christian.



diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index ee2340e31f06..d2c76a3d6816 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -102,21 +102,23 @@ static void qxl_ttm_global_fini(struct qxl_device *qdev)
}
  }
  
-static struct vm_operations_struct qxl_ttm_vm_ops;

-static const struct vm_operations_struct *ttm_vm_ops;
-
  static int qxl_ttm_fault(struct vm_fault *vmf)
  {
struct ttm_buffer_object *bo;
-   int r;
  
  	bo = (struct ttm_buffer_object *)vmf->vma->vm_private_data;

if (bo == NULL)
return VM_FAULT_NOPAGE;
-   r = ttm_vm_ops->fault(vmf);
-   return r;
+   return ttm_bo_vm_fault(vmf);
  }
  
+static const struct vm_operations_struct qxl_ttm_vm_ops = {

+   .fault = qxl_ttm_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+   .access = ttm_bo_vm_access,
+};
+
  int qxl_mmap(struct file *filp, struct vm_area_struct *vma)
  {
struct drm_file *file_priv;
@@ -139,11 +141,6 @@ int qxl_mmap(struct file *filp, struct vm_area_struct *vma)
r = ttm_bo_mmap(filp, vma, >mman.bdev);
if (unlikely(r != 0))
return r;
-   if (unlikely(ttm_vm_ops == NULL)) {
-   ttm_vm_ops = vma->vm_ops;
-   qxl_ttm_vm_ops = *ttm_vm_ops;
-   qxl_ttm_vm_ops.fault = _ttm_fault;
-   }
vma->vm_ops = _ttm_vm_ops;
return 0;
  }
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 8689fcca051c..08cc0c5b9e94 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -944,9 +944,6 @@ void radeon_ttm_set_active_vram_size(struct radeon_device 
*rdev, u64 size)
man->size = size >> PAGE_SHIFT;
  }
  
-static struct vm_operations_struct radeon_ttm_vm_ops;

-static const struct vm_operations_struct *ttm_vm_ops = NULL;
-
  static int radeon_ttm_fault(struct vm_fault *vmf)
  {
struct ttm_buffer_object *bo;
@@ -959,11 +956,18 @@ static int radeon_ttm_fault(struct vm_fault *vmf)
}
rdev = radeon_get_rdev(bo->bdev);
down_read(>pm.mclk_lock);
-   r = ttm_vm_ops->fault(vmf);
+   r = ttm_bo_vm_fault(vmf);
up_read(>pm.mclk_lock);
return r;
  }
  
+static const struct vm_operations_struct radeon_ttm_vm_ops = {

+   .fault = radeon_ttm_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+   .access = ttm_bo_vm_access,
+};
+
  int radeon_mmap(struct file *filp, struct vm_area_struct *vma)
  {
struct drm_file *file_priv;
@@ -983,11 +987,6 @@ int radeon_mmap(struct file *filp, struct vm_area_struct 
*vma)
if (unlikely(r != 0)) {
return r;
}
-   if (unlikely(ttm_vm_ops == NULL)) {
-   ttm_vm_ops = vma->vm_ops;
-   radeon_ttm_vm_ops = *ttm_vm_ops;
-   radeon_ttm_vm_ops.fault = _ttm_fault;
-   }
vma->vm_ops = _ttm_vm_ops;
return 0;
  }
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 8eba95b3c737..f59a8f41aae0 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -104,7 +104,7 @@ static unsigned long ttm_bo_io_mem_pfn(struct 
ttm_buffer_object *bo,
+ page_offset;
  }
  
-static int ttm_bo_vm_fault(struct vm_fault *vmf)

+int ttm_bo_vm_fault(struct vm_fault *vmf)
  {
struct vm_area_struct *vma = vmf->vma;
struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
@@ -294,8 +294,9 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
ttm_bo_unreserve(bo);
return ret;
  }
+EXPORT_SYMBOL_GPL(ttm_bo_vm_fault);
  
-static void ttm_bo_vm_open(struct vm_area_struct *vma)

+void ttm_bo_vm_open(struct vm_area_struct *vma)
  {
struct 

Re: [PATCH] drm/amdgpu: defer test IBs on the rings at boot (V3)

[Christian König]

Am 16.04.2018 um 09:17 schrieb Shirish S:

amdgpu_ib_ring_tests() runs test IB's on rings at boot
contributes to ~500 ms of amdgpu driver's boot time.

This patch defers it and ensures that its executed
in amdgpu_info_ioctl() if it wasn't scheduled.

V2: Use queue_delayed_work() & flush_delayed_work().
V3: removed usage of separate wq, ensure ib tests is
 run before enabling clockgating.

Signed-off-by: Shirish S 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 17 ++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|  3 +++
  2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1762eb4..f225840 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1656,6 +1656,10 @@ static int amdgpu_device_ip_late_set_cg_state(struct 
amdgpu_device *adev)
if (amdgpu_emu_mode == 1)
return 0;
  
+	r = amdgpu_ib_ring_tests(adev);

+   if (r)
+   DRM_ERROR("ib ring test failed (%d).\n", r);
+
for (i = 0; i < adev->num_ip_blocks; i++) {
if (!adev->ip_blocks[i].status.valid)
continue;
@@ -1706,8 +1710,8 @@ static int amdgpu_device_ip_late_init(struct 
amdgpu_device *adev)
}
}
  
-	mod_delayed_work(system_wq, >late_init_work,

-   msecs_to_jiffies(AMDGPU_RESUME_MS));
+   queue_delayed_work(system_wq, >late_init_work,
+  msecs_to_jiffies(AMDGPU_RESUME_MS));
  
  	amdgpu_device_fill_reset_magic(adev);
  
@@ -2374,10 +2378,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,

goto failed;
}
  
-	r = amdgpu_ib_ring_tests(adev);

-   if (r)
-   DRM_ERROR("ib ring test failed (%d).\n", r);
-
if (amdgpu_sriov_vf(adev))
amdgpu_virt_init_data_exchange(adev);
  
@@ -2640,11 +2640,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon)
  
  	amdgpu_fence_driver_resume(adev);
  
-	if (resume) {

-   r = amdgpu_ib_ring_tests(adev);
-   if (r)
-   DRM_ERROR("ib ring test failed (%d).\n", r);
-   }
  
  	r = amdgpu_device_ip_late_init(adev);

if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 487d39e..83d7160 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -279,6 +279,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
if (!info->return_size || !info->return_pointer)
return -EINVAL;
  
+	/* Ensure IB tests are run on ring */

+   flush_delayed_work(>late_init_work);
+
switch (info->query) {
case AMDGPU_INFO_ACCEL_WORKING:
ui32 = adev->accel_working;


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

Re: Raven Ridge Ryzen 2500U hang reproduced

[Nicolai Hähnle]

On 14.04.2018 00:24, Bráulio Bhavamitra wrote:
It ALWAYS crashes on shader15 of 
http://www.graphicsfuzz.com/benchmark/android-v1.html.


This is very likely an unrelated issue to any kind of desktop hang 
you're seeing. The graphics fuzz shaders are the result of fuzzing to 
intentionally generate unusual control flow structures which are likely 
to trigger shader compiler bugs. Typical desktop workloads don't have 
such shaders, so any generic desktop hang you're seeing is almost 
certainly unrelated.


Cheers,
Nicolai




Also reported at https://bugzilla.redhat.com/show_bug.cgi?id=1562530

Using kernel 4.16 with options rcu_nocb=0-15 and amdgpu.dpm=0

Cheers,
Bráulio

On Mon, Mar 26, 2018 at 8:30 PM Bráulio Bhavamitra > wrote:


Hi all,

Following the random crashes happenning with many users (e.g.

https://www.phoronix.com/scan.php?page=news_item=Raven-Ridge-March-Update),
not only on Linux but also Windows, I've been struggling to
reproduce and generate any error log.

After discovering that the error only happenned with KDE and games
(at least for me, see https://bugs.kde.org/show_bug.cgi?id=392378),
I could reproduce after a failing suspend.

The crash most of the times allows the mouse to keep moving, but
anything else works. Except for this time the keyboard worked so I
could switch the tty and save the dmesg messages. After this I had
to force reboot as it got stuck trying to kill the lightdm service
(gpu hanged?).

The errors are, see attached the full dmesg:
[ 2899.525650] amdgpu :03:00.0: couldn't schedule ib on ring 
[ 2899.525769] [drm:amdgpu_job_run [amdgpu]] *ERROR* Error
scheduling IBs (-22)
[ 2909.125047] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx
timeout, last signaled seq=174624, last emitted seq=174627
[ 2909.125060] [drm] IP block:psp is hung!
[ 2909.125063] [drm] GPU recovery disabled.
[ 2914.756931] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR*
amdgpu_cs_list_validate(validated) failed.
[ 2914.756997] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to
process the buffer list -16!
[ 2914.997372] amdgpu :03:00.0: couldn't schedule ib on ring 
[ 2914.997498] [drm:amdgpu_job_run [amdgpu]] *ERROR* Error
scheduling IBs (-22)
[ 2930.117275] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR*
amdgpu_cs_list_validate(validated) failed.
[ 2930.117405] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to
process the buffer list -16!
[ 2930.152015] [drm:amdgpu_fill_buffer [amdgpu]] *ERROR* Trying to
clear memory with ring turned off.
[ 2930.157940] [drm:amdgpu_fill_buffer [amdgpu]] *ERROR* Trying to
clear memory with ring turned off.
[ 2930.180535] [drm:amdgpu_fill_buffer [amdgpu]] *ERROR* Trying to
clear memory with ring turned off.
[ 2933.781692] IPv6: ADDRCONF(NETDEV_CHANGE): wlp2s0: link becomes ready
[ 2945.477205] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR*
amdgpu_cs_list_validate(validated) failed.
[ 2945.477348] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to
process the buffer list -16!

System details:
HP Envy x360 Ryzen 2500U
ArchLinux, kernel 4.16rc6 and 4.15.12

Cheers,
bráulio



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




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Gamma problem after 4096 LUT entries change

[Môshe van der Sterre]

Hello Leo Li,

The current mainline has a gamma regression for me, running wayland on a 
Ryzen 5 2400G (integrated gpu). Colors are much too dark after the 
following commit:

086247a4b2fb "drm/amd/display: Use 4096 lut entries"

From this commit alone I cannot deduce how to properly address the 
problem. Reverting restores the correct colors. Unfortunately, I have no 
other amdgpu hardware to test if this is specific to the integrated gpu 
or not.


I'll gladly test possible fixes or debug a little further if you can 
point me in the right direction.


Thanks,
Môshe van der Sterre




smime.p7s
Description: S/MIME Cryptographic Signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[RFC] Rewrite page fault interception in TTM drivers

[Matthew Wilcox]

Three^W Two of the TTM drivers intercept the pagefault handler in a rather
un-Linuxy and racy way.  If they really must intercept the fault call
(and it's not clear to me that they need to), I'd rather see them do it
like this.

The QXL driver seems least likely to need it; as the virtio driver has
exactly the same code in it, only commented out.  The radeon driver takes
its own lock ... maybe there's a way to avoid that since no other driver
needs to take its own lock at this point?

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index ee2340e31f06..d2c76a3d6816 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -102,21 +102,23 @@ static void qxl_ttm_global_fini(struct qxl_device *qdev)
}
 }
 
-static struct vm_operations_struct qxl_ttm_vm_ops;
-static const struct vm_operations_struct *ttm_vm_ops;
-
 static int qxl_ttm_fault(struct vm_fault *vmf)
 {
struct ttm_buffer_object *bo;
-   int r;
 
bo = (struct ttm_buffer_object *)vmf->vma->vm_private_data;
if (bo == NULL)
return VM_FAULT_NOPAGE;
-   r = ttm_vm_ops->fault(vmf);
-   return r;
+   return ttm_bo_vm_fault(vmf);
 }
 
+static const struct vm_operations_struct qxl_ttm_vm_ops = {
+   .fault = qxl_ttm_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+   .access = ttm_bo_vm_access,
+};
+
 int qxl_mmap(struct file *filp, struct vm_area_struct *vma)
 {
struct drm_file *file_priv;
@@ -139,11 +141,6 @@ int qxl_mmap(struct file *filp, struct vm_area_struct *vma)
r = ttm_bo_mmap(filp, vma, >mman.bdev);
if (unlikely(r != 0))
return r;
-   if (unlikely(ttm_vm_ops == NULL)) {
-   ttm_vm_ops = vma->vm_ops;
-   qxl_ttm_vm_ops = *ttm_vm_ops;
-   qxl_ttm_vm_ops.fault = _ttm_fault;
-   }
vma->vm_ops = _ttm_vm_ops;
return 0;
 }
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 8689fcca051c..08cc0c5b9e94 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -944,9 +944,6 @@ void radeon_ttm_set_active_vram_size(struct radeon_device 
*rdev, u64 size)
man->size = size >> PAGE_SHIFT;
 }
 
-static struct vm_operations_struct radeon_ttm_vm_ops;
-static const struct vm_operations_struct *ttm_vm_ops = NULL;
-
 static int radeon_ttm_fault(struct vm_fault *vmf)
 {
struct ttm_buffer_object *bo;
@@ -959,11 +956,18 @@ static int radeon_ttm_fault(struct vm_fault *vmf)
}
rdev = radeon_get_rdev(bo->bdev);
down_read(>pm.mclk_lock);
-   r = ttm_vm_ops->fault(vmf);
+   r = ttm_bo_vm_fault(vmf);
up_read(>pm.mclk_lock);
return r;
 }
 
+static const struct vm_operations_struct radeon_ttm_vm_ops = {
+   .fault = radeon_ttm_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+   .access = ttm_bo_vm_access,
+};
+
 int radeon_mmap(struct file *filp, struct vm_area_struct *vma)
 {
struct drm_file *file_priv;
@@ -983,11 +987,6 @@ int radeon_mmap(struct file *filp, struct vm_area_struct 
*vma)
if (unlikely(r != 0)) {
return r;
}
-   if (unlikely(ttm_vm_ops == NULL)) {
-   ttm_vm_ops = vma->vm_ops;
-   radeon_ttm_vm_ops = *ttm_vm_ops;
-   radeon_ttm_vm_ops.fault = _ttm_fault;
-   }
vma->vm_ops = _ttm_vm_ops;
return 0;
 }
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 8eba95b3c737..f59a8f41aae0 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -104,7 +104,7 @@ static unsigned long ttm_bo_io_mem_pfn(struct 
ttm_buffer_object *bo,
+ page_offset;
 }
 
-static int ttm_bo_vm_fault(struct vm_fault *vmf)
+int ttm_bo_vm_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
@@ -294,8 +294,9 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
ttm_bo_unreserve(bo);
return ret;
 }
+EXPORT_SYMBOL_GPL(ttm_bo_vm_fault);
 
-static void ttm_bo_vm_open(struct vm_area_struct *vma)
+void ttm_bo_vm_open(struct vm_area_struct *vma)
 {
struct ttm_buffer_object *bo =
(struct ttm_buffer_object *)vma->vm_private_data;
@@ -304,14 +305,16 @@ static void ttm_bo_vm_open(struct vm_area_struct *vma)
 
(void)ttm_bo_reference(bo);
 }
+EXPORT_SYMBOL_GPL(ttm_bo_vm_open);
 
-static void ttm_bo_vm_close(struct vm_area_struct *vma)
+void ttm_bo_vm_close(struct vm_area_struct *vma)
 {
struct ttm_buffer_object *bo = (struct ttm_buffer_object 
*)vma->vm_private_data;
 
ttm_bo_unref();
vma->vm_private_data = NULL;
 }
+EXPORT_SYMBOL_GPL(ttm_bo_vm_close);
 
 static int ttm_bo_vm_access_kmap(struct ttm_buffer_object *bo,
 unsigned long offset,

[PATCH] drm/amdgpu: defer test IBs on the rings at boot (V3)

[Shirish S]

amdgpu_ib_ring_tests() runs test IB's on rings at boot
contributes to ~500 ms of amdgpu driver's boot time.

This patch defers it and ensures that its executed
in amdgpu_info_ioctl() if it wasn't scheduled.

V2: Use queue_delayed_work() & flush_delayed_work().
V3: removed usage of separate wq, ensure ib tests is
run before enabling clockgating.

Signed-off-by: Shirish S 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 17 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|  3 +++
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1762eb4..f225840 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1656,6 +1656,10 @@ static int amdgpu_device_ip_late_set_cg_state(struct 
amdgpu_device *adev)
if (amdgpu_emu_mode == 1)
return 0;
 
+   r = amdgpu_ib_ring_tests(adev);
+   if (r)
+   DRM_ERROR("ib ring test failed (%d).\n", r);
+
for (i = 0; i < adev->num_ip_blocks; i++) {
if (!adev->ip_blocks[i].status.valid)
continue;
@@ -1706,8 +1710,8 @@ static int amdgpu_device_ip_late_init(struct 
amdgpu_device *adev)
}
}
 
-   mod_delayed_work(system_wq, >late_init_work,
-   msecs_to_jiffies(AMDGPU_RESUME_MS));
+   queue_delayed_work(system_wq, >late_init_work,
+  msecs_to_jiffies(AMDGPU_RESUME_MS));
 
amdgpu_device_fill_reset_magic(adev);
 
@@ -2374,10 +2378,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
goto failed;
}
 
-   r = amdgpu_ib_ring_tests(adev);
-   if (r)
-   DRM_ERROR("ib ring test failed (%d).\n", r);
-
if (amdgpu_sriov_vf(adev))
amdgpu_virt_init_data_exchange(adev);
 
@@ -2640,11 +2640,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool 
resume, bool fbcon)
 
amdgpu_fence_driver_resume(adev);
 
-   if (resume) {
-   r = amdgpu_ib_ring_tests(adev);
-   if (r)
-   DRM_ERROR("ib ring test failed (%d).\n", r);
-   }
 
r = amdgpu_device_ip_late_init(adev);
if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 487d39e..83d7160 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -279,6 +279,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
if (!info->return_size || !info->return_pointer)
return -EINVAL;
 
+   /* Ensure IB tests are run on ring */
+   flush_delayed_work(>late_init_work);
+
switch (info->query) {
case AMDGPU_INFO_ACCEL_WORKING:
ui32 = adev->accel_working;
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu: defer test IBs on the rings at boot (V2)

[S, Shirish]

On 4/13/2018 10:20 PM, Alex Deucher wrote:

On Fri, Apr 13, 2018 at 9:25 AM, Christian König
 wrote:

Am 13.04.2018 um 10:31 schrieb Shirish S:

amdgpu_ib_ring_tests() runs test IB's on rings at boot
contributes to ~500 ms of amdgpu driver's boot time.

This patch defers it and ensures that its executed
in amdgpu_info_ioctl() if it wasn't scheduled.

V2: Use queue_delayed_work() & flush_delayed_work().

Signed-off-by: Shirish S 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu.h|  2 ++
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 +---
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|  4 
   3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 5734871..ae8f722 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1611,6 +1611,8 @@ struct amdgpu_device {
 /* delayed work_func for deferring clockgating during resume */
 struct delayed_work late_init_work;
+   /* delayed work_func to defer testing IB's on rings during boot */
+   struct delayed_work late_init_test_ib_work;


That still has the chance of running the late init in parallel with the IB
tests and that really doesn't looks like a good idea to me.

Yeah, at least on older chips we run into problems if we power or
clock gate some engines while they are in use.  Even on engines that
support dynamic gating, you usually have to set it up while the engine
is idle.  Make sure the IB tests run before we enable gating.

Ok Alex.
I have re-spun V3, with only one delayed work and ensuring ib tests are 
run before enabling clock gating.

Regards,
Shirish S

Alex


Is there any issue with putting the IB test into the late init work handler
as well?



 struct amdgpu_virt  virt;
 /* firmware VRAM reservation */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1762eb4..ee84058 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -63,6 +63,7 @@ MODULE_FIRMWARE("amdgpu/vega12_gpu_info.bin");
   MODULE_FIRMWARE("amdgpu/raven_gpu_info.bin");
 #define AMDGPU_RESUME_MS2000
+#define AMDGPU_IB_TEST_SCHED_MS2000
 static const char *amdgpu_asic_name[] = {
 "TAHITI",
@@ -2105,6 +2106,16 @@ bool amdgpu_device_asic_has_dc_support(enum
amd_asic_type asic_type)
 }
   }
   +static void amdgpu_device_late_init_test_ib_func_handler(struct
work_struct *work)
+{
+   struct amdgpu_device *adev =
+   container_of(work, struct amdgpu_device,
late_init_test_ib_work.work);
+   int r = amdgpu_ib_ring_tests(adev);
+
+   if (r)
+   DRM_ERROR("ib ring test failed (%d).\n", r);
+}
+
   /**
* amdgpu_device_has_dc_support - check if dc is supported
*
@@ -2212,6 +2223,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 INIT_LIST_HEAD(>ring_lru_list);
 spin_lock_init(>ring_lru_list_lock);
   + INIT_DELAYED_WORK(>late_init_test_ib_work,
+ amdgpu_device_late_init_test_ib_func_handler);
 INIT_DELAYED_WORK(>late_init_work,
   amdgpu_device_ip_late_init_func_handler);
   @@ -2374,9 +2387,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 goto failed;
 }
   - r = amdgpu_ib_ring_tests(adev);
-   if (r)
-   DRM_ERROR("ib ring test failed (%d).\n", r);
+   /* Schedule amdgpu_ib_ring_tests() */
+   queue_delayed_work(system_wq, >late_init_test_ib_work,
+   msecs_to_jiffies(AMDGPU_IB_TEST_SCHED_MS));
 if (amdgpu_sriov_vf(adev))
 amdgpu_virt_init_data_exchange(adev);
@@ -2469,6 +2482,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 }
 adev->accel_working = false;
 cancel_delayed_work_sync(>late_init_work);
+   cancel_delayed_work_sync(>late_init_test_ib_work);
 /* free i2c buses */
 if (!amdgpu_device_has_dc_support(adev))
 amdgpu_i2c_fini(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 487d39e..6fa326b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -279,6 +279,10 @@ static int amdgpu_info_ioctl(struct drm_device *dev,
void *data, struct drm_file
 if (!info->return_size || !info->return_pointer)
 return -EINVAL;
   + /* Ensure IB tests on ring are executed */
+   if (delayed_work_pending(>late_init_test_ib_work))
+   flush_delayed_work(>late_init_test_ib_work);
+


You just need to call flush_delayed_work() here without the if.

Regards,
Christian.


 switch (info->query) {
 case AMDGPU_INFO_ACCEL_WORKING:
 ui32 =