linux-kernel - Re: [PATCH 3/3] drm/msm/dsi: make sure we have panel or bridge earlier

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 4 Jul 2019 08:39:44 +0200
From:   Andrzej Hajda <a.hajda@...sung.com>
To:     Rob Clark <robdclark@...il.com>, dri-devel@...ts.freedesktop.org
Cc:     Rob Clark <robdclark@...omium.org>,
        David Airlie <airlied@...ux.ie>, linux-arm-msm@...r.kernel.org,
        Abhinav Kumar <abhinavk@...eaurora.org>,
        linux-kernel@...r.kernel.org, Sean Paul <sean@...rly.run>,
        Sibi Sankar <sibis@...eaurora.org>,
        freedreno@...ts.freedesktop.org,
        Chandan Uddaraju <chandanu@...eaurora.org>
Subject: Re: [PATCH 3/3] drm/msm/dsi: make sure we have panel or bridge
 earlier

On 30.06.2019 15:14, Rob Clark wrote:
> From: Rob Clark <robdclark@...omium.org>
>
> If we are going to -EPROBE_DEFER due to panel/bridge not probed yet, we
> want to do it before we start touching hardware.


As the evidence shows, if the driver create bus (mipi-dsi), then it
should not defer probing due to lack of sink (panel/bridge), because the
sink will not appear without the bus.

Instead of defer probing you can defer component binding, or if you like
challenges you can implement dynamic sink binding :)


Regards

Andrzej




>
> Signed-off-by: Rob Clark <robdclark@...omium.org>
> ---
>  drivers/gpu/drm/msm/dsi/dsi.h         |  2 +-
>  drivers/gpu/drm/msm/dsi/dsi_host.c    | 30 +++++++++++++--------------
>  drivers/gpu/drm/msm/dsi/dsi_manager.c |  9 +++-----
>  3 files changed, 19 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> index 53bb124e8259..e15e7534ccd9 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> @@ -171,7 +171,7 @@ int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
>  struct drm_panel *msm_dsi_host_get_panel(struct mipi_dsi_host *host);
>  unsigned long msm_dsi_host_get_mode_flags(struct mipi_dsi_host *host);
>  struct drm_bridge *msm_dsi_host_get_bridge(struct mipi_dsi_host *host);
> -int msm_dsi_host_register(struct mipi_dsi_host *host, bool check_defer);
> +int msm_dsi_host_register(struct mipi_dsi_host *host);
>  void msm_dsi_host_unregister(struct mipi_dsi_host *host);
>  int msm_dsi_host_set_src_pll(struct mipi_dsi_host *host,
>  			struct msm_dsi_pll *src_pll);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 1ae2f5522979..8e5b0ba9431e 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1824,6 +1824,20 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
>  		goto fail;
>  	}
>  
> +	/*
> +	 * Make sure we have panel or bridge early, before we start
> +	 * touching the hw.  If bootloader enabled the display, we
> +	 * want to be sure to keep it running until the bridge/panel
> +	 * is probed and we are all ready to go.  Otherwise we'll
> +	 * kill the display and then -EPROBE_DEFER
> +	 */
> +	if (IS_ERR(of_drm_find_panel(msm_host->device_node)) &&
> +			!of_drm_find_bridge(msm_host->device_node)) {
> +		pr_err("%s: no panel or bridge yet\n", __func__);
> +		return -EPROBE_DEFER;
> +	}
> +
> +
>  	msm_host->ctrl_base = msm_ioremap(pdev, "dsi_ctrl", "DSI CTRL");
>  	if (IS_ERR(msm_host->ctrl_base)) {
>  		pr_err("%s: unable to map Dsi ctrl base\n", __func__);
> @@ -1941,7 +1955,7 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
>  	return 0;
>  }
>  
> -int msm_dsi_host_register(struct mipi_dsi_host *host, bool check_defer)
> +int msm_dsi_host_register(struct mipi_dsi_host *host)
>  {
>  	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
>  	int ret;
> @@ -1955,20 +1969,6 @@ int msm_dsi_host_register(struct mipi_dsi_host *host, bool check_defer)
>  			return ret;
>  
>  		msm_host->registered = true;
> -
> -		/* If the panel driver has not been probed after host register,
> -		 * we should defer the host's probe.
> -		 * It makes sure panel is connected when fbcon detects
> -		 * connector status and gets the proper display mode to
> -		 * create framebuffer.
> -		 * Don't try to defer if there is nothing connected to the dsi
> -		 * output
> -		 */
> -		if (check_defer && msm_host->device_node) {
> -			if (IS_ERR(of_drm_find_panel(msm_host->device_node)))
> -				if (!of_drm_find_bridge(msm_host->device_node))
> -					return -EPROBE_DEFER;
> -		}
>  	}
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index ff39ce6150ad..cd3450dc3481 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -82,7 +82,7 @@ static int dsi_mgr_setup_components(int id)
>  	int ret;
>  
>  	if (!IS_DUAL_DSI()) {
> -		ret = msm_dsi_host_register(msm_dsi->host, true);
> +		ret = msm_dsi_host_register(msm_dsi->host);
>  		if (ret)
>  			return ret;
>  
> @@ -101,14 +101,11 @@ static int dsi_mgr_setup_components(int id)
>  		/* Register slave host first, so that slave DSI device
>  		 * has a chance to probe, and do not block the master
>  		 * DSI device's probe.
> -		 * Also, do not check defer for the slave host,
> -		 * because only master DSI device adds the panel to global
> -		 * panel list. The panel's device is the master DSI device.
>  		 */
> -		ret = msm_dsi_host_register(slave_link_dsi->host, false);
> +		ret = msm_dsi_host_register(slave_link_dsi->host);
>  		if (ret)
>  			return ret;
> -		ret = msm_dsi_host_register(master_link_dsi->host, true);
> +		ret = msm_dsi_host_register(master_link_dsi->host);
>  		if (ret)
>  			return ret;
>