Messages in this thread | | | Date | Tue, 7 Mar 2023 10:47:39 +0100 | Subject | Re: [PATCH v4 12/19] arm64: dts: mediatek: mt8192-asurada: Couple VGPU and VSRAM_OTHER regulators | From | AngeloGioacchino Del Regno <> |
| |
Il 07/03/23 10:44, Chen-Yu Tsai ha scritto: > On Tue, Mar 7, 2023 at 5:30 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> Il 07/03/23 10:24, Chen-Yu Tsai ha scritto: >>> On Fri, Mar 3, 2023 at 12:09 PM Chen-Yu Tsai <wenst@chromium.org> wrote: >>>> >>>> On Thu, Mar 2, 2023 at 6:17 PM AngeloGioacchino Del Regno >>>> <angelogioacchino.delregno@collabora.com> wrote: >>>>> >>>>> Il 02/03/23 11:03, Chen-Yu Tsai ha scritto: >>>>>> On Wed, Mar 1, 2023 at 5:55 PM AngeloGioacchino Del Regno >>>>>> <angelogioacchino.delregno@collabora.com> wrote: >>>>>>> >>>>>>> Add coupling for these regulators, as VSRAM_OTHER is used to power the >>>>>>> GPU SRAM, and they have a strict voltage output relation to satisfy in >>>>>>> order to ensure GPU stable operation. >>>>>>> While at it, also add voltage constraint overrides for the GPU SRAM >>>>>>> regulator "mt6359_vsram_others" so that we stay in a safe range of >>>>>>> 0.75-0.80V. >>>>>>> >>>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >>>>>>> --- >>>>>>> arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi | 9 +++++++++ >>>>>>> 1 file changed, 9 insertions(+) >>>>>>> >>>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi >>>>>>> index 8570b78c04a4..f858eca219d7 100644 >>>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi >>>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi >>>>>>> @@ -447,6 +447,13 @@ &mt6359_vrf12_ldo_reg { >>>>>>> regulator-always-on; >>>>>>> }; >>>>>>> >>>>>>> +&mt6359_vsram_others_ldo_reg { >>>>>>> + regulator-min-microvolt = <750000>; >>>>>>> + regulator-max-microvolt = <800000>; >>>>>>> + regulator-coupled-with = <&mt6315_7_vbuck1>; >>>>>>> + regulator-coupled-max-spread = <10000>; >>>>>> >>>>>> Looking again at the downstream OPP table, it seems there's no voltage >>>>>> difference requirement. It only needs V_SRAM >= V_GPU. Same applies to >>>>>> MT8195. Looks like only MT8183 and MT8186 need V_SRAM - V_GPU >= 10000. >>>>> >>>>> On MT8195 we don't need any regulator coupling. There, the GPU-SRAM voltage >>>>> is fixed at .. I don't remember, 0.7V? - anyway - MT8195 doesn't need to >>>>> scale the vsram. >>>> >>>> Looks like it's fixed at 0.75V. I guess we're Ok on MT8195. >>>> >>>>>> >>>>>> Would setting max-spread to 0 work? I ask because with both regulator's >>>>>> maximum voltage set to 0.8V, there's no way we can reach the highest >>>>>> OPP. >>>>>> >>>>> >>>>> No that doesn't work. I can raise the Vgpu max voltage to 0.88V to solve the >>>>> issue right here and right now, or we can leave it like that and revisit it >>>>> later. >>>>> >>>>> I would at this point go for setting mt6315_7_vbuck1's max-microvolt to >>>>> 880000, as this is the maximum recommended voltage for the GPU as per the >>>>> MT8192 datasheet, it would also make sense as we would be still describing >>>>> the hardware in a correct manner. >>>>> >>>>> What do you think? >>>> >>>> If it's just to accommodate the coupler stuff, I say just set the maximum >>>> at the lowest possible setting that satisfies the coupler constraint and >>>> granularity of the regulator. The regulator does 6250 uV steps, so I guess >>>> we could set the maximum at 812500 uV, with a comment stating the nominal >>>> voltage of 800000 uV and that the extra 12500 uV is to workaround coupler >>>> limitations. >>>> >>>> Does that sound OK? >>> >>> Even without changing anything, the coupler seems to work OK: >>> >>> vsram_others 1 1 0 normal 800mV >>> 0mA 750mV 800mV >>> 10006000.syscon:power-controller-domain 1 >>> 0mA 0mV 0mV >>> Vgpu 2 2 0 normal 800mV >>> 0mA 606mV 800mV >>> 13000000.gpu-mali 1 >>> 0mA 800mV 800mV >>> 10006000.syscon:power-controller-domain 1 >>> 0mA 0mV 0mV >>> >>> Am I missing something? >>> >> >> I don't think you are... I may be getting confused by all of the changesets >> that I'm pushing at once. >> >> Hence, is this commit fine as it is? > > It works for some reason. Maybe it's a bug in the coupler. Either way I > think it works, even though the numbers might be a bit off. We can revisit > it later. > > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
Thanks!
Angelo
| |