[PATCH 0/2] clk: qcom:

[PATCH 0/2] clk: qcom:

[Jordan Crouse]

The GPU GX domain on SDM845 is nominally managed by the GMU microcontroller
but there are certain circumstances when the CPU needs to be sure that the
GX headswitch is off.

This series modifies the GPU GX domain on SDM845 to use a dummy enable
function (leaving the GMU hardware to handle the power on) and the
default disable function to allow the CPU to ensure that the power is off.

With these changes we can attach the domain to the GMU device and control
it from the start up and shut down functions as implemented by [2].

This patchset is based on the core GPU clock support from [1].

[1] https://patchwork.kernel.org/patch/10696671/
[2] https://patchwork.freedesktop.org/patch/262765/

v2: Rebase on the latest from Taniya and remove the GPU functions to make
it easier to apply to the clock tree.

Jordan Crouse (2):
  clk: qcom: gdsc: Don't override existing gdsc pd functions
  clk: qcom: Add a dummy enable function for GX gdsc

 drivers/clk/qcom/gdsc.c         |  6 ++++--
 drivers/clk/qcom/gpucc-sdm845.c | 26 ++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

-- 
2.18.0

[PATCH 1/2] clk: qcom: gdsc: Don't override existing gdsc pd functions

[Jordan Crouse]

In extreme cases an individual gdsc may wish to override the
power domain enable or disable callback functions for their own
purposes. Only set the generic gdsc callback if the function pointers
are not already set.

Acked-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/clk/qcom/gdsc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index a077133c7ce3..dd63aa36b092 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -350,8 +350,10 @@ static int gdsc_init(struct gdsc *sc)
 	else
 		gdsc_clear_mem_on(sc);
 
-	sc->pd.power_off = gdsc_disable;
-	sc->pd.power_on = gdsc_enable;
+	if (!sc->pd.power_off)
+		sc->pd.power_off = gdsc_disable;
+	if (!sc->pd.power_on)
+		sc->pd.power_on = gdsc_enable;
 	pm_genpd_init(&sc->pd, NULL, !on);
 
 	return 0;
-- 
2.18.0

[PATCH 2/2] clk: qcom: Add a dummy enable function for GX gdsc

[Jordan Crouse]

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 a
dummy enable function for the GX gdsc to simulate success
so that the pm_runtime reference counting is correct.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/clk/qcom/gpucc-sdm845.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/clk/qcom/gpucc-sdm845.c b/drivers/clk/qcom/gpucc-sdm845.c
index 11222f487a65..7266e9a1d77e 100644
--- a/drivers/clk/qcom/gpucc-sdm845.c
+++ b/drivers/clk/qcom/gpucc-sdm845.c
@@ -131,11 +131,37 @@ static struct gdsc gpu_cx_gdsc = {
 	.flags = VOTABLE,
 };
 
+/*
+ * On SDM845 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 gpu_gx_gdsc = {
 	.gdscr = 0x100c,
 	.clamp_io_ctrl = 0x1508,
 	.pd = {
 		.name = "gpu_gx_gdsc",
+		.power_on = gx_gdsc_enable,
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.flags = CLAMP_IO | AON_RESET | POLL_CFG_GDSCR,
-- 
2.18.0

Re: [PATCH 1/2] clk: qcom: gdsc: Don't override existing gdsc pd functions

[Bjorn Andersson]

On Mon 26 Nov 08:54 PST 2018, Jordan Crouse wrote:

> In extreme cases an individual gdsc may wish to override the
> power domain enable or disable callback functions for their own
> purposes. Only set the generic gdsc callback if the function pointers
> are not already set.
> 
> Acked-by: Rajendra Nayak <rnayak@codeaurora.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>  drivers/clk/qcom/gdsc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index a077133c7ce3..dd63aa36b092 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -350,8 +350,10 @@ static int gdsc_init(struct gdsc *sc)
>  	else
>  		gdsc_clear_mem_on(sc);
>  
> -	sc->pd.power_off = gdsc_disable;
> -	sc->pd.power_on = gdsc_enable;
> +	if (!sc->pd.power_off)
> +		sc->pd.power_off = gdsc_disable;
> +	if (!sc->pd.power_on)
> +		sc->pd.power_on = gdsc_enable;
>  	pm_genpd_init(&sc->pd, NULL, !on);
>  
>  	return 0;
> -- 
> 2.18.0
> 

Re: [PATCH 2/2] clk: qcom: Add a dummy enable function for GX gdsc

[Bjorn Andersson]

On Mon 26 Nov 08:54 PST 2018, Jordan Crouse 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 a
> dummy enable function for the GX gdsc to simulate success
> so that the pm_runtime reference counting is correct.
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/clk/qcom/gpucc-sdm845.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gpucc-sdm845.c b/drivers/clk/qcom/gpucc-sdm845.c
> index 11222f487a65..7266e9a1d77e 100644
> --- a/drivers/clk/qcom/gpucc-sdm845.c
> +++ b/drivers/clk/qcom/gpucc-sdm845.c
> @@ -131,11 +131,37 @@ static struct gdsc gpu_cx_gdsc = {
>  	.flags = VOTABLE,
>  };
>  
> +/*
> + * On SDM845 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 gpu_gx_gdsc = {
>  	.gdscr = 0x100c,
>  	.clamp_io_ctrl = 0x1508,
>  	.pd = {
>  		.name = "gpu_gx_gdsc",
> +		.power_on = gx_gdsc_enable,
>  	},
>  	.pwrsts = PWRSTS_OFF_ON,
>  	.flags = CLAMP_IO | AON_RESET | POLL_CFG_GDSCR,
> -- 
> 2.18.0
> 

Re: [PATCH 1/2] clk: qcom: gdsc: Don't override existing gdsc pd functions

[Stephen Boyd]

Quoting Jordan Crouse (2018-11-26 09:20:31)
> In extreme cases an individual gdsc may wish to override the
> power domain enable or disable callback functions for their own
> purposes. Only set the generic gdsc callback if the function pointers
> are not already set.
> 
> Acked-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---

Applied to clk-next

[PATCH 1/2] clk: qcom: gdsc: Don't override existing gdsc pd functions

[Jordan Crouse]

In extreme cases an individual gdsc may wish to override the
power domain enable or disable callback functions for their own
purposes. Only set the generic gdsc callback if the function pointers
are not already set.

Acked-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/clk/qcom/gdsc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index a077133c7ce3..dd63aa36b092 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -350,8 +350,10 @@ static int gdsc_init(struct gdsc *sc)
 	else
 		gdsc_clear_mem_on(sc);
 
-	sc->pd.power_off = gdsc_disable;
-	sc->pd.power_on = gdsc_enable;
+	if (!sc->pd.power_off)
+		sc->pd.power_off = gdsc_disable;
+	if (!sc->pd.power_on)
+		sc->pd.power_on = gdsc_enable;
 	pm_genpd_init(&sc->pd, NULL, !on);
 
 	return 0;
-- 
2.18.0