| Message ID | 20191118234043.331542-1-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | bd3fe8119dea46b850b78283b426afaa3cff6a1f |
| Headers | show |
| Series | drm/msm/a6xx: restore previous freq on resume | expand |
On Mon, Nov 18, 2019 at 03:40:38PM -0800, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > Previously, if the freq were overriden (ie. via sysfs), it would get > reset to max on resume. Devfreq goes to sleep assuming that the hardware will still be at the same frequency when it wakes up but the GMU sneaks out in the middle of the night and takes the hardware for a joyride. Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org> > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 8 ++++++-- > drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 3 +++ > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 2ca470eb5cb8..b64867701e5a 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -149,6 +149,8 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) > if (freq == gmu->gpu_freqs[perf_index]) > break; > > + gmu->current_perf_index = perf_index; > + > __a6xx_gmu_set_freq(gmu, perf_index); > } > > @@ -741,8 +743,8 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) > gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_MASK, ~A6XX_HFI_IRQ_MASK); > enable_irq(gmu->hfi_irq); > > - /* Set the GPU to the highest power frequency */ > - __a6xx_gmu_set_freq(gmu, gmu->nr_gpu_freqs - 1); > + /* Set the GPU to the current freq */ > + __a6xx_gmu_set_freq(gmu, gmu->current_perf_index); > > /* > * "enable" the GX power domain which won't actually do anything but it > @@ -1166,6 +1168,8 @@ static int a6xx_gmu_pwrlevels_probe(struct a6xx_gmu *gmu) > gmu->nr_gpu_freqs = a6xx_gmu_build_freq_table(&gpu->pdev->dev, > gmu->gpu_freqs, ARRAY_SIZE(gmu->gpu_freqs)); > > + gmu->current_perf_index = gmu->nr_gpu_freqs - 1; > + > /* Build the list of RPMh votes that we'll send to the GMU */ > return a6xx_gmu_rpmh_votes_init(gmu); > } > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > index 39a26dd63674..2af91ed7ed0c 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > @@ -63,6 +63,9 @@ struct a6xx_gmu { > struct clk_bulk_data *clocks; > struct clk *core_clk; > > + /* current performance index set externally */ > + int current_perf_index; > + > int nr_gpu_freqs; > unsigned long gpu_freqs[16]; > u32 gx_arc_votes[16]; > -- > 2.23.0 >
Quoting Rob Clark (2019-11-18 15:40:38) > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > index 39a26dd63674..2af91ed7ed0c 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > @@ -63,6 +63,9 @@ struct a6xx_gmu { > struct clk_bulk_data *clocks; > struct clk *core_clk; > > + /* current performance index set externally */ > + int current_perf_index; > + Is there a reason this isn't unsigned? It looks like __a6xx_gmu_set_freq() takes an int, but maybe it should take a u16 or something?
On Mon, Nov 18, 2019 at 4:32 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Rob Clark (2019-11-18 15:40:38) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > > index 39a26dd63674..2af91ed7ed0c 100644 > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > > @@ -63,6 +63,9 @@ struct a6xx_gmu { > > struct clk_bulk_data *clocks; > > struct clk *core_clk; > > > > + /* current performance index set externally */ > > + int current_perf_index; > > + > > Is there a reason this isn't unsigned? It looks like > __a6xx_gmu_set_freq() takes an int, but maybe it should take a u16 or > something? > no particular reason, other than other things where already using an int.. this is just an index into the table of opp's so it is never going to be a large int. Depending on GMU_DCVS_PERF_SETTING it could probably be a u8 (I'm not *entirely* sure from the code how large that bitfield is) BR, -R
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 2ca470eb5cb8..b64867701e5a 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -149,6 +149,8 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) if (freq == gmu->gpu_freqs[perf_index]) break; + gmu->current_perf_index = perf_index; + __a6xx_gmu_set_freq(gmu, perf_index); } @@ -741,8 +743,8 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_MASK, ~A6XX_HFI_IRQ_MASK); enable_irq(gmu->hfi_irq); - /* Set the GPU to the highest power frequency */ - __a6xx_gmu_set_freq(gmu, gmu->nr_gpu_freqs - 1); + /* Set the GPU to the current freq */ + __a6xx_gmu_set_freq(gmu, gmu->current_perf_index); /* * "enable" the GX power domain which won't actually do anything but it @@ -1166,6 +1168,8 @@ static int a6xx_gmu_pwrlevels_probe(struct a6xx_gmu *gmu) gmu->nr_gpu_freqs = a6xx_gmu_build_freq_table(&gpu->pdev->dev, gmu->gpu_freqs, ARRAY_SIZE(gmu->gpu_freqs)); + gmu->current_perf_index = gmu->nr_gpu_freqs - 1; + /* Build the list of RPMh votes that we'll send to the GMU */ return a6xx_gmu_rpmh_votes_init(gmu); } diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h index 39a26dd63674..2af91ed7ed0c 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h @@ -63,6 +63,9 @@ struct a6xx_gmu { struct clk_bulk_data *clocks; struct clk *core_clk; + /* current performance index set externally */ + int current_perf_index; + int nr_gpu_freqs; unsigned long gpu_freqs[16]; u32 gx_arc_votes[16];