[PATCH] pwm-backlight: Turn off pwm backlight in probe

Subsystems: backlight class/subsystem, framebuffer layer, the rest

STALE4327d

8 messages, 5 authors, 2014-10-08 · open the first message on its own page

[PATCH] pwm-backlight: Turn off pwm backlight in probe

From: Markus Pargmann <hidden>
Date: 2014-10-06 19:22:44

The backlight will be enabled by the panel again if it is used. So we
can save the default brightness and disable the pwm backlight when
probing.

Signed-off-by: Markus Pargmann <redacted>
---
 drivers/video/backlight/pwm_bl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 336b83be7e2d..b4f433a6f106 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -317,9 +317,11 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 		data->dft_brightness = data->max_brightness;
 	}
 
-	bl->props.brightness = data->dft_brightness;
+	bl->props.brightness = 0;
 	backlight_update_status(bl);
 
+	bl->props.brightness = data->dft_brightness;
+
 	platform_set_drvdata(pdev, bl);
 	return 0;
 
-- 
2.1.0

[PATCH] pwm-backlight: Turn off pwm backlight in probe

From: Thierry Reding <hidden>
Date: 2014-10-07 08:35:49

On Mon, Oct 06, 2014 at 09:22:44PM +0200, Markus Pargmann wrote:
quoted hunk
The backlight will be enabled by the panel again if it is used. So we
can save the default brightness and disable the pwm backlight when
probing.

Signed-off-by: Markus Pargmann <redacted>
---
 drivers/video/backlight/pwm_bl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 336b83be7e2d..b4f433a6f106 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -317,9 +317,11 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 		data->dft_brightness = data->max_brightness;
 	}
 
-	bl->props.brightness = data->dft_brightness;
+	bl->props.brightness = 0;
 	backlight_update_status(bl);
 
+	bl->props.brightness = data->dft_brightness;
+
 	platform_set_drvdata(pdev, bl);
 	return 0;
 
It would be nice if it was that easy. But we can't do this, because it
will regress for users of this driver that don't use a panel or DRM. If
the PWM backlight driver is used for example in conjunction with a plain
fbdev driver it isn't necessarily hooked up with anything and won't be
enabled automatically. That's really bad if fbdev is the only output you
have since you'd have to blindly type the commands to enable the
backlight. Furthermore disabling backlight isn't always what you want to
do. For example if the bootloader already turned it on and you hand over
from bootloader to kernel in a seamless way, then you absolutely want to
keep backlight on all the time.

See also[0] for a different proposal to solve the same problem. Back at
the time that received only a very few replies, but it would be nice if
Lee and Bryan could look at it again and see if we can come up with some
way to deal with this situation.

Thierry

[0]: https://lkml.org/lkml/2014/7/31/259

[PATCH] pwm-backlight: Turn off pwm backlight in probe

From: Lee Jones <hidden>
Date: 2014-10-07 09:06:27

On Tue, 07 Oct 2014, Thierry Reding wrote:
On Mon, Oct 06, 2014 at 09:22:44PM +0200, Markus Pargmann wrote:
quoted
The backlight will be enabled by the panel again if it is used. So we
can save the default brightness and disable the pwm backlight when
probing.

Signed-off-by: Markus Pargmann <redacted>
---
 drivers/video/backlight/pwm_bl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 336b83be7e2d..b4f433a6f106 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -317,9 +317,11 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 		data->dft_brightness = data->max_brightness;
 	}
 
-	bl->props.brightness = data->dft_brightness;
+	bl->props.brightness = 0;
 	backlight_update_status(bl);
 
+	bl->props.brightness = data->dft_brightness;
+
 	platform_set_drvdata(pdev, bl);
 	return 0;
 
It would be nice if it was that easy. But we can't do this, because it
will regress for users of this driver that don't use a panel or DRM. If
the PWM backlight driver is used for example in conjunction with a plain
fbdev driver it isn't necessarily hooked up with anything and won't be
enabled automatically. That's really bad if fbdev is the only output you
have since you'd have to blindly type the commands to enable the
backlight. Furthermore disabling backlight isn't always what you want to
do. For example if the bootloader already turned it on and you hand over
from bootloader to kernel in a seamless way, then you absolutely want to
keep backlight on all the time.

See also[0] for a different proposal to solve the same problem. Back at
the time that received only a very few replies, but it would be nice if
Lee and Bryan could look at it again and see if we can come up with some
way to deal with this situation.
I don't have any experience with this stuff.  Jingoo and Bryan are the
_real_ reviewers for Backlight.  I just maintain the patches.  That
and I'm already swamped with MFD.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] pwm-backlight: Turn off pwm backlight in probe

From: Markus Pargmann <hidden>
Date: 2014-10-07 09:07:18

Hi,

On Tue, Oct 07, 2014 at 10:35:49AM +0200, Thierry Reding wrote:
On Mon, Oct 06, 2014 at 09:22:44PM +0200, Markus Pargmann wrote:
quoted
The backlight will be enabled by the panel again if it is used. So we
can save the default brightness and disable the pwm backlight when
probing.

Signed-off-by: Markus Pargmann <redacted>
---
 drivers/video/backlight/pwm_bl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 336b83be7e2d..b4f433a6f106 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -317,9 +317,11 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 		data->dft_brightness = data->max_brightness;
 	}
 
-	bl->props.brightness = data->dft_brightness;
+	bl->props.brightness = 0;
 	backlight_update_status(bl);
 
+	bl->props.brightness = data->dft_brightness;
+
 	platform_set_drvdata(pdev, bl);
 	return 0;
 
It would be nice if it was that easy. But we can't do this, because it
will regress for users of this driver that don't use a panel or DRM. If
the PWM backlight driver is used for example in conjunction with a plain
fbdev driver it isn't necessarily hooked up with anything and won't be
enabled automatically. That's really bad if fbdev is the only output you
have since you'd have to blindly type the commands to enable the
backlight. Furthermore disabling backlight isn't always what you want to
do. For example if the bootloader already turned it on and you hand over
from bootloader to kernel in a seamless way, then you absolutely want to
keep backlight on all the time.

See also[0] for a different proposal to solve the same problem. Back at
the time that received only a very few replies, but it would be nice if
Lee and Bryan could look at it again and see if we can come up with some
way to deal with this situation.
Yes your proposal looks a lot better to handle the different use cases.
The DT-binding is not a hardware description but I don't see any better
way of passing that information. So it would be good to get your
solution mainline.

Best regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH] pwm-backlight: Turn off pwm backlight in probe

From: Ajay kumar <hidden>
Date: 2014-10-07 09:17:53

On Tue, Oct 7, 2014 at 2:37 PM, Markus Pargmann [off-list ref] wrote:
Hi,

On Tue, Oct 07, 2014 at 10:35:49AM +0200, Thierry Reding wrote:
quoted
On Mon, Oct 06, 2014 at 09:22:44PM +0200, Markus Pargmann wrote:
quoted
The backlight will be enabled by the panel again if it is used. So we
can save the default brightness and disable the pwm backlight when
probing.

Signed-off-by: Markus Pargmann <redacted>
---
 drivers/video/backlight/pwm_bl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 336b83be7e2d..b4f433a6f106 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -317,9 +317,11 @@ static int pwm_backlight_probe(struct platform_device *pdev)
            data->dft_brightness = data->max_brightness;
    }

-   bl->props.brightness = data->dft_brightness;
+   bl->props.brightness = 0;
    backlight_update_status(bl);

+   bl->props.brightness = data->dft_brightness;
+
    platform_set_drvdata(pdev, bl);
    return 0;
It would be nice if it was that easy. But we can't do this, because it
will regress for users of this driver that don't use a panel or DRM. If
the PWM backlight driver is used for example in conjunction with a plain
fbdev driver it isn't necessarily hooked up with anything and won't be
enabled automatically. That's really bad if fbdev is the only output you
have since you'd have to blindly type the commands to enable the
backlight. Furthermore disabling backlight isn't always what you want to
do. For example if the bootloader already turned it on and you hand over
from bootloader to kernel in a seamless way, then you absolutely want to
keep backlight on all the time.

See also[0] for a different proposal to solve the same problem. Back at
the time that received only a very few replies, but it would be nice if
Lee and Bryan could look at it again and see if we can come up with some
way to deal with this situation.
Yes your proposal looks a lot better to handle the different use cases.
The DT-binding is not a hardware description but I don't see any better
way of passing that information. So it would be good to get your
solution mainline.
+1
And, I have already tested Thierry's proposal on Exynos5800 peach_pi.

Ajay

[PATCH] pwm-backlight: Turn off pwm backlight in probe

From: Jingoo Han <hidden>
Date: 2014-10-08 09:44:19

On Tuesday, October 07, 2014 5:36 PM, Thierry Reding wrote:
On Mon, Oct 06, 2014 at 09:22:44PM +0200, Markus Pargmann wrote:
quoted
The backlight will be enabled by the panel again if it is used. So we
can save the default brightness and disable the pwm backlight when
probing.

Signed-off-by: Markus Pargmann <redacted>
---
 drivers/video/backlight/pwm_bl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 336b83be7e2d..b4f433a6f106 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -317,9 +317,11 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 		data->dft_brightness = data->max_brightness;
 	}

-	bl->props.brightness = data->dft_brightness;
+	bl->props.brightness = 0;
 	backlight_update_status(bl);

+	bl->props.brightness = data->dft_brightness;
+
 	platform_set_drvdata(pdev, bl);
 	return 0;
It would be nice if it was that easy. But we can't do this, because it
will regress for users of this driver that don't use a panel or DRM. If
the PWM backlight driver is used for example in conjunction with a plain
fbdev driver it isn't necessarily hooked up with anything and won't be
enabled automatically. That's really bad if fbdev is the only output you
have since you'd have to blindly type the commands to enable the
backlight. Furthermore disabling backlight isn't always what you want to
do. For example if the bootloader already turned it on and you hand over
from bootloader to kernel in a seamless way, then you absolutely want to
keep backlight on all the time.

See also[0] for a different proposal to solve the same problem. Back at
the time that received only a very few replies, but it would be nice if
Lee and Bryan could look at it again and see if we can come up with some
way to deal with this situation.
(+cc Ajay Kumar)

As I said earlier, I agree with this proposal.
Thierry

[0]: https://lkml.org/lkml/2014/7/31/259

[PATCH] pwm-backlight: Turn off pwm backlight in probe

From: Markus Pargmann <hidden>
Date: 2014-10-08 09:55:37

On Tue, Oct 07, 2014 at 02:47:53PM +0530, Ajay kumar wrote:
On Tue, Oct 7, 2014 at 2:37 PM, Markus Pargmann [off-list ref] wrote:
quoted
Hi,

On Tue, Oct 07, 2014 at 10:35:49AM +0200, Thierry Reding wrote:
quoted
On Mon, Oct 06, 2014 at 09:22:44PM +0200, Markus Pargmann wrote:
quoted
The backlight will be enabled by the panel again if it is used. So we
can save the default brightness and disable the pwm backlight when
probing.

Signed-off-by: Markus Pargmann <redacted>
---
 drivers/video/backlight/pwm_bl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 336b83be7e2d..b4f433a6f106 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -317,9 +317,11 @@ static int pwm_backlight_probe(struct platform_device *pdev)
            data->dft_brightness = data->max_brightness;
    }

-   bl->props.brightness = data->dft_brightness;
+   bl->props.brightness = 0;
    backlight_update_status(bl);

+   bl->props.brightness = data->dft_brightness;
+
    platform_set_drvdata(pdev, bl);
    return 0;
It would be nice if it was that easy. But we can't do this, because it
will regress for users of this driver that don't use a panel or DRM. If
the PWM backlight driver is used for example in conjunction with a plain
fbdev driver it isn't necessarily hooked up with anything and won't be
enabled automatically. That's really bad if fbdev is the only output you
have since you'd have to blindly type the commands to enable the
backlight. Furthermore disabling backlight isn't always what you want to
do. For example if the bootloader already turned it on and you hand over
from bootloader to kernel in a seamless way, then you absolutely want to
keep backlight on all the time.

See also[0] for a different proposal to solve the same problem. Back at
the time that received only a very few replies, but it would be nice if
Lee and Bryan could look at it again and see if we can come up with some
way to deal with this situation.
Yes your proposal looks a lot better to handle the different use cases.
The DT-binding is not a hardware description but I don't see any better
way of passing that information. So it would be good to get your
solution mainline.
+1
And, I have already tested Thierry's proposal on Exynos5800 peach_pi.
I also tested Thierry's patch on i.MX6s now and it works fine.

Regards,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH] pwm-backlight: Turn off pwm backlight in probe

From: Thierry Reding <hidden>
Date: 2014-10-08 12:13:37

On Wed, Oct 08, 2014 at 06:44:19PM +0900, Jingoo Han wrote:
On Tuesday, October 07, 2014 5:36 PM, Thierry Reding wrote:
quoted
On Mon, Oct 06, 2014 at 09:22:44PM +0200, Markus Pargmann wrote:
quoted
The backlight will be enabled by the panel again if it is used. So we
can save the default brightness and disable the pwm backlight when
probing.

Signed-off-by: Markus Pargmann <redacted>
---
 drivers/video/backlight/pwm_bl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 336b83be7e2d..b4f433a6f106 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -317,9 +317,11 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 		data->dft_brightness = data->max_brightness;
 	}

-	bl->props.brightness = data->dft_brightness;
+	bl->props.brightness = 0;
 	backlight_update_status(bl);

+	bl->props.brightness = data->dft_brightness;
+
 	platform_set_drvdata(pdev, bl);
 	return 0;
It would be nice if it was that easy. But we can't do this, because it
will regress for users of this driver that don't use a panel or DRM. If
the PWM backlight driver is used for example in conjunction with a plain
fbdev driver it isn't necessarily hooked up with anything and won't be
enabled automatically. That's really bad if fbdev is the only output you
have since you'd have to blindly type the commands to enable the
backlight. Furthermore disabling backlight isn't always what you want to
do. For example if the bootloader already turned it on and you hand over
from bootloader to kernel in a seamless way, then you absolutely want to
keep backlight on all the time.

See also[0] for a different proposal to solve the same problem. Back at
the time that received only a very few replies, but it would be nice if
Lee and Bryan could look at it again and see if we can come up with some
way to deal with this situation.
(+cc Ajay Kumar)

As I said earlier, I agree with this proposal.
I'm not sure which proposal exactly. I originally proposed adding a
backlight-boot-off property, but that received some pushback from DT
people because it isn't strictly hardware information but configuration
data.

Perhaps a better solution would be to default to not touching the
backlight status on probe at all. But we can't do that for non-DT
because of backwards-compatibility. For DT it seems like all users have
some form of DRM/KMS driver too, so I'd expect them to turn it off when
appropriate.

Should we just go and make that change (no changing backlight state for
DT) and see if anybody complains? If so, I'll gladly prepare a patch to
do just that.

Thierry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help