[PATCH 0/5] Random debugging quality-of-life improvements

[PATCH 0/5] Random debugging quality-of-life improvements

[Michał Mirosław]

This is a set of small improvements to debugging and error handling in
amdgpu driver. Please consider each patch independently.

Michał Mirosław (5):
  drm/amd/display/amdgpu_dm: show error names
  drm/amd/pm/smu7: make SMU message reports more readable
  drm/amd/pm/smu7: return error on message send failure
  drm/amd/amdgpu/cgs: Avoid redundant copying of firmware filename
  drm/amdgpu: debugfs: avoid extra EOLs in amdgpu_gem_info

 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c       | 54 +++++++++----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    |  3 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 40 +++++++-------
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 20 +++----
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c  | 12 +++--
 .../drm/amd/pm/powerplay/smumgr/smu7_smumgr.c | 25 ++++-----
 6 files changed, 76 insertions(+), 78 deletions(-)

-- 
2.47.3

[PATCH 1/5] drm/amd/display/amdgpu_dm: show error names

[Michał Mirosław]

Convert printk() args that log error numbers to log the names.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 40 +++++++++----------
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 20 +++++-----
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c  | 12 +++---
 3 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index b97ceabe6173..a95086c3969a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6802,7 +6802,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 
 	ret = drm_atomic_helper_check_modeset(dev, state);
 	if (ret) {
-		drm_dbg_atomic(dev, "drm_atomic_helper_check_modeset() failed\n");
+		drm_dbg_atomic(dev, "drm_atomic_helper_check_modeset() failed: %pe\n", ERR_PTR(ret));
 		goto fail;
 	}
 
@@ -6817,7 +6817,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 
 		new_crtc_state = drm_atomic_get_crtc_state(state, new_con_state->crtc);
 		if (IS_ERR(new_crtc_state)) {
-			drm_dbg_atomic(dev, "drm_atomic_get_crtc_state() failed\n");
+			drm_dbg_atomic(dev, "drm_atomic_get_crtc_state() failed: %pe\n", new_crtc_state);
 			ret = PTR_ERR(new_crtc_state);
 			goto fail;
 		}
@@ -6837,7 +6837,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 			if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {
 				ret = add_affected_mst_dsc_crtcs(state, crtc);
 				if (ret) {
-					drm_dbg_atomic(dev, "add_affected_mst_dsc_crtcs() failed\n");
+					drm_dbg_atomic(dev, "add_affected_mst_dsc_crtcs() failed: %pe\n", ERR_PTR(ret));
 					goto fail;
 				}
 			}
