[PATCH v2 12/26] drm/exynos: Split manager/display/subdrv

[Sean Paul]

On Mon, Oct 28, 2013 at 12:49 PM, Olof Johansson <olof at lixom.net> wrote:
> On Wed, Oct 23, 2013 at 9:09 AM, Sean Paul <seanpaul at chromium.org> wrote:
>> On Wed, Oct 23, 2013 at 11:53 AM, Dave Airlie <airlied at gmail.com> wrote:
>>>>>>>>
>>>>>>>
>>>>>>> I think we need to start considering a framework where subdrivers just
>>>>>>> add drm objects themselves, then the toplevel node is responsible for
>>>>>>> knowing that everything for the current configuration is loaded.
>>>>>>>
>>>>>>
>>>>>> It would be nice to specify the various pieces in dt, then have some
>>>>>> type of drm notifier to the toplevel node when everything has been
>>>>>> probed. Doing it in the dt would allow standalone drm_bridge/drm_panel
>>>>>> drivers to be transparent as far as the device's drm driver is
>>>>>> concerned.
>>>>>>
>>>>>> Sean
>>>>>>
>>>>>>
>>>>>>> I realise we may need to make changes to the core drm to allow this
>>>>>>> but we should probably start to create a strategy for fixing the API
>>>>>>> issues that this throws up.
>>>>>>>
>>>>>>> Note I'm not yet advocating for dynamic addition of nodes once the
>>>>>>> device is in use, or removing them.
>>>>>>>
>>>>>
>>>>> I do wonder if we had some sort of tag in the device tree for any nodes
>>>>> involved in the display, and the core drm layer would read that list,
>>>>> and when every driver registers tick things off, and when the last one
>>>>> joins we get a callback and init the drm layer, we'd of course have the
>>>>> basic drm layer setup prior to that so we can add the objects as the
>>>>> drivers load. It might make development a bit trickier as you'd need
>>>>> to make sure someone claimed ownership of all the bits for init to proceed.
>>>>>
>>>>
>>>> Yeah, that's basically what the strawman looked like in my head.
>>>>
>>>> Instead of a property in each node, I was thinking of having a
>>>> separate gfx pipe nodes that would have dt pointers to the various
>>>> pieces involved in that pipe. This would allow us to associate
>>>> standalone entities like bridges and panels with encoders in dt w/o
>>>> doing it in the drm code. I *think* this should be Ok with the dt guys
>>>> since it is still describing the hardware, but I think we'd have to
>>>> make sure it wasn't drm-specific.
>>>>
>>>
>>> I suppose the question is how much dynamic pipeline construction there is,
>>>
>>> even on things like radeon and i915 we have dynamic clock generator to
>>> crtc to encoder setups, so I worry about static lists per-pipe, so I still
>>> think just stating all these devices are needed for display and a list of valid
>>> interconnections between them, then we can have the generic code model
>>> drm crtc/encoders/connectors on that list, and construct the possible_crtcs
>>> /possible_clones etc at that stage.
>>>
>>
>> I'm, without excuse, hopeless at devicetree, so there are probably
>> some violations, but something like:
>
> This is definitely worth discussing. I know device-tree but not DRM so
> let's see if we can figure this out together. :)
>
>>
>> display-pipelines {
>>   required-elements = <&bridge-a &panel-a &encoder-x &encoder-y
>> &crtc-x &crtc-y>;
>
> What's this supposed to mean? Are all these elements required to get
> DRM up on this particular platform? Or are they just enumerating all
> the possible devices that might be involved?
>

I was thinking that these would be all of the pieces that are required
before drm is loaded.

So, in the multiple device-drivers scenario, these would all register
themselves with drm on probe. drm would tick them off the list as they
probe until all have been accounted for. At this point, drm would load
and call them back with a pointer to the drm_device.

In the case where there is One True DRM Driver, where the various
devices are not standalone drivers (see ptn3460 patch for this model),
this would be the list of devices that drm is responsible for linking
in. To do this, it would need to call one hook when it probes (so it
can be deferred), and than an initialization hook later once
everything is probed and ready.

>>   pipe1 {
>>     bridge = <&bridge-a>;
>>     encoder = <&encoder-x>;
>>     crtc = <&crtc-y>;
>>   };
>>   pipe2 {
>>     encoder = <&encoder-x>;
>>     crtc = <&crtc-x>;
>>   };
>>   pipe3 {
>>     panel = <&panel-a>;
>>     encoder = <&encoder-y>;
>>     crtc = <&crtc-y>;
>>   };
>
> Maybe it's the use of pipe as a keyword that's confusing me here, and
> my lack of DRM knowledge, but wouldn't it make more sense to focus on
> output connectors here? I.e. describe the outputs such as eDP, LVDS,
> HDMI. The bridge, such as edp-to-lvds, should possibly be specified
> under the edp node instead of in the drm node here.
>

Sure, I think "pipeX" is poor nomenclature on my part. It would make
more sense to have these node names translate to the actual connector
name. I was trying to be a bit opaque/non-drm-specific to avoid
describing software in the devicetree.

Again, using the ptn driver as an example, something like:

LVDS-1 {
        connector = <&ptn3460>;
        bridge = <&ptn3460>;
        encoder = <&display-port-controller>;
        crtc = <&fimd>;
};



> Also, while the hardware can allow a very flexible connection of
> pipes, I'm guessing that in reality nearly all usage of these will
> follow a few common patterns, so maybe it's not required to be able to
> describe the full graph in device tree?
>

I suspect you're correct. I think we can make bridge/encoder/crtc
arrays such that you can describe multiple pipes within one connector.
Admittedly, I have a very narrow view of the different hardware that
might use this, so there might be a better way.

Sean



>> I'm tempted to add connector to the pipe nodes as well, so it's
>> obvious which connector should be used in cases where multiple
>> entities in the pipe implement drm_connector. However, I'm not sure if
>> that would be NACKed by dt people.
>
> As mentioned above, it might make sense to turn it around and describe
> it around the connectors instead of the pipes.
>
>> I'm also not sure if there are too many combinations for i915 and
>> radeon to make this unreasonable. I suppose those devices could just
>> use required-elements and leave the pipe nodes out.
>
>
> -Olof