[PATCH] drm/amdgpu: add mutex to protect ras shared memory

2024-04-28 Thread YiPeng Chai
Add mutex to protect ras shared memory.

Signed-off-by: YiPeng Chai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 121 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h|   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c |   2 +
 3 files changed, 84 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 5583e2d1b12f..fa4fea00f6b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -1564,6 +1564,66 @@ static void psp_ras_ta_check_status(struct psp_context 
*psp)
}
 }
 
+static int psp_ras_send_cmd(struct psp_context *psp,
+   enum ras_command cmd_id, void *in, void *out)
+{
+   struct ta_ras_shared_memory *ras_cmd;
+   uint32_t cmd = cmd_id;
+   int ret = 0;
+
+   mutex_lock(&psp->ras_context.mutex);
+   ras_cmd = (struct ta_ras_shared_memory 
*)psp->ras_context.context.mem_context.shared_buf;
+   memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory));
+
+   switch (cmd) {
+   case TA_RAS_COMMAND__ENABLE_FEATURES:
+   case TA_RAS_COMMAND__DISABLE_FEATURES:
+   memcpy(&ras_cmd->ras_in_message,
+   in, sizeof(ras_cmd->ras_in_message));
+   break;
+   case TA_RAS_COMMAND__TRIGGER_ERROR:
+   memcpy(&ras_cmd->ras_in_message.trigger_error,
+   in, sizeof(ras_cmd->ras_in_message.trigger_error));
+   break;
+   case TA_RAS_COMMAND__QUERY_ADDRESS:
+   memcpy(&ras_cmd->ras_in_message.address,
+   in, sizeof(ras_cmd->ras_in_message.address));
+   break;
+   default:
+   dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd);
+   ret = -EINVAL;
+   goto err_out;
+   }
+
+   ras_cmd->cmd_id = cmd;
+   ret = psp_ras_invoke(psp, ras_cmd->cmd_id);
+
+   switch (cmd) {
+   case TA_RAS_COMMAND__TRIGGER_ERROR:
+   if (out) {
+   uint32_t *ras_status = (uint32_t *)out;
+
+   *ras_status = ras_cmd->ras_status;
+   }
+   break;
+   case TA_RAS_COMMAND__QUERY_ADDRESS:
+   if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status)
+   ret = -EINVAL;
+   else if (out)
+   memcpy(out,
+   &ras_cmd->ras_out_message.address,
+   sizeof(ras_cmd->ras_out_message.address));
+   break;
+   default:
+   break;
+   }
+
+err_out:
+   mutex_unlock(&psp->ras_context.mutex);
+
+   return ret;
+}
+
 int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id)
 {
struct ta_ras_shared_memory *ras_cmd;
@@ -1605,23 +1665,15 @@ int psp_ras_invoke(struct psp_context *psp, uint32_t 
ta_cmd_id)
 int psp_ras_enable_features(struct psp_context *psp,
union ta_ras_cmd_input *info, bool enable)
 {
-   struct ta_ras_shared_memory *ras_cmd;
+   enum ras_command cmd_id;
int ret;
 
-   if (!psp->ras_context.context.initialized)
+   if (!psp->ras_context.context.initialized || !info)
return -EINVAL;
 
-   ras_cmd = (struct ta_ras_shared_memory 
*)psp->ras_context.context.mem_context.shared_buf;
-   memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory));
-
-   if (enable)
-   ras_cmd->cmd_id = TA_RAS_COMMAND__ENABLE_FEATURES;
-   else
-   ras_cmd->cmd_id = TA_RAS_COMMAND__DISABLE_FEATURES;
-
-   ras_cmd->ras_in_message = *info;
-
-   ret = psp_ras_invoke(psp, ras_cmd->cmd_id);
+   cmd_id = enable ?
+   TA_RAS_COMMAND__ENABLE_FEATURES : 
TA_RAS_COMMAND__DISABLE_FEATURES;
+   ret = psp_ras_send_cmd(psp, cmd_id, info, NULL);
if (ret)
return -EINVAL;
 
@@ -1645,6 +1697,8 @@ int psp_ras_terminate(struct psp_context *psp)
 
psp->ras_context.context.initialized = false;
 
+   mutex_destroy(&psp->ras_context.mutex);
+
return ret;
 }
 
@@ -1729,9 +1783,10 @@ int psp_ras_initialize(struct psp_context *psp)
 
ret = psp_ta_load(psp, &psp->ras_context.context);
 
-   if (!ret && !ras_cmd->ras_status)
+   if (!ret && !ras_cmd->ras_status) {
psp->ras_context.context.initialized = true;
-   else {
+   mutex_init(&psp->ras_context.mutex);
+   } else {
if (ras_cmd->ras_status)
dev_warn(adev->dev, "RAS Init Status: 0x%X\n", 
ras_cmd->ras_status);
 
@@ -1745,12 +1800,12 @@ int psp_ras_initialize(struct psp_context *psp)
 int psp_ras_trigger_error(struct psp_context *psp,
  struct ta_ras_trigger_error_input *info, uint32_t 
instance_mask)
 {
-   struct ta_ras_shared_memory *ras_cmd;
struct amdgpu_device *adev = psp->adev;
int ret;
  

RE: [PATCH v2 3/4] drm/amdgpu: Fix amdgpu_device_reset_sriov retry logic

2024-04-28 Thread Deng, Emily
[AMD Official Use Only - General]

Reviewed-by: Emily Deng 

Emily Deng
Best Wishes



>-Original Message-
>From: Li, Yunxiang (Teddy) 
>Sent: Saturday, April 27, 2024 2:29 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander ; Koenig, Christian
>; Lazar, Lijo ; Kuehling,
>Felix ; Deng, Emily ; Li,
>Yunxiang (Teddy) 
>Subject: [PATCH v2 3/4] drm/amdgpu: Fix amdgpu_device_reset_sriov retry
>logic
>
>The retry loop for SRIOV reset have refcount and memory leak issue.
>Depending on which function call fails it can potentially call
>amdgpu_amdkfd_pre/post_reset different number of times and causes
>kfd_locked count to be wrong. This will block all future attempts at opening
>/dev/kfd. The retry loop also leakes resources by calling
>amdgpu_virt_init_data_exchange multiple times without calling the
>corresponding fini function.
>
>Align with the bare-metal reset path which doesn't have these issues.
>This means taking the amdgpu_amdkfd_pre/post_reset functions out of the
>reset loop and calling amdgpu_device_pre_asic_reset each retry which
>properly free the resources from previous try by calling
>amdgpu_virt_fini_data_exchange.
>
>Signed-off-by: Yunxiang Li 
>---
>v2: put back release full access and the missed return
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 47 ++
> 1 file changed, 22 insertions(+), 25 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>index 33c889c027a5..b23645f23a2e 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>@@ -5065,10 +5065,6 @@ static int amdgpu_device_reset_sriov(struct
>amdgpu_device *adev,  {
>   int r;
>   struct amdgpu_hive_info *hive = NULL;
>-  int retry_limit = 0;
>-
>-retry:
>-  amdgpu_amdkfd_pre_reset(adev);
>
>   if (test_bit(AMDGPU_HOST_FLR, &reset_context->flags)) {
>   clear_bit(AMDGPU_HOST_FLR, &reset_context->flags); @@ -
>5088,7 +5084,7 @@ static int amdgpu_device_reset_sriov(struct
>amdgpu_device *adev,
>   /* Resume IP prior to SMC */
>   r = amdgpu_device_ip_reinit_early_sriov(adev);
>   if (r)
>-  goto error;
>+  return r;
>
>   amdgpu_virt_init_data_exchange(adev);
>
>@@ -5099,38 +5095,35 @@ static int amdgpu_device_reset_sriov(struct
>amdgpu_device *adev,
>   /* now we are okay to resume SMC/CP/SDMA */
>   r = amdgpu_device_ip_reinit_late_sriov(adev);
>   if (r)
>-  goto error;
>+  return r;
>
>   hive = amdgpu_get_xgmi_hive(adev);
>   /* Update PSP FW topology after reset */
>   if (hive && adev->gmc.xgmi.num_physical_nodes > 1)
>   r = amdgpu_xgmi_update_topology(hive, adev);
>-
>   if (hive)
>   amdgpu_put_xgmi_hive(hive);
>+  if (r)
>+  return r;
>
>-  if (!r) {
>-  r = amdgpu_ib_ring_tests(adev);
>-
>-  amdgpu_amdkfd_post_reset(adev);
>-  }
>+  r = amdgpu_ib_ring_tests(adev);
>+  if (r)
>+  return r;
>
>-error:
>-  if (!r && adev->virt.gim_feature &
>AMDGIM_FEATURE_GIM_FLR_VRAMLOST) {
>+  if (adev->virt.gim_feature &
>AMDGIM_FEATURE_GIM_FLR_VRAMLOST) {
>   amdgpu_inc_vram_lost(adev);
>   r = amdgpu_device_recover_vram(adev);
>   }
>-  amdgpu_virt_release_full_gpu(adev, true);
>+  if (r)
>+  return r;
>
>-  if (AMDGPU_RETRY_SRIOV_RESET(r)) {
>-  if (retry_limit < AMDGPU_MAX_RETRY_LIMIT) {
>-  retry_limit++;
>-  goto retry;
>-  } else
>-  DRM_ERROR("GPU reset retry is beyond the retry
>limit\n");
>-  }
>+  /* need to be called during full access so we can't do it later like
>+   * bare-metal does.
>+   */
>+  amdgpu_amdkfd_post_reset(adev);
>+  amdgpu_virt_release_full_gpu(adev, true);
>
>-  return r;
>+  return 0;
> }
>
> /**
>@@ -5689,6 +5682,7 @@ int amdgpu_device_gpu_recover(struct
>amdgpu_device *adev,
>   int i, r = 0;
>   bool need_emergency_restart = false;
>   bool audio_suspended = false;
>+  int retry_limit = AMDGPU_MAX_RETRY_LIMIT;
>
>   /*
>* Special case: RAS triggered and full reset isn't supported @@ -
>5770,8 +5764,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
>*adev,
>
>   cancel_delayed_work_sync(&tmp_adev->delayed_init_work);
>
>-  if (!amdgpu_sriov_vf(tmp_adev))
>-  amdgpu_amdkfd_pre_reset(tmp_adev);
>+  amdgpu_amdkfd_pre_reset(tmp_adev);
>
>   /*
>* Mark these ASICs to be reseted as untracked first @@ -
>5830,6 +5823,10 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
>*adev,
>   /* Host driver will handle XGMI hive reset for SRIOV */
>   if (amdgpu_sriov_vf(adev)) {
>   r = amdgpu_device_reset_sriov(adev, reset_context);
>+  

RE: [PATCH v4 2/4] drm/amdgpu: Add reset_context flag for host FLR

2024-04-28 Thread Deng, Emily
[AMD Official Use Only - General]

Reviewed-by: Emily Deng 

Emily Deng
Best Wishes



>-Original Message-
>From: Li, Yunxiang (Teddy) 
>Sent: Saturday, April 27, 2024 2:27 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander ; Koenig, Christian
>; Lazar, Lijo ; Kuehling,
>Felix ; Deng, Emily ; Li,
>Yunxiang (Teddy) 
>Subject: [PATCH v4 2/4] drm/amdgpu: Add reset_context flag for host FLR
>
>There are other reset sources that pass NULL as the job pointer, such as
>amdgpu_amdkfd_reset_work. Therefore, using the job pointer to check if the
>FLR comes from the host does not work.
>
>Add a flag in reset_context to explicitly mark host triggered reset, and set
>this flag when we receive host reset notification.
>
>Signed-off-by: Yunxiang Li 
>---
>v2: fix typo
>v3: pass reset_context directly
>v4: clear the flag in case we retry
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 13 -
>drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h  |  1 +
> drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c  |  1 +
> drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c  |  1 +
> drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c  |  1 +
> 5 files changed, 12 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>index 8befd10bf007..33c889c027a5 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>@@ -5055,13 +5055,13 @@ static int amdgpu_device_recover_vram(struct
>amdgpu_device *adev)
>  * amdgpu_device_reset_sriov - reset ASIC for SR-IOV vf
>  *
>  * @adev: amdgpu_device pointer
>- * @from_hypervisor: request from hypervisor
>+ * @reset_context: amdgpu reset context pointer
>  *
>  * do VF FLR and reinitialize Asic
>  * return 0 means succeeded otherwise failed
>  */
> static int amdgpu_device_reset_sriov(struct amdgpu_device *adev,
>-   bool from_hypervisor)
>+   struct amdgpu_reset_context
>*reset_context)
> {
>   int r;
>   struct amdgpu_hive_info *hive = NULL;
>@@ -5070,12 +5070,15 @@ static int amdgpu_device_reset_sriov(struct
>amdgpu_device *adev,
> retry:
>   amdgpu_amdkfd_pre_reset(adev);
>
>-  if (from_hypervisor)
>+  if (test_bit(AMDGPU_HOST_FLR, &reset_context->flags)) {
>+  clear_bit(AMDGPU_HOST_FLR, &reset_context->flags);
>   r = amdgpu_virt_request_full_gpu(adev, true);
>-  else
>+  } else {
>   r = amdgpu_virt_reset_gpu(adev);
>+  }
>   if (r)
>   return r;
>+
>   amdgpu_ras_set_fed(adev, false);
>   amdgpu_irq_gpu_reset_resume_helper(adev);
>
>@@ -5826,7 +5829,7 @@ int amdgpu_device_gpu_recover(struct
>amdgpu_device *adev,
>   /* Actual ASIC resets if needed.*/
>   /* Host driver will handle XGMI hive reset for SRIOV */
>   if (amdgpu_sriov_vf(adev)) {
>-  r = amdgpu_device_reset_sriov(adev, job ? false : true);
>+  r = amdgpu_device_reset_sriov(adev, reset_context);
>   if (r)
>   adev->asic_reset_res = r;
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>index b11d190ece53..5a9cc043b858 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>@@ -33,6 +33,7 @@ enum AMDGPU_RESET_FLAGS {
>   AMDGPU_NEED_FULL_RESET = 0,
>   AMDGPU_SKIP_HW_RESET = 1,
>   AMDGPU_SKIP_COREDUMP = 2,
>+  AMDGPU_HOST_FLR = 3,
> };
>
> struct amdgpu_reset_context {
>diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>index c5ba9c4757a8..f4c47492e0cd 100644
>--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
>@@ -292,6 +292,7 @@ static void xgpu_ai_mailbox_flr_work(struct
>work_struct *work)
>   reset_context.method = AMD_RESET_METHOD_NONE;
>   reset_context.reset_req_dev = adev;
>   clear_bit(AMDGPU_NEED_FULL_RESET, &reset_context.flags);
>+  set_bit(AMDGPU_HOST_FLR, &reset_context.flags);
>
>   amdgpu_device_gpu_recover(adev, NULL, &reset_context);
>   }
>diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
>b/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
>index fa9d1b02f391..14cc7910e5cf 100644
>--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
>+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_nv.c
>@@ -328,6 +328,7 @@ static void xgpu_nv_mailbox_flr_work(struct
>work_struct *work)
>   reset_context.method = AMD_RESET_METHOD_NONE;
>   reset_context.reset_req_dev = adev;
>   clear_bit(AMDGPU_NEED_FULL_RESET, &reset_context.flags);
>+  set_bit(AMDGPU_HOST_FLR, &reset_context.flags);
>
>   amdgpu_device_gpu_recover(adev, NULL, &reset_context);
>   }
>diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
>index 14a065516ae4..78cd07744ebe 100644
>--- a/driver

RE: [PATCH 4/4] drm/amdgpu: Move ras resume into SRIOV function

2024-04-28 Thread Deng, Emily
[AMD Official Use Only - General]

Reviewed-by: Emily Deng 

Emily Deng
Best Wishes



>-Original Message-
>From: Li, Yunxiang (Teddy) 
>Sent: Friday, April 26, 2024 11:58 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander ; Koenig, Christian
>; Lazar, Lijo ; Kuehling,
>Felix ; Deng, Emily ; Li,
>Yunxiang (Teddy) 
>Subject: [PATCH 4/4] drm/amdgpu: Move ras resume into SRIOV function
>
>This is part of the reset, move it into the reset function.
>
>Signed-off-by: Yunxiang Li 
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +---
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>index 3c4755f3c116..8f2c1f71ed9a 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>@@ -5119,6 +5119,11 @@ static int amdgpu_device_reset_sriov(struct
>amdgpu_device *adev,
>   amdgpu_amdkfd_post_reset(adev);
>   amdgpu_virt_release_full_gpu(adev, true);
>
>+  /* Aldebaran and gfx_11_0_3 support ras in SRIOV, so need resume
>ras during reset */
>+  if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 2) ||
>+  amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 3) ||
>+  amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(11, 0, 3))
>+  amdgpu_ras_resume(adev);
>   return 0;
> }
>
>@@ -5823,13 +5828,6 @@ int amdgpu_device_gpu_recover(struct
>amdgpu_device *adev,
>   goto retry;
>   if (r)
>   adev->asic_reset_res = r;
>-
>-  /* Aldebaran and gfx_11_0_3 support ras in SRIOV, so need
>resume ras during reset */
>-  if (amdgpu_ip_version(adev, GC_HWIP, 0) ==
>-  IP_VERSION(9, 4, 2) ||
>-  amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4,
>3) ||
>-  amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(11,
>0, 3))
>-  amdgpu_ras_resume(adev);
>   } else {
>   r = amdgpu_do_asic_reset(device_list_handle,
>reset_context);
>   if (r && r == -EAGAIN)
>--
>2.34.1



RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory

2024-04-28 Thread Wang, Yang(Kevin)
[AMD Official Use Only - General]

-Original Message-
From: Chai, Thomas 
Sent: Sunday, April 28, 2024 3:08 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Zhou1, Tao ; Li, 
Candice ; Wang, Yang(Kevin) ; Yang, 
Stanley ; Chai, Thomas 
Subject: [PATCH] drm/amdgpu: add mutex to protect ras shared memory

Add mutex to protect ras shared memory.

Signed-off-by: YiPeng Chai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 121 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h|   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c |   2 +
 3 files changed, 84 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 5583e2d1b12f..fa4fea00f6b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -1564,6 +1564,66 @@ static void psp_ras_ta_check_status(struct psp_context 
*psp)
}
 }

+static int psp_ras_send_cmd(struct psp_context *psp,
+   enum ras_command cmd_id, void *in, void *out) {
+   struct ta_ras_shared_memory *ras_cmd;
+   uint32_t cmd = cmd_id;
+   int ret = 0;
+
+   mutex_lock(&psp->ras_context.mutex);
+   ras_cmd = (struct ta_ras_shared_memory 
*)psp->ras_context.context.mem_context.shared_buf;
+   memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory));
+
+   switch (cmd) {
+   case TA_RAS_COMMAND__ENABLE_FEATURES:
+   case TA_RAS_COMMAND__DISABLE_FEATURES:
+   memcpy(&ras_cmd->ras_in_message,
+   in, sizeof(ras_cmd->ras_in_message));
+   break;
+   case TA_RAS_COMMAND__TRIGGER_ERROR:
+   memcpy(&ras_cmd->ras_in_message.trigger_error,
+   in, sizeof(ras_cmd->ras_in_message.trigger_error));
+   break;
+   case TA_RAS_COMMAND__QUERY_ADDRESS:
+   memcpy(&ras_cmd->ras_in_message.address,
+   in, sizeof(ras_cmd->ras_in_message.address));
+   break;
+   default:
+   dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd);
+   ret = -EINVAL;
+   goto err_out;
+   }
+
+   ras_cmd->cmd_id = cmd;
+   ret = psp_ras_invoke(psp, ras_cmd->cmd_id);
+
+   switch (cmd) {
+   case TA_RAS_COMMAND__TRIGGER_ERROR:
+   if (out) {
+   uint32_t *ras_status = (uint32_t *)out;
[Kevin]:
It's better to check 'ret' value first before use this 'out' data.

Best Regards,
Kevin
+
+   *ras_status = ras_cmd->ras_status;
+   }
+   break;
+   case TA_RAS_COMMAND__QUERY_ADDRESS:
+   if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status)
+   ret = -EINVAL;
+   else if (out)
+   memcpy(out,
+   &ras_cmd->ras_out_message.address,
+   sizeof(ras_cmd->ras_out_message.address));
+   break;
+   default:
+   break;
+   }
+
+err_out:
+   mutex_unlock(&psp->ras_context.mutex);
+
+   return ret;
+}
+
 int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id)  {
struct ta_ras_shared_memory *ras_cmd;
@@ -1605,23 +1665,15 @@ int psp_ras_invoke(struct psp_context *psp, uint32_t 
ta_cmd_id)  int psp_ras_enable_features(struct psp_context *psp,
union ta_ras_cmd_input *info, bool enable)  {
-   struct ta_ras_shared_memory *ras_cmd;
+   enum ras_command cmd_id;
int ret;

-   if (!psp->ras_context.context.initialized)
+   if (!psp->ras_context.context.initialized || !info)
return -EINVAL;

-   ras_cmd = (struct ta_ras_shared_memory 
*)psp->ras_context.context.mem_context.shared_buf;
-   memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory));
-
-   if (enable)
-   ras_cmd->cmd_id = TA_RAS_COMMAND__ENABLE_FEATURES;
-   else
-   ras_cmd->cmd_id = TA_RAS_COMMAND__DISABLE_FEATURES;
-
-   ras_cmd->ras_in_message = *info;
-
-   ret = psp_ras_invoke(psp, ras_cmd->cmd_id);
+   cmd_id = enable ?
+   TA_RAS_COMMAND__ENABLE_FEATURES : 
TA_RAS_COMMAND__DISABLE_FEATURES;
+   ret = psp_ras_send_cmd(psp, cmd_id, info, NULL);
if (ret)
return -EINVAL;

@@ -1645,6 +1697,8 @@ int psp_ras_terminate(struct psp_context *psp)

psp->ras_context.context.initialized = false;

+   mutex_destroy(&psp->ras_context.mutex);
+
return ret;
 }

@@ -1729,9 +1783,10 @@ int psp_ras_initialize(struct psp_context *psp)

ret = psp_ta_load(psp, &psp->ras_context.context);

-   if (!ret && !ras_cmd->ras_status)
+   if (!ret && !ras_cmd->ras_status) {
psp->ras_context.context.initialized = true;
-   else {
+   mutex_init(&psp->ras_context.mutex);
+   } else {
if (ras_cmd->ras_status)
   

[PATCH 1/2] drm/amd/pm: fix uninitialized variable warnings for vega10_hwmgr

2024-04-28 Thread Tim Huang
Clear warnings that using uninitialized variable when fails
to get the valid value from SMU.

Signed-off-by: Tim Huang 
---
 .../drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 46 ++-
 .../amd/pm/powerplay/smumgr/vega10_smumgr.c   |  6 ++-
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index 9f5bd998c6bf..488ad9de4694 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -354,13 +354,13 @@ static int vega10_odn_initial_default_setting(struct 
pp_hwmgr *hwmgr)
return 0;
 }
 
-static void vega10_init_dpm_defaults(struct pp_hwmgr *hwmgr)
+static int vega10_init_dpm_defaults(struct pp_hwmgr *hwmgr)
 {
struct vega10_hwmgr *data = hwmgr->backend;
-   int i;
uint32_t sub_vendor_id, hw_revision;
uint32_t top32, bottom32;
struct amdgpu_device *adev = hwmgr->adev;
+   int ret, i;
 
vega10_initialize_power_tune_defaults(hwmgr);
 
@@ -485,9 +485,12 @@ static void vega10_init_dpm_defaults(struct pp_hwmgr 
*hwmgr)
if (data->registry_data.vr0hot_enabled)
data->smu_features[GNLD_VR0HOT].supported = true;
 
-   smum_send_msg_to_smc(hwmgr,
+   ret = smum_send_msg_to_smc(hwmgr,
PPSMC_MSG_GetSmuVersion,
&hwmgr->smu_version);
+   if (ret)
+   return ret;
+
/* ACG firmware has major version 5 */
if ((hwmgr->smu_version & 0xff00) == 0x500)
data->smu_features[GNLD_ACG].supported = true;
@@ -505,10 +508,16 @@ static void vega10_init_dpm_defaults(struct pp_hwmgr 
*hwmgr)
data->smu_features[GNLD_PCC_LIMIT].supported = true;
 
/* Get the SN to turn into a Unique ID */
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &top32);
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, &bottom32);
+   ret = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &top32);
+   if (ret)
+   return ret;
+
+   ret = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, 
&bottom32);
+   if (ret)
+   return ret;
 
adev->unique_id = ((uint64_t)bottom32 << 32) | top32;
+   return 0;
 }
 
 #ifdef PPLIB_VEGA10_EVV_SUPPORT
@@ -882,7 +891,9 @@ static int vega10_hwmgr_backend_init(struct pp_hwmgr *hwmgr)
 
vega10_set_features_platform_caps(hwmgr);
 
-   vega10_init_dpm_defaults(hwmgr);
+   result = vega10_init_dpm_defaults(hwmgr);
+   if (result)
+   return result;
 
 #ifdef PPLIB_VEGA10_EVV_SUPPORT
/* Get leakage voltage based on leakage ID. */
@@ -3900,11 +3911,14 @@ static int vega10_get_gpu_power(struct pp_hwmgr *hwmgr,
uint32_t *query)
 {
uint32_t value;
+   int ret;
 
if (!query)
return -EINVAL;
 
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrPkgPwr, &value);
+   ret = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrPkgPwr, &value);
+   if (ret)
+   return ret;
 
/* SMC returning actual watts, keep consistent with legacy asics, low 8 
bit as 8 fractional bits */
*query = value << 8;
@@ -4800,14 +4814,16 @@ static int vega10_print_clock_levels(struct pp_hwmgr 
*hwmgr,
uint32_t gen_speed, lane_width, current_gen_speed, current_lane_width;
PPTable_t *pptable = &(data->smc_state_table.pp_table);
 
-   int i, now, size = 0, count = 0;
+   int i, ret, now,  size = 0, count = 0;
 
switch (type) {
case PP_SCLK:
if (data->registry_data.sclk_dpm_key_disabled)
break;
 
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrentGfxclkIndex, 
&now);
+   ret = smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetCurrentGfxclkIndex, &now);
+   if (ret)
+   break;
 
if (hwmgr->pp_one_vf &&
(hwmgr->dpm_level == AMD_DPM_FORCED_LEVEL_PROFILE_PEAK))
@@ -4823,7 +4839,9 @@ static int vega10_print_clock_levels(struct pp_hwmgr 
*hwmgr,
if (data->registry_data.mclk_dpm_key_disabled)
break;
 
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrentUclkIndex, 
&now);
+   ret = smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetCurrentUclkIndex, &now);
+   if (ret)
+   break;
 
for (i = 0; i < mclk_table->count; i++)
size += sprintf(buf + size, "%d: %uMhz %s\n",
@@ -4834,7 +4852,9 @@ static int vega10_print_clock_levels(struct pp_hwmgr 
*hwmgr,
if (data->registry_data.socclk_dpm_key_disabled)
break;
 
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrentSocclkIndex, 
&now);
+   ret = smum_

[PATCH 2/2] drm/amd/pm: fix uninitialized variable warnings for vangogh_ppt

2024-04-28 Thread Tim Huang
1. Fix a issue that using uninitialized mask to get the ultimate frequency.
2. Check return of smu_cmn_send_smc_msg_with_param to avoid using
uninitialized variable residency.

Signed-off-by: Tim Huang 
---
 drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
index 29295941aca9..b40cb4e255e9 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
@@ -976,6 +976,18 @@ static int vangogh_get_dpm_ultimate_freq(struct 
smu_context *smu,
}
}
if (min) {
+   ret = vangogh_get_profiling_clk_mask(smu,
+
AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK,
+NULL,
+NULL,
+&mclk_mask,
+&fclk_mask,
+&soc_mask);
+   if (ret)
+   goto failed;
+
+   vclk_mask = dclk_mask = 0;
+
switch (clk_type) {
case SMU_UCLK:
case SMU_MCLK:
@@ -2450,6 +2462,8 @@ static u32 vangogh_set_gfxoff_residency(struct 
smu_context *smu, bool start)
 
ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_LogGfxOffResidency,
  start, &residency);
+   if (ret)
+   return ret;
 
if (!start)
adev->gfx.gfx_off_residency = residency;
-- 
2.39.2



RE: [PATCH 1/3] drm/amd/pm: Fix negative array index read warning for pptable->DpmDescriptor

2024-04-28 Thread Huang, Tim
[AMD Official Use Only - General]

> -Original Message-
> From: Jesse Zhang 
> Sent: Friday, April 26, 2024 3:28 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Koenig, Christian
> ; Huang, Tim ; Zhang,
> Jesse(Jie) ; Zhang, Jesse(Jie)
> 
> Subject: [PATCH 1/3] drm/amd/pm: Fix negative array index read warning for
> pptable->DpmDescriptor
>
> Avoid using the negative values
> for clk_idex as an index into an array pptable->DpmDescriptor.
>
> Signed-off-by: Jesse Zhang 
> ---
>  .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 25 +++---
> -
>  1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 5a68d365967f..cd88d2c3841a 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -1219,15 +1219,18 @@ static int
> navi10_get_current_clk_freq_by_table(struct smu_context *smu,
>  value);
>  }
>
> -static bool navi10_is_support_fine_grained_dpm(struct smu_context *smu,
> enum smu_clk_type clk_type)
> +static int navi10_is_support_fine_grained_dpm(struct smu_context *smu,
> +enum smu_clk_type clk_type)
>  {
>   PPTable_t *pptable = smu->smu_table.driver_pptable;
>   DpmDescriptor_t *dpm_desc = NULL;
> - uint32_t clk_index = 0;
> + int clk_index = 0;
>
>   clk_index = smu_cmn_to_asic_specific_index(smu,
>  CMN2ASIC_MAPPING_CLK,
>  clk_type);
> + if(clk_index)
Hi jesse,

If should only be "if(clk_index < 0)" to return an error.

Tim

> + return clk_index;

> +
>   dpm_desc = &pptable->DpmDescriptor[clk_index];
>
>   /* 0 - Fine grained DPM, 1 - Discrete DPM */ @@ -1287,7 +1290,11
> @@ static int navi10_emit_clk_levels(struct smu_context *smu,
>   if (ret)
>   return ret;
>
> - if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
> + ret = navi10_is_support_fine_grained_dpm(smu, clk_type);
> + if (ret < 0)
> + return ret;
> +
> + if (!ret) {
>   for (i = 0; i < count; i++) {
>   ret =
> smu_v11_0_get_dpm_freq_by_index(smu,
> clk_type, 
> i,
> &value);
> @@ -1496,7 +1503,11 @@ static int navi10_print_clk_levels(struct
> smu_context *smu,
>   if (ret)
>   return size;
>
> - if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
> + ret = navi10_is_support_fine_grained_dpm(smu, clk_type);
> + if (ret < 0)
> + return ret;
> +
> + if (!ret) {
>   for (i = 0; i < count; i++) {
>   ret =
> smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
>   if (ret)
> @@ -1665,7 +1676,11 @@ static int navi10_force_clk_levels(struct
> smu_context *smu,
>   case SMU_UCLK:
>   case SMU_FCLK:
>   /* There is only 2 levels for fine grained DPM */
> - if (navi10_is_support_fine_grained_dpm(smu, clk_type)) {
> + ret = navi10_is_support_fine_grained_dpm(smu, clk_type);
> + if (ret < 0)
> + return ret;
> +
> + if (ret) {
>   soft_max_level = (soft_max_level >= 1 ? 1 : 0);
>   soft_min_level = (soft_min_level >= 1 ? 1 : 0);
>   }
> --
> 2.25.1



[PATCH 1/3 V2] drm/amd/pm: Fix negative array index read warning for pptable->DpmDescriptor

2024-04-28 Thread Jesse Zhang
Avoid using the negative values
for clk_idex as an index into an array pptable->DpmDescriptor.

V2: fix clk_index return check (Tim Huang)

Signed-off-by: Jesse Zhang 
---
 .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 27 ++-
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 5a68d365967f..c06e0d6e3017 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1219,19 +1219,22 @@ static int navi10_get_current_clk_freq_by_table(struct 
smu_context *smu,
   value);
 }
 
-static bool navi10_is_support_fine_grained_dpm(struct smu_context *smu, enum 
smu_clk_type clk_type)
+static int navi10_is_support_fine_grained_dpm(struct smu_context *smu, enum 
smu_clk_type clk_type)
 {
PPTable_t *pptable = smu->smu_table.driver_pptable;
DpmDescriptor_t *dpm_desc = NULL;
-   uint32_t clk_index = 0;
+   int clk_index = 0;
 
clk_index = smu_cmn_to_asic_specific_index(smu,
   CMN2ASIC_MAPPING_CLK,
   clk_type);
+   if (clk_index < 0)
+   return clk_index;
+
dpm_desc = &pptable->DpmDescriptor[clk_index];
 
/* 0 - Fine grained DPM, 1 - Discrete DPM */
-   return dpm_desc->SnapToDiscrete == 0;
+   return dpm_desc->SnapToDiscrete == 0 ? 1 : 0;
 }
 
 static inline bool navi10_od_feature_is_supported(struct 
smu_11_0_overdrive_table *od_table, enum SMU_11_0_ODFEATURE_CAP cap)
@@ -1287,7 +1290,11 @@ static int navi10_emit_clk_levels(struct smu_context 
*smu,
if (ret)
return ret;
 
-   if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
+   ret = navi10_is_support_fine_grained_dpm(smu, clk_type);
+   if (ret < 0)
+   return ret;
+
+   if (!ret) {
for (i = 0; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu,
  clk_type, 
i, &value);
@@ -1496,7 +1503,11 @@ static int navi10_print_clk_levels(struct smu_context 
*smu,
if (ret)
return size;
 
-   if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
+   ret = navi10_is_support_fine_grained_dpm(smu, clk_type);
+   if (ret < 0)
+   return ret;
+
+   if (!ret) {
for (i = 0; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu, 
clk_type, i, &value);
if (ret)
@@ -1665,7 +1676,11 @@ static int navi10_force_clk_levels(struct smu_context 
*smu,
case SMU_UCLK:
case SMU_FCLK:
/* There is only 2 levels for fine grained DPM */
-   if (navi10_is_support_fine_grained_dpm(smu, clk_type)) {
+   ret = navi10_is_support_fine_grained_dpm(smu, clk_type);
+   if (ret < 0)
+   return ret;
+
+   if (ret) {
soft_max_level = (soft_max_level >= 1 ? 1 : 0);
soft_min_level = (soft_min_level >= 1 ? 1 : 0);
}
-- 
2.25.1



[PATCH 1/2] drm/amdgpu/pm: Fix uninitialized variable agc_btc_response

2024-04-28 Thread Ma Jun
Assign an default value to agc_btc_response in failed case

Signed-off-by: Ma Jun 
---
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index 9f5bd998c6bf..74a33b9ace6c 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -2350,15 +2350,20 @@ static int vega10_acg_enable(struct pp_hwmgr *hwmgr)
 {
struct vega10_hwmgr *data = hwmgr->backend;
uint32_t agc_btc_response;
+   int ret;
 
if (data->smu_features[GNLD_ACG].supported) {
if (0 == vega10_enable_smc_features(hwmgr, true,

data->smu_features[GNLD_DPM_PREFETCHER].smu_feature_bitmap))
data->smu_features[GNLD_DPM_PREFETCHER].enabled = true;
 
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_InitializeAcg, NULL);
+   ret = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_InitializeAcg, 
NULL);
+   if (ret)
+   return ret;
 
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_RunAcgBtc, 
&agc_btc_response);
+   ret = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_RunAcgBtc, 
&agc_btc_response);
+   if (ret)
+   agc_btc_response = 0;
 
if (1 == agc_btc_response) {
if (1 == data->acg_loop_state)
-- 
2.34.1



[PATCH 2/2] drm/amdgpu/pm: Fix uninitialized variable warning

2024-04-28 Thread Ma Jun
Check return value of smum_send_msg_to_smc to fix
uninitialized variable varning

Signed-off-by: Ma Jun 
---
 .../drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c  | 21 ++-
 .../drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c |  8 +--
 .../drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c |  6 --
 .../drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c |  6 --
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
index 0b181bc8931c..f62381b189ad 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
@@ -1554,7 +1554,10 @@ static int smu10_set_fine_grain_clk_vol(struct pp_hwmgr 
*hwmgr,
}
 
if (input[0] == 0) {
-   smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
+   ret = smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
+   if (ret)
+   return ret;
+
if (input[1] < min_freq) {
pr_err("Fine grain setting minimum sclk (%ld) 
MHz is less than the minimum allowed (%d) MHz\n",
input[1], min_freq);
@@ -1562,7 +1565,10 @@ static int smu10_set_fine_grain_clk_vol(struct pp_hwmgr 
*hwmgr,
}
smu10_data->gfx_actual_soft_min_freq = input[1];
} else if (input[0] == 1) {
-   smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
+   ret = smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
+   if (ret)
+   return ret;
+
if (input[1] > max_freq) {
pr_err("Fine grain setting maximum sclk (%ld) 
MHz is greater than the maximum allowed (%d) MHz\n",
input[1], max_freq);
@@ -1577,10 +1583,15 @@ static int smu10_set_fine_grain_clk_vol(struct pp_hwmgr 
*hwmgr,
pr_err("Input parameter number not correct\n");
return -EINVAL;
}
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetMinGfxclkFrequency, 
&min_freq);
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetMaxGfxclkFrequency, 
&max_freq);
-
+   ret = smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
+   if (ret)
+   return ret;
smu10_data->gfx_actual_soft_min_freq = min_freq;
+
+   ret = smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
+   if (ret)
+   return ret;
+
smu10_data->gfx_actual_soft_max_freq = max_freq;
} else if (type == PP_OD_COMMIT_DPM_TABLE) {
if (size != 0) {
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index 74a33b9ace6c..c60666f64601 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -2486,9 +2486,13 @@ static int 
vega10_populate_and_upload_avfs_fuse_override(struct pp_hwmgr *hwmgr)
struct vega10_hwmgr *data = hwmgr->backend;
AvfsFuseOverride_t *avfs_fuse_table = 
&(data->smc_state_table.avfs_fuse_override_table);
 
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &top32);
+   result = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, 
&top32);
+   if (result)
+   return result;
 
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, &bottom32);
+   result = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, 
&bottom32);
+   if (result)
+   return result;
 
serial_number = ((uint64_t)bottom32 << 32) | top32;
 
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
index c223e3a6bfca..9dd407134770 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
@@ -364,8 +364,10 @@ static void vega12_init_dpm_defaults(struct pp_hwmgr 
*hwmgr)
}
 
/* Get the SN to turn into a Unique ID */
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &top32);
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, &bottom32);
+   if (smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &top32))
+   return;
+   if (smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, 
&bottom32))
+   return;
 
adev->unique_id = ((uint64_t)bottom32 << 32) | top32;
 }
diff --git a/drivers/gpu/drm/amd/pm/powerpl

RE: [PATCH 3/3] drm/amd/pm: fix the uninitialized scalar variable warning

2024-04-28 Thread Huang, Tim
[AMD Official Use Only - General]

> -Original Message-
> From: Jesse Zhang 
> Sent: Friday, April 26, 2024 3:29 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Koenig, Christian
> ; Huang, Tim ; Zhang,
> Jesse(Jie) ; Zhang, Jesse(Jie)
> 
> Subject: [PATCH 3/3] drm/amd/pm: fix the uninitialized scalar variable
> warning
>
> Fix warning for using uninitialized values ​​sclk_mask, mck_mask and
> soc_mask.
>
> Signed-off-by: Jesse Zhang 
> ---
>  drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
> index 8908bbb3ff1f..10f673b651a0 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
> @@ -932,7 +932,7 @@ static int renoir_set_performance_level(struct
> smu_context *smu,
>   enum amd_dpm_forced_level level)
>  {
>   int ret = 0;
> - uint32_t sclk_mask, mclk_mask, soc_mask;
> + uint32_t sclk_mask, mclk_mask, soc_mask = 0;

Hi Jesse,

We should not need to set default here. How about set the correct mask in the
renoir_get_profiling_clk_mask according to the profile.

Tim
>
>   switch (level) {
>   case AMD_DPM_FORCED_LEVEL_HIGH:
> @@ -1018,8 +1018,10 @@ static int renoir_set_performance_level(struct
> smu_context *smu,
>   &soc_mask);
>   if (ret)
>   return ret;
> - renoir_force_clk_levels(smu, SMU_SCLK, 1 << sclk_mask);
> - renoir_force_clk_levels(smu, SMU_MCLK, 1 << mclk_mask);
> + if (level == AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK)
> + renoir_force_clk_levels(smu, SMU_SCLK, 1 <<
> sclk_mask);
> + else
> + renoir_force_clk_levels(smu, SMU_MCLK, 1 <<
> mclk_mask);
We should need to set both the clock levels here, just need to get the correct 
mask before setting them.

Tim
>   renoir_force_clk_levels(smu, SMU_SOCCLK, 1 << soc_mask);
>   break;
>   case AMD_DPM_FORCED_LEVEL_PROFILE_PEAK:
> --
> 2.25.1



[PATCH] drm/amdgpu: Fix signedness bug in sdma_v4_0_process_trap_irq()

2024-04-28 Thread Dan Carpenter
The "instance" variable needs to be signed for the error handling to work.

Fixes: b34ddc71267a ("drm/amdgpu: add error handle to avoid out-of-bounds")
Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 101038395c3b..772604feb6ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -2017,7 +2017,7 @@ static int sdma_v4_0_process_trap_irq(struct 
amdgpu_device *adev,
  struct amdgpu_irq_src *source,
  struct amdgpu_iv_entry *entry)
 {
-   uint32_t instance;
+   int instance;
 
DRM_DEBUG("IH: SDMA trap\n");
instance = sdma_v4_0_irq_id_to_seq(entry->client_id);
-- 
2.43.0



[PATCH] drm/amd/display: re-indent dpp401_dscl_program_isharp()

2024-04-28 Thread Dan Carpenter
Smatch complains because some lines are indented more than they should
be.  I went a bit crazy re-indenting this.  ;)

The comments were not useful except as a marker of things which are left
to implement so I deleted most of them except for the TODO.

I introduced a "data" pointer so that I could replace
"scl_data->dscl_prog_data." with just "data->" and shorten the lines a
bit.  It's more readable without the line breaks.

I also tried to align it so you can see what is changing on each line.

Signed-off-by: Dan Carpenter 
---
 .../display/dc/dpp/dcn401/dcn401_dpp_dscl.c   | 93 ++-
 1 file changed, 30 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c 
b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
index c20376083441..696ccf96b847 100644
--- a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
+++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
@@ -779,75 +779,42 @@ static void dpp401_dscl_program_isharp(struct dpp 
*dpp_base,
const struct scaler_data *scl_data)
 {
struct dcn401_dpp *dpp = TO_DCN401_DPP(dpp_base);
+   const struct dscl_prog_data *data;
 
if (memcmp(&dpp->scl_data, scl_data, sizeof(*scl_data)) == 0)
return;
 
PERF_TRACE();
dpp->scl_data = *scl_data;
-   // ISHARP_EN
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_EN, scl_data->dscl_prog_data.isharp_en);
-   // ISHARP_NOISEDET_EN
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_NOISEDET_EN, 
scl_data->dscl_prog_data.isharp_noise_det.enable);
-   // ISHARP_NOISEDET_MODE
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_NOISEDET_MODE, 
scl_data->dscl_prog_data.isharp_noise_det.mode);
-   // ISHARP_NOISEDET_UTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_UTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);
-   // ISHARP_NOISEDET_DTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_DTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_NOISEDET_MODE, 
scl_data->dscl_prog_data.isharp_noise_det.mode);
-   // ISHARP_NOISEDET_UTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_UTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);
-   // ISHARP_NOISEDET_DTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_DTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);
-   // ISHARP_NOISEDET_PWL_START_IN
-   REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-   ISHARP_NOISEDET_PWL_START_IN, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_start_in);
-   // ISHARP_NOISEDET_PWL_END_IN
-   REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-   ISHARP_NOISEDET_PWL_END_IN, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_end_in);
-   // ISHARP_NOISEDET_PWL_SLOPE
-   REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-   ISHARP_NOISEDET_PWL_SLOPE, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_slope);
-   // ISHARP_LBA_MODE
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_LBA_MODE, 
scl_data->dscl_prog_data.isharp_lba.mode);
-   // TODO: ISHARP_LBA: IN_SEG, BASE_SEG, SLOPE_SEG
-   // ISHARP_FMT_MODE
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_FMT_MODE, 
scl_data->dscl_prog_data.isharp_fmt.mode);
-   // ISHARP_FMT_NORM
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_FMT_NORM, 
scl_data->dscl_prog_data.isharp_fmt.norm);
-   // ISHARP_DELTA_LUT
-   dpp401_dscl_set_isharp_filter(dpp, 
scl_data->dscl_prog_data.isharp_delta);
-   // ISHARP_NLDELTA_SCLIP_EN_P
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   ISHARP_NLDELTA_SCLIP_EN_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.enable_p);
-   // ISHARP_NLDELTA_SCLIP_PIVOT_P
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   ISHARP_NLDELTA_SCLIP_PIVOT_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.pivot_p);
-   // ISHARP_NLDELTA_SCLIP_SLOPE_P
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   ISHARP_NLDELTA_SCLIP_SLOPE_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.slope_p);
-   // ISHARP_NLDELTA_SCLIP_EN_N
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   ISHARP_NLDELTA_SCLIP_EN_N, 
scl_data->dscl_prog_data.isharp_nldel

Re: [PATCH] drm/amd/display: Add MSF panel to DPCD 0x317 patch list

2024-04-28 Thread Tobias Jakobi

On 3/9/24 02:47, tjak...@math.uni-bielefeld.de wrote:


From: Tobias Jakobi 

This 8.4 inch panel is integrated in the Ayaneo Kun handheld
device. The panel resolution is 2560×1600, i.e. it has
portrait dimensions.

Decoding the EDID shows:
Manufacturer: MSF
Model: 4099
Display Product Name: 'TV080WUM-NL0 '

Judging from the product name this might be a clone of a
BOE panel, but with larger dimensions.

Panel frequently shows non-functional backlight control. Adding
some debug prints to update_connector_ext_caps() shows that
something the OLED bit of ext_caps is set, and then the driver
assumes that backlight is controlled via AUX.

Forcing backlight control to PWM via amdgpu.backlight=0 restores
backlight operation.

Signed-off-by: Tobias Jakobi 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 7a09a72e182f..5a017ba94e3c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -68,6 +68,7 @@ static void apply_edid_quirks(struct edid *edid, struct 
dc_edid_caps *edid_caps)
case drm_edid_encode_panel_id('A', 'U', 'O', 0xE69B):
case drm_edid_encode_panel_id('B', 'O', 'E', 0x092A):
case drm_edid_encode_panel_id('L', 'G', 'D', 0x06D1):
+   case drm_edid_encode_panel_id('M', 'S', 'F', 0x1003):
DRM_DEBUG_DRIVER("Clearing DPCD 0x317 on monitor with panel id 
%X\n", panel_id);
edid_caps->panel_patch.remove_sink_ext_caps = true;
break;

Another gentle ping!


Re: [PATCH 2/2] drm/amdgpu/pm: Fix uninitialized variable warning

2024-04-28 Thread Alex Deucher
On Sun, Apr 28, 2024 at 7:12 AM Ma Jun  wrote:
>
> Check return value of smum_send_msg_to_smc to fix
> uninitialized variable varning
>
> Signed-off-by: Ma Jun 
> ---
>  .../drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c  | 21 ++-
>  .../drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c |  8 +--
>  .../drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c |  6 --
>  .../drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c |  6 --
>  4 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c 
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
> index 0b181bc8931c..f62381b189ad 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
> @@ -1554,7 +1554,10 @@ static int smu10_set_fine_grain_clk_vol(struct 
> pp_hwmgr *hwmgr,
> }
>
> if (input[0] == 0) {
> -   smum_send_msg_to_smc(hwmgr, 
> PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
> +   ret = smum_send_msg_to_smc(hwmgr, 
> PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
> +   if (ret)
> +   return ret;
> +
> if (input[1] < min_freq) {
> pr_err("Fine grain setting minimum sclk (%ld) 
> MHz is less than the minimum allowed (%d) MHz\n",
> input[1], min_freq);
> @@ -1562,7 +1565,10 @@ static int smu10_set_fine_grain_clk_vol(struct 
> pp_hwmgr *hwmgr,
> }
> smu10_data->gfx_actual_soft_min_freq = input[1];
> } else if (input[0] == 1) {
> -   smum_send_msg_to_smc(hwmgr, 
> PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
> +   ret = smum_send_msg_to_smc(hwmgr, 
> PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
> +   if (ret)
> +   return ret;
> +
> if (input[1] > max_freq) {
> pr_err("Fine grain setting maximum sclk (%ld) 
> MHz is greater than the maximum allowed (%d) MHz\n",
> input[1], max_freq);
> @@ -1577,10 +1583,15 @@ static int smu10_set_fine_grain_clk_vol(struct 
> pp_hwmgr *hwmgr,
> pr_err("Input parameter number not correct\n");
> return -EINVAL;
> }
> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetMinGfxclkFrequency, 
> &min_freq);
> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetMaxGfxclkFrequency, 
> &max_freq);
> -
> +   ret = smum_send_msg_to_smc(hwmgr, 
> PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
> +   if (ret)
> +   return ret;
> smu10_data->gfx_actual_soft_min_freq = min_freq;
> +
> +   ret = smum_send_msg_to_smc(hwmgr, 
> PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
> +   if (ret)
> +   return ret;
> +
> smu10_data->gfx_actual_soft_max_freq = max_freq;
> } else if (type == PP_OD_COMMIT_DPM_TABLE) {
> if (size != 0) {
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c 
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> index 74a33b9ace6c..c60666f64601 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> @@ -2486,9 +2486,13 @@ static int 
> vega10_populate_and_upload_avfs_fuse_override(struct pp_hwmgr *hwmgr)
> struct vega10_hwmgr *data = hwmgr->backend;
> AvfsFuseOverride_t *avfs_fuse_table = 
> &(data->smc_state_table.avfs_fuse_override_table);
>
> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &top32);
> +   result = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, 
> &top32);
> +   if (result)
> +   return result;
>
> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, 
> &bottom32);
> +   result = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, 
> &bottom32);
> +   if (result)
> +   return result;
>
> serial_number = ((uint64_t)bottom32 << 32) | top32;
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c 
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
> index c223e3a6bfca..9dd407134770 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
> @@ -364,8 +364,10 @@ static void vega12_init_dpm_defaults(struct pp_hwmgr 
> *hwmgr)
> }
>
> /* Get the SN to turn into a Unique ID */
> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &top32);
> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, 
> &bottom32);
> +   if (smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNu

Re: [PATCH] drm/amd/display: re-indent dpp401_dscl_program_isharp()

2024-04-28 Thread Aurabindo Pillai

Thanks for the fix!

Reviewed-by: Aurabindo Pillai 

On 4/28/24 8:42 AM, Dan Carpenter wrote:

Smatch complains because some lines are indented more than they should
be.  I went a bit crazy re-indenting this.  ;)

The comments were not useful except as a marker of things which are left
to implement so I deleted most of them except for the TODO.

I introduced a "data" pointer so that I could replace
"scl_data->dscl_prog_data." with just "data->" and shorten the lines a
bit.  It's more readable without the line breaks.

I also tried to align it so you can see what is changing on each line.

Signed-off-by: Dan Carpenter 
---
  .../display/dc/dpp/dcn401/dcn401_dpp_dscl.c   | 93 ++-
  1 file changed, 30 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c 
b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
index c20376083441..696ccf96b847 100644
--- a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
+++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
@@ -779,75 +779,42 @@ static void dpp401_dscl_program_isharp(struct dpp 
*dpp_base,
const struct scaler_data *scl_data)
  {
struct dcn401_dpp *dpp = TO_DCN401_DPP(dpp_base);
+   const struct dscl_prog_data *data;
  
  	if (memcmp(&dpp->scl_data, scl_data, sizeof(*scl_data)) == 0)

return;
  
  	PERF_TRACE();

dpp->scl_data = *scl_data;
-   // ISHARP_EN
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_EN, scl_data->dscl_prog_data.isharp_en);
-   // ISHARP_NOISEDET_EN
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_NOISEDET_EN, 
scl_data->dscl_prog_data.isharp_noise_det.enable);
-   // ISHARP_NOISEDET_MODE
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_NOISEDET_MODE, 
scl_data->dscl_prog_data.isharp_noise_det.mode);
-   // ISHARP_NOISEDET_UTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_UTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);
-   // ISHARP_NOISEDET_DTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_DTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_NOISEDET_MODE, 
scl_data->dscl_prog_data.isharp_noise_det.mode);
-   // ISHARP_NOISEDET_UTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_UTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);
-   // ISHARP_NOISEDET_DTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_DTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);
-   // ISHARP_NOISEDET_PWL_START_IN
-   REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-   ISHARP_NOISEDET_PWL_START_IN, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_start_in);
-   // ISHARP_NOISEDET_PWL_END_IN
-   REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-   ISHARP_NOISEDET_PWL_END_IN, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_end_in);
-   // ISHARP_NOISEDET_PWL_SLOPE
-   REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-   ISHARP_NOISEDET_PWL_SLOPE, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_slope);
-   // ISHARP_LBA_MODE
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_LBA_MODE, 
scl_data->dscl_prog_data.isharp_lba.mode);
-   // TODO: ISHARP_LBA: IN_SEG, BASE_SEG, SLOPE_SEG
-   // ISHARP_FMT_MODE
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_FMT_MODE, 
scl_data->dscl_prog_data.isharp_fmt.mode);
-   // ISHARP_FMT_NORM
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_FMT_NORM, 
scl_data->dscl_prog_data.isharp_fmt.norm);
-   // ISHARP_DELTA_LUT
-   dpp401_dscl_set_isharp_filter(dpp, 
scl_data->dscl_prog_data.isharp_delta);
-   // ISHARP_NLDELTA_SCLIP_EN_P
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   ISHARP_NLDELTA_SCLIP_EN_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.enable_p);
-   // ISHARP_NLDELTA_SCLIP_PIVOT_P
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   ISHARP_NLDELTA_SCLIP_PIVOT_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.pivot_p);
-   // ISHARP_NLDELTA_SCLIP_SLOPE_P
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   ISHARP_NLDELTA_SCLIP_SLOPE_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.slope_p);
-   // ISHARP_NLDELTA_SCLIP_EN_N
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   

Re: [PATCH] drm/amd/display: re-indent dpp401_dscl_program_isharp()

2024-04-28 Thread Aurabindo Pillai

Patch has been merged to amd-staging-drm-next.

On 4/28/24 12:09 PM, Aurabindo Pillai wrote:

Thanks for the fix!

Reviewed-by: Aurabindo Pillai 

On 4/28/24 8:42 AM, Dan Carpenter wrote:

Smatch complains because some lines are indented more than they should
be.  I went a bit crazy re-indenting this.  ;)

The comments were not useful except as a marker of things which are left
to implement so I deleted most of them except for the TODO.

I introduced a "data" pointer so that I could replace
"scl_data->dscl_prog_data." with just "data->" and shorten the lines a
bit.  It's more readable without the line breaks.

I also tried to align it so you can see what is changing on each line.

Signed-off-by: Dan Carpenter 
---
  .../display/dc/dpp/dcn401/dcn401_dpp_dscl.c   | 93 ++-
  1 file changed, 30 insertions(+), 63 deletions(-)

diff --git 
a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c 
b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c

index c20376083441..696ccf96b847 100644
--- a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
+++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
@@ -779,75 +779,42 @@ static void dpp401_dscl_program_isharp(struct 
dpp *dpp_base,

  const struct scaler_data *scl_data)
  {
  struct dcn401_dpp *dpp = TO_DCN401_DPP(dpp_base);
+    const struct dscl_prog_data *data;
  if (memcmp(&dpp->scl_data, scl_data, sizeof(*scl_data)) == 0)
  return;
  PERF_TRACE();
  dpp->scl_data = *scl_data;
-    // ISHARP_EN
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_EN, scl_data->dscl_prog_data.isharp_en);
-    // ISHARP_NOISEDET_EN
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_NOISEDET_EN, 
scl_data->dscl_prog_data.isharp_noise_det.enable);

-    // ISHARP_NOISEDET_MODE
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_NOISEDET_MODE, 
scl_data->dscl_prog_data.isharp_noise_det.mode);

-    // ISHARP_NOISEDET_UTHRE
-    REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-    ISHARP_NOISEDET_UTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);

-    // ISHARP_NOISEDET_DTHRE
-    REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-    ISHARP_NOISEDET_DTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);

-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_NOISEDET_MODE, 
scl_data->dscl_prog_data.isharp_noise_det.mode);

-    // ISHARP_NOISEDET_UTHRE
-    REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-    ISHARP_NOISEDET_UTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);

-    // ISHARP_NOISEDET_DTHRE
-    REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-    ISHARP_NOISEDET_DTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);

-    // ISHARP_NOISEDET_PWL_START_IN
-    REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-    ISHARP_NOISEDET_PWL_START_IN, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_start_in);

-    // ISHARP_NOISEDET_PWL_END_IN
-    REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-    ISHARP_NOISEDET_PWL_END_IN, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_end_in);

-    // ISHARP_NOISEDET_PWL_SLOPE
-    REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-    ISHARP_NOISEDET_PWL_SLOPE, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_slope);

-    // ISHARP_LBA_MODE
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_LBA_MODE, 
scl_data->dscl_prog_data.isharp_lba.mode);

-    // TODO: ISHARP_LBA: IN_SEG, BASE_SEG, SLOPE_SEG
-    // ISHARP_FMT_MODE
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_FMT_MODE, 
scl_data->dscl_prog_data.isharp_fmt.mode);

-    // ISHARP_FMT_NORM
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_FMT_NORM, 
scl_data->dscl_prog_data.isharp_fmt.norm);

-    // ISHARP_DELTA_LUT
-    dpp401_dscl_set_isharp_filter(dpp, 
scl_data->dscl_prog_data.isharp_delta);

-    // ISHARP_NLDELTA_SCLIP_EN_P
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SCLIP_EN_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.enable_p);

-    // ISHARP_NLDELTA_SCLIP_PIVOT_P
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SCLIP_PIVOT_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.pivot_p);

-    // ISHARP_NLDELTA_SCLIP_SLOPE_P
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SCLIP_SLOPE_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.slope_p);

-    // ISHARP_NLDELTA_SCLIP_EN_N
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SCLIP_EN_N, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.enable_n);

-    // ISHARP_NLDELTA_SCLIP_PIVOT_N
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SCLIP_PIVOT_N, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.pivot_n);

-    // ISHARP_NLDELTA_SCLIP_SLOPE_N
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SC

RE: [PATCH] drm/amdgpu: Fix signedness bug in sdma_v4_0_process_trap_irq()

2024-04-28 Thread Zhou, Bob
[Public]

Reviewed-by: Bob Zhou 

Regards,
Bob

-Original Message-
From: Dan Carpenter 
Sent: 2024年4月28日 20:57
To: Zhou, Bob 
Cc: Deucher, Alexander ; Koenig, Christian 
; Pan, Xinhui ; David Airlie 
; Daniel Vetter ; Kuehling, Felix 
; Zhang, Hawking ; Guchun Chen 
; Ma, Le ; Lazar, Lijo 
; Sharma, Shashank ; 
amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; 
linux-ker...@vger.kernel.org; kernel-janit...@vger.kernel.org
Subject: [PATCH] drm/amdgpu: Fix signedness bug in sdma_v4_0_process_trap_irq()

The "instance" variable needs to be signed for the error handling to work.

Fixes: b34ddc71267a ("drm/amdgpu: add error handle to avoid out-of-bounds")
Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 101038395c3b..772604feb6ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -2017,7 +2017,7 @@ static int sdma_v4_0_process_trap_irq(struct 
amdgpu_device *adev,
  struct amdgpu_irq_src *source,
  struct amdgpu_iv_entry *entry)  {
-   uint32_t instance;
+   int instance;

DRM_DEBUG("IH: SDMA trap\n");
instance = sdma_v4_0_irq_id_to_seq(entry->client_id);
--
2.43.0



[PATCH 3/3 V2] drm/amd/pm: fix the uninitialized scalar variable warning

2024-04-28 Thread Jesse Zhang
Fix warning for using uninitialized values sclk_mask, mck_mask and soc_mask.
 v2: Init the variables in the renoir_get_profiling_clk_mask(Tim Huang)

Signed-off-by: Jesse Zhang 
---
 drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c 
b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
index 8908bbb3ff1f..546a2268823a 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
@@ -253,6 +253,10 @@ static int renoir_get_profiling_clk_mask(struct 
smu_context *smu,
 uint32_t *mclk_mask,
 uint32_t *soc_mask)
 {
+   *sclk_mask = 0;
+   /* mclk levels are in reverse order */
+   *mclk_maks = NUM_MEMCLK_DPM_LEVELS - 1;
+   *sock_mask = 0;
 
if (level == AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK) {
if (sclk_mask)
-- 
2.25.1



[PATCH] drm/amd/pm: fix warning using uninitialized value of max_vid_step

2024-04-28 Thread Jesse Zhang
Check the return of pp_atomfwctrl_get_Voltage_table_v4
as it may fail to initialize max_vid_step

Signed-off-by: Jesse Zhang 
---
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index b602059436a8..70c711cec897 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -2573,8 +2573,12 @@ static int vega10_init_smc_table(struct pp_hwmgr *hwmgr)
}
}
 
-   pp_atomfwctrl_get_voltage_table_v4(hwmgr, VOLTAGE_TYPE_VDDC,
+   result = pp_atomfwctrl_get_voltage_table_v4(hwmgr, VOLTAGE_TYPE_VDDC,
VOLTAGE_OBJ_SVID2,  &voltage_table);
+   PP_ASSERT_WITH_CODE(result < 0,
+   "Failed to get voltage tables!",
+   return result);
+
pp_table->MaxVidStep = voltage_table.max_vid_step;
 
pp_table->GfxDpmVoltageMode =
-- 
2.25.1



RE: [PATCH 2/2] drm/amdgpu/pm: Fix uninitialized variable warning

2024-04-28 Thread Wang, Yang(Kevin)
[AMD Official Use Only - General]

Series is.
Reviewed-by: Yang Wang 

Best Regards,
Kevin

-Original Message-
From: Ma, Jun 
Sent: Sunday, April 28, 2024 5:55 PM
To: amd-gfx@lists.freedesktop.org
Cc: Feng, Kenneth ; Deucher, Alexander 
; Wang, Yang(Kevin) ; 
Koenig, Christian ; Ma, Jun 
Subject: [PATCH 2/2] drm/amdgpu/pm: Fix uninitialized variable warning

Check return value of smum_send_msg_to_smc to fix uninitialized variable varning

Signed-off-by: Ma Jun 
---
 .../drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c  | 21 ++-  
.../drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c |  8 +--  
.../drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c |  6 --  
.../drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c |  6 --
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
index 0b181bc8931c..f62381b189ad 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
@@ -1554,7 +1554,10 @@ static int smu10_set_fine_grain_clk_vol(struct pp_hwmgr 
*hwmgr,
}

if (input[0] == 0) {
-   smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
+   ret = smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
+   if (ret)
+   return ret;
+
if (input[1] < min_freq) {
pr_err("Fine grain setting minimum sclk (%ld) 
MHz is less than the minimum allowed (%d) MHz\n",
input[1], min_freq);
@@ -1562,7 +1565,10 @@ static int smu10_set_fine_grain_clk_vol(struct pp_hwmgr 
*hwmgr,
}
smu10_data->gfx_actual_soft_min_freq = input[1];
} else if (input[0] == 1) {
-   smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
+   ret = smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
+   if (ret)
+   return ret;
+
if (input[1] > max_freq) {
pr_err("Fine grain setting maximum sclk (%ld) 
MHz is greater than the maximum allowed (%d) MHz\n",
input[1], max_freq);
@@ -1577,10 +1583,15 @@ static int smu10_set_fine_grain_clk_vol(struct pp_hwmgr 
*hwmgr,
pr_err("Input parameter number not correct\n");
return -EINVAL;
}
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetMinGfxclkFrequency, 
&min_freq);
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetMaxGfxclkFrequency, 
&max_freq);
-
+   ret = smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
+   if (ret)
+   return ret;
smu10_data->gfx_actual_soft_min_freq = min_freq;
+
+   ret = smum_send_msg_to_smc(hwmgr, 
PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
+   if (ret)
+   return ret;
+
smu10_data->gfx_actual_soft_max_freq = max_freq;
} else if (type == PP_OD_COMMIT_DPM_TABLE) {
if (size != 0) {
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index 74a33b9ace6c..c60666f64601 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -2486,9 +2486,13 @@ static int 
vega10_populate_and_upload_avfs_fuse_override(struct pp_hwmgr *hwmgr)
struct vega10_hwmgr *data = hwmgr->backend;
AvfsFuseOverride_t *avfs_fuse_table = 
&(data->smc_state_table.avfs_fuse_override_table);

-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &top32);
+   result = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, 
&top32);
+   if (result)
+   return result;

-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, &bottom32);
+   result = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, 
&bottom32);
+   if (result)
+   return result;

serial_number = ((uint64_t)bottom32 << 32) | top32;

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
index c223e3a6bfca..9dd407134770 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
@@ -364,8 +364,10 @@ static void vega12_init_dpm_defaults(struct pp_hwmgr 
*hwmgr)
}

/* Get the SN to turn into a Unique ID */
-   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &top32);
-   smum_send_msg_to_smc(hwmgr, PP

Recall: [PATCH 2/2] drm/amdgpu/pm: Fix uninitialized variable warning

2024-04-28 Thread Wang, Yang(Kevin)
Wang, Yang(Kevin) would like to recall the message, "[PATCH 2/2] drm/amdgpu/pm: 
Fix uninitialized variable warning".

Recall: [PATCH 2/2] drm/amdgpu/pm: Fix uninitialized variable warning

2024-04-28 Thread Wang, Yang(Kevin)
Wang, Yang(Kevin) would like to recall the message, "[PATCH 2/2] drm/amdgpu/pm: 
Fix uninitialized variable warning".

RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory

2024-04-28 Thread Chai, Thomas
[AMD Official Use Only - General]

OK


-
Best Regards,
Thomas

-Original Message-
From: Wang, Yang(Kevin) 
Sent: Sunday, April 28, 2024 3:48 PM
To: Chai, Thomas ; amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Zhou1, Tao ; Li, 
Candice ; Yang, Stanley 
Subject: RE: [PATCH] drm/amdgpu: add mutex to protect ras shared memory

[AMD Official Use Only - General]

-Original Message-
From: Chai, Thomas 
Sent: Sunday, April 28, 2024 3:08 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Zhou1, Tao ; Li, 
Candice ; Wang, Yang(Kevin) ; Yang, 
Stanley ; Chai, Thomas 
Subject: [PATCH] drm/amdgpu: add mutex to protect ras shared memory

Add mutex to protect ras shared memory.

Signed-off-by: YiPeng Chai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 121 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h|   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp_ta.c |   2 +
 3 files changed, 84 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 5583e2d1b12f..fa4fea00f6b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -1564,6 +1564,66 @@ static void psp_ras_ta_check_status(struct psp_context 
*psp)
}
 }

+static int psp_ras_send_cmd(struct psp_context *psp,
+   enum ras_command cmd_id, void *in, void *out) {
+   struct ta_ras_shared_memory *ras_cmd;
+   uint32_t cmd = cmd_id;
+   int ret = 0;
+
+   mutex_lock(&psp->ras_context.mutex);
+   ras_cmd = (struct ta_ras_shared_memory 
*)psp->ras_context.context.mem_context.shared_buf;
+   memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory));
+
+   switch (cmd) {
+   case TA_RAS_COMMAND__ENABLE_FEATURES:
+   case TA_RAS_COMMAND__DISABLE_FEATURES:
+   memcpy(&ras_cmd->ras_in_message,
+   in, sizeof(ras_cmd->ras_in_message));
+   break;
+   case TA_RAS_COMMAND__TRIGGER_ERROR:
+   memcpy(&ras_cmd->ras_in_message.trigger_error,
+   in, sizeof(ras_cmd->ras_in_message.trigger_error));
+   break;
+   case TA_RAS_COMMAND__QUERY_ADDRESS:
+   memcpy(&ras_cmd->ras_in_message.address,
+   in, sizeof(ras_cmd->ras_in_message.address));
+   break;
+   default:
+   dev_err(psp->adev->dev, "Invalid ras cmd id: %u\n", cmd);
+   ret = -EINVAL;
+   goto err_out;
+   }
+
+   ras_cmd->cmd_id = cmd;
+   ret = psp_ras_invoke(psp, ras_cmd->cmd_id);
+
+   switch (cmd) {
+   case TA_RAS_COMMAND__TRIGGER_ERROR:
+   if (out) {
+   uint32_t *ras_status = (uint32_t *)out;
[Kevin]:
It's better to check 'ret' value first before use this 'out' data.

Best Regards,
Kevin
+
+   *ras_status = ras_cmd->ras_status;
+   }
+   break;
+   case TA_RAS_COMMAND__QUERY_ADDRESS:
+   if (ret || ras_cmd->ras_status || psp->cmd_buf_mem->resp.status)
+   ret = -EINVAL;
+   else if (out)
+   memcpy(out,
+   &ras_cmd->ras_out_message.address,
+   sizeof(ras_cmd->ras_out_message.address));
+   break;
+   default:
+   break;
+   }
+
+err_out:
+   mutex_unlock(&psp->ras_context.mutex);
+
+   return ret;
+}
+
 int psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id)  {
struct ta_ras_shared_memory *ras_cmd; @@ -1605,23 +1665,15 @@ int 
psp_ras_invoke(struct psp_context *psp, uint32_t ta_cmd_id)  int 
psp_ras_enable_features(struct psp_context *psp,
union ta_ras_cmd_input *info, bool enable)  {
-   struct ta_ras_shared_memory *ras_cmd;
+   enum ras_command cmd_id;
int ret;

-   if (!psp->ras_context.context.initialized)
+   if (!psp->ras_context.context.initialized || !info)
return -EINVAL;

-   ras_cmd = (struct ta_ras_shared_memory 
*)psp->ras_context.context.mem_context.shared_buf;
-   memset(ras_cmd, 0, sizeof(struct ta_ras_shared_memory));
-
-   if (enable)
-   ras_cmd->cmd_id = TA_RAS_COMMAND__ENABLE_FEATURES;
-   else
-   ras_cmd->cmd_id = TA_RAS_COMMAND__DISABLE_FEATURES;
-
-   ras_cmd->ras_in_message = *info;
-
-   ret = psp_ras_invoke(psp, ras_cmd->cmd_id);
+   cmd_id = enable ?
+   TA_RAS_COMMAND__ENABLE_FEATURES : 
TA_RAS_COMMAND__DISABLE_FEATURES;
+   ret = psp_ras_send_cmd(psp, cmd_id, info, NULL);
if (ret)
return -EINVAL;

@@ -1645,6 +1697,8 @@ int psp_ras_terminate(struct psp_context *psp)

psp->ras_context.context.initialized = false;

+   mutex_destroy(&psp->ras_context.mutex);
+
return ret;
 }

@@ -1729,9 +1783,10 @@ int psp_ras_initialize(struct psp_context *ps

Re: [PATCH 2/2] drm/amdgpu/pm: Fix uninitialized variable warning

2024-04-28 Thread Ma, Jun



On 4/28/2024 10:18 PM, Alex Deucher wrote:
> On Sun, Apr 28, 2024 at 7:12 AM Ma Jun  wrote:
>>
>> Check return value of smum_send_msg_to_smc to fix
>> uninitialized variable varning
>>
>> Signed-off-by: Ma Jun 
>> ---
>>  .../drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c  | 21 ++-
>>  .../drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c |  8 +--
>>  .../drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c |  6 --
>>  .../drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c |  6 --
>>  4 files changed, 30 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c 
>> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>> index 0b181bc8931c..f62381b189ad 100644
>> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>> @@ -1554,7 +1554,10 @@ static int smu10_set_fine_grain_clk_vol(struct 
>> pp_hwmgr *hwmgr,
>> }
>>
>> if (input[0] == 0) {
>> -   smum_send_msg_to_smc(hwmgr, 
>> PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
>> +   ret = smum_send_msg_to_smc(hwmgr, 
>> PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
>> +   if (ret)
>> +   return ret;
>> +
>> if (input[1] < min_freq) {
>> pr_err("Fine grain setting minimum sclk 
>> (%ld) MHz is less than the minimum allowed (%d) MHz\n",
>> input[1], min_freq);
>> @@ -1562,7 +1565,10 @@ static int smu10_set_fine_grain_clk_vol(struct 
>> pp_hwmgr *hwmgr,
>> }
>> smu10_data->gfx_actual_soft_min_freq = input[1];
>> } else if (input[0] == 1) {
>> -   smum_send_msg_to_smc(hwmgr, 
>> PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
>> +   ret = smum_send_msg_to_smc(hwmgr, 
>> PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
>> +   if (ret)
>> +   return ret;
>> +
>> if (input[1] > max_freq) {
>> pr_err("Fine grain setting maximum sclk 
>> (%ld) MHz is greater than the maximum allowed (%d) MHz\n",
>> input[1], max_freq);
>> @@ -1577,10 +1583,15 @@ static int smu10_set_fine_grain_clk_vol(struct 
>> pp_hwmgr *hwmgr,
>> pr_err("Input parameter number not correct\n");
>> return -EINVAL;
>> }
>> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetMinGfxclkFrequency, 
>> &min_freq);
>> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetMaxGfxclkFrequency, 
>> &max_freq);
>> -
>> +   ret = smum_send_msg_to_smc(hwmgr, 
>> PPSMC_MSG_GetMinGfxclkFrequency, &min_freq);
>> +   if (ret)
>> +   return ret;
>> smu10_data->gfx_actual_soft_min_freq = min_freq;
>> +
>> +   ret = smum_send_msg_to_smc(hwmgr, 
>> PPSMC_MSG_GetMaxGfxclkFrequency, &max_freq);
>> +   if (ret)
>> +   return ret;
>> +
>> smu10_data->gfx_actual_soft_max_freq = max_freq;
>> } else if (type == PP_OD_COMMIT_DPM_TABLE) {
>> if (size != 0) {
>> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c 
>> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>> index 74a33b9ace6c..c60666f64601 100644
>> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>> @@ -2486,9 +2486,13 @@ static int 
>> vega10_populate_and_upload_avfs_fuse_override(struct pp_hwmgr *hwmgr)
>> struct vega10_hwmgr *data = hwmgr->backend;
>> AvfsFuseOverride_t *avfs_fuse_table = 
>> &(data->smc_state_table.avfs_fuse_override_table);
>>
>> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &top32);
>> +   result = smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, 
>> &top32);
>> +   if (result)
>> +   return result;
>>
>> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumBottom32, 
>> &bottom32);
>> +   result = smum_send_msg_to_smc(hwmgr, 
>> PPSMC_MSG_ReadSerialNumBottom32, &bottom32);
>> +   if (result)
>> +   return result;
>>
>> serial_number = ((uint64_t)bottom32 << 32) | top32;
>>
>> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c 
>> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
>> index c223e3a6bfca..9dd407134770 100644
>> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
>> @@ -364,8 +364,10 @@ static void vega12_init_dpm_defaults(struct pp_hwmgr 
>> *hwmgr)
>> }
>>
>> /* Get the SN to turn into a Unique ID */
>> -   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_ReadSerialNumTop32, &

[PATCH v9 2/5] drm/amdgpu: Add mqd support for the fence address

2024-04-28 Thread Arunpravin Paneer Selvam
- Add a field in struct v11_gfx_mqd for userqueue
  fence address.

- Assign fence gpu VA address to the userqueue mqd
  fence address fields.

v2: Remove the mask and replace with lower_32_bits (Christian)

Signed-off-by: Arunpravin Paneer Selvam 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c | 11 +++
 drivers/gpu/drm/amd/include/v11_structs.h|  4 ++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c 
b/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c
index 6369a26fb07e..d8592cfef534 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c
@@ -25,6 +25,7 @@
 #include "v11_structs.h"
 #include "mes_v11_0.h"
 #include "amdgpu_userqueue.h"
+#include "amdgpu_userq_fence.h"
 
 #define AMDGPU_USERQ_PROC_CTX_SZ PAGE_SIZE
 #define AMDGPU_USERQ_GANG_CTX_SZ PAGE_SIZE
@@ -204,6 +205,14 @@ static int mes_v11_0_userq_create_ctx_space(struct 
amdgpu_userq_mgr *uq_mgr,
return 0;
 }
 
+static void mes_v11_0_userq_set_fence_space(struct amdgpu_usermode_queue 
*queue)
+{
+   struct v11_gfx_mqd *mqd = queue->mqd.cpu_ptr;
+
+   mqd->fenceaddress_lo = lower_32_bits(queue->fence_drv->gpu_addr);
+   mqd->fenceaddress_hi = upper_32_bits(queue->fence_drv->gpu_addr);
+}
+
 static int mes_v11_0_userq_mqd_create(struct amdgpu_userq_mgr *uq_mgr,
  struct drm_amdgpu_userq_in *args_in,
  struct amdgpu_usermode_queue *queue)
@@ -274,6 +283,8 @@ static int mes_v11_0_userq_mqd_create(struct 
amdgpu_userq_mgr *uq_mgr,
goto free_mqd;
}
 
+   mes_v11_0_userq_set_fence_space(queue);
+
/* FW expects WPTR BOs to be mapped into GART */
r = mes_v11_0_create_wptr_mapping(uq_mgr, queue, 
userq_props->wptr_gpu_addr);
if (r) {
diff --git a/drivers/gpu/drm/amd/include/v11_structs.h 
b/drivers/gpu/drm/amd/include/v11_structs.h
index f8008270f813..797ce6a1e56e 100644
--- a/drivers/gpu/drm/amd/include/v11_structs.h
+++ b/drivers/gpu/drm/amd/include/v11_structs.h
@@ -535,8 +535,8 @@ struct v11_gfx_mqd {
uint32_t reserved_507; // offset: 507  (0x1FB)
uint32_t reserved_508; // offset: 508  (0x1FC)
uint32_t reserved_509; // offset: 509  (0x1FD)
-   uint32_t reserved_510; // offset: 510  (0x1FE)
-   uint32_t reserved_511; // offset: 511  (0x1FF)
+   uint32_t fenceaddress_lo; // offset: 510  (0x1FE)
+   uint32_t fenceaddress_hi; // offset: 511  (0x1FF)
 };
 
 struct v11_sdma_mqd {
-- 
2.25.1



[PATCH v9 1/5] drm/amdgpu: Implement a new userqueue fence driver

2024-04-28 Thread Arunpravin Paneer Selvam
Developed a userqueue fence driver for the userqueue process shared
BO synchronization.

Create a dma fence having write pointer as the seqno and allocate a
seq64 memory for each user queue process and feed this memory address
into the firmware/hardware, thus the firmware writes the read pointer
into the given address when the process completes it execution.
Compare wptr and rptr, if rptr >= wptr, signal the fences for the waiting
process to consume the buffers.

v2: Worked on review comments from Christian for the following
modifications

- Add wptr as sequence number into the fence
- Add a reference count for the fence driver
- Add dma_fence_put below the list_del as it might
  frees the userq fence.
- Trim unnecessary code in interrupt handler.
- Check dma fence signaled state in dma fence creation
  function for a potential problem of hardware completing
  the job processing beforehand.
- Add necessary locks.
- Create a list and process all the unsignaled fences.
- clean up fences in destroy function.
- implement .signaled callback function

v3: Worked on review comments from Christian
- Modify naming convention for reference counted objects
- Fix fence driver reference drop issue
- Drop amdgpu_userq_fence_driver_process() function return value

v4: Worked on review comments from Christian
- Moved fence driver allocation into amdgpu_userq_fence_driver_alloc()
- Added detail doc mentioning the differences b/w
  two spinlocks declared.

v5: Worked on review comments from Christian
- Check before upcast and remove local variable
- Add error handling in fence_drv alloc function.
- Move rptr read fn outside of the loop and remove WARN_ON in
  destroy function.

v6:
  - clear the seq64 memory in user fence driver(Christian)
  - fix for the wptr va bo mapping(Christian)
  - move the fence_drv xa entry erase code from the interrupt handler
into user fence destroy function

Signed-off-by: Arunpravin Paneer Selvam 
Reviewed-by: Christian König 
Suggested-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/Makefile   |   3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |   6 +
 .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   | 257 ++
 .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h   |  69 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c |  12 +-
 .../gpu/drm/amd/include/amdgpu_userqueue.h|   5 +-
 6 files changed, 347 insertions(+), 5 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index a640bfa468ad..f8dd808bd71c 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -80,7 +80,8 @@ amdgpu-y += amdgpu_device.o amdgpu_doorbell_mgr.o 
amdgpu_kms.o \
amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
amdgpu_fw_attestation.o amdgpu_securedisplay.o \
amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o \
-   amdgpu_ring_mux.o amdgpu_xcp.o amdgpu_seq64.o amdgpu_aca.o
+   amdgpu_ring_mux.o amdgpu_xcp.o amdgpu_seq64.o amdgpu_aca.o \
+   amdgpu_userq_fence.o
 
 amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index acee1c279abb..844f7b5f90db 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -51,6 +51,7 @@
 #include "amdgpu_sched.h"
 #include "amdgpu_xgmi.h"
 #include "amdgpu_userqueue.h"
+#include "amdgpu_userq_fence.h"
 #include "../amdxcp/amdgpu_xcp_drv.h"
 
 /*
@@ -3012,6 +3013,10 @@ static int __init amdgpu_init(void)
if (r)
goto error_fence;
 
+   r = amdgpu_userq_fence_slab_init();
+   if (r)
+   goto error_fence;
+
DRM_INFO("amdgpu kernel modesetting enabled.\n");
amdgpu_register_atpx_handler();
amdgpu_acpi_detect();
@@ -3037,6 +3042,7 @@ static void __exit amdgpu_exit(void)
amdgpu_acpi_release();
amdgpu_sync_fini();
amdgpu_fence_slab_fini();
+   amdgpu_userq_fence_slab_fini();
mmu_notifier_synchronize();
amdgpu_xcp_drv_release();
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
new file mode 100644
index ..f7baea2c67ab
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2023 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute

[PATCH v9 4/5] drm/amdgpu: Implement userqueue signal/wait IOCTL

2024-04-28 Thread Arunpravin Paneer Selvam
This patch introduces new IOCTL for userqueue secure semaphore.

The signal IOCTL called from userspace application creates a drm
syncobj and array of bo GEM handles and passed in as parameter to
the driver to install the fence into it.

The wait IOCTL gets an array of drm syncobjs, finds the fences
attached to the drm syncobjs and obtain the array of
memory_address/fence_value combintion which are returned to
userspace.

v2: (Christian)
- Install fence into GEM BO object.
- Lock all BO's using the dma resv subsystem
- Reorder the sequence in signal IOCTL function.
- Get write pointer from the shadow wptr
- use userq_fence to fetch the va/value in wait IOCTL.

v3: (Christian)
- Use drm_exec helper for the proper BO drm reserve and avoid BO
  lock/unlock issues.
- fence/fence driver reference count logic for signal/wait IOCTLs.

v4: (Christian)
- Fixed the drm_exec calling sequence
- use dma_resv_for_each_fence_unlock if BO's are not locked
- Modified the fence_info array storing logic.

v5: (Christian)
- Keep fence_drv until wait queue execution.
- Add dma_fence_wait for other fences.
- Lock BO's using drm_exec as the number of fences in them could
  change.
- Install signaled fences as well into BO/Syncobj.
- Move Syncobj fence installation code after the drm_exec_prepare_array.
- Directly add dma_resv_usage_rw(args->bo_flags
- remove unnecessary dma_fence_put.

v6: (Christian)
- Add xarray stuff to store the fence_drv
- Implement a function to iterate over the xarray and drop
  the fence_drv references.
- Add drm_exec_until_all_locked() wrapper
- Add a check that if we haven't exceeded the user allocated num_fences
  before adding dma_fence to the fences array.

v7: (Christian)
- Use memdup_user() for kmalloc_array + copy_from_user
- Move the fence_drv references from the xarray into the newly created fence
  and drop the fence_drv references when we signal this fence.
- Move this locking of BOs before the "if (!wait_info->num_fences)",
  this way you need this code block only once.
- Merge the error handling code and the cleanup + return 0 code.
- Initializing the xa should probably be done in the userq code.
- Remove the userq back pointer stored in fence_drv.
- Pass xarray as parameter in amdgpu_userq_walk_and_drop_fence_drv()

v8: (Christian)
- Move fence_drv references must come before adding the fence to the list.
- Use xa_lock_irqsave_nested for nested spinlock operations.
- userq_mgr should be per fpriv and not one per device.
- Restructure the interrupt process code for the early exit of the loop.
- The reference acquired in the syncobj fence replace code needs to be
  kept around.
- Modify the dma_fence acquire placement in wait IOCTL.
- Move USERQ_BO_WRITE flag to UAPI header file.
- drop the fence drv reference after telling the hw to stop accessing it.
- Add multi sync object support to userq signal IOCTL.

Signed-off-by: Arunpravin Paneer Selvam 
Suggested-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |   2 +
 .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   | 454 +-
 .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h   |   5 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c |  29 +-
 .../gpu/drm/amd/include/amdgpu_userqueue.h|   1 +
 5 files changed, 484 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 844f7b5f90db..5892a4c1a92e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2918,6 +2918,8 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_OP, amdgpu_gem_op_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_USERQ, amdgpu_userq_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_SIGNAL, amdgpu_userq_signal_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
 };
 
 static const struct drm_driver amdgpu_kms_driver = {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index f7baea2c67ab..6fb75cc1d20c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 
+#include 
 #include 
 
 #include "amdgpu.h"
@@ -102,8 +103,11 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device 
*adev,
 
 void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver 
*fence_drv)
 {
+   struct amdgpu_userq_fence_driver *stored_fence_drv;
struct amdgpu_userq_fence *userq_fence, *tmp;
+   unsigned long index, flags;

[PATCH v9 3/5] drm/amdgpu: UAPI headers for userqueue Secure semaphore

2024-04-28 Thread Arunpravin Paneer Selvam
Add UAPI header support for userqueue Secure semaphore

v2: Worked on review comments from Christian for the following
modifications

- Add bo handles, bo flags and padding fields.
- Include value/va in a combined array.

v3: Worked on review comments from Christian

- Add num_fences field to obtain the number of objects required
  to allocate memory for userq_fence_info.
- Replace obj_handle name with syncobj_handle.
- Replace point name with syncobj_point.
- Replace count_handles name with num_syncobj_handles.
- Fix structure padding related issues.

v4: Worked on review comments from Christian
- Modify the bo flags description.

Signed-off-by: Alex Deucher 
Signed-off-by: Arunpravin Paneer Selvam 
Reviewed-by: Christian König 
---
 include/uapi/drm/amdgpu_drm.h | 115 ++
 1 file changed, 115 insertions(+)

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index def6874a588c..1217ae30a8d8 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -55,6 +55,8 @@ extern "C" {
 #define DRM_AMDGPU_FENCE_TO_HANDLE 0x14
 #define DRM_AMDGPU_SCHED   0x15
 #define DRM_AMDGPU_USERQ   0x16
+#define DRM_AMDGPU_USERQ_SIGNAL0x17
+#define DRM_AMDGPU_USERQ_WAIT  0x18
 
 #define DRM_IOCTL_AMDGPU_GEM_CREATEDRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
 #define DRM_IOCTL_AMDGPU_GEM_MMAP  DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
@@ -73,6 +75,8 @@ extern "C" {
 #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
 #define DRM_IOCTL_AMDGPU_SCHED DRM_IOW(DRM_COMMAND_BASE + 
DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
 #define DRM_IOCTL_AMDGPU_USERQ DRM_IOW(DRM_COMMAND_BASE + 
DRM_AMDGPU_USERQ, union drm_amdgpu_userq)
+#define DRM_IOCTL_AMDGPU_USERQ_SIGNAL  DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_USERQ_SIGNAL, struct drm_amdgpu_userq_signal)
+#define DRM_IOCTL_AMDGPU_USERQ_WAITDRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_USERQ_WAIT, struct drm_amdgpu_userq_wait)
 
 /**
  * DOC: memory domains
@@ -431,6 +435,117 @@ union drm_amdgpu_userq {
struct drm_amdgpu_userq_out out;
 };
 
+/* dma_resv usage flag */
+#define AMDGPU_USERQ_BO_WRITE  1
+
+/* userq signal/wait ioctl */
+struct drm_amdgpu_userq_signal {
+   /**
+* @queue_id: Queue handle used by the userq fence creation function
+* to retrieve the WPTR.
+*/
+   __u32   queue_id;
+   /**
+* @flags: flags to indicate special function for userq fence creation.
+* Unused for now.
+*/
+   __u32   flags;
+   /**
+* @syncobj_handles_array: An array of syncobj handles used by the 
userq fence
+* creation IOCTL to install the created dma_fence object which can be
+* utilized by userspace to explicitly synchronize GPU commands.
+*/
+   __u64   syncobj_handles_array;
+   /**
+* @num_syncobj_handles: A count that represents the number of syncobj 
handles in
+* @syncobj_handles_array.
+*/
+   __u64   num_syncobj_handles;
+   /**
+* @syncobj_point: A given point on the timeline to be signaled.
+* Unused for now.
+*/
+   __u64   syncobj_point;
+   /**
+* @bo_handles_array: An array of GEM BO handles used by the userq 
fence creation
+* IOCTL to install the created dma_fence object which can be utilized 
by
+* userspace to synchronize the BO usage between user processes.
+*/
+   __u64   bo_handles_array;
+   /**
+* @num_bo_handles: A count that represents the number of GEM BO 
handles in
+* @bo_handles_array.
+*/
+   __u32   num_bo_handles;
+   /**
+* @bo_flags: flags to indicate BOs synchronize for READ or WRITE
+*/
+   __u32   bo_flags;
+};
+
+struct drm_amdgpu_userq_fence_info {
+   /**
+* @va: A gpu address allocated for each queue which stores the
+* read pointer (RPTR) value.
+*/
+   __u64   va;
+   /**
+* @value: A 64 bit value represents the write pointer (WPTR) of the
+* queue commands which compared with the RPTR value to signal the
+* fences.
+*/
+   __u64   value;
+};
+
+struct drm_amdgpu_userq_wait {
+   /**
+* @waitq_id: Queue handle used to retrieve the queue information to 
store
+* the fence driver references in the wait user queue structure.
+*/
+   __u32   waitq_id;
+   /**
+* @flags: flags to specify special function for userq wait information.
+* Unused for now.
+*/
+   __u32   flags;
+   /**
+* @bo_wait_flags: flags to define the BOs for READ or WRITE to store 
the
+* matching fence wait info pair in @userq_fence_info

[PATCH v9 5/5] drm/amdgpu: Enable userq fence interrupt support

2024-04-28 Thread Arunpravin Paneer Selvam
Add support to handle the userqueue protected fence signal hardware
interrupt.

Create a xarray which maps the doorbell index to the fence driver address.
This would help to retrieve the fence driver information when an userq fence
interrupt is triggered. Firmware sends the doorbell offset value and
this info is compared with the queue's mqd doorbell offset value.
If they are same, we process the userq fence interrupt.

Signed-off-by: Arunpravin Paneer Selvam 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|  5 ++--
 .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |  6 +
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c| 25 ++-
 drivers/gpu/drm/amd/amdgpu/mes_v10_1.c|  5 
 drivers/gpu/drm/amd/amdgpu/mes_v11_0.c|  7 --
 6 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4ca14b02668b..2d5ef2e74c71 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1043,6 +1043,8 @@ struct amdgpu_device {
struct amdgpu_mqd   mqds[AMDGPU_HW_IP_NUM];
const struct amdgpu_userq_funcs *userq_funcs[AMDGPU_HW_IP_NUM];
 
+   struct xarray   userq_xa;
+
/* df */
struct amdgpu_dfdf;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7753a2e64d41..fd919105a181 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3982,6 +3982,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
spin_lock_init(&adev->audio_endpt_idx_lock);
spin_lock_init(&adev->mm_stats.lock);
 
+   xa_init_flags(&adev->userq_xa, XA_FLAGS_LOCK_IRQ);
+
INIT_LIST_HEAD(&adev->shadow_list);
mutex_init(&adev->shadow_list_lock);
 
@@ -4719,9 +4721,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool 
fbcon)
}
adev->in_suspend = false;
 
-   if (adev->enable_mes)
-   amdgpu_mes_self_test(adev);
-
if (amdgpu_acpi_smart_shift_update(dev, AMDGPU_SS_DEV_D0))
DRM_WARN("smart shift update failed\n");
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 6fb75cc1d20c..614953b0fc19 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -70,6 +70,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device 
*adev,
struct amdgpu_usermode_queue *userq)
 {
struct amdgpu_userq_fence_driver *fence_drv;
+   unsigned long flags;
int r;
 
fence_drv = kzalloc(sizeof(*fence_drv), GFP_KERNEL);
@@ -96,6 +97,11 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device 
*adev,
fence_drv->context = dma_fence_context_alloc(1);
get_task_comm(fence_drv->timeline_name, current);
 
+   xa_lock_irqsave(&adev->userq_xa, flags);
+   __xa_store(&adev->userq_xa, userq->doorbell_index,
+  fence_drv, GFP_KERNEL);
+   xa_unlock_irqrestore(&adev->userq_xa, flags);
+
userq->fence_drv = fence_drv;
 
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index a786e25432ae..d6cdca0a652f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -49,6 +49,7 @@
 #include "gfx_v11_0_3.h"
 #include "nbio_v4_3.h"
 #include "mes_v11_0.h"
+#include "amdgpu_userq_fence.h"
 
 #define GFX11_NUM_GFX_RINGS1
 #define GFX11_MEC_HPD_SIZE 2048
@@ -5939,25 +5940,25 @@ static int gfx_v11_0_eop_irq(struct amdgpu_device *adev,
 struct amdgpu_irq_src *source,
 struct amdgpu_iv_entry *entry)
 {
-   int i;
+   u32 doorbell_offset = entry->src_data[0];
u8 me_id, pipe_id, queue_id;
struct amdgpu_ring *ring;
-   uint32_t mes_queue_id = entry->src_data[0];
+   int i;
 
DRM_DEBUG("IH: CP EOP\n");
 
-   if (adev->enable_mes && (mes_queue_id & AMDGPU_FENCE_MES_QUEUE_FLAG)) {
-   struct amdgpu_mes_queue *queue;
+   if (adev->enable_mes && doorbell_offset) {
+   struct amdgpu_userq_fence_driver *fence_drv = NULL;
+   struct xarray *xa = &adev->userq_xa;
+   unsigned long index, flags;
 
-   mes_queue_id &= AMDGPU_FENCE_MES_QUEUE_ID_MASK;
+   xa_lock_irqsave(xa, flags);
+   xa_for_each(xa, index, fence_drv)
+   if (doorbell_offset == index)
+   break;
+   xa_unlock_irqrestore(xa, flags);
 
-   spin_lock(&adev->mes.queue_id_lock);
-   queue = idr_find(&adev->mes.queue_id_idr, mes_queue_id);
-   

[PATCH] drm/amdgpu/vpe: fix vpe dpm clk ratio setup failed

2024-04-28 Thread Peyton Lee
Some version of BIOS does not enable all clock levels,
resulting in high level clock frequency of 0.
The number of valid CLKs must be confirmed in advance.

Signed-off-by: Peyton Lee 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
index c23d97d34b7e..49881073ff58 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c
@@ -128,6 +128,7 @@ int amdgpu_vpe_configure_dpm(struct amdgpu_vpe *vpe)
struct dpm_clock *VPEClks;
struct dpm_clock *SOCClks;
uint32_t idx;
+   uint32_t vpeclk_enalbled_num = 0;
uint32_t pratio_vmax_vnorm = 0, pratio_vnorm_vmid = 0, 
pratio_vmid_vmin = 0;
uint16_t pratio_vmin_freq = 0, pratio_vmid_freq = 0, 
pratio_vnorm_freq = 0, pratio_vmax_freq = 0;
 
@@ -144,6 +145,14 @@ int amdgpu_vpe_configure_dpm(struct amdgpu_vpe *vpe)
SOCClks = clock_table.SocClocks;
VPEClks = clock_table.VPEClocks;
 
+   /* Comfirm enabled vpe clk num
+* Enabled VPE clocks are ordered from low to high in VPEClks
+* The highest valid clock index+1 is the number of VPEClks
+*/
+   for (idx = PP_SMU_NUM_VPECLK_DPM_LEVELS; idx && 
!vpeclk_enalbled_num; idx--)
+   if (VPEClks[idx-1].Freq)
+   vpeclk_enalbled_num = idx;
+
/* vpe dpm only cares 4 levels. */
for (idx = 0; idx < VPE_MAX_DPM_LEVEL; idx++) {
uint32_t soc_dpm_level;
@@ -155,8 +164,8 @@ int amdgpu_vpe_configure_dpm(struct amdgpu_vpe *vpe)
soc_dpm_level = (idx * 2) + 1;
 
/* clamp the max level */
-   if (soc_dpm_level > PP_SMU_NUM_VPECLK_DPM_LEVELS - 1)
-   soc_dpm_level = PP_SMU_NUM_VPECLK_DPM_LEVELS - 
1;
+   if (soc_dpm_level > vpeclk_enalbled_num - 1)
+   soc_dpm_level = vpeclk_enalbled_num - 1;
 
min_freq = (SOCClks[soc_dpm_level].Freq < 
VPEClks[soc_dpm_level].Freq) ?
   SOCClks[soc_dpm_level].Freq : 
VPEClks[soc_dpm_level].Freq;
-- 
2.34.1



[PATCH] drm/amd/display: Refactor construct_phy function in dc/link/link_factory.c

2024-04-28 Thread Srinivasan Shanmugam
This commit refactors the construct_phy function. The original function
was large and complex.

The following functions were created:

- initialize_link: Handles the initial setup of the link object.
- handle_connector_type: Sets the connector_signal and irq_source_hpd_rx
  based on the link_id.id.
- initialize_ddc_service: Initializes the ddc_service for the link.
- initialize_link_encoder: Initializes the link_encoder for the link.

Additionally, the error handling code that was originally in
construct_phy has been moved to the new functions. Each function now
returns a boolean value indicating whether the operation was successful.
If an error occurs, the construct_phy function jumps to the appropriate
label for error handling.

This refactoring reduces the size of the stack frame for each individual
function, fixes about the frame size being larger than 1024 bytes.

Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_factory.c: In function 
‘construct_phy’:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_factory.c:743:1: warning: 
the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]

Cc: Wenjing Liu 
Cc: Qingqing Zhuo 
Cc: Tom Chung 
Cc: Alvin Lee 
Cc: Rodrigo Siqueira 
Cc: Roman Li 
Cc: Hersen Wu 
Cc: Alex Hung 
Cc: Aurabindo Pillai 
Cc: Harry Wentland 
Signed-off-by: Srinivasan Shanmugam 
---
 .../drm/amd/display/dc/link/link_factory.c| 221 ++
 1 file changed, 122 insertions(+), 99 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/link_factory.c 
b/drivers/gpu/drm/amd/display/dc/link/link_factory.c
index cf22b8f28ba6..af373824db8c 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_factory.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_factory.c
@@ -448,73 +448,74 @@ static enum channel_id get_ddc_line(struct dc_link *link)
return channel;
 }
 
-static bool construct_phy(struct dc_link *link,
- const struct link_init_data *init_params)
+static bool initialize_link_encoder(struct dc_link *link,
+   struct dc_context *dc_ctx,
+   const struct dc_vbios_funcs *bp_funcs)
 {
-   uint8_t i;
-   struct ddc_service_init_data ddc_service_init_data = { 0 };
-   struct dc_context *dc_ctx = init_params->ctx;
struct encoder_init_data enc_init_data = { 0 };
-   struct panel_cntl_init_data panel_cntl_init_data = { 0 };
-   struct integrated_info info = { 0 };
-   struct dc_bios *bios = init_params->dc->ctx->dc_bios;
-   const struct dc_vbios_funcs *bp_funcs = bios->funcs;
-   struct bp_disp_connector_caps_info disp_connect_caps_info = { 0 };
 
-   DC_LOGGER_INIT(dc_ctx->logger);
+   enc_init_data.ctx = dc_ctx;
+   bp_funcs->get_src_obj(dc_ctx->dc_bios, link->link_id, 0, 
&enc_init_data.encoder);
+   enc_init_data.connector = link->link_id;
+   enc_init_data.channel = get_ddc_line(link);
+   enc_init_data.hpd_source = get_hpd_line(link);
 
-   link->irq_source_hpd = DC_IRQ_SOURCE_INVALID;
-   link->irq_source_hpd_rx = DC_IRQ_SOURCE_INVALID;
-   link->link_status.dpcd_caps = &link->dpcd_caps;
+   link->hpd_src = enc_init_data.hpd_source;
 
-   link->dc = init_params->dc;
-   link->ctx = dc_ctx;
-   link->link_index = init_params->link_index;
+   enc_init_data.transmitter = 
translate_encoder_to_transmitter(enc_init_data.encoder);
+   link->link_enc = link->dc->res_pool->funcs->link_enc_create(dc_ctx, 
&enc_init_data);
 
-   memset(&link->preferred_training_settings, 0,
-  sizeof(struct dc_link_training_overrides));
-   memset(&link->preferred_link_setting, 0,
-  sizeof(struct dc_link_settings));
-
-   link->link_id =
-   bios->funcs->get_connector_id(bios, 
init_params->connector_index);
+   DC_LOG_DC("BIOS object table - DP_IS_USB_C: %d",
+ link->link_enc->features.flags.bits.DP_IS_USB_C);
+   DC_LOG_DC("BIOS object table - IS_DP2_CAPABLE: %d",
+ link->link_enc->features.flags.bits.IS_DP2_CAPABLE);
 
-   link->ep_type = DISPLAY_ENDPOINT_PHY;
+   if (!link->link_enc) {
+   DC_ERROR("Failed to create link encoder!\n");
+   return false;
+   }
 
-   DC_LOG_DC("BIOS object table - link_id: %d", link->link_id.id);
+   /* Update link encoder tracking variables. These are used for the 
dynamic
+* assignment of link encoders to streams.
+*/
+   link->eng_id = link->link_enc->preferred_engine;
+   link->dc->res_pool->link_encoders[link->eng_id - ENGINE_ID_DIGA] = 
link->link_enc;
+   link->dc->res_pool->dig_link_enc_count++;
 
-   if (bios->funcs->get_disp_connector_caps_info) {
-   bios->funcs->get_disp_connector_caps_info(bios, link->link_id, 
&disp_connect_caps_info);
-   link->is_internal_display = 
disp_connect_caps_info.INTERNAL_DISPLAY;
- 

RE: [PATCH] drm/amd/pm: fix warning using uninitialized value of max_vid_step

2024-04-28 Thread Huang, Tim
[AMD Official Use Only - General]

> -Original Message-
> From: Jesse Zhang 
> Sent: Monday, April 29, 2024 10:29 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Koenig, Christian
> ; Huang, Tim ; Zhang,
> Jesse(Jie) ; Zhang, Jesse(Jie) 
> Subject: [PATCH] drm/amd/pm: fix warning using uninitialized value of
> max_vid_step
>
> Check the return of pp_atomfwctrl_get_Voltage_table_v4
> as it may fail to initialize max_vid_step
>
> Signed-off-by: Jesse Zhang 
> ---
>  drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> index b602059436a8..70c711cec897 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> @@ -2573,8 +2573,12 @@ static int vega10_init_smc_table(struct pp_hwmgr
> *hwmgr)
>   }
>   }
>
> - pp_atomfwctrl_get_voltage_table_v4(hwmgr, VOLTAGE_TYPE_VDDC,
> + result = pp_atomfwctrl_get_voltage_table_v4(hwmgr,
> VOLTAGE_TYPE_VDDC,
>   VOLTAGE_OBJ_SVID2,  &voltage_table);
> + PP_ASSERT_WITH_CODE(result < 0,

Hi jesse,

It should be PP_ASSERT_WITH_CODE(!result, right?

Tim
> + "Failed to get voltage tables!",
> + return result);
> +
>   pp_table->MaxVidStep = voltage_table.max_vid_step;
>
>   pp_table->GfxDpmVoltageMode =
> --
> 2.25.1