On Tue, 29 Apr 2014, Jani Nikula <jani.nikula@xxxxxxxxx> wrote: > Historically we've exposed the full backlight PWM duty cycle range to > the userspace, in the name of "mechanism, not policy". However, it turns > out there are both panels and board designs where there is a minimum > duty cycle that is required for proper operation. The minimum duty cycle > is available in the VBT. > > The backlight class sysfs interface does not make any promises to the > userspace about the physical meaning of the range > 0..max_brightness. Specifically there is no guarantee that 0 means off; > indeed for acpi_backlight 0 usually is not off, but the minimum > acceptable value. > > Respect the minimum backlight, and expose the range acceptable to the > hardware as 0..max_brightness to the userspace via the backlight class > device; 0 means the minimum acceptable enabled value. To switch off the > backlight, the user must disable the encoder. > > As a side effect, make the backlight class device max brightness and > physical PWM modulation frequency (i.e. max duty cycle) independent. > > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> Acknowledgement that I meant to include: this is based on both Jesse's and Ben's earlier work. > > --- > > UNTESTED! > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_bios.c | 3 +- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_panel.c | 97 +++++++++++++++++++++++++++++++------- > 4 files changed, 85 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 50dfc3a1a9d1..d9dad3498b87 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1164,6 +1164,7 @@ struct intel_vbt_data { > u16 pwm_freq_hz; > bool present; > bool active_low_pwm; > + u8 min_brightness; /* min_brightness/255 of max */ > } backlight; > > /* MIPI DSI */ > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 2945f57c53ee..1a3e172029b3 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -339,11 +339,12 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header *bdb) > > dev_priv->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz; > dev_priv->vbt.backlight.active_low_pwm = entry->active_low_pwm; > + dev_priv->vbt.backlight.min_brightness = entry->min_brightness; > DRM_DEBUG_KMS("VBT backlight PWM modulation frequency %u Hz, " > "active %s, min brightness %u, level %u\n", > dev_priv->vbt.backlight.pwm_freq_hz, > dev_priv->vbt.backlight.active_low_pwm ? "low" : "high", > - entry->min_brightness, > + dev_priv->vbt.backlight.min_brightness, > backlight_data->level[panel_type]); > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index d8b540b891d1..2af74dd03e31 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -165,6 +165,7 @@ struct intel_panel { > struct { > bool present; > u32 level; > + u32 min; > u32 max; > bool enabled; > bool combination_mode; /* gen 2/4 only */ > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 776249bab488..360ae203aacb 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -398,6 +398,30 @@ intel_panel_detect(struct drm_device *dev) > } > } > > +/* Scale user_level in range [0..user_max] to [hw_min..hw_max]. */ > +static u32 scale_user_to_hw(struct intel_connector *connector, > + u32 user_level, u32 user_max) > +{ > + struct intel_panel *panel = &connector->panel; > + > + user_level = clamp(user_level, 0U, user_max); > + > + return panel->backlight.min + user_level * > + (panel->backlight.max - panel->backlight.min) / user_max; > +} > + > +/* Scale hw_level in range [hw_min..hw_max] to [0..user_max]. */ > +static u32 scale_hw_to_user(struct intel_connector *connector, > + u32 hw_level, u32 user_max) > +{ > + struct intel_panel *panel = &connector->panel; > + > + hw_level = clamp(hw_level, panel->backlight.min, panel->backlight.max); > + > + return (hw_level - panel->backlight.min) * user_max / > + (panel->backlight.max - panel->backlight.min); > +} > + > static u32 intel_panel_compute_brightness(struct intel_connector *connector, > u32 val) > { > @@ -558,14 +582,14 @@ intel_panel_actually_set_backlight(struct intel_connector *connector, u32 level) > } > > /* set backlight brightness to level in range [0..max] */ > -void intel_panel_set_backlight(struct intel_connector *connector, u32 level, > - u32 max) > +void intel_panel_set_backlight(struct intel_connector *connector, > + u32 user_level, u32 user_max) > { > struct drm_device *dev = connector->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_panel *panel = &connector->panel; > enum pipe pipe = intel_get_pipe_from_connector(connector); > - u32 freq; > + u32 hw_level; > unsigned long flags; > > if (!panel->backlight.present || pipe == INVALID_PIPE) > @@ -575,19 +599,25 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level, > > WARN_ON(panel->backlight.max == 0); > > - /* scale to hardware max, but be careful to not overflow */ > - freq = panel->backlight.max; > - if (freq < max) > - level = level * freq / max; > - else > - level = freq / max * level; > + hw_level = scale_user_to_hw(connector, user_level, user_max); > + panel->backlight.level = hw_level; > > - panel->backlight.level = level; > - if (panel->backlight.device) > - panel->backlight.device->props.brightness = level; > + if (panel->backlight.device) { > + /* > + * Update backlight device brightness. In most cases, the > + * request comes from the backlight device sysfs, user_max == > + * props.max_brightness, and this is redundant. However, this > + * serves to sync ACPI opregion backlight requests to the > + * backlight device. > + */ > + panel->backlight.device->props.brightness = > + user_level * > + panel->backlight.device->props.max_brightness / > + user_max; > + } > > if (panel->backlight.enabled) > - intel_panel_actually_set_backlight(connector, level); > + intel_panel_actually_set_backlight(connector, hw_level); > > spin_unlock_irqrestore(&dev_priv->backlight_lock, flags); > } > @@ -861,7 +891,9 @@ void intel_panel_enable_backlight(struct intel_connector *connector) > panel->backlight.level = panel->backlight.max; > if (panel->backlight.device) > panel->backlight.device->props.brightness = > - panel->backlight.level; > + scale_hw_to_user(connector, > + panel->backlight.level, > + panel->backlight.device->props.max_brightness); > } > > dev_priv->display.enable_backlight(connector); > @@ -890,11 +922,15 @@ static int intel_backlight_device_get_brightness(struct backlight_device *bd) > struct intel_connector *connector = bl_get_data(bd); > struct drm_device *dev = connector->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > + u32 hw_level; > int ret; > > intel_runtime_pm_get(dev_priv); > mutex_lock(&dev->mode_config.mutex); > - ret = intel_panel_get_backlight(connector); > + > + hw_level = intel_panel_get_backlight(connector); > + ret = scale_hw_to_user(connector, hw_level, bd->props.max_brightness); > + > mutex_unlock(&dev->mode_config.mutex); > intel_runtime_pm_put(dev_priv); > > @@ -918,8 +954,15 @@ static int intel_backlight_device_register(struct intel_connector *connector) > > memset(&props, 0, sizeof(props)); > props.type = BACKLIGHT_RAW; > - props.brightness = panel->backlight.level; > + > + /* > + * Note: Everything should work even if the backlight device max > + * presented to the userspace is arbitrarily chosen. > + */ > props.max_brightness = panel->backlight.max; > + props.brightness = scale_hw_to_user(connector, > + panel->backlight.level, > + props.max_brightness); > > /* > * Note: using the same name independent of the connector prevents > @@ -965,6 +1008,18 @@ static void intel_backlight_device_unregister(struct intel_connector *connector) > * XXX: Query mode clock or hardware clock and program PWM modulation frequency > * appropriately when it's 0. Use VBT and/or sane defaults. > */ > +static inline u32 get_backlight_min(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; > + > + BUG_ON(panel->backlight.max == 0); > + > + return dev_priv->vbt.backlight.min_brightness * > + panel->backlight.max / 255; > +} > + > static int bdw_setup_backlight(struct intel_connector *connector) > { > struct drm_device *dev = connector->base.dev; > @@ -980,6 +1035,8 @@ static int bdw_setup_backlight(struct intel_connector *connector) > if (!panel->backlight.max) > return -ENODEV; > > + panel->backlight.min = get_backlight_min(connector); > + > val = bdw_get_backlight(connector); > panel->backlight.level = intel_panel_compute_brightness(connector, val); > > @@ -1004,6 +1061,8 @@ static int pch_setup_backlight(struct intel_connector *connector) > if (!panel->backlight.max) > return -ENODEV; > > + panel->backlight.min = get_backlight_min(connector); > + > val = pch_get_backlight(connector); > panel->backlight.level = intel_panel_compute_brightness(connector, val); > > @@ -1036,6 +1095,8 @@ static int i9xx_setup_backlight(struct intel_connector *connector) > if (!panel->backlight.max) > return -ENODEV; > > + panel->backlight.min = get_backlight_min(connector); > + > val = i9xx_get_backlight(connector); > panel->backlight.level = intel_panel_compute_brightness(connector, val); > > @@ -1063,6 +1124,8 @@ static int i965_setup_backlight(struct intel_connector *connector) > if (!panel->backlight.max) > return -ENODEV; > > + panel->backlight.min = get_backlight_min(connector); > + > val = i9xx_get_backlight(connector); > panel->backlight.level = intel_panel_compute_brightness(connector, val); > > @@ -1100,6 +1163,8 @@ static int vlv_setup_backlight(struct intel_connector *connector) > if (!panel->backlight.max) > return -ENODEV; > > + panel->backlight.min = get_backlight_min(connector); > + > val = _vlv_get_backlight(dev, PIPE_A); > panel->backlight.level = intel_panel_compute_brightness(connector, val); > > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx