| Message ID | 20180814102654.29113-7-heiko@sntech.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/rockchip: migrate to common dw-mipi-dsi bridge and dual-dsi | expand |
On 14.08.2018 12:26, Heiko Stuebner wrote: > >From a specified output port of one dsi controller this function allows to > iterate over the list of registered dsi controllers trying to find a second > instance connected to the same display, like it is used in dual-dsi setups. As I said earlier it looks for me incorrectly. You create helper to access/control one DSI controller from another but there is no point in doing it. If CRTC is able to send video stream via two links it should be CRTC's responsibility to setup and use DSI controllers attached to these links. If the panel is able to combine stream from two DSI links into one image it should be panel's responsibility to inform DSI controllers about each link's configuration. Moreover from usage of this helper (patch 8/8) it looks like you assumes, that second host must be also synopsys, this is not true/generic at all. I know there could be many obstacles in alternative approach, so you really insist on this hack, please use it inside the driver, you can use for example driver_for_each_device/driver_find_device to find 2nd DSI host. Regards Andrzej > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > drivers/gpu/drm/drm_mipi_dsi.c | 56 ++++++++++++++++++++++++++++++++++ > include/drm/drm_mipi_dsi.h | 2 ++ > 2 files changed, 58 insertions(+) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index bc73b7f5b9fc..0c3c9c7aa3b8 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -30,6 +30,7 @@ > #include <linux/device.h> > #include <linux/module.h> > #include <linux/of_device.h> > +#include <linux/of_graph.h> > #include <linux/pm_runtime.h> > #include <linux/slab.h> > > @@ -282,6 +283,61 @@ struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node) > } > EXPORT_SYMBOL(of_find_mipi_dsi_host_by_node); > > +struct device_node *of_mipi_dsi_find_second_host(struct device_node *first_np, > + int port, int endpoint) > +{ > + struct mipi_dsi_host *first, *host; > + struct device_node *remote, *np, *second_np = NULL; > + int num = 0; > + > + first = of_find_mipi_dsi_host_by_node(first_np); > + if (!first) { > + pr_err("no dsi-host for node %s\n", first_np->full_name); > + return ERR_PTR(-ENODEV); > + } > + > + /* output-node of the known dsi-host */ > + remote = of_graph_get_remote_node(first_np, port, endpoint); > + if (!remote) { > + dev_err(first->dev, "no output node found\n"); > + return ERR_PTR(-ENODEV); > + } > + > + mutex_lock(&host_lock); > + > + list_for_each_entry(host, &host_list, list) { > + np = of_graph_get_remote_node(host->dev->of_node, > + port, endpoint); > + > + /* found a host connected to this panel */ > + if (np == remote) > + num++; > + > + /* found one second host */ > + if (host->dev->of_node != first_np) > + second_np = host->dev->of_node; > + > + of_node_put(np); > + } > + > + /* of_node_get the node under host_lock */ > + if (num == 2) > + of_node_get(second_np); > + > + mutex_unlock(&host_lock); > + > + of_node_put(remote); > + > + if (num > 2) { > + dev_err(first->dev, > + "too many DSI links for output: %d links\n", num); > + return ERR_PTR(-EINVAL); > + } > + > + return second_np; > +} > +EXPORT_SYMBOL(of_mipi_dsi_find_second_host); > + > int mipi_dsi_host_register(struct mipi_dsi_host *host) > { > struct device_node *node; > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h > index 4fef19064b0f..89532ae69c91 100644 > --- a/include/drm/drm_mipi_dsi.h > +++ b/include/drm/drm_mipi_dsi.h > @@ -107,6 +107,8 @@ struct mipi_dsi_host { > int mipi_dsi_host_register(struct mipi_dsi_host *host); > void mipi_dsi_host_unregister(struct mipi_dsi_host *host); > struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node); > +struct device_node *of_mipi_dsi_find_second_host(struct device_node *first_np, > + int port, int endpoint); > > /* DSI mode flags */ >
Hi Andrzej, Am Montag, 20. August 2018, 13:37:24 CEST schrieb Andrzej Hajda: > On 14.08.2018 12:26, Heiko Stuebner wrote: > > >From a specified output port of one dsi controller this function allows to > > iterate over the list of registered dsi controllers trying to find a second > > instance connected to the same display, like it is used in dual-dsi setups. > > As I said earlier it looks for me incorrectly. You create helper to > access/control one DSI controller from another but there is no point in > doing it. If CRTC is able to send video stream via two links it should > be CRTC's responsibility to setup and use DSI controllers attached to > these links. If the panel is able to combine stream from two DSI links > into one image it should be panel's responsibility to inform DSI > controllers about each link's configuration. > > Moreover from usage of this helper (patch 8/8) it looks like you > assumes, that second host must be also synopsys, this is not > true/generic at all. > > I know there could be many obstacles in alternative approach, so you > really insist on this hack, please use it inside the driver, you can use > for example driver_for_each_device/driver_find_device to find 2nd DSI host. Ok, I'll move that inside the rockchip-driver. My rationale still is that both msm (with a Committer-Signed-off by Rob Clark) and Tegra (code from Thierry), do follow the expectation of two controllers of the same type) ... people doing graphics stuff way longer than me. So I'd like to believe the valid issue you are raising will have been discussed at least two times already ;-) . So I really prefer not to open that can of worms myself if I don't have to ;-) . Heiko
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index bc73b7f5b9fc..0c3c9c7aa3b8 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -30,6 +30,7 @@ #include <linux/device.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/of_graph.h> #include <linux/pm_runtime.h> #include <linux/slab.h> @@ -282,6 +283,61 @@ struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node) } EXPORT_SYMBOL(of_find_mipi_dsi_host_by_node); +struct device_node *of_mipi_dsi_find_second_host(struct device_node *first_np, + int port, int endpoint) +{ + struct mipi_dsi_host *first, *host; + struct device_node *remote, *np, *second_np = NULL; + int num = 0; + + first = of_find_mipi_dsi_host_by_node(first_np); + if (!first) { + pr_err("no dsi-host for node %s\n", first_np->full_name); + return ERR_PTR(-ENODEV); + } + + /* output-node of the known dsi-host */ + remote = of_graph_get_remote_node(first_np, port, endpoint); + if (!remote) { + dev_err(first->dev, "no output node found\n"); + return ERR_PTR(-ENODEV); + } + + mutex_lock(&host_lock); + + list_for_each_entry(host, &host_list, list) { + np = of_graph_get_remote_node(host->dev->of_node, + port, endpoint); + + /* found a host connected to this panel */ + if (np == remote) + num++; + + /* found one second host */ + if (host->dev->of_node != first_np) + second_np = host->dev->of_node; + + of_node_put(np); + } + + /* of_node_get the node under host_lock */ + if (num == 2) + of_node_get(second_np); + + mutex_unlock(&host_lock); + + of_node_put(remote); + + if (num > 2) { + dev_err(first->dev, + "too many DSI links for output: %d links\n", num); + return ERR_PTR(-EINVAL); + } + + return second_np; +} +EXPORT_SYMBOL(of_mipi_dsi_find_second_host); + int mipi_dsi_host_register(struct mipi_dsi_host *host) { struct device_node *node; diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h index 4fef19064b0f..89532ae69c91 100644 --- a/include/drm/drm_mipi_dsi.h +++ b/include/drm/drm_mipi_dsi.h @@ -107,6 +107,8 @@ struct mipi_dsi_host { int mipi_dsi_host_register(struct mipi_dsi_host *host); void mipi_dsi_host_unregister(struct mipi_dsi_host *host); struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node); +struct device_node *of_mipi_dsi_find_second_host(struct device_node *first_np, + int port, int endpoint); /* DSI mode flags */
From a specified output port of one dsi controller this function allows to iterate over the list of registered dsi controllers trying to find a second instance connected to the same display, like it is used in dual-dsi setups. Signed-off-by: Heiko Stuebner <heiko@sntech.de> --- drivers/gpu/drm/drm_mipi_dsi.c | 56 ++++++++++++++++++++++++++++++++++ include/drm/drm_mipi_dsi.h | 2 ++ 2 files changed, 58 insertions(+)