From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/2 v3] drm/i915: intel_backlight scale() math WA Date: Tue, 30 Sep 2014 10:04:37 +0200 Message-ID: <20140930080437.GD24667@phenom.ffwll.local> References: <1412030973-16738-1-git-send-email-ullysses.a.eoff@intel.com> <1412030973-16738-2-git-send-email-ullysses.a.eoff@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f176.google.com (mail-wi0-f176.google.com [209.85.212.176]) by gabe.freedesktop.org (Postfix) with ESMTP id 823C56E6BE for ; Tue, 30 Sep 2014 01:04:42 -0700 (PDT) Received: by mail-wi0-f176.google.com with SMTP id n3so4982870wiv.3 for ; Tue, 30 Sep 2014 01:04:40 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1412030973-16738-2-git-send-email-ullysses.a.eoff@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "U. Artie Eoff" Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Sep 29, 2014 at 03:49:32PM -0700, U. Artie Eoff wrote: > 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 > > v3: -DIV_ROUND_CLOSEST() fails to build with CONFIG_X86_32=y. (Jani) > -Use DIV_ROUND_CLOSEST_ULL() instead. (Damien) > -v1 and v2 originally authored by Joe Konno. > > Signed-off-by: U. Artie Eoff Is there some bug report, internal jira, mailing list reference or similar about this? Note that at least for OTC jira tasks we now want them to be added to commit message with e.g. OTC-Jria: VIZ-4932 And I guess I should merge patch 2 before patch 1, right? Cheers, Daniel > --- > drivers/gpu/drm/i915/intel_panel.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index f17ada3..f7da913 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -398,6 +398,9 @@ intel_panel_detect(struct drm_device *dev) > } > } > > +#define DIV_ROUND_CLOSEST_ULL(ll, d) \ > +({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; }) > + > /** > * scale - scale values from one range to another > * > @@ -419,9 +422,8 @@ static uint32_t scale(uint32_t source_val, > source_val = clamp(source_val, source_min, source_max); > > /* 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_ULL((uint64_t)(source_val - source_min) * > + (target_max - target_min), source_max - source_min); > target_val += target_min; > > return target_val; > -- > 1.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch