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: <0a4f3234-ab54-8e2e-0945-b0be159e38a0@quicinc.com>
Date:   Fri, 2 Sep 2022 01:05:22 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.2
Subject: Re: [Freedreno] [PATCH v6 4/6] clk: qcom: gpucc-sc7280: Add cx
 collapse reset support
Content-Language: en-US
To:     Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Philipp Zabel <p.zabel@pengutronix.de>
CC:     Stephen Boyd <sboyd@kernel.org>, <linux-arm-msm@vger.kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Douglas Anderson <dianders@chromium.org>,
        <dri-devel@lists.freedesktop.org>,
        Stephen Boyd <swboyd@chromium.org>,
        <krzysztof.kozlowski@linaro.org>, Rob Clark <robdclark@gmail.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        freedreno <freedreno@lists.freedesktop.org>,
        <linux-clk@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <1661923108-789-1-git-send-email-quic_akhilpo@quicinc.com>
 <20220831104741.v6.4.I5e64ff4b77bb9079eb2edeea8a02585c9e76778f@changeid>
 <20220901103449.GC32271@pengutronix.de>
 <ff2d8784-0cf9-fac9-4360-71cac30fbd76@linaro.org>
From:   Akhil P Oommen <quic_akhilpo@quicinc.com>
In-Reply-To: <ff2d8784-0cf9-fac9-4360-71cac30fbd76@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 9/1/2022 4:16 PM, Dmitry Baryshkov wrote:
> On 01/09/2022 13:34, Philipp Zabel wrote:
>> On Wed, Aug 31, 2022 at 10:48:25AM +0530, Akhil P Oommen wrote:
>>> Allow a consumer driver to poll for cx gdsc collapse through Reset
>>> framework.
>>>
>>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>
>>> (no changes since v3)
>>>
>>> Changes in v3:
>>> - Convert 'struct qcom_reset_ops cx_gdsc_reset' to 'static const' 
>>> (Krzysztof)
>>>
>>> Changes in v2:
>>> - Minor update to use the updated custom reset ops implementation
>>>
>>>   drivers/clk/qcom/gpucc-sc7280.c | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/clk/qcom/gpucc-sc7280.c 
>>> b/drivers/clk/qcom/gpucc-sc7280.c
>>> index 9a832f2..fece3f4 100644
>>> --- a/drivers/clk/qcom/gpucc-sc7280.c
>>> +++ b/drivers/clk/qcom/gpucc-sc7280.c
>>> @@ -433,12 +433,22 @@ static const struct regmap_config 
>>> gpu_cc_sc7280_regmap_config = {
>>>       .fast_io = true,
>>>   };
>>>   +static const struct qcom_reset_ops cx_gdsc_reset = {
>>> +    .reset = gdsc_wait_for_collapse,
>>
>> This should be accompanied by a comment explaining the not-quite-reset
>> nature of this workaround, i.e. what is the prerequisite for this to
>> actually work as expected?
>>
>>> +};
>>> +
>>> +static const struct qcom_reset_map gpucc_sc7280_resets[] = {
>>> +    [GPU_CX_COLLAPSE] = { .ops = &cx_gdsc_reset, .priv = &cx_gdsc },
>>> +};
>>> +
>>>   static const struct qcom_cc_desc gpu_cc_sc7280_desc = {
>>>       .config = &gpu_cc_sc7280_regmap_config,
>>>       .clks = gpu_cc_sc7280_clocks,
>>>       .num_clks = ARRAY_SIZE(gpu_cc_sc7280_clocks),
>>>       .gdscs = gpu_cc_sc7180_gdscs,
>>>       .num_gdscs = ARRAY_SIZE(gpu_cc_sc7180_gdscs),
>>> +    .resets = gpucc_sc7280_resets,
>>> +    .num_resets = ARRAY_SIZE(gpucc_sc7280_resets),
>>
>> See my comment on patch 2. I think instead of adding a const struct
>> qcom_reset_ops * to gpucc_sc7280_resets, this should just add a const
>> struct reset_control * to gpu_cc_sc7280_desc.
>
> While this will work for the sc7280, the platform that Akhil was 
> developing, this will not work for other platforms (like sm8250), 
> where the dispcc also provides traditional BCR resets.
>
Like Dimtry mentioned, we should eventually implement this feature on 
all gpucc drivers and some of them already use the existing reset ops. 
The current implementation creates the least code churn and duplication's.

-Akhil