Re: [PATCH] drm/amd/display: Scale custom brightness curve from full range

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Double scaling of the brightness value when a custom brightness curve is used, causing the minimum backlight level to be incorrectly raised.
--

--- Patch [1]: [PATCH] drm/amd/display: Scale custom brightness curve from full range ---
commit d7add749f0052e896a6fee3206021e51b2fd8540
Author: Akhmed Zhitaev <[email protected]>

drm/amd/display: Scale custom brightness curve from full range

This patch changes the input scaling for custom brightness curves to
map the userspace [0..max] range to the curve's [0..255] range. This
prevents generating a curve input greater than 255 for panels with a
nonzero minimum PWM level.

> 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 941c1a3128249..7908b15370845 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
[ ... ]
> -/* Rescale from [min..max] to [0..AMDGPU_MAX_BL_LEVEL] */
> -static inline u32 scale_input_to_fw(int min, int max, u64 input)
> +/* Rescale userspace [0..max] to the firmware curve's [0..255]. */
> +static inline u32 scale_input_to_fw(int max, u64 input)
>  {
> -	return DIV_ROUND_CLOSEST_ULL(input * AMDGPU_MAX_BL_LEVEL, max - min);
> +	return DIV_ROUND_CLOSEST_ULL(input * AMDGPU_MAX_BL_LEVEL, max);
>  }
>  
>  /* Rescale from [0..AMDGPU_MAX_BL_LEVEL] to [min..max] */
> @@ -5281,7 +5281,7 @@ static void convert_custom_brightness(const struct amdgpu_dm_backlight_caps *cap
>  				      unsigned int min, unsigned int max,
>  				      uint32_t *user_brightness)
>  {
> -	u32 brightness = scale_input_to_fw(min, max, *user_brightness);
> +	u32 brightness = scale_input_to_fw(max, *user_brightness);
>  	u8 lower_signal, upper_signal, upper_lum, lower_lum, lum;
>  	int left, right;

[Severity: High]
This isn't a bug introduced by this patch, but does the current code
double-scale the brightness when a custom brightness curve is used?

Looking at scale_fw_to_input() in drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c,
it maps the curve's [0..255] output to [min..max]:

static inline u32 scale_fw_to_input(int min, int max, u64 input)
{
	return min + DIV_ROUND_CLOSEST_ULL(input * (max - min),
					   AMDGPU_MAX_BL_LEVEL);
}

This is used at the end of convert_custom_brightness(), meaning the output
brightness value is returned in the [min..max] range.

However, in convert_brightness_from_user(), the output gets rescaled again:

	convert_custom_brightness(caps, min, max, &brightness);

	// Rescale 0..max to min..max
	return min + DIV_ROUND_CLOSEST_ULL((u64)(max - min) * brightness, max);

Because scale_fw_to_input() already mapped it to [min..max], and
convert_brightness_from_user() rescales it to [min..max] again under the
assumption that the value is in [0..max], the minimum offset is applied twice.

Since this patch explicitly aims to fix the brightness scaling logic introduced
by commit 8dbd72cb7900, should scale_fw_to_input() or
convert_brightness_from_user() also be updated to avoid raising the minimum
backlight level incorrectly for panels using custom brightness curves?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1