Re: [Freedreno] [PATCH v2 2/3] drm: msm: a6xx: Add support for A618

[Jordan Crouse]

On Mon, Feb 03, 2020 at 04:40:40PM -0800, Rob Clark wrote:
> On Mon, Feb 3, 2020 at 4:21 PM John Stultz <john.stultz@xxxxxxxxxx> wrote:
> >
> > On Wed, Jan 22, 2020 at 11:19 PM Sharat Masetty <smasetty@xxxxxxxxxxxxxx> wrote:
> > >
> > > This patch adds support for enabling Graphics Bus Interface(GBIF)
> > > used in multiple A6xx series chipets. Also makes changes to the
> > > PDC/RSC sequencing specifically required for A618. This is needed
> > > for proper interfacing with RPMH.
> > >
> > > Signed-off-by: Sharat Masetty <smasetty@xxxxxxxxxxxxxx>
> > > ---
> > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > > index dc8ec2c..2ac9a51 100644
> > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> > > @@ -378,6 +378,18 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
> > >         struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> > >         int ret;
> > >
> > > +       /*
> > > +        * During a previous slumber, GBIF halt is asserted to ensure
> > > +        * no further transaction can go through GPU before GPU
> > > +        * headswitch is turned off.
> > > +        *
> > > +        * This halt is deasserted once headswitch goes off but
> > > +        * incase headswitch doesn't goes off clear GBIF halt
> > > +        * here to ensure GPU wake-up doesn't fail because of
> > > +        * halted GPU transactions.
> > > +        */
> > > +       gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0);
> > > +
> > >         /* Make sure the GMU keeps the GPU on while we set it up */
> > >         a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
> > >
> >
> > So I already brought this up on #freedreno but figured I'd follow up
> > on the list.
> >
> > With linus/master, I'm seeing hard crashes (into usb crash mode) with
> > the db845c, which I isolated down to this patch, and then to the chunk
> > above.
> 
> (repeating my speculation from #freedreno for benefit of those not on IRC)
> 
> I'm suspecting, that like the registers to take the GPU out of secure
> mode, this register is being blocked on LA devices (like db845c),
> which is why we didn't see this on cheza.
> 
> Maybe we can make this write conditional on whether we have a zap shader?

Sorry, I was WFH yesterday and didn't have IRC on.

The 845 doesn't have GBIF (it still uses VBIF) and on a AC enabled target large
chunks of unused register space would be blocked by default so Rob's hypothesis
is correct. Since the 845 is the only a6xx target that still has a VBIF a
!adreno_is_a630() check would do here, but I'm not 100% convinced we need this
code at all. We explicitly clear the GBIF halt in the stop function before the
headswitch is turned off so I think this is mostly unneeded paranoia.

I need to get a tree with the 618 code in it and I'll try to get a fix out
shortly.

Jordan

> > Dropping the gpu_write line above gets things booting again for me.
> >
> > Let me know if there are any follow on patches I can help validate.
> >
> > thanks
> > -john
> > _______________________________________________
> > Freedreno mailing list
> > Freedreno@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/freedreno
> _______________________________________________
> Freedreno mailing list
> Freedreno@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/freedreno

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