@@ -6854,7 +6854,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 
 		ret = amdgpu_dm_verify_lut_sizes(new_crtc_state);
 		if (ret) {
-			drm_dbg_atomic(dev, "amdgpu_dm_verify_lut_sizes() failed\n");
+			drm_dbg_atomic(dev, "amdgpu_dm_verify_lut_sizes() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 
@@ -6863,13 +6863,13 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 
 		ret = drm_atomic_add_affected_connectors(state, crtc);
 		if (ret) {
-			drm_dbg_atomic(dev, "drm_atomic_add_affected_connectors() failed\n");
+			drm_dbg_atomic(dev, "drm_atomic_add_affected_connectors() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 
 		ret = drm_atomic_add_affected_planes(state, crtc);
 		if (ret) {
-			drm_dbg_atomic(dev, "drm_atomic_add_affected_planes() failed\n");
+			drm_dbg_atomic(dev, "drm_atomic_add_affected_planes() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 
@@ -6908,7 +6908,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 
 			if (IS_ERR(new_plane_state)) {
 				ret = PTR_ERR(new_plane_state);
-				drm_dbg_atomic(dev, "new_plane_state is BAD\n");
+				drm_dbg_atomic(dev, "new_plane_state is BAD: %pe\n", new_plane_state);
 				goto fail;
 			}
 		}
@@ -6922,7 +6922,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 	 */
 	ret = drm_atomic_normalize_zpos(dev, state);
 	if (ret) {
-		drm_dbg(dev, "drm_atomic_normalize_zpos() failed\n");
+		drm_dbg(dev, "drm_atomic_normalize_zpos() failed: %pe\n", ERR_PTR(ret));
 		goto fail;
 	}
 
@@ -6936,7 +6936,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 		ret = dm_crtc_get_cursor_mode(adev, state, dm_new_crtc_state,
 					      &dm_new_crtc_state->cursor_mode);
 		if (ret) {
-			drm_dbg(dev, "Failed to determine cursor mode\n");
+			drm_dbg(dev, "Failed to determine cursor mode: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 
@@ -6968,7 +6968,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 					    &lock_and_validation_needed,
 					    &is_top_most_overlay);
 		if (ret) {
-			drm_dbg_atomic(dev, "dm_update_plane_state() failed\n");
+			drm_dbg_atomic(dev, "dm_update_plane_state() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 	}
@@ -6981,7 +6981,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 					   false,
 					   &lock_and_validation_needed);
 		if (ret) {
-			drm_dbg_atomic(dev, "DISABLE: dm_update_crtc_state() failed\n");
+			drm_dbg_atomic(dev, "DISABLE: dm_update_crtc_state() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 	}
@@ -6994,7 +6994,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 					   true,
 					   &lock_and_validation_needed);
 		if (ret) {
-			drm_dbg_atomic(dev, "ENABLE: dm_update_crtc_state() failed\n");
+			drm_dbg_atomic(dev, "ENABLE: dm_update_crtc_state() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 	}
@@ -7008,7 +7008,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 					    &lock_and_validation_needed,
 					    &is_top_most_overlay);
 		if (ret) {
-			drm_dbg_atomic(dev, "dm_update_plane_state() failed\n");
+			drm_dbg_atomic(dev, "dm_update_plane_state() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 	}
@@ -7024,7 +7024,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 	/* Run this here since we want to validate the streams we created */
 	ret = drm_atomic_helper_check_planes(dev, state);
 	if (ret) {
-		drm_dbg_atomic(dev, "drm_atomic_helper_check_planes() failed\n");
+		drm_dbg_atomic(dev, "drm_atomic_helper_check_planes() failed: %pe\n", ERR_PTR(ret));
 		goto fail;
 	}
 
@@ -7162,13 +7162,13 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 	if (lock_and_validation_needed) {
 		ret = dm_atomic_get_state(state, &dm_state);
 		if (ret) {
-			drm_dbg_atomic(dev, "dm_atomic_get_state() failed\n");
+			drm_dbg_atomic(dev, "dm_atomic_get_state() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 
 		ret = do_aquire_global_lock(dev, state);
 		if (ret) {
-			drm_dbg_atomic(dev, "do_aquire_global_lock() failed\n");
+			drm_dbg_atomic(dev, "do_aquire_global_lock() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 
@@ -7176,7 +7176,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 		if (dc_resource_is_dsc_encoding_supported(dc)) {
 			ret = compute_mst_dsc_configs_for_state(state, dm_state->context, vars);
 			if (ret) {
-				drm_dbg_atomic(dev, "MST_DSC compute_mst_dsc_configs_for_state() failed\n");
+				drm_dbg_atomic(dev, "MST_DSC compute_mst_dsc_configs_for_state() failed: %pe\n", ERR_PTR(ret));
 				ret = -EINVAL;
 				goto fail;
 			}
@@ -7185,7 +7185,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 
 		ret = dm_update_mst_vcpi_slots_for_dsc(state, dm_state->context, vars);
 		if (ret) {
-			drm_dbg_atomic(dev, "dm_update_mst_vcpi_slots_for_dsc() failed\n");
+			drm_dbg_atomic(dev, "dm_update_mst_vcpi_slots_for_dsc() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 
@@ -7197,7 +7197,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 		 */
 		ret = drm_dp_mst_atomic_check(state);
 		if (ret) {
-			drm_dbg_atomic(dev, "MST drm_dp_mst_atomic_check() failed\n");
+			drm_dbg_atomic(dev, "MST drm_dp_mst_atomic_check() failed: %pe\n", ERR_PTR(ret));
 			goto fail;
 		}
 		status = dc_validate_global_state(dc, dm_state->context, DC_VALIDATE_MODE_ONLY);
@@ -7286,7 +7286,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 	else if (ret == -EINTR || ret == -EAGAIN || ret == -ERESTARTSYS)
 		drm_dbg_atomic(dev, "Atomic check stopped due to signal.\n");
 	else
-		drm_dbg_atomic(dev, "Atomic check failed with err: %d\n", ret);
+		drm_dbg_atomic(dev, "Atomic check failed: %pe\n", ERR_PTR(ret));
 
 	trace_amdgpu_dm_atomic_check_finish(state, ret);
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 204b4641f07d..a12da0b60065 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -374,7 +374,7 @@ STATIC_IFN_KUNIT int amdgpu_dm_plane_fill_gfx9_plane_attributes_from_modifiers(s
 
 	ret = amdgpu_dm_plane_validate_dcc(adev, format, rotation, tiling_info, dcc, address, plane_size);
 	if (ret)
-		drm_dbg_kms(adev_to_drm(adev), "amdgpu_dm_plane_validate_dcc: returned error: %d\n", ret);
+		drm_dbg_kms(adev_to_drm(adev), "amdgpu_dm_plane_validate_dcc: returned error: %pe\n", ERR_PTR(ret));
 
 	return ret;
 }
@@ -415,7 +415,7 @@ STATIC_IFN_KUNIT int amdgpu_dm_plane_fill_gfx12_plane_attributes_from_modifiers(
 	/* TODO: This seems wrong because there is no DCC plane on GFX12. */
 	ret = amdgpu_dm_plane_validate_dcc(adev, format, rotation, tiling_info, dcc, address, plane_size);
 	if (ret)
-		drm_dbg_kms(adev_to_drm(adev), "amdgpu_dm_plane_validate_dcc: returned error: %d\n", ret);
+		drm_dbg_kms(adev_to_drm(adev), "amdgpu_dm_plane_validate_dcc: returned: %pe\n", ERR_PTR(ret));
 
 	return ret;
 }
@@ -975,13 +975,15 @@ static int amdgpu_dm_plane_helper_prepare_fb(struct drm_plane *plane,
 	adev = amdgpu_ttm_adev(rbo->tbo.bdev);
 	r = amdgpu_bo_reserve(rbo, true);
 	if (r) {
-		drm_err(adev_to_drm(adev), "fail to reserve bo (%d)\n", r);
+		drm_err(adev_to_drm(adev), "fail to reserve bo: %pe\n", ERR_PTR(r));
 		return r;
 	}
 
 	r = dma_resv_reserve_fences(rbo->tbo.base.resv, TTM_NUM_MOVE_FENCES);
-	if (r)
+	if (r) {
+		drm_err(adev_to_drm(adev), "reserving fence slot failed: %pe\n", ERR_PTR(r));
 		goto error_unlock;
+	}
 
 	if (plane->type != DRM_PLANE_TYPE_CURSOR)
 		domain = amdgpu_display_supported_domains(adev, rbo->flags);
@@ -992,13 +994,13 @@ static int amdgpu_dm_plane_helper_prepare_fb(struct drm_plane *plane,
 	r = amdgpu_bo_pin(rbo, domain);
 	if (unlikely(r != 0)) {
 		if (r != -ERESTARTSYS)
-			DRM_ERROR("Failed to pin framebuffer with error %d\n", r);
+			DRM_ERROR("Failed to pin framebuffer: %pe\n", ERR_PTR(r));
 		goto error_unlock;
 	}
 
 	r = amdgpu_ttm_alloc_gart(&rbo->tbo);
 	if (unlikely(r != 0)) {
-		DRM_ERROR("%p bind failed\n", rbo);
+		DRM_ERROR("%p bind failed: %pe\n", rbo, ERR_PTR(r));
 		goto error_unpin;
 	}
 
@@ -1058,7 +1060,7 @@ static void amdgpu_dm_plane_helper_cleanup_fb(struct drm_plane *plane,
 	rbo = gem_to_amdgpu_bo(old_state->fb->obj[0]);
 	r = amdgpu_bo_reserve(rbo, false);
 	if (unlikely(r)) {
-		DRM_ERROR("failed to reserve rbo before unpin\n");
+		DRM_ERROR("failed to reserve rbo before unpin: %pe\n", ERR_PTR(r));
 		return;
 	}
 
@@ -1861,8 +1863,8 @@ dm_plane_init_colorops(struct drm_plane *plane)
 	if (dc->ctx->dce_version >= DCN_VERSION_3_0) {
 		ret = amdgpu_dm_initialize_default_pipeline(plane, &pipelines[len]);
 		if (ret) {
-			drm_err(plane->dev, "Failed to create color pipeline for plane %d: %d\n",
-				plane->base.id, ret);
+			drm_err(plane->dev, "Failed to create color pipeline for plane %d: %pe\n",
+				plane->base.id, ERR_PTR(ret));
 			goto out;
 		}
 		len++;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
index 0bf82e46f773..a69657213ce0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c
@@ -107,13 +107,15 @@ static int amdgpu_dm_wb_prepare_job(struct drm_writeback_connector *wb_connector
 
 	r = amdgpu_bo_reserve(rbo, true);
 	if (r) {
-		drm_err(adev_to_drm(adev), "fail to reserve bo (%d)\n", r);
+		drm_err(adev_to_drm(adev), "fail to reserve bo: %pe\n", ERR_PTR(r));
 		return r;
 	}
 
 	r = dma_resv_reserve_fences(rbo->tbo.base.resv, TTM_NUM_MOVE_FENCES);
-	if (r)
+	if (r) {
+		drm_err(adev_to_drm(adev), "reserving fence slot failed: %pe\n", ERR_PTR(r));
 		goto error_unlock;
+	}
 
 	domain = amdgpu_display_supported_domains(adev, rbo->flags);
 
@@ -121,13 +123,13 @@ static int amdgpu_dm_wb_prepare_job(struct drm_writeback_connector *wb_connector
 	r = amdgpu_bo_pin(rbo, domain);
 	if (unlikely(r != 0)) {
 		if (r != -ERESTARTSYS)
-			DRM_ERROR("Failed to pin framebuffer with error %d\n", r);
+			DRM_ERROR("Failed to pin framebuffer: %pe\n", ERR_PTR(r));
 		goto error_unlock;
 	}
 
 	r = amdgpu_ttm_alloc_gart(&rbo->tbo);
 	if (unlikely(r != 0)) {
-		DRM_ERROR("%p bind failed\n", rbo);
+		DRM_ERROR("%p bind failed: %pe\n", rbo, ERR_PTR(r));
 		goto error_unpin;
 	}
 
@@ -159,7 +161,7 @@ static void amdgpu_dm_wb_cleanup_job(struct drm_writeback_connector *connector,
 	rbo = gem_to_amdgpu_bo(job->fb->obj[0]);
 	r = amdgpu_bo_reserve(rbo, false);
 	if (unlikely(r)) {
-		DRM_ERROR("failed to reserve rbo before unpin\n");
+		DRM_ERROR("failed to reserve rbo before unpin: %pe\n", ERR_PTR(r));
 		return;
 	}
 
-- 
2.47.3

[PATCH 3/5] drm/amd/pm/smu7: return error on message send failure

[Michał Mirosław]

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 .../gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c    | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c b/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c
index ffe3ce87d907..2e6dfe864306 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c
@@ -141,12 +141,16 @@ int smu7_send_msg_to_smc(struct pp_hwmgr *hwmgr, uint16_t msg)
 
 	ret = PHM_READ_FIELD(hwmgr->device, SMC_RESP_0, SMC_RESP);
 
-	if (ret == 0xFE)
+	switch (ret) {
+	case 1:
+		return 0;
+	case 0xFE:
 		dev_dbg(adev->dev, "SMU message %#x was not supported\n", msg);
-	else if (ret != 1)
+		return -EOPNOTSUPP;
+	default:
 		dev_info(adev->dev, "SMU message %#x failed: response is %d\n", msg, ret);
-
-	return 0;
+		return ret != 0xFFFF ? -EIO : -ENXIO;
+	}
 }
 
 int smu7_send_msg_to_smc_with_parameter(struct pp_hwmgr *hwmgr, uint16_t msg, uint32_t parameter)
-- 
2.47.3

Re: [PATCH 3/5] drm/amd/pm/smu7: return error on message send failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Changing `smu7_send_msg_to_smc` to return error codes (like `-EOPNOTSUPP`) breaks caller contracts across the powerplay subsystem, leading to severe initialization failures and incomplete hardware teardowns.
--

commit 69f2b8c2068f5f9f311a575e997b86049cf9f826
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Subject: drm/amd/pm/smu7: return error on message send failure

This commit changes smu7_send_msg_to_smc() to return standard error codes
like -EOPNOTSUPP and -EIO when the SMU message fails or is unsupported.
Previously, the function always returned 0 regardless of the SMU response.

> diff --git a/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c b/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c
> index ffe3ce87d9073..2e6dfe864306c 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c
> @@ -141,12 +141,16 @@ int smu7_send_msg_to_smc(struct pp_hwmgr *hwmgr, uint16_t msg)
>  
>  	ret = PHM_READ_FIELD(hwmgr->device, SMC_RESP_0, SMC_RESP);
>  
> -	if (ret == 0xFE)
> +	switch (ret) {
> +	case 1:
> +		return 0;
> +	case 0xFE:
>  		dev_dbg(adev->dev, "SMU message %#x was not supported\n", msg);
> -	else if (ret != 1)
> +		return -EOPNOTSUPP;

[Severity: High]
Does returning -EOPNOTSUPP here break caller contracts across the powerplay
subsystem?

Many callers appear to rely on the previous behavior where unsupported
messages (0xFE) still returned 0. For example, in smu7_stop_dpm():

drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c:smu7_stop_dpm() {
        ...
        PP_ASSERT_WITH_CODE(
                (smum_send_msg_to_smc(hwmgr,
                                      PPSMC_MSG_PCIeDPM_Disable,
                                      NULL) == 0),
                "Failed to disable pcie DPM during DPM Stop Function!",
                return -EINVAL);
        ...
}

If the hardware doesn't support PPSMC_MSG_PCIeDPM_Disable, the function now
returns -EOPNOTSUPP. Will this cause smu7_stop_dpm() to abort early and
skip critical teardown steps like smu7_disable_sclk_mclk_dpm()?

Similarly, in fiji_thermal_setup_fan_table(), an unsupported
PPSMC_MSG_SetFanMinPwm message will cause the return value to be non-zero,
which might mistakenly disable microcode fan control entirely.

> +	default:
>  		dev_info(adev->dev, "SMU message %#x failed: response is %d\n", msg, ret);
> -
> -	return 0;
> +		return ret != 0xFFFF ? -EIO : -ENXIO;
> +	}
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783009338.git.mirq-linux@rere.qmqm.pl?part=3

[PATCH 2/5] drm/amd/pm/smu7: make SMU message reports more readable

[Michał Mirosław]

Make the lines greppable by removing an extra EOL and report the
status only once - after the reply is detected.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 .../gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c   | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c b/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c
index 0d4cbe4113a0..ffe3ce87d907 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/smumgr/smu7_smumgr.c
@@ -134,14 +134,6 @@ int smu7_send_msg_to_smc(struct pp_hwmgr *hwmgr, uint16_t msg)
 
 	PHM_WAIT_FIELD_UNEQUAL(hwmgr, SMC_RESP_0, SMC_RESP, 0);
 
-	ret = PHM_READ_FIELD(hwmgr->device, SMC_RESP_0, SMC_RESP);
-
-	if (ret == 0xFE)
-		dev_dbg(adev->dev, "last message was not supported\n");
-	else if (ret != 1)
-		dev_info(adev->dev,
-			"\nlast message was failed ret is %d\n", ret);
-
 	cgs_write_register(hwmgr->device, mmSMC_RESP_0, 0);
 	cgs_write_register(hwmgr->device, mmSMC_MESSAGE_0, msg);
 
@@ -150,10 +142,9 @@ int smu7_send_msg_to_smc(struct pp_hwmgr *hwmgr, uint16_t msg)
 	ret = PHM_READ_FIELD(hwmgr->device, SMC_RESP_0, SMC_RESP);
 
 	if (ret == 0xFE)
-		dev_dbg(adev->dev, "message %x was not supported\n", msg);
+		dev_dbg(adev->dev, "SMU message %#x was not supported\n", msg);
 	else if (ret != 1)
-		dev_dbg(adev->dev,
-			"failed to send message %x ret is %d \n",  msg, ret);
+		dev_info(adev->dev, "SMU message %#x failed: response is %d\n", msg, ret);
 
 	return 0;
 }
-- 
2.47.3

[PATCH 4/5] drm/amd/amdgpu/cgs: Avoid redundant copying of firmware filename

[Michał Mirosław]

While at it, remove redundant error message - request_firmware() will
log a failure anyway.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 54 ++++++++++++-------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index 09c8942c22d3..a43cd4980d44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -241,7 +241,7 @@ static int amdgpu_cgs_get_firmware_info(struct cgs_device *cgs_device,
 		info->fw_version = amdgpu_get_firmware_version(cgs_device, type);
 		info->feature_version = (uint16_t)le32_to_cpu(header->ucode_feature_version);
 	} else {
-		char fw_name[30] = {0};
+		const char *fw_name = NULL;
 		int err = 0;
 		uint32_t ucode_size;
 		uint32_t ucode_start_address;
@@ -257,17 +257,17 @@ static int amdgpu_cgs_get_firmware_info(struct cgs_device *cgs_device,
 					(adev->pdev->revision == 0x81) ||
 					(adev->pdev->device == 0x665f)) {
 					info->is_kicker = true;
-					strscpy(fw_name, "amdgpu/bonaire_k_smc.bin");
+					fw_name = "bonaire_k_smc.bin";
 				} else {
-					strscpy(fw_name, "amdgpu/bonaire_smc.bin");
+					fw_name = "bonaire_smc.bin";
 				}
 				break;
 			case CHIP_HAWAII:
 				if (adev->pdev->revision == 0x80) {
 					info->is_kicker = true;
-					strscpy(fw_name, "amdgpu/hawaii_k_smc.bin");
+					fw_name = "hawaii_k_smc.bin";
 				} else {
-					strscpy(fw_name, "amdgpu/hawaii_smc.bin");
+					fw_name = "hawaii_smc.bin";
 				}
 				break;
 			case CHIP_TOPAZ:
@@ -277,76 +277,76 @@ static int amdgpu_cgs_get_firmware_info(struct cgs_device *cgs_device,
 				    ((adev->pdev->device == 0x6900) && (adev->pdev->revision == 0xD1)) ||
 				    ((adev->pdev->device == 0x6900) && (adev->pdev->revision == 0xD3))) {
 					info->is_kicker = true;
-					strscpy(fw_name, "amdgpu/topaz_k_smc.bin");
+					fw_name = "topaz_k_smc.bin";
 				} else
-					strscpy(fw_name, "amdgpu/topaz_smc.bin");
+					fw_name = "topaz_smc.bin";
 				break;
 			case CHIP_TONGA:
 				if (((adev->pdev->device == 0x6939) && (adev->pdev->revision == 0xf1)) ||
 				    ((adev->pdev->device == 0x6938) && (adev->pdev->revision == 0xf1))) {
 					info->is_kicker = true;
-					strscpy(fw_name, "amdgpu/tonga_k_smc.bin");
+					fw_name = "tonga_k_smc.bin";
 				} else
-					strscpy(fw_name, "amdgpu/tonga_smc.bin");
+					fw_name = "tonga_smc.bin";
 				break;
 			case CHIP_FIJI:
-				strscpy(fw_name, "amdgpu/fiji_smc.bin");
+				fw_name = "fiji_smc.bin";
 				break;
 			case CHIP_POLARIS11:
 				if (type == CGS_UCODE_ID_SMU) {
 					if (ASICID_IS_P21(adev->pdev->device, adev->pdev->revision)) {
 						info->is_kicker = true;
-						strscpy(fw_name, "amdgpu/polaris11_k_smc.bin");
+						fw_name = "polaris11_k_smc.bin";
 					} else if (ASICID_IS_P31(adev->pdev->device, adev->pdev->revision)) {
 						info->is_kicker = true;
-						strscpy(fw_name, "amdgpu/polaris11_k2_smc.bin");
+						fw_name = "polaris11_k2_smc.bin";
 					} else {
-						strscpy(fw_name, "amdgpu/polaris11_smc.bin");
+						fw_name = "polaris11_smc.bin";
 					}
 				} else if (type == CGS_UCODE_ID_SMU_SK) {
-					strscpy(fw_name, "amdgpu/polaris11_smc_sk.bin");
+					fw_name = "polaris11_smc_sk.bin";
 				}
 				break;
 			case CHIP_POLARIS10:
 				if (type == CGS_UCODE_ID_SMU) {
 					if (ASICID_IS_P20(adev->pdev->device, adev->pdev->revision)) {
 						info->is_kicker = true;
-						strscpy(fw_name, "amdgpu/polaris10_k_smc.bin");
+						fw_name = "polaris10_k_smc.bin";
 					} else if (ASICID_IS_P30(adev->pdev->device, adev->pdev->revision)) {
 						info->is_kicker = true;
-						strscpy(fw_name, "amdgpu/polaris10_k2_smc.bin");
+						fw_name = "polaris10_k2_smc.bin";
 					} else {
-						strscpy(fw_name, "amdgpu/polaris10_smc.bin");
+						fw_name = "polaris10_smc.bin";
 					}
 				} else if (type == CGS_UCODE_ID_SMU_SK) {
-					strscpy(fw_name, "amdgpu/polaris10_smc_sk.bin");
+					fw_name = "polaris10_smc_sk.bin";
 				}
 				break;
 			case CHIP_POLARIS12:
 				if (ASICID_IS_P23(adev->pdev->device, adev->pdev->revision)) {
 					info->is_kicker = true;
-					strscpy(fw_name, "amdgpu/polaris12_k_smc.bin");
+					fw_name = "polaris12_k_smc.bin";
 				} else {
-					strscpy(fw_name, "amdgpu/polaris12_smc.bin");
+					fw_name = "polaris12_smc.bin";
 				}
 				break;
 			case CHIP_VEGAM:
-				strscpy(fw_name, "amdgpu/vegam_smc.bin");
+				fw_name = "vegam_smc.bin";
 				break;
 			case CHIP_VEGA10:
 				if ((adev->pdev->device == 0x687f) &&
 					((adev->pdev->revision == 0xc0) ||
 					(adev->pdev->revision == 0xc1) ||
 					(adev->pdev->revision == 0xc3)))
-					strscpy(fw_name, "amdgpu/vega10_acg_smc.bin");
+					fw_name = "vega10_acg_smc.bin";
 				else
-					strscpy(fw_name, "amdgpu/vega10_smc.bin");
+					fw_name = "vega10_smc.bin";
 				break;
 			case CHIP_VEGA12:
-				strscpy(fw_name, "amdgpu/vega12_smc.bin");
+				fw_name = "vega12_smc.bin";
 				break;
 			case CHIP_VEGA20:
-				strscpy(fw_name, "amdgpu/vega20_smc.bin");
+				fw_name = "vega20_smc.bin";
 				break;
 			default:
 				drm_err(adev_to_drm(adev), "SMC firmware not supported\n");
@@ -355,10 +355,8 @@ static int amdgpu_cgs_get_firmware_info(struct cgs_device *cgs_device,
 
 			err = amdgpu_ucode_request(adev, &adev->pm.fw,
 						   AMDGPU_UCODE_REQUIRED,
-						   "%s", fw_name);
+						   "amdgpu/%s", fw_name);
 			if (err) {
-				drm_err(adev_to_drm(adev),
-					"Failed to load firmware \"%s\"\n", fw_name);
 				amdgpu_ucode_release(&adev->pm.fw);
 				return err;
 			}
-- 
2.47.3

[PATCH 5/5] drm/amdgpu: debugfs: avoid extra EOLs in amdgpu_gem_info

[Michał Mirosław]

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 4dd7c712b8c3..9ee894e0dbe2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1696,8 +1696,9 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
 	if (dma_resv_trylock(bo->tbo.base.resv)) {
 		dma_resv_describe(bo->tbo.base.resv, m);
 		dma_resv_unlock(bo->tbo.base.resv);
+	} else {
+		seq_puts(m, "\n");
 	}
-	seq_puts(m, "\n");
 
 	return size;
 }
-- 
2.47.3

Re: [PATCH 5/5] drm/amdgpu: debugfs: avoid extra EOLs in amdgpu_gem_info

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing newline in debugfs output for buffer objects without fences.
--

commit 642a225a01d32a7d60ac3ca80f3eb6f52693b3f4
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>

drm/amdgpu: debugfs: avoid extra EOLs in amdgpu_gem_info

This commit aims to avoid printing extra newlines in the amdgpu_gem_info
debugfs output. It does this by moving the newline into the lock failure
path when attempting to dump buffer object fences.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1696,8 +1696,9 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
>  	if (dma_resv_trylock(bo->tbo.base.resv)) {
>  		dma_resv_describe(bo->tbo.base.resv, m);
>  		dma_resv_unlock(bo->tbo.base.resv);
> +	} else {
> +		seq_puts(m, "\n");
>  	}
> -	seq_puts(m, "\n");
>  
>  	return size;
>  }

[Severity: Medium]
Will this cause garbled debugfs output for buffer objects that do not have
any fences attached?

When dma_resv_trylock() succeeds, amdgpu_bo_print_info() calls
dma_resv_describe(). Looking at dma_resv_describe() in
drivers/dma-buf/dma-resv.c:

void dma_resv_describe(struct dma_resv *obj, struct seq_file *seq)
{
	...
	dma_resv_for_each_fence(&cursor, obj, DMA_RESV_USAGE_READ, fence) {
		seq_printf(seq, "\t%s fence:", ...);
		dma_fence_describe(fence, seq);
	}
}

If there are no fences, dma_resv_describe() iterates zero times and prints
nothing. Because the unconditional newline was moved to the lock-failure
branch, the success path for a fenceless buffer object now prints nothing
at all.

This means the next item dumped in the debugfs file will be concatenated onto
the same line, breaking readability and parsing tools. Should a newline still
be printed when the lock succeeds but no fences are present?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783009338.git.mirq-linux@rere.qmqm.pl?part=5