Linux-Clk Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 1/2] dt-bindings: clk: qcom: Add support for GPU GX GDSCR
@ 2020-02-10  4:01 Taniya Das
  2020-02-10  4:01 ` [PATCH v2 2/2] clk: qcom: gpucc: Add support for GX GDSC for SC7180 Taniya Das
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Taniya Das @ 2020-02-10  4:01 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette, robh
  Cc: David Brown, Rajendra Nayak, linux-arm-msm, linux-soc, linux-clk,
	linux-kernel, Andy Gross, devicetree, robh+dt, Taniya Das

In the cases where the GPU SW requires to use the GX GDSCR add
support for the same.

Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 include/dt-bindings/clock/qcom,gpucc-sc7180.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/dt-bindings/clock/qcom,gpucc-sc7180.h b/include/dt-bindings/clock/qcom,gpucc-sc7180.h
index 0e4643b..65e706d 100644
--- a/include/dt-bindings/clock/qcom,gpucc-sc7180.h
+++ b/include/dt-bindings/clock/qcom,gpucc-sc7180.h
@@ -15,7 +15,8 @@
 #define GPU_CC_CXO_CLK			6
 #define GPU_CC_GMU_CLK_SRC		7
 
-/* CAM_CC GDSCRs */
+/* GPU_CC GDSCRs */
 #define CX_GDSC				0
+#define GX_GDSC				1
 
 #endif
-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the  Linux Foundation.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 2/2] clk: qcom: gpucc: Add support for GX GDSC for SC7180
  2020-02-10  4:01 [PATCH v2 1/2] dt-bindings: clk: qcom: Add support for GPU GX GDSCR Taniya Das
@ 2020-02-10  4:01 ` Taniya Das
  2020-02-10 17:49   ` Doug Anderson
  2020-02-12 23:04   ` Stephen Boyd
  2020-02-10 17:55 ` [PATCH v2 1/2] dt-bindings: clk: qcom: Add support for GPU GX GDSCR Doug Anderson
  2020-02-12 23:04 ` Stephen Boyd
  2 siblings, 2 replies; 7+ messages in thread
From: Taniya Das @ 2020-02-10  4:01 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette, robh
  Cc: David Brown, Rajendra Nayak, linux-arm-msm, linux-soc, linux-clk,
	linux-kernel, Andy Gross, devicetree, robh+dt, Taniya Das

Most of the time the CPU should not be touching the GX domain on the
GPU except for a very special use case when the CPU needs to force the
GX headswitch off. Add the GX domain for that use case.  As part of
this add a dummy enable function for the GX gdsc to simulate success
so that the pm_runtime reference counting is correct.  This matches
what was done in sdm845 in commit 85a3d920d30a ("clk: qcom: Add a
dummy enable function for GX gdsc").

Signed-off-by: Taniya Das <tdas@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
 drivers/clk/qcom/gpucc-sc7180.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/clk/qcom/gpucc-sc7180.c b/drivers/clk/qcom/gpucc-sc7180.c
index a96c0b9..7b656b6 100644
--- a/drivers/clk/qcom/gpucc-sc7180.c
+++ b/drivers/clk/qcom/gpucc-sc7180.c
@@ -170,8 +170,45 @@ static struct gdsc cx_gdsc = {
 	.flags = VOTABLE,
 };

+/*
+ * On SC7180 the GPU GX domain is *almost* entirely controlled by the GMU
+ * running in the CX domain so the CPU doesn't need to know anything about the
+ * GX domain EXCEPT....
+ *
+ * Hardware constraints dictate that the GX be powered down before the CX. If
+ * the GMU crashes it could leave the GX on. In order to successfully bring back
+ * the device the CPU needs to disable the GX headswitch. There being no sane
+ * way to reach in and touch that register from deep inside the GPU driver we
+ * need to set up the infrastructure to be able to ensure that the GPU can
+ * ensure that the GX is off during this super special case. We do this by
+ * defining a GX gdsc with a dummy enable function and a "default" disable
+ * function.
+ *
+ * This allows us to attach with genpd_dev_pm_attach_by_name() in the GPU
+ * driver. During power up, nothing will happen from the CPU (and the GMU will
+ * power up normally but during power down this will ensure that the GX domain
+ * is *really* off - this gives us a semi standard way of doing what we need.
+ */
+static int gx_gdsc_enable(struct generic_pm_domain *domain)
+{
+	/* Do nothing but give genpd the impression that we were successful */
+	return 0;
+}
+
+static struct gdsc gx_gdsc = {
+	.gdscr = 0x100c,
+	.clamp_io_ctrl = 0x1508,
+	.pd = {
+		.name = "gx_gdsc",
+		.power_on = gx_gdsc_enable,
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = CLAMP_IO,
+};
+
 static struct gdsc *gpu_cc_sc7180_gdscs[] = {
 	[CX_GDSC] = &cx_gdsc,
+	[GX_GDSC] = &gx_gdsc,
 };

 static struct clk_regmap *gpu_cc_sc7180_clocks[] = {
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the  Linux Foundation.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] clk: qcom: gpucc: Add support for GX GDSC for SC7180
  2020-02-10  4:01 ` [PATCH v2 2/2] clk: qcom: gpucc: Add support for GX GDSC for SC7180 Taniya Das
@ 2020-02-10 17:49   ` Doug Anderson
  2020-02-11  2:50     ` Taniya Das
  2020-02-12 23:04   ` Stephen Boyd
  1 sibling, 1 reply; 7+ messages in thread
From: Doug Anderson @ 2020-02-10 17:49 UTC (permalink / raw)
  To: Taniya Das
  Cc: Stephen Boyd, Michael Turquette, Rob Herring, David Brown,
	Rajendra Nayak, linux-arm-msm, open list:ARM/QUALCOMM SUPPORT,
	linux-clk, LKML, Andy Gross,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rob Herring

Hi,

On Sun, Feb 9, 2020 at 8:01 PM Taniya Das <tdas@codeaurora.org> wrote:
>
> Most of the time the CPU should not be touching the GX domain on the
> GPU except for a very special use case when the CPU needs to force the
> GX headswitch off. Add the GX domain for that use case.  As part of
> this add a dummy enable function for the GX gdsc to simulate success
> so that the pm_runtime reference counting is correct.  This matches
> what was done in sdm845 in commit 85a3d920d30a ("clk: qcom: Add a
> dummy enable function for GX gdsc").
>
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

For future reference, if you have someone's tag in your commit message
it's nice to CC them on the email.


> ---
>  drivers/clk/qcom/gpucc-sc7180.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/drivers/clk/qcom/gpucc-sc7180.c b/drivers/clk/qcom/gpucc-sc7180.c
> index a96c0b9..7b656b6 100644
> --- a/drivers/clk/qcom/gpucc-sc7180.c
> +++ b/drivers/clk/qcom/gpucc-sc7180.c
> @@ -170,8 +170,45 @@ static struct gdsc cx_gdsc = {
>         .flags = VOTABLE,
>  };
>
> +/*
> + * On SC7180 the GPU GX domain is *almost* entirely controlled by the GMU
> + * running in the CX domain so the CPU doesn't need to know anything about the
> + * GX domain EXCEPT....
> + *
> + * Hardware constraints dictate that the GX be powered down before the CX. If
> + * the GMU crashes it could leave the GX on. In order to successfully bring back
> + * the device the CPU needs to disable the GX headswitch. There being no sane
> + * way to reach in and touch that register from deep inside the GPU driver we
> + * need to set up the infrastructure to be able to ensure that the GPU can
> + * ensure that the GX is off during this super special case. We do this by
> + * defining a GX gdsc with a dummy enable function and a "default" disable
> + * function.
> + *
> + * This allows us to attach with genpd_dev_pm_attach_by_name() in the GPU
> + * driver. During power up, nothing will happen from the CPU (and the GMU will
> + * power up normally but during power down this will ensure that the GX domain
> + * is *really* off - this gives us a semi standard way of doing what we need.
> + */
> +static int gx_gdsc_enable(struct generic_pm_domain *domain)
> +{
> +       /* Do nothing but give genpd the impression that we were successful */
> +       return 0;
> +}
> +
> +static struct gdsc gx_gdsc = {
> +       .gdscr = 0x100c,
> +       .clamp_io_ctrl = 0x1508,
> +       .pd = {
> +               .name = "gx_gdsc",
> +               .power_on = gx_gdsc_enable,
> +       },
> +       .pwrsts = PWRSTS_OFF_ON,
> +       .flags = CLAMP_IO,

In my previous reply [1], I asked about these flags and if it was
intentional that they were different from sdm845.  I did see a private
response, but no public one.  In the future note that it's good to
reply publicly so everyone understands what happened.  In this case, I
was told "the GDSC's on 845 and SC7180 are different and hence the
change in flags is expected".  That answers my question and thus I'm
fine with my tag being here.  It also looks like you took my other
review feedback on v1, which is nice.


-Doug


[1] https://lore.kernel.org/r/CAD=FV=V6yM7UJwu0ZLPCqmDgV9FS4=g+wcLg0TV51b72zvWT9Q@mail.gmail.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: clk: qcom: Add support for GPU GX GDSCR
  2020-02-10  4:01 [PATCH v2 1/2] dt-bindings: clk: qcom: Add support for GPU GX GDSCR Taniya Das
  2020-02-10  4:01 ` [PATCH v2 2/2] clk: qcom: gpucc: Add support for GX GDSC for SC7180 Taniya Das
@ 2020-02-10 17:55 ` Doug Anderson
  2020-02-12 23:04 ` Stephen Boyd
  2 siblings, 0 replies; 7+ messages in thread
From: Doug Anderson @ 2020-02-10 17:55 UTC (permalink / raw)
  To: Taniya Das
  Cc: Stephen Boyd, Michael Turquette, Rob Herring, David Brown,
	Rajendra Nayak, linux-arm-msm, open list:ARM/QUALCOMM SUPPORT,
	linux-clk, LKML, Andy Gross,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rob Herring

Hi,

On Sun, Feb 9, 2020 at 8:01 PM Taniya Das <tdas@codeaurora.org> wrote:
>
> In the cases where the GPU SW requires to use the GX GDSCR add
> support for the same.
>
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> ---
>  include/dt-bindings/clock/qcom,gpucc-sc7180.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

I already added my tag to this exact same patch in:

https://lore.kernel.org/linux-arm-msm/CAD=FV=VeMaKq3KR=t7dbG+VyVs5DS=gHasSdJQSqNQreTUoZig@mail.gmail.com/

Please make sure to carry forward tags unless something major has
changed.  In any case, re-adding my tag:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] clk: qcom: gpucc: Add support for GX GDSC for SC7180
  2020-02-10 17:49   ` Doug Anderson
@ 2020-02-11  2:50     ` Taniya Das
  0 siblings, 0 replies; 7+ messages in thread
From: Taniya Das @ 2020-02-11  2:50 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Stephen Boyd, Michael Turquette, Rob Herring, David Brown,
	Rajendra Nayak, linux-arm-msm, open list:ARM/QUALCOMM SUPPORT,
	linux-clk, LKML, Andy Gross,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rob Herring

Hi Doug,

On 2/10/2020 11:19 PM, Doug Anderson wrote:
> Hi,
> 
> On Sun, Feb 9, 2020 at 8:01 PM Taniya Das <tdas@codeaurora.org> wrote:
>>
>> Most of the time the CPU should not be touching the GX domain on the
>> GPU except for a very special use case when the CPU needs to force the
>> GX headswitch off. Add the GX domain for that use case.  As part of
>> this add a dummy enable function for the GX gdsc to simulate success
>> so that the pm_runtime reference counting is correct.  This matches
>> what was done in sdm845 in commit 85a3d920d30a ("clk: qcom: Add a
>> dummy enable function for GX gdsc").
>>
>> Signed-off-by: Taniya Das <tdas@codeaurora.org>
>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 
> For future reference, if you have someone's tag in your commit message
> it's nice to CC them on the email.
> 
> 

My bad my miss.

>> ---
>>   drivers/clk/qcom/gpucc-sc7180.c | 37 +++++++++++++++++++++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/gpucc-sc7180.c b/drivers/clk/qcom/gpucc-sc7180.c
>> index a96c0b9..7b656b6 100644
>> --- a/drivers/clk/qcom/gpucc-sc7180.c
>> +++ b/drivers/clk/qcom/gpucc-sc7180.c
>> @@ -170,8 +170,45 @@ static struct gdsc cx_gdsc = {
>>          .flags = VOTABLE,
>>   };
>>
>> +/*
>> + * On SC7180 the GPU GX domain is *almost* entirely controlled by the GMU
>> + * running in the CX domain so the CPU doesn't need to know anything about the
>> + * GX domain EXCEPT....
>> + *
>> + * Hardware constraints dictate that the GX be powered down before the CX. If
>> + * the GMU crashes it could leave the GX on. In order to successfully bring back
>> + * the device the CPU needs to disable the GX headswitch. There being no sane
>> + * way to reach in and touch that register from deep inside the GPU driver we
>> + * need to set up the infrastructure to be able to ensure that the GPU can
>> + * ensure that the GX is off during this super special case. We do this by
>> + * defining a GX gdsc with a dummy enable function and a "default" disable
>> + * function.
>> + *
>> + * This allows us to attach with genpd_dev_pm_attach_by_name() in the GPU
>> + * driver. During power up, nothing will happen from the CPU (and the GMU will
>> + * power up normally but during power down this will ensure that the GX domain
>> + * is *really* off - this gives us a semi standard way of doing what we need.
>> + */
>> +static int gx_gdsc_enable(struct generic_pm_domain *domain)
>> +{
>> +       /* Do nothing but give genpd the impression that we were successful */
>> +       return 0;
>> +}
>> +
>> +static struct gdsc gx_gdsc = {
>> +       .gdscr = 0x100c,
>> +       .clamp_io_ctrl = 0x1508,
>> +       .pd = {
>> +               .name = "gx_gdsc",
>> +               .power_on = gx_gdsc_enable,
>> +       },
>> +       .pwrsts = PWRSTS_OFF_ON,
>> +       .flags = CLAMP_IO,
> 
> In my previous reply [1], I asked about these flags and if it was
> intentional that they were different from sdm845.  I did see a private
> response, but no public one.  In the future note that it's good to
> reply publicly so everyone understands what happened.  In this case, I
> was told "the GDSC's on 845 and SC7180 are different and hence the
> change in flags is expected".  That answers my question and thus I'm
> fine with my tag being here.  It also looks like you took my other
> review feedback on v1, which is nice.
> 
> 
> -Doug
> 

I am unable to respond to the other thread, thus we put out the reply.

> 
> [1] https://lore.kernel.org/r/CAD=FV=V6yM7UJwu0ZLPCqmDgV9FS4=g+wcLg0TV51b72zvWT9Q@mail.gmail.com
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: clk: qcom: Add support for GPU GX GDSCR
  2020-02-10  4:01 [PATCH v2 1/2] dt-bindings: clk: qcom: Add support for GPU GX GDSCR Taniya Das
  2020-02-10  4:01 ` [PATCH v2 2/2] clk: qcom: gpucc: Add support for GX GDSC for SC7180 Taniya Das
  2020-02-10 17:55 ` [PATCH v2 1/2] dt-bindings: clk: qcom: Add support for GPU GX GDSCR Doug Anderson
@ 2020-02-12 23:04 ` Stephen Boyd
  2 siblings, 0 replies; 7+ messages in thread
From: Stephen Boyd @ 2020-02-12 23:04 UTC (permalink / raw)
  To: Michael Turquette, Taniya Das, robh
  Cc: David Brown, Rajendra Nayak, linux-arm-msm, linux-soc, linux-clk,
	linux-kernel, Andy Gross, devicetree, robh+dt, Taniya Das

Quoting Taniya Das (2020-02-09 20:01:05)
> In the cases where the GPU SW requires to use the GX GDSCR add
> support for the same.
> 
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> ---

Applied to clk-next

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] clk: qcom: gpucc: Add support for GX GDSC for SC7180
  2020-02-10  4:01 ` [PATCH v2 2/2] clk: qcom: gpucc: Add support for GX GDSC for SC7180 Taniya Das
  2020-02-10 17:49   ` Doug Anderson
@ 2020-02-12 23:04   ` Stephen Boyd
  1 sibling, 0 replies; 7+ messages in thread
From: Stephen Boyd @ 2020-02-12 23:04 UTC (permalink / raw)
  To: Michael Turquette, Taniya Das, robh
  Cc: David Brown, Rajendra Nayak, linux-arm-msm, linux-soc, linux-clk,
	linux-kernel, Andy Gross, devicetree, robh+dt, Taniya Das

Quoting Taniya Das (2020-02-09 20:01:06)
> Most of the time the CPU should not be touching the GX domain on the
> GPU except for a very special use case when the CPU needs to force the
> GX headswitch off. Add the GX domain for that use case.  As part of
> this add a dummy enable function for the GX gdsc to simulate success
> so that the pm_runtime reference counting is correct.  This matches
> what was done in sdm845 in commit 85a3d920d30a ("clk: qcom: Add a
> dummy enable function for GX gdsc").
> 
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---

Applied to clk-next

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10  4:01 [PATCH v2 1/2] dt-bindings: clk: qcom: Add support for GPU GX GDSCR Taniya Das
2020-02-10  4:01 ` [PATCH v2 2/2] clk: qcom: gpucc: Add support for GX GDSC for SC7180 Taniya Das
2020-02-10 17:49   ` Doug Anderson
2020-02-11  2:50     ` Taniya Das
2020-02-12 23:04   ` Stephen Boyd
2020-02-10 17:55 ` [PATCH v2 1/2] dt-bindings: clk: qcom: Add support for GPU GX GDSCR Doug Anderson
2020-02-12 23:04 ` Stephen Boyd

Linux-Clk Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-clk/0 linux-clk/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-clk linux-clk/ https://lore.kernel.org/linux-clk \
		linux-clk@vger.kernel.org
	public-inbox-index linux-clk

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-clk


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git