[Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace

[Stéphane Marchesin]

On Tue, Nov 18, 2014 at 3:18 PM, Eoff, Ullysses A
<ullysses.a.eoff at intel.com> wrote:
> Thanks Jesse for the ack.
>
> Unfortunately I just learned from Stéphane that there
> are certain devices which only support 256 levels, so this
> patch would do us no good at solving the real issue for
> such devices.
>
> Why can't we just use a dynamic 1:1 mapping as was
> suggested before?  I would vote for that instead.
>

Right, from my (consumer's) perspective, a 1:1 mapping is simpler. But
the confusing part for me is that (as far as I can see) the current
mapping should be 1:1 (because the user and hw ranges are the same),
even though it goes through the scale logic. Is the scale() function
maybe not the identity? If it isn't, maybe we just need to make it
so...

Stéphane


> ----
> U. Artie
>
>> -----Original Message-----
>> From: Jesse Barnes [mailto:jbarnes at virtuousgeek.org]
>> Sent: Tuesday, November 18, 2014 9:23 AM
>> To: Eoff, Ullysses A
>> Cc: intel-gfx at lists.freedesktop.org; Jani Nikula
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace
>>
>> On Tue, 11 Nov 2014 12:30:38 -0800
>> "U. Artie Eoff" <ullysses.a.eoff at intel.com> wrote:
>>
>> > A userspace brightness range that is larger than the hardware
>> > brightness range does not have a 1:1 mapping and can result
>> > in brightness != actual_brightness for some values.
>> >
>> > Expose a fixed brightness range of [0..1000] to userspace so
>> > that all values can map 1:1 into the hardware brightness
>> > range.  This would assume that the hardware range is always
>> > greater than 1000, otherwise we're right back to the original
>> > issue.
>> >
>> > This patch is based on Jani Nikula's proposed change in the
>> > referenced ML thread, except use the range [0..1000] instead;
>> > which was recommended by Jesse Barnes for smoother backlight
>> > transitions.
>> >
>> > Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html
>> > Signed-off-by: U. Artie Eoff <ullysses.a.eoff at intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_panel.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> > index 4d63839..f74f5f2 100644
>> > --- a/drivers/gpu/drm/i915/intel_panel.c
>> > +++ b/drivers/gpu/drm/i915/intel_panel.c
>> > @@ -1057,7 +1057,7 @@ static int intel_backlight_device_register(struct intel_connector *connector)
>> >      * Note: Everything should work even if the backlight device max
>> >      * presented to the userspace is arbitrarily chosen.
>> >      */
>> > -   props.max_brightness = panel->backlight.max;
>> > +   props.max_brightness = 1000;
>> >     props.brightness = scale_hw_to_user(connector,
>> >                                         panel->backlight.level,
>> >                                         props.max_brightness);
>>
>> Yeah looks ok to me.  I guess Jani has to ack it too.
>>
>> Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
>>
>> --
>> Jesse Barnes, Intel Open Source Technology Center