| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a5182129-27fa-d0ff-8aa5-a125aaaf8408@amd.com>
Date: Tue, 18 Aug 2020 12:48:56 -0400
From: "Kazlauskas, Nicholas" <nicholas.kazlauskas@....com>
To: Alex Deucher <alexdeucher@...il.com>,
Alexander Monakov <amonakov@...ras.ru>
Cc: amd-gfx list <amd-gfx@...ts.freedesktop.org>,
Alex Deucher <alexander.deucher@....com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] drm/amd/display: use correct scale for
actual_brightness
No objections from my side - and thanks for addressing my feedback.
Regards,
Nicholas Kazlauskas
On 2020-08-18 12:15 p.m., Alex Deucher wrote:
> Applied. Thanks!
>
> Alex
>
> On Mon, Aug 17, 2020 at 1:59 PM Alex Deucher <alexdeucher@...il.com> wrote:
>>
>> On Mon, Aug 17, 2020 at 3:09 AM Alexander Monakov <amonakov@...ras.ru> wrote:
>>>
>>> Ping.
>>
>> Patch looks good to me:
>> Reviewed-by: Alex Deucher <alexander.deucher@....com>
>>
>> Nick, unless you have any objections, I'll go ahead and apply it.
>>
>> Alex
>>
>>>
>>> On Tue, 4 Aug 2020, Alexander Monakov wrote:
>>>
>>>> Documentation for sysfs backlight level interface requires that
>>>> values in both 'brightness' and 'actual_brightness' files are
>>>> interpreted to be in range from 0 to the value given in the
>>>> 'max_brightness' file.
>>>>
>>>> With amdgpu, max_brightness gives 255, and values written by the user
>>>> into 'brightness' are internally rescaled to a wider range. However,
>>>> reading from 'actual_brightness' gives the raw register value without
>>>> inverse rescaling. This causes issues for various userspace tools such
>>>> as PowerTop and systemd that expect the value to be in the correct
>>>> range.
>>>>
>>>> Introduce a helper to retrieve internal backlight range. Use it to
>>>> reimplement 'convert_brightness' as 'convert_brightness_from_user' and
>>>> introduce 'convert_brightness_to_user'.
>>>>
>>>> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=203905
>>>> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1242
>>>> Cc: Alex Deucher <alexander.deucher@....com>
>>>> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@....com>
>>>> Signed-off-by: Alexander Monakov <amonakov@...ras.ru>
>>>> ---
>>>> v2: split convert_brightness to &_from_user and &_to_user (Nicholas)
>>>>
>>>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 81 +++++++++----------
>>>> 1 file changed, 40 insertions(+), 41 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 710edc70e37e..b60a763f3f95 100644
>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> @@ -2881,51 +2881,50 @@ static int set_backlight_via_aux(struct dc_link *link, uint32_t brightness)
>>>> return rc ? 0 : 1;
>>>> }
>>>>
>>>> -static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
>>>> - const uint32_t user_brightness)
>>>> +static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
>>>> + unsigned *min, unsigned *max)
>>>> {
>>>> - u32 min, max, conversion_pace;
>>>> - u32 brightness = user_brightness;
>>>> -
>>>> if (!caps)
>>>> - goto out;
>>>> + return 0;
>>>>
>>>> - if (!caps->aux_support) {
>>>> - max = caps->max_input_signal;
>>>> - min = caps->min_input_signal;
>>>> - /*
>>>> - * The brightness input is in the range 0-255
>>>> - * It needs to be rescaled to be between the
>>>> - * requested min and max input signal
>>>> - * It also needs to be scaled up by 0x101 to
>>>> - * match the DC interface which has a range of
>>>> - * 0 to 0xffff
>>>> - */
>>>> - conversion_pace = 0x101;
>>>> - brightness =
>>>> - user_brightness
>>>> - * conversion_pace
>>>> - * (max - min)
>>>> - / AMDGPU_MAX_BL_LEVEL
>>>> - + min * conversion_pace;
>>>> + if (caps->aux_support) {
>>>> + // Firmware limits are in nits, DC API wants millinits.
>>>> + *max = 1000 * caps->aux_max_input_signal;
>>>> + *min = 1000 * caps->aux_min_input_signal;
>>>> } else {
>>>> - /* TODO
>>>> - * We are doing a linear interpolation here, which is OK but
>>>> - * does not provide the optimal result. We probably want
>>>> - * something close to the Perceptual Quantizer (PQ) curve.
>>>> - */
>>>> - max = caps->aux_max_input_signal;
>>>> - min = caps->aux_min_input_signal;
>>>> -
>>>> - brightness = (AMDGPU_MAX_BL_LEVEL - user_brightness) * min
>>>> - + user_brightness * max;
>>>> - // Multiple the value by 1000 since we use millinits
>>>> - brightness *= 1000;
>>>> - brightness = DIV_ROUND_CLOSEST(brightness, AMDGPU_MAX_BL_LEVEL);
>>>> + // Firmware limits are 8-bit, PWM control is 16-bit.
>>>> + *max = 0x101 * caps->max_input_signal;
>>>> + *min = 0x101 * caps->min_input_signal;
>>>> }
>>>> + return 1;
>>>> +}
>>>>
>>>> -out:
>>>> - return brightness;
>>>> +static u32 convert_brightness_from_user(const struct amdgpu_dm_backlight_caps *caps,
>>>> + uint32_t brightness)
>>>> +{
>>>> + unsigned min, max;
>>>> +
>>>> + if (!get_brightness_range(caps, &min, &max))
>>>> + return brightness;
>>>> +
>>>> + // Rescale 0..255 to min..max
>>>> + return min + DIV_ROUND_CLOSEST((max - min) * brightness,
>>>> + AMDGPU_MAX_BL_LEVEL);
>>>> +}
>>>> +
>>>> +static u32 convert_brightness_to_user(const struct amdgpu_dm_backlight_caps *caps,
>>>> + uint32_t brightness)
>>>> +{
>>>> + unsigned min, max;
>>>> +
>>>> + if (!get_brightness_range(caps, &min, &max))
>>>> + return brightness;
>>>> +
>>>> + if (brightness < min)
>>>> + return 0;
>>>> + // Rescale min..max to 0..255
>>>> + return DIV_ROUND_CLOSEST(AMDGPU_MAX_BL_LEVEL * (brightness - min),
>>>> + max - min);
>>>> }
>>>>
>>>> static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
>>>> @@ -2941,7 +2940,7 @@ 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_from_user(&caps, bd->props.brightness);
>>>> // Change brightness based on AUX property
>>>> if (caps.aux_support)
>>>> return set_backlight_via_aux(link, brightness);
>>>> @@ -2958,7 +2957,7 @@ static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)
>>>>
>>>> if (ret == DC_ERROR_UNEXPECTED)
>>>> return bd->props.brightness;
>>>> - return ret;
>>>> + return convert_brightness_to_user(&dm->backlight_caps, ret);
>>>> }
>>>>
>>>> static const struct backlight_ops amdgpu_dm_backlight_ops = {
>>>>
>>>> base-commit: bcf876870b95592b52519ed4aafcf9d95999bc9c
>>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@...ts.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Powered by blists - more mailing lists