[PATCH 1/5] backlight: pwm_bl: fix cie1913 comments and constant

[PATCH 1/5] backlight: pwm_bl: fix cie1913 comments and constant

[Rasmus Villemoes]

The "break-even" point for the two formulas is L==8, which is also
what the code actually implements. [Incidentally, at that point one
has Y=0.008856, not 0.08856].

Moreover, all the sources I can find say the linear factor is 903.3
rather than 902.3, which makes sense since then the formulas agree at
L==8, both yielding the 0.008856 figure to four significant digits.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/video/backlight/pwm_bl.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 2201b8c78641..be36be1cacb7 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -155,8 +155,8 @@ static const struct backlight_ops pwm_backlight_ops = {
  *
  * The CIE 1931 lightness formula is what actually describes how we perceive
  * light:
- *          Y = (L* / 902.3)           if L* ≤ 0.08856
- *          Y = ((L* + 16) / 116)^3    if L* > 0.08856
+ *          Y = (L* / 903.3)           if L* ≤ 8
+ *          Y = ((L* + 16) / 116)^3    if L* > 8
  *
  * Where Y is the luminance, the amount of light coming out of the screen, and
  * is a number between 0.0 and 1.0; and L* is the lightness, how bright a human
@@ -169,9 +169,15 @@ static u64 cie1931(unsigned int lightness, unsigned int scale)
 {
 	u64 retval;
 
+	/*
+	 * @lightness is given as a number between 0 and 1, expressed
+	 * as a fixed-point number in scale @scale. Convert to a
+	 * percentage, still expressed as a fixed-point number, so the
+	 * above formulas can be applied.
+	 */
 	lightness *= 100;
 	if (lightness <= (8 * scale)) {
-		retval = DIV_ROUND_CLOSEST_ULL(lightness * 10, 9023);
+		retval = DIV_ROUND_CLOSEST_ULL(lightness * 10, 9033);
 	} else {
 		retval = int_pow((lightness + (16 * scale)) / 116, 3);
 		retval = DIV_ROUND_CLOSEST_ULL(retval, (scale * scale));
-- 
2.20.1

[PATCH 2/5] backlight: pwm_bl: eliminate a 64/32 division

[Rasmus Villemoes]

lightness*1000 is nowhere near overflowing 32 bits, so we can just use
an ordinary 32/32 division, which is much cheaper than the 64/32 done
via do_div().

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/video/backlight/pwm_bl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index be36be1cacb7..9252d51f31b9 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -177,7 +177,7 @@ static u64 cie1931(unsigned int lightness, unsigned int scale)
 	 */
 	lightness *= 100;
 	if (lightness <= (8 * scale)) {
-		retval = DIV_ROUND_CLOSEST_ULL(lightness * 10, 9033);
+		retval = DIV_ROUND_CLOSEST(lightness * 10, 9033);
 	} else {
 		retval = int_pow((lightness + (16 * scale)) / 116, 3);
 		retval = DIV_ROUND_CLOSEST_ULL(retval, (scale * scale));
-- 
2.20.1

[PATCH 3/5] backlight: pwm_bl: drop use of int_pow()

[Rasmus Villemoes]

The scheduler uses a (currently private) fixed_power_int() in its load
average computation for computing powers of numbers 0 < x < 1
expressed as fixed-point numbers, which is also what we want here. But
that requires the scale to be a power-of-2.

We could (and a following patch will) change to use a power-of-2 scale,
but for a fixed small exponent of 3, there's no advantage in using
repeated squaring.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/video/backlight/pwm_bl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 9252d51f31b9..aee6839e024a 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -179,7 +179,8 @@ static u64 cie1931(unsigned int lightness, unsigned int scale)
 	if (lightness <= (8 * scale)) {
 		retval = DIV_ROUND_CLOSEST(lightness * 10, 9033);
 	} else {
-		retval = int_pow((lightness + (16 * scale)) / 116, 3);
+		retval = (lightness + (16 * scale)) / 116;
+		retval *= retval * retval;
 		retval = DIV_ROUND_CLOSEST_ULL(retval, (scale * scale));
 	}
 
-- 
2.20.1

[PATCH 4/5] backlight: pwm_bl: switch to power-of-2 base for fixed-point math

[Rasmus Villemoes]

Using a power-of-2 instead of power-of-10 base makes the computations
much cheaper. 2^16 is safe; retval never becomes more than 2^48 +
2^16/2. On a 32 bit platform, the very expensive 64/32 division at the
end of cie1931() instead becomes essentially free (a shift by 32 is
just a register rename).

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/video/backlight/pwm_bl.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index aee6839e024a..102bc191310f 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -148,7 +148,8 @@ static const struct backlight_ops pwm_backlight_ops = {
 };
 
 #ifdef CONFIG_OF
-#define PWM_LUMINANCE_SCALE	10000 /* luminance scale */
+#define PWM_LUMINANCE_SHIFT	16
+#define PWM_LUMINANCE_SCALE	(1 << PWM_LUMINANCE_SHIFT) /* luminance scale */
 
 /*
  * CIE lightness to PWM conversion.
@@ -165,23 +166,25 @@ static const struct backlight_ops pwm_backlight_ops = {
  * The following function does the fixed point maths needed to implement the
  * above formula.
  */
-static u64 cie1931(unsigned int lightness, unsigned int scale)
+static u64 cie1931(unsigned int lightness)
 {
 	u64 retval;
 
 	/*
 	 * @lightness is given as a number between 0 and 1, expressed
-	 * as a fixed-point number in scale @scale. Convert to a
-	 * percentage, still expressed as a fixed-point number, so the
-	 * above formulas can be applied.
+	 * as a fixed-point number in scale
+	 * PWM_LUMINANCE_SCALE. Convert to a percentage, still
+	 * expressed as a fixed-point number, so the above formulas
+	 * can be applied.
 	 */
 	lightness *= 100;
-	if (lightness <= (8 * scale)) {
+	if (lightness <= (8 * PWM_LUMINANCE_SCALE)) {
 		retval = DIV_ROUND_CLOSEST(lightness * 10, 9033);
 	} else {
-		retval = (lightness + (16 * scale)) / 116;
+		retval = (lightness + (16 * PWM_LUMINANCE_SCALE)) / 116;
 		retval *= retval * retval;
-		retval = DIV_ROUND_CLOSEST_ULL(retval, (scale * scale));
+		retval += PWM_LUMINANCE_SCALE/2;
+		retval >>= 2*PWM_LUMINANCE_SHIFT;
 	}
 
 	return retval;
@@ -215,8 +218,7 @@ int pwm_backlight_brightness_default(struct device *dev,
 	/* Fill the table using the cie1931 algorithm */
 	for (i = 0; i < data->max_brightness; i++) {
 		retval = cie1931((i * PWM_LUMINANCE_SCALE) /
-				 data->max_brightness, PWM_LUMINANCE_SCALE) *
-				 period;
+				 data->max_brightness) * period;
 		retval = DIV_ROUND_CLOSEST_ULL(retval, PWM_LUMINANCE_SCALE);
 		if (retval > UINT_MAX)
 			return -EINVAL;
-- 
2.20.1

[PATCH 5/5] lib/math: remove int_pow()

[Rasmus Villemoes]

No users left.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 Documentation/core-api/kernel-api.rst |  3 ---
 include/linux/kernel.h                |  1 -
 lib/math/Makefile                     |  2 +-
 lib/math/int_pow.c                    | 32 ---------------------------
 4 files changed, 1 insertion(+), 37 deletions(-)
 delete mode 100644 lib/math/int_pow.c

diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst
index 08af5caf036d..5f9cf47581b3 100644
--- a/Documentation/core-api/kernel-api.rst
+++ b/Documentation/core-api/kernel-api.rst
@@ -144,9 +144,6 @@ Base 2 log and power Functions
 Integer power Functions
 -----------------------
 
-.. kernel-doc:: lib/math/int_pow.c
-   :export:
-
 .. kernel-doc:: lib/math/int_sqrt.c
    :export:
 
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 4fa360a13c1e..afe7c2cc81aa 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -487,7 +487,6 @@ extern int __kernel_text_address(unsigned long addr);
 extern int kernel_text_address(unsigned long addr);
 extern int func_ptr_is_kernel_text(void *ptr);
 
-u64 int_pow(u64 base, unsigned int exp);
 unsigned long int_sqrt(unsigned long);
 
 #if BITS_PER_LONG < 64
diff --git a/lib/math/Makefile b/lib/math/Makefile
index be6909e943bd..3e5db680a404 100644
--- a/lib/math/Makefile
+++ b/lib/math/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-y += div64.o gcd.o lcm.o int_pow.o int_sqrt.o reciprocal_div.o
+obj-y += div64.o gcd.o lcm.o int_sqrt.o reciprocal_div.o
 
 obj-$(CONFIG_CORDIC)		+= cordic.o
 obj-$(CONFIG_PRIME_NUMBERS)	+= prime_numbers.o
diff --git a/lib/math/int_pow.c b/lib/math/int_pow.c
deleted file mode 100644
index 622fc1ab3c74..000000000000
--- a/lib/math/int_pow.c
+++ /dev/null
@@ -1,32 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * An integer based power function
- *
- * Derived from drivers/video/backlight/pwm_bl.c
- */
-
-#include <linux/export.h>
-#include <linux/kernel.h>
-#include <linux/types.h>
-
-/**
- * int_pow - computes the exponentiation of the given base and exponent
- * @base: base which will be raised to the given power
- * @exp: power to be raised to
- *
- * Computes: pow(base, exp), i.e. @base raised to the @exp power
- */
-u64 int_pow(u64 base, unsigned int exp)
-{
-	u64 result = 1;
-
-	while (exp) {
-		if (exp & 1)
-			result *= base;
-		exp >>= 1;
-		base *= base;
-	}
-
-	return result;
-}
-EXPORT_SYMBOL_GPL(int_pow);
-- 
2.20.1

Re: [PATCH 5/5] lib/math: remove int_pow()

[Andy Shevchenko]

On Thu, Sep 19, 2019 at 04:06:20PM +0200, Rasmus Villemoes wrote:
> No users left.

There are in linux-next.

NAK.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/5] backlight: pwm_bl: fix cie1913 comments and constant

[Daniel Thompson]

Hi Rasmus

Has something gone wrong with the mail out for this patch set. I didn't
get a covering letter or patch 5/5?


Daniel.

Re: [PATCH 1/5] backlight: pwm_bl: fix cie1913 comments and constant

[Rasmus Villemoes]

On 20/09/2019 12.14, Daniel Thompson wrote:
> Hi Rasmus
> 
> Has something gone wrong with the mail out for this patch set. I didn't
> get a covering letter or patch 5/5?

Sorry about that. I should have included a cover letter so you'd know
that patch 5 wasn't directly related to the other patches.

https://lore.kernel.org/lkml/20190919140620.32407-5-linux@rasmusvillemoes.dk/

I was removing the now unused int_pow() function, but Andy has told me
there are new users in -next, so it's moot. Only the first four patches
are relevant.

Thanks,
Rasmus

Re: [PATCH 5/5] lib/math: remove int_pow()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5651 bytes --]

Hi Rasmus,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190918]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Rasmus-Villemoes/backlight-pwm_bl-fix-cie1913-comments-and-constant/20190919-225123
config: x86_64-randconfig-s0-201937 (attached as .config)
compiler: gcc-5 (Ubuntu 5.5.0-12ubuntu1) 5.5.0 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/iio/common/hid-sensors/hid-sensor-attributes.c: In function 'simple_div':
>> drivers/iio/common/hid-sensors/hid-sensor-attributes.c:94:35: error: implicit declaration of function 'int_pow' [-Werror=implicit-function-declaration]
      *micro_frac = (rem / divisor) * int_pow(10, 6 - exp);
                                      ^
   Cyclomatic Complexity 3 include/linux/hid-sensor-hub.h:hid_sensor_convert_exponent
   Cyclomatic Complexity 1 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_convert_timestamp
   Cyclomatic Complexity 3 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_batch_mode_supported
   Cyclomatic Complexity 4 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_read_poll_value
   Cyclomatic Complexity 2 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_get_report_latency
   Cyclomatic Complexity 4 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:simple_div
   Cyclomatic Complexity 5 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_read_samp_freq_value
   Cyclomatic Complexity 1 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:split_micro_fraction
   Cyclomatic Complexity 4 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:convert_from_vtf_format
   Cyclomatic Complexity 3 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_read_raw_hyst_value
   Cyclomatic Complexity 4 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:convert_to_vtf_format
   Cyclomatic Complexity 7 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:adjust_exponent_nano
   Cyclomatic Complexity 4 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_format_scale
   Cyclomatic Complexity 9 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_write_samp_freq_value
   Cyclomatic Complexity 6 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_write_raw_hyst_value
   Cyclomatic Complexity 1 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_set_report_latency
   Cyclomatic Complexity 2 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_get_reporting_interval
   Cyclomatic Complexity 2 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_get_report_latency_info
   Cyclomatic Complexity 6 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_parse_common_attributes
   cc1: some warnings being treated as errors

# https://github.com/0day-ci/linux/commit/d89cf2c7a3840d1691169c9e099c7def0dc16c46
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout d89cf2c7a3840d1691169c9e099c7def0dc16c46
vim +/int_pow +94 drivers/iio/common/hid-sensors/hid-sensor-attributes.c

5d02edfc395744 Srinivas Pandruvada 2014-04-19  75  
73c6768b710a16 srinivas pandruvada 2012-09-05  76  static void simple_div(int dividend, int divisor, int *whole,
73c6768b710a16 srinivas pandruvada 2012-09-05  77  				int *micro_frac)
73c6768b710a16 srinivas pandruvada 2012-09-05  78  {
73c6768b710a16 srinivas pandruvada 2012-09-05  79  	int rem;
73c6768b710a16 srinivas pandruvada 2012-09-05  80  	int exp = 0;
73c6768b710a16 srinivas pandruvada 2012-09-05  81  
73c6768b710a16 srinivas pandruvada 2012-09-05  82  	*micro_frac = 0;
73c6768b710a16 srinivas pandruvada 2012-09-05  83  	if (divisor == 0) {
73c6768b710a16 srinivas pandruvada 2012-09-05  84  		*whole = 0;
73c6768b710a16 srinivas pandruvada 2012-09-05  85  		return;
73c6768b710a16 srinivas pandruvada 2012-09-05  86  	}
73c6768b710a16 srinivas pandruvada 2012-09-05  87  	*whole = dividend/divisor;
73c6768b710a16 srinivas pandruvada 2012-09-05  88  	rem = dividend % divisor;
73c6768b710a16 srinivas pandruvada 2012-09-05  89  	if (rem) {
73c6768b710a16 srinivas pandruvada 2012-09-05  90  		while (rem <= divisor) {
73c6768b710a16 srinivas pandruvada 2012-09-05  91  			rem *= 10;
73c6768b710a16 srinivas pandruvada 2012-09-05  92  			exp++;
73c6768b710a16 srinivas pandruvada 2012-09-05  93  		}
473d12f7638c93 Andy Shevchenko     2019-06-19 @94  		*micro_frac = (rem / divisor) * int_pow(10, 6 - exp);
73c6768b710a16 srinivas pandruvada 2012-09-05  95  	}
73c6768b710a16 srinivas pandruvada 2012-09-05  96  }
73c6768b710a16 srinivas pandruvada 2012-09-05  97  

:::::: The code at line 94 was first introduced by commit
:::::: 473d12f7638c93acbd9296a8cd455b203d5eb528 iio: hid-sensor-attributes: Convert to use int_pow()

:::::: TO: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
:::::: CC: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31905 bytes --]

Re: [PATCH 5/5] lib/math: remove int_pow()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5651 bytes --]

Hi Rasmus,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190918]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Rasmus-Villemoes/backlight-pwm_bl-fix-cie1913-comments-and-constant/20190919-225123
config: x86_64-randconfig-s0-201937 (attached as .config)
compiler: gcc-5 (Ubuntu 5.5.0-12ubuntu1) 5.5.0 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/iio/common/hid-sensors/hid-sensor-attributes.c: In function 'simple_div':
>> drivers/iio/common/hid-sensors/hid-sensor-attributes.c:94:35: error: implicit declaration of function 'int_pow' [-Werror=implicit-function-declaration]
      *micro_frac = (rem / divisor) * int_pow(10, 6 - exp);
                                      ^
   Cyclomatic Complexity 3 include/linux/hid-sensor-hub.h:hid_sensor_convert_exponent
   Cyclomatic Complexity 1 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_convert_timestamp
   Cyclomatic Complexity 3 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_batch_mode_supported
   Cyclomatic Complexity 4 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_read_poll_value
   Cyclomatic Complexity 2 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_get_report_latency
   Cyclomatic Complexity 4 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:simple_div
   Cyclomatic Complexity 5 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_read_samp_freq_value
   Cyclomatic Complexity 1 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:split_micro_fraction
   Cyclomatic Complexity 4 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:convert_from_vtf_format
   Cyclomatic Complexity 3 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_read_raw_hyst_value
   Cyclomatic Complexity 4 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:convert_to_vtf_format
   Cyclomatic Complexity 7 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:adjust_exponent_nano
   Cyclomatic Complexity 4 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_format_scale
   Cyclomatic Complexity 9 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_write_samp_freq_value
   Cyclomatic Complexity 6 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_write_raw_hyst_value
   Cyclomatic Complexity 1 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_set_report_latency
   Cyclomatic Complexity 2 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_get_reporting_interval
   Cyclomatic Complexity 2 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_get_report_latency_info
   Cyclomatic Complexity 6 drivers/iio/common/hid-sensors/hid-sensor-attributes.c:hid_sensor_parse_common_attributes
   cc1: some warnings being treated as errors

# https://github.com/0day-ci/linux/commit/d89cf2c7a3840d1691169c9e099c7def0dc16c46
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout d89cf2c7a3840d1691169c9e099c7def0dc16c46
vim +/int_pow +94 drivers/iio/common/hid-sensors/hid-sensor-attributes.c

5d02edfc395744 Srinivas Pandruvada 2014-04-19  75  
73c6768b710a16 srinivas pandruvada 2012-09-05  76  static void simple_div(int dividend, int divisor, int *whole,
73c6768b710a16 srinivas pandruvada 2012-09-05  77  				int *micro_frac)
73c6768b710a16 srinivas pandruvada 2012-09-05  78  {
73c6768b710a16 srinivas pandruvada 2012-09-05  79  	int rem;
73c6768b710a16 srinivas pandruvada 2012-09-05  80  	int exp = 0;
73c6768b710a16 srinivas pandruvada 2012-09-05  81  
73c6768b710a16 srinivas pandruvada 2012-09-05  82  	*micro_frac = 0;
73c6768b710a16 srinivas pandruvada 2012-09-05  83  	if (divisor == 0) {
73c6768b710a16 srinivas pandruvada 2012-09-05  84  		*whole = 0;
73c6768b710a16 srinivas pandruvada 2012-09-05  85  		return;
73c6768b710a16 srinivas pandruvada 2012-09-05  86  	}
73c6768b710a16 srinivas pandruvada 2012-09-05  87  	*whole = dividend/divisor;
73c6768b710a16 srinivas pandruvada 2012-09-05  88  	rem = dividend % divisor;
73c6768b710a16 srinivas pandruvada 2012-09-05  89  	if (rem) {
73c6768b710a16 srinivas pandruvada 2012-09-05  90  		while (rem <= divisor) {
73c6768b710a16 srinivas pandruvada 2012-09-05  91  			rem *= 10;
73c6768b710a16 srinivas pandruvada 2012-09-05  92  			exp++;
73c6768b710a16 srinivas pandruvada 2012-09-05  93  		}
473d12f7638c93 Andy Shevchenko     2019-06-19 @94  		*micro_frac = (rem / divisor) * int_pow(10, 6 - exp);
73c6768b710a16 srinivas pandruvada 2012-09-05  95  	}
73c6768b710a16 srinivas pandruvada 2012-09-05  96  }
73c6768b710a16 srinivas pandruvada 2012-09-05  97  

:::::: The code at line 94 was first introduced by commit
:::::: 473d12f7638c93acbd9296a8cd455b203d5eb528 iio: hid-sensor-attributes: Convert to use int_pow()

:::::: TO: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
:::::: CC: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31905 bytes --]

