On Tue, 10 Nov 2015, "Shih-Yuan Lee (FourDollars)" <sy...@canonical.com> wrote: > Taking Dell XPS 13 (2015) as an example. The lowest PWM brightness is 10 > and the highest PWM brightness is 937. Before this change, we can only > use from 37 to 937, and 37 is used to turn off the backlight because it > is mapped to 0 of sysfs brightness however the maximum sysfs brightness > is still 937 so it makes some sysfs brightness values are mapped to the > same PWM brightness values. > > After this change, we can use the whole PWM brightness range from 10 to > 937, and they are mapped into the range from 0 to 927 in the sysfs > brightness and vice versa because they are 1-1 mapping. > > 10 is the lowest PWM brightness. We should make users able to use it for > the extreme power saving reason and users can still see the display. > > Signed-off-by: Shih-Yuan Lee (FourDollars) <sy...@canonical.com> > --- > drivers/gpu/drm/i915/intel_panel.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > b/drivers/gpu/drm/i915/intel_panel.c > index a24df35..57bc2fe 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -1154,8 +1154,7 @@ 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 && > - bd->props.brightness != 0; > + bool enable = bd->props.power == FB_BLANK_UNBLANK; > panel->backlight.power(connector, enable);
This hunk would be a revert of commit e6755fb78e8f20ecadf2a4080084121336624ad9 Author: Jani Nikula <jani.nik...@intel.com> Date: Tue Aug 12 17:11:42 2014 +0300 drm/i915: switch off backlight for backlight class 0 brightness > } > } else { > @@ -1211,7 +1210,7 @@ static int intel_backlight_device_register(struct > intel_connector *connector) > * Note: Everything should work even if the backlight device max > * presented to the userspace is arbitrarily chosen. > */ > - props.max_brightness = panel->backlight.max; > + props.max_brightness = panel->backlight.max - panel->backlight.min; IMO we should consider setting max to fixed 100 here instead. That was the plan all along when I wrote the scaling stuff and added the Note: above in commit 6dda730e55f412a6dfb181cae6784822ba463847 Author: Jani Nikula <jani.nik...@intel.com> Date: Tue Jun 24 18:27:40 2014 +0300 drm/i915: respect the VBT minimum backlight brightness I just chickened out of it at that time. The commit message is worth reading in full. > props.brightness = scale_hw_to_user(connector, > panel->backlight.level, > props.max_brightness); > @@ -1429,10 +1428,11 @@ static u32 get_backlight_min_vbt(struct > intel_connector *connector) > if (min != dev_priv->vbt.backlight.min_brightness) { > DRM_DEBUG_KMS("clamping VBT min backlight %d/255 to %d/255\n", > dev_priv->vbt.backlight.min_brightness, min); > + /* vbt value is a coefficient in range [0..255] */ > + return scale(min, 0, 255, 0, panel->backlight.max); > } > > - /* vbt value is a coefficient in range [0..255] */ > - return scale(min, 0, 255, 0, panel->backlight.max); > + return min; This change would interpret the VBT minimum value as a coefficient x/255 if it's in range 0..64, and as an absolute value x if it's in range 65..255. This seems like an odd thing to do, although I admit what I did in commit e1c412e75754ab7b7002f3e18a2652d999c40d4b Author: Jani Nikula <jani.nik...@intel.com> Date: Wed Nov 5 14:46:31 2014 +0200 drm/i915: safeguard against too high minimum brightness is not that much better. The problem is, there's a spec on VBT (internal, I'm afraid), there's a tool to generate VBT (Windows only, probably not freely available) with some interpretation of the spec, and then there are users of the tool that tweak the variables until they see something they like on the machine they're developing, running some Windows version with a driver version with some interpretation of the spec. So no, I don't agree with any of the changes you propose. I do acknowledge that we have a minor bug that prevents the user from setting the brightness to the minimum acceptable by the hardware if the minimum is non-zero, and 0 means off. Keeping the 0 = off meaning, the fix would be to have 1 mean the minimum brightness acceptable by the hardware... which really isn't that much different from what 1 currently means. The bigger problem is that the userspace probably never sets brightness to 1 except by accident. 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