drm/msm: remove explicit devfreq status reset
Subsystems (4)
PATCH 1/3 drm/msm: remove explicit devfreq status reset
It is redundant since commit 7c0ffcd40b16 ("drm/msm/gpu: Respect PM QoS constraints") because dev_pm_qos_update_request triggers get_dev_status.
Signed-off-by Chia-I Wu <[email protected]>
Cc Rob Clark <[email protected]>
--- drivers/gpu/drm/msm/msm_gpu_devfreq.c | 7 ------- 1 file changed, 7 deletions(-)
drivers/gpu/drm/msm/msm_gpu_devfreq.c +0 -7
@@ -224,7 +224,6 @@ void msm_devfreq_boost(struct msm_gpu *gpu, unsigned factor)void msm_devfreq_active (struct msm_gpu *gpu) {struct msm_gpu_devfreq *df = &gpu->devfreq; -struct devfreq_dev_status status;unsigned int idle_time;if (!has_devfreq (gpu))
@@ -248,12 +247,6 @@ void msm_devfreq_active(struct msm_gpu *gpu)dev_pm_qos_update_request (&df->idle_freq,PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE ); - -/* -* Reset the polling interval so we aren't inconsistent -* about freq vs busy/total cycles -*/ -msm_devfreq_get_dev_status (&gpu->pdev->dev, &status); }
Signature · 1 line
2.36.0.rc0.470.gd361397f0d-goog
PATCH 2/3 drm/msm: simplify gpu_busy callback
Move tracking and busy time calculation to msm_devfreq_get_dev_status.
Signed-off-by Chia-I Wu <[email protected]>
Cc Rob Clark <[email protected]>
--- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 19 ++++++---------- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 15 +++++-------- drivers/gpu/drm/msm/msm_gpu.h | 9 +++----- drivers/gpu/drm/msm/msm_gpu_devfreq.c | 32 ++++++++++++++++++++++----- 4 files changed, 41 insertions(+), 34 deletions(-)
drivers/gpu/drm/msm/adreno/a5xx_gpu.c +7 -12
@@ -1662,28 +1662,23 @@ static struct msm_ringbuffer *a5xx_active_ring(struct msm_gpu *gpu)return a5xx_gpu->cur_ring; } -static unsigned long a5xx_gpu_busy (struct msm_gpu *gpu) +static u64 a5xx_gpu_busy (struct msm_gpu *gpu,unsigned long *out_sample_rate) { -u64 busy_cycles, busy_time; +u64 busy_cycles;/* Only read the gpu busy if the hardware is already active */ -if (pm_runtime_get_if_in_use (&gpu->pdev->dev) ==0 ) +if (pm_runtime_get_if_in_use (&gpu->pdev->dev) ==0 ) { + *out_sample_rate =1 ;return 0 ; + } busy_cycles =gpu_read64 (gpu,REG_A5XX_RBBM_PERFCTR_RBBM_0_LO ,REG_A5XX_RBBM_PERFCTR_RBBM_0_HI ); - - busy_time = busy_cycles - gpu->devfreq.busy_cycles; -do_div (busy_time,clk_get_rate (gpu->core_clk) /1000000 ); - - gpu->devfreq.busy_cycles = busy_cycles; + *out_sample_rate =clk_get_rate (gpu->core_clk);pm_runtime_put (&gpu->pdev->dev); -if (WARN_ON (busy_time > ~0LU )) -return ~0LU ; - -return (unsigned long )busy_time; +return busy_cycles; }static uint32_t a5xx_get_rptr (struct msm_gpu *gpu,struct msm_ringbuffer *ring)
drivers/gpu/drm/msm/adreno/a6xx_gpu.c +5 -10
@@ -1649,12 +1649,14 @@ static void a6xx_destroy(struct msm_gpu *gpu)kfree (a6xx_gpu); } -static unsigned long a6xx_gpu_busy (struct msm_gpu *gpu) +static u64 a6xx_gpu_busy (struct msm_gpu *gpu,unsigned long *out_sample_rate) {struct adreno_gpu *adreno_gpu =to_adreno_gpu (gpu);struct a6xx_gpu *a6xx_gpu =to_a6xx_gpu (adreno_gpu); -u64 busy_cycles, busy_time; +u64 busy_cycles; +/* 19.2MHz */ + *out_sample_rate =19200000 ;/* Only read the gpu busy if the hardware is already active */ if (pm_runtime_get_if_in_use (a6xx_gpu->gmu.dev) ==0 )
@@ -1664,17 +1666,10 @@ static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu)REG_A6XX_GMU_CX_GMU_POWER_COUNTER_XOCLK_0_L , REG_A6XX_GMU_CX_GMU_POWER_COUNTER_XOCLK_0_H); - busy_time = (busy_cycles - gpu->devfreq.busy_cycles) *10 ; -do_div (busy_time,192 ); - - gpu->devfreq.busy_cycles = busy_cycles;pm_runtime_put (a6xx_gpu->gmu.dev); -if (WARN_ON (busy_time > ~0LU )) -return ~0LU ; - -return (unsigned long )busy_time; +return busy_cycles; }static void a6xx_gpu_set_freq (struct msm_gpu *gpu,struct dev_pm_opp *opp)
drivers/gpu/drm/msm/msm_gpu.h +3 -6
@@ -62,7 +62,7 @@ struct msm_gpu_funcs {/* for generation specific debugfs: */ void (*debugfs_init)(struct msm_gpu *gpu,struct drm_minor *minor);#endif -unsigned long (*gpu_busy)(struct msm_gpu *gpu); +u64 (*gpu_busy)(struct msm_gpu *gpu,unsigned long *out_sample_rate);struct msm_gpu_state *(*gpu_state_get)(struct msm_gpu *gpu);int (*gpu_state_put)(struct msm_gpu_state *state);unsigned long (*gpu_get_freq)(struct msm_gpu *gpu);
@@ -106,11 +106,8 @@ struct msm_gpu_devfreq {struct dev_pm_qos_request boost_freq;/** -* 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. +* busy_cycles: Last busy counter value, for calculating elapsed busy +* cycles since last sampling period. */ u64 busy_cycles;
drivers/gpu/drm/msm/msm_gpu_devfreq.c +26 -6
@@ -49,18 +49,38 @@ static unsigned long get_freq(struct msm_gpu *gpu)return clk_get_rate (gpu->core_clk); } -static int msm_devfreq_get_dev_status (struct device *dev, +static void get_raw_dev_status (struct msm_gpu *gpu,struct devfreq_dev_status *status) { -struct msm_gpu *gpu =dev_to_gpu (dev); +struct msm_gpu_devfreq *df = &gpu->devfreq; +u64 busy_cycles, busy_time; +unsigned long sample_rate;ktime_t time; status->current_frequency =get_freq (gpu); - status->busy_time = gpu->funcs->gpu_busy (gpu); - + busy_cycles = gpu->funcs->gpu_busy (gpu, &sample_rate); time =ktime_get (); - status->total_time =ktime_us_delta (time, gpu->devfreq.time); - gpu->devfreq.time = time; + + busy_time = busy_cycles - df->busy_cycles; + status->total_time =ktime_us_delta (time, df->time); + + df->busy_cycles = busy_cycles; + df->time = time; + + busy_time *=USEC_PER_SEC ; +do_div (busy_time, sample_rate); +if (WARN_ON (busy_time > ~0LU )) + busy_time = ~0LU ; + + status->busy_time = busy_time; +} + +static int msm_devfreq_get_dev_status (struct device *dev, +struct devfreq_dev_status *status) +{ +struct msm_gpu *gpu =dev_to_gpu (dev); + +get_raw_dev_status (gpu, status);return 0 ; }
Signature · 1 line
2.36.0.rc0.470.gd361397f0d-goog
PATCH 3/3 drm/msm: return the average load over the polling period
In reply to Chia-I Wusimple_ondemand interacts poorly with clamp_to_idle. It only looks at the load since the last get_dev_status call, while it should really look at the load over polling_ms. When clamp_to_idle true, it almost always picks the lowest frequency on active because the gpu is idle between msm_devfreq_idle/msm_devfreq_active.
This logic could potentially be moved into devfreq core.
Fixes 7c0ffcd40b16 ("drm/msm/gpu: Respect PM QoS constraints")
Signed-off-by Chia-I Wu <[email protected]>
Cc Rob Clark <[email protected]>
--- drivers/gpu/drm/msm/msm_gpu.h | 3 ++ drivers/gpu/drm/msm/msm_gpu_devfreq.c | 60 ++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-)
drivers/gpu/drm/msm/msm_gpu.h +3 -0
@@ -9,6 +9,7 @@#include <linux/adreno-smmu-priv.h> #include <linux/clk.h> +#include <linux/devfreq.h> #include <linux/interconnect.h> #include <linux/pm_opp.h> #include <linux/regulator/consumer.h>
@@ -117,6 +118,8 @@ struct msm_gpu_devfreq {/** idle_time: Time of last transition to idle: */ ktime_t idle_time; +struct devfreq_dev_status average_status; +/** * idle_work: *
drivers/gpu/drm/msm/msm_gpu_devfreq.c +59 -1
77 lines not on this page · raw message
Signature · 1 line
2.36.0.rc0.470.gd361397f0d-goog
Re: [PATCH 3/3] drm/msm: return the average load over the polling period
Quoted 8 lines · Chia-I Wu
simple_ondemandinteracts poorly withclamp_to_idle. It only looks at the load since the lastget_dev_statuscall, while it should really look at the load overpolling_ms. Whenclamp_to_idletrue, it almost always picks the lowest frequency on active because the gpu is idle between msm_devfreq_idle/msm_devfreq_active.This logic could potentially be moved into devfreq core.
The idea is to extend devfreq_simple_ondemand_data to specify whether devfreq_simple_ondemand_func should use "last status" or "average status" to determine the target frequency. devfreq core will need to store "struct devfreq_dev_status average_status", which will be updated when a device uses simple_ondemand and asks for average_status instead of last_status.
Quoted 116 lines
Fixes 7c0ffcd40b16 ("drm/msm/gpu: Respect PM QoS constraints")Signed-off-by Chia-I Wu <[email protected]>Cc Rob Clark <[email protected]>--- drivers/gpu/drm/msm/msm_gpu.h | 3 ++ drivers/gpu/drm/msm/msm_gpu_devfreq.c | 60 ++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-)drivers/gpu/drm/msm/msm_gpu.h +3
-0@@ -9,6 +9,7 @@#include <linux/adreno-smmu-priv.h> #include <linux/clk.h> +#include <linux/devfreq.h> #include <linux/interconnect.h> #include <linux/pm_opp.h> #include <linux/regulator/consumer.h> @@ -117,6 +118,8 @@ struct msm_gpu_devfreq {/** idle_time: Time of last transition to idle: */ ktime_t idle_time; +struct devfreq_dev_status average_status; +/** * idle_work: * drivers/gpu/drm/msm/msm_gpu_devfreq.c +59
-2@@ -9,6 +9,7 @@#include <linux/devfreq.h> #include <linux/devfreq_cooling.h> +#include <linux/math64.h> #include <linux/units.h> /* @@ -75,12 +76,69 @@ static void get_raw_dev_status(struct msm_gpu *gpu, status->busy_time = busy_time; } +static void update_average_dev_status (struct msm_gpu *gpu, +const struct devfreq_dev_status *raw) +{ +struct msm_gpu_devfreq *df = &gpu->devfreq; +const u32 polling_ms = df->devfreq->profile->polling_ms; +const u32 max_history_ms = polling_ms *11 /10 ; +struct devfreq_dev_status *avg = &df->average_status; +u64 avg_freq; + +/* simple_ondemand governor interacts poorly with gpu->clamp_to_idle. +* When we enforce the constraint on idle, it calls get_dev_status +* which would normally reset the stats. When we remove the +* constraint on active, it calls get_dev_status again where busy_time +* would be 0. +* +* To remedy this, we always return the average load over the past +* polling_ms. +*/ + +/* raw is longer than polling_ms or avg has no history */ +if (div_u64 (raw->total_time,USEC_PER_MSEC ) >= polling_ms || + !avg->total_time) { + *avg = *raw; +return ; + } + +/* Truncate the oldest history first. +* +* Because we keep the history with a single devfreq_dev_status, +* rather than a list of devfreq_dev_status, we have to assume freq +* and load are the same over avg->total_time. We can scale down +* avg->busy_time and avg->total_time by the same factor to drop +* history. +*/ +if (div_u64 (avg->total_time + raw->total_time,USEC_PER_MSEC ) >= + max_history_ms) { +const u32 new_total_time = polling_ms *USEC_PER_MSEC - + raw->total_time; + avg->busy_time =div_u64 ( +mul_u32_u32 (avg->busy_time, new_total_time), + avg->total_time); + avg->total_time = new_total_time; + } + +/* compute the average freq over avg->total_time + raw->total_time */ + avg_freq =mul_u32_u32 (avg->current_frequency, avg->total_time); + avg_freq +=mul_u32_u32 (raw->current_frequency, raw->total_time); +do_div (avg_freq, avg->total_time + raw->total_time); + + avg->current_frequency = avg_freq; + avg->busy_time += raw->busy_time; + avg->total_time += raw->total_time; +} +static int msm_devfreq_get_dev_status (struct device *dev,struct devfreq_dev_status *status) {struct msm_gpu *gpu =dev_to_gpu (dev); +struct devfreq_dev_status raw; -get_raw_dev_status (gpu, status); +get_raw_dev_status (gpu, &raw); +update_average_dev_status (gpu, &raw); + *status = gpu->devfreq.average_status;return 0 ; } --2.36.0.rc0.470.gd361397f0d-goog