Re: [PATCH 3/5] drm/amd/display: Avoid operating on copies of backlight caps

[Alex Hung <alex.hung@amd.com>]

On 2/21/25 10:10, Mario Limonciello wrote:
> Making a copy of the backlight caps structure between uses is unnecessary.
> Refer to pointers to the same structure when using it.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 ++++++++-----------
>   1 file changed, 21 insertions(+), 29 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 0d21448ea700..70c8d800e173 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4646,47 +4646,39 @@ static void amdgpu_dm_update_backlight_caps(struct amdgpu_display_manager *dm,
>   					    int bl_idx)
>   {
>   #if defined(CONFIG_ACPI)
> -	struct amdgpu_dm_backlight_caps caps;
> -
> -	memset(&caps, 0, sizeof(caps));
> +	struct amdgpu_dm_backlight_caps *caps = &dm->backlight_caps[bl_idx];
>   
> -	if (dm->backlight_caps[bl_idx].caps_valid)
> +	if (caps->caps_valid)
>   		return;
>   
> -	amdgpu_acpi_get_backlight_caps(&caps);
> +	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->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 < 0 ||
> +		if (caps->max_input_signal > AMDGPU_DM_DEFAULT_MAX_BACKLIGHT ||
> +		    caps->min_input_signal < 0 ||
>   		    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;
> +				      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)
> -			return;
> -		dm->backlight_caps[bl_idx].min_input_signal = caps.min_input_signal;
> -		dm->backlight_caps[bl_idx].max_input_signal = caps.max_input_signal;
> -	} else {
> -		dm->backlight_caps[bl_idx].min_input_signal =
> -				AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> -		dm->backlight_caps[bl_idx].max_input_signal =
> -				AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
> +	if (!caps->caps_valid) {
> +		caps->min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> +		caps->max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
> +		caps->caps_valid = true;
>   	}
>   #else
> -	if (dm->backlight_caps[bl_idx].aux_support)
> +	if (caps->aux_support)
>   		return;
>   
> -	dm->backlight_caps[bl_idx].min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> -	dm->backlight_caps[bl_idx].max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
> +	caps->min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
> +	caps->max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
> +	caps->caps_valid = true;


caps is not defined in "#else" so this fails when CONFIG_ACPI is not 
defined.

Below are errors messages for your references

[2025-02-27T05:12:05.659Z] 
/jenkins/workspace/github/dal-linux-promotion-nightly-github/linux_temp/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4714:6: 
error: use of undeclared identifier 'caps'
[2025-02-27T05:12:05.659Z]         if (caps->aux_support)
[2025-02-27T05:12:05.659Z]             ^
[2025-02-27T05:12:05.659Z] 
/jenkins/workspace/github/dal-linux-promotion-nightly-github/linux_temp/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4717:2: 
error: use of undeclared identifier 'caps'
[2025-02-27T05:12:05.659Z]         caps->min_input_signal = 
AMDGPU_DM_DEFAULT_MIN_BACKLIGHT;
[2025-02-27T05:12:05.659Z]         ^
[2025-02-27T05:12:05.659Z] 
/jenkins/workspace/github/dal-linux-promotion-nightly-github/linux_temp/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4718:2: 
error: use of undeclared identifier 'caps'
[2025-02-27T05:12:05.659Z]         caps->max_input_signal = 
AMDGPU_DM_DEFAULT_MAX_BACKLIGHT;
[2025-02-27T05:12:05.659Z]         ^
[2025-02-27T05:12:05.659Z] 
/jenkins/workspace/github/dal-linux-promotion-nightly-github/linux_temp/drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4719:2: 
error: use of undeclared identifier 'caps'
[2025-02-27T05:12:05.659Z]         caps->caps_valid = true;
[2025-02-27T05:12:05.659Z]         ^
[2025-02-27T05:12:05.760Z] 4 errors generated.

>   #endif
>   }
>   
> @@ -4740,19 +4732,19 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
>   					 int bl_idx,
>   					 u32 user_brightness)
>   {
> -	struct amdgpu_dm_backlight_caps caps;
> +	struct amdgpu_dm_backlight_caps *caps;
>   	struct dc_link *link;
>   	u32 brightness;
>   	bool rc, reallow_idle = false;
>   
>   	amdgpu_dm_update_backlight_caps(dm, bl_idx);
> -	caps = dm->backlight_caps[bl_idx];
> +	caps = &dm->backlight_caps[bl_idx];
>   
>   	dm->brightness[bl_idx] = user_brightness;
>   	/* update scratch register */
>   	if (bl_idx == 0)
>   		amdgpu_atombios_scratch_regs_set_backlight_level(dm->adev, dm->brightness[bl_idx]);
> -	brightness = convert_brightness_from_user(&caps, dm->brightness[bl_idx]);
> +	brightness = convert_brightness_from_user(caps, dm->brightness[bl_idx]);
>   	link = (struct dc_link *)dm->backlight_link[bl_idx];
>   
>   	/* Change brightness based on AUX property */
> @@ -4762,7 +4754,7 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
>   		reallow_idle = true;
>   	}
>   
> -	if (caps.aux_support) {
> +	if (caps->aux_support) {
>   		rc = dc_link_set_backlight_level_nits(link, true, brightness,
>   						      AUX_BL_DEFAULT_TRANSITION_TIME_MS);
>   		if (!rc)