Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936181AbcKOOEs (ORCPT ); Tue, 15 Nov 2016 09:04:48 -0500 Received: from mailout1.w1.samsung.com ([210.118.77.11]:29753 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932297AbcKOOEo (ORCPT ); Tue, 15 Nov 2016 09:04:44 -0500 X-AuditID: cbfec7f4-f791c6d000006eac-38-582b15f80d5f Subject: Re: LEDs that change brightness "itself" -- that's a trigger. Re: PM regression with LED changes in next-20161109 To: Hans de Goede , Pavel Machek Cc: Jacek Anaszewski , Tony Lindgren , linux-leds@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Darren Hart From: Jacek Anaszewski Message-id: <3bb538bb-3c80-70c7-2c44-46b174c90503@samsung.com> Date: Tue, 15 Nov 2016 15:04:38 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-version: 1.0 In-reply-to: <9d42546d-5917-891c-2b18-ccf7f7328624@redhat.com> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrAKsWRmVeSWpSXmKPExsWy7djPc7o/RLUjDO4c07HoWmhg8eb4dCaL 21s3sFhsenyN1eLyrjlsFlvfrGO0mL2kn8Xi7qmjbBb7r3g5cHp8+zqJxWPnrLvsHptXaHls XlLv8X7fVTaPFau/s3t83iQXwB7FZZOSmpNZllqkb5fAldE0YQlzwSLFilnTX7A2MH6T6mLk 5JAQMJHYvvk/C4QtJnHh3nq2LkYuDiGBpYwSOxs2M0M4nxkl1n88wgrT0fvrBFiHkMAyRoll 7xghip4xSvzrv84GkhAWqJG4tLkZqJuDQ0TARaK92RukhlngJ6NE7/ousBo2AUOJny9eM4HU 8ArYSVw8WgoSZhFQlfj0cgMLSFhUIEJi991UkDCvgKDEj8n3wNZyAlWfbd7LDGIzCzhKPFi0 kxXClpfYvOYt2M0SAofYJf50tYPNkRCQldh0gBnifBeJL08vQj0sLPHq+BZ2CFtG4vLkbhaI 3smMEheP3WSFcFYzSmzs7ITqsJZo+P+LBWIbn8SkbdOZIRbwSnS0CUGUeEjc2nMGaqijROuG u9DwOcQq0Xv7GssERvlZSB6aheSJWUieWMDIvIpRJLW0ODc9tdhErzgxt7g0L10vOT93EyMw 3Zz+d/zLDsbFx6wOMQpwMCrx8O44qhkhxJpYVlyZe4hRgoNZSYQ3TFg7Qog3JbGyKrUoP76o NCe1+BCjNAeLkjjvngVXwoUE0hNLUrNTUwtSi2CyTBycUg2MJjym2QrztR8EKecbnDjHbGgv VblsjWfpxzqDlS5i5ap/zR6ot3Z4/l27fdumjuiH31cyf3/xnC3u+Io2iVsc99bnOMcu2pA2 oYOnnN05Vm7jpWPTPfLv1bXtfRG+1WVuQVP25SAx3fl7Zpns8m1Z6Ssl1ld3ON/jyB4+FYH3 16f8nFvMGimgxFKckWioxVxUnAgAEQrBQDMDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrIIsWRmVeSWpSXmKPExsVy+t/xy7p3RbUjDL708lp0LTSweHN8OpPF 7a0bWCw2Pb7GanF51xw2i61v1jFazF7Sz2Jx99RRNov9V7wcOD2+fZ3E4rFz1l12j80rtDw2 L6n3eL/vKpvHitXf2T0+b5ILYI9ys8lITUxJLVJIzUvOT8nMS7dVCg1x07VQUshLzE21VYrQ 9Q0JUlIoS8wpBfKMDNCAg3OAe7CSvl2CW0bThCXMBYsUK2ZNf8HawPhNqouRk0NCwESi99cJ FghbTOLCvfVsXYxcHEICSxglDnX9YIdwnjFK/P5whhWkSligRuL09C+MXYwcHCICLhLtzd4Q NftYJXbtWswK4jAL/GSU+PNqC9hYNgFDiZ8vXjOBNPAK2ElcPFoKEmYRUJX49HIDWImoQITE rVUfGUFsXgFBiR+T74HFOYHKzzbvZQaxmQVsJRa8X8cCYctLbF7zlnkCo8AsJC2zkJTNQlK2 gJF5FaNIamlxbnpusZFecWJucWleul5yfu4mRmD0bTv2c8sOxq53wYcYBTgYlXh4dxzVjBBi TSwrrsw9xCjBwawkwhsmrB0hxJuSWFmVWpQfX1Sak1p8iNEU6ImJzFKiyfnAxJBXEm9oYmhu aWhkbGFhbmSkJM479cOVcCGB9MSS1OzU1ILUIpg+Jg5OqQbGzJ1bpz8TqJwf6fA/8MvlFP1U pcXZe7cwNP8tKFHxVAhfknFVZG1GOpd/2IrVGlnnNt30mHR+ihCjoVXI7x9Xy0NaTm97sePg 9NnVKx1X8lVU2Z/7YdVvKlDXET+PT4d70Y+vk66HrdZxE2FbM8/h4B7umUwOG4s+Z3Ct3ryT c4ZN+3pHLrnfSizFGYmGWsxFxYkAI28vE9QCAAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20161115140440eucas1p25f33729e8ed11860c5fdc81fec68731b X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 X-Local-Sender: =?UTF-8?B?SmFjZWsgQW5hc3pld3NraRtTUlBPTC1TeXN0ZW0gRlcgIChN?= =?UTF-8?B?Qikb7IK87ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?SmFjZWsgQW5hc3pld3NraRtTUlBPTC1TeXN0ZW0gRlcgIChN?= =?UTF-8?B?QikbU2Ftc3VuZyBFbGVjdHJvbmljcxtTZW5pb3IgU29mdHdhcmUgRW5naW5l?= =?UTF-8?B?ZXI=?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjc1MjY=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20161115103141epcas2p251f2521354a8cd7b9dfcfe55bfa482f0 X-RootMTR: 20161115103141epcas2p251f2521354a8cd7b9dfcfe55bfa482f0 References: <9b476f85-d45e-deb6-335d-fc56f6d90350@redhat.com> <127cdd42-6fd8-c671-60b7-3826b351577f@samsung.com> <15cafbf5-d842-e184-2fd4-65f8272f505a@redhat.com> <20161115103133.GA22860@amd> <4e392d5d-eb10-f285-517e-976a55c3e318@samsung.com> <20161115111154.GA5482@amd> <128aae59-b790-42f1-7d66-81391c9330c3@redhat.com> <20161115114859.GA7018@amd> <9e7d5e0f-c4ca-6930-63b9-83dc28517f33@samsung.com> <9d42546d-5917-891c-2b18-ccf7f7328624@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4153 Lines: 105 On 11/15/2016 02:48 PM, Hans de Goede wrote: > Hi, > > On 15-11-16 14:28, Jacek Anaszewski wrote: >> On 11/15/2016 01:06 PM, Hans de Goede wrote: >>> 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. >> >> Word "current" is not precise here. >> >> It can be thought of as either last brightness set by the >> user or the brightness currently written to the device >> (returned by brightness file). >> >> There is a semantic discrepancy in our requirements - >> we want the file representing both permanent brightness >> set by the user and brightness set by the hardware. >> >> The two stand in contradiction to each other since >> brightness set by the user can be adjusted by the hardware. >> >> Reading the file shouldn't update brightness property of >> struct led_classdev, so it shouldn't call led_update_brightness() >> but it still should allow reading brightness set by the >> hardware, as a result of each POLLPRI event. So in fact in >> the same time it should report both according to our requirements >> which is impossible. Do we need three brightness files ? > > I don't think so, current_brightness actually is an accurate > name, if the brightness was last changed by writing from > sysfs, the keyboard backlight will honor that and the current_brightness > attribute will show the brightness last set through writing it, > which matches the actual current brightness of the keyboard backlight. > > Likewise if it was changed with the hotkey last then the keyboard > backlight brightness will be changed and reading from current_brightness > will return the new actual brightness. Basically reading from this > file will be no different then reading from the normal "brightness" > file the difference will be in that it is poll-able and that > writing 0 turns off the LED without stopping blinking. If so then when software blinking enabled, it will return 0 on low blink cycle no matter what current brightness level is. -- Best regards, Jacek Anaszewski