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

[Bjorn Andersson <bjorn.andersson@linaro.org>]

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
>