[Intel-gfx] [PATCH v2] drm/i915: restore backlight precision when converting from opregion
Aaron Lu
aaron.lu at intel.com
Sun May 4 09:31:01 CEST 2014
On 05/04/2014 03:22 PM, Chris Wilson wrote:
> On Sun, May 04, 2014 at 03:16:05PM +0800, Aaron Lu wrote:
>> On 04/28/2014 09:41 PM, Daniel Vetter wrote:
>>> 64bit divisions won't compile on 32bit. You need one of the DO_DIV macros,
>>> or whatever they're called again. I pain, I know ;-)
>>
>> Thanks for the correction, here is an updated patch :-)
>>
>> From: Aaron Lu <aaron.lu at intel.com>
>> Date: Mon, 28 Apr 2014 11:02:52 +0800
>> Subject: [PATCH v2] drm/i915: restore backlight precision when converting from
>> ACPI
>>
>> When we set backlight on behalf of ACPI opregion, we will convert the
>> backlight value in the 0-255 range defined in opregion to the actual
>> hardware level. Commit 22505b82a2 (drm/i915: avoid brightness overflow
>> when doing scale) is meant to fix the overflow problem when doing the
>> conversion, but it also caused a problem that the converted hardware
>> level doesn't quite represent the intended value: say user wants maximum
>> backlight level(255 in opregion's range), then we will calculate the
>> actual hardware level to be: level = freq / max * level, where freq is
>> the hardware's max backlight level(937 on an user's box), and max and
>> level are all 255. The converted value should be 937 but the above
>> calculation will yield 765.
>>
>> To fix this issue, just use 64 bits to do the calculation to keep the
>> precision and avoid overflow at the same time.
>>
>> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=72491
>> Reported-by: Nico Schottelius <nico-bugzilla.kernel.org at schottelius.org>
>> Signed-off-by: Aaron Lu <aaron.lu at intel.com>
>> ---
>> v2: use do_div as reminded by Daniel.
>>
>> drivers/gpu/drm/i915/intel_panel.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> index a953b081ee38..8725917a3d0d 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -492,6 +492,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
>> enum pipe pipe = intel_get_pipe_from_connector(connector);
>> u32 freq;
>> unsigned long flags;
>> + u64 n;
>>
>> if (!panel->backlight.present || pipe == INVALID_PIPE)
>> return;
>> @@ -502,10 +503,9 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
>>
>> /* scale to hardware max, but be careful to not overflow */
>> freq = panel->backlight.max;
>> - if (freq < max)
>> - level = level * freq / max;
>> - else
>> - level = freq / max * level;
>> + n = level * freq;
>
> 32b * 32b = 32b
>
> n = (u64)level * freq; to avoid overflow as you claim.
Ah...yes, my fault, thanks.
>
> Also this still has the same rounding error as before.
I didn't get this, care to explain?
More information about the Intel-gfx
mailing list