[PATCH 1/5] drm/msm/adreno: Use OPP for every GPU generation

Dmitry Baryshkov dmitry.baryshkov at linaro.org
Wed Feb 22 22:38:26 UTC 2023


On 22/02/2023 23:47, Konrad Dybcio wrote:
> Some older GPUs (namely a2xx with no opp tables at all and a320 with
> downstream-remnants gpu pwrlevels) used not to have OPP tables. They
> both however had just one frequency defined, making it extremely easy
> to construct such an OPP table from within the driver if need be.
> 
> Do so and switch all clk_set_rate calls on core_clk to their OPP
> counterparts.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio at linaro.org>
> ---
>   drivers/gpu/drm/msm/adreno/adreno_gpu.c | 94 +++++++++++++++------------------
>   drivers/gpu/drm/msm/msm_gpu.c           |  4 +-
>   drivers/gpu/drm/msm/msm_gpu_devfreq.c   |  2 +-
>   3 files changed, 45 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index ce6b76c45b6f..9b940c0f063f 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -922,73 +922,50 @@ void adreno_wait_ring(struct msm_ringbuffer *ring, uint32_t ndwords)
>   			ring->id);
>   }
>   
> -/* Get legacy powerlevels from qcom,gpu-pwrlevels and populate the opp table */
> -static int adreno_get_legacy_pwrlevels(struct device *dev)
> -{
> -	struct device_node *child, *node;
> -	int ret;
> -
> -	node = of_get_compatible_child(dev->of_node, "qcom,gpu-pwrlevels");
> -	if (!node) {
> -		DRM_DEV_DEBUG(dev, "Could not find the GPU powerlevels\n");
> -		return -ENXIO;
> -	}
> -
> -	for_each_child_of_node(node, child) {
> -		unsigned int val;
> -
> -		ret = of_property_read_u32(child, "qcom,gpu-freq", &val);
> -		if (ret)
> -			continue;
> -
> -		/*
> -		 * Skip the intentionally bogus clock value found at the bottom
> -		 * of most legacy frequency tables
> -		 */
> -		if (val != 27000000)
> -			dev_pm_opp_add(dev, val, 0);
> -	}
> -
> -	of_node_put(node);
> -
> -	return 0;
> -}
> -
> -static void adreno_get_pwrlevels(struct device *dev,
> +static int adreno_get_pwrlevels(struct device *dev,
>   		struct msm_gpu *gpu)
>   {
> +	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
>   	unsigned long freq = ULONG_MAX;
>   	struct dev_pm_opp *opp;
>   	int ret;
>   
>   	gpu->fast_rate = 0;
>   
> -	/* You down with OPP? */
> -	if (!of_find_property(dev->of_node, "operating-points-v2", NULL))
> -		ret = adreno_get_legacy_pwrlevels(dev);
> -	else {
> -		ret = devm_pm_opp_of_add_table(dev);
> -		if (ret)
> -			DRM_DEV_ERROR(dev, "Unable to set the OPP table\n");
> -	}
> -
> -	if (!ret) {
> +	/* devm_pm_opp_of_add_table may error out but will still create an OPP table */
> +	ret = devm_pm_opp_of_add_table(dev);
> +	if (ret == -ENODEV) {
> +		/* Special cases for ancient hw with ancient DT bindings */
> +		if (adreno_is_a2xx(adreno_gpu)) {
> +			dev_warn(dev, "Unable to find the OPP table. Falling back to 200 MHz.\n");
> +			dev_pm_opp_add(dev, 200000000, 0);
> +			gpu->fast_rate = 200000000;

We can skip setting the fast_rate, dev_pm_opp_find_freq_floor below will 
get it from our freshly generated opp table.

> +		} else if (adreno_is_a320(adreno_gpu)) {
> +			dev_warn(dev, "Unable to find the OPP table. Falling back to 450 MHz.\n");
> +			dev_pm_opp_add(dev, 450000000, 0);
> +			gpu->fast_rate = 450000000;
> +		} else {
> +			DRM_DEV_ERROR(dev, "Unable to find the OPP table\n");
> +			return -ENODEV;
> +		}
> +	} else if (ret) {
> +		DRM_DEV_ERROR(dev, "Unable to set the OPP table\n");
> +		return ret;
> +	} else {
>   		/* Find the fastest defined rate */
>   		opp = dev_pm_opp_find_freq_floor(dev, &freq);
> -		if (!IS_ERR(opp)) {
> +
> +		if (IS_ERR(opp))
> +			return PTR_ERR(opp);
> +		else {
>   			gpu->fast_rate = freq;
>   			dev_pm_opp_put(opp);
>   		}
>   	}
>   
> -	if (!gpu->fast_rate) {
> -		dev_warn(dev,
> -			"Could not find a clock rate. Using a reasonable default\n");
> -		/* Pick a suitably safe clock speed for any target */
> -		gpu->fast_rate = 200000000;
> -	}
> -
>   	DBG("fast_rate=%u, slow_rate=27000000", gpu->fast_rate);
> +
> +	return 0;
>   }
>   
>   int adreno_gpu_ocmem_init(struct device *dev, struct adreno_gpu *adreno_gpu,
> @@ -1046,6 +1023,17 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>   	struct adreno_rev *rev = &config->rev;
>   	const char *gpu_name;
>   	u32 speedbin;
> +	int ret;
> +
> +	/* This can only be done here, or devm_pm_opp_set_supported_hw will WARN_ON() */

I'd rephrase this to '...done before devm_pm_opp_of_add_table(), or 
dev_pm_opp_set_config() will...'. It took me a while to find correct 
limitation.

I wanted to move the code below to msm_gpu_init(), but after digging in 
found that it's not possible.


> +	if (IS_ERR(devm_clk_get(dev, "core"))) {
> +		/*
> +		 * If "core" is absent, go for the legacy clock name.
> +		 * If we got this far in probing, it's a given one of them exists.
> +		 */
> +		devm_pm_opp_set_clkname(dev, "core_clk");
> +	} else
> +		devm_pm_opp_set_clkname(dev, "core");
>   
>   	adreno_gpu->funcs = funcs;
>   	adreno_gpu->info = adreno_info(config->rev);
> @@ -1070,7 +1058,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>   
>   	adreno_gpu_config.nr_rings = nr_rings;
>   
> -	adreno_get_pwrlevels(dev, gpu);
> +	ret = adreno_get_pwrlevels(dev, gpu);
> +	if (ret)
> +		return ret;
>   
>   	pm_runtime_set_autosuspend_delay(dev,
>   		adreno_gpu->info->inactive_period);
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 380249500325..cdcb00df3f25 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -59,7 +59,7 @@ static int disable_pwrrail(struct msm_gpu *gpu)
>   static int enable_clk(struct msm_gpu *gpu)
>   {
>   	if (gpu->core_clk && gpu->fast_rate)
> -		clk_set_rate(gpu->core_clk, gpu->fast_rate);
> +		dev_pm_opp_set_rate(&gpu->pdev->dev, gpu->fast_rate);
>   
>   	/* Set the RBBM timer rate to 19.2Mhz */
>   	if (gpu->rbbmtimer_clk)
> @@ -78,7 +78,7 @@ static int disable_clk(struct msm_gpu *gpu)
>   	 * will be rounded down to zero anyway so it all works out.
>   	 */
>   	if (gpu->core_clk)
> -		clk_set_rate(gpu->core_clk, 27000000);
> +		dev_pm_opp_set_rate(&gpu->pdev->dev, 27000000);
>   
>   	if (gpu->rbbmtimer_clk)
>   		clk_set_rate(gpu->rbbmtimer_clk, 0);
> diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> index e27dbf12b5e8..ea70c1c32d94 100644
> --- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> +++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> @@ -48,7 +48,7 @@ static int msm_devfreq_target(struct device *dev, unsigned long *freq,
>   		gpu->funcs->gpu_set_freq(gpu, opp, df->suspended);
>   		mutex_unlock(&df->lock);
>   	} else {
> -		clk_set_rate(gpu->core_clk, *freq);
> +		dev_pm_opp_set_rate(dev, *freq);
>   	}
>   
>   	dev_pm_opp_put(opp);
> 

-- 
With best wishes
Dmitry



More information about the dri-devel mailing list