On 29.02.24 10:47, Luca Ceresoli wrote:
> Hello Alexander,
>
> On Wed, 28 Feb 2024 09:15:46 +0100
> Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx> wrote:
>
>
> [...]
>
>> Oh I mistook this DSI-LVDS bridge with the DSI-DP bridge on a different
>> board, my bad. I hope I can provide some insights. My platform is
>> imx8mm-tqma8mqml-mba8mx-lvds-tm070jvhg33.dtb.
>> I can easily cause a PLL lock failure by reducing the delay for the
>> enable-gpios 'gpio_delays'. This will result in a PLL lock faiure.
>> On my platform the vcc-supply counters do look sane:
>>> /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
>>> /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:0
>
> Interesting. Thanks for taking time to report your initial issue!
>
>> Once I remove the ti_sn65dsi83 module, the open_count decrements to 0 as
>> well. Looks sane to me.
>>
>> If I revert commit c81cd8f7c774 ("Revert "drm/bridge: ti-sn65dsi83:
>> Fix enable error path""), vcc-supply counters are:
>>> /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1
>>> /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:1
>>
>> So in my case the use_count does not decrease! If I remove the module
>> ti_sn65dsi83, I get the WARN_ON (enable_count is still non-zero):
>>> WARNING: CPU: 2 PID: 402 at drivers/regulator/core.c:2398 _regulator_put+0x15c/0x164
>>
>> This is on 6.8.0-rc6-next-20240228 with the following diff applied:
>> --->8---
>> diff --git a/arch/arm64/boot/dts/freescale/mba8mx.dtsi b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
>> index 427467df42bf..8461e1fd396f 100644
>> --- a/arch/arm64/boot/dts/freescale/mba8mx.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/mba8mx.dtsi
>> @@ -285,7 +285,7 @@ &i2c3 {
>> dsi_lvds_bridge: bridge@2d {
>> compatible = "ti,sn65dsi84";
>> reg = <0x2d>;
>> - enable-gpios = <&gpio_delays 0 130000 0>;
>> + enable-gpios = <&gpio_delays 0 0 0>;
>> vcc-supply = <®_sn65dsi83_1v8>;
>> status = "disabled";
>>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> index 4814b7b6d1fd..57a7ed13f996 100644
>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> @@ -478,7 +478,6 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
>> dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
>> /* On failure, disable PLL again and exit. */
>> regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
>> - regulator_disable(ctx->vcc);
>> return;
>> }
>> --->8---
>>
>> So my patch indeed did fix an actual problem. On the other hand it seems
>> sn65dsi83_atomic_disable is not called in my case for some reason.
>
> So you remove the module and atomic_disable is not called, after
> having called atomic_pre_enable?
>
> I'm very possibly missing something, but this looks like a bug in the
> DRM bridge code at first sight.
I'm just guessing, but could it be that this patch [1] would fix it?
It looks like nobody cared to pick this up, although there are several
reports for defects caused by [2] and this patch is supposed to fix them.
[1] https://patchwork.freedesktop.org/patch/529288/
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=4fb912e5e19075874379cfcf074d90bd51ebf8ea
