[PATCH] drm/amd/powerplay: issue no PPSMC_MSG_GetCurrPkgPwr on unsupported ASICs

[Evan Quan]

Otherwise, the error message prompted will confuse user.

Change-Id: I44b9f870a8663714d715a1d5bf2aa24abe75bb8e
Signed-off-by: Evan Quan 
---
 .../gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c  | 23 +++
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index d3c3b3512a16..5c6b71b356e7 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -3476,18 +3476,31 @@ static int smu7_get_pp_table_entry(struct pp_hwmgr 
*hwmgr,
 
 static int smu7_get_gpu_power(struct pp_hwmgr *hwmgr, u32 *query)
 {
+   struct amdgpu_device *adev = hwmgr->adev;
int i;
u32 tmp = 0;
 
if (!query)
return -EINVAL;
 
-   smum_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_GetCurrPkgPwr, 0);
-   tmp = cgs_read_register(hwmgr->device, mmSMC_MSG_ARG_0);
-   *query = tmp;
+   /*
+* PPSMC_MSG_GetCurrPkgPwr is not supported on:
+*  - Hawaii
+*  - Bonaire
+*  - Fiji
+*  - Tonga
+*/
+   if ((adev->asic_type != CHIP_HAWAII) &&
+   (adev->asic_type != CHIP_BONAIRE) &&
+   (adev->asic_type != CHIP_FIJI) &&
+   (adev->asic_type != CHIP_TONGA)) {
+   smum_send_msg_to_smc_with_parameter(hwmgr, 
PPSMC_MSG_GetCurrPkgPwr, 0);
+   tmp = cgs_read_register(hwmgr->device, mmSMC_MSG_ARG_0);
+   *query = tmp;
 
-   if (tmp != 0)
-   return 0;
+   if (tmp != 0)
+   return 0;
+   }
 
smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogStart);
cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC,
-- 
2.24.0

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

[Regression] Broken Raven Ridge firmware on kernel 5.3.11 and up

[Luya Tshimbalanga]

Hello team,

I filed a bug on the kernel with a broken Raven Ridge firmware resulting 
a blank screen thus unable to boot to either graphical and text mode. 
Could someone investigate the issue on:


https://bugzilla.kernel.org/show_bug.cgi?id=205521 which include the 
dmesg log


The firmware still works on kernel 5.3.9 I currently use.


Thanks in advance.

--
Luya Tshimbalanga
Fedora Design Team
Fedora Design Suite maintainer

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

[PATCH v2] drm/amd/display: Fix Apple dongle cannot be successfully detected

[Louis Li]

[Why]
External monitor cannot be displayed consistently, if connecting
via this Apple dongle (A1621, USB Type-C to HDMI).
By experiments, it is confirmed that the dongle needs 200ms at least
to be ready for communication, after it sets HPD signal high.

[How]
When receiving HPD IRQ, delay 300ms at the beginning of handle_hpd_irq().
Then run the original procedure.
With this patch applied, the problem cannot be reproduced.
With other dongles, test results are PASS.
Test result is PASS to plug in HDMI cable while playing video,
and the video is being playing smoothly.
Test result is PASS after system resumes from suspend.

Signed-off-by: Louis Li 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 0aef92b7c037..5b844b6a5a59 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1025,6 +1025,11 @@ static void handle_hpd_irq(void *param)
struct drm_device *dev = connector->dev;
enum dc_connection_type new_connection_type = dc_connection_none;
 
+   /* Some monitors/dongles need around 200ms to be ready for communication
+* after those devices drive HPD signal high.
+*/
+   msleep(300);
+
/* In case of failure or MST no need to update connector status or 
notify the OS
 * since (for MST case) MST does this in it's own context.
 */
-- 
2.21.0

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

[PATCH] drm/amdkfd: remove set but not used variable 'top_dev'

[zhengbin]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/amdkfd/kfd_iommu.c: In function kfd_iommu_device_init:
drivers/gpu/drm/amd/amdkfd/kfd_iommu.c:65:30: warning: variable top_dev set but 
not used [-Wunused-but-set-variable]

Reported-by: Hulk Robot 
Fixes: 1ae99eab34f9 ("drm/amdkfd: Initialize HSA_CAP_ATS_PRESENT capability in 
topology codes")
Signed-off-by: zhengbin 
---
 drivers/gpu/drm/amd/amdkfd/kfd_iommu.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
index 193e283..8d87151 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
@@ -62,9 +62,6 @@ int kfd_iommu_device_init(struct kfd_dev *kfd)
struct amd_iommu_device_info iommu_info;
unsigned int pasid_limit;
int err;
-   struct kfd_topology_device *top_dev;
-
-   top_dev = kfd_topology_device_by_id(kfd->id);

if (!kfd->device_info->needs_iommu_device)
return 0;
--
2.7.4

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

RE: [PATCH 1/2] drm/amdgpu/ras: Extract amdgpu_ras_save_bad_pages from amdgpu_ras_reserve_bad_pages

[Zhou1, Tao]

Two questions:

1. "we lose all reservation during ASIC reset"
Are you sure of it? I remember the content of vram may be lost after reset but 
the allocated status could be reserved.

2. You change the bad page handle flow from:

detect bad page -> reserve vram for bad page -> save bad page info to eeprom -> 
gpu reset

to:

detect bad page (this time) -> save bad page (last time) info to eeprom -> gpu 
reset -> reserve vram for bad page (this time)

Is that right?  Saving bad page (this time) info to eeprom is delayed to the 
next time that bad page is detected? But the time of detecting bad page is 
random.
I think the bad page info would be lost without saving to eeprom if power off 
occurs.

detect bad page (this time) -> save bad page (last time) info to eeprom -> gpu 
reset -> reserve vram for bad page (this time) -> poweroff/system reset (and 
bad page info (this time) is lost)

Tao

> -Original Message-
> From: amd-gfx  On Behalf Of
> Andrey Grodzovsky
> Sent: 2019年11月14日 6:45
> To: amd-gfx@lists.freedesktop.org
> Cc: alexdeuc...@gmail.com; Grodzovsky, Andrey
> ; Chen, Guchun ;
> Zhang, Hawking 
> Subject: [PATCH 1/2] drm/amdgpu/ras: Extract
> amdgpu_ras_save_bad_pages from amdgpu_ras_reserve_bad_pages
> 
> We want to be be able to call them independently from one another so that
> before GPU reset just amdgpu_ras_save_bad_pages is called.
> 
> Signed-off-by: Andrey Grodzovsky 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 14 --
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h |  4 +++-
>  2 files changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 4044834..600a86d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -1421,7 +1421,7 @@ int amdgpu_ras_add_bad_pages(struct
> amdgpu_device *adev,
>   * write error record array to eeprom, the function should be
>   * protected by recovery_lock
>   */
> -static int amdgpu_ras_save_bad_pages(struct amdgpu_device *adev)
> +int amdgpu_ras_save_bad_pages(struct amdgpu_device *adev)
>  {
>   struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
>   struct ras_err_handler_data *data;
> @@ -1520,7 +1520,7 @@ int amdgpu_ras_reserve_bad_pages(struct
> amdgpu_device *adev)
>   struct ras_err_handler_data *data;
>   uint64_t bp;
>   struct amdgpu_bo *bo = NULL;
> - int i, ret = 0;
> + int i;
> 
>   if (!con || !con->eh_data)
>   return 0;
> @@ -1548,12 +1548,9 @@ int amdgpu_ras_reserve_bad_pages(struct
> amdgpu_device *adev)
>   data->last_reserved = i + 1;
>   bo = NULL;
>   }
> -
> - /* continue to save bad pages to eeprom even reesrve_vram fails */
> - ret = amdgpu_ras_save_bad_pages(adev);
>  out:
>   mutex_unlock(>recovery_lock);
> - return ret;
> + return 0;
>  }
> 
>  /* called when driver unload */
> @@ -1615,14 +1612,11 @@ int amdgpu_ras_recovery_init(struct
> amdgpu_device *adev)
>   ret = amdgpu_ras_load_bad_pages(adev);
>   if (ret)
>   goto free;
> - ret = amdgpu_ras_reserve_bad_pages(adev);
> - if (ret)
> - goto release;
> + amdgpu_ras_reserve_bad_pages(adev);
>   }
> 
>   return 0;
> 
> -release:
>   amdgpu_ras_release_bad_pages(adev);
>  free:
>   kfree((*data)->bps);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> index f80fd34..12b0797 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> @@ -492,6 +492,8 @@ unsigned long
> amdgpu_ras_query_error_count(struct amdgpu_device *adev,  int
> amdgpu_ras_add_bad_pages(struct amdgpu_device *adev,
>   struct eeprom_table_record *bps, int pages);
> 
> +int amdgpu_ras_save_bad_pages(struct amdgpu_device *adev);
> +
>  int amdgpu_ras_reserve_bad_pages(struct amdgpu_device *adev);
> 
>  static inline int amdgpu_ras_reset_gpu(struct amdgpu_device *adev, @@ -
> 503,7 +505,7 @@ static inline int amdgpu_ras_reset_gpu(struct
> amdgpu_device *adev,
>* i2c may be unstable in gpu reset
>*/
>   if (in_task())
> - amdgpu_ras_reserve_bad_pages(adev);
> + amdgpu_ras_save_bad_pages(adev);
> 
>   if (atomic_cmpxchg(>in_recovery, 0, 1) == 0)
>   schedule_work(>recovery_work);
> --
> 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

RE: [PATCH 2/2] drm/amdgpu: init umc functions for arcturus umc ras

[Clements, John]

Reviewed-by: John Clements 

-Original Message-
From: Hawking Zhang  
Sent: Wednesday, November 13, 2019 10:57 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander 
; Li, Dennis ; Clements, John 

Cc: Zhang, Hawking 
Subject: [PATCH 2/2] drm/amdgpu: init umc functions for arcturus umc ras

reuse vg20 umc functions for arcturus umc ras

Change-Id: Ia8af3c20a717c76ec18aa5fa332cfd81ca60ff69
Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 3784b62201b0..8a5b722ce5b7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -635,6 +635,7 @@ static void gmc_v9_0_set_umc_funcs(struct amdgpu_device 
*adev)
adev->umc.funcs = _v6_0_funcs;
break;
case CHIP_VEGA20:
+   case CHIP_ARCTURUS:
adev->umc.max_ras_err_cnt_per_query = 
UMC_V6_1_TOTAL_CHANNEL_NUM;
adev->umc.channel_inst_num = UMC_V6_1_CHANNEL_INSTANCE_NUM;
adev->umc.umc_inst_num = UMC_V6_1_UMC_INSTANCE_NUM; @@ -748,6 
+749,7 @@ static int gmc_v9_0_late_init(void *handle)
switch (adev->asic_type) {
case CHIP_VEGA10:
case CHIP_VEGA20:
+   case CHIP_ARCTURUS:
r = amdgpu_atomfirmware_mem_ecc_supported(adev);
if (!r) {
DRM_INFO("ECC is not present.\n");
--
2.17.1

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

RE: [PATCH 1/2] drm/amdgpu/powerplay: properly set PP_GFXOFF_MASK

[Quan, Evan]

Reviewed-by: Evan Quan 

> -Original Message-
> From: amd-gfx  On Behalf Of Alex
> Deucher
> Sent: Thursday, November 14, 2019 12:23 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander 
> Subject: [PATCH 1/2] drm/amdgpu/powerplay: properly set PP_GFXOFF_MASK
> 
> So that the setting reflects what the hw supports. This will be used in a
> subsequent patch so needs to be correct.
> 
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c  | 2 ++
> drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c | 7 +++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 57459a65eb44..ad39db49a29d 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -719,6 +719,7 @@ static int smu_set_funcs(struct amdgpu_device *adev)
> 
>   switch (adev->asic_type) {
>   case CHIP_VEGA20:
> + adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
>   vega20_set_ppt_funcs(smu);
>   break;
>   case CHIP_NAVI10:
> @@ -727,6 +728,7 @@ static int smu_set_funcs(struct amdgpu_device *adev)
>   navi10_set_ppt_funcs(smu);
>   break;
>   case CHIP_ARCTURUS:
> + adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
>   arcturus_set_ppt_funcs(smu);
>   /* OD is not supported on Arcturus */
>   smu->od_enabled =false;
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> index a24beaa4fb01..443625c83ec9 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> @@ -81,6 +81,8 @@ static void hwmgr_init_workload_prority(struct
> pp_hwmgr *hwmgr)
> 
>  int hwmgr_early_init(struct pp_hwmgr *hwmgr)  {
> + struct amdgpu_device *adev = hwmgr->adev;
> +
>   if (!hwmgr)
>   return -EINVAL;
> 
> @@ -96,6 +98,7 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
> 
>   switch (hwmgr->chip_family) {
>   case AMDGPU_FAMILY_CI:
> + adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
>   hwmgr->smumgr_funcs = _smu_funcs;
>   ci_set_asic_special_caps(hwmgr);
>   hwmgr->feature_mask &= ~(PP_VBI_TIME_SUPPORT_MASK |
> @@ -106,12 +109,14 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
>   smu7_init_function_pointers(hwmgr);
>   break;
>   case AMDGPU_FAMILY_CZ:
> + adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
>   hwmgr->od_enabled = false;
>   hwmgr->smumgr_funcs = _smu_funcs;
>   hwmgr->feature_mask &= ~PP_GFXOFF_MASK;
>   smu8_init_function_pointers(hwmgr);
>   break;
>   case AMDGPU_FAMILY_VI:
> + adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
>   hwmgr->feature_mask &= ~PP_GFXOFF_MASK;
>   switch (hwmgr->chip_id) {
>   case CHIP_TOPAZ:
> @@ -153,6 +158,7 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
>   case AMDGPU_FAMILY_AI:
>   switch (hwmgr->chip_id) {
>   case CHIP_VEGA10:
> + adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
>   hwmgr->feature_mask &= ~PP_GFXOFF_MASK;
>   hwmgr->smumgr_funcs = _smu_funcs;
>   vega10_hwmgr_init(hwmgr);
> @@ -162,6 +168,7 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
>   vega12_hwmgr_init(hwmgr);
>   break;
>   case CHIP_VEGA20:
> + adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
>   hwmgr->feature_mask &= ~PP_GFXOFF_MASK;
>   hwmgr->smumgr_funcs = _smu_funcs;
>   vega20_hwmgr_init(hwmgr);
> --
> 2.23.0
> 
> ___
> 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/amdkfd: Rename kfd_kernel_queue_*.c to kfd_packet_manager_*.c

[Felix Kuehling]

On 2019-11-13 5:50 p.m., Yong Zhao wrote:

After the recent cleanup, the functionalities provided by the previous
kfd_kernel_queue_*.c are actually all packet manager related. So rename
them to reflect that.

Change-Id: I6544ccb38da827c747544c0787aa949df20edbb0
Signed-off-by: Yong Zhao 


Reviewed-by: Felix Kuehling 



---
  drivers/gpu/drm/amd/amdkfd/Makefile   | 4 ++--
  .../amdkfd/{kfd_kernel_queue_v9.c => kfd_packet_manager_v9.c} | 0
  .../amdkfd/{kfd_kernel_queue_vi.c => kfd_packet_manager_vi.c} | 0
  3 files changed, 2 insertions(+), 2 deletions(-)
  rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_v9.c => 
kfd_packet_manager_v9.c} (100%)
  rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_vi.c => 
kfd_packet_manager_vi.c} (100%)

diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile 
b/drivers/gpu/drm/amd/amdkfd/Makefile
index f93a16372325..61474627a32c 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -38,9 +38,9 @@ AMDKFD_FILES  := $(AMDKFD_PATH)/kfd_module.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v9.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v10.o \
$(AMDKFD_PATH)/kfd_kernel_queue.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_vi.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_v9.o \
$(AMDKFD_PATH)/kfd_packet_manager.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_vi.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_v9.o \
$(AMDKFD_PATH)/kfd_process_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager_cik.o \
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c

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

[PATCH] drm/amdkfd: Rename kfd_kernel_queue_*.c to kfd_packet_manager_*.c

[Yong Zhao]

After the recent cleanup, the functionalities provided by the previous
kfd_kernel_queue_*.c are actually all packet manager related. So rename
them to reflect that.

Change-Id: I6544ccb38da827c747544c0787aa949df20edbb0
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/Makefile   | 4 ++--
 .../amdkfd/{kfd_kernel_queue_v9.c => kfd_packet_manager_v9.c} | 0
 .../amdkfd/{kfd_kernel_queue_vi.c => kfd_packet_manager_vi.c} | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_v9.c => 
kfd_packet_manager_v9.c} (100%)
 rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_vi.c => 
kfd_packet_manager_vi.c} (100%)

diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile 
b/drivers/gpu/drm/amd/amdkfd/Makefile
index f93a16372325..61474627a32c 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -38,9 +38,9 @@ AMDKFD_FILES  := $(AMDKFD_PATH)/kfd_module.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v9.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v10.o \
$(AMDKFD_PATH)/kfd_kernel_queue.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_vi.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_v9.o \
$(AMDKFD_PATH)/kfd_packet_manager.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_vi.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_v9.o \
$(AMDKFD_PATH)/kfd_process_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager_cik.o \
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
-- 
2.17.1

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

Re: [PATCH] drm/amdkfd: Rename kfd_kernel_queue_*.c to kfd_packet_manager_*.c

[Felix Kuehling]

On 2019-11-13 5:39 p.m., Yong Zhao wrote:

After the recent cleanup, the functionalities provided by the previous
kfd_kernel_queue_*.c are actually all packet manager related. So rename
them to reflect that.


NAK. Like I mentioned in the other email, AI refers to the SOC 
generation by its deprecated code name that predates the Vega code name.


I also want to avoid having a mix of kfd_*_v9 and kfd_*_ai files and 
functions referring to the same GFX generation.


Regards,
  Felix



Change-Id: I6544ccb38da827c747544c0787aa949df20edbb0
Signed-off-by: Yong Zhao 
---
  drivers/gpu/drm/amd/amdkfd/Makefile   |  4 +--
  .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   |  4 +--
  ...nel_queue_v9.c => kfd_packet_manager_ai.c} | 26 +--
  ...nel_queue_vi.c => kfd_packet_manager_vi.c} |  2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_priv.h |  4 +--
  5 files changed, 20 insertions(+), 20 deletions(-)
  rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_v9.c => 
kfd_packet_manager_ai.c} (94%)
  rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_vi.c => 
kfd_packet_manager_vi.c} (99%)

diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile 
b/drivers/gpu/drm/amd/amdkfd/Makefile
index f93a16372325..55bfecf04239 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -38,9 +38,9 @@ AMDKFD_FILES  := $(AMDKFD_PATH)/kfd_module.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v9.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v10.o \
$(AMDKFD_PATH)/kfd_kernel_queue.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_vi.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_v9.o \
$(AMDKFD_PATH)/kfd_packet_manager.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_vi.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_ai.o \
$(AMDKFD_PATH)/kfd_process_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager_cik.o \
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
index 6cabed06ef5d..cc945a2acd66 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
@@ -233,7 +233,7 @@ int pm_init(struct packet_manager *pm, struct 
device_queue_manager *dqm)
case CHIP_POLARIS11:
case CHIP_POLARIS12:
case CHIP_VEGAM:
-   pm->pmf = _vi_pm_funcs;
+   pm->pmf = _pm_funcs_vi;
break;
case CHIP_VEGA10:
case CHIP_VEGA12:
@@ -244,7 +244,7 @@ int pm_init(struct packet_manager *pm, struct 
device_queue_manager *dqm)
case CHIP_NAVI10:
case CHIP_NAVI12:
case CHIP_NAVI14:
-   pm->pmf = _v9_pm_funcs;
+   pm->pmf = _pm_funcs_ai;
break;
default:
WARN(1, "Unexpected ASIC family %u",
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
similarity index 94%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
index 2de01009f1b6..713530cd9760 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
@@ -27,7 +27,7 @@
  #include "kfd_pm4_opcodes.h"
  #include "gc/gc_10_1_0_sh_mask.h"
  
-static int pm_map_process_v9(struct packet_manager *pm,

+static int pm_map_process_ai(struct packet_manager *pm,
uint32_t *buffer, struct qcm_process_device *qpd)
  {
struct pm4_mes_map_process *packet;
@@ -73,7 +73,7 @@ static int pm_map_process_v9(struct packet_manager *pm,
return 0;
  }
  
-static int pm_runlist_v9(struct packet_manager *pm, uint32_t *buffer,

+static int pm_runlist_ai(struct packet_manager *pm, uint32_t *buffer,
uint64_t ib, size_t ib_size_in_dwords, bool chain)
  {
struct pm4_mes_runlist *packet;
@@ -111,7 +111,7 @@ static int pm_runlist_v9(struct packet_manager *pm, 
uint32_t *buffer,
return 0;
  }
  
-static int pm_set_resources_v9(struct packet_manager *pm, uint32_t *buffer,

+static int pm_set_resources_ai(struct packet_manager *pm, uint32_t *buffer,
struct scheduling_resources *res)
  {
struct pm4_mes_set_resources *packet;
@@ -139,7 +139,7 @@ static int pm_set_resources_v9(struct packet_manager *pm, 
uint32_t *buffer,
return 0;
  }
  
-static int pm_map_queues_v9(struct packet_manager *pm, uint32_t *buffer,

+static int pm_map_queues_ai(struct packet_manager *pm, uint32_t *buffer,
struct queue *q, bool is_static)
  {
struct pm4_mes_map_queues *packet;
@@ -206,7 +206,7 @@ static int pm_map_queues_v9(struct packet_manager *pm, 
uint32_t *buffer,
return 0;
  }
  
-static int pm_unmap_queues_v9(struct packet_manager *pm, uint32_t *buffer,

[PATCH 2/2] drm/amdgpu: Reserve bad pages after ASIC was reset.

[Andrey Grodzovsky]

Problem:
There is no point to reserve bad pages before GPU reset as
it was done untill now since we need to do it after ASIC was
reset as we lose all reservation during ASIC reset.

Fix:
Call amdgpu_ras_reserve_bad_pages after ASIC is reset and not before.

Signed-off-by: Andrey Grodzovsky 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f2feff4..eaac2b7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3804,6 +3804,10 @@ static int amdgpu_do_asic_reset(struct amdgpu_hive_info 
*hive,
break;
}
}
+
+   /* Mark vram pages with errors as bad after ASIC was 
reset */
+   list_for_each_entry(tmp_adev, device_list_handle, 
gmc.xgmi.head)
+  amdgpu_ras_reserve_bad_pages(tmp_adev);
}
}
 
-- 
2.7.4

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

[PATCH 1/2] drm/amdgpu/ras: Extract amdgpu_ras_save_bad_pages from amdgpu_ras_reserve_bad_pages

[Andrey Grodzovsky]

We want to be be able to call them independently from one another
so that before GPU reset just amdgpu_ras_save_bad_pages is called.

Signed-off-by: Andrey Grodzovsky 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 14 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h |  4 +++-
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 4044834..600a86d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -1421,7 +1421,7 @@ int amdgpu_ras_add_bad_pages(struct amdgpu_device *adev,
  * write error record array to eeprom, the function should be
  * protected by recovery_lock
  */
-static int amdgpu_ras_save_bad_pages(struct amdgpu_device *adev)
+int amdgpu_ras_save_bad_pages(struct amdgpu_device *adev)
 {
struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
struct ras_err_handler_data *data;
@@ -1520,7 +1520,7 @@ int amdgpu_ras_reserve_bad_pages(struct amdgpu_device 
*adev)
struct ras_err_handler_data *data;
uint64_t bp;
struct amdgpu_bo *bo = NULL;
-   int i, ret = 0;
+   int i;
 
if (!con || !con->eh_data)
return 0;
@@ -1548,12 +1548,9 @@ int amdgpu_ras_reserve_bad_pages(struct amdgpu_device 
*adev)
data->last_reserved = i + 1;
bo = NULL;
}
-
-   /* continue to save bad pages to eeprom even reesrve_vram fails */
-   ret = amdgpu_ras_save_bad_pages(adev);
 out:
mutex_unlock(>recovery_lock);
-   return ret;
+   return 0;
 }
 
 /* called when driver unload */
@@ -1615,14 +1612,11 @@ int amdgpu_ras_recovery_init(struct amdgpu_device *adev)
ret = amdgpu_ras_load_bad_pages(adev);
if (ret)
goto free;
-   ret = amdgpu_ras_reserve_bad_pages(adev);
-   if (ret)
-   goto release;
+   amdgpu_ras_reserve_bad_pages(adev);
}
 
return 0;
 
-release:
amdgpu_ras_release_bad_pages(adev);
 free:
kfree((*data)->bps);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
index f80fd34..12b0797 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
@@ -492,6 +492,8 @@ unsigned long amdgpu_ras_query_error_count(struct 
amdgpu_device *adev,
 int amdgpu_ras_add_bad_pages(struct amdgpu_device *adev,
struct eeprom_table_record *bps, int pages);
 
+int amdgpu_ras_save_bad_pages(struct amdgpu_device *adev);
+
 int amdgpu_ras_reserve_bad_pages(struct amdgpu_device *adev);
 
 static inline int amdgpu_ras_reset_gpu(struct amdgpu_device *adev,
@@ -503,7 +505,7 @@ static inline int amdgpu_ras_reset_gpu(struct amdgpu_device 
*adev,
 * i2c may be unstable in gpu reset
 */
if (in_task())
-   amdgpu_ras_reserve_bad_pages(adev);
+   amdgpu_ras_save_bad_pages(adev);
 
if (atomic_cmpxchg(>in_recovery, 0, 1) == 0)
schedule_work(>recovery_work);
-- 
2.7.4

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

Re: [PATCH] drm/amdkfd: Rename kfd_kernel_queue_*.c to kfd_packet_manager_*.c

[Felix Kuehling]

On 2019-11-13 5:26 p.m., Yong Zhao wrote:
Oh, I did not realize the part inside of the file. I think v9->ai is 
better, because the packet format header uses ai. Also v9 will give 
people an impression of gfx9.


There are a lot of other places in KFD that use the "v9" suffix. Like I 
said, if we decide to change this, it will affect lots of other KFD 
files and functions.


The code in the packet manager deals with the MEC, which is part of the 
GFX engine. So referring to the GFX version (v9) makes sense. AI refers 
to the whole SOC generation by its deprecated code name that the Vega 
code name for the SOC that's used elsewhere in the code.


Regards,
  Felix



Yong

On 2019-11-13 5:19 p.m., Felix Kuehling wrote:

On 2019-11-13 5:09 p.m., Yong Zhao wrote:

After the recent cleanup, the functionalities provided by the previous
kfd_kernel_queue_*.c are actually all packet manager related. So rename
them to reflect that.

Change-Id: I6544ccb38da827c747544c0787aa949df20edbb0
Signed-off-by: Yong Zhao 
---
  drivers/gpu/drm/amd/amdkfd/Makefile | 4 ++--
  .../amdkfd/{kfd_kernel_queue_v9.c => kfd_packet_manager_ai.c} | 0
  .../amdkfd/{kfd_kernel_queue_vi.c => kfd_packet_manager_vi.c} | 0
  3 files changed, 2 insertions(+), 2 deletions(-)
  rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_v9.c => 
kfd_packet_manager_ai.c} (100%)
  rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_vi.c => 
kfd_packet_manager_vi.c} (100%)


diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile 
b/drivers/gpu/drm/amd/amdkfd/Makefile

index f93a16372325..55bfecf04239 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -38,9 +38,9 @@ AMDKFD_FILES    := $(AMDKFD_PATH)/kfd_module.o \
  $(AMDKFD_PATH)/kfd_mqd_manager_v9.o \
  $(AMDKFD_PATH)/kfd_mqd_manager_v10.o \
  $(AMDKFD_PATH)/kfd_kernel_queue.o \
-    $(AMDKFD_PATH)/kfd_kernel_queue_vi.o \
-    $(AMDKFD_PATH)/kfd_kernel_queue_v9.o \
  $(AMDKFD_PATH)/kfd_packet_manager.o \
+    $(AMDKFD_PATH)/kfd_packet_manager_vi.o \
+    $(AMDKFD_PATH)/kfd_packet_manager_ai.o \


This naming convention is inconsistent with the rest of KFD. We use 
_v9, not _ai. Also the function s inside this file are named _v9. If 
we decide to change that naming convention, it should not be 
accidental and piece-meal. It should be deliberate and comprehensive.


Regards,
  Felix



$(AMDKFD_PATH)/kfd_process_queue_manager.o \
  $(AMDKFD_PATH)/kfd_device_queue_manager.o \
  $(AMDKFD_PATH)/kfd_device_queue_manager_cik.o \
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c

similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c

similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c

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

[PATCH] drm/amdkfd: Rename kfd_kernel_queue_*.c to kfd_packet_manager_*.c

[Yong Zhao]

After the recent cleanup, the functionalities provided by the previous
kfd_kernel_queue_*.c are actually all packet manager related. So rename
them to reflect that.

Change-Id: I6544ccb38da827c747544c0787aa949df20edbb0
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/Makefile   |  4 +--
 .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   |  4 +--
 ...nel_queue_v9.c => kfd_packet_manager_ai.c} | 26 +--
 ...nel_queue_vi.c => kfd_packet_manager_vi.c} |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h |  4 +--
 5 files changed, 20 insertions(+), 20 deletions(-)
 rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_v9.c => 
kfd_packet_manager_ai.c} (94%)
 rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_vi.c => 
kfd_packet_manager_vi.c} (99%)

diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile 
b/drivers/gpu/drm/amd/amdkfd/Makefile
index f93a16372325..55bfecf04239 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -38,9 +38,9 @@ AMDKFD_FILES  := $(AMDKFD_PATH)/kfd_module.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v9.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v10.o \
$(AMDKFD_PATH)/kfd_kernel_queue.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_vi.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_v9.o \
$(AMDKFD_PATH)/kfd_packet_manager.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_vi.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_ai.o \
$(AMDKFD_PATH)/kfd_process_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager_cik.o \
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
index 6cabed06ef5d..cc945a2acd66 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
@@ -233,7 +233,7 @@ int pm_init(struct packet_manager *pm, struct 
device_queue_manager *dqm)
case CHIP_POLARIS11:
case CHIP_POLARIS12:
case CHIP_VEGAM:
-   pm->pmf = _vi_pm_funcs;
+   pm->pmf = _pm_funcs_vi;
break;
case CHIP_VEGA10:
case CHIP_VEGA12:
@@ -244,7 +244,7 @@ int pm_init(struct packet_manager *pm, struct 
device_queue_manager *dqm)
case CHIP_NAVI10:
case CHIP_NAVI12:
case CHIP_NAVI14:
-   pm->pmf = _v9_pm_funcs;
+   pm->pmf = _pm_funcs_ai;
break;
default:
WARN(1, "Unexpected ASIC family %u",
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
similarity index 94%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
index 2de01009f1b6..713530cd9760 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
@@ -27,7 +27,7 @@
 #include "kfd_pm4_opcodes.h"
 #include "gc/gc_10_1_0_sh_mask.h"
 
-static int pm_map_process_v9(struct packet_manager *pm,
+static int pm_map_process_ai(struct packet_manager *pm,
uint32_t *buffer, struct qcm_process_device *qpd)
 {
struct pm4_mes_map_process *packet;
@@ -73,7 +73,7 @@ static int pm_map_process_v9(struct packet_manager *pm,
return 0;
 }
 
-static int pm_runlist_v9(struct packet_manager *pm, uint32_t *buffer,
+static int pm_runlist_ai(struct packet_manager *pm, uint32_t *buffer,
uint64_t ib, size_t ib_size_in_dwords, bool chain)
 {
struct pm4_mes_runlist *packet;
@@ -111,7 +111,7 @@ static int pm_runlist_v9(struct packet_manager *pm, 
uint32_t *buffer,
return 0;
 }
 
-static int pm_set_resources_v9(struct packet_manager *pm, uint32_t *buffer,
+static int pm_set_resources_ai(struct packet_manager *pm, uint32_t *buffer,
struct scheduling_resources *res)
 {
struct pm4_mes_set_resources *packet;
@@ -139,7 +139,7 @@ static int pm_set_resources_v9(struct packet_manager *pm, 
uint32_t *buffer,
return 0;
 }
 
-static int pm_map_queues_v9(struct packet_manager *pm, uint32_t *buffer,
+static int pm_map_queues_ai(struct packet_manager *pm, uint32_t *buffer,
struct queue *q, bool is_static)
 {
struct pm4_mes_map_queues *packet;
@@ -206,7 +206,7 @@ static int pm_map_queues_v9(struct packet_manager *pm, 
uint32_t *buffer,
return 0;
 }
 
-static int pm_unmap_queues_v9(struct packet_manager *pm, uint32_t *buffer,
+static int pm_unmap_queues_ai(struct packet_manager *pm, uint32_t *buffer,
enum kfd_queue_type type,
enum kfd_unmap_queues_filter filter,
uint32_t filter_param, bool reset,
@@ -282,7 +282,7 @@ static int pm_unmap_queues_v9(struct packet_manager *pm, 
uint32_t *buffer,
 
 }
 
-static 

Re: [PATCH] drm/amdkfd: Rename kfd_kernel_queue_*.c to kfd_packet_manager_*.c

[Yong Zhao]

Oh, I did not realize the part inside of the file. I think v9->ai is 
better, because the packet format header uses ai. Also v9 will give 
people an impression of gfx9.


Yong

On 2019-11-13 5:19 p.m., Felix Kuehling wrote:

On 2019-11-13 5:09 p.m., Yong Zhao wrote:

After the recent cleanup, the functionalities provided by the previous
kfd_kernel_queue_*.c are actually all packet manager related. So rename
them to reflect that.

Change-Id: I6544ccb38da827c747544c0787aa949df20edbb0
Signed-off-by: Yong Zhao 
---
  drivers/gpu/drm/amd/amdkfd/Makefile | 4 ++--
  .../amdkfd/{kfd_kernel_queue_v9.c => kfd_packet_manager_ai.c} | 0
  .../amdkfd/{kfd_kernel_queue_vi.c => kfd_packet_manager_vi.c} | 0
  3 files changed, 2 insertions(+), 2 deletions(-)
  rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_v9.c => 
kfd_packet_manager_ai.c} (100%)
  rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_vi.c => 
kfd_packet_manager_vi.c} (100%)


diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile 
b/drivers/gpu/drm/amd/amdkfd/Makefile

index f93a16372325..55bfecf04239 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -38,9 +38,9 @@ AMDKFD_FILES    := $(AMDKFD_PATH)/kfd_module.o \
  $(AMDKFD_PATH)/kfd_mqd_manager_v9.o \
  $(AMDKFD_PATH)/kfd_mqd_manager_v10.o \
  $(AMDKFD_PATH)/kfd_kernel_queue.o \
-    $(AMDKFD_PATH)/kfd_kernel_queue_vi.o \
-    $(AMDKFD_PATH)/kfd_kernel_queue_v9.o \
  $(AMDKFD_PATH)/kfd_packet_manager.o \
+    $(AMDKFD_PATH)/kfd_packet_manager_vi.o \
+    $(AMDKFD_PATH)/kfd_packet_manager_ai.o \


This naming convention is inconsistent with the rest of KFD. We use 
_v9, not _ai. Also the function s inside this file are named _v9. If 
we decide to change that naming convention, it should not be 
accidental and piece-meal. It should be deliberate and comprehensive.


Regards,
  Felix



$(AMDKFD_PATH)/kfd_process_queue_manager.o \
  $(AMDKFD_PATH)/kfd_device_queue_manager.o \
  $(AMDKFD_PATH)/kfd_device_queue_manager_cik.o \
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c

similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c

similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c

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

Re: [PATCH] drm/amdkfd: Rename kfd_kernel_queue_*.c to kfd_packet_manager_*.c

[Felix Kuehling]

On 2019-11-13 5:09 p.m., Yong Zhao wrote:

After the recent cleanup, the functionalities provided by the previous
kfd_kernel_queue_*.c are actually all packet manager related. So rename
them to reflect that.

Change-Id: I6544ccb38da827c747544c0787aa949df20edbb0
Signed-off-by: Yong Zhao 
---
  drivers/gpu/drm/amd/amdkfd/Makefile   | 4 ++--
  .../amdkfd/{kfd_kernel_queue_v9.c => kfd_packet_manager_ai.c} | 0
  .../amdkfd/{kfd_kernel_queue_vi.c => kfd_packet_manager_vi.c} | 0
  3 files changed, 2 insertions(+), 2 deletions(-)
  rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_v9.c => 
kfd_packet_manager_ai.c} (100%)
  rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_vi.c => 
kfd_packet_manager_vi.c} (100%)

diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile 
b/drivers/gpu/drm/amd/amdkfd/Makefile
index f93a16372325..55bfecf04239 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -38,9 +38,9 @@ AMDKFD_FILES  := $(AMDKFD_PATH)/kfd_module.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v9.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v10.o \
$(AMDKFD_PATH)/kfd_kernel_queue.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_vi.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_v9.o \
$(AMDKFD_PATH)/kfd_packet_manager.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_vi.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_ai.o \


This naming convention is inconsistent with the rest of KFD. We use _v9, 
not _ai. Also the function s inside this file are named _v9. If we 
decide to change that naming convention, it should not be accidental and 
piece-meal. It should be deliberate and comprehensive.


Regards,
  Felix



$(AMDKFD_PATH)/kfd_process_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager_cik.o \
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c

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

[PATCH] drm/amdkfd: Rename kfd_kernel_queue_*.c to kfd_packet_manager_*.c

[Yong Zhao]

After the recent cleanup, the functionalities provided by the previous
kfd_kernel_queue_*.c are actually all packet manager related. So rename
them to reflect that.

Change-Id: I6544ccb38da827c747544c0787aa949df20edbb0
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/Makefile   | 4 ++--
 .../amdkfd/{kfd_kernel_queue_v9.c => kfd_packet_manager_ai.c} | 0
 .../amdkfd/{kfd_kernel_queue_vi.c => kfd_packet_manager_vi.c} | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_v9.c => 
kfd_packet_manager_ai.c} (100%)
 rename drivers/gpu/drm/amd/amdkfd/{kfd_kernel_queue_vi.c => 
kfd_packet_manager_vi.c} (100%)

diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile 
b/drivers/gpu/drm/amd/amdkfd/Makefile
index f93a16372325..55bfecf04239 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -38,9 +38,9 @@ AMDKFD_FILES  := $(AMDKFD_PATH)/kfd_module.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v9.o \
$(AMDKFD_PATH)/kfd_mqd_manager_v10.o \
$(AMDKFD_PATH)/kfd_kernel_queue.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_vi.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_v9.o \
$(AMDKFD_PATH)/kfd_packet_manager.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_vi.o \
+   $(AMDKFD_PATH)/kfd_packet_manager_ai.o \
$(AMDKFD_PATH)/kfd_process_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager_cik.o \
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_ai.c
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
similarity index 100%
rename from drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
rename to drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
-- 
2.17.1

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

[PATCH] drm/amdkfd: Eliminate unnecessary kernel queue function pointers

[Yong Zhao]

Up to this point, those functions are all the same for all ASICs, so
no need to call them by functions pointers. Removing the function
pointers will greatly increase the code readablity. If there is ever
need for those function pointers, we can add it back then.

Change-Id: I9515fdece70110067cda66e2d24d6768b4846c2f
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c   |  8 ++---
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 30 
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h | 34 +--
 .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   | 28 +++
 4 files changed, 41 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
index 1d33c4f25263..27bcc5b472f6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
@@ -72,11 +72,11 @@ static int dbgdev_diq_submit_ib(struct kfd_dbgdev *dbgdev,
 * The receive packet buff will be sitting on the Indirect Buffer
 * and in the PQ we put the IB packet + sync packet(s).
 */
-   status = kq->ops.acquire_packet_buffer(kq,
+   status = kq_acquire_packet_buffer(kq,
pq_packets_size_in_bytes / sizeof(uint32_t),
_packet_buff);
if (status) {
-   pr_err("acquire_packet_buffer failed\n");
+   pr_err("kq_acquire_packet_buffer failed\n");
return status;
}
 
@@ -115,7 +115,7 @@ static int dbgdev_diq_submit_ib(struct kfd_dbgdev *dbgdev,
 
if (status) {
pr_err("Failed to allocate GART memory\n");
-   kq->ops.rollback_packet(kq);
+   kq_rollback_packet(kq);
return status;
}
 
@@ -151,7 +151,7 @@ static int dbgdev_diq_submit_ib(struct kfd_dbgdev *dbgdev,
 
rm_packet->data_lo = QUEUESTATE__ACTIVE;
 
-   kq->ops.submit_packet(kq);
+   kq_submit_packet(kq);
 
/* Wait till CP writes sync code: */
status = amdkfd_fence_wait_timeout(
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index 59ee9053498c..2d56dc534459 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -34,7 +34,10 @@
 
 #define PM4_COUNT_ZERO (((1 << 15) - 1) << 16)
 
-static bool initialize(struct kernel_queue *kq, struct kfd_dev *dev,
+/* Initialize a kernel queue, including allocations of GART memory
+ * needed for the queue.
+ */
+static bool kq_initialize(struct kernel_queue *kq, struct kfd_dev *dev,
enum kfd_queue_type type, unsigned int queue_size)
 {
struct queue_properties prop;
@@ -88,7 +91,7 @@ static bool initialize(struct kernel_queue *kq, struct 
kfd_dev *dev,
kq->pq_gpu_addr = kq->pq->gpu_addr;
 
/* For CIK family asics, kq->eop_mem is not needed */
-   if (dev->device_info->asic_family > CHIP_HAWAII) {
+   if (dev->device_info->asic_family > CHIP_MULLINS) {
retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
if (retval != 0)
goto err_eop_allocate_vidmem;
@@ -191,7 +194,8 @@ static bool initialize(struct kernel_queue *kq, struct 
kfd_dev *dev,
 
 }
 
-static void uninitialize(struct kernel_queue *kq)
+/* Uninitialize a kernel queue and free all its memory usages. */
+static void kq_uninitialize(struct kernel_queue *kq)
 {
if (kq->queue->properties.type == KFD_QUEUE_TYPE_HIQ)
kq->mqd_mgr->destroy_mqd(kq->mqd_mgr,
@@ -220,7 +224,7 @@ static void uninitialize(struct kernel_queue *kq)
uninit_queue(kq->queue);
 }
 
-static int acquire_packet_buffer(struct kernel_queue *kq,
+int kq_acquire_packet_buffer(struct kernel_queue *kq,
size_t packet_size_in_dwords, unsigned int **buffer_ptr)
 {
size_t available_size;
@@ -281,7 +285,7 @@ static int acquire_packet_buffer(struct kernel_queue *kq,
return -ENOMEM;
 }
 
-static void submit_packet(struct kernel_queue *kq)
+void kq_submit_packet(struct kernel_queue *kq)
 {
 #ifdef DEBUG
int i;
@@ -304,7 +308,7 @@ static void submit_packet(struct kernel_queue *kq)
}
 }
 
-static void rollback_packet(struct kernel_queue *kq)
+void kq_rollback_packet(struct kernel_queue *kq)
 {
if (kq->dev->device_info->doorbell_size == 8) {
kq->pending_wptr64 = *kq->wptr64_kernel;
@@ -324,13 +328,7 @@ struct kernel_queue *kernel_queue_init(struct kfd_dev *dev,
if (!kq)
return NULL;
 
-   kq->ops.initialize = initialize;
-   kq->ops.uninitialize = uninitialize;
-   kq->ops.acquire_packet_buffer = acquire_packet_buffer;
-   kq->ops.submit_packet = submit_packet;
-   kq->ops.rollback_packet = rollback_packet;
-
-   if (kq->ops.initialize(kq, dev, type, KFD_KERNEL_QUEUE_SIZE))
+   if (kq_initialize(kq, 

Re: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

[Yong Zhao]

I will change it to CHIP_MULLINS.

Yes , I also spotted the kq->ops cleanup, will send it out shortly.

Regards,

Yong

On 2019-11-13 2:31 p.m., Felix Kuehling wrote:

See one comment inline. With that fixed, the series is

Reviewed-by: Felix Kuehling 

I could think of more follow-up cleanup while you're at it:

1. Can you see any reason why the kq->ops need to be function pointers.
   Looks to me like they are the same for all kernel queues, so those
   functions could be called without the pointer indirection.
2. The only think left in the ASIC-specific kfd_kernel_queue_*.c files
   is the PM4 packet writer functions that are called by the
   kfd_packet_manager. It may make sense to rename them to reflect
   that. Maybe kfd_packet_manager_*.c

Regards,
  Felix

On 2019-11-12 5:18 p.m., Yong Zhao wrote:

The ops_asic_specific function pointers are actually quite generic after
using a simple if condition. Eliminate it by code refactoring.

Change-Id: Icb891289cca31acdbe2d2eea76a426f1738b9c08
Signed-off-by: Yong Zhao 
---
  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 63 ---
  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h |  4 --
  .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c  | 36 ---
  .../gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c  | 48 --
  4 files changed, 26 insertions(+), 125 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c

index a750b1d110eb..59ee9053498c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -87,9 +87,17 @@ static bool initialize(struct kernel_queue *kq, 
struct kfd_dev *dev,

  kq->pq_kernel_addr = kq->pq->cpu_ptr;
  kq->pq_gpu_addr = kq->pq->gpu_addr;
  -    retval = kq->ops_asic_specific.initialize(kq, dev, type, 
queue_size);

-    if (!retval)
-    goto err_eop_allocate_vidmem;
+    /* For CIK family asics, kq->eop_mem is not needed */
+    if (dev->device_info->asic_family > CHIP_HAWAII) {


This is not the correct condition to distinguish GFXv7 (CIK) vs v8 
(VI). CHIP_MULLINS comes after Hawaii, but it is also GFXv7 (CIK), 
even though KFD current doesn't support it.




+    retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
+    if (retval != 0)
+    goto err_eop_allocate_vidmem;
+
+    kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
+    kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
+
+    memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
+    }
    retval = kfd_gtt_sa_allocate(dev, sizeof(*kq->rptr_kernel),
  >rptr_mem);
@@ -200,7 +208,12 @@ static void uninitialize(struct kernel_queue *kq)
    kfd_gtt_sa_free(kq->dev, kq->rptr_mem);
  kfd_gtt_sa_free(kq->dev, kq->wptr_mem);
-    kq->ops_asic_specific.uninitialize(kq);
+
+    /* For CIK family asics, kq->eop_mem is Null, kfd_gtt_sa_free()
+ * is able to handle NULL properly.
+ */
+    kfd_gtt_sa_free(kq->dev, kq->eop_mem);
+
  kfd_gtt_sa_free(kq->dev, kq->pq);
  kfd_release_kernel_doorbell(kq->dev,
  kq->queue->properties.doorbell_ptr);
@@ -280,8 +293,15 @@ static void submit_packet(struct kernel_queue *kq)
  }
  pr_debug("\n");
  #endif
-
-    kq->ops_asic_specific.submit_packet(kq);
+    if (kq->dev->device_info->doorbell_size == 8) {
+    *kq->wptr64_kernel = kq->pending_wptr64;
+ write_kernel_doorbell64(kq->queue->properties.doorbell_ptr,
+    kq->pending_wptr64);
+    } else {
+    *kq->wptr_kernel = kq->pending_wptr;
+ write_kernel_doorbell(kq->queue->properties.doorbell_ptr,
+    kq->pending_wptr);
+    }
  }
    static void rollback_packet(struct kernel_queue *kq)
@@ -310,42 +330,11 @@ struct kernel_queue *kernel_queue_init(struct 
kfd_dev *dev,

  kq->ops.submit_packet = submit_packet;
  kq->ops.rollback_packet = rollback_packet;
  -    switch (dev->device_info->asic_family) {
-    case CHIP_KAVERI:
-    case CHIP_HAWAII:
-    case CHIP_CARRIZO:
-    case CHIP_TONGA:
-    case CHIP_FIJI:
-    case CHIP_POLARIS10:
-    case CHIP_POLARIS11:
-    case CHIP_POLARIS12:
-    case CHIP_VEGAM:
-    kernel_queue_init_vi(>ops_asic_specific);
-    break;
-
-    case CHIP_VEGA10:
-    case CHIP_VEGA12:
-    case CHIP_VEGA20:
-    case CHIP_RAVEN:
-    case CHIP_RENOIR:
-    case CHIP_ARCTURUS:
-    case CHIP_NAVI10:
-    case CHIP_NAVI12:
-    case CHIP_NAVI14:
-    kernel_queue_init_v9(>ops_asic_specific);
-    break;
-    default:
-    WARN(1, "Unexpected ASIC family %u",
- dev->device_info->asic_family);
-    goto out_free;
-    }
-
  if (kq->ops.initialize(kq, dev, type, KFD_KERNEL_QUEUE_SIZE))
  return kq;
    pr_err("Failed to init kernel queue\n");
  -out_free:
  kfree(kq);
  return NULL;
  }
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h

index a9a35897d8b7..475e9499c0af 

Re: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

[Felix Kuehling]

See one comment inline. With that fixed, the series is

Reviewed-by: Felix Kuehling 

I could think of more follow-up cleanup while you're at it:

1. Can you see any reason why the kq->ops need to be function pointers.
   Looks to me like they are the same for all kernel queues, so those
   functions could be called without the pointer indirection.
2. The only think left in the ASIC-specific kfd_kernel_queue_*.c files
   is the PM4 packet writer functions that are called by the
   kfd_packet_manager. It may make sense to rename them to reflect
   that. Maybe kfd_packet_manager_*.c

Regards,
  Felix

On 2019-11-12 5:18 p.m., Yong Zhao wrote:

The ops_asic_specific function pointers are actually quite generic after
using a simple if condition. Eliminate it by code refactoring.

Change-Id: Icb891289cca31acdbe2d2eea76a426f1738b9c08
Signed-off-by: Yong Zhao 
---
  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 63 ---
  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h |  4 --
  .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c  | 36 ---
  .../gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c  | 48 --
  4 files changed, 26 insertions(+), 125 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index a750b1d110eb..59ee9053498c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -87,9 +87,17 @@ static bool initialize(struct kernel_queue *kq, struct 
kfd_dev *dev,
kq->pq_kernel_addr = kq->pq->cpu_ptr;
kq->pq_gpu_addr = kq->pq->gpu_addr;
  
-	retval = kq->ops_asic_specific.initialize(kq, dev, type, queue_size);

-   if (!retval)
-   goto err_eop_allocate_vidmem;
+   /* For CIK family asics, kq->eop_mem is not needed */
+   if (dev->device_info->asic_family > CHIP_HAWAII) {


This is not the correct condition to distinguish GFXv7 (CIK) vs v8 (VI). 
CHIP_MULLINS comes after Hawaii, but it is also GFXv7 (CIK), even though 
KFD current doesn't support it.




+   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
+   if (retval != 0)
+   goto err_eop_allocate_vidmem;
+
+   kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
+   kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
+
+   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
+   }
  
  	retval = kfd_gtt_sa_allocate(dev, sizeof(*kq->rptr_kernel),

>rptr_mem);
@@ -200,7 +208,12 @@ static void uninitialize(struct kernel_queue *kq)
  
  	kfd_gtt_sa_free(kq->dev, kq->rptr_mem);

kfd_gtt_sa_free(kq->dev, kq->wptr_mem);
-   kq->ops_asic_specific.uninitialize(kq);
+
+   /* For CIK family asics, kq->eop_mem is Null, kfd_gtt_sa_free()
+* is able to handle NULL properly.
+*/
+   kfd_gtt_sa_free(kq->dev, kq->eop_mem);
+
kfd_gtt_sa_free(kq->dev, kq->pq);
kfd_release_kernel_doorbell(kq->dev,
kq->queue->properties.doorbell_ptr);
@@ -280,8 +293,15 @@ static void submit_packet(struct kernel_queue *kq)
}
pr_debug("\n");
  #endif
-
-   kq->ops_asic_specific.submit_packet(kq);
+   if (kq->dev->device_info->doorbell_size == 8) {
+   *kq->wptr64_kernel = kq->pending_wptr64;
+   write_kernel_doorbell64(kq->queue->properties.doorbell_ptr,
+   kq->pending_wptr64);
+   } else {
+   *kq->wptr_kernel = kq->pending_wptr;
+   write_kernel_doorbell(kq->queue->properties.doorbell_ptr,
+   kq->pending_wptr);
+   }
  }
  
  static void rollback_packet(struct kernel_queue *kq)

@@ -310,42 +330,11 @@ struct kernel_queue *kernel_queue_init(struct kfd_dev 
*dev,
kq->ops.submit_packet = submit_packet;
kq->ops.rollback_packet = rollback_packet;
  
-	switch (dev->device_info->asic_family) {

-   case CHIP_KAVERI:
-   case CHIP_HAWAII:
-   case CHIP_CARRIZO:
-   case CHIP_TONGA:
-   case CHIP_FIJI:
-   case CHIP_POLARIS10:
-   case CHIP_POLARIS11:
-   case CHIP_POLARIS12:
-   case CHIP_VEGAM:
-   kernel_queue_init_vi(>ops_asic_specific);
-   break;
-
-   case CHIP_VEGA10:
-   case CHIP_VEGA12:
-   case CHIP_VEGA20:
-   case CHIP_RAVEN:
-   case CHIP_RENOIR:
-   case CHIP_ARCTURUS:
-   case CHIP_NAVI10:
-   case CHIP_NAVI12:
-   case CHIP_NAVI14:
-   kernel_queue_init_v9(>ops_asic_specific);
-   break;
-   default:
-   WARN(1, "Unexpected ASIC family %u",
-dev->device_info->asic_family);
-   goto out_free;
-   }
-
if (kq->ops.initialize(kq, dev, type, KFD_KERNEL_QUEUE_SIZE))
return kq;
  
  	pr_err("Failed to init kernel queue\n");
  
-out_free:

kfree(kq);
 

[PATCH] drm/amdgpu/nv: add asic func for fetching vbios from rom directly

[Alex Deucher]

Needed as a fallback if the vbios can't be fetched by other means.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/nv.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 7283d6198b89..ad04d1d6e9c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -40,6 +40,7 @@
 #include "gc/gc_10_1_0_sh_mask.h"
 #include "hdp/hdp_5_0_0_offset.h"
 #include "hdp/hdp_5_0_0_sh_mask.h"
+#include "smuio/smuio_11_0_0_offset.h"
 
 #include "soc15.h"
 #include "soc15_common.h"
@@ -156,8 +157,27 @@ static bool nv_read_disabled_bios(struct amdgpu_device 
*adev)
 static bool nv_read_bios_from_rom(struct amdgpu_device *adev,
  u8 *bios, u32 length_bytes)
 {
-   /* TODO: will implement it when SMU header is available */
-   return false;
+   u32 *dw_ptr;
+   u32 i, length_dw;
+
+   if (bios == NULL)
+   return false;
+   if (length_bytes == 0)
+   return false;
+   /* APU vbios image is part of sbios image */
+   if (adev->flags & AMD_IS_APU)
+   return false;
+
+   dw_ptr = (u32 *)bios;
+   length_dw = ALIGN(length_bytes, 4) / 4;
+
+   /* set rom index to 0 */
+   WREG32(SOC15_REG_OFFSET(SMUIO, 0, mmROM_INDEX), 0);
+   /* read out the rom data */
+   for (i = 0; i < length_dw; i++)
+   dw_ptr[i] = RREG32(SOC15_REG_OFFSET(SMUIO, 0, mmROM_DATA));
+
+   return true;
 }
 
 static struct soc15_allowed_register_entry nv_allowed_read_registers[] = {
-- 
2.23.0

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

Re: [PATCH 1/7] drm/amdgpu: remove set but not used variable 'mc_shared_chmap' from 'gfx_v6_0.c' and 'gfx_v7_0.c'

[Alex Deucher]

On Wed, Nov 13, 2019 at 11:56 AM Joe Perches  wrote:
>
> On Wed, 2019-11-13 at 20:44 +0800, yu kuai wrote:
> > Fixes gcc '-Wunused-but-set-variable' warning:
> >
> > drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c: In function
> > ‘gfx_v6_0_constants_init’:
> > drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c:1579:6: warning: variable
> > ‘mc_shared_chmap’ set but not used [-Wunused-but-set-variable]
> []
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> []
> > @@ -1678,7 +1678,6 @@ static void gfx_v6_0_constants_init(struct 
> > amdgpu_device *adev)
> >
> >   WREG32(mmBIF_FB_EN, BIF_FB_EN__FB_READ_EN_MASK | 
> > BIF_FB_EN__FB_WRITE_EN_MASK);
> >
> > - mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP);
>
> I do not know the hardware but frequently hardware like
> this has read ordering requirements and various registers
> can not be read in a random order.
>
> Does removing this read have no effect on the hardware?

There is no dependency.  It's safe.  Same thing below.

Alex

>
> >   adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG);
> >   mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg;
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> []
> > @@ -4336,7 +4336,6 @@ static void gfx_v7_0_gpu_early_init(struct 
> > amdgpu_device *adev)
> >   break;
> >   }
> >
> > - mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP);
> >   adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG);
> >   mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg;
>
> Same question.
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/2] drm/dsc: Update drm_dsc to reflect native 4.2.0 DSC spec

[mikita.lipski]

From: Mikita Lipski 

[Why]
Some parts of the DSC spec relating to 4.2.0 were not reflected in
drm_dsc_compute_rc_parameters, causing unexpected config failures

[How]
Add nsl_bpg_offset and rbs_min computation

Signed-off-by: David Francis 
Signed-off-by: Mikita Lipski 
---
 drivers/gpu/drm/drm_dsc.c | 72 ---
 1 file changed, 68 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index 77f4e5ae4197..79c71e3fc973 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -245,6 +245,38 @@ void drm_dsc_pps_payload_pack(struct 
drm_dsc_picture_parameter_set *pps_payload,
 }
 EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
 
+static int compute_offset(struct drm_dsc_config *vdsc_cfg, int 
pixels_per_group,
+   int groups_per_line, int grpcnt)
+{
+   int offset = 0;
+   int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
pixels_per_group);
+
+   if (grpcnt <= grpcnt_id)
+   offset = DIV_ROUND_UP(grpcnt * pixels_per_group * 
vdsc_cfg->bits_per_pixel, 16);
+   else
+   offset = DIV_ROUND_UP(grpcnt_id * pixels_per_group * 
vdsc_cfg->bits_per_pixel, 16)
+   - (((grpcnt - grpcnt_id) * vdsc_cfg->slice_bpg_offset) 
>> 11);
+
+   if (grpcnt <= groups_per_line)
+   offset += grpcnt * vdsc_cfg->first_line_bpg_offset;
+   else
+   offset += groups_per_line * vdsc_cfg->first_line_bpg_offset
+   - (((grpcnt - groups_per_line) * 
vdsc_cfg->nfl_bpg_offset) >> 11);
+
+   if (vdsc_cfg->native_420) {
+   if (grpcnt <= groups_per_line)
+   offset -= (grpcnt * vdsc_cfg->nsl_bpg_offset) >> 11;
+   else if (grpcnt <= 2 * groups_per_line)
+   offset += (grpcnt - groups_per_line) * 
vdsc_cfg->second_line_bpg_offset
+   - ((groups_per_line * vdsc_cfg->nsl_bpg_offset) 
>> 11);
+   else
+   offset += (grpcnt - groups_per_line) * 
vdsc_cfg->second_line_bpg_offset
+   - (((grpcnt - groups_per_line) * 
vdsc_cfg->nsl_bpg_offset) >> 11);
+   }
+
+   return offset;
+}
+
 /**
  * drm_dsc_compute_rc_parameters() - Write rate control
  * parameters to the dsc configuration defined in
@@ -264,6 +296,7 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config 
*vdsc_cfg)
unsigned long hrd_delay = 0;
unsigned long final_scale = 0;
unsigned long rbs_min = 0;
+   unsigned long max_offset = 0;
 
if (vdsc_cfg->native_420 || vdsc_cfg->native_422) {
/* Number of groups used to code each line of a slice */
@@ -342,6 +375,17 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config 
*vdsc_cfg)
return -ERANGE;
}
 
+   if (vdsc_cfg->slice_height > 2)
+   vdsc_cfg->nsl_bpg_offset = 
DIV_ROUND_UP((vdsc_cfg->second_line_bpg_offset << 11),
+   (vdsc_cfg->slice_height 
- 1));
+   else
+   vdsc_cfg->nsl_bpg_offset = 0;
+
+   if (vdsc_cfg->nsl_bpg_offset > 65535) {
+   DRM_DEBUG_KMS("NslBpgOffset is too large for this slice 
height\n");
+   return -ERANGE;
+   }
+
/* Number of groups used to code the entire slice */
groups_total = groups_per_line * vdsc_cfg->slice_height;
 
@@ -361,6 +405,7 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config 
*vdsc_cfg)
vdsc_cfg->scale_increment_interval =
(vdsc_cfg->final_offset * (1 << 11)) /
((vdsc_cfg->nfl_bpg_offset +
+   vdsc_cfg->nsl_bpg_offset +
vdsc_cfg->slice_bpg_offset) *
(final_scale - 9));
} else {
@@ -381,10 +426,29 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config 
*vdsc_cfg)
 * bits/pixel (bpp) rate that is used by the encoder,
 * in steps of 1/16 of a bit per pixel
 */
-   rbs_min = vdsc_cfg->rc_model_size - vdsc_cfg->initial_offset +
-   DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay *
-vdsc_cfg->bits_per_pixel, 16) +
-   groups_per_line * vdsc_cfg->first_line_bpg_offset;
+   if (vdsc_cfg->dsc_version_minor == 2 && (vdsc_cfg->native_420 || 
vdsc_cfg->native_422)) {
+
+   max_offset = compute_offset(vdsc_cfg, DSC_RC_PIXELS_PER_GROUP, 
groups_per_line,
+   
DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay,
+   DSC_RC_PIXELS_PER_GROUP));
+
+   max_offset = max(max_offset,
+   compute_offset(vdsc_cfg, 
DSC_RC_PIXELS_PER_GROUP, groups_per_line,
+   

[PATCH 2/2] drm/dsc: Use 32-bit integers for some DSC parameter calculations

[mikita.lipski]

From: Mikita Lipski 

[why]
There are a few DSC PPS parameters, such as scale_increment_interval, that
could overflow 16-bit integer if non-DSC-spec-compliant configuration was
used. There is a check in the code against that, however 16-bit integer is
used to store those values, which invalidates the check, letting invalid
configurations through.

[how]
Use 32-bit integers for the affected DSC PPS parameters calculations

Signed-off-by: Nikola Cornij 
Signed-off-by: Mikita Lipski 
---
 drivers/gpu/drm/drm_dsc.c | 31 ---
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index 79c71e3fc973..74f3527f567d 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -297,6 +297,9 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config 
*vdsc_cfg)
unsigned long final_scale = 0;
unsigned long rbs_min = 0;
unsigned long max_offset = 0;
+   u32 nfl_bpg_offset;
+   u32 nsl_bpg_offset;
+   u32 scale_increment_interval;
 
if (vdsc_cfg->native_420 || vdsc_cfg->native_422) {
/* Number of groups used to code each line of a slice */
@@ -364,28 +367,32 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config 
*vdsc_cfg)
 * hence we multiply by 2^11 for preserving the
 * fractional part
 */
-   vdsc_cfg->nfl_bpg_offset = 
DIV_ROUND_UP((vdsc_cfg->first_line_bpg_offset << 11),
+   nfl_bpg_offset = DIV_ROUND_UP((vdsc_cfg->first_line_bpg_offset 
<< 11),
(vdsc_cfg->slice_height 
- 1));
else
-   vdsc_cfg->nfl_bpg_offset = 0;
+   nfl_bpg_offset = 0;
 
/* 2^16 - 1 */
-   if (vdsc_cfg->nfl_bpg_offset > 65535) {
+   if (nfl_bpg_offset > 65535) {
DRM_DEBUG_KMS("NflBpgOffset is too large for this slice 
height\n");
return -ERANGE;
}
 
+   vdsc_cfg->nfl_bpg_offset = (u16)nfl_bpg_offset;
+
if (vdsc_cfg->slice_height > 2)
-   vdsc_cfg->nsl_bpg_offset = 
DIV_ROUND_UP((vdsc_cfg->second_line_bpg_offset << 11),
+   nsl_bpg_offset = DIV_ROUND_UP((vdsc_cfg->second_line_bpg_offset 
<< 11),
(vdsc_cfg->slice_height 
- 1));
else
-   vdsc_cfg->nsl_bpg_offset = 0;
+   nsl_bpg_offset = 0;
 
-   if (vdsc_cfg->nsl_bpg_offset > 65535) {
+   if (nsl_bpg_offset > 65535) {
DRM_DEBUG_KMS("NslBpgOffset is too large for this slice 
height\n");
return -ERANGE;
}
 
+   vdsc_cfg->nsl_bpg_offset = (u16)nsl_bpg_offset;
+
/* Number of groups used to code the entire slice */
groups_total = groups_per_line * vdsc_cfg->slice_height;
 
@@ -402,10 +409,10 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config 
*vdsc_cfg)
 * as (NflBpgOffset + SliceBpgOffset) has 11 bit fractional 
value,
 * we need divide by 2^11 from pstDscCfg values
 */
-   vdsc_cfg->scale_increment_interval =
+   scale_increment_interval =
(vdsc_cfg->final_offset * (1 << 11)) /
-   ((vdsc_cfg->nfl_bpg_offset +
-   vdsc_cfg->nsl_bpg_offset +
+   ((nfl_bpg_offset +
+   nsl_bpg_offset +
vdsc_cfg->slice_bpg_offset) *
(final_scale - 9));
} else {
@@ -413,14 +420,16 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config 
*vdsc_cfg)
 * If finalScaleValue is less than or equal to 9, a value of 0 
should
 * be used to disable the scale increment at the end of the 
slice
 */
-   vdsc_cfg->scale_increment_interval = 0;
+   scale_increment_interval = 0;
}
 
-   if (vdsc_cfg->scale_increment_interval > 65535) {
+   if (scale_increment_interval > 65535) {
DRM_DEBUG_KMS("ScaleIncrementInterval is large for slice 
height\n");
return -ERANGE;
}
 
+   vdsc_cfg->scale_increment_interval = (u16)scale_increment_interval;
+
/*
 * DSC spec mentions that bits_per_pixel specifies the target
 * bits/pixel (bpp) rate that is used by the encoder,
-- 
2.17.1

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

Re: [PATCH v3 02/14] mm/mmu_notifier: add an interval tree notifier

[Jason Gunthorpe]

On Wed, Nov 13, 2019 at 05:59:52AM -0800, Christoph Hellwig wrote:
> > +int mmu_interval_notifier_insert(struct mmu_interval_notifier *mni,
> > + struct mm_struct *mm, unsigned long start,
> > + unsigned long length,
> > + const struct mmu_interval_notifier_ops 
> > *ops);
> > +int mmu_interval_notifier_insert_locked(
> > +   struct mmu_interval_notifier *mni, struct mm_struct *mm,
> > +   unsigned long start, unsigned long length,
> > +   const struct mmu_interval_notifier_ops *ops);
> 
> Very inconsistent indentation between these two related functions.

clang-format.. The kernel config is set to prefer a line up under the
( if all the arguments will fit within the 80 cols otherwise it does a
1 tab continuation indent.

> > +   /*
> > +* The inv_end incorporates a deferred mechanism like
> > +* rtnl_unlock(). Adds and removes are queued until the final inv_end
> > +* happens then they are progressed. This arrangement for tree updates
> > +* is used to avoid using a blocking lock during
> > +* invalidate_range_start.
> 
> Nitpick:  That comment can be condensed into one less line:

The rtnl_unlock can move up a line too. My editor is failing me on
this.

> > +   /*
> > +* TODO: Since we already have a spinlock above, this would be faster
> > +* as wake_up_q
> > +*/
> > +   if (need_wake)
> > +   wake_up_all(_mm->wq);
> 
> So why is this important enough for a TODO comment, but not important
> enough to do right away?

Lets drop the comment, I'm noto sure wake_up_q is even a function this
layer should be calling.
 
> > +* release semantics on the initialization of the mmu_notifier_mm's
> > + * contents are provided for unlocked readers.  acquire can only be
> > + * used while holding the mmgrab or mmget, and is safe because once
> > + * created the mmu_notififer_mm is not freed until the mm is
> > + * destroyed.  As above, users holding the mmap_sem or one of the
> > + * mm_take_all_locks() do not need to use acquire semantics.
> >  */
> 
> Some spaces instead of tabs here.

Got it

> > +static int __mmu_interval_notifier_insert(
> > +   struct mmu_interval_notifier *mni, struct mm_struct *mm,
> > +   struct mmu_notifier_mm *mmn_mm, unsigned long start,
> > +   unsigned long length, const struct mmu_interval_notifier_ops *ops)
> 
> Odd indentation - we usuall do two tabs (my preference) or align after
> the opening brace.

This is one tab. I don't think one tab is odd, it seems pretty popular
even just in mm/.

But two tabs is considered usual? I didn't even know that.

> > + * This function must be paired with mmu_interval_notifier_insert(). It 
> > cannot be
> 
> line > 80 chars.

got it, was missed during the rename

> Otherwise this looks good and very similar to what I reviewed earlier:
> 
> Reviewed-by: Christoph Hellwig 

Thanks a bunch, your comments have been very helpful on this series!

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

Re: [PATCH 1/7] drm/amdgpu: remove set but not used variable 'mc_shared_chmap' from 'gfx_v6_0.c' and 'gfx_v7_0.c'

[Joe Perches]

On Wed, 2019-11-13 at 20:44 +0800, yu kuai wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c: In function
> ‘gfx_v6_0_constants_init’:
> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c:1579:6: warning: variable
> ‘mc_shared_chmap’ set but not used [-Wunused-but-set-variable]
[]
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
[]
> @@ -1678,7 +1678,6 @@ static void gfx_v6_0_constants_init(struct 
> amdgpu_device *adev)
>  
>   WREG32(mmBIF_FB_EN, BIF_FB_EN__FB_READ_EN_MASK | 
> BIF_FB_EN__FB_WRITE_EN_MASK);
>  
> - mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP);

I do not know the hardware but frequently hardware like
this has read ordering requirements and various registers
can not be read in a random order.

Does removing this read have no effect on the hardware?

>   adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG);
>   mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg;
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
[]
> @@ -4336,7 +4336,6 @@ static void gfx_v7_0_gpu_early_init(struct 
> amdgpu_device *adev)
>   break;
>   }
>  
> - mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP);
>   adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG);
>   mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg;

Same question.

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

Re: [bug report] drm/amd/display: Add MST atomic routines

[Mikita Lipski]

Hi Dan,

The bug has been fixed by the patch "drm/amd/display: Fix unsigned 
variable compared to less than zero" by Gustavo A. R. Silva


Thanks

On 13.11.2019 13:28, Dan Carpenter wrote:

Hello Mikita Lipski,

The patch b4c578f08378: "drm/amd/display: Add MST atomic routines"
from Nov 6, 2019, leads to the following static checker warning:

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4870 
dm_encoder_helper_atomic_check()
warn: unsigned 'dm_new_connector_state->vcpi_slots' is never less than 
zero.

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
   4851  if (!aconnector->port || !aconnector->dc_sink)
   4852  return 0;
   4853
   4854  mst_port = aconnector->port;
   4855  mst_mgr = >mst_port->mst_mgr;
   4856
   4857  if (!crtc_state->connectors_changed && 
!crtc_state->mode_changed)
   4858  return 0;
   4859
   4860  if (!state->duplicated) {
   4861  color_depth = 
convert_color_depth_from_display_info(connector, conn_state);
   4862  bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
   4863  clock = adjusted_mode->clock;
   4864  dm_new_connector_state->pbn = 
drm_dp_calc_pbn_mode(clock, bpp);
   4865  }
   4866  dm_new_connector_state->vcpi_slots = 
drm_dp_atomic_find_vcpi_slots(state,
   4867 
mst_mgr,
   4868 
mst_port,
   4869
 dm_new_connector_state->pbn);
   4870  if (dm_new_connector_state->vcpi_slots < 0) {
 ^^
Impossible condition.

   4871  DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n", 
(int)dm_new_connector_state->vcpi_slots);
   4872  return dm_new_connector_state->vcpi_slots;
   4873  }
   4874  return 0;
   4875  }

regards,
dan carpenter



--
Thanks,
Mikita Lipski
Software Engineer, AMD
mikita.lip...@amd.com
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

[Russell, Kent]

Thanks Alex, I appreciate the explanation!

Kent

From: Deucher, Alexander 
Sent: Wednesday, November 13, 2019 1:31 PM
To: Russell, Kent ; Zhao, Yong ; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

CI just refers to the dGPUs (bonaire and hawaii), the CIK refers to the whole 
family (CI dGPUs, Indus (kaveri platform), and Kerala (kabini/mullins platform).

Alex

From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Russell, Kent mailto:kent.russ...@amd.com>>
Sent: Wednesday, November 13, 2019 8:28 AM
To: Zhao, Yong mailto:yong.z...@amd.com>>; 
amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Zhao, Yong mailto:yong.z...@amd.com>>
Subject: RE: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

Should we use "CI" instead of "CIK" in the comments? I thought CIK was a short 
form for CI kickers, while CI encompasses all CI ASICs. Even though we had _cik 
as the suffix for most of the CI stuff. Just wondering about accuracy.

 Kent

-Original Message-
From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 On Behalf Of Yong Zhao
Sent: Tuesday, November 12, 2019 5:19 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhao, Yong mailto:yong.z...@amd.com>>
Subject: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

The ops_asic_specific function pointers are actually quite generic after using 
a simple if condition. Eliminate it by code refactoring.

Change-Id: Icb891289cca31acdbe2d2eea76a426f1738b9c08
Signed-off-by: Yong Zhao mailto:yong.z...@amd.com>>
---
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 63 ---  
drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h |  4 --  
.../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c  | 36 ---  
.../gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c  | 48 --
 4 files changed, 26 insertions(+), 125 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index a750b1d110eb..59ee9053498c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -87,9 +87,17 @@ static bool initialize(struct kernel_queue *kq, struct 
kfd_dev *dev,
 kq->pq_kernel_addr = kq->pq->cpu_ptr;
 kq->pq_gpu_addr = kq->pq->gpu_addr;

-   retval = kq->ops_asic_specific.initialize(kq, dev, type, queue_size);
-   if (!retval)
-   goto err_eop_allocate_vidmem;
+   /* For CIK family asics, kq->eop_mem is not needed */
+   if (dev->device_info->asic_family > CHIP_HAWAII) {
+   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
+   if (retval != 0)
+   goto err_eop_allocate_vidmem;
+
+   kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
+   kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
+
+   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
+   }

 retval = kfd_gtt_sa_allocate(dev, sizeof(*kq->rptr_kernel),
 >rptr_mem);
@@ -200,7 +208,12 @@ static void uninitialize(struct kernel_queue *kq)

 kfd_gtt_sa_free(kq->dev, kq->rptr_mem);
 kfd_gtt_sa_free(kq->dev, kq->wptr_mem);
-   kq->ops_asic_specific.uninitialize(kq);
+
+   /* For CIK family asics, kq->eop_mem is Null, kfd_gtt_sa_free()
+* is able to handle NULL properly.
+*/
+   kfd_gtt_sa_free(kq->dev, kq->eop_mem);
+
 kfd_gtt_sa_free(kq->dev, kq->pq);
 kfd_release_kernel_doorbell(kq->dev,
 kq->queue->properties.doorbell_ptr);
@@ -280,8 +293,15 @@ static void submit_packet(struct kernel_queue *kq)
 }
 pr_debug("\n");
 #endif
-
-   kq->ops_asic_specific.submit_packet(kq);
+   if (kq->dev->device_info->doorbell_size == 8) {
+   *kq->wptr64_kernel = kq->pending_wptr64;
+   write_kernel_doorbell64(kq->queue->properties.doorbell_ptr,
+   kq->pending_wptr64);
+   } else {
+   *kq->wptr_kernel = kq->pending_wptr;
+   write_kernel_doorbell(kq->queue->properties.doorbell_ptr,
+   kq->pending_wptr);
+   }
 }

 static void rollback_packet(struct kernel_queue *kq) @@ -310,42 +330,11 @@ 
struct kernel_queue *kernel_queue_init(struct kfd_dev *dev,
 kq->ops.submit_packet = submit_packet;
 kq->ops.rollback_packet = rollback_packet;

-   switch (dev->device_info->asic_family) {
-   case CHIP_KAVERI:
-   case CHIP_HAWAII:
-   case CHIP_CARRIZO:
-   case CHIP_TONGA:
-   case CHIP_FIJI:
-   case CHIP_POLARIS10:
-   case CHIP_POLARIS11:
-   case CHIP_POLARIS12:
-   case CHIP_VEGAM:
-   

[bug report] drm/amd/display: Register DMUB service with DC

[Dan Carpenter]

Hello Nicholas Kazlauskas,

This is a semi-automatic email about new static checker warnings.

The patch 0f80e7cdc012: "drm/amd/display: Register DMUB service with 
DC" from Oct 28, 2019, leads to the following Smatch complaint:

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:1049 
amdgpu_dm_fini()
 warn: variable dereferenced before check 'adev->dm.dc' (see line 1038)

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
  1029  #ifdef CONFIG_DRM_AMD_DC_HDCP
  1030  if (adev->dm.hdcp_workqueue) {
  1031  hdcp_destroy(adev->dm.hdcp_workqueue);
  1032  adev->dm.hdcp_workqueue = NULL;
  1033  }
  1034  
  1035  if (adev->dm.dc)
^^^
Check

  1036  dc_deinit_callbacks(adev->dm.dc);
  1037  #endif
  1038  if (adev->dm.dc->ctx->dmub_srv) {

Unchecked dereference

  1039  dc_dmub_srv_destroy(>dm.dc->ctx->dmub_srv);
  1040  adev->dm.dc->ctx->dmub_srv = NULL;
  1041  }
  1042  
  1043  if (adev->dm.dmub_bo)
  1044  amdgpu_bo_free_kernel(>dm.dmub_bo,
  1045>dm.dmub_bo_gpu_addr,
  1046>dm.dmub_bo_cpu_addr);
  1047  
  1048  /* DC Destroy TODO: Replace destroy DAL */
  1049  if (adev->dm.dc)
^^^
Check

  1050  dc_destroy(>dm.dc);
  1051  /*

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

Re: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

[Deucher, Alexander]

CI just refers to the dGPUs (bonaire and hawaii), the CIK refers to the whole 
family (CI dGPUs, Indus (kaveri platform), and Kerala (kabini/mullins platform).

Alex

From: amd-gfx  on behalf of Russell, 
Kent 
Sent: Wednesday, November 13, 2019 8:28 AM
To: Zhao, Yong ; amd-gfx@lists.freedesktop.org 

Cc: Zhao, Yong 
Subject: RE: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

Should we use "CI" instead of "CIK" in the comments? I thought CIK was a short 
form for CI kickers, while CI encompasses all CI ASICs. Even though we had _cik 
as the suffix for most of the CI stuff. Just wondering about accuracy.

 Kent

-Original Message-
From: amd-gfx  On Behalf Of Yong Zhao
Sent: Tuesday, November 12, 2019 5:19 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhao, Yong 
Subject: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

The ops_asic_specific function pointers are actually quite generic after using 
a simple if condition. Eliminate it by code refactoring.

Change-Id: Icb891289cca31acdbe2d2eea76a426f1738b9c08
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 63 ---  
drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h |  4 --  
.../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c  | 36 ---  
.../gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c  | 48 --
 4 files changed, 26 insertions(+), 125 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index a750b1d110eb..59ee9053498c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -87,9 +87,17 @@ static bool initialize(struct kernel_queue *kq, struct 
kfd_dev *dev,
 kq->pq_kernel_addr = kq->pq->cpu_ptr;
 kq->pq_gpu_addr = kq->pq->gpu_addr;

-   retval = kq->ops_asic_specific.initialize(kq, dev, type, queue_size);
-   if (!retval)
-   goto err_eop_allocate_vidmem;
+   /* For CIK family asics, kq->eop_mem is not needed */
+   if (dev->device_info->asic_family > CHIP_HAWAII) {
+   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
+   if (retval != 0)
+   goto err_eop_allocate_vidmem;
+
+   kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
+   kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
+
+   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
+   }

 retval = kfd_gtt_sa_allocate(dev, sizeof(*kq->rptr_kernel),
 >rptr_mem);
@@ -200,7 +208,12 @@ static void uninitialize(struct kernel_queue *kq)

 kfd_gtt_sa_free(kq->dev, kq->rptr_mem);
 kfd_gtt_sa_free(kq->dev, kq->wptr_mem);
-   kq->ops_asic_specific.uninitialize(kq);
+
+   /* For CIK family asics, kq->eop_mem is Null, kfd_gtt_sa_free()
+* is able to handle NULL properly.
+*/
+   kfd_gtt_sa_free(kq->dev, kq->eop_mem);
+
 kfd_gtt_sa_free(kq->dev, kq->pq);
 kfd_release_kernel_doorbell(kq->dev,
 kq->queue->properties.doorbell_ptr);
@@ -280,8 +293,15 @@ static void submit_packet(struct kernel_queue *kq)
 }
 pr_debug("\n");
 #endif
-
-   kq->ops_asic_specific.submit_packet(kq);
+   if (kq->dev->device_info->doorbell_size == 8) {
+   *kq->wptr64_kernel = kq->pending_wptr64;
+   write_kernel_doorbell64(kq->queue->properties.doorbell_ptr,
+   kq->pending_wptr64);
+   } else {
+   *kq->wptr_kernel = kq->pending_wptr;
+   write_kernel_doorbell(kq->queue->properties.doorbell_ptr,
+   kq->pending_wptr);
+   }
 }

 static void rollback_packet(struct kernel_queue *kq) @@ -310,42 +330,11 @@ 
struct kernel_queue *kernel_queue_init(struct kfd_dev *dev,
 kq->ops.submit_packet = submit_packet;
 kq->ops.rollback_packet = rollback_packet;

-   switch (dev->device_info->asic_family) {
-   case CHIP_KAVERI:
-   case CHIP_HAWAII:
-   case CHIP_CARRIZO:
-   case CHIP_TONGA:
-   case CHIP_FIJI:
-   case CHIP_POLARIS10:
-   case CHIP_POLARIS11:
-   case CHIP_POLARIS12:
-   case CHIP_VEGAM:
-   kernel_queue_init_vi(>ops_asic_specific);
-   break;
-
-   case CHIP_VEGA10:
-   case CHIP_VEGA12:
-   case CHIP_VEGA20:
-   case CHIP_RAVEN:
-   case CHIP_RENOIR:
-   case CHIP_ARCTURUS:
-   case CHIP_NAVI10:
-   case CHIP_NAVI12:
-   case CHIP_NAVI14:
-   kernel_queue_init_v9(>ops_asic_specific);
-   break;
-   default:
-   WARN(1, "Unexpected ASIC family %u",
-dev->device_info->asic_family);
-   goto out_free;
-   }
-
 if (kq->ops.initialize(kq, dev, type, KFD_KERNEL_QUEUE_SIZE))
 

[bug report] drm/amd/display: Add MST atomic routines

[Dan Carpenter]

Hello Mikita Lipski,

The patch b4c578f08378: "drm/amd/display: Add MST atomic routines"
from Nov 6, 2019, leads to the following static checker warning:

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4870 
dm_encoder_helper_atomic_check()
warn: unsigned 'dm_new_connector_state->vcpi_slots' is never less than 
zero.

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
  4851  if (!aconnector->port || !aconnector->dc_sink)
  4852  return 0;
  4853  
  4854  mst_port = aconnector->port;
  4855  mst_mgr = >mst_port->mst_mgr;
  4856  
  4857  if (!crtc_state->connectors_changed && 
!crtc_state->mode_changed)
  4858  return 0;
  4859  
  4860  if (!state->duplicated) {
  4861  color_depth = 
convert_color_depth_from_display_info(connector, conn_state);
  4862  bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
  4863  clock = adjusted_mode->clock;
  4864  dm_new_connector_state->pbn = 
drm_dp_calc_pbn_mode(clock, bpp);
  4865  }
  4866  dm_new_connector_state->vcpi_slots = 
drm_dp_atomic_find_vcpi_slots(state,
  4867  
   mst_mgr,
  4868  
   mst_port,
  4869  
   dm_new_connector_state->pbn);
  4870  if (dm_new_connector_state->vcpi_slots < 0) {
^^
Impossible condition.

  4871  DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n", 
(int)dm_new_connector_state->vcpi_slots);
  4872  return dm_new_connector_state->vcpi_slots;
  4873  }
  4874  return 0;
  4875  }

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

[PATCH 1/2] drm/amdgpu/powerplay: properly set PP_GFXOFF_MASK

[Alex Deucher]

So that the setting reflects what the hw supports. This will
be used in a subsequent patch so needs to be correct.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c  | 2 ++
 drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c | 7 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 57459a65eb44..ad39db49a29d 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -719,6 +719,7 @@ static int smu_set_funcs(struct amdgpu_device *adev)
 
switch (adev->asic_type) {
case CHIP_VEGA20:
+   adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
vega20_set_ppt_funcs(smu);
break;
case CHIP_NAVI10:
@@ -727,6 +728,7 @@ static int smu_set_funcs(struct amdgpu_device *adev)
navi10_set_ppt_funcs(smu);
break;
case CHIP_ARCTURUS:
+   adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
arcturus_set_ppt_funcs(smu);
/* OD is not supported on Arcturus */
smu->od_enabled =false;
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
index a24beaa4fb01..443625c83ec9 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
@@ -81,6 +81,8 @@ static void hwmgr_init_workload_prority(struct pp_hwmgr 
*hwmgr)
 
 int hwmgr_early_init(struct pp_hwmgr *hwmgr)
 {
+   struct amdgpu_device *adev = hwmgr->adev;
+
if (!hwmgr)
return -EINVAL;
 
@@ -96,6 +98,7 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
 
switch (hwmgr->chip_family) {
case AMDGPU_FAMILY_CI:
+   adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
hwmgr->smumgr_funcs = _smu_funcs;
ci_set_asic_special_caps(hwmgr);
hwmgr->feature_mask &= ~(PP_VBI_TIME_SUPPORT_MASK |
@@ -106,12 +109,14 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
smu7_init_function_pointers(hwmgr);
break;
case AMDGPU_FAMILY_CZ:
+   adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
hwmgr->od_enabled = false;
hwmgr->smumgr_funcs = _smu_funcs;
hwmgr->feature_mask &= ~PP_GFXOFF_MASK;
smu8_init_function_pointers(hwmgr);
break;
case AMDGPU_FAMILY_VI:
+   adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
hwmgr->feature_mask &= ~PP_GFXOFF_MASK;
switch (hwmgr->chip_id) {
case CHIP_TOPAZ:
@@ -153,6 +158,7 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
case AMDGPU_FAMILY_AI:
switch (hwmgr->chip_id) {
case CHIP_VEGA10:
+   adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
hwmgr->feature_mask &= ~PP_GFXOFF_MASK;
hwmgr->smumgr_funcs = _smu_funcs;
vega10_hwmgr_init(hwmgr);
@@ -162,6 +168,7 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
vega12_hwmgr_init(hwmgr);
break;
case CHIP_VEGA20:
+   adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
hwmgr->feature_mask &= ~PP_GFXOFF_MASK;
hwmgr->smumgr_funcs = _smu_funcs;
vega20_hwmgr_init(hwmgr);
-- 
2.23.0

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

[PATCH 2/2] drm/amdgpu: don't read registers if gfxoff is enabled (v2)

[Alex Deucher]

When gfxoff is enabled, accessing gfx registers via MMIO
can lead to a hang.

v2: return cached registers properly.

Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
Reviewed-by: Evan Quan 
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/nv.c| 27 --
 drivers/gpu/drm/amd/amdgpu/soc15.c | 31 ++
 2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index af68f9815f28..7283d6198b89 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -201,17 +201,25 @@ static uint32_t nv_read_indexed_register(struct 
amdgpu_device *adev, u32 se_num,
return val;
 }
 
-static uint32_t nv_get_register_value(struct amdgpu_device *adev,
+static int nv_get_register_value(struct amdgpu_device *adev,
  bool indexed, u32 se_num,
- u32 sh_num, u32 reg_offset)
+ u32 sh_num, u32 reg_offset,
+ u32 *value)
 {
if (indexed) {
-   return nv_read_indexed_register(adev, se_num, sh_num, 
reg_offset);
+   if (adev->pm.pp_feature & PP_GFXOFF_MASK)
+   return -EINVAL;
+   *value = nv_read_indexed_register(adev, se_num, sh_num, 
reg_offset);
} else {
-   if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
-   return adev->gfx.config.gb_addr_config;
-   return RREG32(reg_offset);
+   if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG)) {
+   *value = adev->gfx.config.gb_addr_config;
+   } else {
+   if (adev->pm.pp_feature & PP_GFXOFF_MASK)
+   return -EINVAL;
+   *value = RREG32(reg_offset);
+   }
}
+   return 0;
 }
 
 static int nv_read_register(struct amdgpu_device *adev, u32 se_num,
@@ -227,10 +235,9 @@ static int nv_read_register(struct amdgpu_device *adev, 
u32 se_num,
(adev->reg_offset[en->hwip][en->inst][en->seg] + 
en->reg_offset))
continue;
 
-   *value = nv_get_register_value(adev,
-  
nv_allowed_read_registers[i].grbm_indexed,
-  se_num, sh_num, reg_offset);
-   return 0;
+   return nv_get_register_value(adev,
+
nv_allowed_read_registers[i].grbm_indexed,
+se_num, sh_num, reg_offset, value);
}
return -EINVAL;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 305ad3eec987..2cc16e9f39fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -363,19 +363,27 @@ static uint32_t soc15_read_indexed_register(struct 
amdgpu_device *adev, u32 se_n
return val;
 }
 
-static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
+static int soc15_get_register_value(struct amdgpu_device *adev,
 bool indexed, u32 se_num,
-u32 sh_num, u32 reg_offset)
+u32 sh_num, u32 reg_offset,
+u32 *value)
 {
if (indexed) {
-   return soc15_read_indexed_register(adev, se_num, sh_num, 
reg_offset);
+   if (adev->pm.pp_feature & PP_GFXOFF_MASK)
+   return -EINVAL;
+   *value = soc15_read_indexed_register(adev, se_num, sh_num, 
reg_offset);
} else {
-   if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG))
-   return adev->gfx.config.gb_addr_config;
-   else if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmDB_DEBUG2))
-   return adev->gfx.config.db_debug2;
-   return RREG32(reg_offset);
+   if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG)) {
+   *value = adev->gfx.config.gb_addr_config;
+   } else if (reg_offset == SOC15_REG_OFFSET(GC, 0, mmDB_DEBUG2)) {
+   *value = adev->gfx.config.db_debug2;
+   } else {
+   if (adev->pm.pp_feature & PP_GFXOFF_MASK)
+   return -EINVAL;
+   *value = RREG32(reg_offset);
+   }
}
+   return 0;
 }
 
 static int soc15_read_register(struct amdgpu_device *adev, u32 se_num,
@@ -391,10 +399,9 @@ static int soc15_read_register(struct amdgpu_device *adev, 
u32 se_num,
+ en->reg_offset))
continue;
 
-   *value = 

Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr

[Andrey Grodzovsky]

On 11/13/19 9:20 AM, Christian König wrote:
Another more fundamental question: Could we get rid of the timeout job 
at all?



There are other stuff there besides picking the first unfinished job 
which is common for all the drivers - such as freeing guilty signaled 
job and rearming the timeout work timer.





I mean we used to give this as parameter to the scheduler callback 
because we had the timeout worker in the job, but that is no longer 
the case.


E.g. in drm_sched_job_timedout() we do the following:

    job = list_first_entry_or_null(>ring_mirror_list,
   struct drm_sched_job, node);


Why don't we just remove that here and only get the first job after we 
have stopped the scheduler?



Should be ok since we have the extra check for __kthread_should_park in 
drm_sched_cleanup_jobs which will protect us in this case from a wakeup 
of sched thread and execution of in drm_sched_cleanup_jobs after we 
already parked it. The problem here is we need the drm_sched_job to 
access the private data for each client driver (see amdgpu_job_timedout 
for example). What about instead of peeking at the job to actually 
remove it from ring_mirror_list right there, go ahead with it through 
the reset routine, if it's signaled in the meanwhile that great - 
release it, otherwise put it back into ring_mirror_list in 
drm_sched_resubmit_jobs.


Andrey




Regards,
Christian.

Am 13.11.19 um 15:12 schrieb Andrey Grodzovsky:


This why I asked for a trace with timer enabled, but since there is a 
finite number of places we touch the timer Emily can just put prints 
there. Also, I wonder if this temp fix helps her with the issue or not.


Andrey

On 11/13/19 2:36 AM, Christian König wrote:

The question is where do we rearm the timer for this problem to occur?

Regards,
Christian.

Am 12.11.19 um 20:21 schrieb Andrey Grodzovsky:


I was able to reproduce the crash by using the attached 
simulate_crash.patch - waiting on guilty job to signal in reset 
work and artificially rearming the timeout timer just before the 
check for !cancel_delayed_work(>work_tdr)  in 
drm_sched_cleanup_jobs - crash log attached in crash.log. This I 
think confirms my theory i described earlier in this thread.


basic_fix.patch handles this by testing whether another timer 
already armed ob this scheduler or is there a timeout work in 
execution right now (see documentation for work_busy) - obviously  
this is not a full solution as this will not protect from races if 
for example there is immediate work scheduling such as in 
drm_sched_fault -  so we probably need to account for this by 
making drm_sched_cleanup_jobs (at least in the part where it 
iterates ring mirror list and frees jobs) and GPU reset really 
mutually exclusive and not like now.


Andrey


On 11/11/19 4:11 PM, Christian König wrote:

Hi Emily,

you need to print which scheduler instance is freeing the jobs and 
which one is triggering the reset. The TID and PID is completely 
meaningless here since we are called from different worker threads 
and the TID/PID can change on each call.


Apart from that I will look into this a bit deeper when I have time.

Regards,
Christian.

Am 12.11.19 um 07:02 schrieb Deng, Emily:

Hi Christian,
    I add the follow print in function drm_sched_cleanup_jobs. 
From the log it shows that only use cancel_delayed_work could not 
avoid to free job when the sched is in reset. But don’t know 
exactly where it is wrong about the driver. Do you have any 
suggestion about this?
+ printk("Emily:drm_sched_cleanup_jobs:begin,tid:%lu, pid:%lu\n", 
current->tgid, current->pid);

    /*
 * Don't destroy jobs while the timeout worker is 
running  OR thread
 * is being parked and hence assumed to not touch 
ring_mirror_list

 */
 if ((sched->timeout != MAX_SCHEDULE_TIMEOUT &&
!cancel_delayed_work(>work_tdr)))
    return;
+ printk("Emily:drm_sched_cleanup_jobs,tid:%lu, pid:%lu\n", 
current->tgid, current->pid);

Best wishes
Emily Deng
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695091] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695104] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695105] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695107] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695107] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.222954] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring 
sdma0 timeout, signaled seq=78585, emitted seq=78587
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.224275] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* 

Re: [PATCH 0/7] various '-Wunused-but-set-variable' gcc warning fixes

[Alex Deucher]

Applied the series, although a couple of the patches were already
applied from a previous patch set.

Thanks,

Alex

On Wed, Nov 13, 2019 at 9:12 AM yu kuai  wrote:
>
> This patch set fixes various unrelated gcc '-Wunused-but-set-variable'
> warnings.
>
> yu kuai (7):
>   drm/amdgpu: remove set but not used variable 'mc_shared_chmap' from
> 'gfx_v6_0.c' and 'gfx_v7_0.c'
>   drm/amdgpu: remove set but not used variable 'amdgpu_connector'
>   drm/amdgpu: remove set but not used variable 'count'
>   drm/amdgpu: remove set but not used variable 'invalid'
>   drm/amdgpu: remove set but not used variable 'threshold'
>   drm/amdgpu: remove set but not used variable 'state'
>   drm/amdgpu: remove set but not used variable 'us_mvdd'
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c|  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  2 --
>  drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c   |  3 +--
>  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   |  3 +--
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c |  4 ++--
>  drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c  |  7 ++-
>  drivers/gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c | 12 
>  7 files changed, 8 insertions(+), 27 deletions(-)
>
> --
> 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

Re: [PATCH 2/2] drm/amdgpu: init umc functions for arcturus umc ras

[Alex Deucher]

On Wed, Nov 13, 2019 at 9:57 AM Hawking Zhang  wrote:
>
> reuse vg20 umc functions for arcturus umc ras
>
> Change-Id: Ia8af3c20a717c76ec18aa5fa332cfd81ca60ff69
> Signed-off-by: Hawking Zhang 

Series is:
Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 3784b62201b0..8a5b722ce5b7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -635,6 +635,7 @@ static void gmc_v9_0_set_umc_funcs(struct amdgpu_device 
> *adev)
> adev->umc.funcs = _v6_0_funcs;
> break;
> case CHIP_VEGA20:
> +   case CHIP_ARCTURUS:
> adev->umc.max_ras_err_cnt_per_query = 
> UMC_V6_1_TOTAL_CHANNEL_NUM;
> adev->umc.channel_inst_num = UMC_V6_1_CHANNEL_INSTANCE_NUM;
> adev->umc.umc_inst_num = UMC_V6_1_UMC_INSTANCE_NUM;
> @@ -748,6 +749,7 @@ static int gmc_v9_0_late_init(void *handle)
> switch (adev->asic_type) {
> case CHIP_VEGA10:
> case CHIP_VEGA20:
> +   case CHIP_ARCTURUS:
> r = amdgpu_atomfirmware_mem_ecc_supported(adev);
> if (!r) {
> DRM_INFO("ECC is not present.\n");
> --
> 2.17.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

[PATCH 2/2] drm/amdgpu: init umc functions for arcturus umc ras

[Hawking Zhang]

reuse vg20 umc functions for arcturus umc ras

Change-Id: Ia8af3c20a717c76ec18aa5fa332cfd81ca60ff69
Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 3784b62201b0..8a5b722ce5b7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -635,6 +635,7 @@ static void gmc_v9_0_set_umc_funcs(struct amdgpu_device 
*adev)
adev->umc.funcs = _v6_0_funcs;
break;
case CHIP_VEGA20:
+   case CHIP_ARCTURUS:
adev->umc.max_ras_err_cnt_per_query = 
UMC_V6_1_TOTAL_CHANNEL_NUM;
adev->umc.channel_inst_num = UMC_V6_1_CHANNEL_INSTANCE_NUM;
adev->umc.umc_inst_num = UMC_V6_1_UMC_INSTANCE_NUM;
@@ -748,6 +749,7 @@ static int gmc_v9_0_late_init(void *handle)
switch (adev->asic_type) {
case CHIP_VEGA10:
case CHIP_VEGA20:
+   case CHIP_ARCTURUS:
r = amdgpu_atomfirmware_mem_ecc_supported(adev);
if (!r) {
DRM_INFO("ECC is not present.\n");
-- 
2.17.1

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

[PATCH 1/2] drm/amdgpu: enable ras capablity check on arcturus

[Hawking Zhang]

check hw ras capablity via atomfirmware

Change-Id: I495b73ac6c04910de2ad8d9c46e98873fb5bc44d
Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index d2e0859f57a0..98e41d9c2fc0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -1661,7 +1661,8 @@ static void amdgpu_ras_check_supported(struct 
amdgpu_device *adev,
*supported = 0;
 
if (amdgpu_sriov_vf(adev) ||
-   adev->asic_type != CHIP_VEGA20)
+   (adev->asic_type != CHIP_VEGA20 &&
+adev->asic_type != CHIP_ARCTURUS))
return;
 
if (adev->is_atom_fw &&
-- 
2.17.1

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

Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr

[Christian König]

Another more fundamental question: Could we get rid of the timeout job 
at all?


I mean we used to give this as parameter to the scheduler callback 
because we had the timeout worker in the job, but that is no longer the 
case.


E.g. in drm_sched_job_timedout() we do the following:

    job = list_first_entry_or_null(>ring_mirror_list,
   struct drm_sched_job, node);


Why don't we just remove that here and only get the first job after we 
have stopped the scheduler?


Regards,
Christian.

Am 13.11.19 um 15:12 schrieb Andrey Grodzovsky:


This why I asked for a trace with timer enabled, but since there is a 
finite number of places we touch the timer Emily can just put prints 
there. Also, I wonder if this temp fix helps her with the issue or not.


Andrey

On 11/13/19 2:36 AM, Christian König wrote:

The question is where do we rearm the timer for this problem to occur?

Regards,
Christian.

Am 12.11.19 um 20:21 schrieb Andrey Grodzovsky:


I was able to reproduce the crash by using the attached 
simulate_crash.patch - waiting on guilty job to signal in reset work 
and artificially rearming the timeout timer just before the check 
for !cancel_delayed_work(>work_tdr)  in 
drm_sched_cleanup_jobs - crash log attached in crash.log. This I 
think confirms my theory i described earlier in this thread.


basic_fix.patch handles this by testing whether another timer 
already armed ob this scheduler or is there a timeout work in 
execution right now (see documentation for work_busy) - obviously  
this is not a full solution as this will not protect from races if 
for example there is immediate work scheduling such as in 
drm_sched_fault -  so we probably need to account for this by making 
drm_sched_cleanup_jobs (at least in the part where it iterates ring 
mirror list and frees jobs) and GPU reset really mutually exclusive 
and not like now.


Andrey


On 11/11/19 4:11 PM, Christian König wrote:

Hi Emily,

you need to print which scheduler instance is freeing the jobs and 
which one is triggering the reset. The TID and PID is completely 
meaningless here since we are called from different worker threads 
and the TID/PID can change on each call.


Apart from that I will look into this a bit deeper when I have time.

Regards,
Christian.

Am 12.11.19 um 07:02 schrieb Deng, Emily:

Hi Christian,
    I add the follow print in function drm_sched_cleanup_jobs. 
From the log it shows that only use cancel_delayed_work could not 
avoid to free job when the sched is in reset. But don’t know 
exactly where it is wrong about the driver. Do you have any 
suggestion about this?
+ printk("Emily:drm_sched_cleanup_jobs:begin,tid:%lu, pid:%lu\n", 
current->tgid, current->pid);

    /*
 * Don't destroy jobs while the timeout worker is running  
OR thread
 * is being parked and hence assumed to not touch 
ring_mirror_list

 */
 if ((sched->timeout != MAX_SCHEDULE_TIMEOUT &&
!cancel_delayed_work(>work_tdr)))
    return;
+ printk("Emily:drm_sched_cleanup_jobs,tid:%lu, pid:%lu\n", 
current->tgid, current->pid);

Best wishes
Emily Deng
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695091] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695104] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695105] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695107] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695107] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.222954] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring 
sdma0 timeout, signaled seq=78585, emitted seq=78587
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.224275] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process 
information: process  pid 0 thread pid 0, 
s_job:fe75ab36,tid=15603, pid=15603
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225413] amdgpu :00:08.0: GPU reset begin!
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225417] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225425] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225425] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225428] Emily:amdgpu_job_free_cb,Process information: 
process  pid 0 thread  pid 0, s_job:fe75ab36, tid:2262, 
pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225429] 

[PATCH 14/21] drm/amd/powerplay: add JPEG power control for Renoir

[Leo Liu]

By using its own JPEG PowerUp and PowerDown messages

v2: add argument to PowerDownJpeg message

Signed-off-by: Leo Liu 
---
 drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c 
b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
index 492a201554e8..784903a313b7 100644
--- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
@@ -301,6 +301,31 @@ static int renoir_dpm_set_uvd_enable(struct smu_context 
*smu, bool enable)
return ret;
 }
 
+static int renoir_dpm_set_jpeg_enable(struct smu_context *smu, bool enable)
+{
+   struct smu_power_context *smu_power = >smu_power;
+   struct smu_power_gate *power_gate = _power->power_gate;
+   int ret = 0;
+
+   if (enable) {
+   if (smu_feature_is_enabled(smu, SMU_FEATURE_JPEG_PG_BIT)) {
+   ret = smu_send_smc_msg_with_param(smu, 
SMU_MSG_PowerUpJpeg, 0);
+   if (ret)
+   return ret;
+   }
+   power_gate->jpeg_gated = false;
+   } else {
+   if (smu_feature_is_enabled(smu, SMU_FEATURE_JPEG_PG_BIT)) {
+   ret = smu_send_smc_msg_with_param(smu, 
SMU_MSG_PowerDownJpeg, 0);
+   if (ret)
+   return ret;
+   }
+   power_gate->jpeg_gated = true;
+   }
+
+   return ret;
+}
+
 static int renoir_force_dpm_limit_value(struct smu_context *smu, bool highest)
 {
int ret = 0, i = 0;
@@ -683,6 +708,7 @@ static const struct pptable_funcs renoir_ppt_funcs = {
.print_clk_levels = renoir_print_clk_levels,
.get_current_power_state = renoir_get_current_power_state,
.dpm_set_uvd_enable = renoir_dpm_set_uvd_enable,
+   .dpm_set_jpeg_enable = renoir_dpm_set_jpeg_enable,
.force_dpm_limit_value = renoir_force_dpm_limit_value,
.unforce_dpm_levels = renoir_unforce_dpm_levels,
.get_workload_type = renoir_get_workload_type,
-- 
2.17.1

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

Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr

[Andrey Grodzovsky]

This why I asked for a trace with timer enabled, but since there is a 
finite number of places we touch the timer Emily can just put prints 
there. Also, I wonder if this temp fix helps her with the issue or not.


Andrey

On 11/13/19 2:36 AM, Christian König wrote:

The question is where do we rearm the timer for this problem to occur?

Regards,
Christian.

Am 12.11.19 um 20:21 schrieb Andrey Grodzovsky:


I was able to reproduce the crash by using the attached 
simulate_crash.patch - waiting on guilty job to signal in reset work 
and artificially rearming the timeout timer just before the check for 
!cancel_delayed_work(>work_tdr)  in drm_sched_cleanup_jobs - 
crash log attached in crash.log. This I think confirms my theory i 
described earlier in this thread.


basic_fix.patch handles this by testing whether another timer already 
armed ob this scheduler or is there a timeout work in execution right 
now (see documentation for work_busy) - obviously  this is not a full 
solution as this will not protect from races if for example there is 
immediate work scheduling such as in drm_sched_fault -  so we 
probably need to account for this by making drm_sched_cleanup_jobs 
(at least in the part where it iterates ring mirror list and frees 
jobs) and GPU reset really mutually exclusive and not like now.


Andrey


On 11/11/19 4:11 PM, Christian König wrote:

Hi Emily,

you need to print which scheduler instance is freeing the jobs and 
which one is triggering the reset. The TID and PID is completely 
meaningless here since we are called from different worker threads 
and the TID/PID can change on each call.


Apart from that I will look into this a bit deeper when I have time.

Regards,
Christian.

Am 12.11.19 um 07:02 schrieb Deng, Emily:

Hi Christian,
    I add the follow print in function drm_sched_cleanup_jobs. From 
the log it shows that only use cancel_delayed_work could not avoid 
to free job when the sched is in reset. But don’t know exactly 
where it is wrong about the driver. Do you have any suggestion 
about this?
+ printk("Emily:drm_sched_cleanup_jobs:begin,tid:%lu, pid:%lu\n", 
current->tgid, current->pid);

    /*
 * Don't destroy jobs while the timeout worker is running  
OR thread
 * is being parked and hence assumed to not touch 
ring_mirror_list

 */
 if ((sched->timeout != MAX_SCHEDULE_TIMEOUT &&
!cancel_delayed_work(>work_tdr)))
    return;
+ printk("Emily:drm_sched_cleanup_jobs,tid:%lu, pid:%lu\n", 
current->tgid, current->pid);

Best wishes
Emily Deng
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695091] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695104] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695105] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695107] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11380.695107] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.222954] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring 
sdma0 timeout, signaled seq=78585, emitted seq=78587
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.224275] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process 
information: process  pid 0 thread pid 0, 
s_job:fe75ab36,tid=15603, pid=15603
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225413] amdgpu :00:08.0: GPU reset begin!
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225417] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225425] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225425] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225428] Emily:amdgpu_job_free_cb,Process information: 
process  pid 0 thread  pid 0, s_job:fe75ab36, tid:2262, 
pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225429] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225430] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225473] Emily:drm_sched_cleanup_jobs:begin,tid:2253, pid:2253
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225486] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: 
[11381.225489] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 

[PATCH 13/21] drm/amd/powerplay: add Powergate JPEG for Renoir

[Leo Liu]

Similar to SDMA, VCN etc.

v2: add argument to both PowerUpJpeg and PowerDownJpeg messages

Signed-off-by: Leo Liu 
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c|  2 ++
 drivers/gpu/drm/amd/powerplay/inc/smu_v12_0.h |  2 ++
 drivers/gpu/drm/amd/powerplay/renoir_ppt.c|  1 +
 drivers/gpu/drm/amd/powerplay/smu_internal.h  |  2 ++
 drivers/gpu/drm/amd/powerplay/smu_v12_0.c | 11 +++
 5 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 69243a858dd5..211934521d37 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1229,6 +1229,7 @@ static int smu_hw_init(void *handle)
if (adev->flags & AMD_IS_APU) {
smu_powergate_sdma(>smu, false);
smu_powergate_vcn(>smu, false);
+   smu_powergate_jpeg(>smu, false);
smu_set_gfx_cgpg(>smu, true);
}
 
@@ -1287,6 +1288,7 @@ static int smu_hw_fini(void *handle)
if (adev->flags & AMD_IS_APU) {
smu_powergate_sdma(>smu, true);
smu_powergate_vcn(>smu, true);
+   smu_powergate_jpeg(>smu, true);
}
 
ret = smu_stop_thermal_control(smu);
diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v12_0.h 
b/drivers/gpu/drm/amd/powerplay/inc/smu_v12_0.h
index 9b9f5df0911c..1745e0146fba 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu_v12_0.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v12_0.h
@@ -58,6 +58,8 @@ int smu_v12_0_powergate_sdma(struct smu_context *smu, bool 
gate);
 
 int smu_v12_0_powergate_vcn(struct smu_context *smu, bool gate);
 
+int smu_v12_0_powergate_jpeg(struct smu_context *smu, bool gate);
+
 int smu_v12_0_set_gfx_cgpg(struct smu_context *smu, bool enable);
 
 uint32_t smu_v12_0_get_gfxoff_status(struct smu_context *smu);
diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c 
b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
index 04daf7e9fe05..492a201554e8 100644
--- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
@@ -697,6 +697,7 @@ static const struct pptable_funcs renoir_ppt_funcs = {
.check_fw_version = smu_v12_0_check_fw_version,
.powergate_sdma = smu_v12_0_powergate_sdma,
.powergate_vcn = smu_v12_0_powergate_vcn,
+   .powergate_jpeg = smu_v12_0_powergate_jpeg,
.send_smc_msg = smu_v12_0_send_msg,
.send_smc_msg_with_param = smu_v12_0_send_msg_with_param,
.read_smc_arg = smu_v12_0_read_arg,
diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h 
b/drivers/gpu/drm/amd/powerplay/smu_internal.h
index 8bcda7871309..70c4d66721cd 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
+++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
@@ -42,6 +42,8 @@
((smu)->ppt_funcs->powergate_sdma ? 
(smu)->ppt_funcs->powergate_sdma((smu), (gate)) : 0)
 #define smu_powergate_vcn(smu, gate) \
((smu)->ppt_funcs->powergate_vcn ? 
(smu)->ppt_funcs->powergate_vcn((smu), (gate)) : 0)
+#define smu_powergate_jpeg(smu, gate) \
+   ((smu)->ppt_funcs->powergate_jpeg ? 
(smu)->ppt_funcs->powergate_jpeg((smu), (gate)) : 0)
 
 #define smu_get_vbios_bootup_values(smu) \
((smu)->ppt_funcs->get_vbios_bootup_values ? 
(smu)->ppt_funcs->get_vbios_bootup_values((smu)) : 0)
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v12_0.c 
b/drivers/gpu/drm/amd/powerplay/smu_v12_0.c
index 139dd737eaa5..18b24f954380 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v12_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v12_0.c
@@ -203,6 +203,17 @@ int smu_v12_0_powergate_vcn(struct smu_context *smu, bool 
gate)
return smu_send_smc_msg(smu, SMU_MSG_PowerUpVcn);
 }
 
+int smu_v12_0_powergate_jpeg(struct smu_context *smu, bool gate)
+{
+   if (!(smu->adev->flags & AMD_IS_APU))
+   return 0;
+
+   if (gate)
+   return smu_send_smc_msg_with_param(smu, SMU_MSG_PowerDownJpeg, 
0);
+   else
+   return smu_send_smc_msg_with_param(smu, SMU_MSG_PowerUpJpeg, 0);
+}
+
 int smu_v12_0_set_gfx_cgpg(struct smu_context *smu, bool enable)
 {
if (!(smu->adev->pg_flags & AMD_PG_SUPPORT_GFX_PG))
-- 
2.17.1

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

[PATCH 1/7] drm/amdgpu: remove set but not used variable 'mc_shared_chmap' from 'gfx_v6_0.c' and 'gfx_v7_0.c'

[yu kuai]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c: In function
‘gfx_v6_0_constants_init’:
drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c:1579:6: warning: variable
‘mc_shared_chmap’ set but not used [-Wunused-but-set-variable]

drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c: In function
‘gfx_v7_0_gpu_early_init’:
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:4262:6: warning: variable
‘mc_shared_chmap’ set but not used [-Wunused-but-set-variable]

Fixes: 2cd46ad22383 ("drm/amdgpu: add graphic pipeline implementation for si 
v8")
Fixes: d93f3ca706b8 ("drm/amdgpu/gfx7: rework gpu_init()")
Signed-off-by: yu kuai 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 3 +--
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index e83b6e0..95bb242 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -1576,7 +1576,7 @@ static void gfx_v6_0_config_init(struct amdgpu_device 
*adev)
 static void gfx_v6_0_constants_init(struct amdgpu_device *adev)
 {
u32 gb_addr_config = 0;
-   u32 mc_shared_chmap, mc_arb_ramcfg;
+   u32 mc_arb_ramcfg;
u32 sx_debug_1;
u32 hdp_host_path_cntl;
u32 tmp;
@@ -1678,7 +1678,6 @@ static void gfx_v6_0_constants_init(struct amdgpu_device 
*adev)
 
WREG32(mmBIF_FB_EN, BIF_FB_EN__FB_READ_EN_MASK | 
BIF_FB_EN__FB_WRITE_EN_MASK);
 
-   mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP);
adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG);
mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 6b1c5ef..43ae8fc 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -4259,7 +4259,7 @@ static int gfx_v7_0_late_init(void *handle)
 static void gfx_v7_0_gpu_early_init(struct amdgpu_device *adev)
 {
u32 gb_addr_config;
-   u32 mc_shared_chmap, mc_arb_ramcfg;
+   u32 mc_arb_ramcfg;
u32 dimm00_addr_map, dimm01_addr_map, dimm10_addr_map, dimm11_addr_map;
u32 tmp;
 
@@ -4336,7 +4336,6 @@ static void gfx_v7_0_gpu_early_init(struct amdgpu_device 
*adev)
break;
}
 
-   mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP);
adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG);
mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg;
 
