[led-backlight] default-brightness-level issue

[led-backlight] default-brightness-level issue

[pgeiem]

Dear all,

On a custom board I have a simple DPI panel. Panel's backlight is drive with an I2C led driver (PCA9632). led-backlight driver is sued to manage this as a backlight.

When using brightness-levels and default-brightness-level the backlight stay turned-off even if manually trying to set a different index value to brightness through sysfs.

I traced this issue as follow: When led_bl_update_status() is called the brightness value is returned from backlight_get_brightness() which call backlight_is_blank(). In my case backlight_is_blank() return true due to props.power = FB_BLANK_POWERDOWN which is != FB_BLANK_UNBLANK.

I traced why at startup props.power is FB_BLANK_POWERDOWN and found that in led_bl_probe() when a default brightness is set (>0) props.power is set to FB_BLANK_POWERDOWN which seems incorrect to me.

I made the small change below and default brightness is correctly used at startup.
I am not really sure this is an issue and if my change is correct or if I am doing something incorrect somewhere else. So I first would like to get your opinion on this and if correct will send a patch.

Many thanks in advance
Patrick


diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
--- a/drivers/video/backlight/led_bl.c
+++ b/drivers/video/backlight/led_bl.c
@@ -200,7 +200,7 @@ static int led_bl_probe(struct platform_device *pdev)
        props.type = BACKLIGHT_RAW;
        props.max_brightness = priv->max_brightness;
        props.brightness = priv->default_brightness;
-       props.power = (priv->default_brightness > 0) ? FB_BLANK_POWERDOWN :
+       props.power = (priv->default_brightness == 0) ? FB_BLANK_POWERDOWN :
                      FB_BLANK_UNBLANK;
        priv->bl_dev = backlight_device_register(dev_name(&pdev->dev),
                        &pdev->dev, priv, &led_bl_ops, &props);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [led-backlight] default-brightness-level issue

[Daniel Thompson]

On Fri, Apr 23, 2021 at 01:04:23PM +0000, pgeiem wrote:
> Dear all,
> 
> On a custom board I have a simple DPI panel. Panel's backlight is
> drive with an I2C led driver (PCA9632). led-backlight driver is sued
> to manage this as a backlight.
> 
> When using brightness-levels and default-brightness-level the
> backlight stay turned-off even if manually trying to set a different
> index value to brightness through sysfs.
> 
> I traced this issue as follow: When led_bl_update_status() is called
> the brightness value is returned from backlight_get_brightness() which
> call backlight_is_blank(). In my case backlight_is_blank() return true
> due to props.power = FB_BLANK_POWERDOWN which is != FB_BLANK_UNBLANK.
> 
> I traced why at startup props.power is FB_BLANK_POWERDOWN and found
> that in led_bl_probe() when a default brightness is set (>0)
> props.power is set to FB_BLANK_POWERDOWN which seems incorrect to me.
> 
> I made the small change below and default brightness is correctly used
> at startup.  I am not really sure this is an issue and if my change is
> correct or if I am doing something incorrect somewhere else. So I
> first would like to get your opinion on this and if correct will send
> a patch.

Makes sense. Please send this as a patch.


Daniel.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [led-backlight] default-brightness-level issue

[pgeiem]

On Thursday, April 29, 2021 1:00 PM, Daniel Thompson <daniel.thompson@linaro.org> wrote:

> On Fri, Apr 23, 2021 at 01:04:23PM +0000, pgeiem wrote:
>
> > Dear all,
> > On a custom board I have a simple DPI panel. Panel's backlight is
> > drive with an I2C led driver (PCA9632). led-backlight driver is sued
> > to manage this as a backlight.
> > When using brightness-levels and default-brightness-level the
> > backlight stay turned-off even if manually trying to set a different
> > index value to brightness through sysfs.
> > I traced this issue as follow: When led_bl_update_status() is called
> > the brightness value is returned from backlight_get_brightness() which
> > call backlight_is_blank(). In my case backlight_is_blank() return true
> > due to props.power = FB_BLANK_POWERDOWN which is != FB_BLANK_UNBLANK.
> > I traced why at startup props.power is FB_BLANK_POWERDOWN and found
> > that in led_bl_probe() when a default brightness is set (>0)
> > props.power is set to FB_BLANK_POWERDOWN which seems incorrect to me.
> > I made the small change below and default brightness is correctly used
> > at startup. I am not really sure this is an issue and if my change is
> > correct or if I am doing something incorrect somewhere else. So I
> > first would like to get your opinion on this and if correct will send
> > a patch.
>
> Makes sense. Please send this as a patch.
>
> Daniel.

I finally believe this was correct.

A link between the panel and the backlight was missing in dts. With this link the backlight is turned on (props.power = FB_BLANK_UNBLANK) from drm_panel_enable function after enabling the panel.

Let me know if you disagree and still believe I should send the patch.

Patrick



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [led-backlight] default-brightness-level issue

[Daniel Thompson]

On Thu, Apr 29, 2021 at 11:31:20AM +0000, pgeiem wrote:
> On Thursday, April 29, 2021 1:00 PM, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> 
> > On Fri, Apr 23, 2021 at 01:04:23PM +0000, pgeiem wrote:
> >
> > > Dear all,
> > > On a custom board I have a simple DPI panel. Panel's backlight is
> > > drive with an I2C led driver (PCA9632). led-backlight driver is sued
> > > to manage this as a backlight.
> > > When using brightness-levels and default-brightness-level the
> > > backlight stay turned-off even if manually trying to set a different
> > > index value to brightness through sysfs.
> > > I traced this issue as follow: When led_bl_update_status() is called
> > > the brightness value is returned from backlight_get_brightness() which
> > > call backlight_is_blank(). In my case backlight_is_blank() return true
> > > due to props.power = FB_BLANK_POWERDOWN which is != FB_BLANK_UNBLANK.
> > > I traced why at startup props.power is FB_BLANK_POWERDOWN and found
> > > that in led_bl_probe() when a default brightness is set (>0)
> > > props.power is set to FB_BLANK_POWERDOWN which seems incorrect to me.
> > > I made the small change below and default brightness is correctly used
> > > at startup. I am not really sure this is an issue and if my change is
> > > correct or if I am doing something incorrect somewhere else. So I
> > > first would like to get your opinion on this and if correct will send
> > > a patch.
> >
> > Makes sense. Please send this as a patch.
> >
> > Daniel.
> 
> I finally believe this was correct.
> 
> A link between the panel and the backlight was missing in dts. With this link the backlight is turned on (props.power = FB_BLANK_UNBLANK) from drm_panel_enable function after enabling the panel.
> 
> Let me know if you disagree and still believe I should send the patch.

The original logic is definitely wrong.

However that doesn't mean the driver will be fully correct after fixing
since it does look like is missing the logic found in other similar
drivers (pwm-bl.c and gpio_backlight.c) to handle the various different
cases w.r.t. handover from active backlight by the bootloader.


Daniel.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [led-backlight] default-brightness-level issue

[pgeiem]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, April 29, 2021 2:07 PM, Daniel Thompson <daniel.thompson@linaro.org> wrote:

> On Thu, Apr 29, 2021 at 11:31:20AM +0000, pgeiem wrote:
>
> > On Thursday, April 29, 2021 1:00 PM, Daniel Thompson daniel.thompson@linaro.org wrote:
> >
> > > On Fri, Apr 23, 2021 at 01:04:23PM +0000, pgeiem wrote:
> > >
> > > > Dear all,
> > > > On a custom board I have a simple DPI panel. Panel's backlight is
> > > > drive with an I2C led driver (PCA9632). led-backlight driver is sued
> > > > to manage this as a backlight.
> > > > When using brightness-levels and default-brightness-level the
> > > > backlight stay turned-off even if manually trying to set a different
> > > > index value to brightness through sysfs.
> > > > I traced this issue as follow: When led_bl_update_status() is called
> > > > the brightness value is returned from backlight_get_brightness() which
> > > > call backlight_is_blank(). In my case backlight_is_blank() return true
> > > > due to props.power = FB_BLANK_POWERDOWN which is != FB_BLANK_UNBLANK.
> > > > I traced why at startup props.power is FB_BLANK_POWERDOWN and found
> > > > that in led_bl_probe() when a default brightness is set (>0)
> > > > props.power is set to FB_BLANK_POWERDOWN which seems incorrect to me.
> > > > I made the small change below and default brightness is correctly used
> > > > at startup. I am not really sure this is an issue and if my change is
> > > > correct or if I am doing something incorrect somewhere else. So I
> > > > first would like to get your opinion on this and if correct will send
> > > > a patch.
> > >
> > > Makes sense. Please send this as a patch.
> > > Daniel.
> >
> > I finally believe this was correct.
> > A link between the panel and the backlight was missing in dts. With this link the backlight is turned on (props.power = FB_BLANK_UNBLANK) from drm_panel_enable function after enabling the panel.
> > Let me know if you disagree and still believe I should send the patch.
>
> The original logic is definitely wrong.
>
> However that doesn't mean the driver will be fully correct after fixing
> since it does look like is missing the logic found in other similar
> drivers (pwm-bl.c and gpio_backlight.c) to handle the various different
> cases w.r.t. handover from active backlight by the bootloader.
>
> Daniel.

ok you are right, will see if I can port the logic from others backlight drivers to this one.

Patrick
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel