| Message ID | 1406806970-12561-1-git-send-email-thierry.reding@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 31, 2014 at 01:42:50PM +0200, Thierry Reding wrote: [...] > @@ -317,6 +319,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; > + > backlight_update_status(bl); Looking at this again, perhaps a more sensible thing to do would be to not call backlight_update_status() in the first place. For example if the board defines that backlight should be kept off at boot, but the bootloader had already enabled it, then this would effectively turn off the backlight again. I think it's safe to assume that if the bootloader sets up the backlight then it would also set up the display. Therefore not touching the backlight state at all at probe time seems like the safest default. Of course that doesn't help people who use some dumb framebuffer driver and therefore nothing explicitly enables the backlight. So it would still be changing behaviour for people for whom the bootloader doesn't set up the backlight at all and who therefore rely on the kernel to turn it on. We could perhaps alleviate that pain a little by making this dependent on whether or not the board is booted using DT on the assumption that anything that uses DT would be "modern" enough to provide a means to automatically enable the backlight at the right moment. Thierry
Cc'ing Ajay (who raised this on a different thread recently), therefore quoting all of the original email. Thierry On Thu, Jul 31, 2014 at 01:42:50PM +0200, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > 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". > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > I've been meaning to send this for a while but was always holding back > because of the indoctrination that this type of configuration shouldn't > be part of device tree. However this issue was recently raised again in > the context of power up sequences for display panels. As described above > the issue is that panel datasheets recommend that the backlight attached > to a panel be turned on at the very last step to avoid visual glitches > during the panel's power up sequence. With the current implementation it > is typical for the backlight to be probed before the display panel. That > has, in many cases, the side-effect of enabling the backlight, therefore > making the screen content visible before it's actually initialized. > > Some panels come up with random garbage when uninitialized, others show > all white. With some luck the panel will be all black and users won't > really notice. > > This patch is an attempt to enable boards to override the default of > turning on the backlight for the pwm-backlight driver. I'm not sure if > there was a specific reason to turn on the backlight by default when > this driver was initially written, but the fact is that since it has > pretty much always been like this we can't really go and change the > default, otherwise a lot of people may end up with no backlight and no > clue as to how to enable it. So the only reasonable thing we can do is > to keep the old behaviour and give new boards a way to override it if > they know that some other part of the stack will enable it at the right > moment. > > .../devicetree/bindings/video/backlight/pwm-backlight.txt | 1 + > drivers/video/backlight/pwm_bl.c | 8 ++++++++ > include/linux/pwm_backlight.h | 2 ++ > 3 files changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt > index 764db86d441a..65e001a1733d 100644 > --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt > +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt > @@ -17,6 +17,7 @@ Optional properties: > "pwms" property (see PWM binding[0]) > - enable-gpios: contains a single GPIO specifier for the GPIO which enables > and disables the backlight (see GPIO binding[1]) > + - backlight-boot-off: keep the backlight disabled on boot > > [0]: Documentation/devicetree/bindings/pwm/pwm.txt > [1]: Documentation/devicetree/bindings/gpio/gpio.txt > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c > index d7a3d13e72ec..62adfc9d37a7 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -173,6 +173,8 @@ static int pwm_backlight_parse_dt(struct device *dev, > data->max_brightness--; > } > > + data->boot_off = of_property_read_bool(node, "backlight-boot-off"); > + > return 0; > } > > @@ -317,6 +319,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; > + > backlight_update_status(bl); > > platform_set_drvdata(pdev, bl); > diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h > index efdd9227a49c..1fc14989da4a 100644 > --- a/include/linux/pwm_backlight.h > +++ b/include/linux/pwm_backlight.h > @@ -15,6 +15,8 @@ struct platform_pwm_backlight_data { > unsigned int *levels; > /* TODO remove once all users are switched to gpiod_* API */ > int enable_gpio; > + bool boot_off; > + > int (*init)(struct device *dev); > int (*notify)(struct device *dev, int brightness); > void (*notify_after)(struct device *dev, int brightness); > -- > 2.0.3 >
On Thursday, July 31, 2014 8:55 PM, Thierry Reding wrote: > On Thu, Jul 31, 2014 at 01:42:50PM +0200, Thierry Reding wrote: > [...] > > @@ -317,6 +319,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; > > + > > backlight_update_status(bl); > > Looking at this again, perhaps a more sensible thing to do would be to > not call backlight_update_status() in the first place. For example if > the board defines that backlight should be kept off at boot, but the > bootloader had already enabled it, then this would effectively turn off > the backlight again. (+cc Ajay Kumar) Personally, I prefer not to call backlight_update_status(), when the backlight was already turned on by bootloader. Also, it would be better for subsystems such as DRM to handle the power of panel. > > I think it's safe to assume that if the bootloader sets up the backlight > then it would also set up the display. Therefore not touching the > backlight state at all at probe time seems like the safest default. > > Of course that doesn't help people who use some dumb framebuffer driver > and therefore nothing explicitly enables the backlight. So it would > still be changing behaviour for people for whom the bootloader doesn't > set up the backlight at all and who therefore rely on the kernel to turn > it on. > > We could perhaps alleviate that pain a little by making this dependent > on whether or not the board is booted using DT on the assumption that > anything that uses DT would be "modern" enough to provide a means to > automatically enable the backlight at the right moment. I also agree with the way to use DT. Best regards, Jingoo Han > > Thierry
Hi Thierry, Thanks for adding me. On Thu, Jul 31, 2014 at 5:26 PM, Thierry Reding <thierry.reding@gmail.com> wrote: > Cc'ing Ajay (who raised this on a different thread recently), therefore > quoting all of the original email. > > Thierry > > On Thu, Jul 31, 2014 at 01:42:50PM +0200, Thierry Reding wrote: >> From: Thierry Reding <treding@nvidia.com> >> >> 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". >> >> Signed-off-by: Thierry Reding <treding@nvidia.com> >> --- >> I've been meaning to send this for a while but was always holding back >> because of the indoctrination that this type of configuration shouldn't >> be part of device tree. However this issue was recently raised again in >> the context of power up sequences for display panels. As described above >> the issue is that panel datasheets recommend that the backlight attached >> to a panel be turned on at the very last step to avoid visual glitches >> during the panel's power up sequence. With the current implementation it >> is typical for the backlight to be probed before the display panel. That >> has, in many cases, the side-effect of enabling the backlight, therefore >> making the screen content visible before it's actually initialized. >> >> Some panels come up with random garbage when uninitialized, others show >> all white. With some luck the panel will be all black and users won't >> really notice. Right, most of the DSI panels are lucky. But the case is not the same with few eDP/LVDS panels. >> This patch is an attempt to enable boards to override the default of >> turning on the backlight for the pwm-backlight driver. I'm not sure if >> there was a specific reason to turn on the backlight by default when >> this driver was initially written, but the fact is that since it has >> pretty much always been like this we can't really go and change the >> default, otherwise a lot of people may end up with no backlight and no >> clue as to how to enable it. So the only reasonable thing we can do is >> to keep the old behaviour and give new boards a way to override it if >> they know that some other part of the stack will enable it at the right >> moment. Agreed! And, I have tested this patch. This resolves the only remaining problem for display on peach_pi. I cannot see the boot time glitch anymore :) Ajay >> .../devicetree/bindings/video/backlight/pwm-backlight.txt | 1 + >> drivers/video/backlight/pwm_bl.c | 8 ++++++++ >> include/linux/pwm_backlight.h | 2 ++ >> 3 files changed, 11 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> index 764db86d441a..65e001a1733d 100644 >> --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> @@ -17,6 +17,7 @@ Optional properties: >> "pwms" property (see PWM binding[0]) >> - enable-gpios: contains a single GPIO specifier for the GPIO which enables >> and disables the backlight (see GPIO binding[1]) >> + - backlight-boot-off: keep the backlight disabled on boot >> >> [0]: Documentation/devicetree/bindings/pwm/pwm.txt >> [1]: Documentation/devicetree/bindings/gpio/gpio.txt >> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c >> index d7a3d13e72ec..62adfc9d37a7 100644 >> --- a/drivers/video/backlight/pwm_bl.c >> +++ b/drivers/video/backlight/pwm_bl.c >> @@ -173,6 +173,8 @@ static int pwm_backlight_parse_dt(struct device *dev, >> data->max_brightness--; >> } >> >> + data->boot_off = of_property_read_bool(node, "backlight-boot-off"); >> + >> return 0; >> } >> >> @@ -317,6 +319,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; >> + >> backlight_update_status(bl); >> >> platform_set_drvdata(pdev, bl); >> diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h >> index efdd9227a49c..1fc14989da4a 100644 >> --- a/include/linux/pwm_backlight.h >> +++ b/include/linux/pwm_backlight.h >> @@ -15,6 +15,8 @@ struct platform_pwm_backlight_data { >> unsigned int *levels; >> /* TODO remove once all users are switched to gpiod_* API */ >> int enable_gpio; >> + bool boot_off; >> + >> int (*init)(struct device *dev); >> int (*notify)(struct device *dev, int brightness); >> void (*notify_after)(struct device *dev, int brightness); >> -- >> 2.0.3 >>
On Thu, 31 Jul 2014, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > 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". > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > I've been meaning to send this for a while but was always holding back > because of the indoctrination that this type of configuration shouldn't > be part of device tree. However this issue was recently raised again in > the context of power up sequences for display panels. As described above > the issue is that panel datasheets recommend that the backlight attached > to a panel be turned on at the very last step to avoid visual glitches > during the panel's power up sequence. With the current implementation it > is typical for the backlight to be probed before the display panel. That > has, in many cases, the side-effect of enabling the backlight, therefore > making the screen content visible before it's actually initialized. > > Some panels come up with random garbage when uninitialized, others show > all white. With some luck the panel will be all black and users won't > really notice. > > This patch is an attempt to enable boards to override the default of > turning on the backlight for the pwm-backlight driver. I'm not sure if > there was a specific reason to turn on the backlight by default when > this driver was initially written, but the fact is that since it has > pretty much always been like this we can't really go and change the > default, otherwise a lot of people may end up with no backlight and no > clue as to how to enable it. So the only reasonable thing we can do is > to keep the old behaviour and give new boards a way to override it if > they know that some other part of the stack will enable it at the right > moment. > > .../devicetree/bindings/video/backlight/pwm-backlight.txt | 1 + > drivers/video/backlight/pwm_bl.c | 8 ++++++++ > include/linux/pwm_backlight.h | 2 ++ > 3 files changed, 11 insertions(+) Some people on the list are talking about this again. What was the verdict?
Hi All, Since many panel power sequence request backlight stay disable before panel power ready, but with now pwm-backlight drvier, it default to enable backlight when pwm-backlight probe, it mess up the panel power sequence. So we need this patch. This patch have been fly for a long time, does anyone have plan to merge it? On Thursday, July 31, 2014 07:42 PM, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > 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". > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > I've been meaning to send this for a while but was always holding back > because of the indoctrination that this type of configuration shouldn't > be part of device tree. However this issue was recently raised again in > the context of power up sequences for display panels. As described above > the issue is that panel datasheets recommend that the backlight attached > to a panel be turned on at the very last step to avoid visual glitches > during the panel's power up sequence. With the current implementation it > is typical for the backlight to be probed before the display panel. That > has, in many cases, the side-effect of enabling the backlight, therefore > making the screen content visible before it's actually initialized. > > Some panels come up with random garbage when uninitialized, others show > all white. With some luck the panel will be all black and users won't > really notice. > > This patch is an attempt to enable boards to override the default of > turning on the backlight for the pwm-backlight driver. I'm not sure if > there was a specific reason to turn on the backlight by default when > this driver was initially written, but the fact is that since it has > pretty much always been like this we can't really go and change the > default, otherwise a lot of people may end up with no backlight and no > clue as to how to enable it. So the only reasonable thing we can do is > to keep the old behaviour and give new boards a way to override it if > they know that some other part of the stack will enable it at the right > moment. > > .../devicetree/bindings/video/backlight/pwm-backlight.txt | 1 + > drivers/video/backlight/pwm_bl.c | 8 ++++++++ > include/linux/pwm_backlight.h | 2 ++ > 3 files changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt > index 764db86d441a..65e001a1733d 100644 > --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt > +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt > @@ -17,6 +17,7 @@ Optional properties: > "pwms" property (see PWM binding[0]) > - enable-gpios: contains a single GPIO specifier for the GPIO which enables > and disables the backlight (see GPIO binding[1]) > + - backlight-boot-off: keep the backlight disabled on boot > > [0]: Documentation/devicetree/bindings/pwm/pwm.txt > [1]: Documentation/devicetree/bindings/gpio/gpio.txt > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c > index d7a3d13e72ec..62adfc9d37a7 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -173,6 +173,8 @@ static int pwm_backlight_parse_dt(struct device *dev, > data->max_brightness--; > } > > + data->boot_off = of_property_read_bool(node, "backlight-boot-off"); > + > return 0; > } > > @@ -317,6 +319,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; > + > backlight_update_status(bl); > > platform_set_drvdata(pdev, bl); > diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h > index efdd9227a49c..1fc14989da4a 100644 > --- a/include/linux/pwm_backlight.h > +++ b/include/linux/pwm_backlight.h > @@ -15,6 +15,8 @@ struct platform_pwm_backlight_data { > unsigned int *levels; > /* TODO remove once all users are switched to gpiod_* API */ > int enable_gpio; > + bool boot_off; > + > int (*init)(struct device *dev); > int (*notify)(struct device *dev, int brightness); > void (*notify_after)(struct device *dev, int brightness);
Hi, On 2018-01-04 04:18, hl wrote: > Hi All, > > Since many panel power sequence request backlight stay disable > > before panel power ready, but with now pwm-backlight drvier, it default to > > enable backlight when pwm-backlight probe, it mess up the panel power > sequence. > > So we need this patch. This patch have been fly for a long time, does > anyone have plan > > to merge it? you should not need this anymore since we have: 892c7788c724backlight: pwm_bl: Fix GPIO out for unimplemented .get_direction() d1b812945750 backlight: pwm_bl: Check the PWM state for initial backlight power state 7613c922315e backlight: pwm_bl: Move the checks for initial power state to a separate function With these in place the backlight will be kept disabled if it was disabled during boot _if_ you have booted via DT _and_ you have a phandle pointing to the backlight node (implying that the backlight is managed by the display driver). > > > On Thursday, July 31, 2014 07:42 PM, Thierry Reding wrote: >> From: Thierry Reding <treding@nvidia.com> >> >> 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". >> >> Signed-off-by: Thierry Reding <treding@nvidia.com> >> --- >> I've been meaning to send this for a while but was always holding back >> because of the indoctrination that this type of configuration shouldn't >> be part of device tree. However this issue was recently raised again in >> the context of power up sequences for display panels. As described above >> the issue is that panel datasheets recommend that the backlight attached >> to a panel be turned on at the very last step to avoid visual glitches >> during the panel's power up sequence. With the current implementation it >> is typical for the backlight to be probed before the display panel. That >> has, in many cases, the side-effect of enabling the backlight, therefore >> making the screen content visible before it's actually initialized. >> >> Some panels come up with random garbage when uninitialized, others show >> all white. With some luck the panel will be all black and users won't >> really notice. >> >> This patch is an attempt to enable boards to override the default of >> turning on the backlight for the pwm-backlight driver. I'm not sure if >> there was a specific reason to turn on the backlight by default when >> this driver was initially written, but the fact is that since it has >> pretty much always been like this we can't really go and change the >> default, otherwise a lot of people may end up with no backlight and no >> clue as to how to enable it. So the only reasonable thing we can do is >> to keep the old behaviour and give new boards a way to override it if >> they know that some other part of the stack will enable it at the right >> moment. >> >> .../devicetree/bindings/video/backlight/pwm-backlight.txt | 1 + >> drivers/video/backlight/pwm_bl.c | >> 8 ++++++++ >> include/linux/pwm_backlight.h | >> 2 ++ >> 3 files changed, 11 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> index 764db86d441a..65e001a1733d 100644 >> --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> @@ -17,6 +17,7 @@ Optional properties: >> "pwms" property (see PWM binding[0]) >> - enable-gpios: contains a single GPIO specifier for the GPIO >> which enables >> and disables the backlight (see GPIO binding[1]) >> + - backlight-boot-off: keep the backlight disabled on boot >> [0]: Documentation/devicetree/bindings/pwm/pwm.txt >> [1]: Documentation/devicetree/bindings/gpio/gpio.txt >> diff --git a/drivers/video/backlight/pwm_bl.c >> b/drivers/video/backlight/pwm_bl.c >> index d7a3d13e72ec..62adfc9d37a7 100644 >> --- a/drivers/video/backlight/pwm_bl.c >> +++ b/drivers/video/backlight/pwm_bl.c >> @@ -173,6 +173,8 @@ static int pwm_backlight_parse_dt(struct device *dev, >> data->max_brightness--; >> } >> + data->boot_off = of_property_read_bool(node, >> "backlight-boot-off"); >> + >> return 0; >> } >> @@ -317,6 +319,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; >> + >> backlight_update_status(bl); >> platform_set_drvdata(pdev, bl); >> diff --git a/include/linux/pwm_backlight.h >> b/include/linux/pwm_backlight.h >> index efdd9227a49c..1fc14989da4a 100644 >> --- a/include/linux/pwm_backlight.h >> +++ b/include/linux/pwm_backlight.h >> @@ -15,6 +15,8 @@ struct platform_pwm_backlight_data { >> unsigned int *levels; >> /* TODO remove once all users are switched to gpiod_* API */ >> int enable_gpio; >> + bool boot_off; >> + >> int (*init)(struct device *dev); >> int (*notify)(struct device *dev, int brightness); >> void (*notify_after)(struct device *dev, int brightness); > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Hi On Thursday, January 04, 2018 04:22 PM, Peter Ujfalusi wrote: > Hi, > > On 2018-01-04 04:18, hl wrote: >> Hi All, >> >> Since many panel power sequence request backlight stay disable >> >> before panel power ready, but with now pwm-backlight drvier, it default to >> >> enable backlight when pwm-backlight probe, it mess up the panel power >> sequence. >> >> So we need this patch. This patch have been fly for a long time, does >> anyone have plan >> >> to merge it? > you should not need this anymore since we have: > 892c7788c724backlight: pwm_bl: Fix GPIO out for unimplemented > .get_direction() > > d1b812945750 backlight: pwm_bl: Check the PWM state for initial > backlight power state > > 7613c922315e backlight: pwm_bl: Move the checks for initial power state > to a separate function > > With these in place the backlight will be kept disabled if it was > disabled during boot _if_ you have booted via DT _and_ you have a > phandle pointing to the backlight node (implying that the backlight is > managed by the display driver). Oh, thanks for reminding me. > >> >> On Thursday, July 31, 2014 07:42 PM, Thierry Reding wrote: >>> From: Thierry Reding <treding@nvidia.com> >>> >>> 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". >>> >>> Signed-off-by: Thierry Reding <treding@nvidia.com> >>> --- >>> I've been meaning to send this for a while but was always holding back >>> because of the indoctrination that this type of configuration shouldn't >>> be part of device tree. However this issue was recently raised again in >>> the context of power up sequences for display panels. As described above >>> the issue is that panel datasheets recommend that the backlight attached >>> to a panel be turned on at the very last step to avoid visual glitches >>> during the panel's power up sequence. With the current implementation it >>> is typical for the backlight to be probed before the display panel. That >>> has, in many cases, the side-effect of enabling the backlight, therefore >>> making the screen content visible before it's actually initialized. >>> >>> Some panels come up with random garbage when uninitialized, others show >>> all white. With some luck the panel will be all black and users won't >>> really notice. >>> >>> This patch is an attempt to enable boards to override the default of >>> turning on the backlight for the pwm-backlight driver. I'm not sure if >>> there was a specific reason to turn on the backlight by default when >>> this driver was initially written, but the fact is that since it has >>> pretty much always been like this we can't really go and change the >>> default, otherwise a lot of people may end up with no backlight and no >>> clue as to how to enable it. So the only reasonable thing we can do is >>> to keep the old behaviour and give new boards a way to override it if >>> they know that some other part of the stack will enable it at the right >>> moment. >>> >>> .../devicetree/bindings/video/backlight/pwm-backlight.txt | 1 + >>> drivers/video/backlight/pwm_bl.c | >>> 8 ++++++++ >>> include/linux/pwm_backlight.h | >>> 2 ++ >>> 3 files changed, 11 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >>> b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >>> index 764db86d441a..65e001a1733d 100644 >>> --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >>> +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >>> @@ -17,6 +17,7 @@ Optional properties: >>> "pwms" property (see PWM binding[0]) >>> - enable-gpios: contains a single GPIO specifier for the GPIO >>> which enables >>> and disables the backlight (see GPIO binding[1]) >>> + - backlight-boot-off: keep the backlight disabled on boot >>> [0]: Documentation/devicetree/bindings/pwm/pwm.txt >>> [1]: Documentation/devicetree/bindings/gpio/gpio.txt >>> diff --git a/drivers/video/backlight/pwm_bl.c >>> b/drivers/video/backlight/pwm_bl.c >>> index d7a3d13e72ec..62adfc9d37a7 100644 >>> --- a/drivers/video/backlight/pwm_bl.c >>> +++ b/drivers/video/backlight/pwm_bl.c >>> @@ -173,6 +173,8 @@ static int pwm_backlight_parse_dt(struct device *dev, >>> data->max_brightness--; >>> } >>> + data->boot_off = of_property_read_bool(node, >>> "backlight-boot-off"); >>> + >>> return 0; >>> } >>> @@ -317,6 +319,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; >>> + >>> backlight_update_status(bl); >>> platform_set_drvdata(pdev, bl); >>> diff --git a/include/linux/pwm_backlight.h >>> b/include/linux/pwm_backlight.h >>> index efdd9227a49c..1fc14989da4a 100644 >>> --- a/include/linux/pwm_backlight.h >>> +++ b/include/linux/pwm_backlight.h >>> @@ -15,6 +15,8 @@ struct platform_pwm_backlight_data { >>> unsigned int *levels; >>> /* TODO remove once all users are switched to gpiod_* API */ >>> int enable_gpio; >>> + bool boot_off; >>> + >>> int (*init)(struct device *dev); >>> int (*notify)(struct device *dev, int brightness); >>> void (*notify_after)(struct device *dev, int brightness); >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > - Péter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > >
diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt index 764db86d441a..65e001a1733d 100644 --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt @@ -17,6 +17,7 @@ Optional properties: "pwms" property (see PWM binding[0]) - enable-gpios: contains a single GPIO specifier for the GPIO which enables and disables the backlight (see GPIO binding[1]) + - backlight-boot-off: keep the backlight disabled on boot [0]: Documentation/devicetree/bindings/pwm/pwm.txt [1]: Documentation/devicetree/bindings/gpio/gpio.txt diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index d7a3d13e72ec..62adfc9d37a7 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -173,6 +173,8 @@ static int pwm_backlight_parse_dt(struct device *dev, data->max_brightness--; } + data->boot_off = of_property_read_bool(node, "backlight-boot-off"); + return 0; } @@ -317,6 +319,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; + backlight_update_status(bl); platform_set_drvdata(pdev, bl); diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h index efdd9227a49c..1fc14989da4a 100644 --- a/include/linux/pwm_backlight.h +++ b/include/linux/pwm_backlight.h @@ -15,6 +15,8 @@ struct platform_pwm_backlight_data { unsigned int *levels; /* TODO remove once all users are switched to gpiod_* API */ int enable_gpio; + bool boot_off; + int (*init)(struct device *dev); int (*notify)(struct device *dev, int brightness); void (*notify_after)(struct device *dev, int brightness);