[RFC v2 PATCH] mipi-dsi-bus: add MIPI DSI bus support

Fri Dec 13 03:21:34 PST 2013

Hi,

On 2013-12-12 14:19, Thierry Reding wrote:

> In that case I think maybe a better thing to do would be to dynamically
> assign the VC ID's perhaps using some enumeration algorithm.

Whether or not we can configure the VC IDs depends on the HW in
question. Some allow setting VC ID, or the routing, some don't. So I
don't think we can make any assumptions about this.

> Can it initiate transactions itself? Based on I2C messages that it
> receives? If so I think that would qualify it as a DSI host. If on the
> other hand it only routes DSI packets to downstream ports based on
> registers configured via I2C, then it is not a DSI host, because it
> cannot create DSI packets itself.

Yes, it can initiate DSI transactions. I agree with the above.

> Okay, so we've talked about many of these issues now and I appreciate
> you providing all that feedback. In an attempt to move things forward
> can I propose that we start off with something simple so we can get
> started and gather some experience with the matter.

Agreed.

> The most difficult part will be the DT bindings. I've sent a proposal a
> while ago[0]. It's explicitly very terse and I don't think it would
> prevent any of the more advanced setups involving hubs in the future.
> Basically it says you can have a DSI host and up to four children

Well. I don't know... On what level do we describe things in the DT? If
a host and a DSI peripheral is described based on the physical link,
then a DSI host can have only one child.

If we have a DSI hub, it's a different thing as discussed, and we can
ignore it for now.

So the case where there could be multiple children, is if a single DSI
peripheral accepts multiple VC IDs. In that case we'd have:

dsi-host {
	#address-cells = <1>;
	#size-cells = <0>;

	peripheral at 0 {
		compatible = "...";
		reg = <0>;
	};

	peripheral at 1 {
		compatible = "...";
		reg = <1>;
	};
};

I think you already expressed earlier that you think the above is ok,
and we could as well have separate drivers if a DSI peripheral accepts
multiple VC IDs.

I'm still not sure about that. I think that's more of describing logical
setup, not the hardware. But, then again, maybe that doesn't matter, as
long as the real hardware setup can be deduced from the description. In
the above case it can: we know DSI is one-to-one link on the wire level,
which means the two peripherals described above must use that same link,
and thus must be in the same DSI hardware peripheral.

<DSI hub crap>

I wanted to avoid the DSI hub discussions, but, here goes again. You can
ignore this is you're too tired already =).

If we supported DSI hubs, and the hub had two output DSI ports, and we
had a peripheral answering to two VC IDs attached to the hub. In that
case we would need separate dsi-host sub-nodes for the two DSI ports,
instead of having the peripherals directly under the hub node, as you
had in your reply.

I.e. we could have something like:

dsi-host {
	#address-cells = <1>;
	#size-cells = <0>;

	/* hub is controlled via VC ID 0 */
	hub at 0 {
		compatible = "...";
		reg = <0>;
	};

	/* video data is sent to VC ID 1 */
	hub at 1 {
		compatible = "...";
		reg = <1>;

		/* port 0 of the hub */
		dsi-host at 0 {
			reg = <0>;

			/* panel answers to VC ID 0 */
			panel at 0 {
				reg = <0>;
			};

			/* panel answers also to VC ID 1 */
			panel at 1 {
				reg = <1>;
			};
		};

		/* port 1 of the hub */
		dsi-host at 1 {
			reg = <0>;
			/* nothing attached to hub's second port in this case */
		};
	};
};

Well, that gets messy, and is probably rather unlikely scenario. But I
think that shows (at least to me) that having separate nodes for each VC
ID is problematic. I added the hub's peripherals to hub at 1 node, but
hub at 0 could also make sense.

Adding multiple values to reg field makes it a bit cleaner:

dsi-host {
	#address-cells = <1>;
	#size-cells = <0>;

	/* hub answers to VC ID 0 and 1 */
	hub at 0 {
		compatible = "...";
		reg = <0 1>;

		/* port 0 of the hub */
		dsi-host at 0 {
			reg = <0>;

			/* panel answers to VC ID 0 and 1 */
			panel at 0 {
				reg = <0 1>;
			};
		};

		/* port 1 of the hub */
		dsi-host at 1 {
			reg = <0>;
			/* nothing attached to hub's second port in this case */
		};
	};
};

I think we still need the "dsi-host" nodes in the hub, as the hub needs
to know to which of the ports the panels are connected. The ports
probably need individual configuration, and so on.

</DSI hub crap>

> attached to it. And it describes that you use the reg property to
> describe the VC ID. If we don't have a hub in there, then the global and
> link-local VC IDs will be the same anyway, so it should be safe from
> that point of view. If we ever get something that's more complicated we
> will have to revise the binding documentation and formalize much of what
> we have discussed in this thread.
>
> It would be great if you could have a look and see if you have any
> concerns with it.

My only concern is what I said above. However, if all the devices we
have only accept one VC ID, then it's not a real issue at the moment.

Maybe it would be safest to state in the binding documentation that
there can be only one peripheral, accepting one VC ID. That would work
for all (?) our current cases, and prevent anyone from adding multiple
peripherals without discussing this topic again.

Although, to be honest, even if I totally want to ignore the DSI hubs, I
would like to have an agreement how to handle a single device that
accepts multiple VC IDs. It's not critical though, so if we don't have a
clear "this one is the best"-option, I think it can also be left out.

For the sake of discussion, the other ways I see to express multiple VC
IDs would be:

dsi-host {
	#address-cells = <1>;
	#size-cells = <0>;

	peripheral at 0 {
		compatible = "...";
		/* use reg property, but have multiple */
		reg = <
			0
			1
		>;
	};
};

dsi-host {
	peripheral {
		compatible = "...";
		/* custom peripheral property, VC ID managed manually */
		vcs = <0 1>;
	};
};

All of these three options have their pros, I think. But the last one
feels most versatile, although it also needs extra code. The last one
would also allow dynamically changing VC IDs by the driver.

But, as I said, your proposed bindings look safe to me, if only one
child is allowed. It's easy to convert that in the future to either of
the two other bindings, if we so want.

> As for the code, I suggest we proceed with what Andrzej has proposed for
> now. I know that you don't see this as a good solution, but I don't
> think we'll find a common ground here and now, so we may as well proceed
> with what we have. If it really turns out to cause problems later on I'm
> sure we can rework it to meet everyone's needs. The infrastructure in
> Andrzej's patch is only a kernel internal API, so we can change it in
> whatever way we want if there's a need.

I'm fine with the bus approach. I think it works fine. I just see a bus
as an overkill, if we ever (?) just have a single child. I would add a
bus only if and when it's needed. But as you said, it's kernel internal,
can be changed later if needed.

> Yes, I think we should have a single logical bus per DSI host. All the
> link-local aspects should be hidden within drivers. I'm thinking that
> perhaps a DSI hub could be a special type of peripheral, much like a
> bridge in PCI, with a means to instantiate children of its own (yet
> still make them children of the DSI host bus) and providing a way to
> translate between link-local (physical) and logical VC IDs.

Yes, something like that sounds good to me.

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131213/532c9ed9/attachment-0001.pgp>