On 6/13/25 10:27 PM, Lazar, Lijo wrote:
On 6/14/2025 3:11 AM, Mario Limonciello wrote:
From: Mario Limonciello <[email protected]>
commit 017fbb6690c22 ("drm/amdgpu/discovery: check ip_discovery fw file
available") added support for reading an amdgpu IP discovery bin file
for some specific products. If it's not found then it will fallback to
hardcoded values. However if it's not found there is also a lot of noise
about missing files and errors.
Adjust the error handling to decrease most messages to DEBUG and to show
at most one message to a user about the missing file at INFO level.
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4312
Fixes: 017fbb6690c22 ("drm/amdgpu/discovery: check ip_discovery fw file
available")
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index a0e9bf9b27108..8e4526a8c2600 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -321,10 +321,9 @@ static int amdgpu_discovery_read_binary_from_file(struct
amdgpu_device *adev,
const struct firmware *fw;
int r;
- r = request_firmware(&fw, fw_name, adev->dev);
+ r = firmware_request_nowarn(&fw, fw_name, adev->dev);
if (r) {
- dev_err(adev->dev, "can't load firmware \"%s\"\n",
- fw_name);
+ drm_info(&adev->ddev, "Optional firmware \"%s\" was not
found\n", fw_name);
return r;
}
@@ -459,16 +458,12 @@ static int amdgpu_discovery_init(struct amdgpu_device *adev)
/* Read from file if it is the preferred option */
fw_name = amdgpu_discovery_get_fw_name(adev);
if (fw_name != NULL) {
- dev_info(adev->dev, "use ip discovery information from file");
+ drm_dbg(&adev->ddev, "use ip discovery information from file");
r = amdgpu_discovery_read_binary_from_file(adev,
adev->mman.discovery_bin, fw_name);
-
- if (r) {
- dev_err(adev->dev, "failed to read ip discovery binary from
file\n");
- r = -EINVAL;
+ if (r)
goto out;
- }
-
} else {
+ drm_dbg(&adev->ddev, "use ip discovery information from
memory");
r = amdgpu_discovery_read_binary_from_mem(
adev, adev->mman.discovery_bin);
if (r)
@@ -1339,7 +1334,7 @@ static int amdgpu_discovery_reg_base_init(struct
amdgpu_device *adev)
r = amdgpu_discovery_init(adev);
if (r) {
- DRM_ERROR("amdgpu_discovery_init failed\n");
+ drm_warn(&adev->ddev, "%s failed: %d\n", __func__, r);
This indeed is an error. Rest of the changes are fine.
How about pushing this down into amdgpu_discovery_set_ip_blocks()?
I was thinking we can put it in the default case only so that the
fallback path doesn't make a lot of noise for vega/raven/arcturus/aldebaran.
Something like this:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index 3877b43d9f863..d5bd105de1b31 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -1333,10 +1333,8 @@ static int amdgpu_discovery_reg_base_init(struct
amdgpu_device *adev)
int r;
r = amdgpu_discovery_init(adev);
- if (r) {
- DRM_ERROR("amdgpu_discovery_init failed\n");
+ if (r)
return r;
- }
wafl_ver = 0;
adev->gfx.xcc_mask = 0;
@@ -2574,8 +2572,10 @@ int amdgpu_discovery_set_ip_blocks(struct
amdgpu_device *adev)
break;
default:
r = amdgpu_discovery_reg_base_init(adev);
- if (r)
- return -EINVAL;
+ if (r) {
+ drm_err(&adev->ddev, "discovery failed: %d\n", ret);
+ return r;
+ }
amdgpu_discovery_harvest_ip(adev);
amdgpu_discovery_get_gfx_info(adev);
Thanks,
Lijo
return r;
}
