Re: [RFR 2/2] drm/panel: Add simple panel support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thu, Oct 17, 2013 at 02:23:32PM +0200, Laurent Pinchart wrote:
> Hi Thierry,
> 
> On Thursday 17 October 2013 14:06:47 Thierry Reding wrote:
> > On Thu, Oct 17, 2013 at 01:15:22PM +0200, Laurent Pinchart wrote:
> > > On Thursday 17 October 2013 13:05:18 Thierry Reding wrote:
> > > > On Thu, Oct 17, 2013 at 01:22:21PM +0300, Tomi Valkeinen wrote:
> > > > > On 17/10/13 11:53, Thierry Reding wrote:
> > > > > > I keep wondering why we absolutely must have compatibility between
> > > > > > CDF and this simple panel binding. DT content is supposed to concern
> > > > > > itself with the description of hardware only. What about the
> > > > > > following isn't an accurate description of the panel hardware?
> > > > > > 
> > > > > > 	panel: panel {
> > > > > > 		compatible = "cptt,claa101wb01";
> > > > > > 		
> > > > > > 		power-supply = <&vdd_pnl_reg>;
> > > > > > 		enable-gpios = <&gpio 90 0>;
> > > > > > 		
> > > > > > 		backlight = <&backlight>;
> > > > > > 	};
> > > > > > 	
> > > > > > 	dsi-controller {
> > > > > > 		panel = <&panel>;
> > > > > > 	};
> > > > > 
> > > > > That's quite similar to how my current out-of-mainline OMAP DSS DT
> > > > > bindings work. The difference is that I have a reference from the
> > > > > panel node to the video source (dsi-controller), instead of the other
> > > > > way around. I just find it more natural. It works the same way as,
> > > > > say, regulators, gpios, etc.
> > > > 
> > > > I suppose that depends on the way you look at it. In the above proposal
> > > > I consider the output (DSI controller) to use the panel as a resource
> > > > that provides a certain set of services (query mode timings, provide a
> > > > way to enable or disable the panel). Similarly the panel uses the
> > > > backlight as a resource that provides a certain set of services (such as
> > > > changing the brightness).
> > > > 
> > > > The above also follows the natural order of control. The panel has no
> > > > way to control the DSI output. However, it is the output that controls
> > > > when a panel is required and turn it on as appropriate.
> > > 
> > > I'm no DSI expert, but I know enough about it to be sure that Tomi will
> > > disagree. DSI panels can have complex power sequences that require the
> > > input stream to be finely controlled (for instance it might require a
> > > clock without video frames for some time, switch a GPIO or regulator,
> > > send a command to the panel, and then only get video frames). For that
> > > reason all developers I've talked to who have an in-depth knowledge of
> > > DSI and DSI panels told me that the panel needs to control the video bus,
> > > and request the video source to start/stop the video stream.
> > 
> > Oh, I'm very well aware of the various flavours of funkiness that DSI
> > panels come in. But it's wrong to say that the panel needs to control
> > the video bus. There's simply no way that a panel can actually do that.
> > It is true, however, that in order to make this work in a maintainable
> > fashion, the DSI panel *driver* may need to control the DSI bus. That's
> > an entirely different story.
> 
> Sure, but I don't think that's really related to the DT bindings. We don't 
> have to model every electrical signal in a detailed way that matches the 
> direction of the electrons flow :-) What we need to model is a connection 
> between a display controller and a panel (possibly with a direction). What I'd 
> like to do is to express that link in a way that can also express more complex 
> pipeline topologies. I don't want to make it overly complex, I had hoped that 
> my DT bindings proposal would be a good approach as it's both generic and 
> still pretty simple.

I get that, and for what it's worth I do think that your proposal looks
simple enough and if it can solve any of the problem you're facing with
CDF, then that's great.

But I don't think we should force inclusion of these properties on every
panel, even if it doesn't use any of the graph functionality. Is there
any problem with making them optional?

Thierry

Attachment: pgp_yVWhBTmT_.pgp
Description: PGP signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux