[PATCHv2 4/4] backlight: led_bl: fix led -> backlight brightness mapping

Daniel Thompson daniel.thompson at linaro.org
Tue Apr 21 15:47:35 UTC 2020


On Tue, Apr 21, 2020 at 03:46:29PM +0300, Tomi Valkeinen wrote:
> The code that maps the LED default brightness to backlight levels has
> two issues: 1) if the default brightness is the first backlight level
> (usually 0), the code fails to find it, and 2) when the code fails to
> find a backlight level, it ends up using max_brightness + 1 as the
> default brightness.
> 
> Fix these two issues.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> ---
>  drivers/video/backlight/led_bl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
> index 63693c4f0883..43a5302f163a 100644
> --- a/drivers/video/backlight/led_bl.c
> +++ b/drivers/video/backlight/led_bl.c
> @@ -159,10 +159,11 @@ static int led_bl_parse_levels(struct device *dev,
>  		 */
>  		db = priv->default_brightness;
>  		for (i = 0 ; i < num_levels; i++) {
> -			if ((i && db > levels[i - 1]) && db <= levels[i])
> +			if ((i == 0 || db > levels[i - 1]) && db <= levels[i])
>  				break;
>  		}

I looked at this loop again and realized the entire check of
levels[i-1] is pointless anyway: we already know that db is greater
then levels[i-1] otherwise the loop would have exited on its previous
iteration.


> -		priv->default_brightness = i;
> +
> +		priv->default_brightness = min(i, num_levels - 1);

Perhaps this min() also tells us the loop exit condition is wrong as
well...  and whilst we are at it the final comparison is arguably
not in the best order (since to describe what the loop does we have to
a complex clauses like "such that").

In natural English what the code is trying to do is "find the first
value in the lookup table that is larger than or equal to db or, if that
does not exist, choose the brightest value".

In other words:

		for (i=0; i<num_levels-1; i++)
			if (levels[i] >= db)
				break;


Daniel.


> -                     if ((i && db > levels[i - 1]) && db <=
> levels[i])
> +                     if ((i == 0 || db > levels[i - 1]) && db <=
> levels[i])
>                               break;
>               }

>  		priv->max_brightness = num_levels - 1;
>  		priv->levels = levels;
>  	} else if (num_levels >= 0) {
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 


More information about the dri-devel mailing list