[Intel-gfx] [PATCH 3/9] drm/i915: Use the CRC gpio for panel enable/disable

[Linus Walleij]

On Tue, Mar 24, 2015 at 10:50 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Tue, Mar 24, 2015 at 10:35:32AM +0100, Linus Walleij wrote:

>> Some stuff may be needed to associate the regulator with the right
>> device indeed but nothing horribly complicated.
>
> Nack, really. We've had an epic discussion at ks two years ago about how
> arm gpu drivers go overboard with abstraction.

I remember it clearly as I was in the room.

And yes IIRC that was indeed very much about the panel abstractions
suggested by Laurent Pinchart.

> We have all the insanity
> with power domains, clock trees and whatnoelse in i915.ko, but since it's
> all mostly from the same company we have it integrated into one driver
> with our own infrastructure. Dave Airlie was telling this everyone and I
> fully agree with him - pushing abstraction in the middle of the driver
> without the need for it just causes tears.

I fully understand this stance and I kind of understand why it came to
this. I had the same discussion a bit with some different graphics
people and DRM+panel drivers really are a special chapter when it
comes to sequencing & stuff. (As is ALSA-soundcard type audio
it seems, or anything multimedia.)

> The problem I have here is that two different pieces on the same board
> from the same company which won't ever get reused anywhere else need to do
> cross-driver communication about a gpio line. I don't want any kind of
> additional abstraction to get into the way at all, the only thing I want
> are:
> - some function to switch that line without delays or cleverness
>   interspersed.
> - dynamic lookup.
>
> GPIO seemed a perfect fit, but apparently it isn't. We'll roll our own.

I'm not dealing in absolutes so I want to know what makes GPIO
such a good fit compared to rolling your own.

I guess the alternative is that the i915 driver gets a handle from
another platform_device on the MFD cell (a different one, say
named panel-power or whatever) and pokes that register itself
(using regmap in the same way that my suggested regulator
code does basically).

I kind of like that because it makes the usecase all evident
like I wanted. And arguably it's a better solution if the i915 driver
want to be as self-contained as possible, using this pattern
that the DRM drivers should take care of all sequencing business.

The DRM driver can then also be used even if GPIO is configured
out of the kernel.

> And yeah your code isn't any longer than the gpio version, but regulators
> drag in all that conceptual stuff about voltage/delays and depencies that
> imo just isn't controlled by the pmic. We've originally abused the panel
> interface for all this and Thierry (imo rightfully suggested) that this
> isn't a panel but it'd be better to expose the underlying stuff. Again imo
> faking all that stuff since you think this looks like a regulator is imo a
> worse lie than the exposing this as a gpio.

Nah it's all lies :)

I think you are right: if the DRM driver wants to control everything
itself it should not use the regulator framework, neither the GPIO
framework, it seems to be an established pattern to roll your own
in these drivers so let's keep with that.

I guess the use case is analogous to a monitor cable sticking out
of a graphics card and sending these power-up/down sequences
to that monitor in some magic way, and that is how DRM thinks
of things. For what I know DRM frowns at the abstracted out
backlight control used by framebuffers in drivers/video/backlight
as well, and prefer to be on top of stuff, so then it should access
stuff on as low level as possible.

Yours,
Linus Walleij