| Message ID | 20250430-b4-sm8750-display-v5-19-8cab30c3e4df@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm: Add support for SM8750 | expand |
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@linaro.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 >
On 03/05/2025 00:52, Dmitry Baryshkov wrote: > 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 Yes, as we discussed many times - this is still needed even with the proper flags. > there would be no need to perform this in the driver. assigned-clock-xxx are not respecting that flag and anyway, even if that was solved, they are executed too early - before PHY is initialized. You cannot prepare PHY PLL before PHY is initialized and enabled. Best regards, Krzysztof
On Mon, May 05, 2025 at 08:45:01AM +0200, Krzysztof Kozlowski wrote: > On 03/05/2025 00:52, Dmitry Baryshkov wrote: > > 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 > > Yes, as we discussed many times - this is still needed even with the > proper flags. > > > there would be no need to perform this in the driver. > > assigned-clock-xxx are not respecting that flag and anyway, even if that This is really strange as the flag should be handled by the framework itself. > was solved, they are executed too early - before PHY is initialized. You > cannot prepare PHY PLL before PHY is initialized and enabled. Ack
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(). > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.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. > + */ According to the docs this should be handled by the CLK_OPS_PARENT_ENABLE flag. Please correct me if I'm wrong. > + 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); And this. > + 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 >
On 5/5/2025 5:35 AM, Dmitry Baryshkov wrote: > 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(). >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.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. >> + */ > > According to the docs this should be handled by the > CLK_OPS_PARENT_ENABLE flag. Please correct me if I'm wrong. > I am also interested to know that if we are indeed setting CLK_OPS_PARENT_ENABLE flag, do we need this logic in the dsi driver. If CLK_OPS_PARENT_ENABLE flag is not working as expected, shouldnt this be something fixed on the clk fwk side? Thanks Abhinav >> + 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); > > And this. > >> + 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 >> >
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;
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(). Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.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(+)