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

[Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3661 bytes --]

* Stephen Warren wrote:
> On 04/12/2012 12:50 AM, Thierry Reding wrote:
> > 	drm {
> > 		compatible = "nvidia,tegra20-drm";
> 
> I'm don't think having an explicit "drm" node is the right approach; drm
> is after all a SW term and the DT should be describing HW. Having some
> kind of top-level node almost certainly makes sense, but naming it
> something related to "tegra display" than "drm" would be appropriate.

In this case there really isn't a HW device that can be represented. But in
the end it's still the DRM driver that needs to bind to the device. However
the other graphics devices (MPE, VI/CSI, EPP, GR2D and GR3D) probably need
to be bound against.

Would it be possible for someone at NVIDIA to provide some more details about
what those other devices are? GR2D and GR3D seem obvious, MPE might be video
decoding, VI/CSI video input and camera interface? As to EPP I have no idea.

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.

> > 		lvds {
> > 			compatible = "...";
> > 			dc = <&disp1>;
> > 		};
> 
> Aren't the outputs separate HW blocks too, such that they would have
> their own compatible/reg properties and their own drivers, and be
> outside the top-level drm/display node?

The RGB output is programmed via the display controller registers. For HDMI,
TVO and DSI there are indeed separate sets of registers in addition to the
display controller's. So perhaps for those more nodes would be required:

	hdmi : hdmi@54280000 {
		compatible = "nvidia,tegra20-hdmi";
		reg = <0x54280000 0x00040000>;
	};

And hook that up with the HDMI output node of the "DRM" node:

	drm {
		hdmi {
			compatible = "...";
			connector = <&hdmi>;
			dc = <&disp2>;
		};
	};

Maybe with this setup we no longer need the "compatible" property since it
will already be inherent in the "connector" property. There will have to be
special handling for the RGB output, which could be the default if the
"connector" property is missing.

> I believe the mapping between the output this node represents and the
> display controller ("dc" above) that it uses is not static; the
> connectivity should be set up at runtime, and possibly dynamically
> alterable via xrandr or equivalent.

I think the mapping is always static for a given board. There is no way to
switch an HDMI port to LVDS at runtime. But maybe I misunderstand what you're
saying.

> >> Instead, the active platform data should probably be stored in a
> >> tegra_drm struct that's stored in the dev's private data.
> >> tegra_drm_probe() might then look more like:
> >>
> >> struct tegra_drm *tdev;
> >>
> >> tdev = devm_kzalloc();
> >> tdev->pdata = pdev->dev.platform_data;
> >> if (!tdev->pdata)
> >>     tdev->pdata = tegra_drm_parse_dt();
> >> if (!tdev->pdata)
> >>     return -EINVAL;
> >>
> >> dev_set_drvdata(dev, tdev);
> >>
> >> This is safe, since probe() will never assume that dev_get_drvdata()
> >> might contain something valid before probe() sets it.
> > 
> > I prefer my approach over storing the data in an extra field because the
> > device platform_data field is where everybody would expect it. Furthermore
> > this wouldn't be relevant if we decided not to support non-DT setups.
> 
> Drivers are expected to use pre-existing platform data, if provided.
> This might happen in order to work around bugs in device tree content.

Okay I see. I'll have to store it in a separate field in the private
structure then.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 190 bytes --]

_______________________________________________
iommu mailing list
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu