Message-ID: <CANq1E4Tt0+cb0YHZcTdQYkBmFXyAkbwNkofcyJ6bHsYQW9vH7g@mail.gmail.com>
Date:	Thu, 11 Sep 2014 18:07:24 +0200
From:	David Herrmann <dh.herrmann@...il.com>
To:	David Herrmann <dh.herrmann@...il.com>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	Dave Airlie <airlied@...il.com>,
	Jingoo Han <jg1.han@...sung.com>,
	Bryan Wu <cooloney@...il.com>,
	Lee Jones <lee.jones@...aro.org>,
	Matthew Garrett <matthew.garrett@...ula.com>,
	Adam Jackson <ajax@...hat.com>,
	linux-kernel <linux-kernel@...r.kernel.org>
Cc:	Daniel Vetter <daniel.vetter@...ll.ch>
Subject: Re: [PATCH RFC 4/4] drm: link connectors to backlight devices

Hi

On Thu, Sep 11, 2014 at 3:06 PM, Daniel Vetter <daniel@...ll.ch> wrote:
> On Thu, Sep 11, 2014 at 02:22:55PM +0200, David Herrmann wrote:
>> actual-brightness is a bit more tricky. Currently, DRM caches property
>> values, so there is no read_property() hook. We'd have to add this.
>> But it'll be quite nasty as we have to call into the backlight driver.
>> So I think we want to run an async-interruptible worker on the
>> backlight, drop the locks in the ioctl and wait for the job to finish.
>> Not sure whether it's worth it.. maybe we can add this later.
>
> See Jani's reply - we probably don't need it, at least not in version 1.

I couldn't see any comment regarding "actual-brightness". But I'm
totally fine with dropping this.

>>
>> > - How does udev match on the drm connector name? They are not terribly
>> >   stable atm, and if you reload your drm driver, or much more likely, have
>> >   two gpus with two drm drivers they change. We probably should change the
>> >   name allocation scheme to be per device instance instead of global
>> >   first. Within a driver probe order is hopefully deterministic on a given
>> >   platform, since even with super dynamic setups (based on dt/acpi) the
>> >   firmware tables should change really.
>>
>> You can match on EDID attributes. Ok, so far this is pretty ugly as
>> the EDID property is binary. But we can add rather trivial udev
>> extensions to make EDID binary against text matching possible.
>
> Why EDID? This is purely about the drm connector name, e.g. if I have 2
> gpus, both with an eDP connector (optimus, so just one panel) then the
> first driver gets eDP-1 as the name of it and the 2nd one eDP-2. Which is
> ok if both should control backlight brightness through the same driver,
> but a total mess if not just gpus get switched, but also backlight
> controllers.
>
> And if you reload you get then eDP-2 and eDP-3. Well at least in the past,
> that hilarity at least was fixed in
>
> commit b21e3afe2357c0f49348a5fb61247012bf8262ec
> Author: Ilia Mirkin <imirkin@...m.mit.edu>
> Date:   Wed Aug 7 22:34:48 2013 -0400
>
>     drm: use ida to allocate connector id
>
> What I think we need to do is to make these ida allocators per-device, so
> that both drivers have an eDP-1 connector. Otherwise you need to either
> match both or do funny tricks like "the first eDP connector, no matter
> which one on this gpu". After all we can now support more than one eDP
> (and more than one LVDS since a long time actually).
>
> Or how exactly is the udev hw db supposed to match this stuff for special
> cases. In general we need to duplicate the existing logic from
> libbacklight, like Matthew suggested.

Yeah, I get what you mean. Names are not stable so even if we can
match on the internal card, we cannot match on "lowest available eDP
display". per-device IDs should be totally fine and fix this issue. We
prefix connectors with the device name anyway, so no conflicts can
arise.

But I think we want to make this a udev builtin anyway. So we can
easily implement the same logic as libbacklight.

Thanks
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists