Skip to content

Conversation

CharlesDias
Copy link
Contributor

@CharlesDias CharlesDias commented Aug 13, 2025

Added STM32U5-specific configurations for the STM32 MIPI DSI driver, including:

  • DSI PHY clock source setup.
  • Logging for STM32U5-specific properties (e.g., PHY frequency range, low-power offset).
  • Adjusted lane clock calculations for STM32U5 PLL configurations.
  • Conditional handling for STM32U5-specific properties like PLL tuning, charge pump, and VCO range.

Tested with:
west build -p -b stm32u5g9j_dk1 --shield st_mb1835 samples/drivers/display/
And also
west build -p -b stm32u5g9j_dk1 --shield st_mb1835 samples/modules/lvgl/demos -- -DCONFIG_LV_Z_DEMO_MUSIC=y

Related to:

WhatsApp.Video.2025-08-12.at.19.57.52.mp4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should rather create another binding specific to the U5 IP, include this one, and exclude the properties that are not applicable to U5 (instead of doing it in the driver).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @JarmouniA. Thanks for your review.

I'll create st,stm32u5-mipi-dsi.yaml and update the mipi_dsi node to use both bindings files:

mipi_dsi: dsihost@40016c00 {
	compatible = "st,stm32u5-mipi-dsi", "st,stm32-mipi-dsi";

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm no longer sure about this suggestion. From AN4860 Rev 4, it appears to be the same IP on all MCUs/MPUs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With these new properties, I feel like we are just mapping HAL structs to DT, which is not wrong in itself, but maybe things can be done in a better way.
https://github.com/STMicroelectronics/stm32u5xx-hal-driver/blob/2c5e2568fbdb1900a13ca3b2901fdd302cac3444/Inc/stm32u5xx_hal_dsi.h#L64
https://github.com/STMicroelectronics/stm32u5xx-hal-driver/blob/2c5e2568fbdb1900a13ca3b2901fdd302cac3444/Inc/stm32u5xx_hal_dsi.h#L90
DT is meant to have properties of the underlying hardware IP regardless of which HAL layer is used...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, all! Enlightening comments!

About this "So I think we should have a separate compatible for DSI Wrapper that will be SoC specific, and its node will be a child of DSI Host node in DT.", any suggestions for the node name and binding file? Has ST implemented something similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking to add this node inside zephyr/dts/arm/st/u5/stm32u599.dtsi. Please let me know if you have a different suggestion. Thanks!

mipi_dsi: dsihost@40016c00 {
	compatible = "st,stm32u5-mipi-dsi", "st,stm32-mipi-dsi";
	#address-cells = <1>;
	#size-cells = <0>;
	reg = <0x40016C00 0x1000>;
	clock-names = "dsiclk", "refclk", "pixelclk";
	clocks = <&rcc STM32_CLOCK(APB2, 27U)>,
			<&rcc STM32_SRC_HSE NO_SEL>,
			<&rcc STM32_SRC_PLL3_R NO_SEL>;
	resets = <&rctl STM32_RESET(APB2, 27U)>;
	status = "disabled";

	/* SoC-specific wrapper sits inside the host register space */
	dsi_wrapper@40017034 {
		compatible = "st,stm32u5-dsi-wrapper";
		reg = <0x40017034 0x34>;	/* DSI_WCFGR..DSI_WPTR (0x400–0x430), rounded up */
		/* Default values - can be overridden in board files */
		pll-vco-range = <1>;		/* DSI_DPHY_VCO_FRANGE_800MHZ_1GHZ */
		pll-charge-pump = <0>;		/* DSI_PLL_CHARGE_PUMP_2000HZ_4400HZ */
		pll-tuning = <0>; 		/* DSI_PLL_LOOP_FILTER_2000HZ_4400HZ */
		phy-freq-range = <8>;		/* DSI_DPHY_FRANGE_450MHZ_510MHZ */
		phy-low-power-offset = <0>;	/* PHY_LP_OFFSET_0_CLKP */
	};
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good to me. But if we are going to add a DSI Wrapper node, the DSI Host no longer needs a SoC specific compatible.
But I would wait for the feedback of ST folks before implementing the change.
I also took a look at STM32 MPUs in Linux, see
https://github.com/STMicroelectronics/linux/blob/v6.6-stm32mp/Documentation/devicetree/bindings/display/st%2Cstm32-dsi.yaml
https://wiki.st.com/stm32mpu/wiki/DSI_internal_peripheral#Software_frameworks_and_drivers
but since we are using the HAL in Zephyr, I don't think we can follow it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, just let me know when I can proceed. Thanks for your support!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CharlesDias CharlesDias force-pushed the update_dsi_stm32_for_stm32u5 branch from a87787c to 853fb67 Compare August 13, 2025 19:04
@JarmouniA JarmouniA requested a review from avolmat-st August 14, 2025 13:23
@CharlesDias CharlesDias force-pushed the update_dsi_stm32_for_stm32u5 branch from 853fb67 to aa6ae71 Compare August 14, 2025 18:09
@CharlesDias
Copy link
Contributor Author

Implemented the DSIPHYInitPeriph and LOG changes.

@sonarqubecloud
Copy link

@erwango
Copy link
Member

erwango commented Sep 18, 2025

@CharlesDias No rush, but for my information, do you plan to complete this for next DV ?

@CharlesDias
Copy link
Contributor Author

@CharlesDias No rush, but for my information, do you plan to complete this for next DV ?

Hi, @erwango. I'm expecting a definition from ST team regarding this discussion #94433 (comment) about how to address the STM32U5 properties before move on. @JarmouniA , do we have any updates on this?

Thank you all!

@CharlesDias CharlesDias force-pushed the update_dsi_stm32_for_stm32u5 branch from aa6ae71 to ec0c8ee Compare October 16, 2025 12:58
@CharlesDias
Copy link
Contributor Author

Rebase

uint32_t pixel_clk_khz;
};

#ifdef CONFIG_SOC_SERIES_STM32U5X
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer #if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_mipi_dsi)which provide the advantage to be compatible with the next soc which will use the same IP.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that SoC is not guaranteed to have the same HAL functions' names.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that SoC is not guaranteed to have the same HAL functions' names.

Are you joking ?

I agree it happens to be not always the case for few exceptions, but large majority of our drivers based on this principle: HAL/LL based and selected on a compatible criteria.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't implemented it because I'd like to confirm if this change is to be done in all #ifdef CONFIG_SOC_SERIES_STM32U5X occurrences? Like below:

diff --git a/drivers/mipi_dsi/dsi_stm32.c b/drivers/mipi_dsi/dsi_stm32.c
index 7cdd78ed0fd..9d18425ce9b 100644
--- a/drivers/mipi_dsi/dsi_stm32.c
+++ b/drivers/mipi_dsi/dsi_stm32.c
@@ -57,7 +57,7 @@ struct mipi_dsi_stm32_data {
 	uint32_t pixel_clk_khz;
 };
 
-#ifdef CONFIG_SOC_SERIES_STM32U5X
+#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_mipi_dsi)
 /* Configures DSI PHY as DSI clock source (STM32U5 specific) */
 static int stm32_dsi_clock_source_config(const struct device *dev)
 {
@@ -88,7 +88,7 @@ static void mipi_dsi_stm32_log_config(const struct device *dev)
 	LOG_DBG("  AutomaticClockLaneControl 0x%x", data->hdsi.Init.AutomaticClockLaneControl);
 	LOG_DBG("  TXEscapeCkdiv %u", data->hdsi.Init.TXEscapeCkdiv);
 	LOG_DBG("  NumberOfLanes %u", data->hdsi.Init.NumberOfLanes);
-#ifdef CONFIG_SOC_SERIES_STM32U5X
+#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_mipi_dsi)
 	LOG_DBG("  PHYFrequencyRange 0x%x", data->hdsi.Init.PHYFrequencyRange);
 	LOG_DBG("  PHYLowPowerOffset 0x%x", data->hdsi.Init.PHYLowPowerOffset);
 #endif
@@ -97,7 +97,7 @@ static void mipi_dsi_stm32_log_config(const struct device *dev)
 	LOG_DBG("  PLLNDIV %u", data->pll_init.PLLNDIV);
 	LOG_DBG("  PLLIDF %u", data->pll_init.PLLIDF);
 	LOG_DBG("  PLLODF %u", data->pll_init.PLLODF);
-#ifdef CONFIG_SOC_SERIES_STM32U5X
+#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_mipi_dsi)
 	LOG_DBG("  PLLVCORange 0x%x", data->pll_init.PLLVCORange);
 	LOG_DBG("  PLLChargePump 0x%x", data->pll_init.PLLChargePump);
 	LOG_DBG("  PLLTuning 0x%x", data->pll_init.PLLTuning);
@@ -200,7 +200,7 @@ static int mipi_dsi_stm32_host_init(const struct device *dev)
 		return ret;
 	}
 
-#ifdef CONFIG_SOC_SERIES_STM32U5X
+#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_mipi_dsi)
 	/* LANE_BYTE_CLOCK = CLK_IN / PLLIDF * 2 * PLLNDIV / PLLODF / 8 */
 	data->lane_clk_khz = hse_clock / data->pll_init.PLLIDF * 2 * data->pll_init.PLLNDIV /
 			     data->pll_init.PLLODF / 8 / 1000;
@@ -331,7 +331,7 @@ static int mipi_dsi_stm32_attach(const struct device *dev, uint8_t channel,
 		return -ret;
 	}
 
-#ifdef CONFIG_SOC_SERIES_STM32U5X
+#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_mipi_dsi)
 	ret = stm32_dsi_clock_source_config(dev);
 	if (ret < 0) {
 		LOG_ERR("Failed to configure DSI clock source");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't implemented it because I'd like to confirm if this change is to be done in all #ifdef CONFIG_SOC_SERIES_STM32U5X occurrences?

Please do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

#ifdef CONFIG_SOC_SERIES_STM32U5X
/* LANE_BYTE_CLOCK = CLK_IN / PLLIDF * 2 * PLLNDIV / PLLODF / 8 */
data->lane_clk_khz = hse_clock / data->pll_init.PLLIDF * 2 * data->pll_init.PLLNDIV /
data->pll_init.PLLODF / 8 / 1000;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This highlight a change in the way to define pll-odf between U5 and other platforms. Considering that it is defined the same way in the DT yaml, I don't think it should be used differently between the U5 series and other series.
(or it should be highlighted why this is needed, and highlight in the yaml).
Could you share you dts configuration of your mipi-dsi node for this U5 ?
I haven't check the U5 DSI spec, why lane clock freq calculation has to be different between series ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having checked the HAL code, my understanding is that PLLODF can have a value going from 1 to 9 (1 being divide by 1). So This seems to be ok as far as INIT is concerned, however the real divider factor a 1 << ODF

I am actually wondering if the original computation is correct ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @avolmat-st. Thanks for your review.

Please, take a look at the dts configuration here #94423

The lane_clk_khz calculation described in document AN4860 Rev 4 page 75 is different from what is described in RM0456 Rev 6 page 1782 as PHI. Additionally, this is also aligned with the STM32CudeIDE representation. Please let me know if I understood wrong.

AN4860 Rev 4 page 75
image

RM0456 Rev 6 page 1782
image

STM32CudeIDE for H7
image

STM32CudeIDE for U5
image

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @CharlesDias for the clarification. I didn't notice that important different. It make senses then.
U5 support is ok I think then. What might have to be reworked is thus more the H7 rather than the U5 in fact since in case of H7, the PLLODF is not really giving the divider factor by the value to be written into the register, which is not the divider factor. But that is not part of this PR.

@CharlesDias
Copy link
Contributor Author

CharlesDias commented Oct 16, 2025

Implemented the requests from @JarmouniA and @erwango (except for this one #94433 (comment)).

Please verify if the dts/bindings/mipi-dsi/st,stm32u5-mipi-dsi.yaml is correct this way.

@CharlesDias CharlesDias force-pushed the update_dsi_stm32_for_stm32u5 branch from ef237d8 to 1bca173 Compare October 16, 2025 23:25
@CharlesDias
Copy link
Contributor Author

Implement conditional initialization of dsisrc_clk.

@erwango
Copy link
Member

erwango commented Oct 17, 2025

(except for this one #94433 (comment)).

Can you explicit the reason ?

@JarmouniA JarmouniA dismissed their stale review October 17, 2025 09:16

addressed

These additions enhance the flexibility of the MIPI DSI host configuration
for STM32U5 series, enabling finer control over the DSI PLL and PHY
settings.

Signed-off-by: Charles Dias <[email protected]>
Add DSI host controller node definition for STM32U59x/5Ax series.

Signed-off-by: Charles Dias <[email protected]>
These changes enhance the driver's compatibility with the STM32U5 series,
enabling its use in applications requiring MIPI.

Signed-off-by: Charles Dias <[email protected]>
@CharlesDias CharlesDias force-pushed the update_dsi_stm32_for_stm32u5 branch from 1bca173 to a0caffd Compare October 17, 2025 12:49
@CharlesDias
Copy link
Contributor Author

Clean up the st,stm32u5-mipi-dsi.yaml and implement the latest requests from @erwango.

Tested with:
west build -p -b stm32u5g9j_dk1 --shield st_mb1835 samples/drivers/display/
And also
west build -p -b stm32u5g9j_dk1 --shield st_mb1835 samples/modules/lvgl/demos -- -DCONFIG_LV_Z_DEMO_MUSIC=y

@sonarqubecloud
Copy link

Comment on lines +254 to +255
#ifndef CONFIG_SOC_SERIES_STM32U5X

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non blocking:

Suggested change
#ifndef CONFIG_SOC_SERIES_STM32U5X
#if !DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_mipi_dsi)

@erwango erwango added this to the v4.3.0 milestone Oct 17, 2025
@jhedberg jhedberg merged commit 5bad29f into zephyrproject-rtos:main Oct 17, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants