[Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA v2

[Eoff, Ullysses A]

On 09/24/2014 10:42 AM, Eoff, Ullysses A wrote:
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of Jani Nikula
>> Sent: Wednesday, September 24, 2014 10:08 AM
>> To: Hans de Goede; Joe Konno; intel-gfx at lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA v2
>>
>> On Wed, 24 Sep 2014, Hans de Goede <hdegoede at redhat.com> wrote:
>>> Hi,
>>>
>>> On 09/24/2014 05:54 PM, Joe Konno wrote:
>>>> From: Joe Konno <joe.konno at intel.com>
>>>>
>>>> Improper truncated integer division in the scale() function causes
>>>> actual_brightness != brightness. This (partial) work-around should be
>>>> sufficient for a majority of use-cases, but it is by no means a complete
>>>> solution.
>>>>
>>>> TODO: Determine how best to scale "user" values to "hw" values, and
>>>> vice-versa, when the ranges are of different sizes. That would be a
>>>> buggy scenario even with this work-around.
>>>>
>>>> The issue was introduced in the following (v3.17-rc1) commit:
>>>>
>>>>     6dda730 drm/i915: respect the VBT minimum backlight brightness
>>>>
>>>> v2: (thanks to Chris Wilson) clarify commit message, use rounded division
>>>> macro
>>> I wonder why do scaling at all, why not simply shift hw_min - hw_max range
>>> to 0 - (hw_max - hw_min) range and set max_brightness as seen by userspace
>>> to (hw_max - hw_min) ?
>> Mostly in preparation for a future where we expose an arbitrary range,
>> say 0..100 or 0..255 to the userspace.
>>
> The problem with this scaling method is that scaling from user level to hw level and
> back to user level is ambiguous since there isn't a 1:1 mapping between the user
> range and the hw range.
>
> On the other hand, this patch does fix a bug in the currently used method (scaling).
> That, at least, is an improvement nonetheless.
>
> U. Artie
Apologies for resurrecting an old thread.  But I think we still need to
address
this issue about not having a 1:1 mapping between user and hw levels.

Right now, the problem is that the user range is larger than the hw
range which
results in one or more user levels mapping to the same hw level.  And when
userspace requests one of those levels, the result that is reported back to
userspace might not be the same as what was requested.  Take for example, on
my system the hw range is [398, 7812] and the user range is [0, 7812]. 
Suppose
userspace requests level 7017.  This maps to hw level 7058.  And when
userspace requests the current level, 7018 is reported back (+1 from what
was originally requested).  In fact, with these particular ranges, there
are exactly
398 values that this occurs.

This problem will be compounded the larger the difference in length of the
discrete ranges; so long as user range > hw range.

Hans' solution would fix this problem, giving 1:1 mapping from hw to user
levels.

Jani's [future] solution would work too, since exposing a smaller range to
userspace than the hw range would isolate the non 1:1 mapping inside the
driver.

U. Artie
>> BR,
>> Jani.
>>
>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>>> Signed-off-by: Joe Konno <joe.konno 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 f17ada3..dcdfbb3 100644
>>>> --- a/drivers/gpu/drm/i915/intel_panel.c
>>>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>>>> @@ -421,7 +421,7 @@ static uint32_t scale(uint32_t source_val,
>>>>  	/* avoid overflows */
>>>>  	target_val = (uint64_t)(source_val - source_min) *
>>>>  		(target_max - target_min);
>>>> -	do_div(target_val, source_max - source_min);
>>>> +	target_val = DIV_ROUND_CLOSEST(target_val, source_max - source_min);
>>>>  	target_val += target_min;
>>>>
>>>>  	return target_val;
>>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> --
>> Jani Nikula, Intel Open Source Technology Center
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>