Best practice device tree design for display subsystems/DRM

[Inki Dae]

> -----Original Message-----
> From: dri-devel-bounces+inki.dae=samsung.com at lists.freedesktop.org
> [mailto:dri-devel-bounces+inki.dae=samsung.com at lists.freedesktop.org] On
> Behalf Of Sebastian Hesselbarth
> Sent: Wednesday, July 03, 2013 6:41 AM
> To: Daniel Drake
> Cc: Jean-Francois Moine; devicetree-discuss at lists.ozlabs.org; dri-
> devel at lists.freedesktop.org; Russell King
> Subject: Re: Best practice device tree design for display subsystems/DRM
> 
> On 07/02/2013 11:04 PM, Daniel Drake wrote:
> > On Tue, Jul 2, 2013 at 1:57 PM, Sebastian Hesselbarth
> > <sebastian.hesselbarth at gmail.com>  wrote:
> >> I am against a super node which contains lcd and dcon/ire nodes. You
> can
> >> enable those devices on a per board basis. We add them to dove.dtsi but
> >> disable them by default (status = "disabled").
> >>
> >> The DRM driver itself should get a video-card node outside of
> >> soc/internal-regs where you can put e.g. video memory hole (or video
> >> mem size if it will be taken from RAM later)
> >
> > For completeness of the discussion, and my understanding too, can you
> > explain your objections to the display super-node in a bit more
> > detail?
> 
> lcd-controller nodes and dcon node will need to be children of
> internal-regs nodes. The internal-regs node is required for address
> translation as the mbus base address can be configured. The above does
> not permit a super-node - but you cannot have the nodes above outside
> of internal-regs.
> 
> As Russell stated, he wants a proposal for the "unusual case" i.e.
> you have two lcd controllers. You use one for Xorg and the other for
> e.g. running a linux terminal console.
> 
> This would require some reference from the super node to the lcd
> controller to sort out which DRM device (represented by the super
> node) should be using which lcd controller device.
> 
> Using status = "disabled" alone will only allow to enable or disable
> lcd controller nodes but not assign any of it to your two super-nodes.
> 
> So my current proposal after thinking about Russell's statements
> whould be phandles as Jean-Francois already mentioned. I am not sure
> what OF maintainers will think about it, but that is another thing.
> 
> Basically, you will have:
> (Note: names and property-names are just to show how it could work,
> and example is joined from possible future dove.dtsi and dove-board.dts)
> 
> video {
> 	/* Single video card w/ multiple lcd controllers */
> 	card0 {
> 		compatible = "marvell,armada-510-display";
> 		reg = <0 0x3f000000 0x1000000>; /* video-mem hole */
> 		/* later: linux,video-memory-size = <0x1000000>; */
> 		marvell,video-devices = <&lcd0 &lcd1 &dcon>;
> 	};
> 
> 	/* OR: Multiple video card w/ single lcd controllers */
> 	card0 {
> 		compatible = "marvell,armada-510-display";
> 		...
> 		marvell,video-devices = <&lcd0>;
> 	};
> 
> 	card1 {
> 		compatible = "marvell,armada-510-display";
> 		...
> 		marvell,video-devices = <&lcd1>;
> 	};
> };

Sorry but I'd like to say that this cannot be used commonly. Shouldn't you
really consider Linux framebuffer or other subsystems? The above dtsi file
is specific to DRM subsystem. And I think the dtsi file has no any
dependency on certain subsystem so board dtsi file should be considered for
all device drivers based on other subsystems: i.e., Linux framebuffer, DRM,
and so no. So I *strongly* object to it. All we have to do is to keep the
dtsi file as is, and to find other better way that can be used commonly in
DRM.

Thanks,
Inki Dae

> 
> mbus {
> 	compatible = "marvell,dove-mbus";
> 	ranges = <...>;
> 
> 	sb-regs {
> 		ranges = <0 0xf1000000 0 0x100000>;
> 		...
> 	}
> 
> 	nb-regs {
> 		ranges = <0 0xf1800000 0 0x100000>;
> 
> 		lcd0: lcd-controller at 20000 {
> 			compatible = "marvell,armada-510-lcd";
> 			reg = <0x20000 0x1000>;
> 			interrupts = <47>;
> 			...
> 			/* use EXTCLK0 with lcd0 */
> 			clocks = <&ext_clk0>;
> 			clock-names = "extclk0";
> 			marvell,external-encoder = <&tda998x>;
> 		};
> 
> 		lcd1: lcd-controller at 10000 {
> 			compatible = "marvell,armada-510-lcd";
> 			reg = <0x10000 0x1000>;
> 			interrupts = <46>;
> 			...
> 			/* use LCDPLL with lcd1 */
> 			clocks = <&lcd_pll_clk>;
> 			clock-names = "lcdpll";
> 		};
> 	}
> };
> 
> &i2c0 {
> 	tda998x: hdmi-transmitter at 60 {
> 		compatible = "nxp,tda19988";
> 		reg = <0x60>;
> 		...
> 	}
> };
> 
> Each lcd controller node represents a platform_device and the display
> nodes above should look up phandles and determine type (ctrc or dcon)
> by compatible string of the nodes the phandles point to.
> 
> Sebastian
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel