linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Pavel Machek <pavel@ucw.cz>
Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	Rob Herring <robh@kernel.org>, Richard Purdie <rpurdie@rpsys.net>,
	linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org,
	linux-arm-msm@vger.kernel.org,
	Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 1/2] leds: Add driver for Qualcomm LPG
Date: Mon, 10 Apr 2017 12:19:48 -0700	[thread overview]
Message-ID: <20170410191948.GE15143@minitux> (raw)
In-Reply-To: <20170408133904.GA9020@amd>

On Sat 08 Apr 06:39 PDT 2017, Pavel Machek wrote:

> Hi!
> 
> > [..]
> > > > For the patterns I don't know how a trigger for this would look like,
> > > > how would setting the pattern of a trigger be propagated down to the
> > > > hardware?
> > > 
> > > We'd need a new op and API similar to blink_set()/led_blink_set().
> > > 
> > 
> > I've tried to find different LED circuits with some sort of pattern
> > generator in an attempt to figure out how to design this interface, but
> > turned out to be quite hard to find examples; the three I can compare
> > are:
> > 
> > * LP5xx series "implements" pattern generation by executing code.
> > 
> > * Qualcomm LPG iterates over 2-64 brightness-values in a pattern, at a
> >   fixed rate with knobs to configure what happens before starting and
> >   after finishing iterating over the defined values. It does not support
> >   smooth transitions between values.
> > 
> > * AS3676 supports a pattern of 32 values controlling if the output
> >   should be enabled or disabled for each 32.5ms (or 250ms) time period.
> >   The delay before repeating the pattern can be configured. It support
> >   smooth transitions between the states.
> > 
> > 
> > So, while I think I see how you would like to architect this interface I
> > am not sure how to figure out the details.
> > 
> > The pattern definition would have to be expressive enough to support the
> > features of LP5xx and direct enough to support the limited AS3676. It
> > would likely have to express transitions, so that the LPG could generate
> > intermediate steps (and we will have to adapt the resolution of the
> > ramps based on the other LPGs in the system).
> > 
> > How do we do with patterns that are implementable by the LP5xx but are
> > not with the LPG? Should we reject those or should we do some sort of
> > best-effort approach in the kernel?
> 
> Lets say you get series of
> 
> (red, green, blue, delta_t )
> 
> points, meaning "in delta_t msec, change color to red, green,
> blue. Lets ignore other channels for now. delta_t of 0 would be step
> change. Would such interface work for you?
> 

So I presume this would be input to the RGB trigger that we discussed.
But in my current device I have 6 LEDs, that are not in any RGB-like
configuration. So we would need to come up with an interface that looks
to be the same in both single-LED and RGB-LED setups.


This should be sufficient to describe a subset of the patterns I've seen
so far in products.

But let's consider the standard use case for an RGB LED on an Android
phone; continuously blinking (pulsing based on patterns) as you have
some notifications waiting. In this case you want the LED hardware to do
all the work, so that you can deep-idle the CPU. So we would need to
introduce a "repeat pattern"-command.

Then consider the fact that you want your patterns to have decent
resolution, but you have a limited amount of storage. So we either have
to be able to detect palindromes or have a way to represent this.

> Simple compiler from this to LP5XX code should not be hard to
> do.

It sounds fairly straight forward to convert a pattern to instructions,
but we do have an extremely limited amount of storage so it must be a
quite good implementation for people to be able to use it for anything
real.

We could implement some optimization steps where we try to detect slopes
and generate ramp-instructions instead of set-pwm + wait instructions,
use some variables to handle ramp up/down and we could probably generate
some jump instructions to implement loops.

But do we really want this logic in the kernel, for each LED chip
supporting patterns?

> AS3676 ... I'm not sure what to do, AFAICT it is too limited.
> 

So out of the three examples I've looked at we're skipping one and we're
abstracting away most functionality from another.

I'm sorry for being pessimistic about this, but while I can see the
theoretical benefit of providing a uniform interface for this to user
space I see three very different pieces of hardware that would be used
in three different ways in products.

Regards,
Bjorn

  parent reply	other threads:[~2017-04-10 19:19 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-23  5:54 [PATCH 1/2] leds: Add driver for Qualcomm LPG Bjorn Andersson
2017-03-23  5:54 ` [PATCH 2/2] DT: leds: Add Qualcomm Light Pulse Generator binding Bjorn Andersson
2017-03-29  2:26   ` Rob Herring
2017-03-29 19:26     ` Bjorn Andersson
2017-03-29 22:13   ` Pavel Machek
2017-03-23 20:37 ` [PATCH 1/2] leds: Add driver for Qualcomm LPG Pavel Machek
2017-03-27  4:48   ` Bjorn Andersson
2017-03-29  2:17   ` Rob Herring
2017-03-29 19:07     ` Bjorn Andersson
2017-03-29 22:23       ` Pavel Machek
2017-03-30  0:09         ` Bjorn Andersson
2017-03-30  7:43           ` Pavel Machek
2017-03-31  9:28             ` Jacek Anaszewski
2017-04-02 12:54               ` Jacek Anaszewski
2017-04-03 18:21                 ` Bjorn Andersson
2017-04-03 20:38                   ` Jacek Anaszewski
2017-04-10  9:52                   ` Pavel Machek
2017-04-03 19:00               ` Bjorn Andersson
2017-04-03 20:38                 ` Jacek Anaszewski
2017-04-07 20:26                   ` Bjorn Andersson
2017-04-08  9:57                     ` Pavel Machek
2017-04-08 13:39                     ` Pavel Machek
2017-04-09 12:32                       ` Jacek Anaszewski
2017-04-10 19:19                       ` Bjorn Andersson [this message]
2017-04-11 17:54                         ` Pavel Machek
2017-04-11 23:17                           ` Bjorn Andersson
2017-04-07 13:32                 ` Pavel Machek
2017-04-07 20:36                   ` Bjorn Andersson
2017-04-08  9:33                     ` Pavel Machek
2017-04-07 12:54               ` Pavel Machek
2022-05-23 16:30 ` Pavel Machek
2022-05-23 22:01   ` Marijn Suijten
2022-05-23 22:18     ` Pavel Machek
2022-05-24 18:19       ` Marijn Suijten
2022-05-24 15:02   ` Bjorn Andersson
2022-05-24 18:26     ` Marijn Suijten
2022-05-24 20:10     ` Pavel Machek

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=20170410191948.GE15143@minitux \
    --to=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jacek.anaszewski@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pavel@ucw.cz \
    --cc=robh@kernel.org \
    --cc=rpurdie@rpsys.net \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).