Re: [DPU PATCH v2 2/2] drm/panel: add backlight control support for truly panel

[Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]

On Tue 17 Apr 17:42 PDT 2018, abhinavk@codeaurora.org wrote:

> Adding another point.
> 
> On 2018-04-17 17:04, abhinavk@codeaurora.org wrote:
> > Hi Bjorn
> > 
> > Apologies if the prev reply wasnt clear.
> > 
> > Hope this one is.
> > 
> > reply inline.
> > 
> > On 2018-04-17 14:29, Bjorn Andersson wrote:
> > > On Tue 17 Apr 11:21 PDT 2018, abhinavk@codeaurora.org wrote:
> > > > On 2018-04-16 23:13, Bjorn Andersson wrote:
> > > [..]
> > > > > If the panel isn't actually a piece of backlight hardware then it should
> > > > > not register a backlight device. Why do you need your own sysfs?
> > > > >
> > > > > Regards,
> > > > > Bjorn
> > > > [Abhinav] This particular panel isnt a piece of backlight hardware.
> > > > But, we want to have our own sysfs to give flexibility to our
> > > > userspace
> > > > to write and read stuff for its proprietary uses.
> > > 
> > > Please be more specific in your replies, no one will accept code that
> > > "does stuff" and expecting a reviewer to spend time guessing or
> > > pulling
> > > the information out of you is a sure way to get your patches ignored.
> > > 
> > > Exactly what kind of stuff are you talking about here and exactly
> > > which
> > > problem are you solving.
> > > 
> > > > Thats how our downstream has been working so far and hence to
> > > > maintain
> > > > the compatibility would like to have it.
> > > 
> > > Make your proprietary code work with the upstream kernel and you
> > > shouldn't ever have to modify it.
> > > 
> > > Regards,
> > > Bjorn
> > 
> > [Abhinav] We have a few userspace clients today for the backlight sysfs
> > node
> > which read/write directly to
> > "/sys/class/backlight/panel0-backlight/brightness"
> > When I said "stuff" I was only referring to the brightness value.
> > This separate sysfs node allows us to validate those userspace features
> > of ours
> > which directly edit the backlight value on our reference platform.
> > Since our reference platform uses this panel,hence having our own
> > sysfs alias helps.
> > Otherwise, whenever we try to use this panel with upstream code we
> > will have to end up
> > changing all those places in our userspace/framework to change the sysfs
> > path.
> > Hence we wanted to preserve that sysfs node name.
> > The wled device is not created under /sys/class/backlight but under
> > /sys/class/leds/wled.
> > So we will have to change the name of this node across all our
> > userspace.
> > 
> > If this isnt a convincing reason enough to have its own sysfs node
> > path, I will use
> > your approach.
> > 
> > Let me know your comments or if this is still not clear.
> > 
> [Abhinav] We also might not be using the brightness value "as-it-is".
> 
> We will potentially scale it up/down based on some requirements.
> 
> If we do not have our own sysfs alias for this, how do we account for
> providing this interface for our chipset specific backlight dependent
> feature.
> 
> Can you please comment on this?
> 

What kind of requirements would this be and what do you mean with scale
it up/down?

As the current implementation is proposed any magic added on top would
work for this panel driver and wouldn't be available for any other
panel, which doesn't sounds optimal.

Regards,
Bjorn
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno