Received-SPF: pass (google.com: domain of linux-kernel+bounces-30412-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240116-jz-test-sim-panel-v1-0-f9511f46c9c7@quicinc.com> <20240116-jz-test-sim-panel-v1-4-f9511f46c9c7@quicinc.com>
In-Reply-To: <20240116-jz-test-sim-panel-v1-4-f9511f46c9c7@quicinc.com>
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date: Thu, 18 Jan 2024 19:23:27 +0200
Message-ID: <CAA8EJpoa5hrN4x9sUSXPj1UpFa_BKorRj1Kf9d2SE3OZcpvrPw@mail.gmail.com>
Subject: Re: [PATCH RFC 4/4] drm/msm/dsi: Add simulated panel support
To: Jessica Zhang <quic_jesszhan@quicinc.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>, Sam Ravnborg <sam@ravnborg.org>, 
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, 
	Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, 
	quic_abhinavk@quicinc.com, dri-devel@lists.freedesktop.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

On Wed, 17 Jan 2024 at 00:22, Jessica Zhang <quic_jesszhan@quicinc.com> wrote:
>
> Introduce the sim_panel_enabled module parameter.
>
> When set, this parameter will force DSI to select the simulated panel
> instead of the physical panel.
>
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dsi/dsi.c         | 4 ++++
>  drivers/gpu/drm/msm/dsi/dsi_host.c    | 9 ++++++++-
>  drivers/gpu/drm/msm/dsi/dsi_manager.c | 8 +++++++-
>  3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index c6bd7bf15605c..daea84f5e3c0c 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -5,6 +5,10 @@
>
>  #include "dsi.h"
>
> +bool sim_panel_enabled;
> +MODULE_PARM_DESC(sim_panel_enabled, "Use simulated panel");
> +module_param(sim_panel_enabled, bool, 0444);
> +
>  bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi)
>  {
>         unsigned long host_flags = msm_dsi_host_get_mode_flags(msm_dsi->host);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index deeecdfd6c4e4..fa0cab09fff71 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -35,6 +35,8 @@
>
>  #define DSI_RESET_TOGGLE_DELAY_MS 20
>
> +extern bool sim_panel_enabled;
> +
>  static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc_config *dsc);
>
>  static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
> @@ -2009,7 +2011,12 @@ int msm_dsi_host_register(struct mipi_dsi_host *host)
>         if (!msm_host->registered) {
>                 host->dev = &msm_host->pdev->dev;
>                 host->ops = &dsi_host_ops;
> -               ret = mipi_dsi_host_register(host);
> +
> +               if (sim_panel_enabled)
> +                       ret = mipi_dsi_host_register_sim_panel(host);
> +               else
> +                       ret = mipi_dsi_host_register(host);
> +
>                 if (ret)
>                         return ret;
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index 896f369fdd535..e33e6be7309f2 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -30,6 +30,8 @@ struct msm_dsi_manager {
>
>  static struct msm_dsi_manager msm_dsim_glb;
>
> +extern bool sim_panel_enabled;
> +
>  #define IS_BONDED_DSI()                (msm_dsim_glb.is_bonded_dsi)
>  #define IS_SYNC_NEEDED()       (msm_dsim_glb.is_sync_needed)
>  #define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id == id)
> @@ -507,7 +509,11 @@ int msm_dsi_manager_ext_bridge_init(u8 id)
>         int ret;
>
>         int_bridge = msm_dsi->bridge;
> -       ext_bridge = devm_drm_of_get_bridge(&msm_dsi->pdev->dev,
> +
> +       if (sim_panel_enabled)
> +               ext_bridge = drm_get_sim_panel_bridge(&msm_dsi->pdev->dev);
> +       else
> +               ext_bridge = devm_drm_of_get_bridge(&msm_dsi->pdev->dev,
>                                             msm_dsi->pdev->dev.of_node, 1, 0);

I think that this is definitely an imperfect way to go. We should not
be modifying the DSI host drivers to be able to use the simulation
panel.
Could you please push this to drm_of_find_panel_or_bridge() ?

>         if (IS_ERR(ext_bridge))
>                 return PTR_ERR(ext_bridge);
>
> --
> 2.43.0
>


-- 
With best wishes
Dmitry