Re: [PATCH v7 06/12] drm/amd/display: Pass up errors reading actual brightness

[sashiko-bot@kernel.org]

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 <mario.limonciello@amd.com>

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/20260831210531.2334221-1-mario.limonciello@amd.com?part=6