diff mbox

[v4] pwm-backlight: Avoid backlight flicker when probed from DT

Message ID 1446483356-14338-1-git-send-email-p.zabel@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Philipp Zabel Nov. 2, 2015, 4:55 p.m. UTC
If the driver is probed from the device tree, and there is a phandle
property set on it, and the enable GPIO is already configured as output,
and the backlight is currently disabled, keep it disabled.
If all these conditions are met, assume there will be some other driver
that can enable the backlight at the appropriate time.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
Changes since v3:
 - No need to call of_find_property(of_node, "phandle", NULL),
   just use of_node->phandle.
 - Rebased onto v4.3
---
 drivers/video/backlight/pwm_bl.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

Comments

Heiko Stübner Nov. 10, 2015, 2:18 p.m. UTC | #1
Hi Philipp,

Am Montag, 2. November 2015, 17:55:56 schrieb Philipp Zabel:
> If the driver is probed from the device tree, and there is a phandle
> property set on it, and the enable GPIO is already configured as output,
> and the backlight is currently disabled, keep it disabled.
> If all these conditions are met, assume there will be some other driver
> that can enable the backlight at the appropriate time.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>

this patch improves the white screen when booting a veyron chromebook
a lot. I still see a small white flash, but that can probably come from
the WIP edp driver.

Tested-by: Heiko Stuebner <heiko@sntech.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding Nov. 10, 2015, 5:32 p.m. UTC | #2
On Tue, Nov 10, 2015 at 03:18:10PM +0100, Heiko Stuebner wrote:
> Hi Philipp,
> 
> Am Montag, 2. November 2015, 17:55:56 schrieb Philipp Zabel:
> > If the driver is probed from the device tree, and there is a phandle
> > property set on it, and the enable GPIO is already configured as output,
> > and the backlight is currently disabled, keep it disabled.
> > If all these conditions are met, assume there will be some other driver
> > that can enable the backlight at the appropriate time.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> 
> this patch improves the white screen when booting a veyron chromebook
> a lot. I still see a small white flash, but that can probably come from
> the WIP edp driver.

Some panels require a couple of frames before they actually enable. You
may want to look at the datasheet of your panel to see if it has some
specific requirement and update the panel driver with that.

From a high-level view the way that this is supposed to work is that
your encoder driver (e.g. eDP) "prepares" the panel, then starts sending
frames and finally "enables" the panel. With something like the simple
panel driver you can influence this by setting the delay.enable field in
the panel descriptor.

See struct panel_desc in drivers/gpu/drm/panel/panel-simple.c

Thierry
Heiko Stübner Nov. 27, 2015, 10:18 a.m. UTC | #3
Hi Thierry,

Am Dienstag, 10. November 2015, 18:32:57 schrieb Thierry Reding:
> On Tue, Nov 10, 2015 at 03:18:10PM +0100, Heiko Stuebner wrote:
> > Hi Philipp,
> > 
> > Am Montag, 2. November 2015, 17:55:56 schrieb Philipp Zabel:
> > > If the driver is probed from the device tree, and there is a phandle
> > > property set on it, and the enable GPIO is already configured as output,
> > > and the backlight is currently disabled, keep it disabled.
> > > If all these conditions are met, assume there will be some other driver
> > > that can enable the backlight at the appropriate time.
> > > 
> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> > 
> > this patch improves the white screen when booting a veyron chromebook
> > a lot. I still see a small white flash, but that can probably come from
> > the WIP edp driver.
> 
> Some panels require a couple of frames before they actually enable. You
> may want to look at the datasheet of your panel to see if it has some
> specific requirement and update the panel driver with that.
> 
> From a high-level view the way that this is supposed to work is that
> your encoder driver (e.g. eDP) "prepares" the panel, then starts sending
> frames and finally "enables" the panel. With something like the simple
> panel driver you can influence this by setting the delay.enable field in
> the panel descriptor.
> 
> See struct panel_desc in drivers/gpu/drm/panel/panel-simple.c

Thanks for the pointers :-)

Just to follow up a little bit, after moving to Yakir's Analogix-dp driver 
from the chromeos-one I was using till now, that actually works nicely for me 
and even the short flash is gone.


Heiko
diff mbox

Patch

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index eff379b..65e57da 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -199,6 +199,8 @@  static int pwm_backlight_probe(struct platform_device *pdev)
 	struct backlight_properties props;
 	struct backlight_device *bl;
 	struct pwm_bl_data *pb;
+	phandle phandle = pdev->dev.of_node->phandle;
+	int initial_blank = FB_BLANK_UNBLANK;
 	int ret;
 
 	if (!data) {
@@ -242,7 +244,7 @@  static int pwm_backlight_probe(struct platform_device *pdev)
 	pb->enabled = false;
 
 	pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
-						  GPIOD_OUT_HIGH);
+						  GPIOD_ASIS);
 	if (IS_ERR(pb->enable_gpio)) {
 		ret = PTR_ERR(pb->enable_gpio);
 		goto err_alloc;
@@ -264,12 +266,30 @@  static int pwm_backlight_probe(struct platform_device *pdev)
 		pb->enable_gpio = gpio_to_desc(data->enable_gpio);
 	}
 
+	if (pb->enable_gpio) {
+		/*
+		 * If the driver is probed from the device tree and there is a
+		 * phandle link pointing to the backlight node, it is safe to
+		 * assume that another driver will enable the backlight at the
+		 * appropriate time. Therefore, if it is disabled, keep it so.
+		 */
+		if (phandle &&
+		    gpiod_get_direction(pb->enable_gpio) == GPIOF_DIR_OUT &&
+		    gpiod_get_value(pb->enable_gpio) == 0)
+			initial_blank = FB_BLANK_POWERDOWN;
+		else
+			gpiod_direction_output(pb->enable_gpio, 1);
+	}
+
 	pb->power_supply = devm_regulator_get(&pdev->dev, "power");
 	if (IS_ERR(pb->power_supply)) {
 		ret = PTR_ERR(pb->power_supply);
 		goto err_alloc;
 	}
 
+	if (phandle && !regulator_is_enabled(pb->power_supply))
+		initial_blank = FB_BLANK_POWERDOWN;
+
 	pb->pwm = devm_pwm_get(&pdev->dev, NULL);
 	if (IS_ERR(pb->pwm)) {
 		ret = PTR_ERR(pb->pwm);
@@ -321,6 +341,7 @@  static int pwm_backlight_probe(struct platform_device *pdev)
 	}
 
 	bl->props.brightness = data->dft_brightness;
+	bl->props.power = initial_blank;
 	backlight_update_status(bl);
 
 	platform_set_drvdata(pdev, bl);