On 04/13/2012 03:14 AM, Thierry Reding wrote:
> * Stephen Warren wrote:
>> On 04/12/2012 11:44 AM, Thierry Reding wrote:
> [...]
>> And given that, I don't think we should name the node after some
>> OS-specific software concept. Device tree is intended to model hardware.
> [...]
>>> Maybe one solution would be to have a top-level DRM device with a register
>>> map from 0x54000000 to 0x547fffff, which the TRM designates as "host
>>> registers". Then subnodes could be used for the subdevices.
>>
>> Ah yes, just what I was thinking above:-)
>
> I came up with the following:
>
> /* host1x */
> host1x : host1x at 50000000 {
> reg = <0x50000000 0x00024000>;
> interrupts = <0 64 0x04 /* cop syncpt */
> 0 65 0x04 /* mpcore syncpt */
> 0 66 0x04 /* cop general */
> 0 67 0x04>; /* mpcore general */
> };
>
> /* graphics host */
> graphics at 54000000 {
> compatible = "nvidia,tegra20-graphics";
>
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0 0x54000000 0x08000000>;
>
> host1x = <&host1x>;
>
> /* video-encoding/decoding */
> mpe at 54040000 {
> reg = <0x54040000 0x00040000>;
> interrupts = <0 68 0x04>;
> };
... [a bunch of nodes for graphics-related HW modules]
That all looks reasonable.
> display-controllers = <&disp1 &disp2>;
> outputs = <&lvds &hdmi &tvo &dsi>;
I don't think you need both the child nodes and those two properties.
In other words, I think you either want:
graphics at 54000000 {
... a bunch of child nodes
};
or you want:
disp1 : dc at 54200000 {
...
};
disp2 : dc at 54240000 {
...
};
... all the other graphics nodes
graphics at 54000000 {
display-controllers = <&disp1 &disp2>;
outputs = <&lvds &hdmi &tvo &dsi>;
};
In the former case, presumably the drivers for the child nodes would
make some API call into the parent node and just register themselves
directly as a certain type of driver, so avoiding the
display-controllers/outputs properties.
> /* initial configuration */
> configuration {
> lvds {
> display-controller = <&disp1>;
> output = <&lvds>;
> };
>
> hdmi {
> display-controller = <&disp2>;
> output = <&hdmi>;
> };
> };
> };
>
> I added an additional node for the initial configuration so that the driver
> knows which mapping to setup at boot.
Isn't that kind of thing usually set up by the video= KMS-related kernel
command-line option? See Documentation/fb/modedb.txt. Again here, I
think the actual display controllers would be allocated to whichever
outputs get used on a first-come first-serve based, so no need for the
display-controller property above either way.
> What I don't quite see yet is where to
> attach EDID data or pass the phandle to the I2C controller for DDC/EDID
> probing.
I think there's a third node type.
1) Nodes for the display controllers in Tegra (Also known as CRTCs I
believe)
2) Nodes for the video outputs from the Tegra chips (what you have as
outputs above)
3) Connectors, which aren't represented in your DT above yet.
I think you need to add additional nodes for (3). These are where you'd
represent all the EDID/I2C/... stuff. Then, the nodes for the outputs
will point at the nodes for the connectors using phandles, or vice-versa.
(IIRC, although I didn't look at them in detail, the sdrm patches
recently mentioned something about splitting up the various object types
in DRM and allowing them to be represented separately, and I vaguely
recall something along the lines of the types I mention above. I might
expect to have a 1:1 mapping between the DRM object types and DT nodes.)
> The initial configuration is certainly not the right place. Perhaps
> the outputs property should be made a node instead:
>
> outputs {
> lvds_out {
> output = <&lvds>;
> edid = <&edid>;
> };
>
> hdmi_out {
> output = <&hdmi>;
> ddc = <&i2c2>;
> };
> };
>
> But then "outputs" should probably become something like "connectors"
> instead and the initial configuration refers to the "_out" phandles.
