[PATCH v2] drm/amd/display: use correct scale for actual_brightness
Alex Deucher
alexdeucher at gmail.com
Mon Aug 17 17:59:28 UTC 2020
On Mon, Aug 17, 2020 at 3:09 AM Alexander Monakov <amonakov at ispras.ru> wrote:
>
> Ping.
Patch looks good to me:
Reviewed-by: Alex Deucher <alexander.deucher at amd.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 at amd.com>
> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> > Signed-off-by: Alexander Monakov <amonakov at ispras.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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list