[RFC PATCH RESEND] drm/amd/display: Add back missing backlight level rounding

[Li, Sun peng (Leo)]

Thanks for the detailed notes! See reply inline.

On 2019-10-15 4:03 p.m., Lukáš Krejčí wrote:
> [Why]
> Having the rounding of the backlight value restored to the way it was
> seemingly gets rid of backlight flickering on certain Stoney Ridge
> laptops.
> 
> [How]
> Rescale the backlight level between min and max input signal value and
> round it to a number between 0x0 and 0xFF. Then, use the rounding mode
> that was previously in driver_set_backlight_level() and
> dmcu_set_backlight_level(), i.e. rescale the backlight level between 0x0
> and 0x1000 by multiplying it by 0x101 and use the most significant bit
> of the fraction (or in this case the 8th bit of the value because it's
> the same thing, e.g. C3 * 0x101 = 0xC3C3 and C3 * 0x10101 = 0xC3C3C3) to
> round it.
> 
> Fixes: 262485a50fd4 ("drm/amd/display: Expand dc to use 16.16 bit backlight")
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204957
> Signed-off-by: Lukáš Krejčí <lskrejci at gmail.com>
> ---
> 
> Notes:
>     Bug:
>     - Can be reproduced on HP 15-rb020nc (Stoney Ridge R2 E2-9000e APU) and
>       Acer Aspire 3 A315-21G-91JL (Stoney Ridge R3 A4-9120 APU).
>     
>     - For me, the bug is inconsistent - it does not happen on every boot,
>       but if it does happen, it's usually within three minutes after bootup.
>     
>     - It concerns the backlight. That means it can be reproduced on the
>       framebuffer console.
>     
>     - Reproduces on Arch Linux (custom built) live CD, linux kernel v5.3.2
>       and Ubuntu 19.04, kernel-ppa/mainline v5.0.0-rc1, v5.0.0, v5.3.2, v5.3.4,
>       v5.4-rc2.
>     
>     - Can not be reproduced on kernel v5.3.4 with this patch applied or on
>       v4.20.0, 4.20.17, 4.19.75 (this bug is a regression).
>     
>     - The other person that reproduced the issue (see the Bugzilla link
>       above) confirmed that the patch works for them too.
>     
>     Patch:
>     - Is the comment modified by this commit correct? Both
>       driver_set_backlight_level() and dmcu_set_backlight_level() check the
>       17th bit of `brightness` aka `backlight_pwm_u16_16`, but
>       262485a50fd4532a8d71165190adc7a0a19bcc9e ("drm/amd/display: Expand dc
>       to use 16.16 bit backlight") specifically mentions 0xFFFF as the max
>       backlight value>     
>     - use_smooth_brightness is false (no DMCU firmware available) on my
>       laptop, so the other code path (dmcu_set_backlight_level()) is
>       untested.
>     
>     - I'm not sure why the rounding fixes the issue and whether this
>       function is the right place to add back the rounding (and whether it
>       even is the right way to solve the issue), so that's why this patch is
>       RFC.

We've seen some similar issues when fractional duty cycles are enabled for backlight pwm.
I attached a hack to the bugzilla ticket that disables it, please give that patch a shot
first. I'd rather not change this calculation for all panels if the flickering issue is
only a quirk for some.

Thanks,
Leo

> 
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> 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 a52f0b13a2c8..af9a5f46b671 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2083,17 +2083,22 @@ static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
>  	 * The brightness input is in the range 0-255
>  	 * It needs to be rescaled to be between the
>  	 * requested min and max input signal
> -	 *
> -	 * It also needs to be scaled up by 0x101 to
> -	 * match the DC interface which has a range of
> -	 * 0 to 0xffff
>  	 */
>  	brightness =
>  		brightness
> -		* 0x101
> +		* 0x100
>  		* (caps.max_input_signal - caps.min_input_signal)
>  		/ AMDGPU_MAX_BL_LEVEL
> -		+ caps.min_input_signal * 0x101;
> +		+ caps.min_input_signal * 0x100;
> +
> +	brightness = (brightness >> 8) + ((brightness >> 7) & 1);
> +	/*
> +	 * It also needs to be scaled up by 0x101 and
> +	 * rounded off to match the DC interface which
> +	 * has a range of 0 to 0x10000
> +	 */
> +	brightness *= 0x101;
> +	brightness += (brightness >> 7) & 1;
>  
>  	if (dc_link_set_backlight_level(dm->backlight_link,
>  			brightness, 0))
>