On 11/30/2017 11:02 PM, Nickey Yang wrote:
> Hi Archit,
>
>
> On 2017?10?26? 12:53, Archit Taneja wrote:
>>
>>
>> On 10/25/2017 09:21 AM, Nickey Yang wrote:
>>> Configure dsi slave channel when driving a panel
>>> which needs 2 DSI links.
>>>
>>> Signed-off-by: Nickey Yang <nickey.yang at rock-chips.com>
>>> ---
>>> .../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 2 ++
>>> ? 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
>>> b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
>>> index 6bb59ab..a2bea22 100644
>>> --- a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
>>> +++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt
>>> @@ -19,6 +19,8 @@ Optional properties:
>>> ? - power-domains: a phandle to mipi dsi power domain node.
>>> ? - resets: list of phandle + reset specifier pairs, as described in [3].
>>> ? - reset-names: string reset name, must be "apb".
>>> +- rockchip,dual-channel: phandle to a 2nd DSI channel, useful as a slave
>>> +channel when driving a panel which needs 2 DSI links.
>> The example below is how dual DSI bindings could look like. Let me know what
>> you think of it.
>>
>> If both DSI outputs drive the same device (i.e, point to the same panel DT
>> node), then I think it's reasonable enough to assume that the DSIs are
>> operating in a 'dual-channel' mode. That being said, we still need DT to
>> describe which of the DSIs generates the clock for both the channels. This
>> is done with the 'clock-master' DT binding.
>>
>> Thanks,
>> Archit
>>
>> mipi_dsi: mipi at ff960000 {
>> ????...
>> ????...
>>
>> ????clock-master;??? /* implies that this DSI instance drivers the clock
>> ???????????? * for both the DSIs.
>> ???????????? */
>>
>> ????ports {
>> ??????? mipi_in: port {
>> ??????????? ...
>> ??????????? ...
>> ??????? };
>>
>> ??????? /* add extra output ports for both DSIs */
>> ??????? mipi_out: port {
>> ??????????? mipi_panel_out: endpoint {
>> ??????????????? remote-endpoint = <&panel_in_channel0>;
>> ??????????? };
>> ??????? };
>> ????};
>>
>> ????panel {
>> ??????? ...
>> ??????? ...
>> ??????? /*
>> ???????? * panel node can describe its input ports, if both the DSIs output
>> ???????? * ports are connected to the same device (i.e, the same DSI panel),
>> ???????? * we can assume that the DSIs need to operate in dual DSI mode
>> ???????? */
>> ??????? ports {
>> ??????????? ...
>> ??????????? port at 0 {
>> ??????????????? panel_in_channel0: endpoint {
>> ??????????????????? remote-endpoint = <&mipi_panel_out>;
>> ??????????????? };
>> ??????????? };
>>
>> ??????????? port at 1 {
>> ??????????????? panel_in_channel1: endpoint {
>> ??????????????????? remote-endpoint = <&mipi1_panel_out>;
>> ??????????????? };
>>
>> ??????????? };
>> ??????? };
>> ????};
>> };
>>
>>
>> mipi_dsi1: mipi at ff968000 {
>> ????...
>> ????...
>>
>> ????ports {
>> ??????? mipi1_in: port {
>> ??????????? ...
>> ??????????? ...
>> ??????? };
>>
>> ??????? mipi1_out: port {
>> ??????????? mipi1_panel_out: endpoint {
>> ??????????????? remote-endpoint = <&panel_in_channel1>;
>> ??????????? };
>> ??????? };
>> ????};
>> };
>>
> I try to follow as you suggested,use
>
> mipi_dsi: mipi at ff960000 {
> ??? ...
> ??? ...
> ??? clock-master;??? /* implies that this DSI instance drivers the clock
> ??? ??? ??? ?* for both the DSIs.
> ??? ??? ??? ?*/
> ??? ports {
> ??? ??? mipi_in: port {
> ??? ??? ??? ...
> ??? ??? ??? ...
> ??? ??? };
> ??? ??? /* add extra output ports for both DSIs */
> ??? ??? mipi_out: port {
> ??? ??? ??? mipi_panel_out: endpoint {
> ??? ??? ??? ??? remote-endpoint = <&panel_in_channel0>;
> ??? ??? ??? };
> ??? ??? };
> ??? };
> ??? panel {
> ??? ??? ...
> ??? ??? ...
> ??? ??? /*
> ??? ??? ?* panel node can describe its input ports, if both the DSIs output
> ??? ??? ?* ports are connected to the same device (i.e, the same DSI panel),
> ??? ??? ?* we can assume that the DSIs need to operate in dual DSI mode
> ??? ??? ?*/
> ??? ??? ports {
> ??? ??? ??? ...
> ??? ??? ??? port at 0 {
> ??? ??? ??? ??? panel_in_channel0: endpoint {
> ??? ??? ??? ??? ??? remote-endpoint = <&mipi_panel_out>;
> ??? ??? ??? ??? };
> ??? ??? ??? };
> ??? ??? ??? port at 1 {
> ??? ??? ??? ??? panel_in_channel1: endpoint {
> ??? ??? ??? ??? ??? remote-endpoint = <&mipi1_panel_out>;
> ??? ??? ??? ??? };
>
> ??? ??? ??? };
> ??? ??? };
> ??? };
> };
>
> mipi_dsi1: mipi at ff968000 {
> ??? ...
> ??? ...
> ??? ports {
> ??? ??? mipi1_in: port {
> ??? ??? ??? ...
> ??? ??? ??? ...
> ??? ??? };
> ??? ??? mipi1_out: port {
> ??? ??? ??? mipi1_panel_out: endpoint {
> ??? ??? ??? ??? remote-endpoint = <&panel_in_channel1>;
> ??? ??? ??? };
> ??? ??? };
> ??? };
> }
>
> But it seems we can not use of_drm_find_panel(like below)
>
> /*
> ??????? port = of_graph_get_port_by_id(dev->of_node, 1);
> ??????? if (port) {
> ??????????????? endpoint = of_get_child_by_name(port, "endpoint");
> ??????????????? of_node_put(port);
> ??????????????? if (!endpoint) {
> ??????????????????????? dev_err(dev, "no output endpoint found\n");
> ??????????????????????? return -EINVAL;
> ??????????????? }
> ??????????????? panel_node = of_graph_get_remote_port_parent(endpoint);
> ??????????????? of_node_put(endpoint);
> ??????????????? if (!panel_node) {
> ??????????????????????? dev_err(dev, "no output node found\n");
> ??????????????????????? return -EINVAL;
> ??????????????? }
> ??????????????? panel = of_drm_find_panel(panel_node);
> ??????????????? of_node_put(panel_node);
> ??????????????? if (!panel)
> ??????????????????????? return -EPROBE_DEFER;
> ??????? }
> */
> to get DSI1 outputs,because of_drm_find_panel need compare
>
> if (panel->dev->of_node == np)
>
> in dsi_panel driver innolux->base.dev = &innolux->link->dev;
> dsi->dev
Yes, we should only have 1 drm_panel in the global panel list.
Shouldn't it be possible to modify the dsi driver such that dsi1
doesn't care whether it has a drm_panel for it or not, if we are
in dual dsi mode?
I imagine a sequence like this:
1. dsi0 probes, parses the of-graph, finds the panel and saves its device
node.
2. dsi1 probes, parses the of-graph, find the panel's device node
- dsi1 checks if it is the same as the panel attached to dsi0.
- If so, it just takes the drm_panel pointer from dsi0.
- If not, it tries a of_drm_find_panel() on the panel's device node.
A dual DSI panel driver would also be a bit different. It will be a
mipi_dsi_driver with the master DSI (dsi0) as the mipi_dsi_device. Using
the of-graph helpers, we would get the device node of dsi1 using
of_find_mipi_dsi_host_by_node(), and create another DSI device using
mipi_dsi_device_register_full(). Then, we call mipi_dsi_attach() on
both the dsi devices.
>
> struct innolux_panel {
> ??????? struct drm_panel base;
> ??????? struct mipi_dsi_device *link;
> };
> It means one panel can only be found in his dsi node,(like dsi0 above).
>
> I'm doubting about it, Or? may we follow tegra_dsi_ganged_probe
> (drivers/gpu/drm/tergra/dsi.c) method.
This method will add a new binding similar to "nvidia,ganged-mode", which
is something we don't want to do.
Archit
>
>
> Thanks,
> Nickey
>
>>> ? ? [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>>> ? [2] Documentation/devicetree/bindings/media/video-interfaces.txt
>>>
>>
>
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
