Messages in this thread | | | Subject | Re: [PATCH] drm/amd/display: Set AMDGPU_DM_DEFAULT_MIN_BACKLIGHT to 0 | From | Harry Wentland <> | Date | Fri, 19 Mar 2021 11:07:32 -0400 |
| |
On 2021-03-19 10:22 a.m., Alex Deucher wrote: > On Fri, Mar 19, 2021 at 3:23 AM Evan Benn <evanbenn@chromium.org> wrote: >> >> AMDGPU_DM_DEFAULT_MIN_BACKLIGHT was set to the value of 12 >> to ensure no display backlight will flicker at low user brightness >> settings. However this value is quite bright, so for devices that do not >> implement the ACPI ATIF >> ATIF_FUNCTION_QUERY_BRIGHTNESS_TRANSFER_CHARACTERISTICS >> functionality the user cannot set the brightness to a low level even if >> the display would support such a low PWM. >> >> This ATIF feature is not implemented on for example AMD grunt chromebooks. >> >> Signed-off-by: Evan Benn <evanbenn@chromium.org> >> >> --- >> I could not find a justification for the reason for the value. It has >> caused some noticable regression for users: https://bugzilla.kernel.org/show_bug.cgi?id=203439>>> >> Maybe this can be either user controlled or userspace configured, but >> preventing users from turning their backlight dim seems wrong. > > My understanding is that some panels flicker if you set the min to a > value too low. This was a safe minimum if the platform didn't specify > it's own safe minimum. I think we'd just be trading one bug for > another (flickering vs not dim enough). Maybe a whitelist or > blacklist would be a better solution? >
Yeah, this is a NACK from me as-is for the reasons Alex described.
I agree a whitelist approach might be best.
Is this fix perhaps for OLED panels? If so we could use a different min-value for OLED panels that don't do PWM, but use 12 for everything else.
Harry
> Alex > > >> >> Also reviewed here: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2748377>>> >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> 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 573cf17262da..0129bd69b94e 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -3151,7 +3151,7 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev) >> return 0; >> } >> >> -#define AMDGPU_DM_DEFAULT_MIN_BACKLIGHT 12 >> +#define AMDGPU_DM_DEFAULT_MIN_BACKLIGHT 0 >> #define AMDGPU_DM_DEFAULT_MAX_BACKLIGHT 255 >> #define AUX_BL_DEFAULT_TRANSITION_TIME_MS 50 >> >> -- >> 2.31.0.291.g576ba9dcdaf-goog >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel>> _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel>>
| |