| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 27 Jul 2021 23:11:27 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Rob Clark <robdclark@...il.com>, dri-devel@...ts.freedesktop.org
Cc: Rob Clark <robdclark@...omium.org>, Sean Paul <sean@...rly.run>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
Jordan Crouse <jordan@...micpenguin.net>,
Jonathan Marek <jonathan@...ek.ca>,
Akhil P Oommen <akhilpo@...eaurora.org>,
Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>,
Sharat Masetty <smasetty@...eaurora.org>,
Douglas Anderson <dianders@...omium.org>,
open list <linux-kernel@...r.kernel.org>,
"open list:DRM DRIVER FOR MSM ADRENO GPU"
<linux-arm-msm@...r.kernel.org>,
"open list:DRM DRIVER FOR MSM ADRENO GPU"
<freedreno@...ts.freedesktop.org>
Subject: Re: [PATCH v2 1/3] drm/msm: Split out devfreq handling
On 26/07/2021 17:46, Rob Clark wrote:
> From: Rob Clark <robdclark@...omium.org>
>
> Before we start adding more cleverness, split it into it's own file.
>
> Signed-off-by: Rob Clark <robdclark@...omium.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 +-
> drivers/gpu/drm/msm/msm_gpu.c | 116 +---------------------
> drivers/gpu/drm/msm/msm_gpu.h | 32 +++++--
> drivers/gpu/drm/msm/msm_gpu_devfreq.c | 133 ++++++++++++++++++++++++++
> 5 files changed, 165 insertions(+), 121 deletions(-)
> create mode 100644 drivers/gpu/drm/msm/msm_gpu_devfreq.c
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 2c00aa70b708..904535eda0c4 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -90,6 +90,7 @@ msm-y := \
> msm_gem_submit.o \
> msm_gem_vma.o \
> msm_gpu.o \
> + msm_gpu_devfreq.o \
> msm_iommu.o \
> msm_perf.o \
> msm_rd.o \
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 9db9f7847ea8..91f637b908f4 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1477,7 +1477,7 @@ static int a6xx_pm_resume(struct msm_gpu *gpu)
> if (ret)
> return ret;
>
> - msm_gpu_resume_devfreq(gpu);
> + msm_devfreq_resume(gpu);
>
> a6xx_llc_activate(a6xx_gpu);
>
> @@ -1494,7 +1494,7 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu)
>
> a6xx_llc_deactivate(a6xx_gpu);
>
> - devfreq_suspend_device(gpu->devfreq.devfreq);
> + msm_devfreq_suspend(gpu);
>
> ret = a6xx_gmu_stop(a6xx_gpu);
> if (ret)
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 647af45cf892..fedbd785e42f 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -13,8 +13,6 @@
>
> #include <generated/utsrelease.h>
> #include <linux/string_helpers.h>
> -#include <linux/devfreq.h>
> -#include <linux/devfreq_cooling.h>
> #include <linux/devcoredump.h>
> #include <linux/sched/task.h>
>
> @@ -22,106 +20,6 @@
> * Power Management:
> */
>
> -static int msm_devfreq_target(struct device *dev, unsigned long *freq,
> - u32 flags)
> -{
> - struct msm_gpu *gpu = dev_to_gpu(dev);
> - struct dev_pm_opp *opp;
> -
> - opp = devfreq_recommended_opp(dev, freq, flags);
> -
> - if (IS_ERR(opp))
> - return PTR_ERR(opp);
> -
> - trace_msm_gpu_freq_change(dev_pm_opp_get_freq(opp));
> -
> - if (gpu->funcs->gpu_set_freq)
> - gpu->funcs->gpu_set_freq(gpu, opp);
> - else
> - clk_set_rate(gpu->core_clk, *freq);
> -
> - dev_pm_opp_put(opp);
> -
> - return 0;
> -}
> -
> -static int msm_devfreq_get_dev_status(struct device *dev,
> - struct devfreq_dev_status *status)
> -{
> - struct msm_gpu *gpu = dev_to_gpu(dev);
> - ktime_t time;
> -
> - if (gpu->funcs->gpu_get_freq)
> - status->current_frequency = gpu->funcs->gpu_get_freq(gpu);
> - else
> - status->current_frequency = clk_get_rate(gpu->core_clk);
> -
> - status->busy_time = gpu->funcs->gpu_busy(gpu);
> -
> - time = ktime_get();
> - status->total_time = ktime_us_delta(time, gpu->devfreq.time);
> - gpu->devfreq.time = time;
> -
> - return 0;
> -}
> -
> -static int msm_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
> -{
> - struct msm_gpu *gpu = dev_to_gpu(dev);
> -
> - if (gpu->funcs->gpu_get_freq)
> - *freq = gpu->funcs->gpu_get_freq(gpu);
> - else
> - *freq = clk_get_rate(gpu->core_clk);
> -
> - return 0;
> -}
> -
> -static struct devfreq_dev_profile msm_devfreq_profile = {
> - .polling_ms = 10,
> - .target = msm_devfreq_target,
> - .get_dev_status = msm_devfreq_get_dev_status,
> - .get_cur_freq = msm_devfreq_get_cur_freq,
> -};
> -
> -static void msm_devfreq_init(struct msm_gpu *gpu)
> -{
> - /* We need target support to do devfreq */
> - if (!gpu->funcs->gpu_busy)
> - return;
> -
> - msm_devfreq_profile.initial_freq = gpu->fast_rate;
> -
> - /*
> - * Don't set the freq_table or max_state and let devfreq build the table
> - * from OPP
> - * After a deferred probe, these may have be left to non-zero values,
> - * so set them back to zero before creating the devfreq device
> - */
> - msm_devfreq_profile.freq_table = NULL;
> - msm_devfreq_profile.max_state = 0;
> -
> - gpu->devfreq.devfreq = devm_devfreq_add_device(&gpu->pdev->dev,
> - &msm_devfreq_profile, DEVFREQ_GOV_SIMPLE_ONDEMAND,
> - NULL);
> -
> - if (IS_ERR(gpu->devfreq.devfreq)) {
> - DRM_DEV_ERROR(&gpu->pdev->dev, "Couldn't initialize GPU devfreq\n");
> - gpu->devfreq.devfreq = NULL;
> - return;
> - }
> -
> - devfreq_suspend_device(gpu->devfreq.devfreq);
> -
> - gpu->cooling = of_devfreq_cooling_register(gpu->pdev->dev.of_node,
> - gpu->devfreq.devfreq);
> - if (IS_ERR(gpu->cooling)) {
> - DRM_DEV_ERROR(&gpu->pdev->dev,
> - "Couldn't register GPU cooling device\n");
> - gpu->cooling = NULL;
> - }
> -}
> -
> static int enable_pwrrail(struct msm_gpu *gpu)
> {
> struct drm_device *dev = gpu->dev;
> @@ -196,14 +94,6 @@ static int disable_axi(struct msm_gpu *gpu)
> return 0;
> }
>
> -void msm_gpu_resume_devfreq(struct msm_gpu *gpu)
> -{
> - gpu->devfreq.busy_cycles = 0;
> - gpu->devfreq.time = ktime_get();
> -
> - devfreq_resume_device(gpu->devfreq.devfreq);
> -}
> -
> int msm_gpu_pm_resume(struct msm_gpu *gpu)
> {
> int ret;
> @@ -223,7 +113,7 @@ int msm_gpu_pm_resume(struct msm_gpu *gpu)
> if (ret)
> return ret;
>
> - msm_gpu_resume_devfreq(gpu);
> + msm_devfreq_resume(gpu);
>
> gpu->needs_hw_init = true;
>
> @@ -237,7 +127,7 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
> DBG("%s", gpu->name);
> trace_msm_gpu_suspend(0);
>
> - devfreq_suspend_device(gpu->devfreq.devfreq);
> + msm_devfreq_suspend(gpu);
>
> ret = disable_axi(gpu);
> if (ret)
> @@ -1116,5 +1006,5 @@ void msm_gpu_cleanup(struct msm_gpu *gpu)
> kthread_destroy_worker(gpu->worker);
> }
>
> - devfreq_cooling_unregister(gpu->cooling);
> + msm_devfreq_cleanup(gpu);
> }
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index ef41ec09f59c..2e61d05293e6 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -80,6 +80,26 @@ struct msm_gpu_fault_info {
> const char *block;
> };
>
> +/**
> + * struct msm_gpu_devfreq - devfreq related state
> + */
> +struct msm_gpu_devfreq {
> + /** devfreq: devfreq instance */
> + struct devfreq *devfreq;
> +
> + /**
> + * busy_cycles:
> + *
> + * Used by implementation of gpu->gpu_busy() to track the last
> + * busy counter value, for calculating elapsed busy cycles since
> + * last sampling period.
> + */
> + u64 busy_cycles;
> +
> + /** time: Time of last sampling period. */
> + ktime_t time;
> +};
> +
> struct msm_gpu {
> const char *name;
> struct drm_device *dev;
> @@ -151,11 +171,7 @@ struct msm_gpu {
>
> struct drm_gem_object *memptrs_bo;
>
> - struct {
> - struct devfreq *devfreq;
> - u64 busy_cycles;
> - ktime_t time;
> - } devfreq;
> + struct msm_gpu_devfreq devfreq;
>
> uint32_t suspend_count;
>
> @@ -301,7 +317,11 @@ static inline void gpu_write64(struct msm_gpu *gpu, u32 lo, u32 hi, u64 val)
>
> int msm_gpu_pm_suspend(struct msm_gpu *gpu);
> int msm_gpu_pm_resume(struct msm_gpu *gpu);
> -void msm_gpu_resume_devfreq(struct msm_gpu *gpu);
> +
> +void msm_devfreq_init(struct msm_gpu *gpu);
> +void msm_devfreq_cleanup(struct msm_gpu *gpu);
> +void msm_devfreq_resume(struct msm_gpu *gpu);
> +void msm_devfreq_suspend(struct msm_gpu *gpu);
>
> int msm_gpu_hw_init(struct msm_gpu *gpu);
>
> diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> new file mode 100644
> index 000000000000..3bcea0baddab
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2013 Red Hat
> + * Author: Rob Clark <robdclark@...il.com>
> + */
> +
> +#include "msm_gpu.h"
> +#include "msm_gpu_trace.h"
> +
> +#include <linux/devfreq.h>
> +#include <linux/devfreq_cooling.h>
> +
> +/*
> + * Power Management:
> + */
> +
> +static int msm_devfreq_target(struct device *dev, unsigned long *freq,
> + u32 flags)
> +{
> + struct msm_gpu *gpu = dev_to_gpu(dev);
> + struct dev_pm_opp *opp;
> +
> + opp = devfreq_recommended_opp(dev, freq, flags);
> +
> + if (IS_ERR(opp))
> + return PTR_ERR(opp);
> +
> + trace_msm_gpu_freq_change(dev_pm_opp_get_freq(opp));
> +
> + if (gpu->funcs->gpu_set_freq)
> + gpu->funcs->gpu_set_freq(gpu, opp);
> + else
> + clk_set_rate(gpu->core_clk, *freq);
> +
> + dev_pm_opp_put(opp);
> +
> + return 0;
> +}
> +
> +static int msm_devfreq_get_dev_status(struct device *dev,
> + struct devfreq_dev_status *status)
> +{
> + struct msm_gpu *gpu = dev_to_gpu(dev);
> + ktime_t time;
> +
> + if (gpu->funcs->gpu_get_freq)
> + status->current_frequency = gpu->funcs->gpu_get_freq(gpu);
> + else
> + status->current_frequency = clk_get_rate(gpu->core_clk);
> +
> + status->busy_time = gpu->funcs->gpu_busy(gpu);
> +
> + time = ktime_get();
> + status->total_time = ktime_us_delta(time, gpu->devfreq.time);
> + gpu->devfreq.time = time;
> +
> + return 0;
> +}
> +
> +static int msm_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
> +{
> + struct msm_gpu *gpu = dev_to_gpu(dev);
> +
> + if (gpu->funcs->gpu_get_freq)
> + *freq = gpu->funcs->gpu_get_freq(gpu);
> + else
> + *freq = clk_get_rate(gpu->core_clk);
> +
> + return 0;
> +}
> +
> +static struct devfreq_dev_profile msm_devfreq_profile = {
> + .polling_ms = 10,
> + .target = msm_devfreq_target,
> + .get_dev_status = msm_devfreq_get_dev_status,
> + .get_cur_freq = msm_devfreq_get_cur_freq,
> +};
> +
> +void msm_devfreq_init(struct msm_gpu *gpu)
> +{
> + /* We need target support to do devfreq */
> + if (!gpu->funcs->gpu_busy)
> + return;
> +
> + msm_devfreq_profile.initial_freq = gpu->fast_rate;
> +
> + /*
> + * Don't set the freq_table or max_state and let devfreq build the table
> + * from OPP
> + * After a deferred probe, these may have be left to non-zero values,
> + * so set them back to zero before creating the devfreq device
> + */
> + msm_devfreq_profile.freq_table = NULL;
> + msm_devfreq_profile.max_state = 0;
> +
> + gpu->devfreq.devfreq = devm_devfreq_add_device(&gpu->pdev->dev,
> + &msm_devfreq_profile, DEVFREQ_GOV_SIMPLE_ONDEMAND,
> + NULL);
> +
> + if (IS_ERR(gpu->devfreq.devfreq)) {
> + DRM_DEV_ERROR(&gpu->pdev->dev, "Couldn't initialize GPU devfreq\n");
> + gpu->devfreq.devfreq = NULL;
> + return;
> + }
> +
> + devfreq_suspend_device(gpu->devfreq.devfreq);
> +
> + gpu->cooling = of_devfreq_cooling_register(gpu->pdev->dev.of_node,
> + gpu->devfreq.devfreq);
> + if (IS_ERR(gpu->cooling)) {
> + DRM_DEV_ERROR(&gpu->pdev->dev,
> + "Couldn't register GPU cooling device\n");
> + gpu->cooling = NULL;
> + }
> +}
> +
> +void msm_devfreq_cleanup(struct msm_gpu *gpu)
> +{
> + devfreq_cooling_unregister(gpu->cooling);
> +}
> +
> +void msm_devfreq_resume(struct msm_gpu *gpu)
> +{
> + gpu->devfreq.busy_cycles = 0;
> + gpu->devfreq.time = ktime_get();
> +
> + devfreq_resume_device(gpu->devfreq.devfreq);
> +}
> +
> +void msm_devfreq_suspend(struct msm_gpu *gpu)
> +{
> + devfreq_suspend_device(gpu->devfreq.devfreq);
> +}
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists