Re: [PATCH 1/4] ARM: dts: qcom: msm8974: add ocmem clock to GPU

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 8.05.2023 12:32, Dmitry Baryshkov wrote:
> On 08/05/2023 13:28, Konrad Dybcio wrote:
>>
>>
>> On 8.05.2023 12:24, Dmitry Baryshkov wrote:
>>> On 08/05/2023 11:30, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 7.05.2023 21:07, Dmitry Baryshkov wrote:
>>>>> To get GPU working with the OCMEM, the oxili_ocmemgx_clk clock should be
>>>>> enabled. Pass it to the GPU to get it to work on apq8074/msm8974 boards.
>>>>>
>>>>> Fixes: fe079442db63 ("ARM: dts: qcom: msm8974: add gpu support")
>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>> ---
>>>> Uhh.. this clock should be managed by RPM with:
>>>>
>>>> type = MEM
>>>> id = 2
>>>>
>>>> just like it's defined for 8994 today.
>>>>
>>>> Could you try adding it in rpmcc and confirming?
>>>
>>> There is an RPM_SMD_OCMEMGX_CLK clk already, it is a bus clock for the ocmemnoc.
>> It's wrong.
> 
> Can you possibly be more specific here?
> 
> On 8974 RPM_SMD_OCMEMGX_CLK is a core clock for ocmem and a bus clock for ocmemnoc.
The ocmemnoc bus is managed internally, the rpm clock is not used for scaling,
only to make sure the NIU is clocked for ocmem node access.

It's "scaled" with the ocmem-slv node, but the code never sets any rate,
only calls clk_get.

> 
> On 8994 this clocks is just a core clock for ocmem, there is no ocmemnoc.
It's there, named "fab-ovirt" and on 8994 it's actually used for scaling.

Konrad
> 
> 
>>
>> Konrad
>>>
>>>>
>>>> Konrad
>>>>>    arch/arm/boot/dts/qcom-msm8974.dtsi | 3 ++-
>>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
>>>>> index 834ad95515b1..fb661c1bd3d5 100644
>>>>> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
>>>>> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
>>>>> @@ -2104,8 +2104,9 @@ gpu: adreno@fdb00000 {
>>>>>                  clocks = <&mmcc OXILI_GFX3D_CLK>,
>>>>>                     <&mmcc OXILICX_AHB_CLK>,
>>>>> +                 <&mmcc OXILI_OCMEMGX_CLK>,
>>>>>                     <&mmcc OXILICX_AXI_CLK>;
>>>>> -            clock-names = "core", "iface", "mem_iface";
>>>>> +            clock-names = "core", "iface", "mem", "mem_iface";
>>>>>                  sram = <&gmu_sram>;
>>>>>                power-domains = <&mmcc OXILICX_GDSC>;
>>>
>