Re: [PATCH v3] drm/i915: restore backlight precision when converting from opregion

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 12 May 2014, Aaron Lu <aaron.lu@intel.com> wrote:
> On 05/04/2014 03:22 PM, Chris Wilson wrote:
>> 32b * 32b = 32b
>> 
>> n = (u64)level * freq; to avoid overflow as you claim.
>
> Updated patch to fix this problem is here, thanks!

Pushed to -fixes with Chris' IRC r-b, thanks for the patch and review.

BR,
Jani.



>
>
> From a0f41a92d949c814c203672ff7efe219a90ca6df Mon Sep 17 00:00:00 2001
> From: Aaron Lu <aaron.lu@intel.com>
> Date: Mon, 28 Apr 2014 11:02:52 +0800
> Subject: [PATCH] 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@schottelius.org>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
>  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 0eead16aeda7..cb8cfb7e0974 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 = (u64)level * freq;
> +	do_div(n, max);
> +	level = n;
>  
>  	panel->backlight.level = level;
>  	if (panel->backlight.device)
> -- 
> 1.9.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center