Dear Stanley,
Thank you for your patch.
Am 21.03.22 um 06:45 schrieb Stanley.Yang:
Some nits:
Could you please remove the dot from the name:
$ git config --global user.name "Stanley Yang"
$ git commit --amend -s --author="Stanley Yang <Stanley.Yang@xxxxxxx>"
The prefix drm/amd/pm seems to be more common for changes in the file.
Instead of *add … check*, you can also just use the verb *check*:
drm/amd/pm: Check ASIC SMU support
It must check asic whether support smu
before call smu powerplay function, otherwise
it may cause null point on no support smu asic.
Please reflow for 75 characters per line. Also maybe write:
… it may cause a null pointer dereference on systems without an SMU ASIC.
(I am no native speaker myself though.)
On what device did you reproduce this null pointer dereference?
Change-Id: Ib86f3d4c88317b23eb1040b9ce1c5c8dcae42488
Without documenting the Gerrit instance, the Change-Id is of no use.
I guess this also should go to the stable series? Please add a Fixes tag
with the commit missing these checks.
Signed-off-by: Stanley.Yang <Stanley.Yang@xxxxxxx>
---
drivers/gpu/drm/amd/pm/amdgpu_dpm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index 89fbee568be4..c73fb73e9628 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -500,6 +500,9 @@ int amdgpu_dpm_send_hbm_bad_pages_num(struct amdgpu_device *adev, uint32_t size)
struct smu_context *smu = adev->powerplay.pp_handle;
int ret = 0;
+ if (!is_support_sw_smu(adev))
+ return -EOPNOTSUPP;
+
I wonder, why `struct smu_context *smu = adev->powerplay.pp_handle`
seems to work further up? Maybe the assignment should also be moved
after the newly introduced check?
mutex_lock(&adev->pm.mutex);
ret = smu_send_hbm_bad_pages_num(smu, size);
mutex_unlock(&adev->pm.mutex);
@@ -512,6 +515,9 @@ int amdgpu_dpm_send_hbm_bad_channel_flag(struct amdgpu_device *adev, uint32_t si
struct smu_context *smu = adev->powerplay.pp_handle;
int ret = 0;
+ if (!is_support_sw_smu(adev))
+ return -EOPNOTSUPP;
+
mutex_lock(&adev->pm.mutex);
ret = smu_send_hbm_bad_channel_flag(smu, size);
mutex_unlock(&adev->pm.mutex);
Kind regards,
Paul
