On 2024-09-13 14:00, Mario Limonciello wrote: > Currently amdgpu takes backlight caps provided by the ACPI tables > on systems as is. If the firmware sets maximums that are too low > this means that users don't get a good experience. > > To avoid having to maintain a quirk list of such systems, do a sanity > check on the values. Check that the spread is at least half of the > values that amdgpu would use if no ACPI table was found and if not > use the amdgpu defaults. > > Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3020 > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > 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 5942fc4e1c86..ad66f09cd0bb 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -4428,6 +4428,7 @@ static int amdgpu_dm_mode_config_init(struct amdgpu_device *adev) > > #define AMDGPU_DM_DEFAULT_MIN_BACKLIGHT 12 > #define AMDGPU_DM_DEFAULT_MAX_BACKLIGHT 255 > +#define AMDGPU_DM_MIN_SPREAD ((AMDGPU_DM_DEFAULT_MAX_BACKLIGHT - AMDGPU_DM_DEFAULT_MIN_BACKLIGHT) / 2) > #define AUX_BL_DEFAULT_TRANSITION_TIME_MS 50 > > static void amdgpu_dm_update_backlight_caps(struct amdgpu_display_manager *dm, > @@ -4442,6 +4443,21 @@ static void amdgpu_dm_update_backlight_caps(struct amdgpu_display_manager *dm, > return; > > amdgpu_acpi_get_backlight_caps(&caps); > + > + /* validate the firmware value is sane */ > + if (caps.caps_valid) { > + int spread = caps.max_input_signal - caps.min_input_signal; > + > + if (caps.max_input_signal > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT || > + caps.min_input_signal < AMDGPU_DM_DEFAULT_MIN_BACKLIGHT || Would we still want to allow signals below AMDGPU_DM_DEFAULT_MIN_BACKLIGHT (but above 0)? The min backlight value of 12 is a bit conservative and I wouldn't be surprised if systems set it lower via ACPI. The rest looks like great checks that we should absolutely have. Harry > + spread > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT || > + spread < AMDGPU_DM_MIN_SPREAD) { > + DRM_DEBUG_KMS("DM: Invalid backlight caps: min=%d, max=%d\n", > + caps.min_input_signal, caps.max_input_signal); > + caps.caps_valid = false; > + } > + } > + > if (caps.caps_valid) { > dm->backlight_caps[bl_idx].caps_valid = true; > if (caps.aux_support)
