REGRESSION: [RESEND PATCH v3 1/4] backlight: pwm_bl: linear interpolation between brightness-levels

[Marcel Ziswiler <marcel.ziswiler@toradex.com>]

On Mon, 2018-04-09 at 10:33 +0200, Enric Balletbo i Serra wrote:
> Setting num-interpolated-steps in the dts will allow you to have
> linear
> interpolation between values of brightness-levels. This way a high
> resolution pwm duty cycle can be used without having to list out
> every
> possible value in the dts. This system also allows for gamma
> corrected
> values.
> 
> The most simple example is interpolate between two brightness values
> a
> number of steps, this can be done setting the following in the dts:
> 
>   brightness-levels = <0 65535>;
>   num-interpolated-steps = <1024>;
>   default-brightness-level = <512>;
> 
> This will create a brightness-level table with the following values:
> 
>   <0 63 126 189 252 315 378 441 ... 64260 64323 64386 64449 65535>
> 
> Another use case can be describe a gamma corrected curve, as we have
> better sensitivity at low luminance than high luminance we probably
> want have smaller steps for low brightness levels values and bigger
> steps for high brightness levels values. This can be achieved with
> the following in the dts:
> 
>   brightness-levels = <0 4096 65535>;
>   num-interpolated-steps = <1024>;
>   default-brightness-level = <512>;
> 
> This will create a brightness-levels table with the following values:
> 
>   <0 4 8 12 16 20 ... 4096 4156 4216 4276 ... 65535>
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>  drivers/video/backlight/pwm_bl.c | 83
> ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c
> b/drivers/video/backlight/pwm_bl.c
> index 8e3f1245f5c5..f0a108ab570a 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -147,7 +147,11 @@ static int pwm_backlight_parse_dt(struct device
> *dev,
>  				  struct platform_pwm_backlight_data
> *data)
>  {
>  	struct device_node *node = dev->of_node;
> +	unsigned int num_levels = 0;
> +	unsigned int levels_count;
> +	unsigned int num_steps;
>  	struct property *prop;
> +	unsigned int *table;
>  	int length;
>  	u32 value;
>  	int ret;
> @@ -167,6 +171,7 @@ static int pwm_backlight_parse_dt(struct device
> *dev,
>  	/* read brightness levels from DT property */
>  	if (data->max_brightness > 0) {
>  		size_t size = sizeof(*data->levels) * data-
> >max_brightness;
> +		unsigned int i, j, n = 0;
>  
>  		data->levels = devm_kzalloc(dev, size, GFP_KERNEL);
>  		if (!data->levels)
> @@ -184,6 +189,84 @@ static int pwm_backlight_parse_dt(struct device
> *dev,
>  			return ret;
>  
>  		data->dft_brightness = value;
> +
> +		/*
> +		 * This property is optional, if is set enables
> linear
> +		 * interpolation between each of the values of
> brightness levels
> +		 * and creates a new pre-computed table.
> +		 */
> +		of_property_read_u32(node, "num-interpolated-steps",
> +				     &num_steps);
> +
> +		/*
> +		 * Make sure that there is at least two entries in
> the
> +		 * brightness-levels table, otherwise we can't
> interpolate
> +		 * between two points.
> +		 */
> +		if (num_steps) {
> +			if (data->max_brightness < 2) {
> +				dev_err(dev, "can't interpolate\n");
> +				return -EINVAL;
> +			}
> +
> +			/*
> +			 * Recalculate the number of brightness
> levels, now
> +			 * taking in consideration the number of
> interpolated
> +			 * steps between two levels.
> +			 */
> +			for (i = 0; i < data->max_brightness - 1;
> i++) {
> +				if ((data->levels[i + 1] - data-
> >levels[i]) /
> +				   num_steps)
> +					num_levels += num_steps;
> +				else
> +					num_levels++;
> +			}
> +			num_levels++;
> +			dev_dbg(dev, "new number of brightness
> levels: %d\n",
> +				num_levels);
> +
> +			/*
> +			 * Create a new table of brightness levels
> with all the
> +			 * interpolated steps.
> +			 */
> +			size = sizeof(*table) * num_levels;
> +			table = devm_kzalloc(dev, size, GFP_KERNEL);
> +			if (!table)
> +				return -ENOMEM;
> +
> +			/* Fill the interpolated table. */
> +			levels_count = 0;
> +			for (i = 0; i < data->max_brightness - 1;
> i++) {
> +				value = data->levels[i];
> +				n = (data->levels[i + 1] - value) /
> num_steps;
> +				if (n > 0) {
> +					for (j = 0; j < num_steps;
> j++) {
> +						table[levels_count]
> = value;
> +						value += n;
> +						levels_count++;
> +					}
> +				} else {
> +					table[levels_count] = data-
> >levels[i];
> +					levels_count++;
> +				}
> +			}
> +			table[levels_count] = data->levels[i];
> +
> +			/*
> +			 * As we use interpolation lets remove
> current
> +			 * brightness levels table and replace for
> the
> +			 * new interpolated table.
> +			 */
> +			devm_kfree(dev, data->levels);
> +			data->levels = table;
> +
> +			/*
> +			 * Reassign max_brightness value to the new
> total number
> +			 * of brightness levels.
> +			 */
> +			data->max_brightness = num_levels;
> +		}
> +
>  		data->max_brightness--;
>  	}

Unfortunately, bisection has shown this to break graphics on at least
Apalis T30  [1] and Colibri T30 [2] running today's (resp. yesterday's)
-next:

[    3.302618] ------------[ cut here ]------------
[    3.302664] WARNING: CPU: 2 PID: 1 at
/run/media/zim/Build/Sources/linux-next.git/mm/slab_common.c:1031
kmalloc_slab+0x98/0xa0
[    3.302701] Modules linked in:
[    3.302732] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 4.18.0-rc4-
next-20180713-dirty #50
[    3.302763] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[    3.302820] [<c011248c>] (unwind_backtrace) from [<c010ce70>]
(show_stack+0x10/0x14)
[    3.302865] [<c010ce70>] (show_stack) from [<c0a00a74>]
(dump_stack+0x8c/0xa0)
[    3.302905] [<c0a00a74>] (dump_stack) from [<c0123cb0>]
(__warn+0xe0/0xf8)
[    3.302937] [<c0123cb0>] (__warn) from [<c0123de0>]
(warn_slowpath_null+0x40/0x48)
[    3.302971] [<c0123de0>] (warn_slowpath_null) from [<c0231b74>]
(kmalloc_slab+0x98/0xa0)
[    3.303014] [<c0231b74>] (kmalloc_slab) from [<c0258ac0>]
(__kmalloc_track_caller+0x18/0x214)   
[    3.303060] [<c0258ac0>] (__kmalloc_track_caller) from [<c0571264>]
(devm_kmalloc+0x24/0x6c)
[    3.303108] [<c0571264>] (devm_kmalloc) from [<c0477c4c>]
(pwm_backlight_probe+0x4b4/0x9d8)
[    3.303147] [<c0477c4c>] (pwm_backlight_probe) from [<c056f8d0>]
(platform_drv_probe+0x48/0x98) 
[    3.303185] [<c056f8d0>] (platform_drv_probe) from [<c056d9e8>]
(really_probe+0x1d0/0x2bc)
[    3.303219] [<c056d9e8>] (really_probe) from [<c056dc38>]
(driver_probe_device+0x60/0x160)
[    3.303252] [<c056dc38>] (driver_probe_device) from [<c056de08>]
(__driver_attach+0xd0/0xd4)
[    3.303297] [<c056de08>] (__driver_attach) from [<c056bd34>]
(bus_for_each_dev+0x74/0xb4)
[    3.303337] [<c056bd34>] (bus_for_each_dev) from [<c056ce94>]
(bus_add_driver+0x18c/0x210)
[    3.303373] [<c056ce94>] (bus_add_driver) from [<c056ea3c>]
(driver_register+0x7c/0x114)
[    3.303412] [<c056ea3c>] (driver_register) from [<c0102f24>]
(do_one_initcall+0x54/0x278)
[    3.303463] [<c0102f24>] (do_one_initcall) from [<c0e01110>]
(kernel_init_freeable+0x2c0/0x354) 
[    3.303512] [<c0e01110>] (kernel_init_freeable) from [<c0a1715c>]
(kernel_init+0x8/0x10c)
[    3.303551] [<c0a1715c>] (kernel_init) from [<c01010e8>]
(ret_from_fork+0x14/0x2c)
[    3.303579] Exception stack(0xf68a7fb0 to 0xf68a7ff8)
[    3.303602] 7fa0:                                     00000000
00000000 00000000 00000000
[    3.303635] 7fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    3.303666] 7fe0: 00000000 00000000 00000000 00000000 00000013
00000000
[    3.303695] ---[ end trace 8ab8d599271271a0 ]---
[    3.303721] pwm-backlight backlight: failed to find platform data
[    3.303765] pwm-backlight: probe of backlight failed with error -12

Just reverting this single patch makes it work fine as expected again.
Further investigation pending but maybe some of you guys know what is
going on here.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
/tree/arch/arm/boot/dts/tegra30-apalis-eval.dts?h=next-20180713
[2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
/tree/arch/arm/boot/dts/tegra30-colibri-eval-v3.dts?h=next-20180713