Messages in this thread | | | Date | Tue, 30 Apr 2024 11:52:35 +0200 | Subject | Re: [PATCH] arm64: dts: mediatek: mt8192-asurada: Add off-on-delay-us for pp3300_mipibrdg | From | AngeloGioacchino Del Regno <> |
| |
Il 30/04/24 11:32, Pin-yen Lin ha scritto: > Hi Angelo, > > On Tue, Apr 30, 2024 at 4:17 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> Il 29/04/24 11:53, Pin-yen Lin ha scritto: >>> Set off-on-delay-us to 500000 us for pp3300_mipibrdg to make sure it >>> complies with the panel sequence. Explicit configuration on the >>> regulator node is required because mt8192-asurada uses the same power >>> supply for the panel and the anx7625 DP bridge. So powering on/off the >>> DP bridge could break the power sequence requirement for the panel. >>> >>> Fixes: f9f00b1f6b9b ("arm64: dts: mediatek: asurada: Add display regulators") >>> Signed-off-by: Pin-yen Lin <treapking@chromium.org> >>> >> >> Uhm, there might be more to it - I don't think that this should ever happen. >> >> The regulator is refcounted, so... >> * Bridge on: panel goes off, but regulator doesn't turn off (refcount=1) >> * Panel resume -> sequence respected (refcount=2 -> wait -> more vregs, etc) >> * Bridge off: panel is already off (refcount=0) >> * Bridge resume -> refcount=1, no panel commands yet > > The off-on-delay could be violated because the bridge driver does not > check the delay. > >> * Panel resume -> refcount=2, wait -> more vregs, etc >> >> Can you please describe the issue that you're getting? > > The symptom we observed is that the device has a small chance to > reboot to a black panel, and we think the panel's unprepare delay (the > time to power down completely) might not be satisfied because the > bridge doesn't check that when it enables the regulator. Even if the > regulator is enabled by the panel driver, the delay can also be > violated in the following sequence: > > * t=0ms, bridge on: panel goes off, but regulator doesn't turn off > (refcount=1). The .unprepared_time in panel_edp is updated > * t=300ms, bridge off, regulator goes off (refcount=0) > * t=600ms, panel on, the panel driver thinks the unprepare delay > (500ms) is satisfied, but the regulator was disabled 300ms ago. > > Did I miss anything here? Or should I add more detail to the commit message? >
Heh, no you didn't miss anything, this time it's just me :-)
If you can please add that description to the commit message for a v2 that'd be appreciated on my side.
In any case
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> >> Cheers, >> Angelo >> > Regards, > Pin-yen > >>> --- >>> >>> arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi >>> index 7a704246678f..08d71ddf3668 100644 >>> --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi >>> +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi >>> @@ -147,6 +147,7 @@ pp3300_mipibrdg: regulator-3v3-mipibrdg { >>> regulator-boot-on; >>> gpio = <&pio 127 GPIO_ACTIVE_HIGH>; >>> vin-supply = <&pp3300_g>; >>> + off-on-delay-us = <500000>; >>> }; >>> >>> /* separately switched 3.3V power rail */ >>
| |