[PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface

[PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface

[Akhil P Oommen]

Some clients like adreno gpu driver would like to ensure that its gdsc
is collapsed at hardware during a gpu reset sequence. This is because it
has a votable gdsc which could be ON due to a vote from another subsystem
like tz, hyp etc or due to an internal hardware signal. To allow
this, gpucc driver can expose an interface to the client driver using
reset framework. Using this the client driver can trigger a polling within
the gdsc driver.

This series is rebased on top of linus's master branch.

Related discussion: https://patchwork.freedesktop.org/patch/493144/


Akhil P Oommen (5):
  dt-bindings: clk: qcom: Support gpu cx gdsc reset
  clk: qcom: Allow custom reset ops
  clk: qcom: gpucc-sc7280: Add cx collapse reset support
  clk: qcom: gdsc: Add a reset op to poll gdsc collapse
  arm64: dts: qcom: sc7280: Add Reset support for gpu

 arch/arm64/boot/dts/qcom/sc7280.dtsi          |  3 +++
 drivers/clk/qcom/gdsc.c                       | 23 +++++++++++++++++++----
 drivers/clk/qcom/gdsc.h                       |  7 +++++++
 drivers/clk/qcom/gpucc-sc7280.c               |  6 ++++++
 drivers/clk/qcom/reset.c                      |  6 ++++++
 drivers/clk/qcom/reset.h                      |  2 ++
 include/dt-bindings/clock/qcom,gpucc-sc7280.h |  3 +++
 7 files changed, 46 insertions(+), 4 deletions(-)

-- 
2.7.4

[PATCH 1/5] dt-bindings: clk: qcom: Support gpu cx gdsc reset

[Akhil P Oommen]

Add necessary definitions in gpucc bindings to ensure gpu cx gdsc collapse
through 'reset' framework for SC7280.

Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---

 include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,gpucc-sc7280.h b/include/dt-bindings/clock/qcom,gpucc-sc7280.h
index 669b23b..843a31b 100644
--- a/include/dt-bindings/clock/qcom,gpucc-sc7280.h
+++ b/include/dt-bindings/clock/qcom,gpucc-sc7280.h
@@ -32,4 +32,7 @@
 #define GPU_CC_CX_GDSC				0
 #define GPU_CC_GX_GDSC				1
 
+/* GPU_CC reset IDs */
+#define GPU_CX_COLLAPSE				0
+
 #endif
-- 
2.7.4

Re: [PATCH 1/5] dt-bindings: clk: qcom: Support gpu cx gdsc reset

[Krzysztof Kozlowski]

On 30/07/2022 11:17, Akhil P Oommen wrote:
> Add necessary definitions in gpucc bindings to ensure gpu cx gdsc collapse
> through 'reset' framework for SC7280.
> 
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---

Assuming discussion in cover letter sorts out:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

[PATCH 5/5] arm64: dts: qcom: sc7280: Add Reset support for gpu

[Akhil P Oommen]

Add support for Reset using GPUCC driver for GPU. This helps to ensure
that GPU state is reset by making sure that CX head switch is collapsed.

Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---

 arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index e66fc67..f5257d6 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2243,6 +2243,9 @@
 			nvmem-cells = <&gpu_speed_bin>;
 			nvmem-cell-names = "speed_bin";
 
+			resets = <&gpucc GPU_CX_COLLAPSE>;
+			reset-names = "cx_collapse";
+
 			gpu_opp_table: opp-table {
 				compatible = "operating-points-v2";
 
-- 
2.7.4

Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface

[Dmitry Baryshkov]

On 30/07/2022 12:17, Akhil P Oommen wrote:
> 
> Some clients like adreno gpu driver would like to ensure that its gdsc
> is collapsed at hardware during a gpu reset sequence. This is because it
> has a votable gdsc which could be ON due to a vote from another subsystem
> like tz, hyp etc or due to an internal hardware signal.

If this is votable, do we have any guarantee that the gdsc will collapse 
at all? How can we proceed if it did not collapse?

> To allow
> this, gpucc driver can expose an interface to the client driver using
> reset framework. Using this the client driver can trigger a polling within
> the gdsc driver.

Trigger the polling made me think initially that we will actually 
trigger something in the HW. Instead the client uses reset framework to 
poll for the gdsc to be reset.

> 
> This series is rebased on top of linus's master branch.
> 
> Related discussion: https://patchwork.freedesktop.org/patch/493144/
> 
> 
> Akhil P Oommen (5):
>    dt-bindings: clk: qcom: Support gpu cx gdsc reset
>    clk: qcom: Allow custom reset ops
>    clk: qcom: gpucc-sc7280: Add cx collapse reset support
>    clk: qcom: gdsc: Add a reset op to poll gdsc collapse
>    arm64: dts: qcom: sc7280: Add Reset support for gpu
> 
>   arch/arm64/boot/dts/qcom/sc7280.dtsi          |  3 +++
>   drivers/clk/qcom/gdsc.c                       | 23 +++++++++++++++++++----
>   drivers/clk/qcom/gdsc.h                       |  7 +++++++
>   drivers/clk/qcom/gpucc-sc7280.c               |  6 ++++++
>   drivers/clk/qcom/reset.c                      |  6 ++++++
>   drivers/clk/qcom/reset.h                      |  2 ++
>   include/dt-bindings/clock/qcom,gpucc-sc7280.h |  3 +++
>   7 files changed, 46 insertions(+), 4 deletions(-)
> 


-- 
With best wishes
Dmitry

Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface

[Rob Clark]

On Tue, Aug 2, 2022 at 12:02 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 30/07/2022 12:17, Akhil P Oommen wrote:
> >
> > Some clients like adreno gpu driver would like to ensure that its gdsc
> > is collapsed at hardware during a gpu reset sequence. This is because it
> > has a votable gdsc which could be ON due to a vote from another subsystem
> > like tz, hyp etc or due to an internal hardware signal.
>
> If this is votable, do we have any guarantee that the gdsc will collapse
> at all? How can we proceed if it did not collapse?

Other potential votes should be transient.  But I guess we eventually
need to timeout and give up.  At which point we are no worse off than
before.

But hmm, we aren't using RBBM_SW_RESET_CMD for sw reset like we have
on previous generations?  That does seem a bit odd.  Looks like kgsl
does use it.

BR,
-R

> > To allow
> > this, gpucc driver can expose an interface to the client driver using
> > reset framework. Using this the client driver can trigger a polling within
> > the gdsc driver.
>
> Trigger the polling made me think initially that we will actually
> trigger something in the HW. Instead the client uses reset framework to
> poll for the gdsc to be reset.
>
> >
> > This series is rebased on top of linus's master branch.
> >
> > Related discussion: https://patchwork.freedesktop.org/patch/493144/
> >
> >
> > Akhil P Oommen (5):
> >    dt-bindings: clk: qcom: Support gpu cx gdsc reset
> >    clk: qcom: Allow custom reset ops
> >    clk: qcom: gpucc-sc7280: Add cx collapse reset support
> >    clk: qcom: gdsc: Add a reset op to poll gdsc collapse
> >    arm64: dts: qcom: sc7280: Add Reset support for gpu
> >
> >   arch/arm64/boot/dts/qcom/sc7280.dtsi          |  3 +++
> >   drivers/clk/qcom/gdsc.c                       | 23 +++++++++++++++++++----
> >   drivers/clk/qcom/gdsc.h                       |  7 +++++++
> >   drivers/clk/qcom/gpucc-sc7280.c               |  6 ++++++
> >   drivers/clk/qcom/reset.c                      |  6 ++++++
> >   drivers/clk/qcom/reset.h                      |  2 ++
> >   include/dt-bindings/clock/qcom,gpucc-sc7280.h |  3 +++
> >   7 files changed, 46 insertions(+), 4 deletions(-)
> >
>
>
> --
> With best wishes
> Dmitry

Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface

[Akhil P Oommen]

On 8/3/2022 12:02 AM, Rob Clark wrote:
> On Tue, Aug 2, 2022 at 12:02 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>> On 30/07/2022 12:17, Akhil P Oommen wrote:
>>> Some clients like adreno gpu driver would like to ensure that its gdsc
>>> is collapsed at hardware during a gpu reset sequence. This is because it
>>> has a votable gdsc which could be ON due to a vote from another subsystem
>>> like tz, hyp etc or due to an internal hardware signal.
>> If this is votable, do we have any guarantee that the gdsc will collapse
>> at all? How can we proceed if it did not collapse?
> Other potential votes should be transient.  But I guess we eventually
> need to timeout and give up.  At which point we are no worse off than
> before.
>
> But hmm, we aren't using RBBM_SW_RESET_CMD for sw reset like we have
> on previous generations?  That does seem a bit odd.  Looks like kgsl
> does use it.
>
> BR,
> -R
Like Rob mentioned there could be transient votes from other 
clients/subsystem. It could be even stuck ON when hardware is in bad 
shape in some very rare cases. For the worst case scenario, I have added 
a timeout (500msec) in the gdsc reset op.

I have added the Soft reset in [1]. But this resets only the core gpu 
blocks, not everything. For eg. GMU.

[1] [PATCH v3 7/8] drm/msm/a6xx: Improve gpu recovery sequence

>
>>> To allow
>>> this, gpucc driver can expose an interface to the client driver using
>>> reset framework. Using this the client driver can trigger a polling within
>>> the gdsc driver.
>> Trigger the polling made me think initially that we will actually
>> trigger something in the HW. Instead the client uses reset framework to
>> poll for the gdsc to be reset.
Yes. I should replace 'trigger' with 'start' here.

-Akhil.
>>
>>> This series is rebased on top of linus's master branch.
>>>
>>> Related discussion: https://patchwork.freedesktop.org/patch/493144/
>>>
>>>
>>> Akhil P Oommen (5):
>>>     dt-bindings: clk: qcom: Support gpu cx gdsc reset
>>>     clk: qcom: Allow custom reset ops
>>>     clk: qcom: gpucc-sc7280: Add cx collapse reset support
>>>     clk: qcom: gdsc: Add a reset op to poll gdsc collapse
>>>     arm64: dts: qcom: sc7280: Add Reset support for gpu
>>>
>>>    arch/arm64/boot/dts/qcom/sc7280.dtsi          |  3 +++
>>>    drivers/clk/qcom/gdsc.c                       | 23 +++++++++++++++++++----
>>>    drivers/clk/qcom/gdsc.h                       |  7 +++++++
>>>    drivers/clk/qcom/gpucc-sc7280.c               |  6 ++++++
>>>    drivers/clk/qcom/reset.c                      |  6 ++++++
>>>    drivers/clk/qcom/reset.h                      |  2 ++
>>>    include/dt-bindings/clock/qcom,gpucc-sc7280.h |  3 +++
>>>    7 files changed, 46 insertions(+), 4 deletions(-)
>>>
>>
>> --
>> With best wishes
>> Dmitry

Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface

[Bjorn Andersson]

On Sat 30 Jul 04:17 CDT 2022, Akhil P Oommen wrote:

> 
> Some clients like adreno gpu driver would like to ensure that its gdsc
> is collapsed at hardware during a gpu reset sequence. This is because it
> has a votable gdsc which could be ON due to a vote from another subsystem
> like tz, hyp etc or due to an internal hardware signal. To allow
> this, gpucc driver can expose an interface to the client driver using
> reset framework. Using this the client driver can trigger a polling within
> the gdsc driver.
> 
> This series is rebased on top of linus's master branch.
> 
> Related discussion: https://patchwork.freedesktop.org/patch/493144/
> 

Forgive me if I'm assuming too much, but isn't this an extension of:

85a3d920d30a ("clk: qcom: Add a dummy enable function for GX gdsc")

With the additional requirement that disable should really ensure that
the GDSC is turned off?

Regards,
Bjorn

> 
> Akhil P Oommen (5):
>   dt-bindings: clk: qcom: Support gpu cx gdsc reset
>   clk: qcom: Allow custom reset ops
>   clk: qcom: gpucc-sc7280: Add cx collapse reset support
>   clk: qcom: gdsc: Add a reset op to poll gdsc collapse
>   arm64: dts: qcom: sc7280: Add Reset support for gpu
> 
>  arch/arm64/boot/dts/qcom/sc7280.dtsi          |  3 +++
>  drivers/clk/qcom/gdsc.c                       | 23 +++++++++++++++++++----
>  drivers/clk/qcom/gdsc.h                       |  7 +++++++
>  drivers/clk/qcom/gpucc-sc7280.c               |  6 ++++++
>  drivers/clk/qcom/reset.c                      |  6 ++++++
>  drivers/clk/qcom/reset.h                      |  2 ++
>  include/dt-bindings/clock/qcom,gpucc-sc7280.h |  3 +++
>  7 files changed, 46 insertions(+), 4 deletions(-)
> 
> -- 
> 2.7.4
> 

Re: [PATCH 0/5] clk/qcom: Support gdsc collapse polling using 'reset' inteface

[Akhil P Oommen]

On 8/10/2022 2:35 AM, Bjorn Andersson wrote:
> On Sat 30 Jul 04:17 CDT 2022, Akhil P Oommen wrote:
>
>> Some clients like adreno gpu driver would like to ensure that its gdsc
>> is collapsed at hardware during a gpu reset sequence. This is because it
>> has a votable gdsc which could be ON due to a vote from another subsystem
>> like tz, hyp etc or due to an internal hardware signal. To allow
>> this, gpucc driver can expose an interface to the client driver using
>> reset framework. Using this the client driver can trigger a polling within
>> the gdsc driver.
>>
>> This series is rebased on top of linus's master branch.
>>
>> Related discussion: https://patchwork.freedesktop.org/patch/493144/
>>
> Forgive me if I'm assuming too much, but isn't this an extension of:
>
> 85a3d920d30a ("clk: qcom: Add a dummy enable function for GX gdsc")
>
> With the additional requirement that disable should really ensure that
> the GDSC is turned off?
Also, gpu driver needs a way to ensure cx gdsc was collapsed at least 
once before it goes ahead with re-init.

Btw, the patch you mentioned is about gx gdsc in gpucc which is supposed 
to be owned by gmu (except when it is in bad shape). But the current 
series is about cx gdsc which is shared with other subsystems/drivers.

-Akhil.
>
> Regards,
> Bjorn
>
>> Akhil P Oommen (5):
>>    dt-bindings: clk: qcom: Support gpu cx gdsc reset
>>    clk: qcom: Allow custom reset ops
>>    clk: qcom: gpucc-sc7280: Add cx collapse reset support
>>    clk: qcom: gdsc: Add a reset op to poll gdsc collapse
>>    arm64: dts: qcom: sc7280: Add Reset support for gpu
>>
>>   arch/arm64/boot/dts/qcom/sc7280.dtsi          |  3 +++
>>   drivers/clk/qcom/gdsc.c                       | 23 +++++++++++++++++++----
>>   drivers/clk/qcom/gdsc.h                       |  7 +++++++
>>   drivers/clk/qcom/gpucc-sc7280.c               |  6 ++++++
>>   drivers/clk/qcom/reset.c                      |  6 ++++++
>>   drivers/clk/qcom/reset.h                      |  2 ++
>>   include/dt-bindings/clock/qcom,gpucc-sc7280.h |  3 +++
>>   7 files changed, 46 insertions(+), 4 deletions(-)
>>
>> -- 
>> 2.7.4
>>