Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Incomplete refactoring of PWM error path causes KUnit test failure. --
commit 51bad29849babac3047c98c959ac850e269def79 Author: Mario Limonciello <[email protected]> drm/amd/display: Pass up errors reading actual brightness This patch changes the return type of amdgpu_dm_backlight_get_level() from an unsigned integer to a signed integer. It also updates the caller and KUnit tests to handle negative error codes like -EINVAL on backlight read failure. > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c > index b66ca60e697dc..ed1f85650e280 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c [ ... ] > @@ -376,7 +376,7 @@ u32 amdgpu_dm_backlight_get_level(struct > amdgpu_display_manager *dm, int bl_idx) > u32 avg, peak; > > if (!dc_link_get_backlight_level_nits(link, &avg, &peak)) > - return dm->brightness[bl_idx]; > + return -EINVAL; > return convert_brightness_to_user(&caps, avg); > } > [Severity: Medium] Was the legacy PWM error path missed during this refactoring? A few lines down in amdgpu_dm_backlight_get_level(), there is another error path: ret = dc_link_get_backlight_level(link); if (ret == DC_ERROR_UNEXPECTED) return dm->brightness[bl_idx]; Since this still returns the cached fallback brightness instead of -EINVAL, could this cause unexpected behavior for the updated KUnit test? [ ... ] > diff --git > a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c > index 7ca17f803f9d7..e03173d4e5bd8 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c > @@ -382,7 +382,7 @@ static void dm_test_backlight_get_level_pwm_error(struct > kunit *test) > dm->brightness[0] = 4321; > dm->backlight_link[0] = link; > > - KUNIT_EXPECT_EQ(test, amdgpu_dm_backlight_get_level(dm, 0), 4321U); > + KUNIT_EXPECT_EQ(test, amdgpu_dm_backlight_get_level(dm, 0), -EINVAL); [Severity: Medium] Will this test unconditionally fail? Because the legacy error path in amdgpu_dm_backlight_get_level() wasn't updated to return -EINVAL, it seems the function will still return the cached fallback value (4321) when the mock function returns DC_ERROR_UNEXPECTED. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
