From: Hans de Goede <hdegoede@redhat.com> To: Pavel Machek <pavel@ucw.cz> Cc: Jacek Anaszewski <j.anaszewski@samsung.com>, Jacek Anaszewski <jacek.anaszewski@gmail.com>, Tony Lindgren <tony@atomide.com>, linux-leds@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Darren Hart <dvhart@infradead.org>, Hans de Goede <hdegoede@redhat.com> Subject: Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM regression with LED changes in next-20161109 Date: Tue, 15 Nov 2016 13:06:14 +0100 [thread overview] Message-ID: <aeccbb02-6e33-390c-d72d-2a580dd67ed8@redhat.com> (raw) In-Reply-To: <20161115114859.GA7018@amd> Hi, On 15-11-16 12:48, Pavel Machek wrote: > Hi! > >>>>> The LED you are talking about _has_ a trigger, implemented in >>>>> hardware. That trigger can change LED brightness behind kernel's (and >>>>> userspace's) back. Don't pretend the trigger does not exist, it does. >>>>> >>>>> And when you do that, you'll have nice place to report changes to >>>>> userspace -- trigger can now export that information, and offer poll() >>>>> interface. >>>> >>>> Well, that sounds interesting. It is logically justifiable. >>> >>> Thanks. >>> >>>> I initially proposed exactly this solution, with recently >>>> added userspace LED being a trigger listener. It seems a bit >>>> awkward though. How would you listen to the trigger events? >>> >>> Trigger exposes a file in sysfs, with poll() working on that file >> >> Hmm, a new file would give the advantage of making it easy for >> userspace to see if the trigger is poll-able, this is likely >> better then my own proposal I just send. > > Good. > >>> (and >>> probably read exposing the current brightness). >> >> If we do this, can we please make it mirror brightness, iow >> also make it writable, that will make it easier for userspace >> to deal with it. We can simply re-use the existing show / store >> methods for brightness for this. > > Actually, echo 0 > brightness disables the trigger, IIRC. I'd avoid > that here, you want to be able to turn off the backlight but still > keep the trigger (and be notified of future changes). True, that is easy to do the store method will just need to call led_set_brightness_nosleep instead of led_set_brightness, this will skip the checks to stop blinking in led_set_brightness and otherwise is equivalent. >> I suggest we call it: >> >> trigger_brightness >> >> And only register it when a poll-able trigger is present. > > I'd call it 'current_brightness', but that's no big deal. Yes, only > registering it for poll-able triggers makes sense. current_brightness works for me. I will take a shot a patch-set implementing this. Regards, Hans > >>> Key difference is that only triggers where this makes sense (keyboard >>> backlight) expose it and carry the overhead. CPU trigger would >>> definitely not do this. >> >> Ack only having some triggers pollable is important. > > Thanks, > Pavel >
WARNING: multiple messages have this Message-ID (diff)
From: hdegoede@redhat.com (Hans de Goede) To: linux-arm-kernel@lists.infradead.org Subject: LEDs that change brightness "itself" -- that's a trigger. Re: PM regression with LED changes in next-20161109 Date: Tue, 15 Nov 2016 13:06:14 +0100 [thread overview] Message-ID: <aeccbb02-6e33-390c-d72d-2a580dd67ed8@redhat.com> (raw) In-Reply-To: <20161115114859.GA7018@amd> Hi, On 15-11-16 12:48, Pavel Machek wrote: > Hi! > >>>>> The LED you are talking about _has_ a trigger, implemented in >>>>> hardware. That trigger can change LED brightness behind kernel's (and >>>>> userspace's) back. Don't pretend the trigger does not exist, it does. >>>>> >>>>> And when you do that, you'll have nice place to report changes to >>>>> userspace -- trigger can now export that information, and offer poll() >>>>> interface. >>>> >>>> Well, that sounds interesting. It is logically justifiable. >>> >>> Thanks. >>> >>>> I initially proposed exactly this solution, with recently >>>> added userspace LED being a trigger listener. It seems a bit >>>> awkward though. How would you listen to the trigger events? >>> >>> Trigger exposes a file in sysfs, with poll() working on that file >> >> Hmm, a new file would give the advantage of making it easy for >> userspace to see if the trigger is poll-able, this is likely >> better then my own proposal I just send. > > Good. > >>> (and >>> probably read exposing the current brightness). >> >> If we do this, can we please make it mirror brightness, iow >> also make it writable, that will make it easier for userspace >> to deal with it. We can simply re-use the existing show / store >> methods for brightness for this. > > Actually, echo 0 > brightness disables the trigger, IIRC. I'd avoid > that here, you want to be able to turn off the backlight but still > keep the trigger (and be notified of future changes). True, that is easy to do the store method will just need to call led_set_brightness_nosleep instead of led_set_brightness, this will skip the checks to stop blinking in led_set_brightness and otherwise is equivalent. >> I suggest we call it: >> >> trigger_brightness >> >> And only register it when a poll-able trigger is present. > > I'd call it 'current_brightness', but that's no big deal. Yes, only > registering it for poll-able triggers makes sense. current_brightness works for me. I will take a shot a patch-set implementing this. Regards, Hans > >>> Key difference is that only triggers where this makes sense (keyboard >>> backlight) expose it and carry the overhead. CPU trigger would >>> definitely not do this. >> >> Ack only having some triggers pollable is important. > > Thanks, > Pavel >
next prev parent reply other threads:[~2016-11-15 12:06 UTC|newest] Thread overview: 102+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-11-09 19:23 PM regression with LED changes in next-20161109 Tony Lindgren 2016-11-09 19:23 ` Tony Lindgren 2016-11-09 20:45 ` Jacek Anaszewski 2016-11-09 20:45 ` Jacek Anaszewski 2016-11-10 8:49 ` Hans de Goede 2016-11-10 8:49 ` Hans de Goede 2016-11-10 12:56 ` Jacek Anaszewski 2016-11-10 12:56 ` Jacek Anaszewski 2016-11-10 13:04 ` Hans de Goede 2016-11-10 13:04 ` Hans de Goede 2016-11-10 13:55 ` Jacek Anaszewski 2016-11-10 13:55 ` Jacek Anaszewski 2016-11-10 16:36 ` Pavel Machek 2016-11-10 16:36 ` Pavel Machek 2016-11-10 16:29 ` Pavel Machek 2016-11-10 16:29 ` Pavel Machek 2016-11-10 16:44 ` Hans de Goede 2016-11-10 16:44 ` Hans de Goede 2016-11-10 20:48 ` Pavel Machek 2016-11-10 20:48 ` Pavel Machek 2016-11-11 8:25 ` Hans de Goede 2016-11-11 8:25 ` Hans de Goede 2016-11-10 17:55 ` Tony Lindgren 2016-11-10 17:55 ` Tony Lindgren 2016-11-10 20:29 ` Pavel Machek 2016-11-10 20:29 ` Pavel Machek 2016-11-10 21:34 ` Jacek Anaszewski 2016-11-10 21:34 ` Jacek Anaszewski 2016-11-11 12:01 ` Pavel Machek 2016-11-11 12:01 ` Pavel Machek 2016-11-11 17:03 ` Jacek Anaszewski 2016-11-11 17:03 ` Jacek Anaszewski 2016-11-11 19:28 ` Hans de Goede 2016-11-11 19:28 ` Hans de Goede 2016-11-11 22:12 ` Pavel Machek 2016-11-11 22:12 ` Pavel Machek 2016-11-12 8:03 ` Hans de Goede 2016-11-12 8:03 ` Hans de Goede 2016-11-13 9:10 ` Three different LED brightnesses (was Re: PM regression with LED changes in next-20161109) Pavel Machek 2016-11-13 9:10 ` Pavel Machek 2016-11-13 9:44 ` Hans de Goede 2016-11-13 9:44 ` Hans de Goede 2016-11-13 20:45 ` Pavel Machek 2016-11-13 20:45 ` Pavel Machek 2016-11-12 10:24 ` PM regression with LED changes in next-20161109 Jacek Anaszewski 2016-11-12 10:24 ` Jacek Anaszewski 2016-11-12 10:33 ` Hans de Goede 2016-11-12 10:33 ` Hans de Goede 2016-11-12 10:33 ` Hans de Goede 2016-11-12 19:14 ` Jacek Anaszewski 2016-11-12 19:14 ` Jacek Anaszewski 2016-11-12 21:14 ` Hans de Goede 2016-11-12 21:14 ` Hans de Goede 2016-11-13 11:44 ` Jacek Anaszewski 2016-11-13 11:44 ` Jacek Anaszewski 2016-11-13 13:52 ` Hans de Goede 2016-11-13 13:52 ` Hans de Goede 2016-11-14 9:12 ` Jacek Anaszewski 2016-11-14 9:12 ` Jacek Anaszewski 2016-11-14 9:12 ` Jacek Anaszewski 2016-11-14 12:51 ` Hans de Goede 2016-11-14 12:51 ` Hans de Goede 2016-11-15 10:01 ` Jacek Anaszewski 2016-11-15 10:01 ` Jacek Anaszewski 2016-11-15 10:09 ` Hans de Goede 2016-11-15 10:09 ` Hans de Goede 2016-11-15 10:31 ` LEDs that change brightness "itself" -- that's a trigger. " Pavel Machek 2016-11-15 10:31 ` Pavel Machek 2016-11-15 10:58 ` Jacek Anaszewski 2016-11-15 10:58 ` Jacek Anaszewski 2016-11-15 11:11 ` Pavel Machek 2016-11-15 11:11 ` Pavel Machek 2016-11-15 11:21 ` Hans de Goede 2016-11-15 11:21 ` Hans de Goede 2016-11-15 11:48 ` Pavel Machek 2016-11-15 11:48 ` Pavel Machek 2016-11-15 12:06 ` Hans de Goede [this message] 2016-11-15 12:06 ` Hans de Goede 2016-11-15 12:11 ` Pavel Machek 2016-11-15 12:11 ` Pavel Machek 2016-11-15 13:28 ` Jacek Anaszewski 2016-11-15 13:28 ` Jacek Anaszewski 2016-11-15 13:48 ` Hans de Goede 2016-11-15 13:48 ` Hans de Goede 2016-11-15 14:04 ` Jacek Anaszewski 2016-11-15 14:04 ` Jacek Anaszewski 2016-11-15 14:30 ` Hans de Goede 2016-11-15 14:30 ` Hans de Goede 2016-11-15 14:41 ` Jacek Anaszewski 2016-11-15 14:41 ` Jacek Anaszewski 2016-11-17 22:12 ` Hans de Goede 2016-11-17 22:12 ` Hans de Goede 2016-11-15 11:17 ` Hans de Goede 2016-11-15 11:17 ` Hans de Goede 2016-11-14 8:31 ` Pavel Machek 2016-11-14 8:31 ` Pavel Machek 2016-11-11 22:06 ` Pavel Machek 2016-11-11 22:06 ` Pavel Machek 2016-11-10 8:34 ` Hans de Goede 2016-11-10 8:34 ` Hans de Goede 2016-11-10 15:11 ` Tony Lindgren 2016-11-10 15:11 ` Tony Lindgren
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=aeccbb02-6e33-390c-d72d-2a580dd67ed8@redhat.com \ --to=hdegoede@redhat.com \ --cc=dvhart@infradead.org \ --cc=j.anaszewski@samsung.com \ --cc=jacek.anaszewski@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-leds@vger.kernel.org \ --cc=linux-omap@vger.kernel.org \ --cc=pavel@ucw.cz \ --cc=tony@atomide.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.