Re: [PATCH v2] drm/amdgpu: conditionally compile amdgpu's amdkfd files

[Felix Kuehling]

On 2018-05-18 05:57 PM, Oded Gabbay wrote:
>>> +struct kfd2kgd_calls *amdgpu_amdkfd_gfx_7_get_functions(void)
>>> +{
>>> + return NULL;
>> I think this will cause an oops in amdgpu_amdkfd_probe because that
>> function doesn't handle kgd2kfd == NULL. You could remove
>> amdgpu_amdkfd_gfx_*_get_functions and instead turn amdgpu_amdkfd_probe
>> itself into a stub to simplify this. That's the only place where
>> *_get_functions are called.
> Actually this function will never be called if amdkfd is not compiled,
> because amdgpu_amdkfd_probe will exit immediately due to the if
> statement at the start of that function (which checks kgd2kfd is not
> NULL).

Ah, I missed that.

>  I did see that kgd2kfd is not initialized to NULL in case
> amdkfd is not compiled and I fixed that in v3.

kgd2kfd is global uninitialized data. That should be implicitly
initialized to 0. I've got checkpatch warnings before for initializing
global variables to 0 explicitly. Anyway, doing the initialization
explicitly in the code like you did in v3 doesn't do any harm and it
makes things more clear.

Thanks,
  Felix

> I prefer not to add stub for probe because then I will have to #ifdef
> around the real probe function and I prefer to minimize #ifdefs
>
> Oded

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

Re: [PATCH v3] drm/amdgpu: conditionally compile amdgpu's amdkfd files

[Felix Kuehling]

On 2018-05-18 05:57 PM, Oded Gabbay wrote:
> In case CONFIG_HSA_AMD is not chosen, there is no need to compile amdkfd
> files that reside inside amdgpu dirver. In addition, because amdkfd
> depends on x86_64 architecture and amdgpu is not, compiling amdkfd files
> under i386 architecture can cause compiler errors and warnings.
>
> This patch modifies amdgpu's makefile to build amdkfd files only if
> CONFIG_HSA_AMD is chosen. The only file to be compiled unconditionally
> is amdgpu_amdkfd.c
>
> There are stub functions that are compiled only if amdkfd is not
> compiled. In that case, calls from amdgpu driver proper will go to those
> functions instead of the real functions.
>
> v2: instead of using function pointers, use stub functions
>
> v3: initialize kgd2kfd to NULL in case amdkfd is not compiled
>
> Signed-off-by: Oded Gabbay 
Reviewed-by: Felix Kuehling 

Thanks,
  Felix


> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile| 13 +++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 47 
> ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 12 
>  3 files changed, 63 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 68e9f584c570..e03ee41cedcb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -56,8 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>  
>  # add asic specific block
>  amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
> - ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o 
> vce_v2_0.o \
> - amdgpu_amdkfd_gfx_v7.o
> + ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
>  
>  amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o gfx_v6_0.o si_ih.o si_dma.o 
> dce_v6_0.o si_dpm.o si_smc.o
>  
> @@ -130,13 +129,21 @@ amdgpu-y += \
>   vcn_v1_0.o
>  
>  # add amdkfd interfaces
> +amdgpu-y += amdgpu_amdkfd.o
> +
> +ifneq ($(CONFIG_HSA_AMD),)
>  amdgpu-y += \
> -  amdgpu_amdkfd.o \
>amdgpu_amdkfd_fence.o \
>amdgpu_amdkfd_gpuvm.o \
>amdgpu_amdkfd_gfx_v8.o \
>amdgpu_amdkfd_gfx_v9.o
>  
> +ifneq ($(CONFIG_DRM_AMDGPU_CIK),)
> +amdgpu-y += amdgpu_amdkfd_gfx_v7.o
> +endif
> +
> +endif
> +
>  # add cgs
>  amdgpu-y += amdgpu_cgs.o
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index bd36ee9f7e6d..95fcbd8a4bf3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -50,15 +50,21 @@ int amdgpu_amdkfd_init(void)
>   kgd2kfd = NULL;
>   }
>  
> +
>  #elif defined(CONFIG_HSA_AMD)
> +
>   ret = kgd2kfd_init(KFD_INTERFACE_VERSION, );
>   if (ret)
>   kgd2kfd = NULL;
>  
>  #else
> + kgd2kfd = NULL;
>   ret = -ENOENT;
>  #endif
> +
> +#if defined(CONFIG_HSA_AMD_MODULE) || defined(CONFIG_HSA_AMD)
>   amdgpu_amdkfd_gpuvm_init_mem_limits();
> +#endif
>  
>   return ret;
>  }
> @@ -464,3 +470,44 @@ bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device 
> *adev, u32 vmid)
>  
>   return false;
>  }
> +
> +#if !defined(CONFIG_HSA_AMD_MODULE) && !defined(CONFIG_HSA_AMD)
> +bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm)
> +{
> + return false;
> +}
> +
> +void amdgpu_amdkfd_unreserve_system_memory_limit(struct amdgpu_bo *bo)
> +{
> +}
> +
> +void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
> + struct amdgpu_vm *vm)
> +{
> +}
> +
> +struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f)
> +{
> + return NULL;
> +}
> +
> +int amdgpu_amdkfd_evict_userptr(struct kgd_mem *mem, struct mm_struct *mm)
> +{
> + return 0;
> +}
> +
> +struct kfd2kgd_calls *amdgpu_amdkfd_gfx_7_get_functions(void)
> +{
> + return NULL;
> +}
> +
> +struct kfd2kgd_calls *amdgpu_amdkfd_gfx_8_0_get_functions(void)
> +{
> + return NULL;
> +}
> +
> +struct kfd2kgd_calls *amdgpu_amdkfd_gfx_9_0_get_functions(void)
> +{
> + return NULL;
> +}
> +#endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 12367a9951e8..a8418a3f4e9d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -156,14 +156,14 @@ uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev 
> *kgd);
>  
>  /* GPUVM API */
>  int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm,
> -   void **process_info,
> -   struct dma_fence **ef);
> + void **process_info,
> + struct dma_fence **ef);
>  int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
> -struct file *filp,
> -  

Re: [PATCH v2] drm/amdgpu: conditionally compile amdgpu's amdkfd files

[Oded Gabbay]

On Fri, May 18, 2018 at 11:06 PM, Felix Kuehling  wrote:
> Two more comments inline. One cosmetic, one real issue. With that fixed,
> this patch is Reviewed-by: Felix Kuehling 
>
> Regards,
>   Felix
>
> On 2018-05-18 03:42 PM, Oded Gabbay wrote:
>> In case CONFIG_HSA_AMD is not chosen, there is no need to compile amdkfd
>> files that reside inside amdgpu dirver. In addition, because amdkfd
>> depends on x86_64 architecture and amdgpu is not, compiling amdkfd files
>> under i386 architecture can cause compiler errors and warnings.
>>
>> This patch modifies amdgpu's makefile to build amdkfd files only if
>> CONFIG_HSA_AMD is chosen. The only file to be compiled unconditionally
>> is amdgpu_amdkfd.c
>>
>> Direct calls from amdgpu driver proper to functions in other
>> amdgpu_amdkfd_*.c files were changed to calls to functions inside
>> amdgpu_amdkfd.c. These functions call the original functions using a
>> function pointer to allow compilation without the original functions.
>
> The above paragraph needs to be updated.
Yeah, I'll send v3.
>
>>
>> v2:
>> Instead of using function pointers, use stub functions that are compiled
>> only if amdkfd is not compiled.
>>
>> Signed-off-by: Oded Gabbay 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/Makefile| 13 +++--
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 46 
>> ++
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 12 
>>  3 files changed, 62 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
>> b/drivers/gpu/drm/amd/amdgpu/Makefile
>> index 68e9f584c570..e03ee41cedcb 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>> @@ -56,8 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>>
>>  # add asic specific block
>>  amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
>> - ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o 
>> vce_v2_0.o \
>> - amdgpu_amdkfd_gfx_v7.o
>> + ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o 
>> vce_v2_0.o
>>
>>  amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o gfx_v6_0.o si_ih.o 
>> si_dma.o dce_v6_0.o si_dpm.o si_smc.o
>>
>> @@ -130,13 +129,21 @@ amdgpu-y += \
>>   vcn_v1_0.o
>>
>>  # add amdkfd interfaces
>> +amdgpu-y += amdgpu_amdkfd.o
>> +
>> +ifneq ($(CONFIG_HSA_AMD),)
>>  amdgpu-y += \
>> -  amdgpu_amdkfd.o \
>>amdgpu_amdkfd_fence.o \
>>amdgpu_amdkfd_gpuvm.o \
>>amdgpu_amdkfd_gfx_v8.o \
>>amdgpu_amdkfd_gfx_v9.o
>>
>> +ifneq ($(CONFIG_DRM_AMDGPU_CIK),)
>> +amdgpu-y += amdgpu_amdkfd_gfx_v7.o
>> +endif
>> +
>> +endif
>> +
>>  # add cgs
>>  amdgpu-y += amdgpu_cgs.o
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> index bd36ee9f7e6d..b0b39bd83f0f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> @@ -50,7 +50,9 @@ int amdgpu_amdkfd_init(void)
>>   kgd2kfd = NULL;
>>   }
>>
>> +
>>  #elif defined(CONFIG_HSA_AMD)
>> +
>>   ret = kgd2kfd_init(KFD_INTERFACE_VERSION, );
>>   if (ret)
>>   kgd2kfd = NULL;
>> @@ -58,7 +60,10 @@ int amdgpu_amdkfd_init(void)
>>  #else
>>   ret = -ENOENT;
>>  #endif
>> +
>> +#if defined(CONFIG_HSA_AMD_MODULE) || defined(CONFIG_HSA_AMD)
>>   amdgpu_amdkfd_gpuvm_init_mem_limits();
>> +#endif
>>
>>   return ret;
>>  }
>> @@ -464,3 +469,44 @@ bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device 
>> *adev, u32 vmid)
>>
>>   return false;
>>  }
>> +
>> +#if !defined(CONFIG_HSA_AMD_MODULE) && !defined(CONFIG_HSA_AMD)
>> +bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm)
>> +{
>> + return false;
>> +}
>> +
>> +void amdgpu_amdkfd_unreserve_system_memory_limit(struct amdgpu_bo *bo)
>> +{
>> +}
>> +
>> +void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
>> + struct amdgpu_vm *vm)
>> +{
>> +}
>> +
>> +struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f)
>> +{
>> + return NULL;
>> +}
>> +
>> +int amdgpu_amdkfd_evict_userptr(struct kgd_mem *mem, struct mm_struct *mm)
>> +{
>> + return 0;
>> +}
>> +
>> +struct kfd2kgd_calls *amdgpu_amdkfd_gfx_7_get_functions(void)
>> +{
>> + return NULL;
>
> I think this will cause an oops in amdgpu_amdkfd_probe because that
> function doesn't handle kgd2kfd == NULL. You could remove
> amdgpu_amdkfd_gfx_*_get_functions and instead turn amdgpu_amdkfd_probe
> itself into a stub to simplify this. That's the only place where
> *_get_functions are called.

Actually this function will never be called if amdkfd is not compiled,
because amdgpu_amdkfd_probe will exit immediately due to the if
statement at the start of that function (which checks kgd2kfd is not
NULL). I did see that kgd2kfd is not initialized to NULL in case
amdkfd is not 

[PATCH v3] drm/amdgpu: conditionally compile amdgpu's amdkfd files

[Oded Gabbay]

In case CONFIG_HSA_AMD is not chosen, there is no need to compile amdkfd
files that reside inside amdgpu dirver. In addition, because amdkfd
depends on x86_64 architecture and amdgpu is not, compiling amdkfd files
under i386 architecture can cause compiler errors and warnings.

This patch modifies amdgpu's makefile to build amdkfd files only if
CONFIG_HSA_AMD is chosen. The only file to be compiled unconditionally
is amdgpu_amdkfd.c

There are stub functions that are compiled only if amdkfd is not
compiled. In that case, calls from amdgpu driver proper will go to those
functions instead of the real functions.

v2: instead of using function pointers, use stub functions

v3: initialize kgd2kfd to NULL in case amdkfd is not compiled

Signed-off-by: Oded Gabbay 
---
 drivers/gpu/drm/amd/amdgpu/Makefile| 13 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 47 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 12 
 3 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 68e9f584c570..e03ee41cedcb 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -56,8 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 
 # add asic specific block
 amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
-   ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o 
vce_v2_0.o \
-   amdgpu_amdkfd_gfx_v7.o
+   ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
 
 amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o gfx_v6_0.o si_ih.o si_dma.o 
dce_v6_0.o si_dpm.o si_smc.o
 
@@ -130,13 +129,21 @@ amdgpu-y += \
vcn_v1_0.o
 
 # add amdkfd interfaces
+amdgpu-y += amdgpu_amdkfd.o
+
+ifneq ($(CONFIG_HSA_AMD),)
 amdgpu-y += \
-amdgpu_amdkfd.o \
 amdgpu_amdkfd_fence.o \
 amdgpu_amdkfd_gpuvm.o \
 amdgpu_amdkfd_gfx_v8.o \
 amdgpu_amdkfd_gfx_v9.o
 
+ifneq ($(CONFIG_DRM_AMDGPU_CIK),)
+amdgpu-y += amdgpu_amdkfd_gfx_v7.o
+endif
+
+endif
+
 # add cgs
 amdgpu-y += amdgpu_cgs.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index bd36ee9f7e6d..95fcbd8a4bf3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -50,15 +50,21 @@ int amdgpu_amdkfd_init(void)
kgd2kfd = NULL;
}
 
+
 #elif defined(CONFIG_HSA_AMD)
+
ret = kgd2kfd_init(KFD_INTERFACE_VERSION, );
if (ret)
kgd2kfd = NULL;
 
 #else
+   kgd2kfd = NULL;
ret = -ENOENT;
 #endif
+
+#if defined(CONFIG_HSA_AMD_MODULE) || defined(CONFIG_HSA_AMD)
amdgpu_amdkfd_gpuvm_init_mem_limits();
+#endif
 
return ret;
 }
@@ -464,3 +470,44 @@ bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device *adev, 
u32 vmid)
 
return false;
 }
+
+#if !defined(CONFIG_HSA_AMD_MODULE) && !defined(CONFIG_HSA_AMD)
+bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm)
+{
+   return false;
+}
+
+void amdgpu_amdkfd_unreserve_system_memory_limit(struct amdgpu_bo *bo)
+{
+}
+
+void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
+   struct amdgpu_vm *vm)
+{
+}
+
+struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f)
+{
+   return NULL;
+}
+
+int amdgpu_amdkfd_evict_userptr(struct kgd_mem *mem, struct mm_struct *mm)
+{
+   return 0;
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_7_get_functions(void)
+{
+   return NULL;
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_8_0_get_functions(void)
+{
+   return NULL;
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_9_0_get_functions(void)
+{
+   return NULL;
+}
+#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 12367a9951e8..a8418a3f4e9d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -156,14 +156,14 @@ uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev 
*kgd);
 
 /* GPUVM API */
 int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm,
- void **process_info,
- struct dma_fence **ef);
+   void **process_info,
+   struct dma_fence **ef);
 int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
-  struct file *filp,
-  void **vm, void **process_info,
-  struct dma_fence **ef);
+   struct file *filp,
+   void **vm, void **process_info,
+   struct dma_fence **ef);
 void 

Re: [PATCH 1/1] drm/amdgpu: add kernel doc for amdgpu_object.c

[Samuel Li]

On 2018-05-18 02:54 PM, Alex Deucher wrote:
> On Thu, May 17, 2018 at 6:20 PM, Samuel Li  wrote:
>> Signed-off-by: Samuel Li 
> 
> Please also add a separate DOC section at the top of this file giving
> a brief overview of how amdgpu bo API works.  E.g., how you would
> allocate, free, kmap, etc.  What pinning means and why you would need
> to do it, etc.  What domains we support and what they are.  What
> shadow bos are.
OK. For domains, I can create a separate change to add to "amdgpu_drm.h", in 
which they were defined.
For pinning/shadow bo, I can add it before the respective functions.

Sam


> 
> Additional comments below for things to add or clarify to enhance the
> documentation.
> 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 233 
>> +
>>  1 file changed, 233 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 6a9e46a..271dbfa 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -73,6 +73,15 @@ static void amdgpu_ttm_bo_destroy(struct 
>> ttm_buffer_object *tbo)
>> kfree(bo);
>>  }
>>
>> +/**
>> + * amdgpu_ttm_bo_is_amdgpu_bo - if the buffer object belongs to an 
>> _bo
>> + * @bo: buffer object to be checked
>> + *
>> + * Uses destroy function associated with the object to determine if this is
>> + * _bo.
>> + *
>> + * Returns true if the object belongs to _bo, false if not.
>> + */
> 
> Just to clarify, it checks whether a ttm bo is an amdgpu buffer object or not.
> 
>>  bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
>>  {
>> if (bo->destroy == _ttm_bo_destroy)
>> @@ -80,6 +89,14 @@ bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object 
>> *bo)
>> return false;
>>  }
>>
>> +/**
>> + * amdgpu_ttm_placement_from_domain - set buffer's placement
>> + * @abo: _bo buffer object whose placement is to be set
>> + * @domain: requested domain
>> + *
>> + * Sets buffer's placement according to requested domain and the buffer's
>> + * flags.
>> + */
>>  void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
>>  {
>> struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
>> @@ -498,6 +515,17 @@ static int amdgpu_bo_create_shadow(struct amdgpu_device 
>> *adev,
>> return r;
>>  }
>>
>> +/**
>> + * amdgpu_bo_create - create an _bo buffer object
>> + * @adev: amdgpu device object
>> + * @bp: parameters to be used for the buffer object
>> + * @bo_ptr: pointer to the buffer object pointer
>> + *
>> + * Creates an _bo buffer object; and if requested, also creates a
>> + * shadow object.
>> + *
>> + * Returns 0 for success or a negative error code on failure.
>> + */
>>  int amdgpu_bo_create(struct amdgpu_device *adev,
>>  struct amdgpu_bo_param *bp,
>>  struct amdgpu_bo **bo_ptr)
>> @@ -527,6 +555,20 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>> return r;
>>  }
>>
>> +/**
>> + * amdgpu_bo_backup_to_shadow - Backs up an _bo buffer object
>> + * @adev: amdgpu device object
>> + * @ring: amdgpu_ring for the engine handling the buffer operations
>> + * @bo: _bo buffer to be backed up
>> + * @resv: reservation object with embedded fence
>> + * @fence: dma_fence associated with the operation
>> + * @direct: whether to submit the job directly
>> + *
>> + * Copies an _bo buffer object to its shadow object.
>> + * Not used for now.
>> + *
>> + * Returns 0 for success or a negative error code on failure.
>> + */
>>  int amdgpu_bo_backup_to_shadow(struct amdgpu_device *adev,
>>struct amdgpu_ring *ring,
>>struct amdgpu_bo *bo,
>> @@ -559,6 +601,15 @@ int amdgpu_bo_backup_to_shadow(struct amdgpu_device 
>> *adev,
>> return r;
>>  }
>>
>> +/**
>> + * amdgpu_bo_validate - validate an _bo buffer object
>> + * @bo: pointer to the buffer object
>> + *
>> + * Sets placement according to domain; and changes placement and caching
>> + * policy of the buffer object according to the placement.
>> + *
>> + * Returns 0 for success or a negative error code on failure.
>> + */
> 
> This is used for validating shadow bos.  Also worth mentioning that it
> calls ttm_bo_validate() which will make sure the buffer is resident
> where it needs to be.
> 
>>  int amdgpu_bo_validate(struct amdgpu_bo *bo)
>>  {
>> struct ttm_operation_ctx ctx = { false, false };
>> @@ -581,6 +632,19 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)
>> return r;
>>  }
>>
>> +/**
>> + * amdgpu_bo_restore_from_shadow - restore an _bo buffer object
>> + * @adev: amdgpu device object
>> + * @ring: amdgpu_ring for the engine handling the buffer operations
>> + * @bo: _bo buffer to be restored
>> + * @resv: reservation object with embedded fence
>> + * @fence: dma_fence associated with the operation
>> + * @direct: whether to submit 

Re: [PATCH v2] drm/amdgpu: conditionally compile amdgpu's amdkfd files

[Felix Kuehling]

Two more comments inline. One cosmetic, one real issue. With that fixed,
this patch is Reviewed-by: Felix Kuehling 

Regards,
  Felix

On 2018-05-18 03:42 PM, Oded Gabbay wrote:
> In case CONFIG_HSA_AMD is not chosen, there is no need to compile amdkfd
> files that reside inside amdgpu dirver. In addition, because amdkfd
> depends on x86_64 architecture and amdgpu is not, compiling amdkfd files
> under i386 architecture can cause compiler errors and warnings.
>
> This patch modifies amdgpu's makefile to build amdkfd files only if
> CONFIG_HSA_AMD is chosen. The only file to be compiled unconditionally
> is amdgpu_amdkfd.c
>
> Direct calls from amdgpu driver proper to functions in other
> amdgpu_amdkfd_*.c files were changed to calls to functions inside
> amdgpu_amdkfd.c. These functions call the original functions using a
> function pointer to allow compilation without the original functions.

The above paragraph needs to be updated.

>
> v2:
> Instead of using function pointers, use stub functions that are compiled
> only if amdkfd is not compiled.
>
> Signed-off-by: Oded Gabbay 
> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile| 13 +++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 46 
> ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 12 
>  3 files changed, 62 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 68e9f584c570..e03ee41cedcb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -56,8 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>  
>  # add asic specific block
>  amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
> - ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o 
> vce_v2_0.o \
> - amdgpu_amdkfd_gfx_v7.o
> + ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
>  
>  amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o gfx_v6_0.o si_ih.o si_dma.o 
> dce_v6_0.o si_dpm.o si_smc.o
>  
> @@ -130,13 +129,21 @@ amdgpu-y += \
>   vcn_v1_0.o
>  
>  # add amdkfd interfaces
> +amdgpu-y += amdgpu_amdkfd.o
> +
> +ifneq ($(CONFIG_HSA_AMD),)
>  amdgpu-y += \
> -  amdgpu_amdkfd.o \
>amdgpu_amdkfd_fence.o \
>amdgpu_amdkfd_gpuvm.o \
>amdgpu_amdkfd_gfx_v8.o \
>amdgpu_amdkfd_gfx_v9.o
>  
> +ifneq ($(CONFIG_DRM_AMDGPU_CIK),)
> +amdgpu-y += amdgpu_amdkfd_gfx_v7.o
> +endif
> +
> +endif
> +
>  # add cgs
>  amdgpu-y += amdgpu_cgs.o
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index bd36ee9f7e6d..b0b39bd83f0f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -50,7 +50,9 @@ int amdgpu_amdkfd_init(void)
>   kgd2kfd = NULL;
>   }
>  
> +
>  #elif defined(CONFIG_HSA_AMD)
> +
>   ret = kgd2kfd_init(KFD_INTERFACE_VERSION, );
>   if (ret)
>   kgd2kfd = NULL;
> @@ -58,7 +60,10 @@ int amdgpu_amdkfd_init(void)
>  #else
>   ret = -ENOENT;
>  #endif
> +
> +#if defined(CONFIG_HSA_AMD_MODULE) || defined(CONFIG_HSA_AMD)
>   amdgpu_amdkfd_gpuvm_init_mem_limits();
> +#endif
>  
>   return ret;
>  }
> @@ -464,3 +469,44 @@ bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device 
> *adev, u32 vmid)
>  
>   return false;
>  }
> +
> +#if !defined(CONFIG_HSA_AMD_MODULE) && !defined(CONFIG_HSA_AMD)
> +bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm)
> +{
> + return false;
> +}
> +
> +void amdgpu_amdkfd_unreserve_system_memory_limit(struct amdgpu_bo *bo)
> +{
> +}
> +
> +void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
> + struct amdgpu_vm *vm)
> +{
> +}
> +
> +struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f)
> +{
> + return NULL;
> +}
> +
> +int amdgpu_amdkfd_evict_userptr(struct kgd_mem *mem, struct mm_struct *mm)
> +{
> + return 0;
> +}
> +
> +struct kfd2kgd_calls *amdgpu_amdkfd_gfx_7_get_functions(void)
> +{
> + return NULL;

I think this will cause an oops in amdgpu_amdkfd_probe because that
function doesn't handle kgd2kfd == NULL. You could remove
amdgpu_amdkfd_gfx_*_get_functions and instead turn amdgpu_amdkfd_probe
itself into a stub to simplify this. That's the only place where
*_get_functions are called.

> +}
> +
> +struct kfd2kgd_calls *amdgpu_amdkfd_gfx_8_0_get_functions(void)
> +{
> + return NULL;
> +}
> +
> +struct kfd2kgd_calls *amdgpu_amdkfd_gfx_9_0_get_functions(void)
> +{
> + return NULL;
> +}
> +#endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 12367a9951e8..a8418a3f4e9d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -156,14 +156,14 @@ uint64_t 

Re: [PATCH] drm/amd/pp: Fix static checker warning

[Alex Deucher]

On Fri, May 18, 2018 at 2:28 AM, Rex Zhu  wrote:
> error: uninitialized symbol ''
>
> Signed-off-by: Rex Zhu 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c   | 24 
> +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  3 ++-
>  drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c |  6 ++
>  3 files changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> index ec38c9f..7047e29 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> @@ -1104,10 +1104,8 @@ int atomctrl_get_voltage_evv_on_sclk(
> GetIndexIntoMasterTable(COMMAND, GetVoltageInfo),
> (uint32_t *)_voltage_info_param_space);
>
> -   if (0 != result)
> -   return result;
> -
> -   *voltage = le16_to_cpu(((GET_EVV_VOLTAGE_INFO_OUTPUT_PARAMETER_V1_2 *)
> +   *voltage = result ? 0 :
> +   
> le16_to_cpu(((GET_EVV_VOLTAGE_INFO_OUTPUT_PARAMETER_V1_2 *)
> 
> (_voltage_info_param_space))->usVoltageLevel);
>
> return result;
> @@ -1312,8 +1310,7 @@ int atomctrl_read_efuse(struct pp_hwmgr *hwmgr, 
> uint16_t start_index,
> result = amdgpu_atom_execute_table(adev->mode_info.atom_context,
> GetIndexIntoMasterTable(COMMAND, ReadEfuseValue),
> (uint32_t *)_param);
> -   if (!result)
> -   *efuse = le32_to_cpu(efuse_param.ulEfuseValue) & mask;
> +   *efuse = result ? 0 : le32_to_cpu(efuse_param.ulEfuseValue) & mask;
>
> return result;
>  }
> @@ -1354,11 +1351,8 @@ int atomctrl_get_voltage_evv_on_sclk_ai(struct 
> pp_hwmgr *hwmgr, uint8_t voltage_
> GetIndexIntoMasterTable(COMMAND, GetVoltageInfo),
> (uint32_t *)_voltage_info_param_space);
>
> -   if (0 != result)
> -   return result;
> -
> -   *voltage = le32_to_cpu(((GET_EVV_VOLTAGE_INFO_OUTPUT_PARAMETER_V1_3 *)
> -   
> (_voltage_info_param_space))->ulVoltageLevel);
> +   *voltage = result ? 0 :
> +   le32_to_cpu(((GET_EVV_VOLTAGE_INFO_OUTPUT_PARAMETER_V1_3 
> *)(_voltage_info_param_space))->ulVoltageLevel);
>
> return result;
>  }
> @@ -1552,15 +1546,17 @@ void atomctrl_get_voltage_range(struct pp_hwmgr 
> *hwmgr, uint32_t *max_vddc,
> case CHIP_FIJI:
> *max_vddc = 
> le32_to_cpu(((ATOM_ASIC_PROFILING_INFO_V3_3 *)profile)->ulMaxVddc/4);
> *min_vddc = 
> le32_to_cpu(((ATOM_ASIC_PROFILING_INFO_V3_3 *)profile)->ulMinVddc/4);
> -   break;
> +   return;
> case CHIP_POLARIS11:
> case CHIP_POLARIS10:
> case CHIP_POLARIS12:
> *max_vddc = 
> le32_to_cpu(((ATOM_ASIC_PROFILING_INFO_V3_6 *)profile)->ulMaxVddc/100);
> *min_vddc = 
> le32_to_cpu(((ATOM_ASIC_PROFILING_INFO_V3_6 *)profile)->ulMinVddc/100);
> -   break;
> -   default:
> return;
> +   default:
> +   break;
> }
> }
> +   *max_vddc = 0;
> +   *min_vddc = 0;
>  }
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 646c9e9..45e9b8c 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -860,7 +860,8 @@ static void smu7_setup_voltage_range_from_vbios(struct 
> pp_hwmgr *hwmgr)
> struct phm_ppt_v1_clock_voltage_dependency_table *dep_sclk_table;
> struct phm_ppt_v1_information *table_info =
> (struct phm_ppt_v1_information *)(hwmgr->pptable);
> -   uint32_t min_vddc, max_vddc;
> +   uint32_t min_vddc = 0;
> +   uint32_t max_vddc = 0;
>
> if (!table_info)
> return;
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c 
> b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> index 64d33b7..d644a9b 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> @@ -283,11 +283,9 @@ int smu7_read_smc_sram_dword(struct pp_hwmgr *hwmgr, 
> uint32_t smc_addr, uint32_t
>
> result = smu7_set_smc_sram_address(hwmgr, smc_addr, limit);
>
> -   if (result)
> -   return result;
> +   *value = result ? 0 : cgs_read_register(hwmgr->device, 
> mmSMC_IND_DATA_11);
>
> -   *value = cgs_read_register(hwmgr->device, mmSMC_IND_DATA_11);
> -   return 0;
> +   return result;
>  }
>
>  int 

Re: [PATCH] drm/amd/pp: fix a couple locking issues

[Alex Deucher]

On Fri, May 18, 2018 at 3:06 AM, Rex Zhu  wrote:
> We should return unlock on the error path
>
> Signed-off-by: Rex Zhu 

Reviewed-by: Alex Deucher 


> ---
>  .../gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c   | 31 
> +-
>  1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c
> index 99b29ff..c952845 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c
> @@ -936,45 +936,49 @@ int smu7_enable_didt_config(struct pp_hwmgr *hwmgr)
>
> if (hwmgr->chip_id == CHIP_POLARIS10) {
> result = 
> smu7_program_pt_config_registers(hwmgr, GCCACConfig_Polaris10);
> -   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", return result);
> +   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", goto error);
> result = 
> smu7_program_pt_config_registers(hwmgr, DIDTConfig_Polaris10);
> -   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", return result);
> +   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", goto error);
> } else if (hwmgr->chip_id == CHIP_POLARIS11) {
> result = 
> smu7_program_pt_config_registers(hwmgr, GCCACConfig_Polaris11);
> -   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", return result);
> +   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", goto error);
> if (hwmgr->is_kicker)
> result = 
> smu7_program_pt_config_registers(hwmgr, DIDTConfig_Polaris11_Kicker);
> else
> result = 
> smu7_program_pt_config_registers(hwmgr, DIDTConfig_Polaris11);
> -   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", return result);
> +   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", goto error);
> } else if (hwmgr->chip_id == CHIP_POLARIS12) {
> result = 
> smu7_program_pt_config_registers(hwmgr, GCCACConfig_Polaris11);
> -   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", return result);
> +   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", goto error);
> result = 
> smu7_program_pt_config_registers(hwmgr, DIDTConfig_Polaris12);
> -   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", return result);
> +   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", goto error);
> } else if (hwmgr->chip_id == CHIP_VEGAM) {
> result = 
> smu7_program_pt_config_registers(hwmgr, GCCACConfig_VegaM);
> -   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", return result);
> +   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", goto error);
> result = 
> smu7_program_pt_config_registers(hwmgr, DIDTConfig_VegaM);
> -   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", return result);
> +   PP_ASSERT_WITH_CODE((result == 0), "DIDT 
> Config failed.", goto error);
> }
> }
> cgs_write_register(hwmgr->device, mmGRBM_GFX_INDEX, value2);
>
> result = smu7_enable_didt(hwmgr, true);
> -   PP_ASSERT_WITH_CODE((result == 0), "EnableDiDt failed.", 
> return result);
> +   PP_ASSERT_WITH_CODE((result == 0), "EnableDiDt failed.", goto 
> error);
>
> if (hwmgr->chip_id == CHIP_POLARIS11) {
> result = smum_send_msg_to_smc(hwmgr,
> 
> (uint16_t)(PPSMC_MSG_EnableDpmDidt));
> PP_ASSERT_WITH_CODE((0 == result),
> -   "Failed to enable DPM DIDT.", return 
> result);
> +   "Failed to enable DPM DIDT.", goto 
> error);
> }
> mutex_unlock(>grbm_idx_mutex);
> adev->gfx.rlc.funcs->exit_safe_mode(adev);
> }
>
> return 0;
> +error:
> +   mutex_unlock(>grbm_idx_mutex);
> +   adev->gfx.rlc.funcs->exit_safe_mode(adev);
> +   return result;
>  }
>
>  int 

Re: [PATCH 1/4] drm/amdgpu: add new DF 1.7 register defs

[Alex Deucher]

Ping?

On Wed, May 16, 2018 at 4:51 PM, Alex Deucher  wrote:
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/include/asic_reg/df/df_1_7_offset.h  | 4 
>  drivers/gpu/drm/amd/include/asic_reg/df/df_1_7_sh_mask.h | 4 
>  2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/include/asic_reg/df/df_1_7_offset.h 
> b/drivers/gpu/drm/amd/include/asic_reg/df/df_1_7_offset.h
> index 2b305dd021e8..e6044e27a913 100644
> --- a/drivers/gpu/drm/amd/include/asic_reg/df/df_1_7_offset.h
> +++ b/drivers/gpu/drm/amd/include/asic_reg/df/df_1_7_offset.h
> @@ -30,4 +30,8 @@
>  #define mmDF_CS_AON0_DramBaseAddress0
>   0x0044
>  #define mmDF_CS_AON0_DramBaseAddress0_BASE_IDX   
>   0
>
> +#define mmDF_CS_AON0_CoherentSlaveModeCtrlA0 
>   0x0214
> +#define mmDF_CS_AON0_CoherentSlaveModeCtrlA0_BASE_IDX
>   0
> +
> +
>  #endif
> diff --git a/drivers/gpu/drm/amd/include/asic_reg/df/df_1_7_sh_mask.h 
> b/drivers/gpu/drm/amd/include/asic_reg/df/df_1_7_sh_mask.h
> index 2ba849798924..a78c99480e2d 100644
> --- a/drivers/gpu/drm/amd/include/asic_reg/df/df_1_7_sh_mask.h
> +++ b/drivers/gpu/drm/amd/include/asic_reg/df/df_1_7_sh_mask.h
> @@ -45,4 +45,8 @@
>  #define DF_CS_AON0_DramBaseAddress0__IntLvAddrSel_MASK   
>   0x0700L
>  #define DF_CS_AON0_DramBaseAddress0__DramBaseAddr_MASK   
>   0xF000L
>
> +//DF_CS_AON0_CoherentSlaveModeCtrlA0
> +#define DF_CS_AON0_CoherentSlaveModeCtrlA0__ForceParWrRMW__SHIFT 
>   0x3
> +#define DF_CS_AON0_CoherentSlaveModeCtrlA0__ForceParWrRMW_MASK   
>   0x0008L
> +
>  #endif
> --
> 2.13.6
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2] drm/amdgpu: conditionally compile amdgpu's amdkfd files

[Oded Gabbay]

In case CONFIG_HSA_AMD is not chosen, there is no need to compile amdkfd
files that reside inside amdgpu dirver. In addition, because amdkfd
depends on x86_64 architecture and amdgpu is not, compiling amdkfd files
under i386 architecture can cause compiler errors and warnings.

This patch modifies amdgpu's makefile to build amdkfd files only if
CONFIG_HSA_AMD is chosen. The only file to be compiled unconditionally
is amdgpu_amdkfd.c

Direct calls from amdgpu driver proper to functions in other
amdgpu_amdkfd_*.c files were changed to calls to functions inside
amdgpu_amdkfd.c. These functions call the original functions using a
function pointer to allow compilation without the original functions.

v2:
Instead of using function pointers, use stub functions that are compiled
only if amdkfd is not compiled.

Signed-off-by: Oded Gabbay 
---
 drivers/gpu/drm/amd/amdgpu/Makefile| 13 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 46 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 12 
 3 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 68e9f584c570..e03ee41cedcb 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -56,8 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 
 # add asic specific block
 amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
-   ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o 
vce_v2_0.o \
-   amdgpu_amdkfd_gfx_v7.o
+   ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
 
 amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o gfx_v6_0.o si_ih.o si_dma.o 
dce_v6_0.o si_dpm.o si_smc.o
 
@@ -130,13 +129,21 @@ amdgpu-y += \
vcn_v1_0.o
 
 # add amdkfd interfaces
+amdgpu-y += amdgpu_amdkfd.o
+
+ifneq ($(CONFIG_HSA_AMD),)
 amdgpu-y += \
-amdgpu_amdkfd.o \
 amdgpu_amdkfd_fence.o \
 amdgpu_amdkfd_gpuvm.o \
 amdgpu_amdkfd_gfx_v8.o \
 amdgpu_amdkfd_gfx_v9.o
 
+ifneq ($(CONFIG_DRM_AMDGPU_CIK),)
+amdgpu-y += amdgpu_amdkfd_gfx_v7.o
+endif
+
+endif
+
 # add cgs
 amdgpu-y += amdgpu_cgs.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index bd36ee9f7e6d..b0b39bd83f0f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -50,7 +50,9 @@ int amdgpu_amdkfd_init(void)
kgd2kfd = NULL;
}
 
+
 #elif defined(CONFIG_HSA_AMD)
+
ret = kgd2kfd_init(KFD_INTERFACE_VERSION, );
if (ret)
kgd2kfd = NULL;
@@ -58,7 +60,10 @@ int amdgpu_amdkfd_init(void)
 #else
ret = -ENOENT;
 #endif
+
+#if defined(CONFIG_HSA_AMD_MODULE) || defined(CONFIG_HSA_AMD)
amdgpu_amdkfd_gpuvm_init_mem_limits();
+#endif
 
return ret;
 }
@@ -464,3 +469,44 @@ bool amdgpu_amdkfd_is_kfd_vmid(struct amdgpu_device *adev, 
u32 vmid)
 
return false;
 }
+
+#if !defined(CONFIG_HSA_AMD_MODULE) && !defined(CONFIG_HSA_AMD)
+bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm)
+{
+   return false;
+}
+
+void amdgpu_amdkfd_unreserve_system_memory_limit(struct amdgpu_bo *bo)
+{
+}
+
+void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
+   struct amdgpu_vm *vm)
+{
+}
+
+struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f)
+{
+   return NULL;
+}
+
+int amdgpu_amdkfd_evict_userptr(struct kgd_mem *mem, struct mm_struct *mm)
+{
+   return 0;
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_7_get_functions(void)
+{
+   return NULL;
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_8_0_get_functions(void)
+{
+   return NULL;
+}
+
+struct kfd2kgd_calls *amdgpu_amdkfd_gfx_9_0_get_functions(void)
+{
+   return NULL;
+}
+#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 12367a9951e8..a8418a3f4e9d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -156,14 +156,14 @@ uint64_t amdgpu_amdkfd_get_vram_usage(struct kgd_dev 
*kgd);
 
 /* GPUVM API */
 int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm,
- void **process_info,
- struct dma_fence **ef);
+   void **process_info,
+   struct dma_fence **ef);
 int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
-  struct file *filp,
-  void **vm, void **process_info,
-  struct dma_fence **ef);
+   struct file *filp,
+   void **vm, void 

Re: [PATCH] drm/amdgpu: consider user preference when pinning for SG display

[Alex Deucher]

On Fri, May 18, 2018 at 2:22 PM, Samuel Li  wrote:
>
>
> On 2018-05-18 04:21 AM, Michel Dänzer wrote:
>> On 2018-05-17 06:55 PM, Alex Deucher wrote:
>>> If the pin domain is set to GTT | VRAM, look at the preferred domains
>>> for the bo and respect that if it's been set explicitly.
>>>
>>> Signed-off-by: Alex Deucher 
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index 6a9e46ae7f0a..16192f17653e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -704,9 +704,14 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
>>> domain,
>>>   * See function amdgpu_display_supported_domains()
>>>   */
>>>  if (domain == (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
>>> -domain = AMDGPU_GEM_DOMAIN_VRAM;
>>> -if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>>> +if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM)
>>> +domain = AMDGPU_GEM_DOMAIN_VRAM; /* if user really 
>>> wants vram, respect it */
>>> +else if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_GTT)
>>> +domain = AMDGPU_GEM_DOMAIN_GTT; /* if user really 
>>> wants gtt, respect it */
>>
>> I'd spell VRAM and GTT in capital letters in the comments.
>>
>>
>>> +else if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>>>  domain = AMDGPU_GEM_DOMAIN_GTT;
>>> +else
>>> +domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>  }
>>
>> Is everything in place to deal with any issues that might occur when
>> flipping between buffers in VRAM and GTT?
>>
> Besides this question, I am also wondering how to deal with the first display 
> buffer created in kernel at amdgpufb_create_pinned_object()?

amdgpufb_create_pinned_object() calls
amdgpu_display_supported_domains() for the initial domain and
amdgpu_gem_object_create() uses that for the preferred domain so it
won't change the behavior there.

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

Re: [PATCH] drm/amdgpu: consider user preference when pinning for SG display

[Alex Deucher]

On Fri, May 18, 2018 at 4:21 AM, Michel Dänzer  wrote:
> On 2018-05-17 06:55 PM, Alex Deucher wrote:
>> If the pin domain is set to GTT | VRAM, look at the preferred domains
>> for the bo and respect that if it's been set explicitly.
>>
>> Signed-off-by: Alex Deucher 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 6a9e46ae7f0a..16192f17653e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -704,9 +704,14 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
>> domain,
>>* See function amdgpu_display_supported_domains()
>>*/
>>   if (domain == (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
>> - domain = AMDGPU_GEM_DOMAIN_VRAM;
>> - if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>> + if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM)
>> + domain = AMDGPU_GEM_DOMAIN_VRAM; /* if user really 
>> wants vram, respect it */
>> + else if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_GTT)
>> + domain = AMDGPU_GEM_DOMAIN_GTT; /* if user really 
>> wants gtt, respect it */
>
> I'd spell VRAM and GTT in capital letters in the comments.

OK.

>
>
>> + else if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>>   domain = AMDGPU_GEM_DOMAIN_GTT;
>> + else
>> + domain = AMDGPU_GEM_DOMAIN_VRAM;
>>   }
>
> Is everything in place to deal with any issues that might occur when
> flipping between buffers in VRAM and GTT?
>

Ah, right, I forgot about that, we still need to do that.  In practice
I don't think this will change anything compared things now at least
the way mesa is currently configured.  I can hold off on this this
patch for now until we handle that if you think it will be an issue.

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

Re: [PATCH 2/2] drm/amdgpu: Auto switch to video profiling mode on VCN demand

[Alex Deucher]

On Fri, May 18, 2018 at 5:33 AM, Rex Zhu  wrote:

Add a commit message.  Something like:
Switch to the VIDEO power profiles when VCN is active which is the
power profile optimized for video playback.
With that fixed:
Reviewed-by: Alex Deucher 

Want to take a stab at doing similar patches for UVD and VCE?

Also, does the smu/driver properly handle profile arbitration.  E.g.,
if VR profile is active and the user runs VCN which switches to the
VIDEO profile, we should stay in VR profile.  I think vg10 and RV do
the right thing because the SMU handles it, but on older parts we
probably need to double check.

Alex

> Signed-off-by: Rex Zhu 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 3549481..94b221f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -207,11 +207,13 @@ static void amdgpu_vcn_idle_work_handler(struct 
> work_struct *work)
> unsigned fences = amdgpu_fence_count_emitted(>vcn.ring_dec);
>
> if (fences == 0) {
> -   if (adev->pm.dpm_enabled)
> +   if (adev->pm.dpm_enabled) {
> +   amdgpu_dpm_switch_power_profile(adev, 
> PP_SMC_POWER_PROFILE_VIDEO, false);
> amdgpu_dpm_enable_uvd(adev, false);
> -   else
> +   } else {
> amdgpu_device_ip_set_powergating_state(adev, 
> AMD_IP_BLOCK_TYPE_VCN,
>
> AMD_PG_STATE_GATE);
> +   }
> } else {
> schedule_delayed_work(>vcn.idle_work, VCN_IDLE_TIMEOUT);
> }
> @@ -223,11 +225,13 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
> bool set_clocks = !cancel_delayed_work_sync(>vcn.idle_work);
>
> if (set_clocks && adev->pm.dpm_enabled) {
> -   if (adev->pm.dpm_enabled)
> +   if (adev->pm.dpm_enabled) {
> amdgpu_dpm_enable_uvd(adev, true);
> -   else
> +   amdgpu_dpm_switch_power_profile(adev, 
> PP_SMC_POWER_PROFILE_VIDEO, true);
> +   } else {
> amdgpu_device_ip_set_powergating_state(adev, 
> AMD_IP_BLOCK_TYPE_VCN,
>
> AMD_PG_STATE_UNGATE);
> +   }
> }
>  }
>
> --
> 1.9.1
>
> ___
> 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 1/2] drm/amd/pp: Add profiling mode setting on RV

[Alex Deucher]

On Fri, May 18, 2018 at 5:33 AM, Rex Zhu  wrote:
> For power saving, default profiling mode was setted
> to power saving mode.
>
> Currently, not support CUSTOM mode and not display
> detailed profiling mode parameters in sysfs.
>
> Signed-off-by: Rex Zhu 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 42 
> +++
>  1 file changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> index 0882ecf..409a46b 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> @@ -450,6 +450,10 @@ static int smu10_hwmgr_backend_init(struct pp_hwmgr 
> *hwmgr)
>
> hwmgr->backend = data;
>
> +   hwmgr->workload_mask = 1 << 
> hwmgr->workload_prority[PP_SMC_POWER_PROFILE_POWERSAVING];
> +   hwmgr->power_profile_mode = PP_SMC_POWER_PROFILE_POWERSAVING;
> +   hwmgr->default_power_profile_mode = PP_SMC_POWER_PROFILE_POWERSAVING;
> +
> result = smu10_initialize_dpm_defaults(hwmgr);
> if (result != 0) {
> pr_err("smu10_initialize_dpm_defaults failed\n");
> @@ -1163,6 +1167,42 @@ static void smu10_powergate_vcn(struct pp_hwmgr 
> *hwmgr, bool bgate)
> }
>  }
>
> +static int smu10_get_power_profile_mode(struct pp_hwmgr *hwmgr, char *buf)
> +{
> +   uint32_t i, size = 0;
> +
> +   static const char *profile_name[6] = {"3D_FULL_SCREEN",
> +   "POWER_SAVING",
> +   "VIDEO",
> +   "VR",
> +   "COMPUTE"};
> +   static const char *title[2] = {"NUM", "MODE_NAME"};
> +
> +   if (!buf)
> +   return -EINVAL;
> +
> +   size += sprintf(buf + size, "%s %10s\n", title[0], title[1]);
> +
> +   for (i = 0; i < PP_SMC_POWER_PROFILE_CUSTOM; i++)
> +   size += sprintf(buf + size, "%3d %14s%s\n",
> +   i, profile_name[i],
> +   (i == hwmgr->power_profile_mode) ? "*" : " ");
> +   return size;
> +}
> +
> +static int smu10_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, 
> uint32_t size)
> +{
> +   if (input[size] >= PP_SMC_POWER_PROFILE_CUSTOM)
> +   return -EINVAL;
> +
> +   hwmgr->power_profile_mode = input[size];
> +
> +   smum_send_msg_to_smc_with_parameter(hwmgr, 
> PPSMC_MSG_ActiveProcessNotify,
> +   1 +
> +   return 0;
> +}
> +
>  static const struct pp_hwmgr_func smu10_hwmgr_funcs = {
> .backend_init = smu10_hwmgr_backend_init,
> .backend_fini = smu10_hwmgr_backend_fini,
> @@ -1200,6 +1240,8 @@ static void smu10_powergate_vcn(struct pp_hwmgr *hwmgr, 
> bool bgate)
> .set_mmhub_powergating_by_smu = smu10_set_mmhub_powergating_by_smu,
> .smus_notify_pwe = smu10_smus_notify_pwe,
> .gfx_off_control = smu10_gfx_off_control,
> +   .get_power_profile_mode = smu10_get_power_profile_mode,
> +   .set_power_profile_mode = smu10_set_power_profile_mode,
>  };
>
>  int smu10_init_function_pointers(struct pp_hwmgr *hwmgr)
> --
> 1.9.1
>
> ___
> 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 1/1] drm/amdgpu: add kernel doc for amdgpu_object.c

[Alex Deucher]

On Thu, May 17, 2018 at 6:20 PM, Samuel Li  wrote:
> Signed-off-by: Samuel Li 

Please also add a separate DOC section at the top of this file giving
a brief overview of how amdgpu bo API works.  E.g., how you would
allocate, free, kmap, etc.  What pinning means and why you would need
to do it, etc.  What domains we support and what they are.  What
shadow bos are.

Additional comments below for things to add or clarify to enhance the
documentation.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 233 
> +
>  1 file changed, 233 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 6a9e46a..271dbfa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -73,6 +73,15 @@ static void amdgpu_ttm_bo_destroy(struct ttm_buffer_object 
> *tbo)
> kfree(bo);
>  }
>
> +/**
> + * amdgpu_ttm_bo_is_amdgpu_bo - if the buffer object belongs to an _bo
> + * @bo: buffer object to be checked
> + *
> + * Uses destroy function associated with the object to determine if this is
> + * _bo.
> + *
> + * Returns true if the object belongs to _bo, false if not.
> + */

Just to clarify, it checks whether a ttm bo is an amdgpu buffer object or not.

>  bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
>  {
> if (bo->destroy == _ttm_bo_destroy)
> @@ -80,6 +89,14 @@ bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object 
> *bo)
> return false;
>  }
>
> +/**
> + * amdgpu_ttm_placement_from_domain - set buffer's placement
> + * @abo: _bo buffer object whose placement is to be set
> + * @domain: requested domain
> + *
> + * Sets buffer's placement according to requested domain and the buffer's
> + * flags.
> + */
>  void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
>  {
> struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
> @@ -498,6 +515,17 @@ static int amdgpu_bo_create_shadow(struct amdgpu_device 
> *adev,
> return r;
>  }
>
> +/**
> + * amdgpu_bo_create - create an _bo buffer object
> + * @adev: amdgpu device object
> + * @bp: parameters to be used for the buffer object
> + * @bo_ptr: pointer to the buffer object pointer
> + *
> + * Creates an _bo buffer object; and if requested, also creates a
> + * shadow object.
> + *
> + * Returns 0 for success or a negative error code on failure.
> + */
>  int amdgpu_bo_create(struct amdgpu_device *adev,
>  struct amdgpu_bo_param *bp,
>  struct amdgpu_bo **bo_ptr)
> @@ -527,6 +555,20 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
> return r;
>  }
>
> +/**
> + * amdgpu_bo_backup_to_shadow - Backs up an _bo buffer object
> + * @adev: amdgpu device object
> + * @ring: amdgpu_ring for the engine handling the buffer operations
> + * @bo: _bo buffer to be backed up
> + * @resv: reservation object with embedded fence
> + * @fence: dma_fence associated with the operation
> + * @direct: whether to submit the job directly
> + *
> + * Copies an _bo buffer object to its shadow object.
> + * Not used for now.
> + *
> + * Returns 0 for success or a negative error code on failure.
> + */
>  int amdgpu_bo_backup_to_shadow(struct amdgpu_device *adev,
>struct amdgpu_ring *ring,
>struct amdgpu_bo *bo,
> @@ -559,6 +601,15 @@ int amdgpu_bo_backup_to_shadow(struct amdgpu_device 
> *adev,
> return r;
>  }
>
> +/**
> + * amdgpu_bo_validate - validate an _bo buffer object
> + * @bo: pointer to the buffer object
> + *
> + * Sets placement according to domain; and changes placement and caching
> + * policy of the buffer object according to the placement.
> + *
> + * Returns 0 for success or a negative error code on failure.
> + */

This is used for validating shadow bos.  Also worth mentioning that it
calls ttm_bo_validate() which will make sure the buffer is resident
where it needs to be.

>  int amdgpu_bo_validate(struct amdgpu_bo *bo)
>  {
> struct ttm_operation_ctx ctx = { false, false };
> @@ -581,6 +632,19 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)
> return r;
>  }
>
> +/**
> + * amdgpu_bo_restore_from_shadow - restore an _bo buffer object
> + * @adev: amdgpu device object
> + * @ring: amdgpu_ring for the engine handling the buffer operations
> + * @bo: _bo buffer to be restored
> + * @resv: reservation object with embedded fence
> + * @fence: dma_fence associated with the operation
> + * @direct: whether to submit the job directly
> + *
> + * Copies a buffer object's shadow content back to the object.
> + *

Mention that this is used for recovering a buffer from it's shadow in
the case of a gpu reset where vram context may be lost.


> + * Returns 0 for success or a negative error code on failure.
> + */
>  int amdgpu_bo_restore_from_shadow(struct amdgpu_device *adev,
>  

Re: [PATCH] drm/amdgpu: conditionally compile amdgpu's amdkfd files

[Oded Gabbay]

On Thu, May 17, 2018 at 12:09 AM, Felix Kuehling  wrote:
> Hi Oded,
>
> Thanks for working on this! The Makefile changes look good.
>
> Instead of checking and calling function pointers in amdgpu_amdkfd_...
> functions at runtime, couldn't you just define empty stub functions in
> amdgpu_amdkfd.h if KFD is not enabled? I think that would make the code
> shorter and remove the runtime overhead.
>
> Regards,
>   Felix

Yes, I can definitely do that. I agree it would be better.

Oded

>
>
> On 2018-05-16 06:09 AM, Oded Gabbay wrote:
>> In case CONFIG_HSA_AMD is not chosen, there is no need to compile amdkfd
>> files that reside inside amdgpu dirver. In addition, because amdkfd
>> depends on x86_64 architecture and amdgpu is not, compiling amdkfd files
>> under i386 architecture can cause compiler errors and warnings.
>>
>> This patch modifies amdgpu's makefile to build amdkfd files only if
>> CONFIG_HSA_AMD is chosen. The only file to be compiled unconditionally
>> is amdgpu_amdkfd.c
>>
>> Direct calls from amdgpu driver proper to functions in other
>> amdgpu_amdkfd_*.c files were changed to calls to functions inside
>> amdgpu_amdkfd.c. These functions call the original functions using a
>> function pointer to allow compilation without the original functions.
>>
>> Signed-off-by: Oded Gabbay 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/Makefile  | 13 +++--
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   | 66 
>> ++--
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h   | 48 -
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c |  8 ++-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c |  2 +-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  |  2 +-
>>  6 files changed, 112 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
>> b/drivers/gpu/drm/amd/amdgpu/Makefile
>> index f3002020df6c..1464dff1b151 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>> @@ -56,8 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>>
>>  # add asic specific block
>>  amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
>> - ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o 
>> vce_v2_0.o \
>> - amdgpu_amdkfd_gfx_v7.o
>> + ci_smc.o ci_dpm.o dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o 
>> vce_v2_0.o
>>
>>  amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o gfx_v6_0.o si_ih.o 
>> si_dma.o dce_v6_0.o si_dpm.o si_smc.o
>>
>> @@ -126,13 +125,21 @@ amdgpu-y += \
>>   vcn_v1_0.o
>>
>>  # add amdkfd interfaces
>> +amdgpu-y += amdgpu_amdkfd.o
>> +
>> +ifneq ($(CONFIG_HSA_AMD),)
>>  amdgpu-y += \
>> -  amdgpu_amdkfd.o \
>>amdgpu_amdkfd_fence.o \
>>amdgpu_amdkfd_gpuvm.o \
>>amdgpu_amdkfd_gfx_v8.o \
>>amdgpu_amdkfd_gfx_v9.o
>>
>> +ifneq ($(CONFIG_DRM_AMDGPU_CIK),)
>> +amdgpu-y += amdgpu_amdkfd_gfx_v7.o
>> +endif
>> +
>> +endif
>> +
>>  # add cgs
>>  amdgpu-y += amdgpu_cgs.o
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> index cd0e8f192e6a..930d27dd6e27 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>> @@ -27,7 +27,21 @@
>>  #include "amdgpu_gfx.h"
>>  #include 
>>
>> +#if defined(CONFIG_HSA_AMD_MODULE) || defined(CONFIG_HSA_AMD)
>> +static const struct amdgpu_amdkfd_if amdkfd_if = {
>> + .fence_check_mm = amdkfd_fence_check_mm,
>> + .unreserve_system_memory_limit = amdkfd_unreserve_system_memory_limit,
>> + .gpuvm_destroy_cb = amdkfd_gpuvm_destroy_cb,
>> + .to_kfd_fence = to_amdgpu_amdkfd_fence,
>> + .evict_userptr = amdkfd_evict_userptr,
>> + .gfx_7_get_functions = amdgpu_amdkfd_gfx_7_get_functions,
>> + .gfx_8_0_get_functions = amdgpu_amdkfd_gfx_8_0_get_functions,
>> + .gfx_9_0_get_functions = amdgpu_amdkfd_gfx_9_0_get_functions
>> +};
>> +#endif
>> +
>>  const struct kgd2kfd_calls *kgd2kfd;
>> +const struct amdgpu_amdkfd_if *amdgpu_amdkfd_if;
>>  bool (*kgd2kfd_init_p)(unsigned int, const struct kgd2kfd_calls**);
>>
>>  static const unsigned int compute_vmid_bitmap = 0xFF00;
>> @@ -50,15 +64,22 @@ int amdgpu_amdkfd_init(void)
>>   kgd2kfd = NULL;
>>   }
>>
>> +
>>  #elif defined(CONFIG_HSA_AMD)
>> +
>>   ret = kgd2kfd_init(KFD_INTERFACE_VERSION, );
>>   if (ret)
>>   kgd2kfd = NULL;
>>
>>  #else
>> + amdgpu_amdkfd_if = NULL;
>>   ret = -ENOENT;
>>  #endif
>> +
>> +#if defined(CONFIG_HSA_AMD_MODULE) || defined(CONFIG_HSA_AMD)
>> + amdgpu_amdkfd_if = _if;
>>   amdgpu_amdkfd_gpuvm_init_mem_limits();
>> +#endif
>>
>>   return ret;
>>  }
>> @@ -75,14 +96,14 @@ void amdgpu_amdkfd_device_probe(struct amdgpu_device 
>> *adev)
>>  {
>>   const struct kfd2kgd_calls *kfd2kgd;
>>
>> - if (!kgd2kfd)
>> + if ((!kgd2kfd) || (!amdgpu_amdkfd_if))
>>   return;

Re: [PATCH] drm/amdgpu: consider user preference when pinning for SG display

[Samuel Li]

On 2018-05-18 04:21 AM, Michel Dänzer wrote:
> On 2018-05-17 06:55 PM, Alex Deucher wrote:
>> If the pin domain is set to GTT | VRAM, look at the preferred domains
>> for the bo and respect that if it's been set explicitly.
>>
>> Signed-off-by: Alex Deucher 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 6a9e46ae7f0a..16192f17653e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -704,9 +704,14 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
>> domain,
>>   * See function amdgpu_display_supported_domains()
>>   */
>>  if (domain == (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
>> -domain = AMDGPU_GEM_DOMAIN_VRAM;
>> -if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>> +if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM)
>> +domain = AMDGPU_GEM_DOMAIN_VRAM; /* if user really 
>> wants vram, respect it */
>> +else if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_GTT)
>> +domain = AMDGPU_GEM_DOMAIN_GTT; /* if user really wants 
>> gtt, respect it */
> 
> I'd spell VRAM and GTT in capital letters in the comments.
> 
> 
>> +else if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>>  domain = AMDGPU_GEM_DOMAIN_GTT;
>> +else
>> +domain = AMDGPU_GEM_DOMAIN_VRAM;
>>  }
> 
> Is everything in place to deal with any issues that might occur when
> flipping between buffers in VRAM and GTT?
> 
Besides this question, I am also wondering how to deal with the first display 
buffer created in kernel at amdgpufb_create_pinned_object()?

Sam


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

[PATCH libdrm 3/3] amdgpu: Destroy fd_hash table when the last device is removed.

[Jan Vesely]

Fixes memory leak on module unload.
Analogous to mesa commit of the same name.
Signed-off-by: Jan Vesely 
---
 amdgpu/amdgpu_device.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
index e23dd3b3..34ac95b8 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -128,6 +128,10 @@ static void 
amdgpu_device_free_internal(amdgpu_device_handle dev)
 {
pthread_mutex_lock(_mutex);
util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
+   if (util_hash_table_count(fd_tab) == 0) {
+   util_hash_table_destroy(fd_tab);
+   fd_tab = NULL;
+   }
close(dev->fd);
if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
close(dev->flink_fd);
-- 
2.17.0

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

[PATCH libdrm v2 1/3] amdgpu: Take lock before removing devices from fd_tab hash table.

[Jan Vesely]

Close the file descriptors under lock as well.
v2: close fds after removing from hash table

Signed-off-by: Jan Vesely 
---
 amdgpu/amdgpu_device.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
index 983b19ab..e23dd3b3 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -126,6 +126,13 @@ static int amdgpu_get_auth(int fd, int *auth)
 
 static void amdgpu_device_free_internal(amdgpu_device_handle dev)
 {
+   pthread_mutex_lock(_mutex);
+   util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
+   close(dev->fd);
+   if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
+   close(dev->flink_fd);
+   pthread_mutex_unlock(_mutex);
+
amdgpu_vamgr_deinit(>vamgr_32);
amdgpu_vamgr_deinit(>vamgr);
amdgpu_vamgr_deinit(>vamgr_high_32);
@@ -133,10 +140,6 @@ static void 
amdgpu_device_free_internal(amdgpu_device_handle dev)
util_hash_table_destroy(dev->bo_flink_names);
util_hash_table_destroy(dev->bo_handles);
pthread_mutex_destroy(>bo_table_mutex);
-   util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
-   close(dev->fd);
-   if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
-   close(dev->flink_fd);
free(dev->marketing_name);
free(dev);
 }
-- 
2.17.0

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

[PATCH libdrm 2/3] amdgpu/util_hash_table: Add helper function to count the number of entries in hash table

[Jan Vesely]

Analogous to the mesa commit of the same name.
Signed-off-by: Jan Vesely 
---
 amdgpu/util_hash_table.c | 12 
 amdgpu/util_hash_table.h |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/amdgpu/util_hash_table.c b/amdgpu/util_hash_table.c
index 89a8bf9b..e06d4415 100644
--- a/amdgpu/util_hash_table.c
+++ b/amdgpu/util_hash_table.c
@@ -237,6 +237,18 @@ drm_private void util_hash_table_foreach(struct 
util_hash_table *ht,
}
 }
 
+static void util_hash_table_inc(void *k, void *v, void *d)
+{
+   ++*(size_t *)d;
+}
+
+drm_private size_t util_hash_table_count(struct util_hash_table *ht)
+{
+   size_t count = 0;
+   util_hash_table_foreach(ht, util_hash_table_inc, );
+   return count;
+}
+
 drm_private void util_hash_table_destroy(struct util_hash_table *ht)
 {
struct util_hash_iter iter;
diff --git a/amdgpu/util_hash_table.h b/amdgpu/util_hash_table.h
index 5e295a81..3ab81a12 100644
--- a/amdgpu/util_hash_table.h
+++ b/amdgpu/util_hash_table.h
@@ -64,6 +64,8 @@ drm_private void util_hash_table_foreach(struct 
util_hash_table *ht,
void (*callback)(void *key, void *value, void *data),
void *data);
 
+drm_private size_t util_hash_table_count(struct util_hash_table *ht);
+
 drm_private void util_hash_table_destroy(struct util_hash_table *ht);
 
 #endif /* U_HASH_TABLE_H_ */
-- 
2.17.0

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

Re: [PATCH] drm/amd/amdgpu: Convert comments for amdgpu_ttm.c

[Alex Deucher]

On Fri, May 18, 2018 at 12:22 PM, Tom St Denis  wrote:
> We're moving to Sphinx style documentation.  Conver the comments
> to that format.  Also document some new content.
>

Sorry, I should have been more clear.  The current comment formatting
is fine.  What I'd like is a new separate DOC comment at the top of
the file that gives an overview of how amdgpu interacts with ttm.

Alex

> Signed-off-by: Tom St Denis 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 254 
> +++-
>  1 file changed, 185 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 57d4da6bdbe4..367d5bc2630f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -65,8 +65,9 @@ static void amdgpu_ttm_debugfs_fini(struct amdgpu_device 
> *adev);
>   */
>
>  /**
> - * amdgpu_ttm_mem_global_init - Initialize and acquire reference to
> - * memory object
> + * DOC: amdgpu_ttm_mem_global_init
> + *
> + * Initialize and acquire reference to memory object
>   *
>   * @ref: Object for initialization.
>   *
> @@ -79,7 +80,9 @@ static int amdgpu_ttm_mem_global_init(struct 
> drm_global_reference *ref)
>  }
>
>  /**
> - * amdgpu_ttm_mem_global_release - Drop reference to a memory object
> + * DOC: amdgpu_ttm_mem_global_release
> + *
> + * Drop reference to a memory object
>   *
>   * @ref: Object being removed
>   *
> @@ -92,8 +95,9 @@ static void amdgpu_ttm_mem_global_release(struct 
> drm_global_reference *ref)
>  }
>
>  /**
> - * amdgpu_ttm_global_init - Initialize global TTM memory reference
> - * structures.
> + * DOC: amdgpu_ttm_global_init
> + *
> + * Initialize global TTM memory reference structures.
>   *
>   * @adev:  AMDGPU device for which the global structures need to be
>   * registered.
> @@ -177,8 +181,9 @@ static int amdgpu_invalidate_caches(struct ttm_bo_device 
> *bdev, uint32_t flags)
>  }
>
>  /**
> - * amdgpu_init_mem_type -  Initialize a memory manager for a specific
> - * type of memory 
> request.
> + * DOC: amdgpu_init_mem_type
> + *
> + * Initialize a memory manager for a specific type of memory request.
>   *
>   * @bdev:  The TTM BO device object (contains a reference to
>   * amdgpu_device)
> @@ -237,12 +242,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device 
> *bdev, uint32_t type,
>  }
>
>  /**
> - * amdgpu_evict_flags - Compute placement flags
> + * DOC: amdgpu_evict_flags
> + *
> + * Compute placement flags
>   *
>   * @bo: The buffer object to evict
>   * @placement: Possible destination(s) for evicted BO
>   *
> - * Fill in placement data when ttm_bo_evict() is called
> + * Fill in placement data when ttm_bo_evict() is called.
>   */
>  static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
> struct ttm_placement *placement)
> @@ -305,7 +312,9 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
> *bo,
>  }
>
>  /**
> - * amdgpu_verify_access - Verify access for a mmap call
> + * DOC: amdgpu_verify_access
> + *
> + * Verify access for a mmap call
>   *
>   * @bo:The buffer object to map
>   * @filp:  The file pointer from the process performing the mmap
> @@ -331,7 +340,9 @@ static int amdgpu_verify_access(struct ttm_buffer_object 
> *bo, struct file *filp)
>  }
>
>  /**
> - * amdgpu_move_null - Register memory for a buffer object
> + * DOC: amdgpu_move_null
> + *
> + * Register memory for a buffer object
>   *
>   * @bo:The bo to assign the memory to
>   * @new_mem:   The memory to be assigned.
> @@ -350,8 +361,9 @@ static void amdgpu_move_null(struct ttm_buffer_object *bo,
>  }
>
>  /**
> - * amdgpu_mm_node_addr -   Compute the GPU relative offset of a GTT
> - * buffer.
> + * DOC: amdgpu_mm_node_addr
> + *
> + * Compute the GPU relative offset of a GTT buffer.
>   */
>  static uint64_t amdgpu_mm_node_addr(struct ttm_buffer_object *bo,
> struct drm_mm_node *mm_node,
> @@ -367,10 +379,10 @@ static uint64_t amdgpu_mm_node_addr(struct 
> ttm_buffer_object *bo,
>  }
>
>  /**
> - * amdgpu_find_mm_node -   Helper function finds the drm_mm_node
> - * corresponding to @offset. It 
> also modifies
> - * the offset to be 
> within the drm_mm_node
> - * returned
> + * DOC: amdgpu_find_mm_node
> + *
> + * Helper function finds the drm_mm_node corresponding to @offset.  It
> + * also modifies the offset to be within the drm_mm_node returned.
>   */
>  static struct drm_mm_node *amdgpu_find_mm_node(struct ttm_mem_reg *mem,
> 

[PATCH] drm/amd/amdgpu: Convert comments for amdgpu_ttm.c

[Tom St Denis]

We're moving to Sphinx style documentation.  Conver the comments
to that format.  Also document some new content.

Signed-off-by: Tom St Denis 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 254 +++-
 1 file changed, 185 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 57d4da6bdbe4..367d5bc2630f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -65,8 +65,9 @@ static void amdgpu_ttm_debugfs_fini(struct amdgpu_device 
*adev);
  */
 
 /**
- * amdgpu_ttm_mem_global_init - Initialize and acquire reference to
- * memory object
+ * DOC: amdgpu_ttm_mem_global_init
+ *
+ * Initialize and acquire reference to memory object
  *
  * @ref: Object for initialization.
  *
@@ -79,7 +80,9 @@ static int amdgpu_ttm_mem_global_init(struct 
drm_global_reference *ref)
 }
 
 /**
- * amdgpu_ttm_mem_global_release - Drop reference to a memory object
+ * DOC: amdgpu_ttm_mem_global_release
+ *
+ * Drop reference to a memory object
  *
  * @ref: Object being removed
  *
@@ -92,8 +95,9 @@ static void amdgpu_ttm_mem_global_release(struct 
drm_global_reference *ref)
 }
 
 /**
- * amdgpu_ttm_global_init - Initialize global TTM memory reference
- * structures.
+ * DOC: amdgpu_ttm_global_init
+ *
+ * Initialize global TTM memory reference structures.
  *
  * @adev:  AMDGPU device for which the global structures need to be
  * registered.
@@ -177,8 +181,9 @@ static int amdgpu_invalidate_caches(struct ttm_bo_device 
*bdev, uint32_t flags)
 }
 
 /**
- * amdgpu_init_mem_type -  Initialize a memory manager for a specific
- * type of memory request.
+ * DOC: amdgpu_init_mem_type
+ *
+ * Initialize a memory manager for a specific type of memory request.
  *
  * @bdev:  The TTM BO device object (contains a reference to
  * amdgpu_device)
@@ -237,12 +242,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device 
*bdev, uint32_t type,
 }
 
 /**
- * amdgpu_evict_flags - Compute placement flags
+ * DOC: amdgpu_evict_flags
+ *
+ * Compute placement flags
  *
  * @bo: The buffer object to evict
  * @placement: Possible destination(s) for evicted BO
  *
- * Fill in placement data when ttm_bo_evict() is called
+ * Fill in placement data when ttm_bo_evict() is called.
  */
 static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
struct ttm_placement *placement)
@@ -305,7 +312,9 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
 }
 
 /**
- * amdgpu_verify_access - Verify access for a mmap call
+ * DOC: amdgpu_verify_access
+ *
+ * Verify access for a mmap call
  *
  * @bo:The buffer object to map
  * @filp:  The file pointer from the process performing the mmap
@@ -331,7 +340,9 @@ static int amdgpu_verify_access(struct ttm_buffer_object 
*bo, struct file *filp)
 }
 
 /**
- * amdgpu_move_null - Register memory for a buffer object
+ * DOC: amdgpu_move_null
+ *
+ * Register memory for a buffer object
  *
  * @bo:The bo to assign the memory to
  * @new_mem:   The memory to be assigned.
@@ -350,8 +361,9 @@ static void amdgpu_move_null(struct ttm_buffer_object *bo,
 }
 
 /**
- * amdgpu_mm_node_addr -   Compute the GPU relative offset of a GTT
- * buffer.
+ * DOC: amdgpu_mm_node_addr
+ *
+ * Compute the GPU relative offset of a GTT buffer.
  */
 static uint64_t amdgpu_mm_node_addr(struct ttm_buffer_object *bo,
struct drm_mm_node *mm_node,
@@ -367,10 +379,10 @@ static uint64_t amdgpu_mm_node_addr(struct 
ttm_buffer_object *bo,
 }
 
 /**
- * amdgpu_find_mm_node -   Helper function finds the drm_mm_node
- * corresponding to @offset. It 
also modifies
- * the offset to be within 
the drm_mm_node
- * returned
+ * DOC: amdgpu_find_mm_node
+ *
+ * Helper function finds the drm_mm_node corresponding to @offset.  It
+ * also modifies the offset to be within the drm_mm_node returned.
  */
 static struct drm_mm_node *amdgpu_find_mm_node(struct ttm_mem_reg *mem,
   unsigned long *offset)
@@ -385,7 +397,9 @@ static struct drm_mm_node *amdgpu_find_mm_node(struct 
ttm_mem_reg *mem,
 }
 
 /**
- * amdgpu_copy_ttm_mem_to_mem - Helper function for copy
+ * DOC: amdgpu_copy_ttm_mem_to_mem
+ *
+ * Helper function for copy
  *
  * The function copies @size bytes from {src->mem + src->offset} to
  * {dst->mem + dst->offset}. src->bo and dst->bo could be same BO for a
@@ -510,7 +524,9 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
 }
 
 /**
- * 

[PATCH xf86-video-ati 1/2] Bail from dri2_create_buffer2 if we can't get a pixmap

[Michel Dänzer]

From: Michel Dänzer 

We would store the NULL pointer and continue, which would lead to a
crash down the road.

Bugzilla: https://bugs.freedesktop.org/106293
Signed-off-by: Michel Dänzer 
---
 src/radeon_dri2.c | 40 +++-
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
index 9f373589d..3b75f66f3 100644
--- a/src/radeon_dri2.c
+++ b/src/radeon_dri2.c
@@ -233,37 +233,36 @@ radeon_dri2_create_buffer2(ScreenPtr pScreen,
  flags | RADEON_CREATE_PIXMAP_DRI2);
 }
 
+if (!pixmap)
+   return NULL;
+
 buffers = calloc(1, sizeof *buffers);
 if (buffers == NULL)
 goto error;
 
-if (pixmap) {
-   if (!info->use_glamor) {
-   info->exa_force_create = TRUE;
-   exaMoveInPixmap(pixmap);
-   info->exa_force_create = FALSE;
-   if (exaGetPixmapDriverPrivate(pixmap) == NULL) {
-   /* this happen if pixmap is non accelerable */
-   goto error;
-   }
-   } else if (is_glamor_pixmap) {
-   pixmap = radeon_glamor_set_pixmap_bo(drawable, pixmap);
-   pixmap->refcnt++;
-   }
-
-   if (!radeon_get_flink_name(pRADEONEnt, pixmap, >name))
+if (!info->use_glamor) {
+   info->exa_force_create = TRUE;
+   exaMoveInPixmap(pixmap);
+   info->exa_force_create = FALSE;
+   if (exaGetPixmapDriverPrivate(pixmap) == NULL) {
+   /* this happen if pixmap is non accelerable */
goto error;
+   }
+} else if (is_glamor_pixmap) {
+   pixmap = radeon_glamor_set_pixmap_bo(drawable, pixmap);
+   pixmap->refcnt++;
 }
 
+if (!radeon_get_flink_name(pRADEONEnt, pixmap, >name))
+   goto error;
+
 privates = calloc(1, sizeof(struct dri2_buffer_priv));
 if (privates == NULL)
 goto error;
 
 buffers->attachment = attachment;
-if (pixmap) {
-   buffers->pitch = pixmap->devKind;
-   buffers->cpp = cpp;
-}
+buffers->pitch = pixmap->devKind;
+buffers->cpp = cpp;
 buffers->driverPrivate = privates;
 buffers->format = format;
 buffers->flags = 0; /* not tiled */
@@ -275,8 +274,7 @@ radeon_dri2_create_buffer2(ScreenPtr pScreen,
 
 error:
 free(buffers);
-if (pixmap)
-(*pScreen->DestroyPixmap)(pixmap);
+(*pScreen->DestroyPixmap)(pixmap);
 return NULL;
 }
 
-- 
2.17.0

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

[PATCH xf86-video-ati 2/2] glamor: Bail CreatePixmap on unsupported pixmap depth

[Michel Dänzer]

From: Michel Dänzer 

Fixes crash in that case.

Bugzilla: https://bugs.freedesktop.org/106293
Signed-off-by: Michel Dänzer 
---
 src/radeon_glamor.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/radeon_glamor.c b/src/radeon_glamor.c
index 7c09abba5..10d513ea9 100644
--- a/src/radeon_glamor.c
+++ b/src/radeon_glamor.c
@@ -214,6 +214,9 @@ radeon_glamor_create_pixmap(ScreenPtr screen, int w, int h, 
int depth,
struct radeon_pixmap *priv;
PixmapPtr pixmap, new_pixmap = NULL;
 
+   if (!xf86GetPixFormat(scrn, depth))
+   return NULL;
+
if (!RADEON_CREATE_PIXMAP_SHARED(usage)) {
if (info->shadow_primary) {
if (usage != CREATE_PIXMAP_USAGE_BACKING_PIXMAP)
-- 
2.17.0

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

Re: amd gpu in Xen domu -- NULL dereference in drivers/gpu/drm/drm_pci.c

[Håkon Alstadheim]

Den 18. mai 2018 13:18, skrev Håkon Alstadheim:
> TL;DR: How to get some canonical work-around for quirks into official
> module and/or kernel repo ? Running with gpu passed through to virtual
> machine.
>
> -
>
I went ahead and switched to amdgpu driver, and it seems the NULL
dereference of

dev->pdev->bus->self is not done by the amdgpu driver. 


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

amd gpu in Xen domu -- NULL dereference in drivers/gpu/drm/drm_pci.c

[Håkon Alstadheim]

TL;DR: How to get some canonical work-around for quirks into official
module and/or kernel repo ? Running with gpu passed through to virtual
machine.

-

I'm running this card:

00:06.0 VGA compatible controller: Advanced Micro Devices, Inc.
[AMD/ATI] Curacao PRO [Radeon R7 370 / R9 270/370 OEM] (prog-if 00 [VGA
controller])

passed through to a user domain under Xen. (xen-4.11 release candidate ).

I have been running with this patch for a long while now:

---

--- a/drivers/gpu/drm/drm_pci.c    2017-07-30 23:29:43.55000 +0200
+++ b/drivers/gpu/drm/drm_pci.c    2017-07-30 23:28:55.58000 +0200
@@ -337,11 +337,28 @@
 u32 lnkcap, lnkcap2;
 
 *mask = 0;
-    if (!dev->pdev)
-        return -EINVAL;
-
+    if (!dev->pdev) {
+      DRM_INFO("invalid dev->pdev\n");
+      return -EINVAL;
+    }
+   
+    if (!dev->pdev->bus) {
+      DRM_INFO("invalid dev->pdev->bus\n");
+      return -EINVAL;
+    }
+   
+    if (!dev->pdev->bus->self) {
+      DRM_INFO("invalid dev->pdev->bus->self\n");
+      return -EINVAL;
+    }
+   
 root = dev->pdev->bus->self;
 
+    if (!root->vendor) {
+      DRM_INFO("invalid root->vendor\n");
+      return -EINVAL;
+    }
+
 /* we've been informed via and serverworks don't make the cut */
 if (root->vendor == PCI_VENDOR_ID_VIA ||
     root->vendor == PCI_VENDOR_ID_SERVERWORKS)
--

This gets me the following logged on boot:

mai 17 23:27:35 gt kernel: [drm] invalid dev->pdev->bus->self

I created the patch on a hunch without any knowledge of how that
return-value gets interpreted. At the moment I'm using the radeon driver
in kernel  4.16.9-gentoo, but I'm thinking about switching to amdgpu.
Before I do that I'd hope to have the deficiencies running under Xen
could get some kind of "official" work-around.

Works OK, but I have some random stalls in window-manager and browsers,
without anything showing up in top or atop. One of my suspected culprits
is some kind of error time-out in graphics rendering. I'm not a
programmer, so wading through threads in gdb is not my forte.


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

Re: [PATCH] drm/amdgpu: add rcu_barrier after entity fini

[Christian König]

Ah! So we have one RCU grace period caused by the scheduler fence and 
another one from the hardware fence.


Thanks for explaining that once more, that was really not obvious from 
reading the code.


But this means we could we also fix it by moving the 
"dma_fence_put(fence->parent);" from drm_sched_fence_free() into 
drm_sched_fence_release_scheduled() directly before the call_rcu(), 
can't we?


I think that would be cleaner, cause otherwise every driver using the 
GPU scheduler would need that workaround.


And drm_sched_fence_release_scheduled() is only called when the 
reference count becomes zero, so when anybody accesses the structure 
after that then that would be a bug as well.


Thanks,
Christian.

Am 18.05.2018 um 11:56 schrieb Deng, Emily:

Hi Christian,
    When we free an IB fence, we first call one call_rcu in 
drm_sched_fence_release_scheduled as the call trace one, then after 
the call trace one,
we call the call_rcu second in the  amdgpu_fence_release in call trace 
two, as below.

The kmem_cache_free(amdgpu_fence_slab, fence)'s call trace as below:
    1.drm_sched_entity_fini ->
    drm_sched_entity_cleanup ->
    dma_fence_put(entity->last_scheduled) ->
    drm_sched_fence_release_finished ->
    drm_sched_fence_release_scheduled ->
    call_rcu(>finished.rcu, drm_sched_fence_free)
    2.drm_sched_fence_free ->
    dma_fence_put(fence->parent) ->
    amdgpu_fence_release ->
    call_rcu(>rcu, amdgpu_fence_free) ->
    kmem_cache_free(amdgpu_fence_slab, fence);
> -Original Message-
> From: Koenig, Christian
> Sent: Friday, May 18, 2018 5:46 PM
> To: Deng, Emily ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: add rcu_barrier after entity fini
> 
> Ok, I'm lost where do we use call_rcu() twice? Cause that sounds incorrect in

> the first place.
> 
> Christian.
> 
> Am 18.05.2018 um 11:41 schrieb Deng, Emily:

> > Ping..
> >
> > Best Wishes,
> > Emily Deng
> >
> >
> >
> >
> >> -Original Message-
> >> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On
> >> Behalf Of Deng, Emily
> >> Sent: Friday, May 18, 2018 11:20 AM
> >> To: Koenig, Christian >; amd-
> >> g...@lists.freedesktop.org 
> >> Subject: RE: [PATCH] drm/amdgpu: add rcu_barrier after entity fini
> >>
> >> Hi Christian,
> >>   Yes, it has already one rcu_barrier, but it has called twice
> >> call_rcu, so the one rcu_barrier just could barrier one call_rcu some time.
> >>  After I added another rcu_barrier, the kernel issue will disappear.
> >>
> >> Best Wishes,
> >> Emily Deng
> >>
> >>> -Original Message-
> >>> From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com]
> >>> Sent: Thursday, May 17, 2018 7:08 PM
> >>> To: Deng, Emily >; amd- 


> g...@lists.freedesktop.org
> >>> Subject: Re: [PATCH] drm/amdgpu: add rcu_barrier after entity fini
> >>>
> >>> Am 17.05.2018 um 12:03 schrieb Emily Deng:
>  To free the fence from the amdgpu_fence_slab, need twice call_rcu,
>  to avoid the amdgpu_fence_slab_fini call
>  kmem_cache_destroy(amdgpu_fence_slab) before
> >>> kmem_cache_free(amdgpu_fence_slab, fence), add rcu_barrier after
> >>> drm_sched_entity_fini.
>  The kmem_cache_free(amdgpu_fence_slab, fence)'s call trace as
> below:
>  1.drm_sched_entity_fini ->
>  drm_sched_entity_cleanup ->
>  dma_fence_put(entity->last_scheduled) ->
>  drm_sched_fence_release_finished ->
> >>> drm_sched_fence_release_scheduled
>  -> call_rcu(>finished.rcu, drm_sched_fence_free)
> 
>  2.drm_sched_fence_free ->
>  dma_fence_put(fence->parent) ->
>  amdgpu_fence_release ->
>  call_rcu(>rcu, amdgpu_fence_free) ->
>  kmem_cache_free(amdgpu_fence_slab, fence);
> 
>  v2:put the barrier before the kmem_cache_destroy
> 
>  Change-Id: I8dcadd3372f97e72461bf46b41cc26d90f09b8df
>  Signed-off-by: Emily Deng >
>  ---
>    drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 1 +
>     1 file changed, 1 insertion(+)
> 
>  diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>  b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>  index 39ec6b8..42be65b 100644
>  --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>  +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>  @@ -69,6 +69,7 @@ int amdgpu_fence_slab_init(void)
>     void amdgpu_fence_slab_fini(void)
>     {
>    rcu_barrier();
>  +    rcu_barrier();
> >>> Well, you should have noted that there is already an rcu_barrier
> >>> here and adding another one shouldn't have any additional effect. So
> >>> your explanation and the proposed solution doesn't make to much
> sense.
> >>>
> >>> I think the problem you run into is rather that the fence is
> >>> reference 

Re: [PATCH xf86-video-amdgpu 08/13] Set driver-private CRTC's dpms mode on disable

[Michel Dänzer]

On 2018-05-17 11:43 PM, Leo Li wrote:
> On 2018-05-16 01:09 PM, Michel Dänzer wrote:
>> On 2018-05-03 08:31 PM, sunpeng...@amd.com wrote:
>>> From: "Leo (Sunpeng) Li" 
>>>
>>> The dpms_mode flag on the driver-private CRTC was not being set when
>>> it's DPMS state is set to off. This causes some problems when toggling
>>> it back on, as some conditionals check this flag.
>>>
>>> Signed-off-by: Leo (Sunpeng) Li 
>>> ---
>>>   src/drmmode_display.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>>> index 2b38a71..f86f99a 100644
>>> --- a/src/drmmode_display.c
>>> +++ b/src/drmmode_display.c
>>> @@ -347,6 +347,7 @@ drmmode_crtc_dpms(xf86CrtcPtr crtc, int mode)
>>>   drmModeSetCrtc(pAMDGPUEnt->fd,
>>> drmmode_crtc->mode_crtc->crtc_id,
>>>  0, 0, 0, NULL, 0, NULL);
>>>   drmmode_fb_reference(pAMDGPUEnt->fd, _crtc->fb, NULL);
>>> +    drmmode_crtc->dpms_mode = mode;
>>>   } else if (drmmode_crtc->dpms_mode != DPMSModeOn)
>>>   crtc->funcs->set_mode_major(crtc, >mode, crtc->rotation,
>>>   crtc->x, crtc->y);
>>>
>>
>> drmmode_crtc->dpms_mode is updated in drmmode_do_crtc_dpms. I'm a bit
>> worried that doing it here as well might cause subtle breakage. Is this
>> related to patches 10 & 11, or can you describe the scenario that
>> prompted you to make this change?
>>
> 
> After reading your response to patch 10, and the cover letter, I think
> this patch could be dropped. See my reply to patch 10 for details.
> 
> I originally had this change for the `xrandr --output --off` case.
> Unlike xset dpms off, it wasn't triggering drmmode_do_crtc_dpms, and
> therefore not setting the dpms_mode. This causes drmmode_do_crtc_dpms to
> skip over a section of the code when output is turned back on, where
> patch 10 resides in.

Indeed, that's a good catch, thanks. However, it's better to make sure
drmmode_do_crtc_dpms is called in this case as well, please take a look
at https://patchwork.freedesktop.org/patch/224016/ .


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 2/2] Use drmmode_crtc_dpms in drmmode_set_desired_modes

[Michel Dänzer]

From: Michel Dänzer 

Simplifies the latter slightly.

Signed-off-by: Michel Dänzer 
---
 src/drmmode_display.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 5d4627810..8a1a201a8 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -2660,7 +2660,6 @@ Bool drmmode_set_desired_modes(ScrnInfoPtr pScrn, 
drmmode_ptr drmmode,
   Bool set_hw)
 {
xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn);
-   AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
unsigned num_desired = 0, num_on = 0;
int c;
 
@@ -2668,18 +2667,12 @@ Bool drmmode_set_desired_modes(ScrnInfoPtr pScrn, 
drmmode_ptr drmmode,
if (set_hw) {
for (c = 0; c < config->num_crtc; c++) {
xf86CrtcPtr crtc = config->crtc[c];
-   drmmode_crtc_private_ptr drmmode_crtc = 
crtc->driver_private;
 
/* Skip disabled CRTCs */
if (crtc->enabled)
continue;
 
-   drmmode_do_crtc_dpms(crtc, DPMSModeOff);
-   drmModeSetCrtc(pAMDGPUEnt->fd,
-  drmmode_crtc->mode_crtc->crtc_id,
-  0, 0, 0, NULL, 0, NULL);
-   drmmode_fb_reference(pAMDGPUEnt->fd,
-_crtc->fb, NULL);
+   drmmode_crtc_dpms(crtc, DPMSModeOff);
}
}
 
-- 
2.17.0

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

[PATCH xf86-video-amdgpu 1/2] Call drmmode_do_crtc_dpms from drmmode_crtc_dpms as well

[Michel Dänzer]

From: Michel Dänzer 

Leo pointed out that drmmode_do_crtc_dpms wasn't getting called when
turning off an output with

 xrandr --output  --off

This meant that the vblank sequence number and timestamp wouldn't be
saved before turning off the CRTC in this case.

Reported-by: Leo (Sunpeng) Li 
Signed-off-by: Michel Dänzer 
---
 src/drmmode_display.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 0ee89961a..5d4627810 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -342,8 +342,7 @@ drmmode_crtc_dpms(xf86CrtcPtr crtc, int mode)
 
/* Disable unused CRTCs and enable/disable active CRTCs */
if (!crtc->enabled || mode != DPMSModeOn) {
-   drmmode_crtc_wait_pending_event(drmmode_crtc, pAMDGPUEnt->fd,
-   drmmode_crtc->flip_pending);
+   drmmode_do_crtc_dpms(crtc, DPMSModeOff);
drmModeSetCrtc(pAMDGPUEnt->fd, drmmode_crtc->mode_crtc->crtc_id,
   0, 0, 0, NULL, 0, NULL);
drmmode_fb_reference(pAMDGPUEnt->fd, _crtc->fb, NULL);
-- 
2.17.0

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

Re: [PATCH] drm/amd/amdgpu: Code comments for the amdgpu_ttm.c driver. (v2)

[Tom St Denis]

On 05/18/2018 05:52 AM, Christian König wrote:

Am 17.05.2018 um 17:34 schrieb Alex Deucher:
On Tue, May 15, 2018 at 10:02 AM, Tom St Denis  
wrote:

NFC just comments.

(v2):  Updated based on feedback from Alex Deucher.

Signed-off-by: Tom St Denis 

Reviewed-by: Alex Deucher 


Just one comment "Pin pages of memory pointed to..." better write "Grab 
a reference to the memory pointed to...".


get_user_pages() does not pin anything! I've heard that misconception so 
many times now that I can't remember how often we had to explain it and 
we should definitely not leak it into the documentation.


With that fixed the patch is Reviewed-by: Christian König 
.


Hi Christian,

Well it's been pushed but as per Alex's email last night I'll swing back 
to add Sphinx style comments to the file and fix that in the same patch.


Thanks,
tom




Regards,
Christian.




---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 347 
+++-

  1 file changed, 340 insertions(+), 7 deletions(-)

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

index dfd22db13fb1..2eaaa1fb7b59 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -63,16 +63,44 @@ static void amdgpu_ttm_debugfs_fini(struct 
amdgpu_device *adev);

  /*
   * Global memory.
   */
+
+/**
+ * amdgpu_ttm_mem_global_init - Initialize and acquire reference to
+ * memory object
+ *
+ * @ref: Object for initialization.
+ *
+ * This is called by drm_global_item_ref() when an object is being
+ * initialized.
+ */
  static int amdgpu_ttm_mem_global_init(struct drm_global_reference 
*ref)

  {
 return ttm_mem_global_init(ref->object);
  }

+/**
+ * amdgpu_ttm_mem_global_release - Drop reference to a memory object
+ *
+ * @ref: Object being removed
+ *
+ * This is called by drm_global_item_unref() when an object is being
+ * released.
+ */
  static void amdgpu_ttm_mem_global_release(struct 
drm_global_reference *ref)

  {
 ttm_mem_global_release(ref->object);
  }

+/**
+ * amdgpu_ttm_global_init - Initialize global TTM memory reference
+ * structures.
+ *
+ * @adev:  AMDGPU device for which the global structures need to be
+ * registered.
+ *
+ * This is called as part of the AMDGPU ttm init from amdgpu_ttm_init()
+ * during bring up.
+ */
  static int amdgpu_ttm_global_init(struct amdgpu_device *adev)
  {
 struct drm_global_reference *global_ref;
@@ -80,7 +108,9 @@ static int amdgpu_ttm_global_init(struct 
amdgpu_device *adev)

 struct drm_sched_rq *rq;
 int r;

+   /* ensure reference is false in case init fails */
 adev->mman.mem_global_referenced = false;
+
 global_ref = >mman.mem_global_ref;
 global_ref->global_type = DRM_GLOBAL_TTM_MEM;
 global_ref->size = sizeof(struct ttm_mem_global);
@@ -146,6 +176,18 @@ static int amdgpu_invalidate_caches(struct 
ttm_bo_device *bdev, uint32_t flags)

 return 0;
  }

+/**
+ * amdgpu_init_mem_type -  Initialize a memory manager for a 
specific
+ * type of 
memory request.

+ *
+ * @bdev:  The TTM BO device object (contains a reference to
+ * amdgpu_device)
+ * @type:  The type of memory requested
+ * @man:
+ *
+ * This is called by ttm_bo_init_mm() when a buffer object is being
+ * initialized.
+ */
  static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,

 struct ttm_mem_type_manager *man)
  {
@@ -161,6 +203,7 @@ static int amdgpu_init_mem_type(struct 
ttm_bo_device *bdev, uint32_t type,

 man->default_caching = TTM_PL_FLAG_CACHED;
 break;
 case TTM_PL_TT:
+   /* GTT memory  */
 man->func = _gtt_mgr_func;
 man->gpu_offset = adev->gmc.gart_start;
 man->available_caching = TTM_PL_MASK_CACHING;
@@ -193,6 +236,14 @@ static int amdgpu_init_mem_type(struct 
ttm_bo_device *bdev, uint32_t type,

 return 0;
  }

+/**
+ * amdgpu_evict_flags - Compute placement flags
+ *
+ * @bo: The buffer object to evict
+ * @placement: Possible destination(s) for evicted BO
+ *
+ * Fill in placement data when ttm_bo_evict() is called
+ */
  static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
 struct ttm_placement *placement)
  {
@@ -204,12 +255,14 @@ static void amdgpu_evict_flags(struct 
ttm_buffer_object *bo,

 .flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM
 };

+   /* Don't handle scatter gather BOs */
 if (bo->type == ttm_bo_type_sg) {
 placement->num_placement = 0;
 placement->num_busy_placement = 0;
 return;
 

Re: [PATCH 1/5] drm/amdgpu: fix insert nop for VCN decode ring

[Christian König]

Am 17.05.2018 um 20:25 schrieb Alex Deucher:

On Thu, May 17, 2018 at 2:24 PM, Leo Liu  wrote:


On 05/17/2018 02:18 PM, Alex Deucher wrote:

On Thu, May 17, 2018 at 2:12 PM, Leo Liu  wrote:

NO_OP register should be writen to 0

Signed-off-by: Leo Liu 

We discussed this when we first started working on amdgpu.  Does it
have to be 0?

Yes. I confirmed with FW engineer that we need put 0 into noop.


This function is used for padding.  What happens if we
need an odd number of dwords of padding required?

That should be never happening. should be always paired of PM4 packet for
RBC to process.


Good to know.


Cool.  Series is:
Reviewed-by: Alex Deucher 


Reviewed-by: Christian König  as well.





Regards,
Leo



Alex


---
   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 14 --
   1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 0501746b..7fbbdb1 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -1048,14 +1048,17 @@ static int vcn_v1_0_process_interrupt(struct
amdgpu_device *adev,
  return 0;
   }

-static void vcn_v1_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t
count)
+static void vcn_v1_0_dec_ring_insert_nop(struct amdgpu_ring *ring,
uint32_t count)
   {
-   int i;
  struct amdgpu_device *adev = ring->adev;
+   int i;

-   for (i = 0; i < count; i++)
-   amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0,
mmUVD_NO_OP), 0));
+   WARN_ON(ring->wptr % 2 || count % 2);

+   for (i = 0; i < count / 2; i++) {
+   amdgpu_ring_write(ring, PACKET0(SOC15_REG_OFFSET(UVD, 0,
mmUVD_NO_OP), 0));
+   amdgpu_ring_write(ring, 0);
+   }
   }


@@ -1082,7 +1085,6 @@ static const struct amd_ip_funcs vcn_v1_0_ip_funcs
= {
   static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
  .type = AMDGPU_RING_TYPE_VCN_DEC,
  .align_mask = 0xf,
-   .nop = PACKET0(0x81ff, 0),
  .support_64bit_ptrs = false,
  .vmhub = AMDGPU_MMHUB,
  .get_rptr = vcn_v1_0_dec_ring_get_rptr,
@@ -1101,7 +1103,7 @@ static const struct amdgpu_ring_funcs
vcn_v1_0_dec_ring_vm_funcs = {
  .emit_vm_flush = vcn_v1_0_dec_ring_emit_vm_flush,
  .test_ring = amdgpu_vcn_dec_ring_test_ring,
  .test_ib = amdgpu_vcn_dec_ring_test_ib,
-   .insert_nop = vcn_v1_0_ring_insert_nop,
+   .insert_nop = vcn_v1_0_dec_ring_insert_nop,
  .insert_start = vcn_v1_0_dec_ring_insert_start,
  .insert_end = vcn_v1_0_dec_ring_insert_end,
  .pad_ib = amdgpu_ring_generic_pad_ib,
--
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


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

Re: [PATCH 2/2] drm/amdgpu: Take vcn encode rings into account in idle work

[Christian König]

Am 17.05.2018 um 20:03 schrieb Alex Deucher:

Take the encode rings into account in the idle work handler.

Signed-off-by: Alex Deucher 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index e5d234cf804f..60468385e6b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -205,6 +205,11 @@ static void amdgpu_vcn_idle_work_handler(struct 
work_struct *work)
struct amdgpu_device *adev =
container_of(work, struct amdgpu_device, vcn.idle_work.work);
unsigned fences = amdgpu_fence_count_emitted(>vcn.ring_dec);
+   unsigned i;
+
+   for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
+   fences += amdgpu_fence_count_emitted(>vcn.ring_enc[i]);
+   }


"{" and "}" should be dropped here if I'm not completely mistaken.

But either way Reviewed-by: Christian König  
for the whole series.


Christian.

  
  	if (fences == 0) {

if (adev->pm.dpm_enabled) {


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

Re: [PATCH 1/2] Revert "drm/amdgpu/vg20:Restruct uvd.idle_work to support multiple instance (v2)"

[Christian König]

Am 17.05.2018 um 19:38 schrieb Alex Deucher:

This reverts commit 4f7b8507bb4ba19f994e0d72eedd6029961be402.

We don't need separate idle work handles for UVD 7.2.  Both instances are
driven by the same clock and power.

Signed-off-by: Alex Deucher 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 17 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  7 +--
  2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index c016c407ab09..0772680371a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -130,6 +130,8 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
unsigned version_major, version_minor, family_id;
int i, j, r;
  
+	INIT_DELAYED_WORK(>uvd.inst->idle_work, amdgpu_uvd_idle_work_handler);

+
switch (adev->asic_type) {
  #ifdef CONFIG_DRM_AMDGPU_CIK
case CHIP_BONAIRE:
@@ -236,8 +238,6 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
bo_size += 
AMDGPU_GPU_PAGE_ALIGN(le32_to_cpu(hdr->ucode_size_bytes) + 8);
  
  	for (j = 0; j < adev->uvd.num_uvd_inst; j++) {

-   adev->uvd.inst[j].delayed_work.ip_instance = j;
-   INIT_DELAYED_WORK(>uvd.inst[j].delayed_work.idle_work, 
amdgpu_uvd_idle_work_handler);
  
  		r = amdgpu_bo_create_kernel(adev, bo_size, PAGE_SIZE,

AMDGPU_GEM_DOMAIN_VRAM, 
>uvd.inst[j].vcpu_bo,
@@ -318,7 +318,7 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev)
if (adev->uvd.inst[j].vcpu_bo == NULL)
continue;
  
-		cancel_delayed_work_sync(>uvd.inst[j].delayed_work.idle_work);

+   cancel_delayed_work_sync(>uvd.inst[j].idle_work);
  
  		/* only valid for physical mode */

if (adev->asic_type < CHIP_POLARIS10) {
@@ -1144,10 +1144,9 @@ int amdgpu_uvd_get_destroy_msg(struct amdgpu_ring *ring, 
uint32_t handle,
  
  static void amdgpu_uvd_idle_work_handler(struct work_struct *work)

  {
-   struct amdgpu_delayed_work *my_work = (struct amdgpu_delayed_work 
*)work;
struct amdgpu_device *adev =
-   container_of(work, struct amdgpu_device, 
uvd.inst[my_work->ip_instance].delayed_work.idle_work.work);
-   unsigned fences = 
amdgpu_fence_count_emitted(>uvd.inst[my_work->ip_instance].ring);
+   container_of(work, struct amdgpu_device, 
uvd.inst->idle_work.work);
+   unsigned fences = amdgpu_fence_count_emitted(>uvd.inst->ring);
  
  	if (fences == 0) {

if (adev->pm.dpm_enabled) {
@@ -1161,7 +1160,7 @@ static void amdgpu_uvd_idle_work_handler(struct 
work_struct *work)
   
AMD_CG_STATE_GATE);
}
} else {
-   
schedule_delayed_work(>uvd.inst[my_work->ip_instance].delayed_work.idle_work,
 UVD_IDLE_TIMEOUT);
+   schedule_delayed_work(>uvd.inst->idle_work, 
UVD_IDLE_TIMEOUT);
}
  }
  
@@ -1173,7 +1172,7 @@ void amdgpu_uvd_ring_begin_use(struct amdgpu_ring *ring)

if (amdgpu_sriov_vf(adev))
return;
  
-	set_clocks = !cancel_delayed_work_sync(>uvd.inst[ring->me].delayed_work.idle_work);

+   set_clocks = !cancel_delayed_work_sync(>uvd.inst->idle_work);
if (set_clocks) {
if (adev->pm.dpm_enabled) {
amdgpu_dpm_enable_uvd(adev, true);
@@ -1190,7 +1189,7 @@ void amdgpu_uvd_ring_begin_use(struct amdgpu_ring *ring)
  void amdgpu_uvd_ring_end_use(struct amdgpu_ring *ring)
  {
if (!amdgpu_sriov_vf(ring->adev))
-   
schedule_delayed_work(>adev->uvd.inst[ring->me].delayed_work.idle_work, 
UVD_IDLE_TIMEOUT);
+   schedule_delayed_work(>adev->uvd.inst->idle_work, 
UVD_IDLE_TIMEOUT);
  }
  
  /**

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
index 7801eb8d4199..b1579fba134c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
@@ -37,11 +37,6 @@
(AMDGPU_GPU_PAGE_ALIGN(le32_to_cpu(((const struct common_firmware_header 
*)(adev)->uvd.fw->data)->ucode_size_bytes) + \
   8) - AMDGPU_UVD_FIRMWARE_OFFSET)
  
-struct amdgpu_delayed_work{

-   struct delayed_work idle_work;
-   unsigned ip_instance;
-};
-
  struct amdgpu_uvd_inst {
struct amdgpu_bo*vcpu_bo;
void*cpu_addr;
@@ -49,12 +44,12 @@ struct amdgpu_uvd_inst {
void*saved_bo;
atomic_thandles[AMDGPU_MAX_UVD_HANDLES];
struct drm_file *filp[AMDGPU_MAX_UVD_HANDLES];
+   struct delayed_work idle_work;
struct amdgpu_ring  ring;
struct amdgpu_ring  ring_enc[AMDGPU_MAX_UVD_ENC_RINGS];
  

RE: [PATCH] drm/amdgpu: add rcu_barrier after entity fini

[Deng, Emily]

Hi Christian,
When we free an IB fence, we first call one call_rcu in 
drm_sched_fence_release_scheduled as the call trace one, then after the call 
trace one,
 we call the call_rcu second in the  amdgpu_fence_release in call trace two, as 
below.

 The kmem_cache_free(amdgpu_fence_slab, fence)'s call trace as below:
1.drm_sched_entity_fini ->
drm_sched_entity_cleanup ->
dma_fence_put(entity->last_scheduled) ->
drm_sched_fence_release_finished ->
drm_sched_fence_release_scheduled ->
call_rcu(>finished.rcu, drm_sched_fence_free)

2.drm_sched_fence_free ->
dma_fence_put(fence->parent) ->
amdgpu_fence_release ->
call_rcu(>rcu, amdgpu_fence_free) ->
kmem_cache_free(amdgpu_fence_slab, fence);

> -Original Message-
> From: Koenig, Christian
> Sent: Friday, May 18, 2018 5:46 PM
> To: Deng, Emily ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: add rcu_barrier after entity fini
>
> Ok, I'm lost where do we use call_rcu() twice? Cause that sounds incorrect in
> the first place.
>
> Christian.
>
> Am 18.05.2018 um 11:41 schrieb Deng, Emily:
> > Ping..
> >
> > Best Wishes,
> > Emily Deng
> >
> >
> >
> >
> >> -Original Message-
> >> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On
> >> Behalf Of Deng, Emily
> >> Sent: Friday, May 18, 2018 11:20 AM
> >> To: Koenig, Christian 
> >> >; amd-
> >> g...@lists.freedesktop.org
> >> Subject: RE: [PATCH] drm/amdgpu: add rcu_barrier after entity fini
> >>
> >> Hi Christian,
> >>   Yes, it has already one rcu_barrier, but it has called twice
> >> call_rcu, so the one rcu_barrier just could barrier one call_rcu some time.
> >>  After I added another rcu_barrier, the kernel issue will disappear.
> >>
> >> Best Wishes,
> >> Emily Deng
> >>
> >>> -Original Message-
> >>> From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com]
> >>> Sent: Thursday, May 17, 2018 7:08 PM
> >>> To: Deng, Emily >; 
> >>> amd-
> g...@lists.freedesktop.org
> >>> Subject: Re: [PATCH] drm/amdgpu: add rcu_barrier after entity fini
> >>>
> >>> Am 17.05.2018 um 12:03 schrieb Emily Deng:
>  To free the fence from the amdgpu_fence_slab, need twice call_rcu,
>  to avoid the amdgpu_fence_slab_fini call
>  kmem_cache_destroy(amdgpu_fence_slab) before
> >>> kmem_cache_free(amdgpu_fence_slab, fence), add rcu_barrier after
> >>> drm_sched_entity_fini.
>  The kmem_cache_free(amdgpu_fence_slab, fence)'s call trace as
> below:
>  1.drm_sched_entity_fini ->
>  drm_sched_entity_cleanup ->
>  dma_fence_put(entity->last_scheduled) ->
>  drm_sched_fence_release_finished ->
> >>> drm_sched_fence_release_scheduled
>  -> call_rcu(>finished.rcu, drm_sched_fence_free)
> 
>  2.drm_sched_fence_free ->
>  dma_fence_put(fence->parent) ->
>  amdgpu_fence_release ->
>  call_rcu(>rcu, amdgpu_fence_free) ->
>  kmem_cache_free(amdgpu_fence_slab, fence);
> 
>  v2:put the barrier before the kmem_cache_destroy
> 
>  Change-Id: I8dcadd3372f97e72461bf46b41cc26d90f09b8df
>  Signed-off-by: Emily Deng >
>  ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 1 +
> 1 file changed, 1 insertion(+)
> 
>  diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>  b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>  index 39ec6b8..42be65b 100644
>  --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>  +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>  @@ -69,6 +69,7 @@ int amdgpu_fence_slab_init(void)
> void amdgpu_fence_slab_fini(void)
> {
>   rcu_barrier();
>  +rcu_barrier();
> >>> Well, you should have noted that there is already an rcu_barrier
> >>> here and adding another one shouldn't have any additional effect. So
> >>> your explanation and the proposed solution doesn't make to much
> sense.
> >>>
> >>> I think the problem you run into is rather that the fence is
> >>> reference counted and might live longer than the module who created it.
> >>>
> >>> Complicated issue, one possible solution would be to release
> >>> fence->parent earlier in the scheduler fence but that doesn't sound
> >>> fence->like
> >>> a general purpose solution.
> >>>
> >>> Christian.
> >>>
>   kmem_cache_destroy(amdgpu_fence_slab);
> }
> /*
> >> ___
> >> 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: consider user preference when pinning for SG display

[Christian König]

Am 17.05.2018 um 18:55 schrieb Alex Deucher:

If the pin domain is set to GTT | VRAM, look at the preferred domains
for the bo and respect that if it's been set explicitly.

Signed-off-by: Alex Deucher 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 6a9e46ae7f0a..16192f17653e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -704,9 +704,14 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
domain,
 * See function amdgpu_display_supported_domains()
 */
if (domain == (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
-   domain = AMDGPU_GEM_DOMAIN_VRAM;
-   if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
+   if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM)
+   domain = AMDGPU_GEM_DOMAIN_VRAM; /* if user really 
wants vram, respect it */
+   else if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_GTT)
+   domain = AMDGPU_GEM_DOMAIN_GTT; /* if user really wants 
gtt, respect it */
+   else if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
domain = AMDGPU_GEM_DOMAIN_GTT;
+   else
+   domain = AMDGPU_GEM_DOMAIN_VRAM;
}
  
  	if (bo->pin_count) {


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

Re: [PATCH 1/6] drm/amdgpu: rework VM state machine lock handling v2

[Zhang, Jerry (Junwei)]

2, 3, 4, 5 are
Reviewed-by: Junwei Zhang 

Patch 1:
could you show the reserving VM?

Patch 6:
I could read that code, but not sure the purpose.

Jerry

On 05/17/2018 05:49 PM, Christian König wrote:

Only the moved state needs a separate spin lock protection. All other
states are protected by reserving the VM anyway.

v2: fix some more incorrect cases

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 66 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 +--
  2 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 1a8f4e0dd023..f0deedcaf1c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -119,9 +119,7 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base 
*base,
 * is currently evicted. add the bo to the evicted list to make sure it
 * is validated on next vm use to avoid fault.
 * */
-   spin_lock(>status_lock);
list_move_tail(>vm_status, >evicted);
-   spin_unlock(>status_lock);
  }

  /**
@@ -228,7 +226,6 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
struct ttm_bo_global *glob = adev->mman.bdev.glob;
int r;

-   spin_lock(>status_lock);
while (!list_empty(>evicted)) {
struct amdgpu_vm_bo_base *bo_base;
struct amdgpu_bo *bo;
@@ -236,10 +233,8 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
bo_base = list_first_entry(>evicted,
   struct amdgpu_vm_bo_base,
   vm_status);
-   spin_unlock(>status_lock);

bo = bo_base->bo;
-   BUG_ON(!bo);
if (bo->parent) {
r = validate(param, bo);
if (r)
@@ -259,13 +254,14 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
return r;
}

-   spin_lock(>status_lock);
-   if (bo->tbo.type != ttm_bo_type_kernel)
+   if (bo->tbo.type != ttm_bo_type_kernel) {
+   spin_lock(>moved_lock);
list_move(_base->vm_status, >moved);
-   else
+   spin_unlock(>moved_lock);
+   } else {
list_move(_base->vm_status, >relocated);
+   }
}
-   spin_unlock(>status_lock);

return 0;
  }
@@ -279,13 +275,7 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
   */
  bool amdgpu_vm_ready(struct amdgpu_vm *vm)
  {
-   bool ready;
-
-   spin_lock(>status_lock);
-   ready = list_empty(>evicted);
-   spin_unlock(>status_lock);
-
-   return ready;
+   return list_empty(>evicted);
  }

  /**
@@ -477,9 +467,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device 
*adev,
pt->parent = amdgpu_bo_ref(parent->base.bo);

amdgpu_vm_bo_base_init(>base, vm, pt);
-   spin_lock(>status_lock);
list_move(>base.vm_status, >relocated);
-   spin_unlock(>status_lock);
}

if (level < AMDGPU_VM_PTB) {
@@ -926,10 +914,8 @@ static void amdgpu_vm_invalidate_level(struct 
amdgpu_device *adev,
if (!entry->base.bo)
continue;

-   spin_lock(>status_lock);
if (list_empty(>base.vm_status))
list_add(>base.vm_status, >relocated);
-   spin_unlock(>status_lock);
amdgpu_vm_invalidate_level(adev, vm, entry, level + 1);
}
  }
@@ -974,7 +960,6 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
params.func = amdgpu_vm_do_set_ptes;
}

-   spin_lock(>status_lock);
while (!list_empty(>relocated)) {
struct amdgpu_vm_bo_base *bo_base, *parent;
struct amdgpu_vm_pt *pt, *entry;
@@ -984,13 +969,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device 
*adev,
   struct amdgpu_vm_bo_base,
   vm_status);
list_del_init(_base->vm_status);
-   spin_unlock(>status_lock);

bo = bo_base->bo->parent;
-   if (!bo) {
-   spin_lock(>status_lock);
+   if (!bo)
continue;
-   }

parent = list_first_entry(>va, struct amdgpu_vm_bo_base,
  bo_list);
@@ -999,12 +981,10 @@ int amdgpu_vm_update_directories(struct amdgpu_device 

Re: [PATCH] Remove calls to DAL suspend/resume from amdgpu_device_gpu_recover.

[Christian König]

Am 17.05.2018 um 20:41 schrieb Harry Wentland:

On 2018-05-17 11:50 AM, Andrey Grodzovsky wrote:

First of all it's already being called from the display code from 
amd_ip_funcs.suspend/resume hooks.
Second of all, the place in amdgpu_device_gpu_recover it's being called is 
wrong for GPU stalls since
it is called BEFORE we cancel and force completion of all jobs which were not 
yet processed.
So, as Bas pointed in the ticket we will try to wait for fence  in 
amdgpu_pm_compute_clocks but the pipe
is hanged so we end up in deadlock.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106500
Signed-off-by: Andrey Grodzovsky 

Reviewed-by: Harry Wentland 


Reviewed-by: Christian König 



Harry


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 9 +
  1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index fcd4bb2..f5c0a2d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3200,10 +3200,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
/* block TTM */
resched = ttm_bo_lock_delayed_workqueue(>mman.bdev);
  
-	/* store modesetting */

-   if (amdgpu_device_has_dc_support(adev))
-   state = drm_atomic_helper_suspend(adev->ddev);
-
/* block all schedulers and reset given job's ring */
for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
struct amdgpu_ring *ring = adev->rings[i];
@@ -3243,10 +3239,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
kthread_unpark(ring->sched.thread);
}
  
-	if (amdgpu_device_has_dc_support(adev)) {

-   if (drm_atomic_helper_resume(adev->ddev, state))
-   dev_info(adev->dev, "drm resume failed:%d\n", r);
-   } else {
+   if (!amdgpu_device_has_dc_support(adev)) {
drm_helper_resume_force_mode(adev->ddev);
}
  


___
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/amd/amdgpu: Code comments for the amdgpu_ttm.c driver. (v2)

[Christian König]

Am 17.05.2018 um 17:34 schrieb Alex Deucher:

On Tue, May 15, 2018 at 10:02 AM, Tom St Denis  wrote:

NFC just comments.

(v2):  Updated based on feedback from Alex Deucher.

Signed-off-by: Tom St Denis 

Reviewed-by: Alex Deucher 


Just one comment "Pin pages of memory pointed to..." better write "Grab 
a reference to the memory pointed to...".


get_user_pages() does not pin anything! I've heard that misconception so 
many times now that I can't remember how often we had to explain it and 
we should definitely not leak it into the documentation.


With that fixed the patch is Reviewed-by: Christian König 
.


Regards,
Christian.




---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 347 +++-
  1 file changed, 340 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index dfd22db13fb1..2eaaa1fb7b59 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -63,16 +63,44 @@ static void amdgpu_ttm_debugfs_fini(struct amdgpu_device 
*adev);
  /*
   * Global memory.
   */
+
+/**
+ * amdgpu_ttm_mem_global_init - Initialize and acquire reference to
+ * memory object
+ *
+ * @ref: Object for initialization.
+ *
+ * This is called by drm_global_item_ref() when an object is being
+ * initialized.
+ */
  static int amdgpu_ttm_mem_global_init(struct drm_global_reference *ref)
  {
 return ttm_mem_global_init(ref->object);
  }

+/**
+ * amdgpu_ttm_mem_global_release - Drop reference to a memory object
+ *
+ * @ref: Object being removed
+ *
+ * This is called by drm_global_item_unref() when an object is being
+ * released.
+ */
  static void amdgpu_ttm_mem_global_release(struct drm_global_reference *ref)
  {
 ttm_mem_global_release(ref->object);
  }

+/**
+ * amdgpu_ttm_global_init - Initialize global TTM memory reference
+ * structures.
+ *
+ * @adev:  AMDGPU device for which the global structures need to be
+ * registered.
+ *
+ * This is called as part of the AMDGPU ttm init from amdgpu_ttm_init()
+ * during bring up.
+ */
  static int amdgpu_ttm_global_init(struct amdgpu_device *adev)
  {
 struct drm_global_reference *global_ref;
@@ -80,7 +108,9 @@ static int amdgpu_ttm_global_init(struct amdgpu_device *adev)
 struct drm_sched_rq *rq;
 int r;

+   /* ensure reference is false in case init fails */
 adev->mman.mem_global_referenced = false;
+
 global_ref = >mman.mem_global_ref;
 global_ref->global_type = DRM_GLOBAL_TTM_MEM;
 global_ref->size = sizeof(struct ttm_mem_global);
@@ -146,6 +176,18 @@ static int amdgpu_invalidate_caches(struct ttm_bo_device 
*bdev, uint32_t flags)
 return 0;
  }

+/**
+ * amdgpu_init_mem_type -  Initialize a memory manager for a specific
+ * type of memory request.
+ *
+ * @bdev:  The TTM BO device object (contains a reference to
+ * amdgpu_device)
+ * @type:  The type of memory requested
+ * @man:
+ *
+ * This is called by ttm_bo_init_mm() when a buffer object is being
+ * initialized.
+ */
  static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 struct ttm_mem_type_manager *man)
  {
@@ -161,6 +203,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
 man->default_caching = TTM_PL_FLAG_CACHED;
 break;
 case TTM_PL_TT:
+   /* GTT memory  */
 man->func = _gtt_mgr_func;
 man->gpu_offset = adev->gmc.gart_start;
 man->available_caching = TTM_PL_MASK_CACHING;
@@ -193,6 +236,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device 
*bdev, uint32_t type,
 return 0;
  }

+/**
+ * amdgpu_evict_flags - Compute placement flags
+ *
+ * @bo: The buffer object to evict
+ * @placement: Possible destination(s) for evicted BO
+ *
+ * Fill in placement data when ttm_bo_evict() is called
+ */
  static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
 struct ttm_placement *placement)
  {
@@ -204,12 +255,14 @@ static void amdgpu_evict_flags(struct ttm_buffer_object 
*bo,
 .flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM
 };

+   /* Don't handle scatter gather BOs */
 if (bo->type == ttm_bo_type_sg) {
 placement->num_placement = 0;
 placement->num_busy_placement = 0;
 return;
 }

+   /* Object isn't an AMDGPU object so ignore */
 if (!amdgpu_ttm_bo_is_amdgpu_bo(bo)) {
 placement->placement = 
 placement->busy_placement = 
@@ -217,10 +270,12 @@ static void 

Re: [PATCH] drm/amdgpu: add rcu_barrier after entity fini

[Christian König]

Ok, I'm lost where do we use call_rcu() twice? Cause that sounds 
incorrect in the first place.


Christian.

Am 18.05.2018 um 11:41 schrieb Deng, Emily:

Ping..

Best Wishes,
Emily Deng





-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
Of Deng, Emily
Sent: Friday, May 18, 2018 11:20 AM
To: Koenig, Christian ; amd-
g...@lists.freedesktop.org
Subject: RE: [PATCH] drm/amdgpu: add rcu_barrier after entity fini

Hi Christian,
  Yes, it has already one rcu_barrier, but it has called twice call_rcu, so 
the
one rcu_barrier just could barrier one call_rcu some time.
 After I added another rcu_barrier, the kernel issue will disappear.

Best Wishes,
Emily Deng


-Original Message-
From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com]
Sent: Thursday, May 17, 2018 7:08 PM
To: Deng, Emily ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: add rcu_barrier after entity fini

Am 17.05.2018 um 12:03 schrieb Emily Deng:

To free the fence from the amdgpu_fence_slab, need twice call_rcu,
to avoid the amdgpu_fence_slab_fini call
kmem_cache_destroy(amdgpu_fence_slab) before

kmem_cache_free(amdgpu_fence_slab, fence), add rcu_barrier after
drm_sched_entity_fini.

The kmem_cache_free(amdgpu_fence_slab, fence)'s call trace as below:
1.drm_sched_entity_fini ->
drm_sched_entity_cleanup ->
dma_fence_put(entity->last_scheduled) ->
drm_sched_fence_release_finished ->

drm_sched_fence_release_scheduled

-> call_rcu(>finished.rcu, drm_sched_fence_free)

2.drm_sched_fence_free ->
dma_fence_put(fence->parent) ->
amdgpu_fence_release ->
call_rcu(>rcu, amdgpu_fence_free) ->
kmem_cache_free(amdgpu_fence_slab, fence);

v2:put the barrier before the kmem_cache_destroy

Change-Id: I8dcadd3372f97e72461bf46b41cc26d90f09b8df
Signed-off-by: Emily Deng 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 39ec6b8..42be65b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -69,6 +69,7 @@ int amdgpu_fence_slab_init(void)
   void amdgpu_fence_slab_fini(void)
   {
rcu_barrier();
+   rcu_barrier();

Well, you should have noted that there is already an rcu_barrier here
and adding another one shouldn't have any additional effect. So your
explanation and the proposed solution doesn't make to much sense.

I think the problem you run into is rather that the fence is reference
counted and might live longer than the module who created it.

Complicated issue, one possible solution would be to release
fence->parent earlier in the scheduler fence but that doesn't sound
fence->like
a general purpose solution.

Christian.


kmem_cache_destroy(amdgpu_fence_slab);
   }
   /*

___
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: add rcu_barrier after entity fini

[Deng, Emily]

Ping..

Best Wishes,
Emily Deng




> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Deng, Emily
> Sent: Friday, May 18, 2018 11:20 AM
> To: Koenig, Christian ; amd-
> g...@lists.freedesktop.org
> Subject: RE: [PATCH] drm/amdgpu: add rcu_barrier after entity fini
> 
> Hi Christian,
>  Yes, it has already one rcu_barrier, but it has called twice call_rcu, 
> so the
> one rcu_barrier just could barrier one call_rcu some time.
> After I added another rcu_barrier, the kernel issue will disappear.
> 
> Best Wishes,
> Emily Deng
> 
> > -Original Message-
> > From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com]
> > Sent: Thursday, May 17, 2018 7:08 PM
> > To: Deng, Emily ; amd-gfx@lists.freedesktop.org
> > Subject: Re: [PATCH] drm/amdgpu: add rcu_barrier after entity fini
> >
> > Am 17.05.2018 um 12:03 schrieb Emily Deng:
> > > To free the fence from the amdgpu_fence_slab, need twice call_rcu,
> > > to avoid the amdgpu_fence_slab_fini call
> > > kmem_cache_destroy(amdgpu_fence_slab) before
> > kmem_cache_free(amdgpu_fence_slab, fence), add rcu_barrier after
> > drm_sched_entity_fini.
> > >
> > > The kmem_cache_free(amdgpu_fence_slab, fence)'s call trace as below:
> > > 1.drm_sched_entity_fini ->
> > > drm_sched_entity_cleanup ->
> > > dma_fence_put(entity->last_scheduled) ->
> > > drm_sched_fence_release_finished ->
> > drm_sched_fence_release_scheduled
> > > -> call_rcu(>finished.rcu, drm_sched_fence_free)
> > >
> > > 2.drm_sched_fence_free ->
> > > dma_fence_put(fence->parent) ->
> > > amdgpu_fence_release ->
> > > call_rcu(>rcu, amdgpu_fence_free) ->
> > > kmem_cache_free(amdgpu_fence_slab, fence);
> > >
> > > v2:put the barrier before the kmem_cache_destroy
> > >
> > > Change-Id: I8dcadd3372f97e72461bf46b41cc26d90f09b8df
> > > Signed-off-by: Emily Deng 
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > > index 39ec6b8..42be65b 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > > @@ -69,6 +69,7 @@ int amdgpu_fence_slab_init(void)
> > >   void amdgpu_fence_slab_fini(void)
> > >   {
> > >   rcu_barrier();
> > > + rcu_barrier();
> >
> > Well, you should have noted that there is already an rcu_barrier here
> > and adding another one shouldn't have any additional effect. So your
> > explanation and the proposed solution doesn't make to much sense.
> >
> > I think the problem you run into is rather that the fence is reference
> > counted and might live longer than the module who created it.
> >
> > Complicated issue, one possible solution would be to release
> > fence->parent earlier in the scheduler fence but that doesn't sound
> > fence->like
> > a general purpose solution.
> >
> > Christian.
> >
> > >   kmem_cache_destroy(amdgpu_fence_slab);
> > >   }
> > >   /*
> 
> ___
> 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: Add profiling mode setting on RV

[Rex Zhu]

For power saving, default profiling mode was setted
to power saving mode.

Currently, not support CUSTOM mode and not display
detailed profiling mode parameters in sysfs.

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

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
index 0882ecf..409a46b 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
@@ -450,6 +450,10 @@ static int smu10_hwmgr_backend_init(struct pp_hwmgr *hwmgr)
 
hwmgr->backend = data;
 
+   hwmgr->workload_mask = 1 << 
hwmgr->workload_prority[PP_SMC_POWER_PROFILE_POWERSAVING];
+   hwmgr->power_profile_mode = PP_SMC_POWER_PROFILE_POWERSAVING;
+   hwmgr->default_power_profile_mode = PP_SMC_POWER_PROFILE_POWERSAVING;
+
result = smu10_initialize_dpm_defaults(hwmgr);
if (result != 0) {
pr_err("smu10_initialize_dpm_defaults failed\n");
@@ -1163,6 +1167,42 @@ static void smu10_powergate_vcn(struct pp_hwmgr *hwmgr, 
bool bgate)
}
 }
 
+static int smu10_get_power_profile_mode(struct pp_hwmgr *hwmgr, char *buf)
+{
+   uint32_t i, size = 0;
+
+   static const char *profile_name[6] = {"3D_FULL_SCREEN",
+   "POWER_SAVING",
+   "VIDEO",
+   "VR",
+   "COMPUTE"};
+   static const char *title[2] = {"NUM", "MODE_NAME"};
+
+   if (!buf)
+   return -EINVAL;
+
+   size += sprintf(buf + size, "%s %10s\n", title[0], title[1]);
+
+   for (i = 0; i < PP_SMC_POWER_PROFILE_CUSTOM; i++)
+   size += sprintf(buf + size, "%3d %14s%s\n",
+   i, profile_name[i],
+   (i == hwmgr->power_profile_mode) ? "*" : " ");
+   return size;
+}
+
+static int smu10_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, 
uint32_t size)
+{
+   if (input[size] >= PP_SMC_POWER_PROFILE_CUSTOM)
+   return -EINVAL;
+
+   hwmgr->power_profile_mode = input[size];
+
+   smum_send_msg_to_smc_with_parameter(hwmgr, 
PPSMC_MSG_ActiveProcessNotify,
+   1

[PATCH 2/2] drm/amdgpu: Auto switch to video profiling mode on VCN demand

[Rex Zhu]

Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 3549481..94b221f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -207,11 +207,13 @@ static void amdgpu_vcn_idle_work_handler(struct 
work_struct *work)
unsigned fences = amdgpu_fence_count_emitted(>vcn.ring_dec);
 
if (fences == 0) {
-   if (adev->pm.dpm_enabled)
+   if (adev->pm.dpm_enabled) {
+   amdgpu_dpm_switch_power_profile(adev, 
PP_SMC_POWER_PROFILE_VIDEO, false);
amdgpu_dpm_enable_uvd(adev, false);
-   else
+   } else {
amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
   
AMD_PG_STATE_GATE);
+   }
} else {
schedule_delayed_work(>vcn.idle_work, VCN_IDLE_TIMEOUT);
}
@@ -223,11 +225,13 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
bool set_clocks = !cancel_delayed_work_sync(>vcn.idle_work);
 
if (set_clocks && adev->pm.dpm_enabled) {
-   if (adev->pm.dpm_enabled)
+   if (adev->pm.dpm_enabled) {
amdgpu_dpm_enable_uvd(adev, true);
-   else
+   amdgpu_dpm_switch_power_profile(adev, 
PP_SMC_POWER_PROFILE_VIDEO, true);
+   } else {
amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
   
AMD_PG_STATE_UNGATE);
+   }
}
 }
 
-- 
1.9.1

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

Re: [PATCH] drm/amdgpu: consider user preference when pinning for SG display

[Michel Dänzer]

On 2018-05-17 06:55 PM, Alex Deucher wrote:
> If the pin domain is set to GTT | VRAM, look at the preferred domains
> for the bo and respect that if it's been set explicitly.
> 
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 6a9e46ae7f0a..16192f17653e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -704,9 +704,14 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
> domain,
>* See function amdgpu_display_supported_domains()
>*/
>   if (domain == (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
> - domain = AMDGPU_GEM_DOMAIN_VRAM;
> - if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
> + if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM)
> + domain = AMDGPU_GEM_DOMAIN_VRAM; /* if user really 
> wants vram, respect it */
> + else if (bo->preferred_domains == AMDGPU_GEM_DOMAIN_GTT)
> + domain = AMDGPU_GEM_DOMAIN_GTT; /* if user really wants 
> gtt, respect it */

I'd spell VRAM and GTT in capital letters in the comments.


> + else if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
>   domain = AMDGPU_GEM_DOMAIN_GTT;
> + else
> + domain = AMDGPU_GEM_DOMAIN_VRAM;
>   }

Is everything in place to deal with any issues that might occur when
flipping between buffers in VRAM and GTT?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 00/13] Enabling Color Management - Round 2

[Michel Dänzer]

On 2018-05-17 11:43 PM, Leo Li wrote:
> On 2018-05-16 01:06 PM, Michel Dänzer wrote:
>> On 2018-05-03 08:31 PM, sunpeng...@amd.com wrote:
>>>
>>> 3. The three color management properties (Degamma LUT, Color
>>> Transform Matrix
>>>     (CTM), and Gamma LUT) are hard-coded into the DDX driver, to be
>>> listed (as
>>>     disabled) regardless of whether a CRTC is attached on the output,
>>> or whether
>>>     the kernel driver supports it.
>>>
>>>  * If kernel driver does not support color management, the
>>> properties will
>>>    remain disabled. A `xrandr --set` will then error.
>>
>> Is it really useful to expose these properties to clients if the kernel
>> doesn't support them?
>>
> 
> I left them exposed mainly for simplicity. I can see how it would
> confuse a client.
> 
> It should be simpler to hide these properties once the color property
> IDs are cached somewhere (maybe on the AMDGPUInfo struct?)

drmmode_crtc_private_rec seems better.


>>> 4. Color properties are now *staged* inside the driver-private CRTC
>>> object.
>>>     This allows us to *push* the properties into kernel DRM (and
>>> consequently
>>>     into hardware) whenever there is a need.
>>>
>>>  * Along with staging and pushing, an *update* function is used
>>> to notify
>>>    RandR to update the color properties listed on outputs. This
>>> can be used
>>>    when `xrandr --auto` enables a CRTC on an output, and the
>>> output need to
>>>    reflect the CRTC's color properties.
>>
>> I feel like some of this is a bit more complicated than necessary. This
>> is how I'd envision it working:
>>
>> In drmmode_crtc_init, query the properties from the kernel, more or less
>> as done in this series.
>>
>> In drmmode_output_create_resources, create the output properties (or
>> not, per above) based on output->crtc, or if that is NULL, based on
>> xf86_config->crtc[0].
> 
> What are the contents of xf86_config->crtc[0] initialized to? I'm not
> too familiar with how that's setup. Does it stay the same throughout?

I think so, but my point here is mainly that whether or not the
properties exist, and the size of the LUTs should be the same regardless
of which particular CRTC you look at. So for simplicity,
drmmode_output_create_resources could even always look at
xf86_config->crtc[0] (i.e. the first CRTC).


>> In drmmode_output_get_property, if output->crtc != NULL, update the
>> RandR property value from it, otherwise set it to a dummy value.
> 
> Feeling kind of stupid that I skipped over this :) I assumed it wasn't
> useful since it's currently a no-op. This should eliminate the need for
> the cm_update function, assuming it's called every time a client
> requests for the properties.
> 
> This should work even if one CRTC is attached to multiple outputs,
> correct? Since it would first update the RandR property with the
> attached CRTC before returning it.

Yes, I think so.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 10/13] Push staged color properties when DPMS state toggles On

[Michel Dänzer]

On 2018-05-17 11:44 PM, Leo Li wrote:
> On 2018-05-16 01:10 PM, Michel Dänzer wrote:
>> On 2018-05-03 08:31 PM, sunpeng...@amd.com wrote:
>>> From: "Leo (Sunpeng) Li" 
>>>
>>> This will persist color management properties on a CRTC across DPMS
>>> state changes.
>>>
>>> Signed-off-by: Leo (Sunpeng) Li 
>>> ---
>>>   src/drmmode_display.c | 6 ++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>>> index 45c582c..06ae902 100644
>>> --- a/src/drmmode_display.c
>>> +++ b/src/drmmode_display.c
>>> @@ -1294,6 +1294,7 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
>>>   AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
>>>   CARD64 ust;
>>>   int ret;
>>> +    int i;
>>>     if (drmmode_crtc->dpms_mode == DPMSModeOn && mode !=
>>> DPMSModeOn) {
>>>   uint32_t seq;
>>> @@ -1341,6 +1342,11 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
>>>   drmmode_crtc->interpolated_vblanks += delta_seq;
>>>   }
>>>   +    for (i = 0; i < CM_NUM_PROPS; i++) {
>>> +    if (i == CM_GAMMA_LUT_SIZE || i == CM_DEGAMMA_LUT_SIZE)
>>> +    continue;
>>> +    drmmode_crtc_push_cm_prop(crtc, i);
>>> +    }
>>>   }
>>>   drmmode_crtc->dpms_mode = mode;
>>>   }
>>>
>>
>> This and patch 11 smell like workarounds for a kernel issue. The kernel
>> should preserve the property values regardless of DPMS state.
>>
>> This probably explains something I just discovered: the legacy gamma LUT
>> becomes ineffective after turning a CRTC off and on again with DC,
>> whereas it's preserved without DC.
> 
> That's indeed a kernel issue, will look into it. This patch can be
> dropped once the kernel persists the properties across dpms.
> 
> In terms of Patch 11, which persists the properties across hotplugs, is
> it even a valid use-case? I tested with i915 and amdgpu non-dc drivers.
> Both don't seem to persist legacy gamma across hotplugs, or xrandr
> --output --off/--auto.

It persists for me (without this series) with amdgpu without DC on
xrandr --output --off/--auto, or explicitly moving an output between
different CRTCs. Those are definitely expected.

Not sure about hotplug; it's possible that something explicitly
re-initializes the legacy gamma in that case. But other than that, each
CRTC should preserve its values unless they are explicitly modified.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 02/13] Push color properties to kernel DRM on CRTC init

[Michel Dänzer]

On 2018-05-17 11:43 PM, Leo Li wrote:
> On 2018-05-16 01:08 PM, Michel Dänzer wrote:
>> On 2018-05-03 08:31 PM, sunpeng...@amd.com wrote:
>>>
>>> @@ -1458,6 +1586,14 @@ drmmode_crtc_init(ScrnInfoPtr pScrn,
>>> drmmode_ptr drmmode, drmModeResPtr mode_res
>>>   drmmode_crtc->ctm->matrix[0] = drmmode_crtc->ctm->matrix[4] =
>>>   drmmode_crtc->ctm->matrix[8] = (uint64_t)1 << 32;
>>>   +    /* Push properties to initialize them */
>>> +    for (i = 0; i < CM_NUM_PROPS; i++) {
>>> +    if (i == CM_DEGAMMA_LUT_SIZE || i == CM_GAMMA_LUT_SIZE)
>>> +    continue;
>>> +    if (drmmode_crtc_push_cm_prop(crtc, i))
>>> +    return 0;
>>
>> Per my follow-up to the cover letter, I don't think this should be
>> necessary here anyway, but FWIW:
>>
>> Returning 0 early here breaks the pAMDGPUEnt->assigned_crtcs related
>> logic in this function and drmmode_pre_init.
> 
> I originally thought it'd make sense if DDX pushes the properties on
> init, to maintain consistency between what DDX initially reports, and
> what DRM is using. It would also reset color properties if X was restarted.

My point was that this will happen from drmmode_set_mode_major anyway,
therefore no need to do it here.

OTOH if the properties are preserved by the kernel, it might make sense
to only do it here and from drmmode_crtc_gamma_set, not from
drmmode_set_mode_major.


> The other way around could also work, which I assume is what you're
> suggesting? i.e. pull all color properties from DRM into the
> driver-private crtc on crtc init, instead of pushing it to kernel?

That's not what I meant, and I suspect that would be tricky. E.g. how
could we separate the pulled values into the legacy and advanced LUTs?
And what if console (or whatever was displaying before) was using a
different colour depth (could even be 8 bpp pseudo colour)?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amd/pp: fix a couple locking issues

[Rex Zhu]

We should return unlock on the error path

Signed-off-by: Rex Zhu 
---
 .../gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c   | 31 +-
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c
index 99b29ff..c952845 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c
@@ -936,45 +936,49 @@ int smu7_enable_didt_config(struct pp_hwmgr *hwmgr)
 
if (hwmgr->chip_id == CHIP_POLARIS10) {
result = 
smu7_program_pt_config_registers(hwmgr, GCCACConfig_Polaris10);
-   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", return result);
+   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", goto error);
result = 
smu7_program_pt_config_registers(hwmgr, DIDTConfig_Polaris10);
-   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", return result);
+   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", goto error);
} else if (hwmgr->chip_id == CHIP_POLARIS11) {
result = 
smu7_program_pt_config_registers(hwmgr, GCCACConfig_Polaris11);
-   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", return result);
+   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", goto error);
if (hwmgr->is_kicker)
result = 
smu7_program_pt_config_registers(hwmgr, DIDTConfig_Polaris11_Kicker);
else
result = 
smu7_program_pt_config_registers(hwmgr, DIDTConfig_Polaris11);
-   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", return result);
+   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", goto error);
} else if (hwmgr->chip_id == CHIP_POLARIS12) {
result = 
smu7_program_pt_config_registers(hwmgr, GCCACConfig_Polaris11);
-   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", return result);
+   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", goto error);
result = 
smu7_program_pt_config_registers(hwmgr, DIDTConfig_Polaris12);
-   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", return result);
+   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", goto error);
} else if (hwmgr->chip_id == CHIP_VEGAM) {
result = 
smu7_program_pt_config_registers(hwmgr, GCCACConfig_VegaM);
-   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", return result);
+   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", goto error);
result = 
smu7_program_pt_config_registers(hwmgr, DIDTConfig_VegaM);
-   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", return result);
+   PP_ASSERT_WITH_CODE((result == 0), "DIDT Config 
failed.", goto error);
}
}
cgs_write_register(hwmgr->device, mmGRBM_GFX_INDEX, value2);
 
result = smu7_enable_didt(hwmgr, true);
-   PP_ASSERT_WITH_CODE((result == 0), "EnableDiDt failed.", return 
result);
+   PP_ASSERT_WITH_CODE((result == 0), "EnableDiDt failed.", goto 
error);
 
if (hwmgr->chip_id == CHIP_POLARIS11) {
result = smum_send_msg_to_smc(hwmgr,

(uint16_t)(PPSMC_MSG_EnableDpmDidt));
PP_ASSERT_WITH_CODE((0 == result),
-   "Failed to enable DPM DIDT.", return 
result);
+   "Failed to enable DPM DIDT.", goto 
error);
}
mutex_unlock(>grbm_idx_mutex);
adev->gfx.rlc.funcs->exit_safe_mode(adev);
}
 
return 0;
+error:
+   mutex_unlock(>grbm_idx_mutex);
+   adev->gfx.rlc.funcs->exit_safe_mode(adev);
+   return result;
 }
 
 int smu7_disable_didt_config(struct pp_hwmgr *hwmgr)
@@ -992,17 +996,20 @@ int smu7_disable_didt_config(struct pp_hwmgr *hwmgr)
result = smu7_enable_didt(hwmgr, false);
PP_ASSERT_WITH_CODE((result == 0),
"Post DIDT enable clock gating failed.",
-   

Re: [PATCH 6/6] drm/amdgpu: move VM BOs on LRU again

[zhoucm1]

CPU overhead is increased a bit, but we can optimize it later, the 
series is Reviewed-by: Chunming Zhou 



On 2018年05月17日 17:49, Christian König wrote:

Move all BOs belonging to a VM on the LRU with every submission.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 28 +++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  3 +++
  2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f5dee4c6757c..ccba88cc8c54 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -251,6 +251,19 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, 
struct amdgpu_vm *vm,
}
}
  
+	spin_lock(>lru_lock);

+   list_for_each_entry(bo_base, >idle, vm_status) {
+   struct amdgpu_bo *bo = bo_base->bo;
+
+   if (!bo->parent)
+   continue;
+
+   ttm_bo_move_to_lru_tail(>tbo);
+   if (bo->shadow)
+   ttm_bo_move_to_lru_tail(>shadow->tbo);
+   }
+   spin_unlock(>lru_lock);
+
return r;
  }
  
@@ -965,7 +978,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,

   struct amdgpu_vm_bo_base,
   vm_status);
bo_base->moved = false;
-   list_del_init(_base->vm_status);
+   list_move(_base->vm_status, >idle);
  
  		bo = bo_base->bo->parent;

if (!bo)
@@ -1571,10 +1584,14 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 * the evicted list so that it gets validated again on the
 * next command submission.
 */
-   if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv &&
-   !(bo->preferred_domains &
-   amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type)))
-   list_add_tail(_va->base.vm_status, >evicted);
+   if (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
+   uint32_t mem_type = bo->tbo.mem.mem_type;
+
+   if (!(bo->preferred_domains & 
amdgpu_mem_type_to_domain(mem_type)))
+   list_add_tail(_va->base.vm_status, >evicted);
+   else
+   list_add(_va->base.vm_status, >idle);
+   }
  
  	list_splice_init(_va->invalids, _va->valids);

bo_va->cleared = clear;
@@ -2368,6 +2385,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
INIT_LIST_HEAD(>relocated);
spin_lock_init(>moved_lock);
INIT_LIST_HEAD(>moved);
+   INIT_LIST_HEAD(>idle);
INIT_LIST_HEAD(>freed);
  
  	/* create scheduler entity for page table updates */

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 0196b9a782f2..061b99a18cb8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -178,6 +178,9 @@ struct amdgpu_vm {
struct list_headmoved;
spinlock_t  moved_lock;
  
+	/* All BOs of this VM not currently in the state machine */

+   struct list_headidle;
+
/* BO mappings freed, but not yet updated in the PT */
struct list_headfreed;
  


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

[PATCH] drm/amd/pp: Fix static checker warning

[Rex Zhu]

error: uninitialized symbol ''

Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c   | 24 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  3 ++-
 drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c |  6 ++
 3 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
index ec38c9f..7047e29 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
@@ -1104,10 +1104,8 @@ int atomctrl_get_voltage_evv_on_sclk(
GetIndexIntoMasterTable(COMMAND, GetVoltageInfo),
(uint32_t *)_voltage_info_param_space);
 
-   if (0 != result)
-   return result;
-
-   *voltage = le16_to_cpu(((GET_EVV_VOLTAGE_INFO_OUTPUT_PARAMETER_V1_2 *)
+   *voltage = result ? 0 :
+   
le16_to_cpu(((GET_EVV_VOLTAGE_INFO_OUTPUT_PARAMETER_V1_2 *)

(_voltage_info_param_space))->usVoltageLevel);
 
return result;
@@ -1312,8 +1310,7 @@ int atomctrl_read_efuse(struct pp_hwmgr *hwmgr, uint16_t 
start_index,
result = amdgpu_atom_execute_table(adev->mode_info.atom_context,
GetIndexIntoMasterTable(COMMAND, ReadEfuseValue),
(uint32_t *)_param);
-   if (!result)
-   *efuse = le32_to_cpu(efuse_param.ulEfuseValue) & mask;
+   *efuse = result ? 0 : le32_to_cpu(efuse_param.ulEfuseValue) & mask;
 
return result;
 }
@@ -1354,11 +1351,8 @@ int atomctrl_get_voltage_evv_on_sclk_ai(struct pp_hwmgr 
*hwmgr, uint8_t voltage_
GetIndexIntoMasterTable(COMMAND, GetVoltageInfo),
(uint32_t *)_voltage_info_param_space);
 
-   if (0 != result)
-   return result;
-
-   *voltage = le32_to_cpu(((GET_EVV_VOLTAGE_INFO_OUTPUT_PARAMETER_V1_3 *)
-   
(_voltage_info_param_space))->ulVoltageLevel);
+   *voltage = result ? 0 :
+   le32_to_cpu(((GET_EVV_VOLTAGE_INFO_OUTPUT_PARAMETER_V1_3 
*)(_voltage_info_param_space))->ulVoltageLevel);
 
return result;
 }
@@ -1552,15 +1546,17 @@ void atomctrl_get_voltage_range(struct pp_hwmgr *hwmgr, 
uint32_t *max_vddc,
case CHIP_FIJI:
*max_vddc = le32_to_cpu(((ATOM_ASIC_PROFILING_INFO_V3_3 
*)profile)->ulMaxVddc/4);
*min_vddc = le32_to_cpu(((ATOM_ASIC_PROFILING_INFO_V3_3 
*)profile)->ulMinVddc/4);
-   break;
+   return;
case CHIP_POLARIS11:
case CHIP_POLARIS10:
case CHIP_POLARIS12:
*max_vddc = le32_to_cpu(((ATOM_ASIC_PROFILING_INFO_V3_6 
*)profile)->ulMaxVddc/100);
*min_vddc = le32_to_cpu(((ATOM_ASIC_PROFILING_INFO_V3_6 
*)profile)->ulMinVddc/100);
-   break;
-   default:
return;
+   default:
+   break;
}
}
+   *max_vddc = 0;
+   *min_vddc = 0;
 }
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 646c9e9..45e9b8c 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -860,7 +860,8 @@ static void smu7_setup_voltage_range_from_vbios(struct 
pp_hwmgr *hwmgr)
struct phm_ppt_v1_clock_voltage_dependency_table *dep_sclk_table;
struct phm_ppt_v1_information *table_info =
(struct phm_ppt_v1_information *)(hwmgr->pptable);
-   uint32_t min_vddc, max_vddc;
+   uint32_t min_vddc = 0;
+   uint32_t max_vddc = 0;
 
if (!table_info)
return;
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
index 64d33b7..d644a9b 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
@@ -283,11 +283,9 @@ int smu7_read_smc_sram_dword(struct pp_hwmgr *hwmgr, 
uint32_t smc_addr, uint32_t
 
result = smu7_set_smc_sram_address(hwmgr, smc_addr, limit);
 
-   if (result)
-   return result;
+   *value = result ? 0 : cgs_read_register(hwmgr->device, 
mmSMC_IND_DATA_11);
 
-   *value = cgs_read_register(hwmgr->device, mmSMC_IND_DATA_11);
-   return 0;
+   return result;
 }
 
 int smu7_write_smc_sram_dword(struct pp_hwmgr *hwmgr, uint32_t smc_addr, 
uint32_t value, uint32_t limit)
-- 
1.9.1

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