Re: KMS backlight ABI proposition

[Martin Peres <martin.peres@linux.intel.com>]

On 20/02/17 21:57, Hans de Goede wrote:
> Hi,
>
> On 20-02-17 20:27, Dave Airlie wrote:
>> On 17 February 2017 at 22:58, Martin Peres 
>> <martin.peres@linux.intel.com> wrote:
>>> Hey everyone,
>>>
>>> We have been working towards exposing the backlight as a KMS property
>>> instead of relying on the backlight drivers. We have CC:ed the 
>>> people we
>>> have found to be the more likely to be interested in the discussion but
>>> please add everyone you think would have some experience with this 
>>> issue.
>>>
>>> == Introduction ==
>>>
>>> We are trying to bring the same level of support for the backlight 
>>> on both
>>> the xf86-video-intel and -modesetting DDX.
>>>
>>> Looking into the situation of the backlight, we identified these 
>>> problems
>>> which are almost show-stoppers for -modesetting and wayland 
>>> compositors:
>>>
>>>  - There is no mapping between the backlight driver and 
>>> DRM-connectors. This
>>> means that, in case there are multiple backlight drivers, the 
>>> userspace has
>>> to have knowledge of the machine to know which driver should be 
>>> used. See
>>> the priority list for the intel driver [0].
>>>
>>>  - The luminance curve of the backlight drivers is not specified, 
>>> which can
>>> lead to a bad user experience: Little changes in the highest levels but
>>> drastic changes in the low levels.
>>>
>>>  - Writing to the backlight driver still requires root rights. Given 
>>> that
>>> the xserver and wayland compositors are now running root-less, this 
>>> means we
>>> would need a complex dance involving a setuid helper [1].
>>>
>>> Hans de Goede has already given a presentation about these issues at
>>> XDC2014. The slides are a good read[2].
>>>
>>> [0]
>>> https://cgit.freedesktop.org/xorg/driver/xf86-video-intel/tree/src/backlight.c#n259 
>>>
>>>
>>> [1]
>>> https://cgit.freedesktop.org/xorg/driver/xf86-video-intel/tree/src/backlight.c#n348 
>>>
>>>
>>> [2]
>>> https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/backlight.pdf 
>>>
>>>
>>> == Proposal ==
>>>
>>> Since David Hermann already worked on this and proposed what I consider
>>> being greats foundations for building towards a solution addressing the
>>> issues above, I will just ask you to read his original words:
>>>
>>> https://lists.freedesktop.org/archives/dri-devel/2014-September/067984.html 
>>>
>
> Jumping in in the middle of the thread here, I'll reply to the 
> top-level post
> later.
>
>> You might want to read the rest of that thread, the response posted 
>> by Matthew
>>
>> This is really messy and we made a decision to put the policy to pick
>> which backlight
>> driver into userspace because it's not something the kernel can figure
>> out properly.
>
> That is actually not really true, the only policy which in practice lives
> in userspace is picking firmware type backlight interfaces over 
> platform /
> raw. Everything, really EVERYTHING else is already handled in the kernel,
> using quirk tables in various places. David Herrmann's proposal actually
> might improve upon this by having a default binding behavior in the 
> kernel
> and then allow fixing up the binding between backlight interface and
> drm connector through udev rules triggered by hwdb entries, which means
> (some (*)) new quirks could be handled in userspace.

I fully agree with this. Pushing everything to the userspace sounds like 
quite a
risk and adds complexity. This is not something the -modesetting driver and
wayland compositors will not deal in a uniform way...

Also, this means we need to expose a lot of information to the 
userspace, that
means new interfaces (and new ABI).

By trying to handle all the common cases in the kernel space and letting the
userspace override the decision through udev or by hand, it should 
result in a
much-better experience for everyone (no need to recompile your ddx driver
anymore).

Another added benefit of exposing backlight through KMS is that we do not
have to deal with two access control mechanisms, which should help the
userspace.

>
>> Things might have changed now a bit with Win10 not using ACPI 
>> backlight controls
>> if memory serves, but it's still an unholy mess.
>>
>> I think MBPs can expose 3 backlights (ACPI, gmux and driver) and only
>> the gmux one does anything.
>
> Correct which is why we have this in the kernel:
>
> drivers/gpu/drm/nouveau/nouveau_backlight.c :
>
>
>         if (apple_gmux_present()) {
>                 NV_INFO(drm, "Apple GMUX detected: not registering 
> Nouveau backlight interface\n");
>                 return 0;
>         }
>
>> How do you tackle that end of the problem, how does the i915/drm core
>> know when the
>> driver for the one backlight it needs has appeared, and is working, by
>> deferring this to
>> userspace we let the system load all the drivers and the policy of
>> picking the correct one
>> is left there.
>
> Except in practice it is not, each desktop-environment and then some
> other bits and pieces all have their own code to pick which backlight
> interface to use. Which luckily all are as simple as prefer firmware
> over platform/raw, since doing something more smart would be a big
> mess as everything has its own code for handling this.
>
> And to work around this we end up with things like hiding the
> acpi_video interface on win10 devices since it usually is broken,
> but as it has a type of firmware it will be preferred when visible.
>
> Really we are already dealing with this in the kernel, it is one of
> the things I've been spending my time on the last 3 years, since
> I've blogged and presented about this people tend to know to find
> me when it comes to bugs related to this, so I'm quite familiar
> with this.
>
> All this proposal does is make the userspace interface for backlight
> suck less, we already have all the pain in the kernel anyways.
>
>> I'm not saying this is pretty,and we have libbacklight to "solve" the
>> problems for generic
>> userspace,
>
> libbacklight ? Never heard of it, ah google gives me:
>
> https://cgit.freedesktop.org/libbacklight/
>
> Last commit 2011, number of users (AFAIK) 0, certainly not used by
> gnome, upower, xf86-video-intel, etc.

And it only does what you described: Prefer firmware drivers over raw ones.

>
> > but any solution is going to a be a lot uglier than you think.
>
> Oh I know it is ugly, but as said we are already doing it, so we
> might as well make it suck less.
>
> Regards,
>
> Hans
>
>
>
>
> *) I say some because in some cases even loading a driver can put the
> firmware in such a shape that backlight control will not work until a
> hard reset is done.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel