On Mon, 14 Sep 2015, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Mon, Sep 14, 2015 at 01:10:01PM +0300, Jani Nikula wrote: >> On Mon, 14 Sep 2015, Daniel Vetter <daniel@xxxxxxxx> wrote: >> > On Fri, Sep 11, 2015 at 02:28:02PM +0300, Jani Nikula wrote: >> >> On Fri, 11 Sep 2015, Deepak M <m.deepak@xxxxxxxxx> wrote: >> >> > In CABC (Content Adaptive Brightness Control) content grey level >> >> > scale can be increased while simultaneously decreasing >> >> > brightness of the backlight to achieve same perceived brightness. >> >> > >> >> > The CABC is not standardized and panel vendors are free to follow >> >> > their implementation. The CABC implementaion here assumes that the >> >> > panels use standard SW register for control. >> >> > >> >> > In this design there will be no PWM signal from the SoC and DCS >> >> > commands are sent to enable and control the backlight brightness. >> >> > >> >> > v2: >> >> > - Created a new backlight driver for cabc, which will be registered >> >> > only when it cabc is supported by panel. (Daniel Vetter) >> >> >> >> I don't know what Daniel's been telling you, but I think this does need >> >> to get bolted into the regular backlight control mechanism. We'll also >> >> need eDP specific backlight control, and there's the VLV/CVH pwm driver >> >> absed backlight control, so we need to cover this more gracefully than >> >> an if-else ladder anyway. >> >> >> >> My idea all along has been that the backlight hooks in dev_priv.display >> >> need to be moved to connectors (probably intel_panel), and connector >> >> backlight setup can do what they want with these. All the boilerplate >> >> code for sysfs and scaling and so on would be there already. >> >> >> >> I do not approve of the approach here. >> > >> > The current design of backlights in linux is that userspace picks the >> > right backlight device. We've enabled/disabled the backlight pwm >> > ourselves too, but that was always just for the native backlight power >> > thing, not any of the others. >> >> What does that have to do with this series? >> >> > Imo this is the right approach since it fits into the current design. >> >> No, it does *not* fit into the current design, which you can see from >> the if ladders there. > > Well those if ladders are partially breaking the current design, we've > never done that for any of the other backlight drivers. For those > userspace is expected to call them before a modeset. > > Adding if ladders to the kernel is probably not needed if userspace holds > up it's part of the deal. > >> > Fixing the current design so that i915 can get at the right backlight >> > driver should imo be a separate series. I.e. yes this is what I >> > recommended roughly (but I didn't check details nor can I test with >> > xf86-video-intel to make sure it actually works correctly). >> >> It is important this gets done the right way up front, because there's >> now two series in flight to rip up the backlight code that's now neat >> and tidy. >> >> I am *not* volunteering to clean up the backlight code again. I've >> already done it once. > > This isn't about cleaning up the i915 backlight code, it's about pulling > the backlight selection logic from libbacklight into the kernel. At least > for the if ladders in the encoder enable/disable hooks. I assumed that > neither for capc and for the edp dpcd backlight we'd need those (userspace > should frob the backlight for us). I can't be bothered to summarize the rather frustrated IRC discussion on this, but the bottom line is that the kernel implementation should be done using the backlight hooks approach I suggested in [1]. That does not exclude the possibility of exposing more backlight interfaces if needed, but it keeps the kernel implementation neat. BR, Jani. [1] http://mid.gmane.org/cover.1442227790.git.jani.nikula@xxxxxxxxx > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