-- 
2.7.4

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

Re: [PATCH v3 03/14] mm/hmm: allow hmm_range to be used with a mmu_interval_notifier or hmm_mirror

[Christoph Hellwig]

Looks good,

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

[PATCH 3/7] drm/amdgpu: remove set but not used variable 'count'

[yu kuai]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/amdkfd/kfd_device.c: In function
‘kgd2kfd_post_reset’:
drivers/gpu/drm/amd/amdkfd/kfd_device.c:745:11: warning:
variable ‘count’ set but not used [-Wunused-but-set-variable]

'count' is never used, so can be removed. Thus 'atomic_dec_return'
can be replaced as 'atomic_dec'

Fixes: e42051d2133b ("drm/amdkfd: Implement GPU reset handlers in KFD")
Signed-off-by: yu kuai 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 4fa8834..209bfc8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -742,7 +742,7 @@ int kgd2kfd_pre_reset(struct kfd_dev *kfd)
 
 int kgd2kfd_post_reset(struct kfd_dev *kfd)
 {
-   int ret, count;
+   int ret;
 
if (!kfd->init_complete)
return 0;
@@ -750,7 +750,7 @@ int kgd2kfd_post_reset(struct kfd_dev *kfd)
ret = kfd_resume(kfd);
if (ret)
return ret;
-   count = atomic_dec_return(_locked);
+   atomic_dec(_locked);
 
atomic_set(>sram_ecc_flag, 0);
 
-- 
2.7.4

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

[PATCH 0/7] various '-Wunused-but-set-variable' gcc warning fixes

[yu kuai]

This patch set fixes various unrelated gcc '-Wunused-but-set-variable'
warnings.

yu kuai (7):
  drm/amdgpu: remove set but not used variable 'mc_shared_chmap' from
'gfx_v6_0.c' and 'gfx_v7_0.c'
  drm/amdgpu: remove set but not used variable 'amdgpu_connector'
  drm/amdgpu: remove set but not used variable 'count'
  drm/amdgpu: remove set but not used variable 'invalid'
  drm/amdgpu: remove set but not used variable 'threshold'
  drm/amdgpu: remove set but not used variable 'state'
  drm/amdgpu: remove set but not used variable 'us_mvdd'

 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c|  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  2 --
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c   |  3 +--
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   |  3 +--
 drivers/gpu/drm/amd/amdkfd/kfd_device.c |  4 ++--
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c  |  7 ++-
 drivers/gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c | 12 
 7 files changed, 8 insertions(+), 27 deletions(-)

-- 
2.7.4

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

Re: [PATCH v3 02/14] mm/mmu_notifier: add an interval tree notifier

[Christoph Hellwig]

> +int mmu_interval_notifier_insert(struct mmu_interval_notifier *mni,
> +  struct mm_struct *mm, unsigned long start,
> +  unsigned long length,
> +  const struct mmu_interval_notifier_ops *ops);
> +int mmu_interval_notifier_insert_locked(
> + struct mmu_interval_notifier *mni, struct mm_struct *mm,
> + unsigned long start, unsigned long length,
> + const struct mmu_interval_notifier_ops *ops);

Very inconsistent indentation between these two related functions.

> + /*
> +  * The inv_end incorporates a deferred mechanism like
> +  * rtnl_unlock(). Adds and removes are queued until the final inv_end
> +  * happens then they are progressed. This arrangement for tree updates
> +  * is used to avoid using a blocking lock during
> +  * invalidate_range_start.

Nitpick:  That comment can be condensed into one less line:

/*
 * The inv_end incorporates a deferred mechanism like rtnl_unlock().
 * Adds and removes are queued until the final inv_end happens then
 * they are progressed. This arrangement for tree updates is used to
 * avoid using a blocking lock during invalidate_range_start.
 */

> + /*
> +  * TODO: Since we already have a spinlock above, this would be faster
> +  * as wake_up_q
> +  */
> + if (need_wake)
> + wake_up_all(_mm->wq);

So why is this important enough for a TODO comment, but not important
enough to do right away?

> +  * release semantics on the initialization of the mmu_notifier_mm's
> + * contents are provided for unlocked readers.  acquire can only be
> + * used while holding the mmgrab or mmget, and is safe because once
> + * created the mmu_notififer_mm is not freed until the mm is
> + * destroyed.  As above, users holding the mmap_sem or one of the
> + * mm_take_all_locks() do not need to use acquire semantics.
>*/

Some spaces instead of tabs here.

> +static int __mmu_interval_notifier_insert(
> + struct mmu_interval_notifier *mni, struct mm_struct *mm,
> + struct mmu_notifier_mm *mmn_mm, unsigned long start,
> + unsigned long length, const struct mmu_interval_notifier_ops *ops)

Odd indentation - we usuall do two tabs (my preference) or align after
the opening brace.

> + * This function must be paired with mmu_interval_notifier_insert(). It 
> cannot be

line > 80 chars.

Otherwise this looks good and very similar to what I reviewed earlier:

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

[PATCH 5/7] drm/amd/powerplay: remove set but not used variable 'threshold'

[yu kuai]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c: In
function ‘fiji_populate_single_graphic_level’:
drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c:943:11:
warning: variable ‘threshold’ set but not used
[-Wunused-but-set-variable]

It is never used, so can be removed.

Fixes: 2e112b4ae3ba ("drm/amd/pp: remove fiji_smc/smumgr split.")
Signed-off-by: yu kuai 
---
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
index da025b1..c3df3a3 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
@@ -940,7 +940,7 @@ static int fiji_populate_single_graphic_level(struct 
pp_hwmgr *hwmgr,
 {
int result;
/* PP_Clocks minClocks; */
-   uint32_t threshold, mvdd;
+   uint32_t mvdd;
struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
struct phm_ppt_v1_information *table_info =
(struct phm_ppt_v1_information *)(hwmgr->pptable);
@@ -973,8 +973,6 @@ static int fiji_populate_single_graphic_level(struct 
pp_hwmgr *hwmgr,
level->VoltageDownHyst = 0;
level->PowerThrottle = 0;
 
-   threshold = clock * data->fast_watermark_threshold / 100;
-
data->display_timing.min_clock_in_sr = 
hwmgr->display_config->min_core_set_clock_in_sr;
 
if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps, 
PHM_PlatformCaps_SclkDeepSleep))
-- 
2.7.4

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

[PATCH 12/21] drm/amd/powerplay: add JPEG power control for Navi1x

[Leo Liu]

By separating the JPEG power feature, and using its
own PowerUp and PowerDown messages

v2: remove PowerUpJpeg message argument

Signed-off-by: Leo Liu 
---
 drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 32 --
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c 
b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index aeb9c1e341c7..dce6f76ecbe5 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -384,8 +384,10 @@ navi10_get_allowed_feature_mask(struct smu_context *smu,
*(uint64_t *)feature_mask |= FEATURE_MASK(FEATURE_ATHUB_PG_BIT);
 
if (smu->adev->pg_flags & AMD_PG_SUPPORT_VCN)
-   *(uint64_t *)feature_mask |= FEATURE_MASK(FEATURE_VCN_PG_BIT)
-   | FEATURE_MASK(FEATURE_JPEG_PG_BIT);
+   *(uint64_t *)feature_mask |= FEATURE_MASK(FEATURE_VCN_PG_BIT);
+
+   if (smu->adev->pg_flags & AMD_PG_SUPPORT_JPEG)
+   *(uint64_t *)feature_mask |= FEATURE_MASK(FEATURE_JPEG_PG_BIT);
 
/* disable DPM UCLK and DS SOCCLK on navi10 A0 secure board */
if (is_asic_secure(smu)) {
@@ -665,6 +667,31 @@ static int navi10_dpm_set_uvd_enable(struct smu_context 
*smu, bool enable)
return ret;
 }
 
+static int navi10_dpm_set_jpeg_enable(struct smu_context *smu, bool enable)
+{
+   struct smu_power_context *smu_power = >smu_power;
+   struct smu_power_gate *power_gate = _power->power_gate;
+   int ret = 0;
+
+   if (enable) {
+   if (smu_feature_is_enabled(smu, SMU_FEATURE_JPEG_PG_BIT)) {
+   ret = smu_send_smc_msg(smu, SMU_MSG_PowerUpJpeg);
+   if (ret)
+   return ret;
+   }
+   power_gate->jpeg_gated = false;
+   } else {
+   if (smu_feature_is_enabled(smu, SMU_FEATURE_JPEG_PG_BIT)) {
+   ret = smu_send_smc_msg(smu, SMU_MSG_PowerDownJpeg);
+   if (ret)
+   return ret;
+   }
+   power_gate->jpeg_gated = true;
+   }
+
+   return ret;
+}
+
 static int navi10_get_current_clk_freq_by_table(struct smu_context *smu,
   enum smu_clk_type clk_type,
   uint32_t *value)
@@ -1996,6 +2023,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
.get_allowed_feature_mask = navi10_get_allowed_feature_mask,
.set_default_dpm_table = navi10_set_default_dpm_table,
.dpm_set_uvd_enable = navi10_dpm_set_uvd_enable,
+   .dpm_set_jpeg_enable = navi10_dpm_set_jpeg_enable,
.get_current_clk_freq_by_table = navi10_get_current_clk_freq_by_table,
.print_clk_levels = navi10_print_clk_levels,
.force_clk_levels = navi10_force_clk_levels,
-- 
2.17.1

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

[PATCH 6/7] drm/amd/powerplay: remove set but not used variable 'state'

[yu kuai]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c: In
function ‘fiji_populate_memory_timing_parameters’:
drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c:1504:8:
warning: variable ‘state’ set but not used [-Wunused-but-set-variable]

It is never used, so can be removed.

Fixes: 2e112b4ae3ba ("drm/amd/pp: remove fiji_smc/smumgr split.")
Signed-off-by: yu kuai 
---
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
index c3df3a3..32ebb38 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
@@ -1499,7 +1499,7 @@ static int fiji_populate_memory_timing_parameters(struct 
pp_hwmgr *hwmgr,
uint32_t dram_timing;
uint32_t dram_timing2;
uint32_t burstTime;
-   ULONG state, trrds, trrdl;
+   ULONG trrds, trrdl;
int result;
 
result = atomctrl_set_engine_dram_timings_rv770(hwmgr,
@@ -1511,7 +1511,6 @@ static int fiji_populate_memory_timing_parameters(struct 
pp_hwmgr *hwmgr,
dram_timing2 = cgs_read_register(hwmgr->device, mmMC_ARB_DRAM_TIMING2);
burstTime = cgs_read_register(hwmgr->device, mmMC_ARB_BURST_TIME);
 
-   state = PHM_GET_FIELD(burstTime, MC_ARB_BURST_TIME, STATE0);
trrds = PHM_GET_FIELD(burstTime, MC_ARB_BURST_TIME, TRRDS0);
trrdl = PHM_GET_FIELD(burstTime, MC_ARB_BURST_TIME, TRRDL0);
 
-- 
2.7.4

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

[PATCH 2/7] drm/amdgpu: remove set but not used variable 'amdgpu_connector'

[yu kuai]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/amdgpu/amdgpu_display.c: In function
‘amdgpu_display_crtc_scaling_mode_fixup’:
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:693:27: warning: variable
‘amdgpu_connector’ set but not used [-Wunused-but-set-variable]

Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
Signed-off-by: yu kuai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index d2dd59a..6a27027 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -690,7 +690,6 @@ bool amdgpu_display_crtc_scaling_mode_fixup(struct drm_crtc 
*crtc,
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
struct amdgpu_encoder *amdgpu_encoder;
struct drm_connector *connector;
-   struct amdgpu_connector *amdgpu_connector;
u32 src_v = 1, dst_v = 1;
u32 src_h = 1, dst_h = 1;
 
@@ -702,7 +701,6 @@ bool amdgpu_display_crtc_scaling_mode_fixup(struct drm_crtc 
*crtc,
continue;
amdgpu_encoder = to_amdgpu_encoder(encoder);
connector = amdgpu_get_connector_for_encoder(encoder);
-   amdgpu_connector = to_amdgpu_connector(connector);
 
/* set scaling */
if (amdgpu_encoder->rmx_type == RMX_OFF)
-- 
2.7.4

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

Re: [PATCH v3 04/14] mm/hmm: define the pre-processor related parts of hmm.h even if disabled

[Christoph Hellwig]

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

[PATCH 4/7] drm/amdgpu: remove set but not used variable 'invalid'

[yu kuai]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c: In function
‘amdgpu_amdkfd_evict_userptr’:
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:1665:6: warning:
variable ‘invalid’ set but not used [-Wunused-but-set-variable]

'invalid' is never used, so can be removed. Thus 'atomic_inc_return'
can be replaced as 'atomic_inc'

Fixes: 5ae0283e831a ("drm/amdgpu: Add userptr support for KFD")
Signed-off-by: yu kuai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index ae6f544..a1ed8a8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1662,10 +1662,10 @@ int amdgpu_amdkfd_evict_userptr(struct kgd_mem *mem,
struct mm_struct *mm)
 {
struct amdkfd_process_info *process_info = mem->process_info;
-   int invalid, evicted_bos;
+   int evicted_bos;
int r = 0;
 
-   invalid = atomic_inc_return(>invalid);
+   atomic_inc(>invalid);
evicted_bos = atomic_inc_return(_info->evicted_bos);
if (evicted_bos == 1) {
/* First eviction, stop the queues */
-- 
2.7.4

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

[PATCH 7/7] drm/amd/powerplay: remove set but not used variable 'us_mvdd'

[yu kuai]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c: In
function ‘vegam_populate_smc_acpi_level’:
drivers/gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c:1117:11:
warning: variable 'us_mvdd' set but not used [-Wunused-but-set-variable]

It is never used, so can be removed.

Fixes: ac7822b0026f ("drm/amd/powerplay: add smumgr support for VEGAM (v2)")
Signed-off-by: yu kuai 
---
 drivers/gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c
index ae18fbc..2068eb0 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c
@@ -1114,7 +1114,6 @@ static int vegam_populate_smc_acpi_level(struct pp_hwmgr 
*hwmgr,
(struct phm_ppt_v1_information *)(hwmgr->pptable);
SMIO_Pattern vol_level;
uint32_t mvdd;
-   uint16_t us_mvdd;
 
table->ACPILevel.Flags &= ~PPSMC_SWSTATE_FLAG_DC;
 
@@ -1168,17 +1167,6 @@ static int vegam_populate_smc_acpi_level(struct pp_hwmgr 
*hwmgr,
"in Clock Dependency Table",
);
 
-   us_mvdd = 0;
-   if ((SMU7_VOLTAGE_CONTROL_NONE == data->mvdd_control) ||
-   (data->mclk_dpm_key_disabled))
-   us_mvdd = data->vbios_boot_state.mvdd_bootup_value;
-   else {
-   if (!vegam_populate_mvdd_value(hwmgr,
-   data->dpm_table.mclk_table.dpm_levels[0].value,
-   _level))
-   us_mvdd = vol_level.Voltage;
-   }
-
if (!vegam_populate_mvdd_value(hwmgr, 0, _level))
table->MemoryACPILevel.MinMvdd = 
PP_HOST_TO_SMC_UL(vol_level.Voltage);
else
-- 
2.7.4

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

Re: [PATCH v3 01/14] mm/mmu_notifier: define the header pre-processor parts even if disabled

[Christoph Hellwig]

Looks good,

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

Re: [PATCH 4/5] power: avs: smartreflex: Remove superfluous cast in debugfs_create_file() call

[Rafael J. Wysocki]

On Monday, October 21, 2019 4:51:48 PM CET Geert Uytterhoeven wrote:
> There is no need to cast a typed pointer to a void pointer when calling
> a function that accepts the latter.  Remove it, as the cast prevents
> further compiler checks.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/power/avs/smartreflex.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> index 4684e7df833a81e9..5376f3d22f31eade 100644
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -905,7 +905,7 @@ static int omap_sr_probe(struct platform_device *pdev)
>   sr_info->dbg_dir = debugfs_create_dir(sr_info->name, sr_dbg_dir);
>  
>   debugfs_create_file("autocomp", S_IRUGO | S_IWUSR, sr_info->dbg_dir,
> - (void *)sr_info, _sr_fops);
> + sr_info, _sr_fops);
>   debugfs_create_x32("errweight", S_IRUGO, sr_info->dbg_dir,
>  _info->err_weight);
>   debugfs_create_x32("errmaxlimit", S_IRUGO, sr_info->dbg_dir,
> 

Applying as 5.5 material, thanks!




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

Re: [PATCH v3 13/14] mm/hmm: remove hmm_mirror and related

[Christoph Hellwig]

Looks good:

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

Re: [PATCH] drm/amdgpu: don't read registers if gfxoff is enabled (v2)

[Deucher, Alexander]

I can send a patch for that.  Thanks!

Alex

From: Quan, Evan 
Sent: Tuesday, November 12, 2019 9:09 PM
To: Alex Deucher ; amd-gfx@lists.freedesktop.org 

Cc: Deucher, Alexander 
Subject: RE: [PATCH] drm/amdgpu: don't read registers if gfxoff is enabled (v2)

This patch is reviewed-by: Evan Quan 
However, i just find we need a separate patch to clear PP_GFXOFF_MASK support 
from Arcturus.
Can you do that or you want me to do that?

> -Original Message-
> From: amd-gfx  On Behalf Of Alex
> Deucher
> Sent: Tuesday, November 12, 2019 11:13 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander 
> Subject: [PATCH] drm/amdgpu: don't read registers if gfxoff is enabled (v2)
>
> When gfxoff is enabled, accessing gfx registers via MMIO
> can lead to a hang.
>
> v2: return cached registers properly.
>
> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=205497
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/amdgpu/nv.c| 27 --
>  drivers/gpu/drm/amd/amdgpu/soc15.c | 31 ++
>  2 files changed, 36 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
> b/drivers/gpu/drm/amd/amdgpu/nv.c
> index af68f9815f28..7283d6198b89 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -201,17 +201,25 @@ static uint32_t nv_read_indexed_register(struct
> amdgpu_device *adev, u32 se_num,
>return val;
>  }
>
> -static uint32_t nv_get_register_value(struct amdgpu_device *adev,
> +static int nv_get_register_value(struct amdgpu_device *adev,
>  bool indexed, u32 se_num,
> -   u32 sh_num, u32 reg_offset)
> +   u32 sh_num, u32 reg_offset,
> +   u32 *value)
>  {
>if (indexed) {
> - return nv_read_indexed_register(adev, se_num, sh_num,
> reg_offset);
> + if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> + return -EINVAL;
> + *value = nv_read_indexed_register(adev, se_num, sh_num,
> reg_offset);
>} else {
> - if (reg_offset == SOC15_REG_OFFSET(GC, 0,
> mmGB_ADDR_CONFIG))
> - return adev->gfx.config.gb_addr_config;
> - return RREG32(reg_offset);
> + if (reg_offset == SOC15_REG_OFFSET(GC, 0,
> mmGB_ADDR_CONFIG)) {
> + *value = adev->gfx.config.gb_addr_config;
> + } else {
> + if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> + return -EINVAL;
> + *value = RREG32(reg_offset);
> + }
>}
> + return 0;
>  }
>
>  static int nv_read_register(struct amdgpu_device *adev, u32 se_num,
> @@ -227,10 +235,9 @@ static int nv_read_register(struct amdgpu_device
> *adev, u32 se_num,
>(adev->reg_offset[en->hwip][en->inst][en->seg] + en-
> >reg_offset))
>continue;
>
> - *value = nv_get_register_value(adev,
> -
> nv_allowed_read_registers[i].grbm_indexed,
> -se_num, sh_num, reg_offset);
> - return 0;
> + return nv_get_register_value(adev,
> +
> nv_allowed_read_registers[i].grbm_indexed,
> +  se_num, sh_num, reg_offset, value);
>}
>return -EINVAL;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c
> b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 305ad3eec987..2cc16e9f39fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -363,19 +363,27 @@ static uint32_t soc15_read_indexed_register(struct
> amdgpu_device *adev, u32 se_n
>return val;
>  }
>
> -static uint32_t soc15_get_register_value(struct amdgpu_device *adev,
> +static int soc15_get_register_value(struct amdgpu_device *adev,
> bool indexed, u32 se_num,
> -  u32 sh_num, u32 reg_offset)
> +  u32 sh_num, u32 reg_offset,
> +  u32 *value)
>  {
>if (indexed) {
> - return soc15_read_indexed_register(adev, se_num, sh_num,
> reg_offset);
> + if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> + return -EINVAL;
> + *value = soc15_read_indexed_register(adev, se_num, sh_num,
> reg_offset);
>} else {
> - if (reg_offset == SOC15_REG_OFFSET(GC, 0,
> mmGB_ADDR_CONFIG))
> - return adev->gfx.config.gb_addr_config;
> - else if (reg_offset == SOC15_REG_OFFSET(GC, 0,
> mmDB_DEBUG2))
> - return adev->gfx.config.db_debug2;
> - return RREG32(reg_offset);
> + if (reg_offset == SOC15_REG_OFFSET(GC, 0,
> mmGB_ADDR_CONFIG)) {
> + *value = 

Re: [PATCH 12/21] drm/amd/powerplay: add JPEG power control for Navi1x

[Leo Liu]

On 2019-11-12 10:50 p.m., Quan, Evan wrote:

For Navi10, SMU_MSG_PowerUpJpeg message does not need an argument.


The fixes for this and others will be in v2. Thanks for pointing this out.

Regards,

Leo





-Original Message-
From: amd-gfx  On Behalf Of Leo Liu
Sent: Wednesday, November 13, 2019 2:03 AM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Leo 
Subject: [PATCH 12/21] drm/amd/powerplay: add JPEG power control for
Navi1x

By separating the JPEG power feature, and using its own PowerUp and
PowerDown messages

Signed-off-by: Leo Liu 
---
  drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 32 --
  1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index aeb9c1e341c7..760568debe6c 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -384,8 +384,10 @@ navi10_get_allowed_feature_mask(struct
smu_context *smu,
*(uint64_t *)feature_mask |=
FEATURE_MASK(FEATURE_ATHUB_PG_BIT);

if (smu->adev->pg_flags & AMD_PG_SUPPORT_VCN)
-   *(uint64_t *)feature_mask |=
FEATURE_MASK(FEATURE_VCN_PG_BIT)
-   | FEATURE_MASK(FEATURE_JPEG_PG_BIT);
+   *(uint64_t *)feature_mask |=
FEATURE_MASK(FEATURE_VCN_PG_BIT);
+
+   if (smu->adev->pg_flags & AMD_PG_SUPPORT_JPEG)
+   *(uint64_t *)feature_mask |=
FEATURE_MASK(FEATURE_JPEG_PG_BIT);

/* disable DPM UCLK and DS SOCCLK on navi10 A0 secure board */
if (is_asic_secure(smu)) {
@@ -665,6 +667,31 @@ static int navi10_dpm_set_uvd_enable(struct
smu_context *smu, bool enable)
return ret;
  }

+static int navi10_dpm_set_jpeg_enable(struct smu_context *smu, bool
+enable) {
+   struct smu_power_context *smu_power = >smu_power;
+   struct smu_power_gate *power_gate = _power->power_gate;
+   int ret = 0;
+
+   if (enable) {
+   if (smu_feature_is_enabled(smu,
SMU_FEATURE_JPEG_PG_BIT)) {
+   ret = smu_send_smc_msg_with_param(smu,
SMU_MSG_PowerUpJpeg, 1);
+   if (ret)
+   return ret;
+   }
+   power_gate->jpeg_gated = false;
+   } else {
+   if (smu_feature_is_enabled(smu,
SMU_FEATURE_JPEG_PG_BIT)) {
+   ret = smu_send_smc_msg(smu,
SMU_MSG_PowerDownJpeg);
+   if (ret)
+   return ret;
+   }
+   power_gate->jpeg_gated = true;
+   }
+
+   return ret;
+}
+
  static int navi10_get_current_clk_freq_by_table(struct smu_context *smu,
   enum smu_clk_type clk_type,
   uint32_t *value)
@@ -1996,6 +2023,7 @@ static const struct pptable_funcs navi10_ppt_funcs =
{
.get_allowed_feature_mask = navi10_get_allowed_feature_mask,
.set_default_dpm_table = navi10_set_default_dpm_table,
.dpm_set_uvd_enable = navi10_dpm_set_uvd_enable,
+   .dpm_set_jpeg_enable = navi10_dpm_set_jpeg_enable,
.get_current_clk_freq_by_table =
navi10_get_current_clk_freq_by_table,
.print_clk_levels = navi10_print_clk_levels,
.force_clk_levels = navi10_force_clk_levels,
--
2.17.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 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

[Russell, Kent]

Should we use "CI" instead of "CIK" in the comments? I thought CIK was a short 
form for CI kickers, while CI encompasses all CI ASICs. Even though we had _cik 
as the suffix for most of the CI stuff. Just wondering about accuracy.

 Kent

-Original Message-
From: amd-gfx  On Behalf Of Yong Zhao
Sent: Tuesday, November 12, 2019 5:19 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhao, Yong 
Subject: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

The ops_asic_specific function pointers are actually quite generic after using 
a simple if condition. Eliminate it by code refactoring.

Change-Id: Icb891289cca31acdbe2d2eea76a426f1738b9c08
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 63 ---  
drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h |  4 --  
.../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c  | 36 ---  
.../gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c  | 48 --
 4 files changed, 26 insertions(+), 125 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index a750b1d110eb..59ee9053498c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -87,9 +87,17 @@ static bool initialize(struct kernel_queue *kq, struct 
kfd_dev *dev,
kq->pq_kernel_addr = kq->pq->cpu_ptr;
kq->pq_gpu_addr = kq->pq->gpu_addr;
 
-   retval = kq->ops_asic_specific.initialize(kq, dev, type, queue_size);
-   if (!retval)
-   goto err_eop_allocate_vidmem;
+   /* For CIK family asics, kq->eop_mem is not needed */
+   if (dev->device_info->asic_family > CHIP_HAWAII) {
+   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
+   if (retval != 0)
+   goto err_eop_allocate_vidmem;
+
+   kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
+   kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
+
+   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
+   }
 
retval = kfd_gtt_sa_allocate(dev, sizeof(*kq->rptr_kernel),
>rptr_mem);
@@ -200,7 +208,12 @@ static void uninitialize(struct kernel_queue *kq)
 
kfd_gtt_sa_free(kq->dev, kq->rptr_mem);
kfd_gtt_sa_free(kq->dev, kq->wptr_mem);
-   kq->ops_asic_specific.uninitialize(kq);
+
+   /* For CIK family asics, kq->eop_mem is Null, kfd_gtt_sa_free()
+* is able to handle NULL properly.
+*/
+   kfd_gtt_sa_free(kq->dev, kq->eop_mem);
+
kfd_gtt_sa_free(kq->dev, kq->pq);
kfd_release_kernel_doorbell(kq->dev,
kq->queue->properties.doorbell_ptr);
@@ -280,8 +293,15 @@ static void submit_packet(struct kernel_queue *kq)
}
pr_debug("\n");
 #endif
-
-   kq->ops_asic_specific.submit_packet(kq);
+   if (kq->dev->device_info->doorbell_size == 8) {
+   *kq->wptr64_kernel = kq->pending_wptr64;
+   write_kernel_doorbell64(kq->queue->properties.doorbell_ptr,
+   kq->pending_wptr64);
+   } else {
+   *kq->wptr_kernel = kq->pending_wptr;
+   write_kernel_doorbell(kq->queue->properties.doorbell_ptr,
+   kq->pending_wptr);
+   }
 }
 
 static void rollback_packet(struct kernel_queue *kq) @@ -310,42 +330,11 @@ 
struct kernel_queue *kernel_queue_init(struct kfd_dev *dev,
kq->ops.submit_packet = submit_packet;
kq->ops.rollback_packet = rollback_packet;
 
-   switch (dev->device_info->asic_family) {
-   case CHIP_KAVERI:
-   case CHIP_HAWAII:
-   case CHIP_CARRIZO:
-   case CHIP_TONGA:
-   case CHIP_FIJI:
-   case CHIP_POLARIS10:
-   case CHIP_POLARIS11:
-   case CHIP_POLARIS12:
-   case CHIP_VEGAM:
-   kernel_queue_init_vi(>ops_asic_specific);
-   break;
-
-   case CHIP_VEGA10:
-   case CHIP_VEGA12:
-   case CHIP_VEGA20:
-   case CHIP_RAVEN:
-   case CHIP_RENOIR:
-   case CHIP_ARCTURUS:
-   case CHIP_NAVI10:
-   case CHIP_NAVI12:
-   case CHIP_NAVI14:
-   kernel_queue_init_v9(>ops_asic_specific);
-   break;
-   default:
-   WARN(1, "Unexpected ASIC family %u",
-dev->device_info->asic_family);
-   goto out_free;
-   }
-
if (kq->ops.initialize(kq, dev, type, KFD_KERNEL_QUEUE_SIZE))
return kq;
 
pr_err("Failed to init kernel queue\n");
 
-out_free:
kfree(kq);
return NULL;
 }
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h
index a9a35897d8b7..475e9499c0af 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h
@@ -66,7 +66,6 @@ struct kernel_queue_ops {
 
 struct kernel_queue {