On Wed, 11 Nov 2015, "Shih-Yuan Lee (FourDollars)" <sy...@canonical.com> wrote:
> Take Dell XPS 13 (2015) as an example. The vbt min 10 out of [0..255].
> The PWM max is 937 so the corresponding PWM min is 37 (10*937/256).
> This commit makes the sysfs brightness 1 map to the PWM brightness 37
> and 100 map to the PWM brightness 937.
>
> Signed-off-by: Shih-Yuan Lee (FourDollars) <sy...@canonical.com>
> ---
>  drivers/gpu/drm/i915/intel_panel.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c 
> b/drivers/gpu/drm/i915/intel_panel.c
> index a24df35..8652ce2 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1207,11 +1207,12 @@ static int intel_backlight_device_register(struct 
> intel_connector *connector)
>       memset(&props, 0, sizeof(props));
>       props.type = BACKLIGHT_RAW;
>  
> -     /*
> -      * Note: Everything should work even if the backlight device max
> -      * presented to the userspace is arbitrarily chosen.
> -      */
> -     props.max_brightness = panel->backlight.max;
> +     /* Setting max to fixed 100 if the range is large enough. */
> +     if (panel->backlight.max > panel->backlight.min + 99)
> +             props.max_brightness = 100;
> +     else
> +             props.max_brightness = panel->backlight.max - 
> panel->backlight.min;

No, the intention is to have a fixed 0..100 range no ifs no buts.

> +
>       props.brightness = scale_hw_to_user(connector,
>                                           panel->backlight.level,
>                                           props.max_brightness);
> @@ -1414,6 +1415,8 @@ static u32 get_backlight_min_vbt(struct intel_connector 
> *connector)
>       struct drm_device *dev = connector->base.dev;
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       struct intel_panel *panel = &connector->panel;
> +     u32 pwm_min;
> +     u32 pwm_step;
>       int min;
>  
>       WARN_ON(panel->backlight.max == 0);
> @@ -1432,7 +1435,22 @@ static u32 get_backlight_min_vbt(struct 
> intel_connector *connector)
>       }
>  
>       /* vbt value is a coefficient in range [0..255] */
> -     return scale(min, 0, 255, 0, panel->backlight.max);
> +     pwm_min = scale(min, 0, 255, 0, panel->backlight.max);
> +
> +     /* Calculate the PWM step */
> +     if (panel->backlight.max > pwm_min + 99)
> +             pwm_step = scale(1, 0, 99, 0, panel->backlight.max - pwm_min);
> +     else
> +             pwm_step = 1;
> +
> +     /*
> +      * Because sysfs brightness 0 is used to turn off the backlight, we 
> need step
> +      * down a little bit to make sysfs brightness 1 match the lowest 
> brightness.
> +      */
> +     if (pwm_min >= pwm_step)
> +             pwm_min -= pwm_step;
> +
> +     return pwm_min;

This is getting tricky, and you assume too much.

Currently, 0 means off if dev_priv->vbt.backlight.min_brightness == 0 or
the panel is eDP. LVDS panels with non-zero min brightness aren't
switched off with 0 brightness.

Pardon my bluntness, but I'm starting to feel like you just want to get
a patch in. Take my word for it, backlight is not where you should get
started.

BR,
Jani.



>  }
>  
>  static int lpt_setup_backlight(struct intel_connector *connector, enum pipe 
> unused)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to