Hi,
On Fri, Nov 19, 2021 at 2:47 PM Rob Clark <robdclark@xxxxxxxxx> 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
