* Re: [PATCH] Enable backlight when trigger is activated
@ 2019-07-22 20:25 ` Jacek Anaszewski
0 siblings, 0 replies; 23+ messages in thread
From: Jacek Anaszewski @ 2019-07-22 20:25 UTC (permalink / raw)
To: Pavel Machek, Ezequiel Garcia
Cc: linux-fbdev, mpartap, b.zolnierkie, tony, merlijn, kernel list,
dri-devel, sre, nekit1000, linux-leds, linux-omap,
linux-arm-kernel
Hi Pavel,
On 7/22/19 9:50 AM, Pavel Machek wrote:
> Hi!
>
>>> Configuring backlight trigger from dts results in backlight off during
>>> boot. Machine looks dead upon boot, which is not good.
>>>
>>> Fix that by enabling LED on trigger activation.
>
>>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
>>> @@ -114,6 +114,8 @@ static int bl_trig_activate(struct led_classdev *led)
>>> n->old_status = UNBLANK;
>>> n->notifier.notifier_call = fb_notifier_callback;
>>>
>>> + led_set_brightness(led, LED_ON);
>>> +
>>
>> This looks fishy.
>>
>> Maybe you should use a default-state = "keep" instead? (and you'll have
>> to support it in the LED driver).
>>
>> That'll give you proper "don't touch the LED if it was turned on" behavior,
>> which is what you seem to want.
>
> Actually no, that's not what I want. LED should go on if the display
> is active, as soon as trigger is activated.
>
> Unfortunately, I have see no good way to tell if the display is
> active (and display is usually active when trigger is activated).
default-state DT property can be also set to "on"
(see Documentation/devicetree/bindings/leds/common.txt).
You could make use of LED_INIT_DEFAULT_TRIGGER flag and
parse DT property in the activate op. Similar approach has been
applied e.g. in ledtrig-pattern.c.
--
Best regards,
Jacek Anaszewski
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
@ 2019-07-22 20:25 ` Jacek Anaszewski
0 siblings, 0 replies; 23+ messages in thread
From: Jacek Anaszewski @ 2019-07-22 20:25 UTC (permalink / raw)
To: Pavel Machek, Ezequiel Garcia
Cc: kernel list, linux-arm-kernel, linux-omap, tony, sre, nekit1000,
mpartap, merlijn, linux-leds, b.zolnierkie, dri-devel,
linux-fbdev
Hi Pavel,
On 7/22/19 9:50 AM, Pavel Machek wrote:
> Hi!
>
>>> Configuring backlight trigger from dts results in backlight off during
>>> boot. Machine looks dead upon boot, which is not good.
>>>
>>> Fix that by enabling LED on trigger activation.
>
>>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
>>> @@ -114,6 +114,8 @@ static int bl_trig_activate(struct led_classdev *led)
>>> n->old_status = UNBLANK;
>>> n->notifier.notifier_call = fb_notifier_callback;
>>>
>>> + led_set_brightness(led, LED_ON);
>>> +
>>
>> This looks fishy.
>>
>> Maybe you should use a default-state = "keep" instead? (and you'll have
>> to support it in the LED driver).
>>
>> That'll give you proper "don't touch the LED if it was turned on" behavior,
>> which is what you seem to want.
>
> Actually no, that's not what I want. LED should go on if the display
> is active, as soon as trigger is activated.
>
> Unfortunately, I have see no good way to tell if the display is
> active (and display is usually active when trigger is activated).
default-state DT property can be also set to "on"
(see Documentation/devicetree/bindings/leds/common.txt).
You could make use of LED_INIT_DEFAULT_TRIGGER flag and
parse DT property in the activate op. Similar approach has been
applied e.g. in ledtrig-pattern.c.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
2019-07-22 20:25 ` Jacek Anaszewski
(?)
(?)
@ 2019-07-22 21:04 ` Pavel Machek
-1 siblings, 0 replies; 23+ messages in thread
From: Pavel Machek @ 2019-07-22 21:04 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-fbdev, mpartap, b.zolnierkie, tony, merlijn, kernel list,
dri-devel, sre, nekit1000, linux-leds, linux-omap,
Ezequiel Garcia, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]
Hi!
> >> This looks fishy.
> >>
> >> Maybe you should use a default-state = "keep" instead? (and you'll have
> >> to support it in the LED driver).
> >>
> >> That'll give you proper "don't touch the LED if it was turned on" behavior,
> >> which is what you seem to want.
> >
> > Actually no, that's not what I want. LED should go on if the display
> > is active, as soon as trigger is activated.
> >
> > Unfortunately, I have see no good way to tell if the display is
> > active (and display is usually active when trigger is activated).
>
> default-state DT property can be also set to "on"
> (see Documentation/devicetree/bindings/leds/common.txt).
Ok, thanks for the hint, that could work. (I thought we were using
default trigger to set the LED "on").
Now...this gives me option of 0% or 100% brightness, while best would
be 10% brightness.... but I guess we can live with that ;-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
@ 2019-07-22 21:04 ` Pavel Machek
0 siblings, 0 replies; 23+ messages in thread
From: Pavel Machek @ 2019-07-22 21:04 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-fbdev, mpartap, b.zolnierkie, tony, merlijn, kernel list,
dri-devel, sre, nekit1000, linux-leds, linux-omap,
Ezequiel Garcia, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1074 bytes --]
Hi!
> >> This looks fishy.
> >>
> >> Maybe you should use a default-state = "keep" instead? (and you'll have
> >> to support it in the LED driver).
> >>
> >> That'll give you proper "don't touch the LED if it was turned on" behavior,
> >> which is what you seem to want.
> >
> > Actually no, that's not what I want. LED should go on if the display
> > is active, as soon as trigger is activated.
> >
> > Unfortunately, I have see no good way to tell if the display is
> > active (and display is usually active when trigger is activated).
>
> default-state DT property can be also set to "on"
> (see Documentation/devicetree/bindings/leds/common.txt).
Ok, thanks for the hint, that could work. (I thought we were using
default trigger to set the LED "on").
Now...this gives me option of 0% or 100% brightness, while best would
be 10% brightness.... but I guess we can live with that ;-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
@ 2019-07-22 21:04 ` Pavel Machek
0 siblings, 0 replies; 23+ messages in thread
From: Pavel Machek @ 2019-07-22 21:04 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-fbdev, mpartap, b.zolnierkie, tony, merlijn, kernel list,
dri-devel, sre, nekit1000, linux-leds, linux-omap,
Ezequiel Garcia, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1074 bytes --]
Hi!
> >> This looks fishy.
> >>
> >> Maybe you should use a default-state = "keep" instead? (and you'll have
> >> to support it in the LED driver).
> >>
> >> That'll give you proper "don't touch the LED if it was turned on" behavior,
> >> which is what you seem to want.
> >
> > Actually no, that's not what I want. LED should go on if the display
> > is active, as soon as trigger is activated.
> >
> > Unfortunately, I have see no good way to tell if the display is
> > active (and display is usually active when trigger is activated).
>
> default-state DT property can be also set to "on"
> (see Documentation/devicetree/bindings/leds/common.txt).
Ok, thanks for the hint, that could work. (I thought we were using
default trigger to set the LED "on").
Now...this gives me option of 0% or 100% brightness, while best would
be 10% brightness.... but I guess we can live with that ;-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
@ 2019-07-22 21:04 ` Pavel Machek
0 siblings, 0 replies; 23+ messages in thread
From: Pavel Machek @ 2019-07-22 21:04 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Ezequiel Garcia, kernel list, linux-arm-kernel, linux-omap, tony,
sre, nekit1000, mpartap, merlijn, linux-leds, b.zolnierkie,
dri-devel, linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]
Hi!
> >> This looks fishy.
> >>
> >> Maybe you should use a default-state = "keep" instead? (and you'll have
> >> to support it in the LED driver).
> >>
> >> That'll give you proper "don't touch the LED if it was turned on" behavior,
> >> which is what you seem to want.
> >
> > Actually no, that's not what I want. LED should go on if the display
> > is active, as soon as trigger is activated.
> >
> > Unfortunately, I have see no good way to tell if the display is
> > active (and display is usually active when trigger is activated).
>
> default-state DT property can be also set to "on"
> (see Documentation/devicetree/bindings/leds/common.txt).
Ok, thanks for the hint, that could work. (I thought we were using
default trigger to set the LED "on").
Now...this gives me option of 0% or 100% brightness, while best would
be 10% brightness.... but I guess we can live with that ;-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
2019-07-22 20:25 ` Jacek Anaszewski
(?)
@ 2019-07-24 8:33 ` Pavel Machek
-1 siblings, 0 replies; 23+ messages in thread
From: Pavel Machek @ 2019-07-24 8:33 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Ezequiel Garcia, kernel list, linux-arm-kernel, linux-omap, tony,
sre, nekit1000, mpartap, merlijn, linux-leds, b.zolnierkie,
dri-devel, linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 1332 bytes --]
Hi!
> >>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
> >>> @@ -114,6 +114,8 @@ static int bl_trig_activate(struct led_classdev *led)
> >>> n->old_status = UNBLANK;
> >>> n->notifier.notifier_call = fb_notifier_callback;
> >>>
> >>> + led_set_brightness(led, LED_ON);
> >>> +
> >>
> >> This looks fishy.
> >>
> >> Maybe you should use a default-state = "keep" instead? (and you'll have
> >> to support it in the LED driver).
> >>
> >> That'll give you proper "don't touch the LED if it was turned on" behavior,
> >> which is what you seem to want.
> >
> > Actually no, that's not what I want. LED should go on if the display
> > is active, as soon as trigger is activated.
> >
> > Unfortunately, I have see no good way to tell if the display is
> > active (and display is usually active when trigger is activated).
>
> default-state DT property can be also set to "on"
> (see Documentation/devicetree/bindings/leds/common.txt).
Yes, except that it does not work with all drivers :-(. In particular,
it does not work with lm3532.
We should really move more of the device tree parsing into core, so
that there's one place to fix...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
@ 2019-07-24 8:33 ` Pavel Machek
0 siblings, 0 replies; 23+ messages in thread
From: Pavel Machek @ 2019-07-24 8:33 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-fbdev, mpartap, b.zolnierkie, tony, merlijn, kernel list,
dri-devel, sre, nekit1000, linux-leds, linux-omap,
Ezequiel Garcia, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1332 bytes --]
Hi!
> >>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
> >>> @@ -114,6 +114,8 @@ static int bl_trig_activate(struct led_classdev *led)
> >>> n->old_status = UNBLANK;
> >>> n->notifier.notifier_call = fb_notifier_callback;
> >>>
> >>> + led_set_brightness(led, LED_ON);
> >>> +
> >>
> >> This looks fishy.
> >>
> >> Maybe you should use a default-state = "keep" instead? (and you'll have
> >> to support it in the LED driver).
> >>
> >> That'll give you proper "don't touch the LED if it was turned on" behavior,
> >> which is what you seem to want.
> >
> > Actually no, that's not what I want. LED should go on if the display
> > is active, as soon as trigger is activated.
> >
> > Unfortunately, I have see no good way to tell if the display is
> > active (and display is usually active when trigger is activated).
>
> default-state DT property can be also set to "on"
> (see Documentation/devicetree/bindings/leds/common.txt).
Yes, except that it does not work with all drivers :-(. In particular,
it does not work with lm3532.
We should really move more of the device tree parsing into core, so
that there's one place to fix...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
@ 2019-07-24 8:33 ` Pavel Machek
0 siblings, 0 replies; 23+ messages in thread
From: Pavel Machek @ 2019-07-24 8:33 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Ezequiel Garcia, kernel list, linux-arm-kernel, linux-omap, tony,
sre, nekit1000, mpartap, merlijn, linux-leds, b.zolnierkie,
dri-devel, linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 1332 bytes --]
Hi!
> >>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
> >>> @@ -114,6 +114,8 @@ static int bl_trig_activate(struct led_classdev *led)
> >>> n->old_status = UNBLANK;
> >>> n->notifier.notifier_call = fb_notifier_callback;
> >>>
> >>> + led_set_brightness(led, LED_ON);
> >>> +
> >>
> >> This looks fishy.
> >>
> >> Maybe you should use a default-state = "keep" instead? (and you'll have
> >> to support it in the LED driver).
> >>
> >> That'll give you proper "don't touch the LED if it was turned on" behavior,
> >> which is what you seem to want.
> >
> > Actually no, that's not what I want. LED should go on if the display
> > is active, as soon as trigger is activated.
> >
> > Unfortunately, I have see no good way to tell if the display is
> > active (and display is usually active when trigger is activated).
>
> default-state DT property can be also set to "on"
> (see Documentation/devicetree/bindings/leds/common.txt).
Yes, except that it does not work with all drivers :-(. In particular,
it does not work with lm3532.
We should really move more of the device tree parsing into core, so
that there's one place to fix...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
2019-07-24 8:33 ` Pavel Machek
(?)
@ 2019-07-24 21:16 ` Jacek Anaszewski
-1 siblings, 0 replies; 23+ messages in thread
From: Jacek Anaszewski @ 2019-07-24 21:16 UTC (permalink / raw)
To: Pavel Machek
Cc: Ezequiel Garcia, kernel list, linux-arm-kernel, linux-omap, tony,
sre, nekit1000, mpartap, merlijn, linux-leds, b.zolnierkie,
dri-devel, linux-fbdev
On 7/24/19 10:33 AM, Pavel Machek wrote:
> Hi!
>
>>>>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
>>>>> @@ -114,6 +114,8 @@ static int bl_trig_activate(struct led_classdev *led)
>>>>> n->old_status = UNBLANK;
>>>>> n->notifier.notifier_call = fb_notifier_callback;
>>>>>
>>>>> + led_set_brightness(led, LED_ON);
>>>>> +
>>>>
>>>> This looks fishy.
>>>>
>>>> Maybe you should use a default-state = "keep" instead? (and you'll have
>>>> to support it in the LED driver).
>>>>
>>>> That'll give you proper "don't touch the LED if it was turned on" behavior,
>>>> which is what you seem to want.
>>>
>>> Actually no, that's not what I want. LED should go on if the display
>>> is active, as soon as trigger is activated.
>>>
>>> Unfortunately, I have see no good way to tell if the display is
>>> active (and display is usually active when trigger is activated).
>>
>> default-state DT property can be also set to "on"
>> (see Documentation/devicetree/bindings/leds/common.txt).
>
> Yes, except that it does not work with all drivers :-(. In particular,
> it does not work with lm3532.
>
> We should really move more of the device tree parsing into core, so
> that there's one place to fix...
Right. We could have something similar to led_get_default_pattern().
led_get_default_state() ?
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
@ 2019-07-24 21:16 ` Jacek Anaszewski
0 siblings, 0 replies; 23+ messages in thread
From: Jacek Anaszewski @ 2019-07-24 21:16 UTC (permalink / raw)
To: Pavel Machek
Cc: linux-fbdev, mpartap, b.zolnierkie, tony, merlijn, kernel list,
dri-devel, sre, nekit1000, linux-leds, linux-omap,
Ezequiel Garcia, linux-arm-kernel
On 7/24/19 10:33 AM, Pavel Machek wrote:
> Hi!
>
>>>>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
>>>>> @@ -114,6 +114,8 @@ static int bl_trig_activate(struct led_classdev *led)
>>>>> n->old_status = UNBLANK;
>>>>> n->notifier.notifier_call = fb_notifier_callback;
>>>>>
>>>>> + led_set_brightness(led, LED_ON);
>>>>> +
>>>>
>>>> This looks fishy.
>>>>
>>>> Maybe you should use a default-state = "keep" instead? (and you'll have
>>>> to support it in the LED driver).
>>>>
>>>> That'll give you proper "don't touch the LED if it was turned on" behavior,
>>>> which is what you seem to want.
>>>
>>> Actually no, that's not what I want. LED should go on if the display
>>> is active, as soon as trigger is activated.
>>>
>>> Unfortunately, I have see no good way to tell if the display is
>>> active (and display is usually active when trigger is activated).
>>
>> default-state DT property can be also set to "on"
>> (see Documentation/devicetree/bindings/leds/common.txt).
>
> Yes, except that it does not work with all drivers :-(. In particular,
> it does not work with lm3532.
>
> We should really move more of the device tree parsing into core, so
> that there's one place to fix...
Right. We could have something similar to led_get_default_pattern().
led_get_default_state() ?
--
Best regards,
Jacek Anaszewski
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Enable backlight when trigger is activated
@ 2019-07-24 21:16 ` Jacek Anaszewski
0 siblings, 0 replies; 23+ messages in thread
From: Jacek Anaszewski @ 2019-07-24 21:16 UTC (permalink / raw)
To: Pavel Machek
Cc: Ezequiel Garcia, kernel list, linux-arm-kernel, linux-omap, tony,
sre, nekit1000, mpartap, merlijn, linux-leds, b.zolnierkie,
dri-devel, linux-fbdev
On 7/24/19 10:33 AM, Pavel Machek wrote:
> Hi!
>
>>>>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
>>>>> @@ -114,6 +114,8 @@ static int bl_trig_activate(struct led_classdev *led)
>>>>> n->old_status = UNBLANK;
>>>>> n->notifier.notifier_call = fb_notifier_callback;
>>>>>
>>>>> + led_set_brightness(led, LED_ON);
>>>>> +
>>>>
>>>> This looks fishy.
>>>>
>>>> Maybe you should use a default-state = "keep" instead? (and you'll have
>>>> to support it in the LED driver).
>>>>
>>>> That'll give you proper "don't touch the LED if it was turned on" behavior,
>>>> which is what you seem to want.
>>>
>>> Actually no, that's not what I want. LED should go on if the display
>>> is active, as soon as trigger is activated.
>>>
>>> Unfortunately, I have see no good way to tell if the display is
>>> active (and display is usually active when trigger is activated).
>>
>> default-state DT property can be also set to "on"
>> (see Documentation/devicetree/bindings/leds/common.txt).
>
> Yes, except that it does not work with all drivers :-(. In particular,
> it does not work with lm3532.
>
> We should really move more of the device tree parsing into core, so
> that there's one place to fix...
Right. We could have something similar to led_get_default_pattern().
led_get_default_state() ?
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 23+ messages in thread