[Freedreno] [PATCH 13/16] drm/msm: gpu: Add support for the GPMU

[Jordan Crouse]

On Mon, Nov 07, 2016 at 02:58:01PM +0200, Stanimir Varbanov wrote:
> Hi Jordan,
> 
> On 11/05/2016 12:44 AM, Jordan Crouse wrote:
> > Most 5XX targets have GPMU (Graphics Power Management Unit) that
> > handles a lot of the heavy lifting for power management including
> > thermal and limits management and dynamic power collapse. While
> > the GPMU itself is optional, it is usually nessesary to hit
> > aggressive power targets.
> > 
> > The GPMU firmware needs to be loaded into the GPMU at init time via a
> > shared hardware block of registers. Using the GPU to write the microcode
> > is more efficient than using the CPU so at first load create an indirect
> > buffer that can be executed during subsequent initalization sequences.
> > 
> > After loading the GPMU gets initalized through a shared register
> > interface and then we mostly get out of its way and let it do
> > its thing.
> > 
> > Signed-off-by: Jordan Crouse <jcrouse at codeaurora.org>
> > ---
> >  drivers/gpu/drm/msm/Makefile               |   1 +
> >  drivers/gpu/drm/msm/adreno/a5xx_gpu.c      |  64 +++++-
> >  drivers/gpu/drm/msm/adreno/a5xx_gpu.h      |  23 ++
> >  drivers/gpu/drm/msm/adreno/a5xx_power.c    | 341 +++++++++++++++++++++++++++++
> >  drivers/gpu/drm/msm/adreno/adreno_device.c |   1 +
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.h    |   1 +
> >  6 files changed, 428 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/gpu/drm/msm/adreno/a5xx_power.c
> > 
> 
> <cut>
> 
> > +/* Enable the GPMU microcontroller */
> > +static int a5xx_gpmu_init(struct msm_gpu *gpu)
> > +{
> > +	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> > +	struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu);
> > +	struct msm_ringbuffer *ring = gpu->rb;
> > +
> > +	if (!a5xx_gpu->gpmu_dwords)
> > +		return 0;
> > +
> > +	/* Turn off protected mode for this operation */
> > +	OUT_PKT7(ring, CP_SET_PROTECTED_MODE, 1);
> 
> This looks wrong, shouldn't it be?
> 
> OUT_PKT7(ring, CP_SET_PROTECTED_MODE, 0)

I admit this syntax can be really confusing.  Opcode headers consist of a
opcode and a payload size (and some other stuff you don't need to worry
about). So this line is declaring opcode CP_SET_PROTETED_MODE with a payload
length of 1 dword.  The payload is either 0 (disable protected mode) or 1
(enable protected mode):

> > +	OUT_RING(ring, 0);

Here we disable.

> > +
> > +	/* Kick off the IB to load the GPMU microcode */
> > +	OUT_PKT7(ring, CP_INDIRECT_BUFFER_PFE, 3);
> > +	OUT_RING(ring, lower_32_bits(a5xx_gpu->gpmu_iova));
> > +	OUT_RING(ring, upper_32_bits(a5xx_gpu->gpmu_iova));
> > +	OUT_RING(ring, a5xx_gpu->gpmu_dwords);
> > +
> > +	/* Turn back on protected mode */
> > +	OUT_PKT7(ring, CP_SET_PROTECTED_MODE, 1);
> > +	OUT_RING(ring, 1);

And here we re-enable.

Downstream uses inline functions for a lot of the common CP functions to avoid
confusion like this - the problem with those is that you need to pass in the GPU
and do a bunch of logic to get the right opcode for pre-A5XX and post-A5XX and I
don't generally like those when you can just directly go to the ring with a
minimum of fuss.

Jordan
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project