lkml.org 
[lkml]   [2017]   [Apr]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/2] leds: Add driver for Qualcomm LPG
    On Sun 02 Apr 05:54 PDT 2017, Jacek Anaszewski wrote:

    > On 03/31/2017 11:28 AM, Jacek Anaszewski wrote:
    > > Hi Bjorn and Pavel,
    > >
    > > On 03/30/2017 09:43 AM, Pavel Machek wrote:
    [..]
    > > In both RGB and pattern approaches we should assess
    > > if it is acceptable to provide a pattern for trigger name,
    > > e.g. blink-pattern-{num_intervals}.
    >
    > Actually we could achieve the goal by listing all available pattern
    > configurations for given LED class device, so in case of Qualcomm LPG
    > driver we could have transition-pattern-1 to transition-pattern-15
    > listed after executing "cat trigger".
    >

    There's a common pattern-table of 24 (or 64) entries, that is shared
    among the 8 LPGs (each LPG simply has to indices pointing into the
    shared table). Each entry in the table holds a value between 0 and 511.
    So that's a lot of "available pattern configurations".

    Unless you go with the path Qualcomm did in their downstream driver,
    where the table is filled statically from DeviceTree and each LPG is
    statically configured with some range from the table. But I don't like
    this and as far as I can tell neither do you guys.

    And lastly the request is to create a common interface for userspace to
    control patterns among different LED hardware and I do not see how this
    would be acceptable to the LP55xx users.


    Perhaps I'm not getting what you're proposing?

    > In case of RGB trigger we would have qcom-rgb among other triggers
    > listed in a result of executing "cat trigger". Then qcom-rgb-1 LED class
    > device could appear in /sys/class/leds, which would expose files
    > red-led-name, green-led-name and blue-led-name. Once all files are
    > initialized with appropriate LED class device names the RGB brightness
    > could be updated synchronously in all involved LEDs by executing
    > e.g. "echo 1 > update_color". Of course all LEDs that set qcom-rgb
    > trigger would have to avoid changing device state on write to
    > their brightness file.
    >

    I don't see that the brightness of the individual LEDs is the problem,
    writing individual colors to the 3 LEDs in a serial fashion isn't
    user-noticeable. What is a problem is if you start a pulse of some
    combined color it's important to synchronize the starting of the pattern
    generator, of you will have a color that shifts over time - or in the
    case of blink where the colors get out of sync.

    And as I said, I suggest that we just make it possible to configure any
    LPG channel to be grouped with any others and within a group we always
    synchronize the pattern-generator when enabling any LED.


    The issue left open is that we expose 3 independent LEDs to userspace
    and it seems desired to expose it as a single "RGB" LED. Providing a
    "RGB trigger" that any LED in the system can be associated with and then
    have that trigger wrap the individual LEDs sounds like a reasonable path
    forward. But if we're not going to do things like color "calibration" it
    feels like we're replacing the 3 writes in userspace with a single write
    and then 3 calls in the kernel; i.e. the only win in my view is some
    conceptual benefit.

    > I wonder if I'm not missing some vital constraints here that could
    > make this design unfeasible.
    >

    Regardless of how we expose RGBs to userspace, the 8 LPG hardware blocks
    are independent of each other. The fact that they end up controlling
    something that is perceived by the human eye as some mixed color is to
    me a matter of system integration, and as such should not convolute the
    implementation of the individual instances.

    Regards,
    Bjorn

    \
     
     \ /
      Last update: 2017-04-03 20:22    [W:3.816 / U:0.484 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site