Hi,

On Tue, 5 Jul 2016 12:09:02 +0200 Uwe Kleine-K?nig wrote:

Hello Tomi, hello Jean-Christophe,

On Wed, Apr 20, 2016 at 09:17:27PM +0200, Uwe Kleine-K?nig wrote:

quoted

On Fri, Mar 11, 2016 at 01:22:40PM +0200, Tomi Valkeinen wrote:

quoted

On 07/03/16 21:53, Uwe Kleine-K?nig wrote:

quoted

This asserts that the display is on after the driver is initialized.
Otherwise, depending on how the boot loader handled the display, it is
either disabled as the regulator doesn't seem in use, or it stays off.

Signed-off-by: Uwe Kleine-K?nig <redacted>
---
 drivers/video/fbdev/imxfb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c
index c5fcedde2a60..3dd2824e6773 100644
--- a/drivers/video/fbdev/imxfb.c
+++ b/drivers/video/fbdev/imxfb.c

@@ -979,8 +979,17 @@ static int imxfb_probe(struct platform_device *pdev)
 	imxfb_enable_controller(fbi);
 	fbi->pdev = pdev;
 
+	if (!IS_ERR(fbi->lcd_pwr)) {
+		ret = regulator_enable(fbi->lcd_pwr);
+		if (ret)
+			goto failed_regulator;
+	}
+
 	return 0;
 
+failed_regulator:
+	imxfb_disable_controller(fbi);
+
 failed_lcd:
 	unregister_framebuffer(info);

So I didn't go through the code in detail, but this doesn't look correct
to me.

Where is the regulator disabled which now gets enabled in probe?

It isn't, the display should be on after all :-)

quoted

imxfb_lcd_set_power() handles the regulator enable/disable, so doesn't
this mean the regulator would always be enabled? You first enable it in
probe, then imxfb_lcd_set_power() enables it at some point (?), so the
enable-count is two then.

imxfb_lcd_set_power isn't called during boot.

quoted

To be honest, I've never used 'struct lcd_ops', but I think the enabling
of the regulator should happen somehow via that. If the regulator needs
to be enabled at probe time, then the probe should somehow cause
lcd_ops->set_power to get called.

Why does the regulator need to be enabled at probe? Or are you saying
imxfb_lcd_set_power() is never called in your case?

Right. I just confirmed that with next-20160419 with this patch on top:

And now I also checked the code. The driver's lcd_ops is only used in

	lcd = devm_lcd_device_register(&pdev->dev, "imxfb-lcd", &pdev->dev, fbi, &imxfb_lcd_ops);

(i.e. the last argument). The only thing that happes with the lcd_ops is
that it is assigned to a newly allocated struct lcd_device's .ops. There
imxfb_lcd_set_power is only used for some sysfs attributes (for the lcd)
and the function fb_blank which is in turn not called automatically,
only by some sysfs attributes and ioctls.

So as of now regulator_init_complete disables the (unused) power
regulator before userspace is up.

To reach my goal to keep the display on (showing whatever the bootloader
initialized the display with) until the application starts caring for
the display some driver must enable the regulator. The only candidates
are the lcd driver (i.e. the patch under discussion) or some mechanism
in the fb core. As there doesn't seem to be code in the core (and the
core is dead?) what do you suggest me to do?

The simplefb driver (CONFIG_FB_SIMPLE) is designed for exactly this
purpose. If you rely on the bootloader's framebuffer being preserved
until your application starts without any driver that reserves the
corresponding memory, the framebuffer contents may be corrupted by the
booting kernel.


Lothar Wa?mann