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

[Sean Paul <seanpaul@chromium.org>]

On Tue, Apr 17, 2018 at 05:04:56PM -0700, 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.

As I mentioned on the previous review, coding around closed source userspace
isn't something that we want to do. It's hard enough to accommodate open
source u/s as it is.

Sean

> 
> 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.
> 
> > _______________________________________________
> > Freedreno mailing list
> > Freedreno@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/freedreno
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel