<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr">
<p style="margin-top:0;margin-bottom:0">Reviewed-by: Alex Deucher <alexander.deucher@amd.com><br>
</p>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Evan Quan <evan.quan@amd.com><br>
<b>Sent:</b> Monday, December 17, 2018 4:59:02 AM<br>
<b>To:</b> amd-gfx@lists.freedesktop.org<br>
<b>Cc:</b> Quan, Evan<br>
<b>Subject:</b> [PATCH] drm/amdgpu: correct the return value for error case</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">It should not return 0 for error case as '0' is actually<br>
a special value for index.<br>
<br>
Change-Id: Iced8b4427d4403f86826a7c8e3c9d1da3394246c<br>
Signed-off-by: Evan Quan <evan.quan@amd.com><br>
---<br>
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h  | 12 ++++++------<br>
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 15 +++++++++++++--<br>
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c   | 20 ++++++++++++--------<br>
 3 files changed, 31 insertions(+), 16 deletions(-)<br>
<br>
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h<br>
index 24dea17b4161..ee26181222ad 100644<br>
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h<br>
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h<br>
@@ -83,8 +83,8 @@ struct psp_funcs<br>
                                   enum AMDGPU_UCODE_ID ucode_type);<br>
         bool (*smu_reload_quirk)(struct psp_context *psp);<br>
         int (*mode1_reset)(struct psp_context *psp);<br>
-       uint64_t (*xgmi_get_node_id)(struct psp_context *psp);<br>
-       uint64_t (*xgmi_get_hive_id)(struct psp_context *psp);<br>
+       int (*xgmi_get_node_id)(struct psp_context *psp, uint64_t *node_id);<br>
+       int (*xgmi_get_hive_id)(struct psp_context *psp, uint64_t *hive_id);<br>
         int (*xgmi_get_topology_info)(struct psp_context *psp, int number_devices,<br>
                                       struct psp_xgmi_topology_info *topology);<br>
         int (*xgmi_set_topology_info)(struct psp_context *psp, int number_devices,<br>
@@ -216,10 +216,10 @@ struct tmz_clear_vpr {<br>
                 ((psp)->funcs->support_vmr_ring ? (psp)->funcs->support_vmr_ring((psp)) : false)<br>
 #define psp_mode1_reset(psp) \<br>
                 ((psp)->funcs->mode1_reset ? (psp)->funcs->mode1_reset((psp)) : false)<br>
-#define psp_xgmi_get_node_id(psp) \<br>
-               ((psp)->funcs->xgmi_get_node_id ? (psp)->funcs->xgmi_get_node_id((psp)) : 0)<br>
-#define psp_xgmi_get_hive_id(psp) \<br>
-               ((psp)->funcs->xgmi_get_hive_id ? (psp)->funcs->xgmi_get_hive_id((psp)) : 0)<br>
+#define psp_xgmi_get_node_id(psp, node_id) \<br>
+               ((psp)->funcs->xgmi_get_node_id ? (psp)->funcs->xgmi_get_node_id((psp), (node_id)) : -EINVAL)<br>
+#define psp_xgmi_get_hive_id(psp, hive_id) \<br>
+               ((psp)->funcs->xgmi_get_hive_id ? (psp)->funcs->xgmi_get_hive_id((psp), (hive_id)) : -EINVAL)<br>
 #define psp_xgmi_get_topology_info(psp, num_device, topology) \<br>
                 ((psp)->funcs->xgmi_get_topology_info ? \<br>
                 (psp)->funcs->xgmi_get_topology_info((psp), (num_device), (topology)) : -EINVAL)<br>
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c<br>
index 0b263a9857c6..8a8bc60cb6b4 100644<br>
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c<br>
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c<br>
@@ -97,8 +97,19 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)<br>
         if (!adev->gmc.xgmi.supported)<br>
                 return 0;<br>
 <br>
-       adev->gmc.xgmi.node_id = psp_xgmi_get_node_id(&adev->psp);<br>
-       adev->gmc.xgmi.hive_id = psp_xgmi_get_hive_id(&adev->psp);<br>
+       ret = psp_xgmi_get_node_id(&adev->psp, &adev->gmc.xgmi.node_id);<br>
+       if (ret) {<br>
+               dev_err(adev->dev,<br>
+                       "XGMI: Failed to get node id\n");<br>
+               return ret;<br>
+       }<br>
+<br>
+       ret = psp_xgmi_get_hive_id(&adev->psp, &adev->gmc.xgmi.hive_id);<br>
+       if (ret) {<br>
+               dev_err(adev->dev,<br>
+                       "XGMI: Failed to get hive id\n");<br>
+               return ret;<br>
+       }<br>
 <br>
         mutex_lock(&xgmi_mutex);<br>
         hive = amdgpu_get_xgmi_hive(adev);<br>
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c<br>
index 0a390a9544b5..d9b4a3aca3be 100644<br>
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c<br>
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c<br>
@@ -704,7 +704,7 @@ static int psp_v11_0_xgmi_set_topology_info(struct psp_context *psp,<br>
         return psp_xgmi_invoke(psp, TA_COMMAND_XGMI__SET_TOPOLOGY_INFO);<br>
 }<br>
 <br>
-static u64 psp_v11_0_xgmi_get_hive_id(struct psp_context *psp)<br>
+static int psp_v11_0_xgmi_get_hive_id(struct psp_context *psp, uint64_t *hive_id)<br>
 {<br>
         struct ta_xgmi_shared_memory *xgmi_cmd;<br>
         int ret;<br>
@@ -717,12 +717,14 @@ static u64 psp_v11_0_xgmi_get_hive_id(struct psp_context *psp)<br>
         /* Invoke xgmi ta to get hive id */<br>
         ret = psp_xgmi_invoke(psp, xgmi_cmd->cmd_id);<br>
         if (ret)<br>
-               return 0;<br>
-       else<br>
-               return xgmi_cmd->xgmi_out_message.get_hive_id.hive_id;<br>
+               return ret;<br>
+<br>
+       *hive_id = xgmi_cmd->xgmi_out_message.get_hive_id.hive_id;<br>
+<br>
+       return 0;<br>
 }<br>
 <br>
-static u64 psp_v11_0_xgmi_get_node_id(struct psp_context *psp)<br>
+static int psp_v11_0_xgmi_get_node_id(struct psp_context *psp, uint64_t *node_id)<br>
 {<br>
         struct ta_xgmi_shared_memory *xgmi_cmd;<br>
         int ret;<br>
@@ -735,9 +737,11 @@ static u64 psp_v11_0_xgmi_get_node_id(struct psp_context *psp)<br>
         /* Invoke xgmi ta to get the node id */<br>
         ret = psp_xgmi_invoke(psp, xgmi_cmd->cmd_id);<br>
         if (ret)<br>
-               return 0;<br>
-       else<br>
-               return xgmi_cmd->xgmi_out_message.get_node_id.node_id;<br>
+               return ret;<br>
+<br>
+       *node_id = xgmi_cmd->xgmi_out_message.get_node_id.node_id;<br>
+<br>
+       return 0;<br>
 }<br>
 <br>
 static const struct psp_funcs psp_v11_0_funcs = {<br>
-- <br>
2.20.1<br>
<br>
_______________________________________________<br>
amd-gfx mailing list<br>
amd-gfx@lists.freedesktop.org<br>
<a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><br>
</div>
</span></font></div>
</body>
</html>