On Wed, Nov 11, 2015 at 02:05:57PM +0200, Jani Nikula wrote:
> On Wed, 11 Nov 2015, "Shih-Yuan Lee (FourDollars)" <sylee@xxxxxxxxxxxxx> 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) <sylee@xxxxxxxxxxxxx>
> > ---
> > 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.
The following change makes sysfs brightness 0 to turn off the backlight.
commit e6755fb78e8f20ecadf2a4080084121336624ad9
Author: Jani Nikula <jani.nikula at intel.com>
Date: Tue Aug 12 17:11:42 2014 +0300
drm/i915: switch off backlight for backlight class 0 brightness
drivers/gpu/drm/i915/intel_panel.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index af54356..764f928 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -986,7 +986,8 @@ static int intel_backlight_device_update_status(struct backlight_device *bd)
*/
if (panel->backlight.enabled) {
if (panel->backlight_power) {
- bool enable = bd->props.power == FB_BLANK_UNBLANK;
+ bool enable = bd->props.power == FB_BLANK_UNBLANK &&
+ bd->props.brightness != 0;
panel->backlight_power(connector, enable);
}
} else {
But it also abandoned the lowest brightness level from VBT because it is used
to turn off the backlight.
>
> 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.
My intention is to respect the VBT minimum backlight brightness again.
Just like what you did before.
commit 6dda730e55f412a6dfb181cae6784822ba463847
Author: Jani Nikula <jani.nikula at intel.com>
Date: Tue Jun 24 18:27:40 2014 +0300
drm/i915: respect the VBT minimum backlight brightness
I don't mind if you can fix this minor issue for me.
However I am interested to work with the upstream developer.
Thanks a lot for your feedback and let me know many things about the
brightness control in i915.
BTW, I wrote the brightness kernel module (ideapad-laptop) before.
Regards,
$4
>
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
https://about.me/fourdollars
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
