Re: [PATCH] drm/i915/panel: Alwyas record the backlight level again (but cleverly)

[Keith Packard]

On Thu, 13 Oct 2011 10:57:35 +0200, Takashi Iwai <tiwai@xxxxxxx> wrote:

> This patch fixes the bug by recording the backlight level always
> when changed but only when dev_priv->backlight_enabled is set.
> In this way, the bogus value for disabling backlight can be skipped.

I think this is better than what we have, but I'm not sure it's quite
what we want -- the backlight level will only be remembered when it is
enabled, so requested changes that happen while the backlight is off
will have no effect. And, requested changes while the panel is disabled
will still go through to the hardware, which can cause problems with
panel power sequencing.

I think intel_panel_set_backlight should always record the level passed
in, but that intel_panel_disable_backlight and
intel_panel_enable_backlight should use a lower-level function, shared
with intel_panel_set_backlight that doesn't record the value:

intel_panel_actually_set_backlight(dev, level) {
        <internals of current intel_panel_set_backlight>
}

intel_panel_set_backlight(dev, level) {
        dev_priv->backlight_level = level;
        if (dev_priv->backlight_enabled)
                intel_panel_actually_set_backlight(dev, level);
}

intel_panel_enable_backlight(dev) {
        dev_priv->backlight_enabled = true;
        intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
}

intel_panel_disable_backlight(dev) {
        dev_priv->backlight_enabled = false;
        intel_panel_actually_set_backlight(dev, 0);
}

-- 
keith.packard@xxxxxxxxx

Attachment: pgp00000.pgp
Description: PGP signature