[PATCH] drm/amdgpu: backlight rescaling to original range

[S, Shirish]

Some basic nit-picks inline.

On 6/22/2020 7:34 AM, Chauhan, Ikshwaku wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hello All,
>
> Could you please provide your feedback for this patch?
>
> Regards,
> Ikshwaku
>
> -----Original Message-----
> From: Chauhan, Ikshwaku <Ikshwaku.Chauhan at amd.com>
> Sent: Wednesday, June 17, 2020 3:20 AM
> To: Wentland, Harry <Harry.Wentland at amd.com>; Li, Sun peng (Leo) <Sunpeng.Li at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>
> Cc: amd-gfx at lists.freedesktop.org; Chauhan, Ikshwaku <Ikshwaku.Chauhan at amd.com>
> Subject: [PATCH] drm/amdgpu: backlight rescaling to original range

Since you are touching display folder, the commit message should point 
to "drm/amd/display"

Also the commit message is not clear, can you correct it to reflect what 
this patch is doing.

>
> [why]
> The brightness input is in the range 0-255.It is getting scaled between the requested min and max input signal and also scaled up by 0x101 to match the DC interface which has a range of 0 to 0xffff. This scaled brightness value is not rescaled back to original range(0-255) when we reads it back.It returns the brightness value in the range of 0-65535 instead of 0-255.
>
> [how]
> Rescaled the brightness value form the scaled brightness range 0-65535 to input brightness range 0-255.

Please provide a sample output of backlight set & get values with and 
without your patch to understand what this patch is fixing, better.

Regards,

Shirish S

>
> Signed-off-by: Ikshwaku Chauhan <ikshwaku.chauhan at amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 40 ++++++++++++++-----  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  5 +++
>   2 files changed, 34 insertions(+), 11 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 9ab0d8521576..73b0a084e893 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2881,7 +2881,8 @@ static int set_backlight_via_aux(struct dc_link *link, uint32_t brightness)  }
>   
>   static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
> -			      const uint32_t user_brightness)
> +			      const uint32_t user_brightness,
> +			      enum convert_backlight flag)
>   {
>   	u32 min, max, conversion_pace;
>   	u32 brightness = user_brightness;
> @@ -2901,12 +2902,18 @@ static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
>   		 * 0 to 0xffff
>   		 */
>   		conversion_pace = 0x101;
> -		brightness =
> -			user_brightness
> -			* conversion_pace
> -			* (max - min)
> -			/ AMDGPU_MAX_BL_LEVEL
> -			+ min * conversion_pace;
> +		if (flag == set_backlight)
> +			brightness =
> +				user_brightness
> +				* conversion_pace
> +				* (max - min)
> +				/ AMDGPU_MAX_BL_LEVEL
> +				+ min * conversion_pace;
> +		else
> +			brightness =
> +				((user_brightness - min * conversion_pace)
> +				 * AMDGPU_MAX_BL_LEVEL)
> +				 / (conversion_pace * (max - min));
>   	} else {
>   		/* TODO
>   		 * We are doing a linear interpolation here, which is OK but @@ -2940,24 +2947,35 @@ static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
>   
>   	link = (struct dc_link *)dm->backlight_link;
>   
> -	brightness = convert_brightness(&caps, bd->props.brightness);
> +	brightness = convert_brightness(&caps, bd->props.brightness,
> +				set_backlight);
>   	// Change brightness based on AUX property
>   	if (caps.aux_support)
>   		return set_backlight_via_aux(link, brightness);
>   
>   	rc = dc_link_set_backlight_level(dm->backlight_link, brightness, 0);
> -
>   	return rc ? 0 : 1;
>   }
>   
>   static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)  {
>   	struct amdgpu_display_manager *dm = bl_get_data(bd);
> -	int ret = dc_link_get_backlight_level(dm->backlight_link);
> +	struct amdgpu_dm_backlight_caps caps;
> +	int ret;
> +
> +	amdgpu_dm_update_backlight_caps(dm);
> +	caps = dm->backlight_caps;
> +
> +	ret = dc_link_get_backlight_level(dm->backlight_link);
> +	ret = (int)convert_brightness(&caps, (uint32_t)ret, get_backlight);
>   
>   	if (ret == DC_ERROR_UNEXPECTED)
>   		return bd->props.brightness;
> -	return ret;
> +
> +	if (ret == AMDGPU_MAX_BL_LEVEL || ret == 0)
> +		return ret;
> +	else
> +		return ret+1;
>   }
>   
>   static const struct backlight_ops amdgpu_dm_backlight_ops = { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index 1df0ce047e1c..d54fc00148f9 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -60,6 +60,11 @@ struct dc;
>   struct amdgpu_bo;
>   struct dmub_srv;
>   
> +enum convert_backlight {
> +	get_backlight,
> +	set_backlight
> +};
> +
>   struct common_irq_params {
>   	struct amdgpu_device *adev;
>   	enum dc_irq_source irq_src;
> --
> 2.17.1
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CShirish.S%40amd.com%7C346bf19d8b154ef1060d08d81650976c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637283883029643196&sdata=ThmKQHfhlWqYWTi1LBojaQvySj9sMLXqw9xgsgGZe3Y%3D&reserved=0

-- 
Regards,
Shirish S