Re: [PATCH 10/10] pwm-backlight: Allow backlight to remain disabled on boot

[Stephen Warren <swarren@wwwdotorg.org>]

On 09/23/2013 03:41 PM, Thierry Reding wrote:
> The default for backlight devices is to be enabled immediately when
> registering with the backlight core. This can be useful for setups that
> use a simple framebuffer device and where the backlight cannot otherwise
> be hooked up to the panel.
> 
> However, when dealing with more complex setups, such as those of recent
> ARM SoCs, this can be problematic. Since the backlight is usually setup
> separately from the display controller, the probe order is not usually
> deterministic. That can lead to situations where the backlight will be
> powered up and the panel will show an uninitialized framebuffer.
> 
> Furthermore, subsystems such as DRM have advanced functionality to set
> the power mode of a panel. In order to allow such setups to power up the
> panel at exactly the right moment, a way is needed to prevent the
> backlight core from powering the backlight up automatically when it is
> registered.
> 
> This commit introduces a new boot_off field in the platform data (and
> also implements getting the same information from device tree). When set
> the initial backlight power mode will be set to "off".

> diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt

> +  - backlight-boot-off: keep the backlight disabled on boot

A few thoughts:

1) Does this property indicate that:

a) The current state of the backlight at boot. In which case, this will
need bootloader involvement to modify the value in the DT at boot time
based on whether the bootloader turned on the backlight:-(

b) That the driver should not turn on the backlight immediately? That
seems to describe driver behaviour more than HW. Is it appropriate to
put that into DT?

Your suggestion to make the backlight not turn on by default might be a
better fix?

2) Should the driver instead attempt to read the current state of the
GPIO output to determine whether the backlight is on? This may not be
possible on all HW.

3) Doesn't the following code in probe() (added in a previous patch)
need to be updated?

> +	if (gpio_is_valid(pb->enable_gpio)) {
> +		if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
> +			gpio_set_value(pb->enable_gpio, 0);
> +		else
> +			gpio_set_value(pb->enable_gpio, 1);
> +	}

... That assumes that the backlight is on at boot, and hence presumably
after this patch still always turns on the backlight, only to turn it
off very quickly once the following code in this patch executes:

(and perhaps we also need to avoid turning the backlight off then on if
it was already on at boot)

> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c

> @@ -318,6 +320,12 @@ static int pwm_backlight_probe(struct platform_device *pdev)
>  	}
>  
>  	bl->props.brightness = data->dft_brightness;
> +
> +	if (data->boot_off)
> +		bl->props.power = FB_BLANK_POWERDOWN;
> +	else
> +		bl->props.power = FB_BLANK_UNBLANK;