[PATCH] drm/amdgpu: respect the abmlevel module parameter value if it is set

[Mario Limonciello]

On 2/9/2024 15:46, Hamza Mahfooz wrote:
> Currently, if the abmlevel module parameter is set, it is possible for
> user space to override the ABM level at some point after boot. However,
> that is undesirable because it means that we aren't respecting the
> user's wishes with regard to the level that they want to use. So,
> prevent user space from changing the ABM level if the module parameter
> is set to a non-auto value.
> 
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz at amd.com>

Reviewed-by: Mario Limonciello <mario.limonciello at amd.com>
Tested-by: Mario Limonciello <mario.limonciello at amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h               |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c           | 11 ++++++-----
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 ++++++++++-----
>   3 files changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 1291b8eb9dff..f5c8187e0d58 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -196,7 +196,7 @@ extern int amdgpu_smu_pptable_id;
>   extern uint amdgpu_dc_feature_mask;
>   extern uint amdgpu_dc_debug_mask;
>   extern uint amdgpu_dc_visual_confirm;
> -extern uint amdgpu_dm_abm_level;
> +extern int amdgpu_dm_abm_level;
>   extern int amdgpu_backlight;
>   extern int amdgpu_damage_clips;
>   extern struct amdgpu_mgpu_info mgpu_info;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6ef7f22c1152..af7fae7907d7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -849,12 +849,13 @@ module_param_named(visualconfirm, amdgpu_dc_visual_confirm, uint, 0444);
>    * the ABM algorithm, with 1 being the least reduction and 4 being the most
>    * reduction.
>    *
> - * Defaults to 0, or disabled. Userspace can still override this level later
> - * after boot.
> + * Defaults to -1, or disabled. Userspace can only override this level after
> + * boot if it's set to auto.
>    */
> -uint amdgpu_dm_abm_level;
> -MODULE_PARM_DESC(abmlevel, "ABM level (0 = off (default), 1-4 = backlight reduction level) ");
> -module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
> +int amdgpu_dm_abm_level = -1;
> +MODULE_PARM_DESC(abmlevel,
> +		 "ABM level (0 = off, 1-4 = backlight reduction level, -1 auto (default))");
> +module_param_named(abmlevel, amdgpu_dm_abm_level, int, 0444);
>   
>   int amdgpu_backlight = -1;
>   MODULE_PARM_DESC(backlight, "Backlight control (0 = pwm, 1 = aux, -1 auto (default))");
> 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 fbe2aa40c21a..a5b3330879f3 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6513,7 +6513,8 @@ static void amdgpu_dm_connector_unregister(struct drm_connector *connector)
>   {
>   	struct amdgpu_dm_connector *amdgpu_dm_connector = to_amdgpu_dm_connector(connector);
>   
> -	if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> +	if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
> +	    amdgpu_dm_abm_level < 0)
>   		sysfs_remove_group(&connector->kdev->kobj, &amdgpu_group);
>   
>   	drm_dp_aux_unregister(&amdgpu_dm_connector->dm_dp_aux.aux);
> @@ -6577,9 +6578,12 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector)
>   		state->vcpi_slots = 0;
>   		state->pbn = 0;
>   
> -		if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> -			state->abm_level = amdgpu_dm_abm_level ?:
> -				ABM_LEVEL_IMMEDIATE_DISABLE;
> +		if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> +			if (amdgpu_dm_abm_level <= 0)
> +				state->abm_level = ABM_LEVEL_IMMEDIATE_DISABLE;
> +			else
> +				state->abm_level = amdgpu_dm_abm_level;
> +		}
>   
>   		__drm_atomic_helper_connector_reset(connector, &state->base);
>   	}
> @@ -6617,7 +6621,8 @@ amdgpu_dm_connector_late_register(struct drm_connector *connector)
>   		to_amdgpu_dm_connector(connector);
>   	int r;
>   
> -	if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> +	if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
> +	    amdgpu_dm_abm_level < 0) {
>   		r = sysfs_create_group(&connector->kdev->kobj,
>   				       &amdgpu_group);
>   		if (r)