devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Enric Balletbo Serra <eballetbo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Enric Balletbo i Serra
	<enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Cc: Daniel Thompson
	<daniel.thompson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Doug Anderson <dianders-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Jingoo Han <jingoohan1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Richard Purdie <rpurdie-Fm38FmjxZ/leoWH0uzbU5w@public.gmane.org>,
	Jacek Anaszewski
	<jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Brian Norris
	<briannorris-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Guenter Roeck <groeck-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Alexandru Stan <amstan-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	linux-leds-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-kernel
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH 3/4] backlight: pwm_bl: compute brightness of LED linearly to human eye.
Date: Fri, 12 Jan 2018 11:14:54 +0100	[thread overview]
Message-ID: <CAFqH_50zs+TMZaRNegxYPQOcysvwm9EY5iB8DqNeGYN+QaJP6Q@mail.gmail.com> (raw)
In-Reply-To: <20180110223046.17696-4-enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

2018-01-10 23:30 GMT+01:00 Enric Balletbo i Serra
<enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>:
> When you want to change the brightness using a PWM signal, one thing you
> need to consider is how human perceive the brightness. Human perceive
> the brightness change non-linearly, we have better sensitivity at low
> luminance than high luminance, so to achieve perceived linear dimming,
> the brightness must be matches to the way our eyes behave. The CIE 1931
> lightness formula is what actually describes how we perceive light.
>
> This patch computes a default table with the brightness levels filled
> with the numbers provided by the CIE 1931 algorithm, the number of the
> brightness levels is calculated based on the PWM resolution.
>
> The calculation of the table using the CIE 1931 algorithm is enabled by
> default when you do not define the 'brightness-levels' propriety in your
> device tree.
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> ---
> Changes since RFCv2:
> - Pre-compute the table at boot using the cie 1931 algorithm, this
>   introduced again the fixed point calculations that needs to be
>   reviewed.
> - Calculate the number of needed steps based on the number of bits of
>   the PWM.
> - Improve some code documentation.
>
> Changes since RFCv1:
> - Get rid of fixed point calculations and use a table instead.
>
>  drivers/video/backlight/pwm_bl.c | 149 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 136 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index eabe0a4462af..9398516db0ce 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -143,6 +143,107 @@ static const struct backlight_ops pwm_backlight_ops = {
>  };
>
>  #ifdef CONFIG_OF
> +#define PWM_LUMINANCE_SCALE    10000 /* luminance scale */
> +
> +/* An integer based power function */
> +static u64 int_pow(u64 base, int exp)
> +{
> +       u64 result = 1;
> +
> +       while (exp) {
> +               if (exp & 1)
> +                       result *= base;
> +               exp >>= 1;
> +               base *= base;
> +       }
> +
> +       return result;
> +}
> +
> +/*
> + * CIE lightness to PWM conversion.
> + *
> + * 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
> + *
> + * 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
> + * perceives the screen to be, and is a number between 0 and 100.
> + *
> + * The following function does the fixed point maths needed to implement the
> + * above formula.
> + */
> +static u64 cie1931(unsigned int lightness, unsigned int scale)
> +{
> +       u64 retval;
> +
> +       lightness *= 100;
> +       if (lightness <= (8 * scale)) {
> +               retval = DIV_ROUND_CLOSEST_ULL(lightness * 10, 9023);
> +       } else {
> +               retval = int_pow((lightness + (16 * scale)) / 116, 3);
> +               retval = DIV_ROUND_CLOSEST_ULL(retval, (scale * scale));
> +       }
> +
> +       return retval;
> +}
> +
> +/*
> + * Create a default correction table for PWM values to create linear brightness
> + * for LED based backlights using the CIE1931 algorithm.
> + */
> +static
> +int pwm_backlight_brightness_default(struct device *dev,
> +                                    struct platform_pwm_backlight_data *data,
> +                                    unsigned int period)
> +{
> +       unsigned int counter = 0;
> +       unsigned int i, n;
> +       u64 retval;
> +
> +       /*
> +        * Count the number of bits needed to represent the period number. The
> +        * number of bits is used to calculate the number of levels used for the
> +        * brightness-levels table, the purpose of this calculation is have a
> +        * pre-computed table with enough levels to get linear brightness
> +        * perception. The period is divided by the number of bits so for a
> +        * 8-bit PWM we have 255 / 8 = 32 brightness levels or for a 16-bit PWM
> +        * we have 65535 / 16 = 4096 brightness levels.
> +        *
> +        * Note that this method is based on empirical testing on different
> +        * devices with PWM of 8 and 16 bits of resolution.
> +        */
> +       n = period;
> +       while (n) {
> +               counter += n % 2;
> +               n >>= 1;
> +       }
> +
> +       data->max_brightness = DIV_ROUND_UP(period, counter);
> +       data->levels = devm_kcalloc(dev, data->max_brightness,
> +                                   sizeof(*data->levels), GFP_KERNEL);
> +       if (!data->levels)
> +               return -ENOMEM;
> +
> +       /* 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;
> +               retval = DIV_ROUND_CLOSEST_ULL(retval, PWM_LUMINANCE_SCALE);
> +               if (retval > UINT_MAX)
> +                       return -EINVAL;
> +               data->levels[i] = (unsigned int)retval;
> +       }
> +
> +       data->dft_brightness = data->max_brightness / 2;
> +       data->max_brightness--;
> +
> +       return 0;
> +}
> +
>  static int pwm_backlight_parse_dt(struct device *dev,
>                                   struct platform_pwm_backlight_data *data)
>  {
> @@ -161,10 +262,13 @@ static int pwm_backlight_parse_dt(struct device *dev,
>
>         memset(data, 0, sizeof(*data));
>
> -       /* determine the number of brightness levels */
> +       /*
> +        * Determine the number of brightness levels, if this property is not
> +        * set a default table of brightness levels will be used.
> +        */
>         prop = of_find_property(node, "brightness-levels", &length);
>         if (!prop)
> -               return -EINVAL;
> +               return 0;
>
>         data->max_brightness = length / sizeof(u32);
>
> @@ -299,6 +403,14 @@ static int pwm_backlight_parse_dt(struct device *dev,
>  {
>         return -ENODEV;
>  }
> +
> +static
> +int pwm_backlight_brightness_default(struct device *dev,
> +                                    struct platform_pwm_backlight_data *data,
> +                                    unsigned int period)
> +{
> +       return -ENODEV;
> +}
>  #endif
>
>  static int pwm_backlight_initial_power_state(const struct pwm_bl_data *pb)
> @@ -339,7 +451,9 @@ static int pwm_backlight_probe(struct platform_device *pdev)
>         struct backlight_device *bl;
>         struct device_node *node = pdev->dev.of_node;
>         struct pwm_bl_data *pb;
> +       struct pwm_state state;
>         struct pwm_args pargs;
> +       unsigned int i;
>         int ret;
>
>         if (!data) {
> @@ -364,17 +478,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
>                 goto err_alloc;
>         }
>
> -       if (data->levels) {
> -               unsigned int i;
> -
> -               for (i = 0; i <= data->max_brightness; i++)
> -                       if (data->levels[i] > pb->scale)
> -                               pb->scale = data->levels[i];
> -
> -               pb->levels = data->levels;
> -       } else
> -               pb->scale = data->max_brightness;
> -
>         pb->notify = data->notify;
>         pb->notify_after = data->notify_after;
>         pb->check_fb = data->check_fb;
> @@ -441,6 +544,26 @@ static int pwm_backlight_probe(struct platform_device *pdev)
>
>         dev_dbg(&pdev->dev, "got pwm for backlight\n");
>
> +       if (!data->levels) {
> +               /* Get the PWM period (in nanoseconds) */
> +               pwm_get_state(pb->pwm, &state);
> +
> +               ret = pwm_backlight_brightness_default(&pdev->dev, data,
> +                                                      state.period);
> +               if (ret < 0) {
> +                       dev_err(&pdev->dev,
> +                               "failed to setup default brightness table\n");
> +                       goto err_alloc;
> +               }
> +       }
> +
> +       for (i = 0; i <= data->max_brightness; i++)

Oops, horrible and unjustifiable mistake, missing { here :/

> +               if (data->levels[i] > pb->scale)
> +                       pb->scale = data->levels[i];
> +
> +               pb->levels = data->levels;
> +       }
> +
>         /*
>          * FIXME: pwm_apply_args() should be removed when switching to
>          * the atomic PWM API.
> --
> 2.15.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2018-01-12 10:14 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-10 22:30 [PATCH 0/4] backlight: pwm_bl: support linear interpolation and brightness to human eye Enric Balletbo i Serra
2018-01-10 22:30 ` [PATCH 1/4] backlight: pwm_bl: linear interpolation between brightness-levels Enric Balletbo i Serra
2018-01-10 22:30 ` [PATCH 2/4] dt-bindings: pwm-backlight: add a num-interpolation-steps property Enric Balletbo i Serra
2018-01-19 20:52   ` Rob Herring
2018-01-22  9:16     ` Daniel Thompson
2018-01-23 10:21       ` Enric Balletbo Serra
2018-01-10 22:30 ` [PATCH 3/4] backlight: pwm_bl: compute brightness of LED linearly to human eye Enric Balletbo i Serra
     [not found]   ` <20180110223046.17696-4-enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
2018-01-12 10:14     ` Enric Balletbo Serra [this message]
     [not found]       ` <CAFqH_50zs+TMZaRNegxYPQOcysvwm9EY5iB8DqNeGYN+QaJP6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-12 11:50         ` Daniel Thompson
2018-01-10 22:30 ` [PATCH 4/4] dt-bindings: pwm-backlight: move brightness-levels to optional Enric Balletbo i Serra
     [not found]   ` <20180110223046.17696-5-enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
2018-01-19 20:57     ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFqH_50zs+TMZaRNegxYPQOcysvwm9EY5iB8DqNeGYN+QaJP6Q@mail.gmail.com \
    --to=eballetbo-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=amstan-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=briannorris-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=daniel.thompson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=dianders-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org \
    --cc=groeck-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=jingoohan1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-leds-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=pavel-+ZI9xUNit7I@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=rpurdie-Fm38FmjxZ/leoWH0uzbU5w@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).