Re: Tegra DRM device tree bindings

[Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>]

Hello Thierry,

I still haven't found the time to look at device tree things in detail,
but still some comments inline. Overall I think the tree looks ok and is
a great thing to get started.

Am Dienstag, den 26.06.2012, 12:55 +0200 schrieb Thierry Reding:
> Hi,
> 
> while I haven't got much time to work on the actual code right now, I
> think it might still be useful if we could get the device tree binding
> to a point where everybody is happy with it. That'll also save me some
> time once I get to writing the code because I won't have to redo it over
> again. =)
> 
> So here's the current proposal:
> 
> 	host1x {
> 		compatible = "nvidia,tegra20-host1x", "simple-bus";
> 		reg = <0x50000000 0x00024000>;
> 		interrupts = <0 64 0x04   /* cop syncpt */
> 			      0 65 0x04   /* mpcore syncpt */
> 			      0 66 0x04   /* cop general */
> 			      0 67 0x04>; /* mpcore general */
> 
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 
> 		ranges = <0x54000000 0x54000000 0x04000000>;
> 
> 		status = "disabled";
> 
> 		gart = <&gart>;
> 
Do we really need this here? IMHO the GART is one kind of a IOMMU
managing a part of the bus where the host1x driver is hanging of. So I
can't see why we would need a pointer to the gart dev directly.
Though I may be off here, as I don't grok everything related to the
initiation order yet, so please correct me if I'm wrong.
 
> 		/* video-encoding/decoding */
> 		mpe {
> 			reg = <0x54040000 0x00040000>;
> 			interrupts = <0 68 0x04>;
> 			status = "disabled";
> 		};
> 
> 		/* video input */
> 		vi {
> 			reg = <0x54080000 0x00040000>;
> 			interrupts = <0 69 0x04>;
> 			status = "disabled";
> 		};
> 
> 		/* EPP */
> 		epp {
> 			reg = <0x540c0000 0x00040000>;
> 			interrupts = <0 70 0x04>;
> 			status = "disabled";
> 		};
> 
> 		/* ISP */
> 		isp {
> 			reg = <0x54100000 0x00040000>;
> 			interrupts = <0 71 0x04>;
> 			status = "disabled";
> 		};
> 
> 		/* 2D engine */
> 		gr2d {
> 			reg = <0x54140000 0x00040000>;
> 			interrupts = <0 72 0x04>;
> 			status = "disabled";
> 		};
> 
> 		/* 3D engine */
> 		gr3d {
> 			reg = <0x54180000 0x00040000>;
> 			status = "disabled";
> 		};
> 
> 		/* display controllers */
> 		dc1: dc@54200000 {
> 			compatible = "nvidia,tegra20-dc";
> 			reg = <0x54200000 0x00040000>;
> 			interrupts = <0 73 0x04>;
> 			status = "disabled";
> 		};
> 
> 		dc2: dc@54240000 {
> 			compatible = "nvidia,tegra20-dc";
> 			reg = <0x54240000 0x00040000>;
> 			interrupts = <0 74 0x04>;
> 			status = "disabled";
> 		};
> 
> 		/* outputs */
> 		rgb {
> 			compatible = "nvidia,tegra20-rgb";
> 			status = "disabled";
> 		};
> 
> 		hdmi {
> 			compatible = "nvidia,tegra20-hdmi";
> 			reg = <0x54280000 0x00040000>;
> 			interrupts = <0 75 0x04>;
> 			status = "disabled";
> 		};
> 
> 		tvo {
> 			compatible = "nvidia,tegra20-tvo";
> 			reg = <0x542c0000 0x00040000>;
> 			interrupts = <0 76 0x04>;
> 			status = "disabled";
> 		};
> 
> 		dsi {
> 			compatible = "nvidia,tegra20-dsi";
> 			reg = <0x54300000 0x00040000>;
> 			status = "disabled";
> 		};
> 	};
> 
> This really isn't anything new but merely brings the Tegra DRM binding
> in sync with other devices in tegra20.dtsi (disable devices by default,
> leave out unit addresses for unique nodes). The only actual change is
> that host1x clients are now children of the host1x node. The children
> are instantiated by the initial call to of_platform_populate() since the
> host1x node is also marked compatible with "simple-bus".
> 
We should not rely on OF code doing the instantiation of the children,
as expressed by Grant Likely. [1]

> An alternative would be to call of_platform_populate() from the host1x
> driver. This has the advantage that it could integrate better with the
> host1x bus implementation that Terje is working on, but it also needs
> additional code to tear down the devices when the host1x driver is
> unloaded because a module reload would try to create duplicate devices
> otherwise.
> 
[snip]

[1]
http://www.mail-archive.com/linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org/msg28044.html

Thanks,
Lucas