From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: restore backlight precision when converting from opregion Date: Mon, 28 Apr 2014 15:41:47 +0200 Message-ID: <20140428134147.GB32404@phenom.ffwll.local> References: <535DC8C1.5090407@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f49.google.com (mail-ee0-f49.google.com [74.125.83.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 0B3FF6E68C for ; Mon, 28 Apr 2014 06:41:51 -0700 (PDT) Received: by mail-ee0-f49.google.com with SMTP id c41so4915595eek.36 for ; Mon, 28 Apr 2014 06:41:51 -0700 (PDT) Content-Disposition: inline In-Reply-To: <535DC8C1.5090407@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Aaron Lu Cc: "dri-devel@lists.freedesktop.org" , Nico Schottelius , "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Mon, Apr 28, 2014 at 11:19:29AM +0800, Aaron Lu wrote: > 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-and-tested-by: Nico Schottelius > Signed-off-by: Aaron Lu > --- > drivers/gpu/drm/i915/intel_panel.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index a953b081ee38..bdd2f24b7a6b 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -502,10 +502,7 @@ 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; > + level = (u64)level * freq / max; 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 ;-) -Daniel > > panel->backlight.level = level; > if (panel->backlight.device) > -- > 1.9.0 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch