Re: [RFC CABC v3 PATCH 2/2] drm/i915: CABC support for backlight control

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 14 Sep 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 14 Sep 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Fri, Sep 11, 2015 at 02:28:02PM +0300, Jani Nikula wrote:
>>> On Fri, 11 Sep 2015, Deepak M <m.deepak@intel.com> 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.
>
>> 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.

Here's how you can actually do this nicely [1].

BR,
Jani.


[1] http://mid.gmane.org/cover.1442227790.git.jani.nikula@intel.com



>
> BR,
> Jani.
>
>
>> -Daniel
>> -- 
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx