Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1519914

Re: PM regression with LED changes in next-20161109

From Jacek Anaszewski <jacek.anaszewski@gmail.com>
Newsgroups linux.kernel
Subject Re: PM regression with LED changes in next-20161109
Date 2016-11-11 18:10 +0100
Message-ID <sCnuW-77s-17@gated-at.bofh.it> (permalink)
References (5 earlier) <sC0oF-as-21@gated-at.bofh.it> <sC1Xw-1gl-25@gated-at.bofh.it> <sC48V-2ML-3@gated-at.bofh.it> <sC5eF-3qt-1@gated-at.bofh.it> <sCiOC-48t-33@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 11/11/2016 01:01 PM, Pavel Machek wrote:
> On Thu 2016-11-10 22:34:07, Jacek Anaszewski wrote:
>> Hi,
>>
>> On 11/10/2016 09:29 PM, Pavel Machek wrote:
>>> On Thu 2016-11-10 10:55:37, Tony Lindgren wrote:
>>>> * Pavel Machek <pavel@ucw.cz> [161110 09:29]:
>>>>> Hi!
>>>>>
>>>>>>>>> Looks like commit 883d32ce3385 ("leds: core: Add support for poll()ing
>>>>>>>>> the sysfs brightness attr for changes.") breaks runtime PM for me.
>>>>>>>>>
>>>>>>>>> On my omap dm3730 based test system, idle power consumption is over 70
>>>>>>>>> times higher now with this patch! It goes from about 6mW for the core
>>>>>>>>> system to over 440mW during idle meaning there's some busy timer now
>>>>>>>>> active.
>>>>>>>>>
>>>>>>>>> Reverting this patch fixes the issue. Any ideas?
>>>>>
>>>>> Are you using any LED that toggles with high frequency? Like perhaps
>>>>> LED that is lit when CPU is active?
>>>>
>>>> Yeah one of them seems to have cpu0 as the default trigger.
>>>
>>> Aha. Its quite obvious we don't want to notify sysfs each time that
>>> one is toggled, right?
>>>
>>> IMO brightness should display max brightness for the trigger, as Hans
>>> suggested, anything else is madness for trigger such as cpu activity.
>>
>> Are you suggesting that we should revert changes introduced
>> by below patch?
>>
>> commit 29d76dfa29fe22583aefddccda0bc56aa81035dc
>> Author: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
>> Date:   Tue Mar 18 09:47:48 2008 +0000
>>
>>     leds: Add support to leds with readable status
>>
>>     Some led hardware allows drivers to query the led state, and this patch
>>     adds a hook to let the led class take advantage of that information when
>>     available.
>>
>>     Without this functionality, when access to the led hardware is not
>>     exclusive (i.e. firmware or hardware might change its state behind the
>>     kernel's back), reality goes out of sync with the led class' idea of
>> what
>>     the led is doing, which is annoying at best.
>
> Hmm. So userland can read the LED state, and it can get _some_ value
> back, but it can not know if it is current state or not.
>
> I don't think that's a good interface. I see it is from 2008... is
> someone using it? Maybe it is too late for revert.

I can imagine it being used in flash LED use case. E.g. one
could use oneshot trigger to trigger flash strobe, and then
he could periodically read brightness file to check, for whatever
reason, whether the flash is strobing.

> But I'd certainly not extend it with poll.

We could add a dedicated file e.g. hw_brightness_change for that
(maybe someone will have a better candidate for the file name).

This way it would be semantically consistent to report only
hardware originating brightness changes on it, which was the
initial reason for adding the brightness change notification
feature.

Moreover, LED class drivers could report on this file the
brightness level which was set by the firmware, which wouldn't
affect current LED class device brightness setting, unless
brightness file is read (and brightness_get op is supported).

> IMO reading/polling should only be available with some triggers. It
> does not make sense with "CPU load" trigger. It makes sense with
> "keyboard light changeable by hardware" trigger.


-- 
Best regards,
Jacek Anaszewski

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

PM regression with LED changes in next-20161109 Tony Lindgren <tony@atomide.com> - 2016-11-09 20:30 +0100
  Re: PM regression with LED changes in next-20161109 Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-11-09 21:50 +0100
    Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-10 09:50 +0100
      Re: PM regression with LED changes in next-20161109 Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-11-10 14:00 +0100
        Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-10 14:10 +0100
          Re: PM regression with LED changes in next-20161109 Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-11-10 15:00 +0100
            Re: PM regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-10 17:40 +0100
        Re: PM regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-10 17:30 +0100
          Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-10 17:50 +0100
            Re: PM regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-10 21:50 +0100
              Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-11 09:30 +0100
          Re: PM regression with LED changes in next-20161109 Tony Lindgren <tony@atomide.com> - 2016-11-10 19:10 +0100
            Re: PM regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-10 21:30 +0100
              Re: PM regression with LED changes in next-20161109 Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-11-10 22:40 +0100
                Re: PM regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-11 13:10 +0100
                Re: PM regression with LED changes in next-20161109 Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-11-11 18:10 +0100
                Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-11 20:30 +0100
                Re: PM regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-11 23:20 +0100
                Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-12 09:10 +0100
                Three different LED brightnesses (was Re: PM regression with LED  changes in next-20161109) Pavel Machek <pavel@ucw.cz> - 2016-11-13 10:20 +0100
                Re: Three different LED brightnesses (was Re: PM regression with LED  changes in next-20161109) Hans de Goede <hdegoede@redhat.com> - 2016-11-13 10:50 +0100
                Re: Three different LED brightnesses (was Re: PM regression with LED  changes in next-20161109) Pavel Machek <pavel@ucw.cz> - 2016-11-13 21:50 +0100
                Re: PM regression with LED changes in next-20161109 Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-11-12 11:30 +0100
                Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-12 11:50 +0100
                Re: PM regression with LED changes in next-20161109 Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-11-12 20:20 +0100
                Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-12 22:20 +0100
                Re: PM regression with LED changes in next-20161109 Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-11-13 12:50 +0100
                Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-13 15:00 +0100
                Re: PM regression with LED changes in next-20161109 Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-11-14 10:20 +0100
                Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-14 14:00 +0100
                Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-15 11:10 +0100
                Re: PM regression with LED changes in next-20161109 Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-11-15 11:10 +0100
                LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-15 11:40 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-11-15 12:00 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-15 12:20 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-15 12:20 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-15 12:30 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-15 12:50 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-15 13:10 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-15 13:20 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-11-15 14:30 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-15 14:50 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-11-15 15:10 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-15 15:40 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-11-15 15:50 +0100
                Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM  regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-17 23:20 +0100
                Re: PM regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-14 09:40 +0100
                Re: PM regression with LED changes in next-20161109 Pavel Machek <pavel@ucw.cz> - 2016-11-11 23:10 +0100
  Re: PM regression with LED changes in next-20161109 Hans de Goede <hdegoede@redhat.com> - 2016-11-10 09:40 +0100
    Re: PM regression with LED changes in next-20161109 Tony Lindgren <tony@atomide.com> - 2016-11-10 16:20 +0100

csiph-web