Re: [RFC PATCH 0/4] Select the STM32MP25 LTDC pixel clock source for the RGB output

[Gabriel FERNANDEZ]

Hi Dario,

Thanks for working on this and for testing it on the Engicam board.

First, a small remark about patch 2/4: this driver is based on the one
we deliver to our customers in the ST kernel on GitHub [1]. Please keep
the original copyright and the Signed-off-by of the authors when you
reuse such code, and add your own on top of them (e.g. with
Co-developed-by).

Now about your two questions:

On 10/28/26 14:59, Dario Binacchi wrote:
> - the mux parents are referenced by their global names. Should they
>   rather be described by clocks/clock-names in the SYSCFG binding?
>   Moreover, no driver registers "txbyteclk" yet.

Yes, I would also prefer to describe them in DT. I'm working on a
version where the mux is described by a clock-controller sub-node of
the SYSCFG node. This sub-node is populated by the STM32MP2 SYSCFG
driver, so no auxiliary device is needed (patch 3/4 can be dropped):

    syscfg: syscon@44230000 {
        compatible = "st,stm32mp25-syscfg", "syscon";
        ...
        syscfg_clk: clock-controller {
            compatible = "st,stm32mp25-syscfg-clk";
            #clock-cells = <1>;
            clocks = <&rcc CK_KER_LTDC>, <&lvds>, <&clk_dsi_txbyte>;
            clock-names = "ltdc", "lvds", "dsi";
        };
    };

    &ltdc {
        clocks = ..., <&syscfg_clk CK_SYSCFG_PIX_LTDC>, ...;
    };

The driver takes the parents from the "clocks" property
(clk_parent_data), so the inputs which don't exist on a given SoC (no
LVDS/DSI on STM32MP231/251) are simply omitted. For "txbyteclk", we keep
the existing clk_dsi_txbyte placeholder until the DSI driver provides
this clock.

I also want to keep "#clock-cells = <1>" with a onecell provider: the
SYSCFG has other clock selections, and I would like to be able to add
them later in the same driver without changing the binding again.

> - the pixel clock source is selected in the board devicetree by
>   assigned-clock-parents. Should the LTDC driver rather select it
>   according to its output (RGB, LVDS or DSI)?

I think the LTDC driver should select it: the source depends on the
output used, not on the board. This is what we do in our downstream
LTDC driver: it gets the mux as "ref" clock and calls clk_set_parent()
according to the encoder type (LVDS PHY clock for LVDS, LTDC kernel
clock otherwise) before setting the pixel clock rate. The board
overlays then don't need assigned-clock-parents. Yannick and Raphael
(in Cc) can comment on the LTDC side.

Since I have already reworked the clock driver this way (binding,
driver and SoC DT), would you be OK if I take over the
clk-stm32mp25-syscfg patch? I would send it in a series, keeping you as
Co-developed-by, and you could then rebase your Engicam patches on top
of it.

[1] 
https://github.com/STMicroelectronics/linux/blob/v6.6-stm32mp/drivers/clk/stm32/clk-stm32mp25-syscfg.c

Best regards,
Gabriel


On 10/28/26 14:59, Dario Binacchi wrote:
> On STM32MP25 the LTDC pixel clock is selected in the SYSCFG
> DISPLAYCLKCR register among the DSI byte lane clock, the LVDS PHY pixel
> clock (reset value) and the LTDC kernel clock. The SYSCFG node is
> already described as provider of this clock, which the LTDC references
> as "ref" clock, but no driver registers it, so the RGB (DPI) output
> can't work.
>
> This series adds a driver for this mux, probed as an auxiliary device
> of the STM32MP2 SYSCFG driver, and selects the pixel clock source on the
> Engicam MicroGEA-STM32MP257D-RMM display overlays. Patch 1 is an
> independent fix for the RGB display overlay.
>
> Tested on the Engicam MicroGEA-STM32MP257D-RMM board with both the RGB
> (Rocktech RK050HR345-CT106A) and the LVDS (Ampire AM-1280800W8TZQW-T00H)
> displays.
>
> I am sending it as RFC since I am not sure about:
>
> - the mux parents are referenced by their global names. Should they
>    rather be described by clocks/clock-names in the SYSCFG binding?
>    Moreover, no driver registers "txbyteclk" yet.
>
> - the pixel clock source is selected in the board devicetree by
>    assigned-clock-parents. Should the LTDC driver rather select it
>    according to its output (RGB, LVDS or DSI)?
>
>
> Dario Binacchi (4):
>    arm64: dts: st: fix the Engicam RMM RGB display pixel clock
>    clk: stm32: add STM32MP25 SYSCFG clock driver
>    soc: st: stm32mp2-syscfg: register the STM32MP25 clock device
>    arm64: dts: st: select the Engicam RMM display pixel clock source
>
>   ...gea-rmm-overlay-am-1280800w8tzqw-t00h.dtso |   3 +
>   ...icrogea-rmm-overlay-rk050hr345-ct106a.dtso |   5 +
>   drivers/clk/stm32/Kconfig                     |  12 ++
>   drivers/clk/stm32/Makefile                    |   1 +
>   drivers/clk/stm32/clk-stm32mp25-syscfg.c      | 111 ++++++++++++++++++
>   drivers/soc/st/Kconfig                        |   1 +
>   drivers/soc/st/stm32mp2-syscfg.c              |  28 ++++-
>   7 files changed, 159 insertions(+), 2 deletions(-)
>   create mode 100644 drivers/clk/stm32/clk-stm32mp25-syscfg.c
>
> ---
> base-commit: 65ba8f34ec886ec72b2c07ddc8cf3d815c32869d
> branch: stm32mp25-ltdc-pixclk