[PATCH] drm/amd/display: Validate backlight caps are sane

[Harry Wentland]

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 at amd.com>
> ---
>  .../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)