[v4,12/19] arm64: dts: mediatek: mt8192-asurada: Couple VGPU and VSRAM_OTHER regulators

Message ID 20230301095523.428461-13-angelogioacchino.delregno@collabora.com
State New
Headers
Series Enable GPU with DVFS support on MediaTek SoCs |

Commit Message

AngeloGioacchino Del Regno March 1, 2023, 9:55 a.m. UTC
  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(+)
  

Comments

Chen-Yu Tsai March 2, 2023, 10:03 a.m. UTC | #1
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.

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.

ChenYu


> +};
> +
>  &mt6359_vufs_ldo_reg {
>         regulator-always-on;
>  };
> @@ -1411,6 +1418,8 @@ mt6315_7_vbuck1: vbuck1 {
>                                 regulator-max-microvolt = <800000>;
>                                 regulator-enable-ramp-delay = <256>;
>                                 regulator-allowed-modes = <0 1 2>;
> +                               regulator-coupled-with = <&mt6359_vsram_others_ldo_reg>;
> +                               regulator-coupled-max-spread = <10000>;
>                         };
>                 };
>         };
> --
> 2.39.2
>
  
AngeloGioacchino Del Regno March 2, 2023, 10:17 a.m. UTC | #2
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.

> 
> 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?

Angelo

> ChenYu
> 
> 
>> +};
>> +
>>   &mt6359_vufs_ldo_reg {
>>          regulator-always-on;
>>   };
>> @@ -1411,6 +1418,8 @@ mt6315_7_vbuck1: vbuck1 {
>>                                  regulator-max-microvolt = <800000>;
>>                                  regulator-enable-ramp-delay = <256>;
>>                                  regulator-allowed-modes = <0 1 2>;
>> +                               regulator-coupled-with = <&mt6359_vsram_others_ldo_reg>;
>> +                               regulator-coupled-max-spread = <10000>;
>>                          };
>>                  };
>>          };
>> --
>> 2.39.2
>>
  
Chen-Yu Tsai March 3, 2023, 4:09 a.m. UTC | #3
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?

ChenYu
  
Chen-Yu Tsai March 7, 2023, 9:24 a.m. UTC | #4
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?

ChenYu
  
AngeloGioacchino Del Regno March 7, 2023, 9:30 a.m. UTC | #5
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?

Regards,
Angelo
  
Chen-Yu Tsai March 7, 2023, 9:44 a.m. UTC | #6
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>
  
AngeloGioacchino Del Regno March 7, 2023, 9:47 a.m. UTC | #7
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
  

Patch

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>;
+};
+
 &mt6359_vufs_ldo_reg {
 	regulator-always-on;
 };
@@ -1411,6 +1418,8 @@  mt6315_7_vbuck1: vbuck1 {
 				regulator-max-microvolt = <800000>;
 				regulator-enable-ramp-delay = <256>;
 				regulator-allowed-modes = <0 1 2>;
+				regulator-coupled-with = <&mt6359_vsram_others_ldo_reg>;
+				regulator-coupled-max-spread = <10000>;
 			};
 		};
 	};