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

5 messages, 5 authors, 2013-10-25 · open the first message on its own page

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

From: Laurent Pinchart <hidden>
Date: 2013-10-17 12:17:17

Hi Tomi,

On Thursday 17 October 2013 14:59:41 Tomi Valkeinen wrote:
On 17/10/13 14:51, Laurent Pinchart wrote:
quoted
quoted
I'm not sure if there's a specific need for the port or endpoint nodes
in cases like the above. Even if we have common properties describing
the endpoint, I guess they could just be in the parent node.

panel {
	remote = <&dc>;
	common-video-property = <asd>;
};

The above would imply one port and one endpoint. Would that work? If we
had a function like parse_endpoint(node), we could just point it to
either a real endpoint node, or to the device's node.
You reference the display controller here, not a specific display
controller output. Don't most display controllers have several outputs ?
Sure. Then the display controller could have more verbose description.
But the panel could still have what I wrote above, except the 'remote'
property would point to a real endpoint node inside the dispc node, not
to the dispc node.

This would, of course, need some extra code to handle the different
cases, but just from DT point of view, I think all the relevant
information is there.
There's many ways to describe the same information in DT. While we could have 
DT bindings that use different descriptions for different devices and still 
support all of them in our code, why should we opt for that option that will 
make the implementation much more complex when we can describe connections in 
a simple and generic way ?

-- 
Regards,

Laurent Pinchart

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

From: Tomi Valkeinen <hidden>
Date: 2013-10-17 12:32:29

On 17/10/13 15:17, Laurent Pinchart wrote:
Hi Tomi,

On Thursday 17 October 2013 14:59:41 Tomi Valkeinen wrote:
quoted
On 17/10/13 14:51, Laurent Pinchart wrote:
quoted
quoted
I'm not sure if there's a specific need for the port or endpoint nodes
in cases like the above. Even if we have common properties describing
the endpoint, I guess they could just be in the parent node.

panel {
	remote = <&dc>;
	common-video-property = <asd>;
};

The above would imply one port and one endpoint. Would that work? If we
had a function like parse_endpoint(node), we could just point it to
either a real endpoint node, or to the device's node.
You reference the display controller here, not a specific display
controller output. Don't most display controllers have several outputs ?
Sure. Then the display controller could have more verbose description.
But the panel could still have what I wrote above, except the 'remote'
property would point to a real endpoint node inside the dispc node, not
to the dispc node.

This would, of course, need some extra code to handle the different
cases, but just from DT point of view, I think all the relevant
information is there.
There's many ways to describe the same information in DT. While we could have 
DT bindings that use different descriptions for different devices and still 
support all of them in our code, why should we opt for that option that will 
make the implementation much more complex when we can describe connections in 
a simple and generic way ?
My suggestion was simple and generic. I'm not proposing per-device
custom bindings.

My point was, if we can describe the connections as I described above,
which to me sounds possible, we can simplify the panel DT data for 99.9%
of the cases.

To me, the first of these looks much nicer:

panel {
	remote = <&remote-endpoint>;
	common-video-property = <asd>;
};

panel {
	port {
		endpoint {
			remote = <&remote-endpoint>;
			common-video-property = <asd>;
		};
	};
};

If that can be supported in the SW by adding complexity to a few
functions, and it covers practically all the panels, isn't it worth it?

Note that I have not tried this, so I don't know if there are issues.
It's just a thought. Even if there's need for a endpoint node, perhaps
the port node can be made optional.

 Tomi

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

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2013-10-24 10:40:58

Hi Tomi,

On Thursday 17 October 2013 15:32:29 Tomi Valkeinen wrote:
On 17/10/13 15:17, Laurent Pinchart wrote:
quoted
On Thursday 17 October 2013 14:59:41 Tomi Valkeinen wrote:
quoted
On 17/10/13 14:51, Laurent Pinchart wrote:
quoted
quoted
I'm not sure if there's a specific need for the port or endpoint nodes
in cases like the above. Even if we have common properties describing
the endpoint, I guess they could just be in the parent node.

panel {
	remote = <&dc>;
	common-video-property = <asd>;
};

The above would imply one port and one endpoint. Would that work? If we
had a function like parse_endpoint(node), we could just point it to
either a real endpoint node, or to the device's node.
You reference the display controller here, not a specific display
controller output. Don't most display controllers have several outputs ?
Sure. Then the display controller could have more verbose description.
But the panel could still have what I wrote above, except the 'remote'
property would point to a real endpoint node inside the dispc node, not
to the dispc node.

This would, of course, need some extra code to handle the different
cases, but just from DT point of view, I think all the relevant
information is there.
There's many ways to describe the same information in DT. While we could
have DT bindings that use different descriptions for different devices
and still support all of them in our code, why should we opt for that
option that will make the implementation much more complex when we can
describe connections in a simple and generic way ?
My suggestion was simple and generic. I'm not proposing per-device
custom bindings.

My point was, if we can describe the connections as I described above,
which to me sounds possible, we can simplify the panel DT data for 99.9%
of the cases.

To me, the first of these looks much nicer:

panel {
	remote = <&remote-endpoint>;
	common-video-property = <asd>;
};

panel {
	port {
		endpoint {
			remote = <&remote-endpoint>;
			common-video-property = <asd>;
		};
	};
};
Please note that the common video properties would be in the panel node, not 
in the endpoint node (unless you have specific requirements to do so, which 
isn't the common case).
If that can be supported in the SW by adding complexity to a few functions,
and it covers practically all the panels, isn't it worth it?

Note that I have not tried this, so I don't know if there are issues.
It's just a thought. Even if there's need for a endpoint node, perhaps
the port node can be made optional.
It can be worth it, as long as we make sure that simplified bindings cover the 
needs of the generic code.

We could assume that, if the port subnode isn't present, the device will have 
a single port, with a single endpoint. However, isn't the number of endpoints 
a system property rather than a device property ? If a port of a device is 
connected to two remote ports it will require two endpoints. We could select 
the simplified or full bindings based on the system topology though.

I've CC'ed Sylwester Nawrocki and Guennadi Liakhovetski, the V4L2 DT bindings 
authors, as well as the linux-media list, to get their opinion on this.

-- 
Regards,

Laurent Pinchart

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

From: Tomi Valkeinen <hidden>
Date: 2013-10-24 10:52:19

On 24/10/13 13:40, Laurent Pinchart wrote:
quoted
panel {
	remote = <&remote-endpoint>;
	common-video-property = <asd>;
};

panel {
	port {
		endpoint {
			remote = <&remote-endpoint>;
			common-video-property = <asd>;
		};
	};
};
Please note that the common video properties would be in the panel node, not 
in the endpoint node (unless you have specific requirements to do so, which 
isn't the common case).
Hmm, well, the panel driver must look for its properties either in the
panel node, or in the endpoint node (I guess it could look them from
both, but that doesn't sound good).

If you write the panel driver, and in all your cases the properties work
fine in the panel node, does that mean they'll work fine with everybody?

I guess there are different kinds of properties. Something like a
regulator is obviously property of the panel. But anything related to
the video itself, like DPI's bus width, or perhaps even something like
"orientation" if the panel supports such, could need to be in the
endpoint node.

But yes, I understand what you mean. With "common-video-property" I
meant common properties like DPI bus width.
quoted
If that can be supported in the SW by adding complexity to a few functions,
and it covers practically all the panels, isn't it worth it?

Note that I have not tried this, so I don't know if there are issues.
It's just a thought. Even if there's need for a endpoint node, perhaps
the port node can be made optional.
It can be worth it, as long as we make sure that simplified bindings cover the 
needs of the generic code.

We could assume that, if the port subnode isn't present, the device will have 
a single port, with a single endpoint. However, isn't the number of endpoints 
Right.
a system property rather than a device property ? If a port of a device is 
Yes.
connected to two remote ports it will require two endpoints. We could select 
the simplified or full bindings based on the system topology though.
The drivers should not know about simplified/normal bindings. They
should use CDF DT helper functions to get the port and endpoint
information. The helper functions would do the assuming.

 Tomi

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

From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
Date: 2013-10-25 10:54:36

On 10/24/2013 12:52 PM, Tomi Valkeinen wrote:
On 24/10/13 13:40, Laurent Pinchart wrote:
quoted
quoted
panel {
	remote =<&remote-endpoint>;
	common-video-property =<asd>;
};

panel {
	port {
		endpoint {
			remote =<&remote-endpoint>;
			common-video-property =<asd>;
		};
	};
};
Please note that the common video properties would be in the panel node, not
in the endpoint node (unless you have specific requirements to do so, which
isn't the common case).
Hmm, well, the panel driver must look for its properties either in the
panel node, or in the endpoint node (I guess it could look them from
both, but that doesn't sound good).
Presumably the OS could be searching for port node and any endpoint node
inside it first. If that's not found then it could be parsing the panel
node.

Please note that a port node may be required even if there is only one
port, when there are multiple physical bus interfaces, e.g. at an LCD
controller and only one of them is used. The reg property would select
the physical bus interface.

I wonder if a property like #video-port or #video-endpoint could be used
to indicate that a node contains video bus properties. Probably it's too
late to introduce it now and make it a required property for the endpoint
nodes or nodes containing the common video properties.
If you write the panel driver, and in all your cases the properties work
fine in the panel node, does that mean they'll work fine with everybody?
It's likely not safe to assume so. In V4L data bus properties are specified
a both the receiver and the transmitter endpoint nodes separately.
I guess there are different kinds of properties. Something like a
regulator is obviously property of the panel. But anything related to
the video itself, like DPI's bus width, or perhaps even something like
"orientation" if the panel supports such, could need to be in the
endpoint node.
If we use port/endpoint nodes it all seems clear, the video bus properties
are put in an endpoint node.

But since we are considering a simplified binding all the properties would
be placed in the panel or display controller node.
But yes, I understand what you mean. With "common-video-property" I
meant common properties like DPI bus width.
quoted
quoted
If that can be supported in the SW by adding complexity to a few functions,
and it covers practically all the panels, isn't it worth it?

Note that I have not tried this, so I don't know if there are issues.
It's just a thought. Even if there's need for a endpoint node, perhaps
the port node can be made optional.
It can be worth it, as long as we make sure that simplified bindings cover the
needs of the generic code.

We could assume that, if the port subnode isn't present, the device will have
a single port, with a single endpoint. However, isn't the number of endpoints
Right.
quoted
a system property rather than a device property ? If a port of a device is
Yes.
quoted
connected to two remote ports it will require two endpoints. We could select
the simplified or full bindings based on the system topology though.
Yes, I guess it's all about the system topology. Any simplified binding 
would
work only for very simple configuration like single-output LCD 
controller with
single panel attached to it.
The drivers should not know about simplified/normal bindings. They
should use CDF DT helper functions to get the port and endpoint
information. The helper functions would do the assuming.
Yes, anyway all the parsing is supposed to be done within the helpers.

--
Thanks,
Sylwester
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help