Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <fdb76743-de20-91c5-2edc-19dd5ead33e3@linaro.org>
Date:   Mon, 8 May 2023 14:32:26 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] clk: qcom: mmcc-msm8974: Add OXILICX_GDSC for msm8226
Content-Language: en-GB
To:     Konrad Dybcio <konrad.dybcio@linaro.org>,
        Luca Weiss <luca@z3ntu.xyz>,
        ~postmarketos/upstreaming@lists.sr.ht, phone-devel@vger.kernel.org,
        Bjorn Andersson <andersson@kernel.org>,
        Andy Gross <agross@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Bartosz Dudziak <bartosz.dudziak@snejp.pl>
Cc:     linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230506-msm8226-oxilicx-v1-1-52e34b94ff22@z3ntu.xyz>
 <cef7b823-451f-e98a-65d3-3e396124071a@linaro.org>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
In-Reply-To: <cef7b823-451f-e98a-65d3-3e396124071a@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 08/05/2023 10:23, Konrad Dybcio wrote:
> 
> 
> On 6.05.2023 23:20, Luca Weiss wrote:
>> On msm8226 we also have OXILICX_GDSC but we need a slighly different
>> config, with a .cxcs defined for clock but with no parent.
> Hm, on newer (a5xx+) GPUs, CX needs to be turned on first and
> to achieve that, we sometimes define it to be the GX's (also
> implicitly known as "oxili-non-CX" in before-a6xx-times) parent..
> 
> Roughly speaking CX powers the "GPU hardware owned by the broader
> SoC that may not need the GPU core clock to be up" and GX powers
> the "GPU hardware owned strictly by the GPU that needs at least some
> GPU clocks to be enabled"
> 
> Maybe 8974 simply has a bug in the driver that would do the reverse?
> Could you (and perhaps Dmitry on his shiny new 13yo board) test that
> theory, preferably on both SoCs?
> 
> --- a/drivers/clk/qcom/mmcc-msm8974.c
> +++ b/drivers/clk/qcom/mmcc-msm8974.c
> @@ -2431,6 +2431,7 @@ static struct gdsc oxili_gdsc = {
>          .pd = {
>                  .name = "oxili",
>          },
> +       .parent = &oxili_gdsc.pd,
>          .pwrsts = PWRSTS_OFF_ON,
>   };

Are you declaring oxili_gdsc to be a parent of itself?

>   
> @@ -2439,7 +2440,6 @@ static struct gdsc oxilicx_gdsc = {
>          .pd = {
>                  .name = "oxilicx",
>          },
> -       .parent = &oxili_gdsc.pd,
>          .pwrsts = PWRSTS_OFF_ON,
>   };
> 
> Konrad
>>
>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
>> ---
>>   drivers/clk/qcom/mmcc-msm8974.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/mmcc-msm8974.c b/drivers/clk/qcom/mmcc-msm8974.c
>> index 4273fce9a4a4..39ee3953567c 100644
>> --- a/drivers/clk/qcom/mmcc-msm8974.c
>> +++ b/drivers/clk/qcom/mmcc-msm8974.c
>> @@ -2443,6 +2443,16 @@ static struct gdsc oxilicx_gdsc = {
>>   	.pwrsts = PWRSTS_OFF_ON,
>>   };
>>   
>> +static struct gdsc oxilicx_gdsc_msm8226 = {
>> +	.gdscr = 0x4034,
>> +	.cxcs = (unsigned int []){ 0x4028 },
>> +	.cxc_count = 1,
>> +	.pd = {
>> +		.name = "oxilicx",
>> +	},
>> +	.pwrsts = PWRSTS_OFF_ON,
>> +};
>> +
>>   static struct clk_regmap *mmcc_msm8226_clocks[] = {
>>   	[MMSS_AHB_CLK_SRC] = &mmss_ahb_clk_src.clkr,
>>   	[MMSS_AXI_CLK_SRC] = &mmss_axi_clk_src.clkr,
>> @@ -2533,6 +2543,7 @@ static struct gdsc *mmcc_msm8226_gdscs[] = {
>>   	[MDSS_GDSC] = &mdss_gdsc,
>>   	[CAMSS_JPEG_GDSC] = &camss_jpeg_gdsc,
>>   	[CAMSS_VFE_GDSC] = &camss_vfe_gdsc,
>> +	[OXILICX_GDSC] = &oxilicx_gdsc_msm8226,
>>   };
>>   
>>   static const struct regmap_config mmcc_msm8226_regmap_config = {
>>
>> ---
>> base-commit: dd9e11d6477a52ede9ebe575c83285e79e823889
>> change-id: 20230506-msm8226-oxilicx-7f3f0f8e491d
>>
>> Best regards,

-- 
With best wishes
Dmitry