All of lore.kernel.org
 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: 48+ 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
     [not found]   ` <20170323055435.29197-2-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-03-29  2:26     ` Rob Herring
2017-03-29  2:26       ` Rob Herring
2017-03-29 19:26       ` Bjorn Andersson
2017-03-29 19:26         ` Bjorn Andersson
2017-03-29 22:13   ` Pavel Machek
     [not found] ` <20170323055435.29197-1-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-03-23 20:37   ` [PATCH 1/2] leds: Add driver for Qualcomm LPG Pavel Machek
2017-03-23 20:37     ` Pavel Machek
2017-03-27  4:48     ` Bjorn Andersson
2017-03-27  4:48       ` Bjorn Andersson
2017-03-29  2:17     ` Rob Herring
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-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-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-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-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-08  9:33                         ` Pavel Machek
     [not found]                 ` <bf999f34-4509-6f0b-602c-f82d50a18e97-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-04-07 12:54                   ` 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 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.