On Tue, Jun 26, 2012 at 04:01:05PM +0300, Terje Bergström wrote:
> On 26.06.2012 13:55, Thierry Reding wrote:
>
> > 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 */
>
>
> We're only interested in interrupts 65 and 67. The COP interrupts are
> not routed to CPU. I guess we could just delete those lines here.
I remember you mentioned that before. I'll drop them.
> >
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > ranges = <0x54000000 0x54000000 0x04000000>;
>
>
> I'm a newbie on device trees, so I need to ask. Why isn't host1x
> register space covered by "ranges" property?
The ranges property defines the memory aperture that is forwarded to the
children.
> > status = "disabled";
> >
> > gart = <&gart>;
> >
> > /* video-encoding/decoding */
> > mpe {
> > reg = <0x54040000 0x00040000>;
> > interrupts = <0 68 0x04>;
> > status = "disabled";
> > };
>
>
> The client device interrupts are not very interesting, so they could be
> left out, too. Display controller related are probably an exception to this.
If the interrupts aren't used at all we should drop them.
> (...)
>
> Otherwise the proposal looked good.
>
> We also assign certain host1x common resources per device by convention,
> f.ex. sync points, channels etc. We currently encode that information in
> the device node (3D uses sync point number X, 2D uses numbers Y and Z).
> The information is not actually describing hardware, as it just
> describes the convention, so I'm not sure if device tree is the proper
> place for it.
Are they configurable? If so I think we should provide for them being
specified in the device tree. They are still hardware resources being
assigned to devices.
> > 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.
>
>
> Yes, we already have a bus_type for nvhost, and we have nvhost_device
> and nvhost_driver that device from device and device_driver
> respectively. They all accommodate some host1x client device common
> behavior and data that we need to store. We use the bus_type also to
> match each device and driver together, but the matching is version
> sensitive. For example, Tegra2 3D needs different driver than Tegra3 3D.
We'll have to figure out the best place to put this driver. The driver
will need some code to instantiate its children from the DT and fill the
nvhost_device structures with the data parsed from it.
BTW, what's the reason for calling it nvhost and not host1x?
> > Perhaps the ranges property can also be used to remap the reg properties
> > of the child nodes so that they can be specified as an offset into the
> > host1x aperture instead of an address in the CPU address space. But
> > that's just a minor issue because the OF code should be able to handle
> > it transparently.
>
>
> Either way is fine for me. The full addresses are more familiar to me as
> we tend to use them internally.
Using the OF mechanism for translating the host1x bus addresses,
relative to the host1x base address, to CPU addresses seems "purer", but
either way should work fine.
> > Basically this turns on the first display controller and the RGB/LVDS
> > output and hooks up a static EDID block with the LVDS output. There is
> > also a carveout property which might be a better replacement for the
> > "crippled" GART on Tegra20. Alternatively the CMA might work just as
> > well instead.
>
>
> We use carveout for Tegra2. Memory management is a big question mark
> still for tegradrm that I'm trying to find a solution for.
AIUI CMA is one particular implementation of the carveout concept, so I
think we should use it, or extend it if it doesn't suit us.
Thierry
Attachment:
pgpMj2vz5WGbi.pgp
Description: PGP signature
