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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx>
> ---
>  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
> 
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux