Re: [RFC 4/4] drm: Add NVIDIA Tegra support

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 04/15/2012 02:39 AM, Thierry Reding wrote:
> * Stephen Warren wrote:
>> On 04/13/2012 03:14 AM, Thierry Reding wrote:
>>> 		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@54000000 {
>> 		... a bunch of child nodes
>> 	};
>>
>> or you want:
>>
>> 	disp1 : dc@54200000 {
>> 		...
>> 	};
>> 	disp2 : dc@54240000 {
>> 		...
>> 	};
>> 	... all the other graphics nodes
>>
>> 	graphics@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.
> 
> I think I like the former better. The way I understand it the children of the
> graphics node will have to be registered explicitly by the DRM driver because
> of_platform_populate() doesn't work recursively. That would ensure that the
> DRM driver can setup the CRTC and output registries before the other devices
> call back into the DRM to register themselves.

Yes, with the first way, the DRM driver will have to call
of_platform_populate() recursively to make this work.

The thing here is that the device tree should model hardware, not be
designed purely to match the device registration needs of the DRM
driver. I'm not sure that it's correct to model all those devices as
children of the top-level graphics object; I /think/ all the devices are
flat on a single bus, and hence not children of each-other. That all
said, I guess having the nodes as children isn't too far off how the HW
is designed (even if the register accesses aren't on a child bus, the
modules at least logically are grouped together in an umbrella
situation), so I wouldn't push back on the first option above that you
prefer.

>>> 		/* 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.
> 
> Boards should still be able to boot and display a console on the "standard"
> output even if the user doesn't provide a video= option. Shouldn't there be a
> way for a board DTS to specify what the default (or even allowed) connections
> are?

Why wouldn't the default be to light up all outputs that have an
attached display, or an algorithm something like:

* If internal LCD is present, use that
* Else, if HDMI display plugged in, use that
...

> Evaluation hardware like the Harmony might have LVDS, HDMI and VGA connectors
> to provide for a wide range of use cases. The Plutux for instance has only an
> HDMI connector and the Medcom has only LVDS. For the Medcom it would be quite
> confusing for people to suddenly see an HDMI-1 connector pop up f.e. in
> xrandr. It would be equally useless for the Plutux to show up as supporting
> an LVDS or VGA connector.

So the device tree for those devices would disable (or not include) the
connectors that were not present on the board.

...
> I see. Maybe this could be used for board-specific configuration? For
> example, the Plutux could have something like this:
> 
> 	connectors {
> 		hdmi {
> 			output = <&hdmi>;
> 			ddc = <&i2c2>;
> 		};
> 	};
> 
> The Medcom could have:
> 
> 	connectors {
> 		lvds {
> 			output = <&lvds>;
> 			edid = <&edid>;
> 		};
> 	};
> 
> While Harmony could be more generic and provide more outputs:
> 
> 	connectors {
> 		lvds {
> 			output = <&lvds>;
> 			ddc = <&i2c1>;
> 		};
> 
> 		vga {
> 			/* which output is used for VGA? */
> 			output = <...>;
> 			ddc = <&i2c2>;
> 
> 		hdmi {
> 			output = <&hdmi>;
> 			ddc = <&i2c3>;
> 		};
> 	};

That looks like a reasonable start.

> Has there been any discussion as to how EDID data would best be represented
> in DT? Should it just be a binary blob or rather some textual representation?

I think a binary blob makes sense - that's the exact same format it'd
have if read over the DDC I2C bus.