Issue using DSI-to-HDMI adapter B-LCDAD-HDMI1 (adv7533 bridge) with STM32MP25F

[MBassi]

Hello,

with my colleagues @LOrci.1 and @Raggio we are testing a SOM based on STM32MP257F line, running OpenSTLinux 6.6. Our SOM is connected to an expansion board originally designed for MP1 SOM by ST (STM32MP157C line).

For video output we use the DSI-to-HDMI adapter B-LCDAD-HDMI1, based on the ADV7533 bridge. Our MP2 SOM includes the same DSI peripheral as STM32MP257F-EV1 board, with one key difference: for backward compatibility with the MP1 SOM, the DSI connector is located on the expansion board and only 2 data lanes are routed/available.

To use the adapter correctly, the device tree was updated by setting the adi,dsi-lanes property of the adv753x: hdmi@3d node to 2 instead of 4.

When applying this change on STM32MP257F-EV1, the display works correctly and modetest command reports three available resolutions for the connected monitor. When applying the same change to our setup (MP2 SOM + expansion board), the screen remains black, although modetest still detects the same three resolutions as in the working case.

After several attempts to identify other differences between our SOM and STM32MP257F-EV1 board, we used devmem2 and found that the NL field of the DSI_PCONFR register is configured with value “10”, corresponding to 3 dsi lanes on both systems. While investigating where this value was being set, we noticed an apparent mismatch: the DSI peripheral uses 4 lanes by default, and our setting of adi,dsi-lanes in the device tree should force 2 lanes, yet the hardware ends up configured for 3 lanes.

We eventually traced this behavior to a small issue in the ADV7533 driver (drivers/gpu/drm/bridge/adv7511/adv7533.c, function adv7533_mode_fixup) that manifests in our setup:

bool adv7533modefixup(struct adv7511 adv,
                        const struct drmdisplaymode mode,
                        struct drmdisplaymode adjustedmode)
{
    struct mipidsidevice dsi = adv->dsi;
    int lanes, ret;
    if (mode->clock > 80000)
        lanes = 4;
    else
        lanes = 3;
    if (lanes != dsi->lanes) {
        mipidsidetach(dsi);
        dsi->lanes = lanes;
        ret = mipidsiattach(dsi);
        if (ret) {
            deverr(&dsi->dev, "failed to change host lanes\n");
            return false;
        }
    }
    return true;
}

This function forces the use of 4 dsi lanes when required pixel clock is greater than 80 MHz, otherwise it forces 3 lanes, without considering the case where only 2 lanes are physically available. As a result, mipi_dsi_attach() updates the NL field of DSI_PCONFR by increasing the number of lanes.

Looking at how the driver behaved when the DSI-to-HDMI adapter B-LCDAD-HDMI1 had been tested with the MP1 SOM connected to our expansion board, we noticed a different driver implementation: adv7533_mode_fixup is not present, and a similar check is implemented in adv7533_mode_set (commit 62b2f02 from the ST repository). In that version, lane switching is performed only when the number of lanes is 4; the rationale for the change is: “Lower modes on ADV7533 require lower number of DSI lanes for correct operation. If ADV7533 is being used with 4 DSI lanes, then switch the lanes to 3 when the target mode's pixel clock is less than 80 MHz.”

Given the differences between these versions, we investigated the history of drivers/gpu/drm/bridge/adv7511/adv7533.c to understand how it evolved. Starting from commit 62b2f02, the main points are:

• In commit 26ce3f0, it is reported that “dynamic lane switching based on the mode by detaching and attaching the mipi dsi device is prone to deadlock”. To avoid this, adv7533_mode_set is removed and adv7533_mode_valid is added. This is called via the adv7511_bridge_mode_valid callback in mode_valid callback chain. With this approach, the presence of 4 or 3 dsi lanes is only checked to determine whether a mode is valid, but no changes are applied to DSI configuration. The commit also suggests attempting to reintroduce dynamic lane switching using the DRM bridge pre-enable callback chain.
• This is improved in commit f8d28ce, where adv7511_bridge_mode_valid is updated to verify both maximum clock supported by ADV7533/ADV7535 and maximum clock supported per lane.
• In kernel 6.6, commit bd7fe95 introduces adv7533_mode_fixup with the rationale: “Dynamic lane switching is necessary to get all resolutions which require 3 or 4 data lanes.” This motivation seems different from the rationale in commit 62b2f02. In our case, it fails for the reasons described above. Moreover, it reintroduces dynamic detach/attach of the DSI device, despite earlier concerns about deadlocks, and places it in mode_fixup callback chain—which can be invoked repeatedly. In addition, kernel.org documentation appears to discourage using mode_fixup callback for hardware configuration changes.

As first workaround, we implemented a “light” fix by adding a condition so lanes switching is executed only when the adi,dsi-lanes property is 4:

bool adv7533modefixup(struct adv7511 adv,
                        const struct drmdisplaymode mode,
                        struct drmdisplaymode adjustedmode)
{
    struct mipidsidevice dsi = adv->dsi;
    int lanes, ret;

    if (adv->numdsilanes == 4) {
        if (mode->clock > 80000)
            lanes = 4;
        else
            lanes = 3;

        if (lanes != dsi->lanes) {
            mipidsidetach(dsi);
            dsi->lanes = lanes;
            ret = mipidsiattach(dsi);
            if (ret) {
                deverr(&dsi->dev, "failed to change host lanes\n");
                return false;
            }
        }
    }
    return true;

With this change, the display works correctly both on STM32MP257F-EV1 board and on our setup (MP2 SOM + expansion board), and the same three resolutions remain available as before.

However, this solution is only a workaround, since the mode_fixup callback chain can be called repeatedly at runtime. We would like to ask for guidance from whoever maintains the ADV7533 DRM bridge at ST on whether it would be better to move dynamic lane switching to the pre_enable callback chain (as suggested in commit 26ce3f0), remove the mode_fixup callback and understanding whether this approach is feasible or whether there are obstacles or better solutions.

I apologize for the length of the message, but I hope I was clear enough.

Best Regards 

Marco.