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.chau...@amd.com>
Sent: Wednesday, June 17, 2020 3:20 AM
To: Wentland, Harry <harry.wentl...@amd.com>; Li, Sun peng (Leo) 
<sunpeng...@amd.com>; Deucher, Alexander <alexander.deuc...@amd.com>
Cc: amd-gfx@lists.freedesktop.org; Chauhan, Ikshwaku <ikshwaku.chau...@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.chau...@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@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CShirish.S%40amd.com%7C346bf19d8b154ef1060d08d81650976c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637283883029643196&amp;sdata=ThmKQHfhlWqYWTi1LBojaQvySj9sMLXqw9xgsgGZe3Y%3D&amp;reserved=0

--
Regards,
Shirish S

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to