Re: [PATCH 1/5] backlight: pwm_bl: fix cie1913 comments and constant

[Daniel Thompson]

On Thu, Sep 19, 2019 at 04:06:16PM +0200, Rasmus Villemoes wrote:
> The "break-even" point for the two formulas is L==8, which is also
> what the code actually implements. [Incidentally, at that point one
> has Y=0.008856, not 0.08856].
> 
> Moreover, all the sources I can find say the linear factor is 903.3
> rather than 902.3, which makes sense since then the formulas agree at
> L==8, both yielding the 0.008856 figure to four significant digits.

Indeed. Interestingly the following doc (with a high search rank in
Google) has exactly this inconsistency and uses different values at
different times:
http://www.photonstophotos.net/GeneralTopics/Exposure/Psychometric_Lightness_and_Gamma.htm

> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>

> ---
>  drivers/video/backlight/pwm_bl.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 2201b8c78641..be36be1cacb7 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -155,8 +155,8 @@ static const struct backlight_ops pwm_backlight_ops = {
>   *
>   * The CIE 1931 lightness formula is what actually describes how we perceive
>   * light:
> - *          Y = (L* / 902.3)           if L* ≤ 0.08856
> - *          Y = ((L* + 16) / 116)^3    if L* > 0.08856
> + *          Y = (L* / 903.3)           if L* ≤ 8
> + *          Y = ((L* + 16) / 116)^3    if L* > 8
>   *
>   * Where Y is the luminance, the amount of light coming out of the screen, and
>   * is a number between 0.0 and 1.0; and L* is the lightness, how bright a human
> @@ -169,9 +169,15 @@ static u64 cie1931(unsigned int lightness, unsigned int scale)
>  {
>  	u64 retval;
>  
> +	/*
> +	 * @lightness is given as a number between 0 and 1, expressed
> +	 * as a fixed-point number in scale @scale. Convert to a
> +	 * percentage, still expressed as a fixed-point number, so the
> +	 * above formulas can be applied.
> +	 */
>  	lightness *= 100;
>  	if (lightness <= (8 * scale)) {
> -		retval = DIV_ROUND_CLOSEST_ULL(lightness * 10, 9023);
> +		retval = DIV_ROUND_CLOSEST_ULL(lightness * 10, 9033);
>  	} else {
>  		retval = int_pow((lightness + (16 * scale)) / 116, 3);
>  		retval = DIV_ROUND_CLOSEST_ULL(retval, (scale * scale));
> -- 
> 2.20.1

Re: [PATCH 2/5] backlight: pwm_bl: eliminate a 64/32 division

[Daniel Thompson]

On Thu, Sep 19, 2019 at 04:06:17PM +0200, Rasmus Villemoes wrote:
> lightness*1000 is nowhere near overflowing 32 bits, so we can just use
> an ordinary 32/32 division, which is much cheaper than the 64/32 done
> via do_div().
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>

> ---
>  drivers/video/backlight/pwm_bl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index be36be1cacb7..9252d51f31b9 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -177,7 +177,7 @@ static u64 cie1931(unsigned int lightness, unsigned int scale)
>  	 */
>  	lightness *= 100;
>  	if (lightness <= (8 * scale)) {
> -		retval = DIV_ROUND_CLOSEST_ULL(lightness * 10, 9033);
> +		retval = DIV_ROUND_CLOSEST(lightness * 10, 9033);
>  	} else {
>  		retval = int_pow((lightness + (16 * scale)) / 116, 3);
>  		retval = DIV_ROUND_CLOSEST_ULL(retval, (scale * scale));
> -- 
> 2.20.1

Re: [PATCH 3/5] backlight: pwm_bl: drop use of int_pow()

[Daniel Thompson]

On Thu, Sep 19, 2019 at 04:06:18PM +0200, Rasmus Villemoes wrote:
> The scheduler uses a (currently private) fixed_power_int() in its load
> average computation for computing powers of numbers 0 < x < 1
> expressed as fixed-point numbers, which is also what we want here. But
> that requires the scale to be a power-of-2.

It feels like there is some rationale missing in the description here.

What is the benefit of replacing the explicit int_pow() with the
implicit multiplications?


Daniel.


> 
> We could (and a following patch will) change to use a power-of-2 scale,
> but for a fixed small exponent of 3, there's no advantage in using
> repeated squaring.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  drivers/video/backlight/pwm_bl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 9252d51f31b9..aee6839e024a 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -179,7 +179,8 @@ static u64 cie1931(unsigned int lightness, unsigned int scale)
>  	if (lightness <= (8 * scale)) {
>  		retval = DIV_ROUND_CLOSEST(lightness * 10, 9033);
>  	} else {
> -		retval = int_pow((lightness + (16 * scale)) / 116, 3);
> +		retval = (lightness + (16 * scale)) / 116;
> +		retval *= retval * retval;
>  		retval = DIV_ROUND_CLOSEST_ULL(retval, (scale * scale));
>  	}
>  
> -- 
> 2.20.1

Re: [PATCH 4/5] backlight: pwm_bl: switch to power-of-2 base for fixed-point math

[Daniel Thompson]

On Thu, Sep 19, 2019 at 04:06:19PM +0200, Rasmus Villemoes wrote:
> Using a power-of-2 instead of power-of-10 base makes the computations
> much cheaper. 2^16 is safe; retval never becomes more than 2^48 +
> 2^16/2. On a 32 bit platform, the very expensive 64/32 division at the
> end of cie1931() instead becomes essentially free (a shift by 32 is
> just a register rename).
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>

> ---
>  drivers/video/backlight/pwm_bl.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index aee6839e024a..102bc191310f 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -148,7 +148,8 @@ static const struct backlight_ops pwm_backlight_ops = {
>  };
>  
>  #ifdef CONFIG_OF
> -#define PWM_LUMINANCE_SCALE	10000 /* luminance scale */
> +#define PWM_LUMINANCE_SHIFT	16
> +#define PWM_LUMINANCE_SCALE	(1 << PWM_LUMINANCE_SHIFT) /* luminance scale */
>  
>  /*
>   * CIE lightness to PWM conversion.
> @@ -165,23 +166,25 @@ static const struct backlight_ops pwm_backlight_ops = {
>   * The following function does the fixed point maths needed to implement the
>   * above formula.
>   */
> -static u64 cie1931(unsigned int lightness, unsigned int scale)
> +static u64 cie1931(unsigned int lightness)
>  {
>  	u64 retval;
>  
>  	/*
>  	 * @lightness is given as a number between 0 and 1, expressed
> -	 * as a fixed-point number in scale @scale. Convert to a
> -	 * percentage, still expressed as a fixed-point number, so the
> -	 * above formulas can be applied.
> +	 * as a fixed-point number in scale
> +	 * PWM_LUMINANCE_SCALE. Convert to a percentage, still
> +	 * expressed as a fixed-point number, so the above formulas
> +	 * can be applied.
>  	 */
>  	lightness *= 100;
> -	if (lightness <= (8 * scale)) {
> +	if (lightness <= (8 * PWM_LUMINANCE_SCALE)) {
>  		retval = DIV_ROUND_CLOSEST(lightness * 10, 9033);
>  	} else {
> -		retval = (lightness + (16 * scale)) / 116;
> +		retval = (lightness + (16 * PWM_LUMINANCE_SCALE)) / 116;
>  		retval *= retval * retval;
> -		retval = DIV_ROUND_CLOSEST_ULL(retval, (scale * scale));
> +		retval += PWM_LUMINANCE_SCALE/2;
> +		retval >>= 2*PWM_LUMINANCE_SHIFT;
>  	}
>  
>  	return retval;
> @@ -215,8 +218,7 @@ int pwm_backlight_brightness_default(struct device *dev,
>  	/* Fill the table using the cie1931 algorithm */
>  	for (i = 0; i < data->max_brightness; i++) {
>  		retval = cie1931((i * PWM_LUMINANCE_SCALE) /
> -				 data->max_brightness, PWM_LUMINANCE_SCALE) *
> -				 period;
> +				 data->max_brightness) * period;
>  		retval = DIV_ROUND_CLOSEST_ULL(retval, PWM_LUMINANCE_SCALE);
>  		if (retval > UINT_MAX)
>  			return -EINVAL;
> -- 
> 2.20.1

Re: [PATCH 3/5] backlight: pwm_bl: drop use of int_pow()

[Rasmus Villemoes]

On 07/10/2019 17.28, Daniel Thompson wrote:
> On Thu, Sep 19, 2019 at 04:06:18PM +0200, Rasmus Villemoes wrote:
> 
> It feels like there is some rationale missing in the description here.
> 
> What is the benefit of replacing the explicit int_pow() with the
> implicit multiplications?
> 
> 
> Daniel.
> 
> 
>>
>> We could (and a following patch will) change to use a power-of-2 scale,
>> but for a fixed small exponent of 3, there's no advantage in using
>> repeated squaring.

   ^^^^^^^^^^^^^^^^^^                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Apart from the function call overhead (and resulting register pressure
etc.), using int_pow is less efficient (for an exponent of 3, it ends up
doing four 64x64 multiplications instead of just two). But feel free to
drop it, I'm not going to pursue it further - it just seemed like a
sensible thing to do while I was optimizing the code anyway.

[At the time I wrote the patch, this was also the only user of int_pow
in the tree, so it also allowed removing int_pow altogether.]

Rasmus

Re: [PATCH 3/5] backlight: pwm_bl: drop use of int_pow()

[Daniel Thompson]

On Mon, Oct 07, 2019 at 08:43:31PM +0200, Rasmus Villemoes wrote:
> On 07/10/2019 17.28, Daniel Thompson wrote:
> > On Thu, Sep 19, 2019 at 04:06:18PM +0200, Rasmus Villemoes wrote:
> > 
> > It feels like there is some rationale missing in the description here.
> > 
> > What is the benefit of replacing the explicit int_pow() with the
> > implicit multiplications?
> > 
> > 
> > Daniel.
> > 
> > 
> >>
> >> We could (and a following patch will) change to use a power-of-2 scale,
> >> but for a fixed small exponent of 3, there's no advantage in using
> >> repeated squaring.
> 
>    ^^^^^^^^^^^^^^^^^^                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Apart from the function call overhead (and resulting register pressure
> etc.), using int_pow is less efficient (for an exponent of 3, it ends up
> doing four 64x64 multiplications instead of just two). But feel free to
> drop it, I'm not going to pursue it further - it just seemed like a
> sensible thing to do while I was optimizing the code anyway.
> 
> [At the time I wrote the patch, this was also the only user of int_pow
> in the tree, so it also allowed removing int_pow altogether.]

To be honest the change is fine but the patch description doesn't make
sense if the only current purpose of the patch is as a optimization.


Daniel.

Re: [PATCH 3/5] backlight: pwm_bl: drop use of int_pow()

[Rasmus Villemoes]

On 08/10/2019 11.31, Daniel Thompson wrote:
> On Mon, Oct 07, 2019 at 08:43:31PM +0200, Rasmus Villemoes wrote:
>> On 07/10/2019 17.28, Daniel Thompson wrote:
>>> On Thu, Sep 19, 2019 at 04:06:18PM +0200, Rasmus Villemoes wrote:
>>>
>>> It feels like there is some rationale missing in the description here.
>>>
>>
>> Apart from the function call overhead (and resulting register pressure
>> etc.), using int_pow is less efficient (for an exponent of 3, it ends up
>> doing four 64x64 multiplications instead of just two). But feel free to
>> drop it, I'm not going to pursue it further - it just seemed like a
>> sensible thing to do while I was optimizing the code anyway.
>>
>> [At the time I wrote the patch, this was also the only user of int_pow
>> in the tree, so it also allowed removing int_pow altogether.]
> 
> To be honest the change is fine but the patch description doesn't make
> sense if the only current purpose of the patch is as a optimization.

Agreed. Do you want me to resend the series with patch 3 updated to read

"For a fixed small exponent of 3, it is more efficient to simply use two
explicit multiplications rather than calling the int_pow() library
function: Aside from the function call overhead, its implementation
using repeated squaring means it ends up doing four 64x64 multiplications."

(and obviously patch 5 dropped)?

Rasmus

Re: [PATCH 3/5] backlight: pwm_bl: drop use of int_pow()

[Daniel Thompson]

On Tue, Oct 08, 2019 at 12:02:07PM +0200, Rasmus Villemoes wrote:
> On 08/10/2019 11.31, Daniel Thompson wrote:
> > On Mon, Oct 07, 2019 at 08:43:31PM +0200, Rasmus Villemoes wrote:
> >> On 07/10/2019 17.28, Daniel Thompson wrote:
> >>> On Thu, Sep 19, 2019 at 04:06:18PM +0200, Rasmus Villemoes wrote:
> >>>
> >>> It feels like there is some rationale missing in the description here.
> >>>
> >>
> >> Apart from the function call overhead (and resulting register pressure
> >> etc.), using int_pow is less efficient (for an exponent of 3, it ends up
> >> doing four 64x64 multiplications instead of just two). But feel free to
> >> drop it, I'm not going to pursue it further - it just seemed like a
> >> sensible thing to do while I was optimizing the code anyway.
> >>
> >> [At the time I wrote the patch, this was also the only user of int_pow
> >> in the tree, so it also allowed removing int_pow altogether.]
> > 
> > To be honest the change is fine but the patch description doesn't make
> > sense if the only current purpose of the patch is as a optimization.
> 
> Agreed. Do you want me to resend the series with patch 3 updated to read
> 
> "For a fixed small exponent of 3, it is more efficient to simply use two
> explicit multiplications rather than calling the int_pow() library
> function: Aside from the function call overhead, its implementation
> using repeated squaring means it ends up doing four 64x64 multiplications."
> 
> (and obviously patch 5 dropped)?

Yes, please.

When you resend you can add my R-B: to all patches:

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.


PS Don't mind either way but I wondered the following is clearer than
   the slightly funky multiply-and-assign expression (which isn't wrong
   but isn't very common either so my brain won't speed read it):

		retval = DIV_ROUND_CLOSEST_ULL(retval * retval * retval,
		 			       scale * scale);