Tegra DRM device tree bindings

[Thierry Reding]

On Tue, Jun 26, 2012 at 02:47:56PM +0200, Lucas Stach wrote:
> 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.

If the information about this can be obtained in another way, then we
certainly don't need this here. Hiroshi already pointed out that this
can be handled by the IOMMU API now (I don't think it did back when I
first wrote the code), so I'm all for stripping unneeded properties from
the binding.

One thing we need to keep in mind here is that the DRM driver is not the
only one needing access to memory. There may be other users (V4L, ...)
that need memory from the same pool. Maybe the memory handling should be
done in the host1x driver instead, and the DRM driver could be one of
many users obtaining buffers from it?

> > 		/* 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 at 54200000 {
> > 			compatible = "nvidia,tegra20-dc";
> > 			reg = <0x54200000 0x00040000>;
> > 			interrupts = <0 73 0x04>;
> > 			status = "disabled";
> > 		};
> > 
> > 		dc2: dc at 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]

At the end of that email, Grant specifically mentions that using the
simple-bus is one option to instantiate children. Or to add the bus
compatible (nvidia,tegra20-host1x in our case) to the list of busses
when calling of_platform_populate().

However I think if we have a custom bus_type implementation for host1x
anyway, then it makes sense to instantiate the children from that
instead to allow for easier integration.

Thierry

> > 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@lists.ozlabs.org/msg28044.html
> 
> Thanks,
> Lucas
> 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/devicetree-discuss/attachments/20120626/aea76ef8/attachment.sig>