[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ahx623ttvzd62u4fri6iqguj7mirlf22tvwbu6k2ngxw6hwbcp@oh7mmex5fjmz>
Date: Sat, 3 May 2025 01:52:40 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Abhinav Kumar <quic_abhinavk@...cinc.com>, Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Krishna Manikandan <quic_mkrishn@...cinc.com>,
Jonathan Marek <jonathan@...ek.ca>,
Kuogee Hsieh <quic_khsieh@...cinc.com>,
Neil Armstrong <neil.armstrong@...aro.org>,
Dmitry Baryshkov <lumag@...nel.org>, Rob Clark <robdclark@...il.com>,
Bjorn Andersson <andersson@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, linux-arm-msm@...r.kernel.org,
dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Rob Clark <robdclark@...omium.org>, linux-clk@...r.kernel.org,
Srinivas Kandagatla <srini@...nel.org>
Subject: Re: [PATCH v5 19/24] drm/msm/dsi: Add support for SM8750
On Wed, Apr 30, 2025 at 03:00:49PM +0200, Krzysztof Kozlowski wrote:
> Add support for DSI on Qualcomm SM8750 SoC with notable difference:
>
> DSI PHY PLLs, the parents of pixel and byte clocks, cannot be used as
> parents before DSI PHY is configured, the PLLs are prepared and their
> initial rate is set. Therefore assigned-clock-parents are not working
> here and driver is responsible for reparenting clocks with proper
> procedure: see dsi_clk_init_6g_v2_9().
Is it still the case? I thought you've said that with the proper flags
there would be no need to perform this in the driver.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>
> ---
>
> Changes in v5:
> 1. Only reparent byte and pixel clocks while PLLs is prepared. Setting
> rate works fine with earlier DISP CC patch for enabling their parents
> during rate change.
>
> Changes in v3:
> 1. Drop 'struct msm_dsi_config sm8750_dsi_cfg' and use sm8650 one.
>
> SM8750 DSI PHY also needs Dmitry's patch:
> https://patchwork.freedesktop.org/patch/542000/?series=119177&rev=1
> (or some other way of correct early setting of the DSI PHY PLL rate)
> ---
> drivers/gpu/drm/msm/dsi/dsi.h | 2 +
> drivers/gpu/drm/msm/dsi/dsi_cfg.c | 14 +++++++
> drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
> drivers/gpu/drm/msm/dsi/dsi_host.c | 81 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 98 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> index 87496db203d6c7582eadcb74e94eb56a219df292..93c028a122f3a59b1632da76472e0a3e781c6ae8 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> @@ -98,6 +98,7 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi);
> int msm_dsi_runtime_suspend(struct device *dev);
> int msm_dsi_runtime_resume(struct device *dev);
> int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host);
> +int dsi_link_clk_set_rate_6g_v2_9(struct msm_dsi_host *msm_host);
> int dsi_link_clk_set_rate_v2(struct msm_dsi_host *msm_host);
> int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host);
> int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host);
> @@ -115,6 +116,7 @@ int dsi_dma_base_get_6g(struct msm_dsi_host *msm_host, uint64_t *iova);
> int dsi_dma_base_get_v2(struct msm_dsi_host *msm_host, uint64_t *iova);
> int dsi_clk_init_v2(struct msm_dsi_host *msm_host);
> int dsi_clk_init_6g_v2(struct msm_dsi_host *msm_host);
> +int dsi_clk_init_6g_v2_9(struct msm_dsi_host *msm_host);
> int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi);
> int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool is_bonded_dsi);
> void msm_dsi_host_snapshot(struct msm_disp_state *disp_state, struct mipi_dsi_host *host);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> index 7754dcec33d06e3d6eb8a9d55e53f24af073adb9..7f8a8de0897a579a525b466fd01bbcd95454c614 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> @@ -257,6 +257,18 @@ static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = {
> .calc_clk_rate = dsi_calc_clk_rate_6g,
> };
>
> +static const struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_9_host_ops = {
> + .link_clk_set_rate = dsi_link_clk_set_rate_6g_v2_9,
> + .link_clk_enable = dsi_link_clk_enable_6g,
> + .link_clk_disable = dsi_link_clk_disable_6g,
> + .clk_init_ver = dsi_clk_init_6g_v2_9,
> + .tx_buf_alloc = dsi_tx_buf_alloc_6g,
> + .tx_buf_get = dsi_tx_buf_get_6g,
> + .tx_buf_put = dsi_tx_buf_put_6g,
> + .dma_base_get = dsi_dma_base_get_6g,
> + .calc_clk_rate = dsi_calc_clk_rate_6g,
> +};
> +
> static const struct msm_dsi_cfg_handler dsi_cfg_handlers[] = {
> {MSM_DSI_VER_MAJOR_V2, MSM_DSI_V2_VER_MINOR_8064,
> &apq8064_dsi_cfg, &msm_dsi_v2_host_ops},
> @@ -300,6 +312,8 @@ static const struct msm_dsi_cfg_handler dsi_cfg_handlers[] = {
> &sm8550_dsi_cfg, &msm_dsi_6g_v2_host_ops},
> {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_8_0,
> &sm8650_dsi_cfg, &msm_dsi_6g_v2_host_ops},
> + {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_9_0,
> + &sm8650_dsi_cfg, &msm_dsi_6g_v2_9_host_ops},
> };
>
> const struct msm_dsi_cfg_handler *msm_dsi_cfg_get(u32 major, u32 minor)
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
> index 120cb65164c1ba1deb9acb513e5f073bd560c496..859c279afbb0377d16f8406f3e6b083640aff5a1 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
> @@ -30,6 +30,7 @@
> #define MSM_DSI_6G_VER_MINOR_V2_6_0 0x20060000
> #define MSM_DSI_6G_VER_MINOR_V2_7_0 0x20070000
> #define MSM_DSI_6G_VER_MINOR_V2_8_0 0x20080000
> +#define MSM_DSI_6G_VER_MINOR_V2_9_0 0x20090000
>
> #define MSM_DSI_V2_VER_MINOR_8064 0x0
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 4d75529c0e858160761f5eb55db65e5d7565c27b..694ed95897d49c477726a2b0bec1099e75a3ce21 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -119,6 +119,15 @@ struct msm_dsi_host {
> struct clk *pixel_clk;
> struct clk *byte_intf_clk;
>
> + /*
> + * Clocks which needs to be properly parented between DISPCC and DSI PHY
> + * PLL:
> + */
> + struct clk *byte_src_clk;
> + struct clk *pixel_src_clk;
> + struct clk *dsi_pll_byte_clk;
> + struct clk *dsi_pll_pixel_clk;
> +
> unsigned long byte_clk_rate;
> unsigned long byte_intf_clk_rate;
> unsigned long pixel_clk_rate;
> @@ -269,6 +278,38 @@ int dsi_clk_init_6g_v2(struct msm_dsi_host *msm_host)
> return ret;
> }
>
> +int dsi_clk_init_6g_v2_9(struct msm_dsi_host *msm_host)
> +{
> + struct device *dev = &msm_host->pdev->dev;
> + int ret;
> +
> + ret = dsi_clk_init_6g_v2(msm_host);
> + if (ret)
> + return ret;
> +
> + msm_host->byte_src_clk = devm_clk_get(dev, "byte_src");
> + if (IS_ERR(msm_host->byte_src_clk))
> + return dev_err_probe(dev, PTR_ERR(msm_host->byte_src_clk),
> + "can't get byte_src clock\n");
> +
> + msm_host->dsi_pll_byte_clk = devm_clk_get(dev, "dsi_pll_byte");
> + if (IS_ERR(msm_host->dsi_pll_byte_clk))
> + return dev_err_probe(dev, PTR_ERR(msm_host->dsi_pll_byte_clk),
> + "can't get dsi_pll_byte clock\n");
> +
> + msm_host->pixel_src_clk = devm_clk_get(dev, "pixel_src");
> + if (IS_ERR(msm_host->pixel_src_clk))
> + return dev_err_probe(dev, PTR_ERR(msm_host->pixel_src_clk),
> + "can't get pixel_src clock\n");
> +
> + msm_host->dsi_pll_pixel_clk = devm_clk_get(dev, "dsi_pll_pixel");
> + if (IS_ERR(msm_host->dsi_pll_pixel_clk))
> + return dev_err_probe(dev, PTR_ERR(msm_host->dsi_pll_pixel_clk),
> + "can't get dsi_pll_pixel clock\n");
> +
> + return 0;
> +}
> +
> static int dsi_clk_init(struct msm_dsi_host *msm_host)
> {
> struct platform_device *pdev = msm_host->pdev;
> @@ -370,6 +411,46 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host)
> return 0;
> }
>
> +int dsi_link_clk_set_rate_6g_v2_9(struct msm_dsi_host *msm_host)
> +{
> + struct device *dev = &msm_host->pdev->dev;
> + int ret;
> +
> + /*
> + * DSI PHY PLLs have to be enabled to allow reparenting to them and
> + * setting the rates of pixel/byte clocks.
> + */
> + ret = clk_prepare_enable(msm_host->dsi_pll_byte_clk);
> + if (ret) {
> + dev_err(dev, "Failed to enable dsi_pll_byte: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(msm_host->dsi_pll_pixel_clk);
> + if (ret) {
> + dev_err(dev, "Failed to enable dsi_pll_byte: %d\n", ret);
> + goto out_disable_byte_clk;
> + }
> +
> + ret = clk_set_parent(msm_host->byte_src_clk, msm_host->dsi_pll_byte_clk);
> + if (ret)
> + dev_err(dev, "Failed to parent byte_src -> dsi_pll_byte: %d\n", ret);
> +
> + ret = clk_set_parent(msm_host->pixel_src_clk, msm_host->dsi_pll_pixel_clk);
> + if (ret)
> + dev_err(dev, "Failed to parent pixel_src -> dsi_pll_pixel: %d\n", ret);
> +
> + clk_disable_unprepare(msm_host->dsi_pll_pixel_clk);
> + clk_disable_unprepare(msm_host->dsi_pll_byte_clk);
> +
> + return dsi_link_clk_set_rate_6g(msm_host);
> +
> +out_disable_byte_clk:
> + clk_disable_unprepare(msm_host->dsi_pll_byte_clk);
> +
> + return ret;
> +}
> +
> int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host)
> {
> int ret;
>
> --
> 2.45.2
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists