Hi,
On Fri, Nov 19, 2021 at 2:47 PM Rob Clark robdclark@gmail.com wrote:
+void msm_devfreq_boost(struct msm_gpu *gpu, unsigned factor) +{
struct msm_gpu_devfreq *df = &gpu->devfreq;unsigned long freq;freq = get_freq(gpu);freq *= factor;freq /= HZ_PER_KHZ;
Should it do the divide first? I don't know for sure, but it feels like GPU frequency could conceivably be near-ish the u32 overflow? (~4 GHz). Better to be safe and do the / 1000 first?
@@ -201,26 +217,14 @@ static void msm_devfreq_idle_work(struct kthread_work *work) struct msm_gpu_devfreq *df = container_of(work, struct msm_gpu_devfreq, idle_work.work); struct msm_gpu *gpu = container_of(df, struct msm_gpu, devfreq);
unsigned long idle_freq, target_freq = 0; if (!df->devfreq) return;
Why does the msm_devfreq_idle_work() need a check for "!df->devfreq" but the boost work doesn't? Maybe you don't need it anymore now that you're not reaching into the mutex? ...or maybe the boost work does need it?
...and if "df->devfreq" is NULL then doesn't it mean that msm_hrtimer_work_init() was never called? That seems bad...
-Doug