Re: [PATCH v2 1/4] drm/amd/display: keep custom brightness curve in userspace domain

[Mario Limonciello <mario.limonciello@amd.com>]

On 9/2/26 17:31, Andrei Rusu de Castro wrote:
> convert_brightness_from_user() expects convert_custom_brightness() to
> reshape a value in the userspace [0..max] domain before the caller maps
> it once into the firmware [min..max] domain.
> 
> Commit 8dbd72cb7900 ("drm/amd/display: Export full brightness range to
> userspace") instead made convert_custom_brightness() call
> scale_fw_to_input(min, max, ...). That helper adds min and scales by the
> firmware span, entering the firmware domain before the caller applies
> the same span and minimum again.
> 
> With the default PWM range, min is 3084 and max is 65535. A zero request
> through any custom curve consequently returns 6023 instead of 3084, so
> the darkest 2939 firmware levels are unreachable.
> 
> Keep the curve result in [0..max]. scale_fw_to_input() then becomes the
> inverse of scale_input_to_fw(), and the caller remains the sole owner of
> the userspace-to-firmware conversion. The linear path is unchanged.
> 
> A later patch removes this helper from the PWM power-module path because
> the power module owns that curve. AUX panels continue to use it, and
> stable kernels predating the power-module refactor still require this
> correction on PWM panels.
> 
> Commit 6fd83a1c2cde ("drm/amd/display: Scale custom brightness curve from
> full range") fixed the corresponding input-side domain error. This
> completes the output side of the same conversion.
> 
> Source and arithmetic analysis identified the duplicate domain
> conversion. KUnit coverage for a non-zero firmware minimum is added
> separately.
> 
> Fixes: 8dbd72cb7900 ("drm/amd/display: Export full brightness range to userspace")
> Cc: stable@vger.kernel.org # 6.17.x: 6fd83a1c2cde: drm/amd/display: Scale custom brightness curve from full range
> Cc: stable@vger.kernel.org # 6.17.x
> Cc: stable@vger.kernel.org # before 7.3 this code is in amdgpu_dm.c; drop the header and test hunks
> Assisted-by: LLM
> Signed-off-by: Andrei Rusu de Castro <arc@empyreal.works>

There has already been some competing changes for how the power module 
integration handling works in amd-staging-drm-next.

I wanted to see how your series was on top of it, but it doesn't apply 
anymore. [1]  Can you please rebase your series and re-evaluate?

If it still makes sense (in your view); please send out a v3.

[1]
Applying: drm/amd/display: keep custom brightness curve in userspace domain
Patch failed at 0001 drm/amd/display: keep custom brightness curve in 
userspace domain
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
error: patch failed: 
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c:112
error: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c: 
patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch

Thanks,

> ---
>   .../display/amdgpu_dm/amdgpu_dm_backlight.c   | 13 ++++----
>   .../display/amdgpu_dm/amdgpu_dm_backlight.h   |  3 +-
>   .../tests/amdgpu_dm_backlight_test.c          | 30 +++++++++----------
>   3 files changed, 22 insertions(+), 24 deletions(-)
> 
> 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 e61bbc310f33..424b33573a73 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
> @@ -112,16 +112,15 @@ static inline u32 scale_input_to_fw(int max, u64 input)
>   	return DIV_ROUND_CLOSEST_ULL(input * AMDGPU_MAX_BL_LEVEL, max);
>   }
>   
> -/* Rescale from [0..AMDGPU_MAX_BL_LEVEL] to [min..max] */
> -static inline u32 scale_fw_to_input(int min, int max, u64 input)
> +/* Rescale the firmware curve's [0..AMDGPU_MAX_BL_LEVEL] back to userspace [0..max]. */
> +static inline u32 scale_fw_to_input(int max, u64 input)
>   {
> -	return min + DIV_ROUND_CLOSEST_ULL(input * (max - min), AMDGPU_MAX_BL_LEVEL);
> +	return DIV_ROUND_CLOSEST_ULL(input * max, AMDGPU_MAX_BL_LEVEL);
>   }
>   
>   STATIC_IFN_KUNIT
>   void convert_custom_brightness(const struct amdgpu_dm_backlight_caps *caps,
> -			       unsigned int min, unsigned int max,
> -			       uint32_t *user_brightness)
> +			       unsigned int max, uint32_t *user_brightness)
>   {
>   	u32 brightness = scale_input_to_fw(max, *user_brightness);
>   	u8 lower_signal, upper_signal, upper_lum, lower_lum, lum;
> @@ -179,7 +178,7 @@ void convert_custom_brightness(const struct amdgpu_dm_backlight_caps *caps,
>   						    (brightness - lower_signal),
>   						    upper_signal - lower_signal);
>   scale:
> -	*user_brightness = scale_fw_to_input(min, max,
> +	*user_brightness = scale_fw_to_input(max,
>   					     DIV_ROUND_CLOSEST(lum * brightness, 101));
>   }
>   
> @@ -194,7 +193,7 @@ u32 convert_brightness_from_user(const struct amdgpu_dm_backlight_caps *caps,
>   	if (!get_brightness_range(caps, &min, &max))
>   		return brightness;
>   
> -	convert_custom_brightness(caps, min, max, &brightness);
> +	convert_custom_brightness(caps, max, &brightness);
>   
>   	/* Rescale 0..max to min..max */
>   	return min + DIV_ROUND_CLOSEST_ULL((u64)(max - min) * brightness, max);
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.h
> index 07b75064847c..90bed0ea5d00 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.h
> @@ -67,8 +67,7 @@ ssize_t panel_power_savings_store(struct device *device,
>   int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
>   			 unsigned int *min, unsigned int *max);
>   void convert_custom_brightness(const struct amdgpu_dm_backlight_caps *caps,
> -			       unsigned int min, unsigned int max,
> -			       uint32_t *user_brightness);
> +			       unsigned int max, uint32_t *user_brightness);
>   u32 convert_brightness_from_user(const struct amdgpu_dm_backlight_caps *caps,
>   				 uint32_t brightness);
>   u32 convert_brightness_to_user(const struct amdgpu_dm_backlight_caps *caps,
> 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 7ca17f803f9d..1fb171fdbc3c 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
> @@ -1033,7 +1033,7 @@ static void dm_test_custom_brightness_no_data_points(struct kunit *test)
>   
>   	caps.data_points = 0;
>   
> -	convert_custom_brightness(&caps, 3084, 65535, &brightness);
> +	convert_custom_brightness(&caps, 65535, &brightness);
>   
>   	/* No data points → no-op */
>   	KUNIT_EXPECT_EQ(test, brightness, saved);
> @@ -1057,7 +1057,7 @@ static void dm_test_custom_brightness_debug_mask_disables(struct kunit *test)
>   	/* Set the disable flag */
>   	amdgpu_dm_set_dc_debug_mask(amdgpu_dm_get_dc_debug_mask() | DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE);
>   
> -	convert_custom_brightness(&caps, 3084, 65535, &brightness);
> +	convert_custom_brightness(&caps, 65535, &brightness);
>   
>   	/* Should be no-op due to debug mask */
>   	KUNIT_EXPECT_EQ(test, brightness, saved);
> @@ -1100,14 +1100,14 @@ static void dm_test_custom_brightness_exact_match(struct kunit *test)
>   	 */
>   	brightness = 32896;
>   
> -	convert_custom_brightness(&caps, min, max, &brightness);
> +	convert_custom_brightness(&caps, max, &brightness);
>   
>   	/*
>   	 * Exact match: lum=50, brightness_scaled=128
> -	 * result = scale_fw_to_input(min, max, DIV_ROUND_CLOSEST(50*128, 101))
> -	 *        = scale_fw_to_input(0, 65535, DIV_ROUND_CLOSEST(6400, 101))
> -	 *        = scale_fw_to_input(0, 65535, 63)
> -	 *        = 0 + DIV_ROUND_CLOSEST(63 * 65535, 255) = 16191 (approx)
> +	 * result = scale_fw_to_input(max, DIV_ROUND_CLOSEST(50*128, 101))
> +	 *        = scale_fw_to_input(65535, DIV_ROUND_CLOSEST(6400, 101))
> +	 *        = scale_fw_to_input(65535, 63)
> +	 *        = DIV_ROUND_CLOSEST(63 * 65535, 255) = 16191 (approx)
>   	 */
>   	KUNIT_EXPECT_TRUE(test, brightness != 32896);
>   	KUNIT_EXPECT_TRUE(test, brightness < 32896);
> @@ -1146,13 +1146,13 @@ static void dm_test_custom_brightness_below_first(struct kunit *test)
>   	 */
>   	brightness = 12850;
>   
> -	convert_custom_brightness(&caps, min, max, &brightness);
> +	convert_custom_brightness(&caps, max, &brightness);
>   
>   	/*
>   	 * Below first data point: lum = DIV_ROUND_CLOSEST(40 * 50, 100) = 20
> -	 * Then: scale_fw_to_input(0, 65535, DIV_ROUND_CLOSEST(20 * 50, 101))
> -	 *      = scale_fw_to_input(0, 65535, DIV_ROUND_CLOSEST(1000, 101))
> -	 *      = scale_fw_to_input(0, 65535, 10)
> +	 * Then: scale_fw_to_input(65535, DIV_ROUND_CLOSEST(20 * 50, 101))
> +	 *      = scale_fw_to_input(65535, DIV_ROUND_CLOSEST(1000, 101))
> +	 *      = scale_fw_to_input(65535, 10)
>   	 * The output should be significantly less than input.
>   	 */
>   	KUNIT_EXPECT_TRUE(test, brightness < 12850);
> @@ -1190,7 +1190,7 @@ static void dm_test_custom_brightness_interpolation(struct kunit *test)
>   	 */
>   	brightness = 32125;
>   
> -	convert_custom_brightness(&caps, min, max, &brightness);
> +	convert_custom_brightness(&caps, max, &brightness);
>   
>   	/*
>   	 * The function should interpolate between data points and produce
> @@ -1233,7 +1233,7 @@ static void dm_test_custom_brightness_above_last(struct kunit *test)
>   	 */
>   	brightness = 56533;
>   
> -	convert_custom_brightness(&caps, min, max, &brightness);
> +	convert_custom_brightness(&caps, max, &brightness);
>   
>   	/* Output should differ from input (remapped via curve) */
>   	KUNIT_EXPECT_TRUE(test, brightness != 56533);
> @@ -1271,7 +1271,7 @@ static void dm_test_custom_brightness_single_data_point(struct kunit *test)
>   	 */
>   	brightness = 16448;
>   
> -	convert_custom_brightness(&caps, min, max, &brightness);
> +	convert_custom_brightness(&caps, max, &brightness);
>   
>   	KUNIT_EXPECT_TRUE(test, brightness < 16448);
>   
> @@ -1309,7 +1309,7 @@ static void dm_test_custom_brightness_lower_lum_zero(struct kunit *test)
>   	 */
>   	brightness = 32125;
>   
> -	convert_custom_brightness(&caps, min, max, &brightness);
> +	convert_custom_brightness(&caps, max, &brightness);
>   
>   	/* Should remap; result should differ from input */
>   	KUNIT_EXPECT_TRUE(test, brightness != 32125);