[PATCH v4 6/8] drm/dsi: add helper function to find the second host in a dual-dsi setup

[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.

Regards
Andrzej

>
> Signed-off-by: Heiko Stuebner <heiko at 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 */
>