This patch series aims to add several features of the dw-mipi-dsi phy
driver that are missing or need to be updated.
First patch update a PM macro.
Second patch adds runtime PM functionality to the driver.
Third patch adds a clock provider generated by the PHY itself. As
explained in the commit log of the second patch, a clock declaration is
missing. Since this clock is parent of 'dsi_k', it leads to an orphan
clock. Most importantly this patch is an anticipation for future
versions of the DSI PHY, and its inclusion within the display subsystem
and the DRM framework.
Last patch fixes a corner effect introduced previously. Since 'dsi' and
'dsi_k' are gated by the same bit on the same register, both reference
work as peripheral clock in the device-tree.
---
Changes in v2:
- Added patch 1/4 to use SYSTEM_SLEEP_PM_OPS instead of old macro
and removed __maybe_used for accordingly
- Changed SET_RUNTIME_PM_OPS to RUNTIME_PM_OPS
Raphael Gallais-Pou (3):
drm/stm: dsi: use new SYSTEM_SLEEP_PM_OPS() macro
drm/stm: dsi: expose DSI PHY internal clock
arm: dts: st: fix DSI peripheral clock on stm32mp15 boards
Yannick Fertre (1):
drm/stm: dsi: add pm runtime ops
arch/arm/boot/dts/st/stm32mp157.dtsi | 2 +-
arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts | 2 +-
arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts | 2 +-
arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts | 2 +-
arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts | 2 +-
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 278 +++++++++++++++---
6 files changed, 242 insertions(+), 46 deletions(-)
--
2.25.1
Use RUNTIME_PM_OPS() instead of the old SET_SYSTEM_SLEEP_PM_OPS().
This means we don't need __maybe_unused on the functions.
Signed-off-by: Raphael Gallais-Pou <[email protected]>
---
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index d5f8c923d7bc..b1aee43d51e9 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -544,7 +544,7 @@ static void dw_mipi_dsi_stm_remove(struct platform_device *pdev)
regulator_disable(dsi->vdd_supply);
}
-static int __maybe_unused dw_mipi_dsi_stm_suspend(struct device *dev)
+static int dw_mipi_dsi_stm_suspend(struct device *dev)
{
struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
@@ -556,7 +556,7 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct device *dev)
return 0;
}
-static int __maybe_unused dw_mipi_dsi_stm_resume(struct device *dev)
+static int dw_mipi_dsi_stm_resume(struct device *dev)
{
struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
int ret;
@@ -580,8 +580,8 @@ static int __maybe_unused dw_mipi_dsi_stm_resume(struct device *dev)
}
static const struct dev_pm_ops dw_mipi_dsi_stm_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(dw_mipi_dsi_stm_suspend,
- dw_mipi_dsi_stm_resume)
+ SYSTEM_SLEEP_PM_OPS(dw_mipi_dsi_stm_suspend,
+ dw_mipi_dsi_stm_resume)
};
static struct platform_driver dw_mipi_dsi_stm_driver = {
--
2.25.1
DSISRC __________
__\_
| \
pll4_p_ck ->| 1 |____dsi_k
ck_dsi_phy ->| 0 |
|____/
A DSI clock is missing in the clock framework. Looking at the
clk_summary, it appears that 'ck_dsi_phy' is not implemented. Since the
DSI kernel clock is based on the internal DSI pll. The common clock
driver can not directly expose this 'ck_dsi_phy' clock because it does
not contain any common registers with the DSI. Thus it needs to be done
directly within the DSI phy driver.
Signed-off-by: Raphael Gallais-Pou <[email protected]>
---
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 246 ++++++++++++++++++++++----
1 file changed, 215 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index 82fff9e84345..283f546a7198 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -7,7 +7,9 @@
*/
#include <linux/clk.h>
+#include <linux/clk-provider.h>
#include <linux/iopoll.h>
+#include <linux/kernel.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/platform_device.h>
@@ -77,9 +79,12 @@ enum dsi_color {
struct dw_mipi_dsi_stm {
void __iomem *base;
+ struct device *dev;
struct clk *pllref_clk;
struct clk *pclk;
+ struct clk_hw txbyte_clk;
struct dw_mipi_dsi *dsi;
+ struct dw_mipi_dsi_plat_data pdata;
u32 hw_version;
int lane_min_kbps;
int lane_max_kbps;
@@ -196,29 +201,198 @@ static int dsi_pll_get_params(struct dw_mipi_dsi_stm *dsi,
return 0;
}
-static int dw_mipi_dsi_phy_init(void *priv_data)
+#define clk_to_dw_mipi_dsi_stm(clk) \
+ container_of(clk, struct dw_mipi_dsi_stm, txbyte_clk)
+
+static void dw_mipi_dsi_clk_disable(struct clk_hw *clk)
{
- struct dw_mipi_dsi_stm *dsi = priv_data;
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(clk);
+
+ DRM_DEBUG_DRIVER("\n");
+
+ /* Disable the DSI PLL */
+ dsi_clear(dsi, DSI_WRPCR, WRPCR_PLLEN);
+
+ /* Disable the regulator */
+ dsi_clear(dsi, DSI_WRPCR, WRPCR_REGEN | WRPCR_BGREN);
+}
+
+static int dw_mipi_dsi_clk_enable(struct clk_hw *clk)
+{
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(clk);
u32 val;
int ret;
+ DRM_DEBUG_DRIVER("\n");
+
/* Enable the regulator */
dsi_set(dsi, DSI_WRPCR, WRPCR_REGEN | WRPCR_BGREN);
- ret = readl_poll_timeout(dsi->base + DSI_WISR, val, val & WISR_RRS,
- SLEEP_US, TIMEOUT_US);
+ ret = readl_poll_timeout_atomic(dsi->base + DSI_WISR, val, val & WISR_RRS,
+ SLEEP_US, TIMEOUT_US);
if (ret)
DRM_DEBUG_DRIVER("!TIMEOUT! waiting REGU, let's continue\n");
/* Enable the DSI PLL & wait for its lock */
dsi_set(dsi, DSI_WRPCR, WRPCR_PLLEN);
- ret = readl_poll_timeout(dsi->base + DSI_WISR, val, val & WISR_PLLLS,
- SLEEP_US, TIMEOUT_US);
+ ret = readl_poll_timeout_atomic(dsi->base + DSI_WISR, val, val & WISR_PLLLS,
+ SLEEP_US, TIMEOUT_US);
if (ret)
DRM_DEBUG_DRIVER("!TIMEOUT! waiting PLL, let's continue\n");
return 0;
}
+static int dw_mipi_dsi_clk_is_enabled(struct clk_hw *hw)
+{
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw);
+
+ return dsi_read(dsi, DSI_WRPCR) & WRPCR_PLLEN;
+}
+
+static unsigned long dw_mipi_dsi_clk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw);
+ unsigned int idf, ndiv, odf, pll_in_khz, pll_out_khz;
+ u32 val;
+
+ DRM_DEBUG_DRIVER("\n");
+
+ pll_in_khz = (unsigned int)(parent_rate / 1000);
+
+ val = dsi_read(dsi, DSI_WRPCR);
+
+ idf = (val & WRPCR_IDF) >> 11;
+ if (!idf)
+ idf = 1;
+ ndiv = (val & WRPCR_NDIV) >> 2;
+ odf = int_pow(2, (val & WRPCR_ODF) >> 16);
+
+ /* Get the adjusted pll out value */
+ pll_out_khz = dsi_pll_get_clkout_khz(pll_in_khz, idf, ndiv, odf);
+
+ return (unsigned long)pll_out_khz * 1000;
+}
+
+static long dw_mipi_dsi_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw);
+ unsigned int idf, ndiv, odf, pll_in_khz, pll_out_khz;
+ int ret;
+
+ DRM_DEBUG_DRIVER("\n");
+
+ pll_in_khz = (unsigned int)(*parent_rate / 1000);
+
+ /* Compute best pll parameters */
+ idf = 0;
+ ndiv = 0;
+ odf = 0;
+
+ ret = dsi_pll_get_params(dsi, pll_in_khz, rate / 1000,
+ &idf, &ndiv, &odf);
+ if (ret)
+ DRM_WARN("Warning dsi_pll_get_params(): bad params\n");
+
+ /* Get the adjusted pll out value */
+ pll_out_khz = dsi_pll_get_clkout_khz(pll_in_khz, idf, ndiv, odf);
+
+ return pll_out_khz * 1000;
+}
+
+static int dw_mipi_dsi_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct dw_mipi_dsi_stm *dsi = clk_to_dw_mipi_dsi_stm(hw);
+ unsigned int idf, ndiv, odf, pll_in_khz, pll_out_khz;
+ int ret;
+ u32 val;
+
+ DRM_DEBUG_DRIVER("\n");
+
+ pll_in_khz = (unsigned int)(parent_rate / 1000);
+
+ /* Compute best pll parameters */
+ idf = 0;
+ ndiv = 0;
+ odf = 0;
+
+ ret = dsi_pll_get_params(dsi, pll_in_khz, rate / 1000, &idf, &ndiv, &odf);
+ if (ret)
+ DRM_WARN("Warning dsi_pll_get_params(): bad params\n");
+
+ /* Get the adjusted pll out value */
+ pll_out_khz = dsi_pll_get_clkout_khz(pll_in_khz, idf, ndiv, odf);
+
+ /* Set the PLL division factors */
+ dsi_update_bits(dsi, DSI_WRPCR, WRPCR_NDIV | WRPCR_IDF | WRPCR_ODF,
+ (ndiv << 2) | (idf << 11) | ((ffs(odf) - 1) << 16));
+
+ /* Compute uix4 & set the bit period in high-speed mode */
+ val = 4000000 / pll_out_khz;
+ dsi_update_bits(dsi, DSI_WPCR0, WPCR0_UIX4, val);
+
+ return 0;
+}
+
+static void dw_mipi_dsi_clk_unregister(void *data)
+{
+ struct dw_mipi_dsi_stm *dsi = data;
+
+ DRM_DEBUG_DRIVER("\n");
+
+ of_clk_del_provider(dsi->dev->of_node);
+ clk_hw_unregister(&dsi->txbyte_clk);
+}
+
+static const struct clk_ops dw_mipi_dsi_stm_clk_ops = {
+ .enable = dw_mipi_dsi_clk_enable,
+ .disable = dw_mipi_dsi_clk_disable,
+ .is_enabled = dw_mipi_dsi_clk_is_enabled,
+ .recalc_rate = dw_mipi_dsi_clk_recalc_rate,
+ .round_rate = dw_mipi_dsi_clk_round_rate,
+ .set_rate = dw_mipi_dsi_clk_set_rate,
+};
+
+static struct clk_init_data cdata_init = {
+ .name = "ck_dsi_phy",
+ .ops = &dw_mipi_dsi_stm_clk_ops,
+ .parent_names = (const char * []) {"ck_hse"},
+ .num_parents = 1,
+};
+
+static int dw_mipi_dsi_clk_register(struct dw_mipi_dsi_stm *dsi,
+ struct device *dev)
+{
+ struct device_node *node = dev->of_node;
+ int ret;
+
+ DRM_DEBUG_DRIVER("Registering clk\n");
+
+ dsi->txbyte_clk.init = &cdata_init;
+
+ ret = clk_hw_register(dev, &dsi->txbyte_clk);
+ if (ret)
+ return ret;
+
+ ret = of_clk_add_hw_provider(node, of_clk_hw_simple_get,
+ &dsi->txbyte_clk);
+ if (ret)
+ clk_hw_unregister(&dsi->txbyte_clk);
+
+ return ret;
+}
+
+static int dw_mipi_dsi_phy_init(void *priv_data)
+{
+ struct dw_mipi_dsi_stm *dsi = priv_data;
+ int ret;
+
+ ret = clk_prepare_enable(dsi->txbyte_clk.clk);
+ return ret;
+}
+
static void dw_mipi_dsi_phy_power_on(void *priv_data)
{
struct dw_mipi_dsi_stm *dsi = priv_data;
@@ -235,6 +409,8 @@ static void dw_mipi_dsi_phy_power_off(void *priv_data)
DRM_DEBUG_DRIVER("\n");
+ clk_disable_unprepare(dsi->txbyte_clk.clk);
+
/* Disable the DSI wrapper */
dsi_clear(dsi, DSI_WCR, WCR_DSIEN);
}
@@ -245,9 +421,8 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode *mode,
unsigned int *lane_mbps)
{
struct dw_mipi_dsi_stm *dsi = priv_data;
- unsigned int idf, ndiv, odf, pll_in_khz, pll_out_khz;
+ unsigned int pll_in_khz, pll_out_khz;
int ret, bpp;
- u32 val;
pll_in_khz = (unsigned int)(clk_get_rate(dsi->pllref_clk) / 1000);
@@ -268,25 +443,10 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode *mode,
DRM_WARN("Warning min phy mbps is used\n");
}
- /* Compute best pll parameters */
- idf = 0;
- ndiv = 0;
- odf = 0;
- ret = dsi_pll_get_params(dsi, pll_in_khz, pll_out_khz,
- &idf, &ndiv, &odf);
+ ret = clk_set_rate((dsi->txbyte_clk.clk), pll_out_khz * 1000);
if (ret)
- DRM_WARN("Warning dsi_pll_get_params(): bad params\n");
-
- /* Get the adjusted pll out value */
- pll_out_khz = dsi_pll_get_clkout_khz(pll_in_khz, idf, ndiv, odf);
-
- /* Set the PLL division factors */
- dsi_update_bits(dsi, DSI_WRPCR, WRPCR_NDIV | WRPCR_IDF | WRPCR_ODF,
- (ndiv << 2) | (idf << 11) | ((ffs(odf) - 1) << 16));
-
- /* Compute uix4 & set the bit period in high-speed mode */
- val = 4000000 / pll_out_khz;
- dsi_update_bits(dsi, DSI_WPCR0, WPCR0_UIX4, val);
+ DRM_DEBUG_DRIVER("ERROR Could not set rate of %d to %s clk->name",
+ pll_out_khz, clk_hw_get_name(&dsi->txbyte_clk));
/* Select video mode by resetting DSIM bit */
dsi_clear(dsi, DSI_WCFGR, WCFGR_DSIM);
@@ -445,6 +605,7 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct dw_mipi_dsi_stm *dsi;
+ const struct dw_mipi_dsi_plat_data *pdata = of_device_get_match_data(dev);
int ret;
dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
@@ -514,18 +675,40 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
dsi->lane_max_kbps *= 2;
}
- dw_mipi_dsi_stm_plat_data.base = dsi->base;
- dw_mipi_dsi_stm_plat_data.priv_data = dsi;
+ dsi->pdata = *pdata;
+ dsi->pdata.base = dsi->base;
+ dsi->pdata.priv_data = dsi;
+
+ dsi->pdata.max_data_lanes = 2;
+ dsi->pdata.phy_ops = &dw_mipi_dsi_stm_phy_ops;
platform_set_drvdata(pdev, dsi);
- dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
+ dsi->dsi = dw_mipi_dsi_probe(pdev, &dsi->pdata);
if (IS_ERR(dsi->dsi)) {
ret = PTR_ERR(dsi->dsi);
dev_err_probe(dev, ret, "Failed to initialize mipi dsi host\n");
goto err_dsi_probe;
}
+ /*
+ * We need to wait for the generic bridge to probe before enabling and
+ * register the internal pixel clock.
+ */
+ ret = clk_prepare_enable(dsi->pclk);
+ if (ret) {
+ DRM_ERROR("%s: Failed to enable peripheral clk\n", __func__);
+ goto err_dsi_probe;
+ }
+
+ ret = dw_mipi_dsi_clk_register(dsi, dev);
+ if (ret) {
+ DRM_ERROR("Failed to register DSI pixel clock: %d\n", ret);
+ goto err_dsi_probe;
+ }
+
+ clk_disable_unprepare(dsi->pclk);
+
return 0;
err_dsi_probe:
@@ -542,12 +725,13 @@ static void dw_mipi_dsi_stm_remove(struct platform_device *pdev)
dw_mipi_dsi_remove(dsi->dsi);
clk_disable_unprepare(dsi->pllref_clk);
+ dw_mipi_dsi_clk_unregister(dsi);
regulator_disable(dsi->vdd_supply);
}
static int dw_mipi_dsi_stm_suspend(struct device *dev)
{
- struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
+ struct dw_mipi_dsi_stm *dsi = dev_get_drvdata(dev);
DRM_DEBUG_DRIVER("\n");
@@ -560,7 +744,7 @@ static int dw_mipi_dsi_stm_suspend(struct device *dev)
static int dw_mipi_dsi_stm_resume(struct device *dev)
{
- struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
+ struct dw_mipi_dsi_stm *dsi = dev_get_drvdata(dev);
int ret;
DRM_DEBUG_DRIVER("\n");
--
2.25.1
In RCC driver, 'DSI_K' is a kernel clock while 'DSI' has pclk4 as parent
clock, which means that it is an APB peripheral clock. Swap the clocks
in the DSI peripheral clock reference.
Signed-off-by: Raphael Gallais-Pou <[email protected]>
---
arch/arm/boot/dts/st/stm32mp157.dtsi | 2 +-
arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts | 2 +-
arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts | 2 +-
arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts | 2 +-
arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/st/stm32mp157.dtsi b/arch/arm/boot/dts/st/stm32mp157.dtsi
index 6197d878894d..97cd24227cef 100644
--- a/arch/arm/boot/dts/st/stm32mp157.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp157.dtsi
@@ -20,7 +20,7 @@ gpu: gpu@59000000 {
dsi: dsi@5a000000 {
compatible = "st,stm32-dsi";
reg = <0x5a000000 0x800>;
- clocks = <&rcc DSI_K>, <&clk_hse>, <&rcc DSI_PX>;
+ clocks = <&rcc DSI>, <&clk_hse>, <&rcc DSI_PX>;
clock-names = "pclk", "ref", "px_clk";
phy-dsi-supply = <®18>;
resets = <&rcc DSI_R>;
diff --git a/arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts b/arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts
index afcd6285890c..8634699cc65e 100644
--- a/arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts
+++ b/arch/arm/boot/dts/st/stm32mp157a-dk1-scmi.dts
@@ -30,7 +30,7 @@ &cpu1 {
};
&dsi {
- clocks = <&rcc DSI_K>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
+ clocks = <&rcc DSI>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
};
&gpioz {
diff --git a/arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts b/arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts
index 39358d902000..3a897fa7e167 100644
--- a/arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts
+++ b/arch/arm/boot/dts/st/stm32mp157c-dk2-scmi.dts
@@ -36,7 +36,7 @@ &cryp1 {
&dsi {
phy-dsi-supply = <&scmi_reg18>;
- clocks = <&rcc DSI_K>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
+ clocks = <&rcc DSI>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
};
&gpioz {
diff --git a/arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts b/arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts
index 07ea765a4553..29d6465b1fe6 100644
--- a/arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts
+++ b/arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts
@@ -35,7 +35,7 @@ &cryp1 {
};
&dsi {
- clocks = <&rcc DSI_K>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
+ clocks = <&rcc DSI>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
};
&gpioz {
diff --git a/arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts b/arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts
index 813086ec2489..5acb78f0a084 100644
--- a/arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts
+++ b/arch/arm/boot/dts/st/stm32mp157c-ev1-scmi.dts
@@ -37,7 +37,7 @@ &cryp1 {
&dsi {
phy-dsi-supply = <&scmi_reg18>;
- clocks = <&rcc DSI_K>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
+ clocks = <&rcc DSI>, <&scmi_clk CK_SCMI_HSE>, <&rcc DSI_PX>;
};
&gpioz {
--
2.25.1
On Mon, Dec 04, 2023 at 11:11:12AM +0100, Raphael Gallais-Pou wrote:
...
> @@ -514,18 +675,40 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
> dsi->lane_max_kbps *= 2;
> }
>
> - dw_mipi_dsi_stm_plat_data.base = dsi->base;
> - dw_mipi_dsi_stm_plat_data.priv_data = dsi;
> + dsi->pdata = *pdata;
> + dsi->pdata.base = dsi->base;
> + dsi->pdata.priv_data = dsi;
> +
> + dsi->pdata.max_data_lanes = 2;
> + dsi->pdata.phy_ops = &dw_mipi_dsi_stm_phy_ops;
>
> platform_set_drvdata(pdev, dsi);
>
> - dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dsi->pdata);
> if (IS_ERR(dsi->dsi)) {
> ret = PTR_ERR(dsi->dsi);
> dev_err_probe(dev, ret, "Failed to initialize mipi dsi host\n");
> goto err_dsi_probe;
> }
>
> + /*
> + * We need to wait for the generic bridge to probe before enabling and
> + * register the internal pixel clock.
> + */
> + ret = clk_prepare_enable(dsi->pclk);
> + if (ret) {
> + DRM_ERROR("%s: Failed to enable peripheral clk\n", __func__);
> + goto err_dsi_probe;
> + }
> +
> + ret = dw_mipi_dsi_clk_register(dsi, dev);
> + if (ret) {
> + DRM_ERROR("Failed to register DSI pixel clock: %d\n", ret);
Hi Raphael,
Does clk_disable_unprepare(dsi->pclk) need to be added to this unwind
chain?
Flagged by Smatch.
> + goto err_dsi_probe;
> + }
> +
> + clk_disable_unprepare(dsi->pclk);
> +
> return 0;
>
> err_dsi_probe:
...
On 12/8/23 17:58, Simon Horman wrote:
> On Mon, Dec 04, 2023 at 11:11:12AM +0100, Raphael Gallais-Pou wrote:
>
> ...
>
>> @@ -514,18 +675,40 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
>> dsi->lane_max_kbps *= 2;
>> }
>>
>> - dw_mipi_dsi_stm_plat_data.base = dsi->base;
>> - dw_mipi_dsi_stm_plat_data.priv_data = dsi;
>> + dsi->pdata = *pdata;
>> + dsi->pdata.base = dsi->base;
>> + dsi->pdata.priv_data = dsi;
>> +
>> + dsi->pdata.max_data_lanes = 2;
>> + dsi->pdata.phy_ops = &dw_mipi_dsi_stm_phy_ops;
>>
>> platform_set_drvdata(pdev, dsi);
>>
>> - dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
>> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dsi->pdata);
>> if (IS_ERR(dsi->dsi)) {
>> ret = PTR_ERR(dsi->dsi);
>> dev_err_probe(dev, ret, "Failed to initialize mipi dsi host\n");
>> goto err_dsi_probe;
>> }
>>
>> + /*
>> + * We need to wait for the generic bridge to probe before enabling and
>> + * register the internal pixel clock.
>> + */
>> + ret = clk_prepare_enable(dsi->pclk);
>> + if (ret) {
>> + DRM_ERROR("%s: Failed to enable peripheral clk\n", __func__);
>> + goto err_dsi_probe;
>> + }
>> +
>> + ret = dw_mipi_dsi_clk_register(dsi, dev);
>> + if (ret) {
>> + DRM_ERROR("Failed to register DSI pixel clock: %d\n", ret);
> Hi Raphael,
Hi Simon,
You are right, dsi->clk needs to be disabled in case the clock register fails
before exiting the probe.
I've sent a v3, which normally fixes it.
Regards,
Raphaël
>
> Does clk_disable_unprepare(dsi->pclk) need to be added to this unwind
> chain?
>
> Flagged by Smatch.
>
>> + goto err_dsi_probe;
>> + }
>> +
>> + clk_disable_unprepare(dsi->pclk);
>> +
>> return 0;
>>
>> err_dsi_probe:
> ...