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

2019-10-17 Thread 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čí 
> ---
> 
> 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 0x 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 0x
>*/
>   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 0x1
> +  */
> + brightness *= 0x101;
> + brightness += (brightness >> 7) & 1;
>  
>   if (dc_link_set_backlight_level(dm->backlight_link,
>   brightness, 0))
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

2019-10-15 Thread Lukáš Krejčí
[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čí 
---

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 0x 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.

 .../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 0x
 */
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 0x1
+*/
+   brightness *= 0x101;
+   brightness += (brightness >> 7) & 1;
 
if (dc_link_set_backlight_level(dm->backlight_link,
brightness, 0))
-- 
2.23.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx