diff mbox

[16/26] ARM: omap4-sdp.dts: add display information

Message ID 1386160133-24026-17-git-send-email-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomi Valkeinen Dec. 4, 2013, 12:28 p.m. UTC
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/boot/dts/omap4-sdp.dts | 91 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

Comments

archit taneja Dec. 13, 2013, 9:27 a.m. UTC | #1
On Wednesday 04 December 2013 05:58 PM, Tomi Valkeinen wrote:
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   arch/arm/boot/dts/omap4-sdp.dts | 91 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 91 insertions(+)
>
> diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
> index 5fc3f43c5a81..e3048f849612 100644
> --- a/arch/arm/boot/dts/omap4-sdp.dts
> +++ b/arch/arm/boot/dts/omap4-sdp.dts
> @@ -550,3 +550,94 @@
>   	mode = <3>;
>   	power = <50>;
>   };
> +
> +&dsi1 {
> +	vdds_dsi-supply = <&vcxio>;
> +
> +	dsi1_out_ep: endpoint {
> +		remote-endpoint = <&lcd0_in>;
> +		lanes = <0 1 2 3 4 5>;

In the previous revision omapdss DT patchset, the lanes node was a 
member of the panel DT node, and not the dsi DT node. Any reason to 
change this? Does it make more sense this way?

I suppose it's more suitable for dsi to hold the property if 2 panels 
are connected on the same bus. Say, one with 4 data lanes, and other 
with 2. It would be tricky for the dsi driver to get lane params from 2 
different places and merge them somehow.

> +	};
> +
> +	lcd0: display@0 {
> +		compatible = "tpo,taal", "panel-dsi-cm";
> +
> +		gpios = <&gpio4 6 0>;	/* 102, reset */
> +
> +		lcd0_in: endpoint {
> +			remote-endpoint = <&dsi1_out_ep>;
> +		};
> +	};

Is there a reason why lcd0 and lcd1 are children nodes of dsi1 and dsi2 
respectively? I don't see this for panels on other boards.

Archit

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen Dec. 13, 2013, 9:39 a.m. UTC | #2
On 2013-12-13 11:27, Archit Taneja wrote:
> On Wednesday 04 December 2013 05:58 PM, Tomi Valkeinen wrote:
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   arch/arm/boot/dts/omap4-sdp.dts | 91
>> +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 91 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/omap4-sdp.dts
>> b/arch/arm/boot/dts/omap4-sdp.dts
>> index 5fc3f43c5a81..e3048f849612 100644
>> --- a/arch/arm/boot/dts/omap4-sdp.dts
>> +++ b/arch/arm/boot/dts/omap4-sdp.dts
>> @@ -550,3 +550,94 @@
>>       mode = <3>;
>>       power = <50>;
>>   };
>> +
>> +&dsi1 {
>> +    vdds_dsi-supply = <&vcxio>;
>> +
>> +    dsi1_out_ep: endpoint {
>> +        remote-endpoint = <&lcd0_in>;
>> +        lanes = <0 1 2 3 4 5>;
> 
> In the previous revision omapdss DT patchset, the lanes node was a
> member of the panel DT node, and not the dsi DT node. Any reason to
> change this? Does it make more sense this way?

Well, the lane configuration is programmed into the DSI HW. So DSI needs
to know them. Thus the lanes can be considered a property of the DSI.

In some cases, the external encoder or panel also needs to know about
the lanes. In that case, both DSI and the encoder/panel would contain
the same data.

My reasoning where a property belongs to:

If a property is clearly internal to a device, it belongs there. For
example, in this case vdds_dsi-supply is clearly a property of the DSI.

If a property is about the link between two devices, like "lanes" here,
it belongs to both devices. If a device does not need that data for
anything, it can be omitted.

> I suppose it's more suitable for dsi to hold the property if 2 panels
> are connected on the same bus. Say, one with 4 data lanes, and other
> with 2. It would be tricky for the dsi driver to get lane params from 2
> different places and merge them somehow.

It doesn't matter, both would work fine. If the lanes property is in the
DSI node, then the DSI driver finds out the lane config by finding out
which endpoint is used for the panel that's being enabled.

If the lanes property would be in the panel, the panel would pass the
lane config to the DSI when it's enabled.

But I think passing the lane config from panel to DSI (like we do
currently) is not so nice.

>> +    };
>> +
>> +    lcd0: display@0 {
>> +        compatible = "tpo,taal", "panel-dsi-cm";
>> +
>> +        gpios = <&gpio4 6 0>;    /* 102, reset */
>> +
>> +        lcd0_in: endpoint {
>> +            remote-endpoint = <&dsi1_out_ep>;
>> +        };
>> +    };
> 
> Is there a reason why lcd0 and lcd1 are children nodes of dsi1 and dsi2
> respectively? I don't see this for panels on other boards.

Yes. The panels are _controlled_ with DSI. We model the child-parent
relationships in DT data based on the control. So an i2c peripheral is
controlled via i2c master, and is thus a child of the i2c master. Same
here. The ports/endpoints are used to define the data path, which is
separate thing from the control path.

DPI panels which don't have any way to control them (except basic things
like gpios) are platform devices without any parent.

If the DPI panel would be controlled with i2c, it'd be a child of an i2c
master.

 Tomi
archit taneja Dec. 13, 2013, 9:58 a.m. UTC | #3
On Friday 13 December 2013 03:09 PM, Tomi Valkeinen wrote:
> On 2013-12-13 11:27, Archit Taneja wrote:
>> On Wednesday 04 December 2013 05:58 PM, Tomi Valkeinen wrote:
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> ---
>>>    arch/arm/boot/dts/omap4-sdp.dts | 91
>>> +++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 91 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/omap4-sdp.dts
>>> b/arch/arm/boot/dts/omap4-sdp.dts
>>> index 5fc3f43c5a81..e3048f849612 100644
>>> --- a/arch/arm/boot/dts/omap4-sdp.dts
>>> +++ b/arch/arm/boot/dts/omap4-sdp.dts
>>> @@ -550,3 +550,94 @@
>>>        mode = <3>;
>>>        power = <50>;
>>>    };
>>> +
>>> +&dsi1 {
>>> +    vdds_dsi-supply = <&vcxio>;
>>> +
>>> +    dsi1_out_ep: endpoint {
>>> +        remote-endpoint = <&lcd0_in>;
>>> +        lanes = <0 1 2 3 4 5>;
>>
>> In the previous revision omapdss DT patchset, the lanes node was a
>> member of the panel DT node, and not the dsi DT node. Any reason to
>> change this? Does it make more sense this way?
>
> Well, the lane configuration is programmed into the DSI HW. So DSI needs
> to know them. Thus the lanes can be considered a property of the DSI.
>
> In some cases, the external encoder or panel also needs to know about
> the lanes. In that case, both DSI and the encoder/panel would contain
> the same data.
>
> My reasoning where a property belongs to:
>
> If a property is clearly internal to a device, it belongs there. For
> example, in this case vdds_dsi-supply is clearly a property of the DSI.
>
> If a property is about the link between two devices, like "lanes" here,
> it belongs to both devices. If a device does not need that data for
> anything, it can be omitted.
>
>> I suppose it's more suitable for dsi to hold the property if 2 panels
>> are connected on the same bus. Say, one with 4 data lanes, and other
>> with 2. It would be tricky for the dsi driver to get lane params from 2
>> different places and merge them somehow.
>
> It doesn't matter, both would work fine. If the lanes property is in the
> DSI node, then the DSI driver finds out the lane config by finding out
> which endpoint is used for the panel that's being enabled.
>
> If the lanes property would be in the panel, the panel would pass the
> lane config to the DSI when it's enabled.
>
> But I think passing the lane config from panel to DSI (like we do
> currently) is not so nice.

Okay, that makes sense.

>
>>> +    };
>>> +
>>> +    lcd0: display@0 {
>>> +        compatible = "tpo,taal", "panel-dsi-cm";
>>> +
>>> +        gpios = <&gpio4 6 0>;    /* 102, reset */
>>> +
>>> +        lcd0_in: endpoint {
>>> +            remote-endpoint = <&dsi1_out_ep>;
>>> +        };
>>> +    };
>>
>> Is there a reason why lcd0 and lcd1 are children nodes of dsi1 and dsi2
>> respectively? I don't see this for panels on other boards.
>
> Yes. The panels are _controlled_ with DSI. We model the child-parent
> relationships in DT data based on the control. So an i2c peripheral is
> controlled via i2c master, and is thus a child of the i2c master. Same
> here. The ports/endpoints are used to define the data path, which is
> separate thing from the control path.
>
> DPI panels which don't have any way to control them (except basic things
> like gpios) are platform devices without any parent.
>
> If the DPI panel would be controlled with i2c, it'd be a child of an i2c
> master.

Ah, I thought the port/endpoint stuff had something to do with this. I 
forgot about the parent-child relationship for the control path.

In that case, for the sake of accuracy, the dsi-cm panel could get the 
"in" parameter via the parent node wherever it's used for control, like 
setting a DCS command for sleep out. And via 
omapdss_of_find_source_for_first_ep() when it's used to start data 
transfer, even though both the "in's" are finally the same dsi device?

Archit

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen Dec. 13, 2013, 10:15 a.m. UTC | #4
On 2013-12-13 11:58, Archit Taneja wrote:

>>>> +    };
>>>> +
>>>> +    lcd0: display@0 {
>>>> +        compatible = "tpo,taal", "panel-dsi-cm";
>>>> +
>>>> +        gpios = <&gpio4 6 0>;    /* 102, reset */
>>>> +
>>>> +        lcd0_in: endpoint {
>>>> +            remote-endpoint = <&dsi1_out_ep>;
>>>> +        };
>>>> +    };
>>>
>>> Is there a reason why lcd0 and lcd1 are children nodes of dsi1 and dsi2
>>> respectively? I don't see this for panels on other boards.
>>
>> Yes. The panels are _controlled_ with DSI. We model the child-parent
>> relationships in DT data based on the control. So an i2c peripheral is
>> controlled via i2c master, and is thus a child of the i2c master. Same
>> here. The ports/endpoints are used to define the data path, which is
>> separate thing from the control path.
>>
>> DPI panels which don't have any way to control them (except basic things
>> like gpios) are platform devices without any parent.
>>
>> If the DPI panel would be controlled with i2c, it'd be a child of an i2c
>> master.
> 
> Ah, I thought the port/endpoint stuff had something to do with this. I
> forgot about the parent-child relationship for the control path.
> 
> In that case, for the sake of accuracy, the dsi-cm panel could get the
> "in" parameter via the parent node wherever it's used for control, like
> setting a DCS command for sleep out. And via
> omapdss_of_find_source_for_first_ep() when it's used to start data
> transfer, even though both the "in's" are finally the same dsi device?

Don't mix the DT data and the current driver =). The current driver does
not handle things properly. The driver still uses the current model,
where we don't have separate control and data path handling. I.e. both
control and data are handled via the single API, using the "in" field.

The important thing with this DT data is that in the future we can
change the driver model, if we so want, to manage data and control
separately. Or maybe add a DSI bus, as has been proposed elsewhere.

It's true that we could change the driver as you describe, but I don't
think it helps anything, as the current model in the driver only has a
single control-data path.

 Tomi
diff mbox

Patch

diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 5fc3f43c5a81..e3048f849612 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -550,3 +550,94 @@ 
 	mode = <3>;
 	power = <50>;
 };
+
+&dsi1 {
+	vdds_dsi-supply = <&vcxio>;
+
+	dsi1_out_ep: endpoint {
+		remote-endpoint = <&lcd0_in>;
+		lanes = <0 1 2 3 4 5>;
+	};
+
+	lcd0: display@0 {
+		compatible = "tpo,taal", "panel-dsi-cm";
+
+		gpios = <&gpio4 6 0>;	/* 102, reset */
+
+		lcd0_in: endpoint {
+			remote-endpoint = <&dsi1_out_ep>;
+		};
+	};
+};
+
+&dsi2 {
+	vdds_dsi-supply = <&vcxio>;
+
+	dsi2_out_ep: endpoint {
+		remote-endpoint = <&lcd1_in>;
+		lanes = <0 1 2 3 4 5>;
+	};
+
+	lcd1: display@1 {
+		compatible = "tpo,taal", "panel-dsi-cm";
+
+		gpios = <&gpio4 8 0>;	/* 104, reset */
+
+		lcd1_in: endpoint {
+			remote-endpoint = <&dsi2_out_ep>;
+		};
+	};
+};
+
+&hdmi {
+	vdda_hdmi_dac-supply = <&vdac>;
+
+	hdmi_out: endpoint {
+		remote-endpoint = <&tpd12s015_in>;
+	};
+};
+
+/ {
+	aliases {
+		display0 = &lcd0;
+		display1 = &lcd1;
+		display2 = &hdmi0;
+	};
+
+	tpd12s015: encoder@0 {
+		compatible = "ti,tpd12s015";
+
+		gpios = <&gpio2 28 0>,	/* 60, CT CP HPD */
+			<&gpio2 9 0>,	/* 41, LS OE */
+			<&gpio2 31 0>;	/* 63, HPD */
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				tpd12s015_in: endpoint@0 {
+					remote-endpoint = <&hdmi_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+
+				tpd12s015_out: endpoint@1 {
+					remote-endpoint = <&hdmi_connector_in>;
+				};
+			};
+		};
+	};
+
+	hdmi0: connector@0 {
+		compatible = "ti,hdmi-connector";
+
+		hdmi_connector_in: endpoint {
+			remote-endpoint = <&tpd12s015_out>;
+		};
+	};
+};