| Message ID | 20191015200304.30470-1-lskrejci@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,RESEND] drm/amd/display: Add back missing backlight level rounding | expand |
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@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)) >
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))
[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@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. .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)