[Bug report] Possible wrong condition
Hello,
It seems there is some dead or not-needed code. Either the if condition
isn't needed or condition is wrong. As this
greater-than-or-equal-to-zero comparison of an unsigned value is always
true. "version_minor >= 0". Please have a look at it.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
index 012b72d00e04..be9a6aad8541 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
@@ -526,6 +526,8 @@ int amdgpu_gfx_rlc_init_microcode(struct
amdgpu_device *adev,
if (version_major == 2 && version_minor == 1)
adev->gfx.rlc.is_rlc_v2_1 = true;
+ // The following condition is always true as version_minor is
unsigned.
+ // Why is this condition needed at all?
if (version_minor >= 0) {
err = amdgpu_gfx_rlc_init_microcode_v2_0(adev);
if (err) {
--
Muhammad Usama Anjum
[PATCH] drm/amdgpu: remove dead code
The less than zero comparison of unsigned variable "value" is never
true. Remove dead code.
Fixes: c3ed0e72c872 ("drm/amdgpu: added a sysfs interface for thermal
throttling")
Signed-off-by: Muhammad Usama Anjum
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index f212cae0353f..0ffe351c1a1d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -1738,7 +1738,7 @@ static ssize_t amdgpu_set_apu_thermal_cap(struct device
*dev,
if (ret)
return ret;
- if (value < 0 || value > 100) {
+ if (value > 100) {
dev_err(dev, "Invalid argument !\n");
return -EINVAL;
}
--
2.39.2
[Bug Report] Warning from __flush_work() on next-20241126
Hi, We are getting this warning on x86_64 and i386 targets: [8.677157] amdgpu :03:00.0: [drm:amdgpu_ib_ring_tests] *ERROR* IB test failed on sdma0 (-110). [8.698661] [ cut here ] [8.703310] WARNING: CPU: 1 PID: 49 at kernel/workqueue.c:4192 __flush_work+0xb8/0xd0 [8.711206] Modules linked in: [8.714285] CPU: 1 UID: 0 PID: 49 Comm: kworker/1:1 Tainted: GW 6.12.0-next-20241126 #1 [8.723790] Tainted: [W]=WARN [8.726781] Hardware name: Google Shuboz/Shuboz, BIOS Google_Shuboz.13434.780.2022_10_13_1418 09/12/2022 [8.736273] Workqueue: events amdgpu_device_delayed_init_work_handler [8.742768] EIP: __flush_work+0xb8/0xd0 [8.746632] Code: 00 00 f3 90 8d 45 c0 e8 92 1d 04 00 84 c0 74 f2 eb d2 8d b4 26 00 00 00 00 90 0f 0b c6 45 9b 00 eb c2 8d b4 26 00 00 00 00 90 <0f> 0b eb ee 8d 74 26 00 0f 0b eb a6 8d 74 26 00 e8 8f c4 8d 01 8d [8.765410] EAX: EBX: c272ee01 ECX: 0001 EDX: [8.771726] ESI: c272eea8 EDI: c194d77c EBP: c17bbd64 ESP: c17bbcfc [8.778011] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010246 [8.784829] CR0: 80050033 CR2: CR3: 1355b000 CR4: 003506d0 [8.791120] Call Trace: [8.793591] ? show_regs+0x51/0x58 [8.797028] ? __flush_work+0xb8/0xd0 [8.800724] ? __warn+0x8d/0x1b8 [8.804024] ? __flush_work+0xb8/0xd0 [8.807710] ? __flush_work+0xb8/0xd0 [8.811403] ? report_bug+0x186/0x1b0 [8.815094] ? __flush_work+0xb9/0xd0 [8.818785] ? exc_overflow+0x50/0x50 [8.822474] ? handle_bug+0x56/0x90 For complete details: [1] https://kcidb.kernelci.org/d/test/test?orgId=1&var-datasource=default&var-build_architecture=i386&var-build_config_name=defconfig&var-id=maestro:67455b9a3be6da94b19fde77&from=now-100y&to=now&timezone=browser&var-origin=$__all&var-test_path=&var-issue_presence=$__all [2]https://kcidb.kernelci.org/d/test/test?orgId=1&var-datasource=default&var-build_architecture=x86_64&var-build_config_name=cros:%2F%2Fchromeos-6.6%2Fx86_64%2Fchromeos-amd-stoneyridge.flavour.config&var-id=maestro:67455b923be6da94b19fde4b&from=now-100y&to=now&timezone=browser&var-origin=$__all&var-test_path=&var-issue_presence=$__all [3]https://kcidb.kernelci.org/d/test/test?orgId=1&var-datasource=default&var-build_architecture=x86_64&var-build_config_name=cros:%2F%2Fchromeos-6.6%2Fx86_64%2Fchromeos-amd-stoneyridge.flavour.config&var-id=maestro:67455b923be6da94b19fde48&from=now-100y&to=now&timezone=browser&var-origin=$__all&var-test_path=&var-issue_presence=$__all [4]https://kcidb.kernelci.org/d/test/test?orgId=1&var-datasource=default&var-build_architecture=x86_64&var-build_config_name=cros:%2F%2Fchromeos-6.6%2Fx86_64%2Fchromeos-amd-stoneyridge.flavour.config&var-id=maestro:67455b8c3be6da94b19fde34&from=now-100y&to=now&timezone=browser&var-origin=$__all&var-test_path=&var-issue_presence=$__all -- BR, Muhammad Usama Anjum
Re: [PATCH] drm/amd: Keep display off while going into S4
Hi,
Thank you Mario for finding and fixing!
On 3/6/25 11:51 PM, Mario Limonciello wrote:
> When userspace invokes S4 the flow is:
>
> 1) amdgpu_pmops_prepare()
> 2) amdgpu_pmops_freeze()
> 3) Create hibernation image
> 4) amdgpu_pmops_thaw()
> 5) Write out image to disk
> 6) Turn off system
>
> Then on resume amdgpu_pmops_restore() is called.
>
> This flow has a problem that because amdgpu_pmops_thaw() is called
> it will call amdgpu_device_resume() which will resume all of the GPU.
>
> This includes turning the display hardware back on and discovering
> connectors again.
>
> This is an unexpected experience for the display to turn back on.
> Adjust the flow so that during the S4 sequence display hardware is
> not turned back on.
>
> Reported-by: Xaver Hugl
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2038
> Cc: Muhammad Usama Anjum
> Signed-off-by: Mario Limonciello
Tested on top of amd-drm-next branch and 6.11.11 on 3 different devices.
It fixes all of them.
It should be included in recent LTS kernels at least.
Tested-by: Muhammad Usama Anjum
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +--
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index b161daa90019..b54c4b2f3f7f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2565,7 +2565,6 @@ static int amdgpu_pmops_freeze(struct device *dev)
> int r;
>
> r = amdgpu_device_suspend(drm_dev, true);
> - adev->in_s4 = false;
> if (r)
> return r;
>
> @@ -2577,8 +2576,13 @@ static int amdgpu_pmops_freeze(struct device *dev)
> static int amdgpu_pmops_thaw(struct device *dev)
> {
> struct drm_device *drm_dev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
> + int r;
>
> - return amdgpu_device_resume(drm_dev, true);
> + r = amdgpu_device_resume(drm_dev, true);
> + adev->in_s4 = false;
> +
> + return r;
> }
>
> static int amdgpu_pmops_poweroff(struct device *dev)
> @@ -2591,6 +2595,9 @@ static int amdgpu_pmops_poweroff(struct device *dev)
> static int amdgpu_pmops_restore(struct device *dev)
> {
> struct drm_device *drm_dev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
> +
> + adev->in_s4 = false;
>
> return amdgpu_device_resume(drm_dev, true);
> }
> 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 6f9331fe91c3..5939796db74c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3431,6 +3431,11 @@ static int dm_resume(struct amdgpu_ip_block *ip_block)
>
> return 0;
> }
> +
> + /* leave display off for S4 sequence */
> + if (adev->in_s4)
> + return 0;
> +
> /* Recreate dc_state - DC invalidates it when setting power state to
> S3. */
> dc_state_release(dm_state->context);
> dm_state->context = dc_state_create(dm->dc, NULL);
--
BR,
Muhammad Usama Anjum
