Re: [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/24/2016 03:26 PM, Pali Rohár wrote:
> On Thursday 24 November 2016 15:21:36 Jacek Anaszewski wrote:
>> On 11/24/2016 10:15 AM, Pali Rohár wrote:
>>> On Wednesday 23 November 2016 12:01:02 Jacek Anaszewski wrote:
>>>> I would also appreciate your opinion on the other solution to the
>>>> problem of notifying brightness changes originating from hardware,
>>>> i.e. hw_brightness_change{_ro} file, that would support POLLPRI events,
>>>> and reading brightness.
>>>
>>> Another idea:
>>>
>>> If no trigger is active then led subsystem will invoke POLLPRI on
>>> "brightness" sysfs file.
>>>
>>> And if there is active trigger then only trigger code could invoke
>>> POLLPRI on "brightness" file.
>>>
>>> This could solve problem with high CPU load and power usage when e.g.
>>> cpu trigger is active (and cpu trigger will not implement any POLLPRI).
>>>
>>> Do not know if this is really enough for your situation, it is just and
>>> another idea.
>>
>> This way we would be losing POLLPRI events when trigger is active,
>> whereas it would be useful to have ones in some use cases.
>
> In case it makes sense, trigger can implement that POLLPRI event. E.g.
> for CPU trigger it probably does not make sense (or at least send
> POLLPRI event lot of times per second).

I'd like to have uniform semantics of this across all triggers.
The more exceptions we make the more chances we will miss something.
We would have to modify all triggers in drivers/led/triggers, but
there are also in-driver triggers scattered outside LED subsystem
which would also need the update. Let's not do any risky
modifications affecting LED Trigger core clients.

Since it has been reported that POLLPRI notifications on brightness
file can lead to increased power consumption, and having my above
statement I don't think that it is a good idea to use brightness
file for this.

So, let's focus on the hw_brightness_change_ro I've already mentioned.
I added "ro" postfix to make it clear that it is read only.

Four words in the sysfs file name make it somehow noisy,
so maybe someone will be able to come up with a better name.

>
>>> But first please update documentation in ABI/testing to match current
>>> situation. That is really needed.
>>>
>>
>> I suppose that you're thinking about behaviour on brightness file
>> reading? Is there anything else you'd like to have clarified in the doc?
>
> Yes, how triggers interact with brightness file, what happen when you
> write 0 on active trigger,

There is already a patch in linux-next adding the following:


+		Writing 0 to this file clears active trigger.
+
+		Writing non-zero to this file while trigger is active changes the
+		top brightness trigger is going to use.
+

> what happen when you read brightness file
> with active trigger / without trigger.
>

Yes, this needs to be covered too.

-- 
Best regards,
Jacek Anaszewski


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel




[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux