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

[Daniel Vetter]

On Tue, Mar 24, 2015 at 11:16 AM, Linus Walleij
<linus.walleij at linaro.org> wrote:
> 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.

Yeah the discussion started because of Laurent's panel framework, but
it was very much a general concern. A bunch of the arm gfx drivers are
a pile of modules, and the resulting coordinatino chaos makes
suspend/resume and driver load a real challenge to get right. Drivers
which only modularize where it's absolutely needed are much easier to
understand.

>> 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).

The alternative would be to use the component framework with some
hardcoded names. Think board file distributed through all drivers with
no abstraction to get at handles for different pieces. We already have
that between i915 and snd-hda since the coordination to get
audio-over-dp/hdmi right is a bit tricky. We could have partially
reused e.g. power domains, but since the power domain/clock tree i915
manages isn't using generic infrastructure (yet, who knows) that
didn't look like a good idea.

power domains are actually a nice example for the tradeoffs: Currently
the i915 power domain code doen't implement hystersis. So for now we
just implement it ourselves for the one domain that really needs it.
But it might make sense to rebase the i915 power domain code to use
core power domains internally if we need hystersis in more places.

> 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.

So the main reason I thought reusing something common would make sense
here is that we wouldn't need to roll our own lookup structure stuff
and could reuse the logic provided by gpio for board files.
Longer-term we might even have switched to gpio abstraction internally
(maybe, not sure) since there's a bunch more board design floating
around (and sometimes shipping already like this one here) where the
panel line isn't controlled by i915 directly.

Also I just wanted to figure out whether use-cases like this
(irrespective of gpio, pwm, regulator or whatever) which are in some
sense worse than board files would be even acceptable. At least I
expect even more fun in the future since intel socs get integrated
ever tighter. Most of the interactions are handled by magic in some
power/clock firmware controllers (to make windows work), but there's
always small rifts in the seams like here where they didn't remap some
random register across the entire chip. Which the hw people generally
do, e.g. we can access the southbridge i2c engine through the same
mmio window as we access the gpu on the main die ;-)

Fundamentally we just don't have a nice description of the hw like in
dt with all the power domains/regulators and depencies described in an
abstract and unified way. It's a bad mess of hard-coded knowledge in
the driver, magic firmware to orchestrate cross-driver interactions
and the occasional explicit cross-driver communication in software
like this one where it all falls apart.

>> 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.

Yeah backlight is similar, for most panels we need to frob it at
precise points or otherwise the source port or panel sink falls over.
So overall it's a judgement call, but here I've thought the benefits
of using the gpio lookup stuff (and maybe the benefits to the
subsystem for accepting yet another crazy use-case and learning some
interesting things) outweight the costs - also because the gpio
abstraction is a very dumb/simple one.

What I wanted is something that magically gives me a driver to flip a
line without asking questions or imposing anything, that seemed to fit
the gpio abstraction. Of course there's a full panel driver sitting on
top, but in a way that's one layer up. At least to me. And in that
upper layer the abstraction will most likely get in the way sooner or
later.

So summary:
- Reusing the dynamic gpio lookup stuff would be nice, and might be
interesting as a new crazy use-case (or maybe not). But not a
requirement since we have the component framework to handroll
something.
- More abstraction than "pls flick this line for me" is probably too
much since all the higher level logic must be in i915 because of the
already shipping design of the vbios tables and all that.
- I expect more of this kind of remote panel stuff and if we handroll
we'll probably end up reinventing gpio&pwm partially.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch